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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON5.stringify should avoid quoting keys where appropriate #32

Closed
aeisenberg opened this issue Nov 1, 2012 · 13 comments
Closed

JSON5.stringify should avoid quoting keys where appropriate #32

aeisenberg opened this issue Nov 1, 2012 · 13 comments

Comments

@aeisenberg
Copy link
Collaborator

Currently, JSON5.stringify just delegates to JSON.stringify. The result does not seem very JSON5 to me. It makes sense that strings produced by JSON5 should be the shortest string that JSON5 can still parse and produce the original object.

So, one enhancement would be to remove quotes around keys that are already valid JS identifiers.

Since this would break compatibility with the builtin JSON object, perhaps a new function could be created (eg- stringify5).

@aseemk
Copy link
Member

aseemk commented Nov 2, 2012

Hey @aeisenberg, thanks for the suggestion. The current behavior of outputting regular/strict JSON with stringify() was actually a purposeful design decision: the output is then readable by existing JSON parsers.

And indeed, much of the feedback after release was around the importance of maintaining interoperability to help adoption. We're breaking it with parse(), so it'll help to preserve it with stringify().

It also gels with JSON5's purpose: to help humans write JSON (by hand) more easily. If you're outputting JSON programmatically, it's probably worth adding a bit of verbosity in order to maintain interoperability.

That said, it might be nice to implement what you suggest as an extra, to help with things like conversion tools. I probably won't be able to get to this anytime soon, so if you want to take a stab, a pull request would be welcome!

@aseemk aseemk closed this as completed Nov 2, 2012
@aseemk aseemk reopened this Nov 2, 2012
@aeisenberg
Copy link
Collaborator Author

I'll see what I can do.

@sindresorhus
Copy link

I was hit by this too. I just assumed it supported outputting the same format that it reads...

My use-case is having user editable config files which can also be modified programatically. Which means I would need to be able both read/write JSON5. Going with YAML instead.

@dpashkevich
Copy link

+1 on this, there should be an option to programmatically output human-friendly JSON5

@aseemk
Copy link
Member

aseemk commented Dec 11, 2012

Okay guys, I'm convinced -- supporting stringifying to "clean" JSON5 sounds reasonable. The only question is, should that be the default behavior of JSON5.stringify()? Or should it be an option?

(Maybe overload the space option -- if any given, pretty-print, but if not, use the default JSON stringify?)

I can be convinced to make stringifying to JSON5 the default, since you can always use the regular/native JSON.stringify() if you want regular JSON.

I'm busy at the moment, so if anyone wants to take a stab, pull requests welcome!

@dpashkevich
Copy link

I think the sentence JSON5.stringify() should pretty much do what it
says: that is stringify JSON5. So I vote for human-friendly output by
default, that's what JSON5 is about anyway.

Dmitry Pashkevich
On Dec 11, 2012 10:37 PM, "Aseem Kishore" notifications@github.com wrote:

Okay guys, I'm convinced -- supporting stringifying to "clean" JSON5
sounds reasonable. The only question is, should that be the default
behavior of JSON5.stringify()? Or should it be an option?

(Maybe overload the space option -- if any given, pretty-print, but if
not, use the default JSON stringify?)

I can be convinced to make stringifying to JSON5 the default, since you
can always use the regular/native JSON.stringify() if you want regular
JSON.

I'm busy at the moment, so if anyone wants to take a stab, pull requests
welcome!


Reply to this email directly or view it on GitHubhttps://github.com//issues/32#issuecomment-11256181.

@sindresorhus
Copy link

^ +1

@aseemk
Copy link
Member

aseemk commented Dec 12, 2012

Sounds reasonable!

@aeisenberg
Copy link
Collaborator Author

I've had Issue #35 open for a while with a pull request, but I haven't had any feedback on it.

@aseemk
Copy link
Member

aseemk commented Dec 28, 2012

Sorry Andrew, I've been at home for a family emergency and haven't had any
time or energy to look through your pull req yet, but I appreciate it and
hope to get to it in the new year. Hope you're having a great holiday, and
happy new year. =)

@aeisenberg
Copy link
Collaborator Author

No problem. Hope things work out for you. Happy holidays as well!

On Fri, Dec 28, 2012 at 12:00 PM, Aseem Kishore notifications@github.comwrote:

Sorry Andrew, I've been at home for a family emergency and haven't had any
time or energy to look through your pull req yet, but I appreciate it and
hope to get to it in the new year. Hope you're having a great holiday, and
happy new year. =)


Reply to this email directly or view it on GitHubhttps://github.com//issues/32#issuecomment-11740186.

@aseemk
Copy link
Member

aseemk commented Sep 14, 2013

Hey folks! I've granted @aeisenberg contributor rights to merge his pull request whenever it's deemed ready, so please help both of us out by taking a look at it and giving us your feedback!

#35

And while I have your guys' attention, consider joining our new JSON5 google group to stay up-to-date on new releases, announcements, and have a place to discuss new ideas.

https://groups.google.com/group/json5

Exciting times. =)

@aeisenberg
Copy link
Collaborator Author

Well, this is now available in the 0.3.0 release.

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

No branches or pull requests

4 participants