/core

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

Improve docs for Json.(En|De)code #243

Merged
merged 3 commits into from May 27, 2015

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mgold @evancz
@mgold
Contributor

mgold commented May 12, 2015

Improve docs for the Json libraries, most notably

  • Rename a type alias Task that has nothing to do with 0.15 Tasks
  • Give an example use for Decode.tuple1
  • Fix assorted errors, clarify wording, add information
  • Give a brief introduction to what a Decoder a is
  • Note that Infinity and NaN serialize to null (which means encode and decode aren't quite inverses)

Hopefully an easy merge - if there's anything odd, please let me know.

mgold added some commits May 11, 2015

@mgold
Improve docs for Json.(En|De)code 
Improve docs for the Json libraries, most notably
* Rename a type alias Task that has nothing to do with 0.15 Tasks
* Give an example use for `Decode.tuple1`
* Fix assorted errors, clarify wording, add information
* Give a brief introduction to what a `Decoder a` is
* Note that Infinity and NaN serialize to null (which means encode and decode aren't quite inverses)

Hopefully an easy merge - if there's anything odd, please let me know.
dba0fee
@mgold
Signal.Mailbox is not a function; lowercase the m
ffd137c
@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 27, 2015

Contributor

Rebased on to master and added a second commit fixing Graphic.Input's docs - previous they invoked Signal.Mailbox as a function when it should be Signal.mailbox, lowercase. CI should pass.
Contributor

mgold commented May 27, 2015

Rebased on to master and added a second commit fixing Graphic.Input's docs - previous they invoked Signal.Mailbox as a function when it should be Signal.mailbox, lowercase. CI should pass.
@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 27, 2015

Contributor

Closing and reopening to force CI to rebuild; it seems to be a flaky download.
Contributor

mgold commented May 27, 2015

Closing and reopening to force CI to rebuild; it seems to be a flaky download.

@mgold mgold closed this May 27, 2015

@mgold mgold reopened this May 27, 2015

Show outdated Hide outdated src/Json/Decode.elm
{-| Great for handling optional fields. The following code decodes JSON
objects that may not have a profession field.
{-| Extract a Maybe value, wrapping successes with `Just` and turning any
failure in `Nothing`. Great for handling fields that may be missing or null. The

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 27, 2015

Member

This is actually not that great for things that may be null because it actually admits all sorts of errors. A decoder that can be null or something, is different than a decoder that is something or anything. If the given decoder messes up, it'll give Nothing (corresponding to null which it is not) rather than failing.
Copy URL
@evancz

evancz May 27, 2015

Member

This is actually not that great for things that may be null because it actually admits all sorts of errors. A decoder that can be null or something, is different than a decoder that is something or anything. If the given decoder messes up, it'll give Nothing (corresponding to null which it is not) rather than failing.

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 27, 2015

Contributor

That's a valid concern and I'll revisit this when I get a chance.
Copy URL
@mgold

mgold May 27, 2015

Contributor

That's a valid concern and I'll revisit this when I get a chance.
src/Json/Encode.elm
@@ -55,6 +55,8 @@ int =
Native.Json.identity
{-| Encode a Float. `Infinity` and `NaN` are encoded as `null`.
-}

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 27, 2015

Member

Is this true? This seems incorrect.
Copy URL
@evancz

evancz May 27, 2015

Member

Is this true? This seems incorrect.

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 27, 2015

Contributor 
> import Json.Encode as E
> E.encode 0 (E.float (1/0))
"null" : String
> E.encode 0 (E.float (0/0))
"null" : String

Personally I'm fine with this behavior. I like how encoding always succeeds.
Copy URL
@mgold

mgold May 27, 2015

Contributor 
> import Json.Encode as E
> E.encode 0 (E.float (1/0))
"null" : String
> E.encode 0 (E.float (0/0))
"null" : String

Personally I'm fine with this behavior. I like how encoding always succeeds.

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 27, 2015

Member

Shouldn't Elm's floats map on to the same floats though? Like, why would NaN not result in Nan? Is that not valid JSON or something?
Copy URL
@evancz

evancz May 27, 2015

Member

Shouldn't Elm's floats map on to the same floats though? Like, why would NaN not result in Nan? Is that not valid JSON or something?

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 27, 2015

Contributor

Numeric values that cannot be represented as sequences of digits (such as Infinity and NaN) are not permitted. [page 9]

$ node
> JSON.stringify(NaN)
'null'
> JSON.stringify(Infinity)
'null'
Copy URL
@mgold

mgold May 27, 2015

Contributor

Numeric values that cannot be represented as sequences of digits (such as Infinity and NaN) are not permitted. [page 9]

$ node
> JSON.stringify(NaN)
'null'
> JSON.stringify(Infinity)
'null'
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 27, 2015

Member

Thanks! Besides the two comments, this looks really great!
Member

evancz commented May 27, 2015

Thanks! Besides the two comments, this looks really great!
@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 27, 2015

Contributor

New commit should address the null issue.
Contributor

mgold commented May 27, 2015

New commit should address the null issue.

evancz pushed a commit that referenced this pull request May 27, 2015

@process-bot
Merge pull request #243 from mgold/docs-refinements 
Improve docs for Json.(En|De)code
5ee7261

@evancz evancz merged commit 5ee7261 into elm:master May 27, 2015
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mgold mgold deleted the mgold:docs-refinements branch May 28, 2015

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold May 28, 2015

Contributor

Thanks!
Contributor

mgold commented May 28, 2015

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