-
Notifications
You must be signed in to change notification settings - Fork 81
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
JSON with Comments (jsonc) support #197
Comments
Hi @denosaurtrain! Thanks for taking your time to explain me your use case in such a detailed way! |
I have read a bit about comments in JSON since my last message, and I have gained a different opinion in the meantime. In particular, I'm now more skeptical about allowing comments in JSON. In discussions like on Stack Overflow, there is a relatively strong consensus that comments should not be present in JSON, and if one wants to put in comments nonetheless, they can be removed for further processing (that's what you called your dead end no. 3). I tried |
I respect your choice to not support comments. I'll close this issue. Hopefully the workaround will also help other folks who need this in the future. Thanks for the quick responses and thoughtful feedback! |
@denosaurtrain - Another possibility for pre-processing is https://hjson.github.io/ @01mf02 - Since there are tools that strip comments from JSON, the horse is already out of the barn, so on balance I think it would probably make the world a better place if tools like jq/jaq had an option that would allow comments or at least one form of comments. Since jq and jaq do allow "#" comments in JSON that is presented as (part of) a jq/jaq program, my hope is you will get the ball rolling and add an option that would at least allow them. Not the highest priority by any means, of course, but maybe worth. re-opening this issue. |
what it means to support jsonc
I've read a few of these threads, and I agree that they're correct — JSON, per the spec, does not allow comments — but also not the whole story. The way I see it, once you add comments, it's no longer valid JSON but rather "JSON with Comments" ( the decisionI think either of these are reasonable decisions to make here:
While I personally want option 2, the argument for option 1 is strong. why add jsonc (option 2)Really it comes down to this:
From reading threads about this, it's clear that lots and LOTS of people stub their toe on this and need to find a workaround. The feature should be opt-in, so there's no impact to users who want strict JSON only. If it's easy to ignore comments during parsing, why not? why not add jsonc (option 1)Many workarounds are available, at least for the use case where comments can be ignored. "JSON with Comments" doesn't have a spec, and although Considering mikefarah's
The most obvious argument against adding jsonc support is (I think) also the weakest argument: it's tempting to say, " what about other forms of comments
News to me! If that could be hacked to handle
Although other characters like conclusionIdeally, for me, However, 01mf02's decision is also reasonable, and I'm happy to have a clear decision one way or the other. Now people can read this in the future and not go down quite so deep a rabbit hole as I have. Plus, I have at least one workaround (which mostly works for me), so it's not a blocker. |
I'm happy that jsmin works for you. I think that there is value in sticking to the most widely recognised definition of what JSON is, and that is the spec. Everything else opens a pandora's box, IMO. Just for completeness: In the future, I think that the road forward will be for jaq to support YAML. That would allow not only for comments, but also for other syntax that JSON does not offer. jaq 1.5 has removed the roadblocks to support different input formats in jaq, it now "only" remains to integrate a YAML parser with jaq, define the operations on YAML values, and you got YAML support (and with that basically JSON with comments for free). |
Just to be clear, when I say that jaq supports YAML, that means that it should provide a mode where it reads input data as JSON and another where it reads input data as YAML. |
I have come across JSON5, which is a fairly popular JSON extension (including comments) with a proper specification. |
what is JSON? what is
|
I also thought that |
Absolutely! You've been extraordinarily patient with my long, pedantic posts.
Indeed. YAML won't help me because I'm forced to use # list all recommended VS Code extensions that are not currently installed
./jsmin < ~/code/default.code-workspace \
| jaq -r '.extensions.recommendations[]' \
| rg -v $(code --list-extensions \
| tail --lines=+2 \
| xargs -I {} echo -en "({})|" \
| head --bytes -1) |
It's my pleasure. I think it is important to explain the reasoning behind decisions to gain acceptance for them.
Ah, I was under the misconception that YAML supported block comments --- in that case, you could just have interpreted your JSONC file as YAML and everything would have worked. Too bad that this is not the case. :( Well, at least I'm happy that your jsmin workflow now works. :) |
Feature/Workaround Request
Request: an option to make
jaq
ignore comments in input like this:Today,
jaq . file.json
fails with this error:If this is considered a "Won't Fix", any suggestions for workarounds?
What is jsonc, and who cares?
This is common for config files, such as:
How VS Code describes it:
IMO, trailing commas are out of scope for this issue, but I don't feel strongly about that.
Concrete use case
Say I want to enforce in CI/pre-commit that certain config parameters aren't being used. I can't use
jaq
for this today because my settings files almost always have comments, including ones for this use case:If
jaq
were able to parse this file (ignoring the comments), I could use a CI/pre-commit script like this:Dead Ends
rg
/sed
to strip comments before passing tojaq
, but this is difficult to get right.jaq
feature myself. I added some failing tests but didn't get far before realizing I should open an issue to discuss it. Plus, while editinghifijson
, I noticed that thejaq
branch,faster-lexer
, is being actively developed as of July 2024, so I worried that adding tohifijson
could be a mistake. I see that branch now has a PR (Rewrite lexer and parser #196) -- perhaps that's only for parsing input expressions, not JSON input?The text was updated successfully, but these errors were encountered: