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

Rename to something other than JSON5, to not be incorrect or misleading #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rename to something other than JSON5, to not be incorrect or misleading #2

wants to merge 1 commit into from

Conversation

vassudanagunta
Copy link

@vassudanagunta vassudanagunta commented May 1, 2020

JSON5 is a specification. Anything carrying that name should adhere to the spec. In addition, the name JSON5 has special meaning, which is not true of this fork. As I stated in my suggested changes to this fork's Into:

JSON5 is designed to be valid ES5 Javascript (hence the name), as its maintainer points out:

JSON5 indicates a specific, reliable target—ES5. JSON5 syntax will never go beyond the syntax of ES5. Sure, that means it will miss out newer features, but it also means that users don't need to worry about what version of JSONX is supported.

This fork does not maintain this guarantee. Though it includes support for ES6-style multiline string template literals, it shouldn't be named JSON6 either because it includes other syntax that is not valid ES6.

As your own Pull Request Template says:

If you are submitting a bug fix for an an error or fixing an incompatibility
with the [official JSON5 specification][spec], please continue.

While it is compatible in functionality (being a superset), it is not compatible in name. It needs a different name, for the same reason that JSON5 is "JSON5" not "JSON", for the same reasons jsonc, Hjson, and YAML have their own names.

NOTE: this PR is an incomplete change made to raise the issue (since Issues have been disabled for this repo).

The new name itself is just an example. The fork's author should of course pick a name he likes. Other files in the repo will need to change to update the name change as well.

…ding

JSON5 is a specification. Anything carrying that name should adhere to the spec. In addition, the name JSON5 has special meaning, which is not true of this fork. As I stated in my suggested changes to this fork's Into:
JSON5 is designed to be valid ES5 Javascript (hence the name), [as its maintainer points out](json5#190 (comment)):
> JSON5 indicates a specific, reliable target—ES5. JSON5 syntax will never go beyond the syntax of ES5. Sure, that means it will miss out newer features, but it also means that users don't need to worry about what version of JSONX is supported.

JSON-X doesn't maintain this guarantee. Though it includes support for ES6-style multiline string template literals,
it's not named JSON6 because it includes other syntax that is not valid ES6.
@vassudanagunta
Copy link
Author

@jordanbtucker, @aseemk would you agree?

@vassudanagunta vassudanagunta changed the title Rename to something other than JSON5, as that is incorrect and mislea… Rename to something other than JSON5, to not be incorrect or misleading May 1, 2020
@jordanbtucker
Copy link

@vassudanagunta Thanks for bringing this to my attention. I do agree with you. JSON5 has an official spec. Any documents that do not comply with the spec are not JSON5 documents. In fact I just had a conversation about this with someone the other day. Here's how that went:

How would you feel if certain implementations did support number separators as valid grammar?

Feel free to extend JSON5, however if you do, please don't call it JSON5 anymore. If your library can parse JSON5-compliant documents, that is fine to promote that. You could say, for example, that your library is backward compatible with JSON5.

However, if your library can generate documents not compliant with JSON5, then please don't call those documents JSON5.

@GerHobbelt I think it's cool that you are extending JSON5, and I'm totally fine with that, but please stop calling it JSON5 as it will only confuse people.

@vassudanagunta
Copy link
Author

@jordanbtucker, I only raise this because this isn't a private fork for personal use, but published on npmjs.com.

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