Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat JSON null as empty value #82

Merged
merged 1 commit into from Aug 29, 2014

Conversation

Projects
None yet
2 participants
@wicz
Copy link
Contributor

commented Aug 28, 2014

I need to have JSON null literal values normalised to empty strings.

I realise the JSON specification also allows true and false; thus this specific implementation for null MAY set a precedent for the other two. Should we normalise those as strings "true" or "false". If so, why not set null to "null" as well?

Anyway, I just want to bring up the discussion based on my personal needs. Ultimately, it all depends on how much opinionated you want PapaParse to be. Feel free to close this PR without merging in case you don't agree with the solution.

Thanks for your great work here! 馃憤

@mholt

This comment has been minimized.

Copy link
Owner

commented Aug 28, 2014

I like where this is going. The problem is knowing whether the literal value should be the word "null" or to interpret it as an empty string. Currently we do a type conversion from string to number or boolean when dynamicTyping is enabled. To stay consistent, perhaps we should refrain from interpreting null as empty string except when dynamicTyping is enabled.

To do that instead, just add a condition in this block to check for the string "null" and then change it to empty string.

Then just update the test to turn on dynamicTyping.

Does that sound like a good way to do it?

@mholt mholt added the enhancement label Aug 28, 2014

@wicz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2014

Hello @mholt, actually I'm referring to the unparse method. Sorry for not making that clear. When I try to unparse a JSON with a null value, PapaParse raises a TypeError: str is null exception.

Regarding parse, your suggestion would definitely work, unfortunately I don't have the time to implement this now.

@mholt

This comment has been minimized.

Copy link
Owner

commented Aug 29, 2014

@wicz Ah! Right, I should have expanded the diff to see that your change was in the unparse utility. This looks good, I'm gonna merge it in and also add in what I was describing above.

I'm almost done with version 3.1 anyway, so this will make a nice addition. Thanks!

mholt added a commit that referenced this pull request Aug 29, 2014

Merge pull request #82 from wicz/handle-json-null
Treat JSON null as empty value in unparse

@mholt mholt merged commit a0991ad into mholt:master Aug 29, 2014

@wicz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2014

Awesome, thanks!

@wicz wicz deleted the wicz:handle-json-null branch Aug 29, 2014

@mholt

This comment has been minimized.

Copy link
Owner

commented Aug 29, 2014

No problem. Actually, now that I think about what I suggested above a little more, I'm not sure that changing the string "null" into an empty string (with dynamicTyping on) is desired. I'm going to leave things as they are with your change and see if anyone has any gripes. Hopefully not. 馃槃 Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.