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

Updated json to correctly handle double and float nan #860

Merged
merged 1 commit into from Oct 9, 2014

Conversation

Projects
None yet
3 participants
@dmonagle
Contributor

dmonagle commented Oct 9, 2014

Previously a double nan was serializing to the string "nan" which is invalid JSON.

Updated to use undefined instead of nan for both serializing and deserializing.

Rationale:

ECMA-262 states "The undefined value is [...] used when a variable has not been assigned a value", while "The null value [...] represents the null, empty or non-existent reference."

While I was at it, cleaned up incorrect grammar and spelling mistakes.

Updated json to correctly handle double and float nan
Previously a double nan was serializing to the string "nan" which is invalid JSON.

Updated to use undefined instead of nan for both serializing and deserializing.

Rationale:

ECMA-262 states "The undefined value is [...] used when a variable has not been assigned a value", while "The null value [...] represents the null, empty or non-existent reference."

While I was at it, cleaned up incorrect grammar and spelling mistakes.
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Oct 9, 2014

Member

Looks good, thanks. BTW, I've not fixed most of the issues found while developing the aspiring std.json successor, because my hope is to be able to switch to the new implementation soon.

Member

s-ludwig commented Oct 9, 2014

Looks good, thanks. BTW, I've not fixed most of the issues found while developing the aspiring std.json successor, because my hope is to be able to switch to the new implementation soon.

s-ludwig added a commit that referenced this pull request Oct 9, 2014

Merge pull request #860 from intrica/fix-json-nan
Updated json to correctly handle double and float nan

@s-ludwig s-ludwig merged commit 1d3e9e0 into vibe-d:master Oct 9, 2014

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@dmonagle

dmonagle Oct 9, 2014

Contributor

Thanks. std_data_json looks interesting. I wasn't aware of it previously. After a quick look through the source, it looks quite a bit different from what you've built with Vibe.d. Do you intend for it to work with the serialisation build into vibe.d?

Where will this leave the state of the Vibe.d BSON library? It seems that it was once almost identical to the JSON code but I think the JSON code has improved over time whilst the BSON has remained fairly static.

Contributor

dmonagle commented Oct 9, 2014

Thanks. std_data_json looks interesting. I wasn't aware of it previously. After a quick look through the source, it looks quite a bit different from what you've built with Vibe.d. Do you intend for it to work with the serialisation build into vibe.d?

Where will this leave the state of the Vibe.d BSON library? It seems that it was once almost identical to the JSON code but I think the JSON code has improved over time whilst the BSON has remained fairly static.

@yazd

This comment has been minimized.

Show comment
Hide comment
@yazd

yazd Nov 5, 2015

Contributor

I think nan should be serialized as null instead of undefined. undefined is invalid JSON AFAIK.

Contributor

yazd commented Nov 5, 2015

I think nan should be serialized as null instead of undefined. undefined is invalid JSON AFAIK.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Nov 5, 2015

Member

You are right (see also #958). I was hoping that I could avoid most work on vibe.data.json in favor of stdx.data.json, but this should be fixed. This could also be made configurable like for stdx.data.json.

Member

s-ludwig commented Nov 5, 2015

You are right (see also #958). I was hoping that I could avoid most work on vibe.data.json in favor of stdx.data.json, but this should be fixed. This could also be made configurable like for stdx.data.json.

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