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

Lost simple unquoted string results #44

Closed
d3x0r opened this issue Jun 26, 2020 · 12 comments
Closed

Lost simple unquoted string results #44

d3x0r opened this issue Jun 26, 2020 · 12 comments

Comments

@d3x0r
Copy link
Owner

d3x0r commented Jun 26, 2020

In the last pass of 'error' compliance that tr as a partial word is bad, I've lost compatibility to handle unquoted identifiers. {op:update}...

brettz9 added a commit to brettz9/JSON6 that referenced this issue Jul 12, 2020
@brettz9
Copy link
Contributor

brettz9 commented Jul 12, 2020

I'm wondering if you're forgetting that that the values need to be quoted in {op:update}, i.e., as {op:'update'} instead. The former would not be valid JavaScript, so I presume that may have just been an oversight.

In any case, I've added #45 to explicitly add a test case for unquoted non-keywords (to give an explicit example alongside the other unquoted keywords).

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

Just because it's not valid JS, or JSON doesn't mean it isn't easy to parse, and easy for the machine to understand...
I'm not sure how this recovered unquoted strings; they would have to not include (FALSETRUNIDNaY) otherwise they would blow up on pretty much anything.
I tried to step back to an older version but couldn't identify where the failure was... I had a lot of code using this JSON6 (and the C version) and a lot of things suddenly stopped working...

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

This is a 'superset' not a 'subset' ... so it's allowed to have syntax that is not supported at a lower layer; if you want to stay at the intersection of valid JS and valid JSON6 that's a usage/stringification choice...

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

I took the short path out and just replaced JSON6 with JSOX (which definitely handles unquoted single words; not keywords - they would be translated to their own value of course)... It also has code for like partially collected 'tru' which it can unwind into text... so I began to think I was hallucinating that the code ever worked with JSON6...(it did)

@brettz9
Copy link
Contributor

brettz9 commented Jul 13, 2020

Ah, ok, I see.

Yeah, I had been aware of it being a superset (would be very nice to have a listing of where it diverges from being a subset though--I guess I could go through the tests to get an idea), but by your saying "lost", and without tests failing, I worried it was what I'd personally consider a more serious failure (of breaking on the expected JavaScript/JSON subset).

We discussed the unquoted keyword change in #30 , so I don't know if your changes for that could have impacted things.

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

I'm not going to assign 'blame' :) It was probably just barely working, like if the word didn't start with one of the characters in keywords... but if it was like 'fat' .. 'f' and 'a' would progress the word_state along False, but then 't' would be in true and fall through and throw an error...

@brettz9
Copy link
Contributor

brettz9 commented Jul 13, 2020

Would you be open to config that disallowed the non-JS subset, or do you only want to allow stringify+parse? One immediate case where this might be useful: ota-meshi/eslint-plugin-jsonc#4

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

I don't see the purpose in enforcing syntax limitations in the parser (throwing errors at things that can be accepted)... the builtin stringify isn't much different than JSON stringify; so that's not going to leverage the 'superset' features...

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

Also; I'm thinking it might have been when I copied the tests to here... which in making that also compily it 'broke' early modifications on the C side...

I had started to modify this in-place but at the same time noticed a lot of other people sometimes appreciate specialized machete's rather than swiss army knives (?) :)

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 13, 2020

ya know; thanks for talking to me about this :) I'm just going to close it....
is that additional test needed?

@d3x0r d3x0r closed this as completed Jul 13, 2020
@brettz9
Copy link
Contributor

brettz9 commented Jul 14, 2020

The ability to just copy-paste some JSON6 from a data file into one's JavaScript application--or vice versa, and have it just work, is quite appealing. But if one's parser doesn't report errors with syntax that won't work when moving to Javascript, it is catching the incompatibility too late. Better to catch it earlier on, imo, e.g., when editing a JSON6-as-subset file.

While I understand the goal is human-readability, I think if we were to successfully convince, say new versions of npm to work with JSON6 package.json, or ESLint configs, etc., it'd be a much easier sell if there were at least a variant of JSON6 that was a subset of JavaScript, or otherwise, people might see it as opening a can of worms as to whether to use Yaml instead or such. Some might see such efforts as potentially subject to fashion (e.g., with ES6 incorporating CoffeeScript-like features, people seem to be moving away from it, despite it having had utility and maybe still some special utility), whereas, if tied to being a (data-oriented) subset of ECMAScript, it is more plausible to be seen as having staying power, particularly given that it would not likely conflict with future versions of ECMAScript (as these seek to keep compatibility with their own past versions) if they were to assign their own meaning to those superset-only features.

Also, I think plain JS is pretty readable as it is. I don't know all of the superset-only changes, but I don't recall that many differences--e.g., backticks on properties would not be too essential, imo.

There is admittedly the case you mention (though seem content to close) of avoiding quotes (and the need to escape them) for strings (the one case I think where even HTML shines over JSON or even JSON6, e.g., <tag>Aren't quotes nice?</tag> vs. ['tag', ['Aren\'t quotes nice?']]--despite the closing tag overhead in HTML, the delimiting need is less). This could be appealing, but JavaScript couldn't distinguish it from inline variables, and it seems like it would only work for single word identifiers anyways. (And JSON6 thankfully allows template literals at least as far as minimizing the need to escape.)

Btw, re: "blame", thanks for the assurance. :-) I was just concerned as I would hope to remedy any regressions I had caused.

@d3x0r
Copy link
Owner Author

d3x0r commented Jul 14, 2020

Ya; I uhh also read through the documentation and it wouldn't lead me to believe that 'strings' are just white-space punctuated sequences of characters that aren't anything else :)
(or rather control character which is ws [{:},]'"`` and '/ '(maybe '#') for comments (ws is whitespace)

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

No branches or pull requests

2 participants