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

Support for Jsonc (.json files with comments) #167

Open
Eonfge opened this issue Sep 7, 2020 · 7 comments
Open

Support for Jsonc (.json files with comments) #167

Eonfge opened this issue Sep 7, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@Eonfge
Copy link

Eonfge commented Sep 7, 2020

Jsonc is a simplified json format which allows comments and unquoted values delimited by whitespace. A jsonc formatted file can be transformed to a json file. Comments will be stripped out and quotes added.

https://komkom.github.io/

Right now, parsing json files with comments, causes it to fail. This is annoying since comments are an all-round good practice and they are often de facto required when working in teams.

Originally reported it at Boop-gtk, but it's better to list it here.
zoeyfyi/Boop-GTK#106

@IvanMathy IvanMathy added the enhancement New feature or request label Sep 17, 2020
@IvanMathy
Copy link
Owner

Hey there!

Thanks for the suggestion, I have a quick question to make sure I understand: Are you looking for a script capable of turning JSONC into regular JSON, or do you want every existing JSON scripts to support JSONC?

@zoeyfyi
Copy link
Contributor

zoeyfyi commented Sep 17, 2020

The reason for JSONC is the comments, so maintaining them across transformations would be valuable. I implemented a format-jsonc script here, its a pretty easy change using jsonc-parser. I can send a PR if you are interested.

@IvanMathy
Copy link
Owner

IvanMathy commented Sep 17, 2020

Makes sense to me!

Here's my concern: currently, every script with JSON uses the native parser built by Apple, that's been battle-tested in production for years and most likely optimized at the C level. Swapping that out for a custom parser built in Javascript just to add support for comments, which are not part of the JSON spec and may not be used by the majority of Boop users seems like a dangerous move.

On top of that, I'd argue that besides formatting, there is no real reason to keep comments in scripts because they usually transform the doc into something completely different. If that different thing does support comments, we would need to rewrite each script to make it understand the JSONC, and be able to keep comments. Then we would need to do the same for the reverse script, since we'd expect the comments to be preserved when coming back to JSON.

However, what if I paste let's say some yaml with comments, and convert to JSON. Should I get JSONC? I don't think so, because it would be invalid to use in most places where JSON is commonly used, and would make Boop essentially useless. But what if I paste JSONC; convert to yaml, then back to JSON? I'd expect the comments to be preserved.

My gut feeling is: We could add a JSONC formatter and a JSONC to JSON script which removes comment, and agree that the common ground for Boop scripts is JSON. I agree that it sucks to lose comments but the implications of switching to JSONC might make the output of scripts much more difficult to use.

@zoeyfyi
Copy link
Contributor

zoeyfyi commented Sep 17, 2020

Here's my concern: currently, every script with JSON uses the native parser built by Apple, that's been battle-tested in production for years and most likely optimized at the C level.

jsonc-parser is build by Microsoft and powers vscode. On v8 I got similar performance formatting with jsonc-parser vs the default formatter on a 2mb json file, I think you will probably run into editor performance problems before you notice an appreciable difference in formatting time.

However, what if I paste let's say some yaml with comments, and convert to JSON. Should I get JSONC? I don't think so, because it would be invalid to use in most places where JSON is commonly used, and would make Boop essentially useless. But what if I paste JSONC; convert to yaml, then back to JSON? I'd expect the comments to be preserved.

Definitely, not even sure there is a good JSONC to yaml library anyway.

My gut feeling is: We could add a JSONC formatter and a JSONC to JSON script which removes comment, and agree that the common ground for Boop scripts is JSON. I agree that it sucks to lose comments but the implications of switching to JSONC might make the output of scripts much more difficult to use.

Totally agree, although JSON -> JSON translations such as formatting and minification could be made to work with both (since JSONC is a superset of JSON), which prevents unnecessary duplication of scripts. What do you think?

@Eonfge
Copy link
Author

Eonfge commented Sep 17, 2020

Are you looking for a script capable of turning JSONC into regular JSON, or do you want every existing JSON scripts to support JSONC?

I was indeed thinking of the VS Code method of supporting JSONC: As an alternative to JSON. that way, I can validate and format JSONC without losing comments.

Optionally, a converter from JSONC to JSON would be a nice to have, although I see little reason myself, to strip a document of comments if it already functions with them.

@nhnicwaller
Copy link
Contributor

However, what if I paste let's say some yaml with comments, and convert to JSON. Should I get JSONC? I don't think so, because it would be invalid to use in most places where JSON is commonly used, and would make Boop essentially useless.

This is exactly my concern with the suggestion for JSONC support. I'm probably using Boop to prepare a JSON object as input for some other system, and usually that other system doesn't support JSONC. It would be fairly astonishing if I used Boop to convert anything to JSON, and the output wasn't actually valid JSON.

@Eonfge
Copy link
Author

Eonfge commented Sep 20, 2020

JSON should stay JSON, and if a user intentionally selects 'JSONC to JSON', then it should be valid JSON.

I hope we can all agree.

But if you add a snippet, and you select 'Format JSONC', it would be nice desirable if it permits and indents comments, where possible. To extend on that, if you were to say 'Convert JSONC to CSV' then it should also strip comments since CSV* is without comments. 'Convert JSONC to YAML' would imply that it has comments, since YAML has comments, be preserved.

I'm probably using Boop to prepare a JSON object as input for some other system, and usually that other system doesn't support JSONC

That's really implementation specific. The place where I use it, in the Flathub Builderbot, comments are even encouraged since it helps maintainability: https://github.com/flathub/org.kde.kid3/blob/master/org.kde.kid3.json (Powered by this library)

The way to address all these concerns, is to not implement JSONC as a drop-in replacement for JSON. Make it pretty clear that JSON and JSONC are not the same and that to follow different rules. as I alluded to before, I expect 'Format JSON' and 'Format JSONC' to be different drop-down options.

*CSV actually has some conflicting implementations and standards, but I'm not touching that can of worms.

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

No branches or pull requests

4 participants