dlang
DRAFT: Json5 for std.json
This PR adds Json5 support to std.json.
done:
- single tick strings
- json5 identifier
- single line comments //
- multi line comments /*
- multi line strings
todo:
- Numbers may be hexadecimal.
- Numbers may have a leading or trailing decimal point.
- Numbers may be IEEE 754 positive infinity, negative infinity, and NaN.
- Numbers may begin with an explicit plus sign.
Thanks for your pull request, @burner!
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + phobos#8806"
I think this should be a separate module.
Was this discussed/approved somewhere? Because I faintly recall a discussion with the consensus that this is a bad idea.
Probably worth clarifying to everyone that JSON5 is not standard, it's a different language that's a superset of JSON. Someone just decided to add a few random things to JSON, and call it "JSON5". It's definitely not a "newer version of JSON". I could take the YAML spec and rename it to JSON6 with the same effect.
So, adding this to Phobos will make our JSON implementation non-conformant (or if it isn't already, even more so).
Maybe if it was optional?
I like it. std.json has always been slow and non-conformant, so this will finally give it some real value
I like it. std.json has always been slow and non-conformant, so this will finally give it some real value
OK, but please rename the module to std.json5 and make std.json a deprecated module containing just a public import.
I like it. std.json has always been slow and non-conformant, so this will finally give it some real value
OK, but please rename the module to
std.json5and makestd.jsona deprecated module containing just a public import.
might as well, it always seemed like std.json was kinda silently deprecated anyway. maybe that'll finally get std.data.json brought in?
I'm going to expand on the above a bit because this is kind of frustrating to me.
JSON5's name is a scam, and it successfully tricked a lot of people. Adding JSON5 to std.json makes as much sense as adding any other JSON superset, such as YAML or Amazon ION. But, forcing std.json to support only one particular superset kills off any chance of supporting any other superset. To me, this is an obvious argument that we should not support any of these supersets in std.json (but other modules which support them, possibly even sharing code, is of course fine).
So, I think this is a bad idea, and it further supports and propagates the lie in JSON5's name. Don't get tricked!
OK, but please rename the module to std.json5 and make std.json a deprecated module containing just a public import.
that is a copy paste error, will fix by the time the DRAFT is removed.
This was discussed in a foundation meeting @WalterBright @atilaneves
Someone just decided to add a few random things to JSON, and call it "JSON5".
No, JSON5 means JSON with ECMAScript 5 syntax for data. The 5 corresponds to the ECMAScript revision.
JSON5's name is a scam, and it successfully tricked a lot of people.
I hope you can reconsider, there is a purpose behind the name, and it's not to make people think that it's a revision of JSON.
However, I'm of the opinion that json5 support is not worth adding to std.json. The existing module supports a canonical format -- JSON. There are no options (aside from whitespace) when it comes to reading and writing JSON files -- everything has one representation.
In order to properly support json5, you need to start adding more types to JSONType, which is going to break a lot of code. If people want to implement e.g. a json5 editor, they will not be able to use a library that throws away the other information (comments, single-quoted strings, are identifiers quoted, was hexadecimal used, etc.). To that end also, someone might want to validate that their json files are strictly json and not json5.
I'll note that I actually have written a complete json5 parser/serializer in https://github.com/schveiguy/jsoniopipe/tree/master
In my experience while adding this, I found that dealing with whitespace and non-quoted identifiers significantly complicates the parser. Parsing JSON only requires ASCII. Someone using std.json might not expect the most optimized performance, but still shouldn't have to pay the penalty of parsing full unicode for JSON, just in case they have actually passed JSON5 data.
Given that most likely the json5 support should be added as its own file, why not a dub package instead of adding to the standard library?
No, JSON5 means JSON with ECMAScript 5 syntax for data. The 5 corresponds to the ECMAScript revision.
Ah, thanks; still misleading IMO!
I hope you can reconsider, there is a purpose behind the name, and it's not to make people think that it's a revision of JSON.
Personally I would be more OK with this if the proposal for the format came from a more established standards body, e.g. as an annex to the ECMAScript spec. As it stands, the author still needed to make some arbitrary decisions, such as which ECMAScript features are supported and the exact syntax subset.
Given that most likely the json5 support should be added as its own file, why not a dub package instead of adding to the standard library?
:+1: and in line with our current policy for major Phobos additions.
I rebased this PR and moved the code to std.json5. See #8875. Would that be enough to get this code merged?
