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

Allow optionally interpreting .json files as JSON5 #341

Open
Destroy666x opened this issue Oct 18, 2023 · 4 comments
Open

Allow optionally interpreting .json files as JSON5 #341

Destroy666x opened this issue Oct 18, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@Destroy666x
Copy link

Destroy666x commented Oct 18, 2023

As of now you get parse errors for common modern/human-readable JSON config usage - trailing commas, comments, etc. Changing extension of some of these files is impossible because they belong to external tools that expect .json, yet are able to parse/interpret it correctly as either JSON5 or JSONC. Examples: bunch of configs related to VSCode and its plugins.

Overall JSON standards are a mess because the initial standard sucks and it's hard to have consensus now, but would be nice for tools like this to support as many valuable options as possible.

@sirosen
Copy link
Member

sirosen commented Oct 18, 2023

JSONC or JSON5 with a .json suffix strike me as cases in which the file extension is actively misinforming us about the contents of the file. So that's not great and definitely not something I would want on by default.

I'll note that inclusion of a JSON5 parser with check-jsonschema is optional -- so this would also hinge on running in an environment where one of the supported parsers is installed.

This is achievable today if you pipe your files via stdin (so that their filenames are removed from the equation) and use --default-filetype:

$ cat foo.json 
{
  "a": 1  // an int
}
$ cat foo.json | check-jsonschema \
  --schemafile <(echo '{"properties": {"a": {"type": "integer"}}}') - \
  --default-filetype json5
ok -- validation done

Before investing time and thought in something like failover parsing behaviors, does that usage suit your needs?
This seems relatively niche to me -- but not being a VSCode user myself, perhaps I'm missing how common this situation is.


Please let's refrain from getting into any OT disparagement of JSON or other specs.
I actually quite like JSON, in spite of a few flaws which it has, but more importantly I don't think that talking about it is likely to be productive here.

@sirosen sirosen added the enhancement New feature or request label Oct 18, 2023
@Destroy666x
Copy link
Author

Destroy666x commented Oct 18, 2023

It's not just VSCode. It's any sort of config that you edit frequently, but VSCode is the most optimal example as it has a lot of .json configs that are included in user created repos. E.g. devcontainer.json that's part of one of most popular plugins (20 million downloads) that I'd like to validate in my projects with pre-commit. And which this comments sums up, pretty much: devcontainers/spec#14 (comment) Why don't they use .jsonc or .json5 extension for it or just e.g. .yaml more suitable for configs - don't ask me.

As for the mentioned usage, not sure that'd be efficient with pre-commit, especially in a cross-system project.

JSON was supposed to be data format normally, the other specifications extend it for usage like configs, where you like to comment stuff out, add comments for why things are configured like they are and have trailing commas for entries that get manually edited a lot. All of which don't matter for regular JSONs from e.g. web development that are mostly automatically generated.

Thus IMO considering the config part, which I don't think is niche considering how big VSCode is alone, is or might be important sooner or later, assuming people that are setting such .json configs as standards won't come to consensus of using more accurate extensions.

@sirosen
Copy link
Member

sirosen commented Nov 3, 2023

I'm circling back on this to make sure I post an update (before taking a short vacation, so I'll be out for a bit).

I'm basically convinced that

  • 👍 ✨ this should be a feature ✨
  • 🤷 it will be off by default (per the issue title, that seems like an easy agreement)
  • your point about it not working well with pre-commit is well-taken -- enabling it should work in pre-commit and failing to meet that standard means failing to properly implement the feature

I don't know exactly when I can get to this. I'd be open to and grateful for a PR but it will also be some time before I can review one.

The interface will basically be a flag to turn this on. Some possible names to bat around:

# names for using the JSON5 parser on all `.json` files
check-jsonschema --prefer-json5 ...
check-jsonschema --parse-json-as-json5 ...

# names for *forcing* the use of a parser (json5|yaml|toml|json) regardless of filename
check-jsonschema --force-parser json5 ...
check-jsonschema --override-filetype json5 ...

@janosh
Copy link

janosh commented Feb 4, 2024

another option:

check-jsonschema --parse-as json5 ...

fwiw, after reading this thread, i still think the optimal default behavior would be to attempt importing a JSON5 parser and if successful, default parsing mode on all JSON files to JSON5.

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

3 participants