New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More thorough documancy for Platform.program because it's complex and… #757

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@Pilatch

Pilatch commented Nov 21, 2016

… confusing, and other sources of information don't paint a clear enough picture for beginners.

I spent a long time trying to figure this out, and nothing official was helping. This is documentation from a beginner, for beginners. It spells out what's going on in more gory detail to spare others pain of integrating Elm into arbitrary JavaScript.

Ethan Martin Ethan Martin
More thorough documancy for Platform.program because it's complex and…
… confusing, and other sources of information don't paint a clear enough picture for beginners.
@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Nov 21, 2016

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Nov 21, 2016

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@lukewestby lukewestby added the docs label Nov 21, 2016

@richardhaven

"The commad should be generated by passing data through an outbound port."
This is a bit confusing for beginners itself. Also, note the typo.
Perhaps just repeat the bit from init about it's usually Cmd.none

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 21, 2016

Working on making it better. I should have an update soon.

Pilatch commented Nov 21, 2016

Working on making it better. I should have an update soon.

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 21, 2016

How do I get this thing to re-run the tests without making another commit?

Pilatch commented Nov 21, 2016

How do I get this thing to re-run the tests without making another commit?

@OvermindDL1

This comment has been minimized.

Show comment
Hide comment
@OvermindDL1

OvermindDL1 Nov 21, 2016

How do I get this thing to re-run the tests without making another commit?

An owner of this repo can restart it, but as a user I think the only way is to just close then re-open this pull request (not open a new one, just re-open the same old one).

OvermindDL1 commented Nov 21, 2016

How do I get this thing to re-run the tests without making another commit?

An owner of this repo can restart it, but as a user I think the only way is to just close then re-open this pull request (not open a new one, just re-open the same old one).

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 21, 2016

Cool. I'll try that.

Pilatch commented Nov 21, 2016

Cool. I'll try that.

@Pilatch Pilatch closed this Nov 21, 2016

@Pilatch Pilatch reopened this Nov 21, 2016

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 21, 2016

Neato. It's doing it.

Pilatch commented Nov 21, 2016

Neato. It's doing it.

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 21, 2016

Trying one last time just in case the test failure is intermittent.

Pilatch commented Nov 21, 2016

Trying one last time just in case the test failure is intermittent.

@Pilatch Pilatch closed this Nov 21, 2016

@Pilatch Pilatch reopened this Nov 21, 2016

@richardhaven

This comment has been minimized.

Show comment
Hide comment
@richardhaven

richardhaven Nov 21, 2016

I am a detailed documentation editor. I'm happy to review stuff

richardhaven commented Nov 21, 2016

I am a detailed documentation editor. I'm happy to review stuff

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 21, 2016

Yea, a review would be great!

Pilatch commented Nov 21, 2016

Yea, a review would be great!

@richardhaven

Maybe documenting the plumbing and tips on how to use subscriptions is too much

Show outdated Hide outdated src/Platform.elm
The second argument, `update` is a function that takes these two arguments:
1. A message from the outside world, that has the information your code will act upon.

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

remove comma before "that"

@richardhaven

richardhaven Nov 21, 2016

remove comma before "that"

This comment has been minimized.

@Pilatch
@Pilatch
Show outdated Hide outdated src/Platform.elm
1. A message from the outside world, that has the information your code will act upon.
Usually this is a `Msg` type that you specify, (a constructor), which takes a record type
matching the data coming from an incoming port. Unpack that data with `case` ... `of`.

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

give a simple example of the Msg object (e.g. how to create one)

@richardhaven

richardhaven Nov 21, 2016

give a simple example of the Msg object (e.g. how to create one)

This comment has been minimized.

Show outdated Hide outdated src/Platform.elm
Usually this is a `Msg` type that you specify, (a constructor), which takes a record type
matching the data coming from an incoming port. Unpack that data with `case` ... `of`.
2. The current state of your program's model, as maintained by the Elm runtime.

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

"model" is a variable in this application. It's not really "maintained by the Elm runtime

@richardhaven

richardhaven Nov 21, 2016

"model" is a variable in this application. It's not really "maintained by the Elm runtime

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

I just deleted the back half of that sentence.

@Pilatch

Pilatch Nov 22, 2016

I just deleted the back half of that sentence.

Show outdated Hide outdated src/Platform.elm
2. The current state of your program's model, as maintained by the Elm runtime.
The `update` function must return a [Tuple](Tuple)
containing the modified form of your program's model, and a command.

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

The "command" gets fed back into the update unless it is Cmd.none

@richardhaven

