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

[json] VSCode does not complain about syntactically broken JSON files #3641

Closed
scharf opened this issue Mar 2, 2016 · 21 comments
Closed

[json] VSCode does not complain about syntactically broken JSON files #3641

scharf opened this issue Mar 2, 2016 · 21 comments
Assignees
Labels
feature-request Request for new features or functionality json JSON support issues on-testplan
Milestone

Comments

@scharf
Copy link

scharf commented Mar 2, 2016

The Problem

VSCode can nicely handle comments in JSON files. However, the JSON specification does not allow comments in JSON files. Therefore most JSON parsers fail when JSON files contain comments. E.g. package.json fails, and even the typescript compiler fails on tsconfig.json files that include comments.

Another problem is that syntax highlighting does not work on github. For example, the file extensions/xml/xml.configuration.json ist not legal JSON. This is symptomatic for repositories that are managed with VSCode:

Proposal: create a new file type, e.g. .tson

Instead of misleading people about the syntax of JSON by providing support for comments in JSON files, VSCode should rather use a new format like TSON, that would be an extension of JSON that allows for comments. There could be a simpler pre-processor for TSON (like JSON.minify) that converts TSON into JSON by stripping away the comments.

With such a new file type, VSCode could happily use them without compromising the integrity of existing .json files.

In addition, it would be cool to use typescript ambient definitions for .tson files. I really like the typescript schema for tasks.json.

@isidorn isidorn added the json JSON support issues label Mar 2, 2016
@aeschli aeschli added the feature-request Request for new features or functionality label Mar 2, 2016
@aeschli aeschli added this to the Backlog milestone Mar 2, 2016
@aeschli
Copy link
Contributor

aeschli commented Mar 2, 2016

I think it's ok to add comments to files that we (VSCode) own: tasks.json, launch.json, settings.json and keybinding.json. In all others (tsconfig.json, package.json) we can not and must not add comments.

Your suggestion of having a separate file suffix makes sense, it would prevent some confusion. It wouldn't help with GitHub tough. But we should have thought about this earlier.

@scharf
Copy link
Author

scharf commented Mar 2, 2016

It wouldn't help with GitHub tough.

It would, if '.tson' would be established as a new file type and you can easily use js as renderer, because JSON and TSON is legal JavaScript:

{
    "version": "0.1.0",
    "configurations": [
        {
            // path to VSCode executable
            "runtimeExecutable": "${execPath}",
        }
    ]
}

Alternatively, I guess it should not be that hard to add a new parser to rouge, see for example json-doc

But we should have thought about this earlier.

Hmm, I should have reported this earlier -- when I first tried VSCode I stumbled over this a few months ago (after the official announcement)....

@scharf
Copy link
Author

scharf commented Mar 2, 2016

@bgse
Copy link
Contributor

bgse commented Mar 3, 2016

@aeschli Kinda torn on this one tbh, while it is definitely convenient in the vscode-owned files to know what all the settings are for, this still has the potential to teach bad habits and dilute standards.

A junior developer starting out with vscode might not even realize that it is against specification, and telling people "you can do it here, but generally it is not ok to do" leaves a bad taste in my mouth.

Still, 50/50 on leaving it as it is for the vscode-owned files.

@weinand
Copy link
Contributor

weinand commented Mar 3, 2016

In debug land we dropped comments in json files for all the reasons mentioned here and elsewhere (and we only introduce them by accident because VSCode is not complaining about them).

@aeschli aeschli changed the title VSCode does not complain about syntactically broken JSON files [json] VSCode does not complain about syntactically broken JSON files Mar 3, 2016
@stkb
Copy link
Contributor

stkb commented Mar 4, 2016

There's also JSON5. Not sure how much traction it's got but from looking at the npm download numbers (120k in the last day) apparently quite a bit.

@scharf
Copy link
Author

scharf commented Mar 4, 2016

JSON5 looks great, especially, because it is much easier to edit because there is no need to use quotes for keys (though . separated keys like "editor.fontSize" would still need quotes).

@scharf
Copy link
Author

scharf commented Mar 4, 2016

Suppose, VSCode would use .json5 files for the user settings as alternative format. For new projects it would propose to use .json5 and it could obviously load the existing .json but propose to rename them. There could be a setting "settings.use.json5": false if users want to use the .json files.

@pflannery
Copy link

Here's something from Douglas Crockford on using comments in JSON config files - https://plus.google.com/+DouglasCrockfordEsq/posts/RK8qyGVaGSr

Myself I prefer something like yaml or cson for configuration which allow comments and no need for the extra chaff i.e. comma's, braces etc..makes it much more human friendlier to read.

@glen-84
Copy link

glen-84 commented May 21, 2016

Déjà vu

Here is a letter that I sent to Douglas Crockford on the subject. Unfortunately, I didn't receive a reply.

My opinion regarding VS Code:

  1. Comments in *.json files should not be supported.
  2. For VS Code configuration files, either special-case these files in the editor, or better yet, use a proper file format for configuration, such as YAML or JSON5.

@clavecoder
Copy link

npm sanctions the use of the "//" property for comments. Unfortunately, vscode flags it as a duplicate object key. See #9040 for more details.

@waldekmastykarz
Copy link

@aeschli:

I think it's ok to add comments to files that we (VSCode) own: tasks.json, launch.json, settings.json and keybinding.json. In all others (tsconfig.json, package.json) we can not and must not add comments.

One problem that this introduces is that it's impossible to parse the .json file as an object to change its contents. For example imagine that you wanted to use a Yeoman generator to add tasks to tasks.json. Currently parsing the tasks.json file breaks due to comments and you either need to first remove the comments or process the file as a string, both of which are far from perfect.

@clavecoder
Copy link

I closed #9040. While they say the will "support" "//" it probably means they won't reject that property name, not that they would support/allow duplicates. We can add "//" properties now, so I think there is nothing to do. Blame Douglas Crockford, I suppose. We could always petition ECMA 😄

http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf

@aeschli
Copy link
Contributor

aeschli commented Aug 2, 2016

I agree, we should have used your own file extension, or used JSON5. But at the moment we don't have any plans to make waves in that area.
@waldekmastykarz To parse our configuration files, you can use the jsonc-parser node module.

@vogler
Copy link

vogler commented Aug 31, 2016

Another thing that makes the config annoying to edit is that trailing commas are not allowed.
+1 for JSON5

@alexewerlof
Copy link

+1 for JSON5

@shanalikhan
Copy link

I have referenced an issue, above.

When User have trailing commas in settings.json file. API doenst allow to save or add keys and values in it and extension failed on it.

Code should allow such feature from API level also.

@fabiospampinato
Copy link
Contributor

This is a problem, I've made a few extensions that read from JSON configuration files, and there are issues (fabiospampinato/vscode-projects-plus#8) about people writing comments in them and wondering why things do not work as they expect to.

@aeschli
Copy link
Contributor

aeschli commented Nov 27, 2017

Added a separate language mode jsonc for JSON with comments. Only our settings file are associated with that mode.

@weinand
Copy link
Contributor

weinand commented Nov 27, 2017

@aeschli this includes "launch.json" and "task.json", correct?

@aeschli
Copy link
Contributor

aeschli commented Nov 27, 2017

@weinand Yes

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality json JSON support issues on-testplan
Projects
None yet
Development

No branches or pull requests