flyingpie / windows-terminal-quake

Turn any app into a Quake-style toggleable app.
https://wtq.flyingpie.nl
MIT License
547 stars 35 forks source link

Suggestion: Use JSON5 for configuration file. #104

Open webbertakken opened 2 years ago

webbertakken commented 2 years ago

Currently when I'm editing the JSON configuration file, I see errors and warnings all over the file for every comment.

I will illustrate this with a screenshot. It happens the same for all WebStorm, Rider and VS Code:

image

This makes it hard to spot warnings like mistyping Oemtilde with OemTilde.

My suggestion would be to move to JSON5 for configuration files, so that comment are allowed, and part of the syntax.

flyingpie commented 2 years ago

@webbertakken Cool suggestion! Does this just entail a change of file extension, or how do you signal this to the editor?

webbertakken commented 2 years ago

Yes, as far as I understand we can simply rename the file to have json5 as an extension.

VS Code users would need to install a plugin for JSON5 syntax.

image

JetBrains editors like WebStorm already natively support JSON5 syntax.

We can keep json-schema exactly the same. It seems to work just fine:

image

Obviously we'd need to read the config from this new file as well.

Lastly, we would probably need some kind of strategy with backward compatibility (i.e. prefer .json5, but fallback to .json if it exists), so that we do not break existing configurations.

Maybe it's also nice to write up a migration guide between versions. For this step a user would just rename their .json file to .json5.

flyingpie commented 2 years ago

@webbertakken I definitely think that's a great suggestion, I've been annoyed by the lack of support for comments for while already.

One thing I do worry about though, is the requirement for additional plugins or a specific editor, in order for the JSON stuff to work. Currently, most editors understand both the JSON syntax and the accompanying schema file. Moving to JSON5 could mean that this is no longer the cause. Do you have any thoughts on this?

webbertakken commented 2 years ago

As pointed out in my last comment, It's natively supported in Webstorm, but VS Code needs a plugin. JSON-schema support stays unchanged.

Note that the plugin is offered immediately, the moment you open the file. All you have to do is click install.

For further rationale you could look at this discussion about the config file from VS Code itself , where the community seems to be in favour, but the developers do not see enough value in the change (VS Code internally supports their own format of JSONC (what is?), thus they don't have the comment problem we do).

In my opinion both YAML and JSON5 are well defined formats, and both more suitable than plain JSON files, which are intended for data-interchange (or simply payloads of data).

flyingpie commented 2 years ago

@webbertakken So what about just supporting yaml instead? This would allow comments, and should work on most major editors.

It's nice to know that VS Code suggests the plugin right away, though.

webbertakken commented 2 years ago

Afaik YAML is natively supported in all IDEs, so there's that.

I feel YAMLs upside is that it's more concise and much easier to read once you're used to it while its downside is that people new to it may mess up both syntax and spacing. Maybe you're right and YAML would be the better option. But lets make sure we do have a definition file in that case as well.

Personally I have no preference between JSON5 or YAML. If you want to consider all viable options, there's TOML as well.

flyingpie commented 2 years ago

@webbertakken Thank you for your thoughts on this, I really appreciate it.

I'll look into a YAML parser, and specifically see whether this works with VS Code as nicely as JSON does, when combined with a schema file. I'll also do some tests with other well-known editors, since I like having stuff "just work" without specific editors or addons.

juventus18 commented 2 years ago

I noticed today that the settings.json file used directly for windows terminal settings does not produce errors for comments when using vscode, so it must be possible without changing the file extension or going to YAML.

I noticed the schema files for WT and w-t-q each reference a different base schema, maybe a place to start?

flyingpie commented 2 years ago

@juventus18 Could you tell us what VSCode is saying about the file format? It may be that you're using "jsonc", or "JSON + Comments":

image

juventus18 commented 2 years ago

@flyingpie yeah, when I open WT settings.json it auto-detects "JSON with comments". When I open windows-terminal-quake.json it auto-detects to plain "JSON". A little quirky, but it seems that simply renaming windows-terminal-quake.json to settings.json makes VSC auto-detect "JSON with comments" rather than "JSON".

The red lines did bother me a little bit before, but now that I know I can just manually switch, seems nbd (and a handy tidbit for other bothersome JSON files).

webbertakken commented 2 years ago

VS Code allows this for their own settings file. See also earlier references.

It's not a good solution for any product that isn't vs code itself. Also editing that config in other IDEs will not auto-detect jsonc because it doesn't match the spec.

allan-bonadio commented 1 year ago

json5 is still a subset of JS itself. So you can set your editor to JS for syntax when editing a file. Also, it's a superset of JSON and JSONC and probably any 'json with comments' format MS uses, so a rename to .json5 is an easy way to upgrade files.

flyingpie commented 8 months ago

I've updated wtq to also look for ".jsonc" and ".json5" files (and we can easily add more extensions of course), so if you now use a file called "windows-terminal-quake.jsonc" and VS Code at least will automatically switch to "JSON with comments" highlighting.

jmjoy commented 5 months ago

Regardless of whether to use json5 or standardize jsonc, I think it is a good idea.

For jsonc, we should use extension name jsonc, not json.

flyingpie commented 5 months ago

Regardless of whether to use json5 or standardize jsonc, I think it is a good idea.

For jsonc, we should use extension name jsonc, not json.

Yeah good point, I'll update the shipped/generated config file in the next update.