richardhaven Nov 21, 2016

The "command" gets fed back into the update unless it is Cmd.none

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

This does not appear to be true. I've been debugging a program here - https://github.com/Pilatch/elm-port-program-examples/blob/master/greeter/Greet.elm - and can't find any evidence to show that update is getting called multiple times when I supply a non-none command in the second half of the tuple that's returned from update.

Here I have the result of debugging the result of a Cmd.map:

{ type = "map", tagger = <function>, tree = { type = "leaf", home = "greet", value = "jomomma" } }

Here is the result of debugging Cmd.none:

{ type = "node", branches = [] }

They are significantly different, yet in both cases the update function is not run again, with the supplied message coming from Cmd.map.

@Pilatch

Pilatch Nov 22, 2016

This does not appear to be true. I've been debugging a program here - https://github.com/Pilatch/elm-port-program-examples/blob/master/greeter/Greet.elm - and can't find any evidence to show that update is getting called multiple times when I supply a non-none command in the second half of the tuple that's returned from update.

Here I have the result of debugging the result of a Cmd.map:

{ type = "map", tagger = <function>, tree = { type = "leaf", home = "greet", value = "jomomma" } }

Here is the result of debugging Cmd.none:

{ type = "node", branches = [] }

They are significantly different, yet in both cases the update function is not run again, with the supplied message coming from Cmd.map.

Show outdated Hide outdated src/Platform.elm
let newModel =
model.greetings + 1 |> Model
greeting = case msg of

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

single-case is a bad code example. Use "if" to check for a single case

@richardhaven

richardhaven Nov 21, 2016

single-case is a bad code example. Use "if" to check for a single case

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

How do you extract a message's payload with an if statement?

@Pilatch

Pilatch Nov 22, 2016

How do you extract a message's payload with an if statement?

Show outdated Hide outdated src/Platform.elm
The `update` function must return a [Tuple](Tuple)
containing the modified form of your program's model, and a command.
Generate that command by passing data to an outbound port. The data type

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

The command can be created by passing data to a port, but that is a specific (if common) case. Separate that from this quick overview

@richardhaven

richardhaven Nov 21, 2016

The command can be created by passing data to a port, but that is a specific (if common) case. Separate that from this quick overview

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

The code sample that follows that text illustrates that common use case, so I think it's safe to just delete the text in question.

@Pilatch

Pilatch Nov 22, 2016

The code sample that follows that text illustrates that common use case, so I think it's safe to just delete the text in question.

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

Thought about it. The common use case is important in the demystification process. Had I deleted text as I commented above, then my documentation would be no better than the type annotations.

Nevertheless I have taken the suggestion into account, reworded, and separated the common use case into a new paragraph.

@Pilatch

Pilatch Nov 22, 2016

Thought about it. The common use case is important in the demystification process. Had I deleted text as I commented above, then my documentation would be no better than the type annotations.

Nevertheless I have taken the suggestion into account, reworded, and separated the common use case into a new paragraph.

Show outdated Hide outdated src/Platform.elm
type Msg = Name String
type alias Model = { greetings : Int }

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

where is model = Model(0)

@richardhaven

richardhaven Nov 21, 2016

where is model = Model(0)

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

If I include that, then I have an init function, and may as well have a subscriptions function, then may as well have the Process.program call for completeness.

@Pilatch

Pilatch Nov 22, 2016

If I include that, then I have an init function, and may as well have a subscriptions function, then may as well have the Process.program call for completeness.

Show outdated Hide outdated src/Platform.elm
type alias Model =
{ numChanges : Int}
-- Assume the update function increments `numChanges`.

This comment has been minimized.

@richardhaven

richardhaven Nov 21, 2016

where is model = Model(0) ?

@richardhaven

richardhaven Nov 21, 2016

where is model = Model(0) ?

This comment has been minimized.

@Pilatch

Pilatch Nov 22, 2016

Went the completeness route, and put one big code sample in there.

@Pilatch

Pilatch Nov 22, 2016

Went the completeness route, and put one big code sample in there.

@Pilatch

This comment has been minimized.

Show comment
Hide comment
@Pilatch

Pilatch Nov 22, 2016

Regarding plumbing and tips for subscriptions, I'll have to look into when/how frequently subscriptions is called, then I'll better reform that section.

Pilatch commented Nov 22, 2016

Regarding plumbing and tips for subscriptions, I'll have to look into when/how frequently subscriptions is called, then I'll better reform that section.

@Pilatch Pilatch closed this Dec 15, 2016

@Pilatch Pilatch reopened this Dec 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment