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

Whitelist comments in all JSON files #129206

Merged
merged 2 commits into from Sep 20, 2021
Merged

Whitelist comments in all JSON files #129206

merged 2 commits into from Sep 20, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jul 23, 2021

This PR fixes false positives for error highlights in files, for example:

https://github.com/microsoft/vscode/blob/2fb5525/extensions/theme-defaults/themes/dark_plus.json#L13

Currently many files contain comments but are being matched as JSON incorrectly.
To fix this, we associate *.json with JSONC and fix the false positive error highlights.

SNDST00M: M.U.N.I.N added 2 commits July 23, 2021 02:08
Currently many files contain comments but are being matched as JSON incorrectly.
To fix this, let's associate `*.json` with JSONC and fix the false positive error highlights.
@aeschli
Copy link
Contributor

aeschli commented Aug 17, 2021

@sndst00m which tools make use of the linguist-language attribute?
Will GitHub open all json files as jsonc?

@ghost
Copy link
Author

ghost commented Aug 17, 2021

Yes, GitHub will register all .json files as JSON with Comments.
I'm not aware of any method for applying JSONC on a case to case basis.

@ghost
Copy link
Author

ghost commented Sep 18, 2021

I am cleaning my forks and my pulls list, is there any blocker to this one?

@aeschli aeschli added this to the September 2021 milestone Sep 20, 2021
@aeschli aeschli merged commit fdf1738 into microsoft:main Sep 20, 2021
@aeschli
Copy link
Contributor

aeschli commented Sep 20, 2021

Let's give it a try...
Thanks @sndst00m !

@ghost ghost deleted the jsonc branch September 20, 2021 11:34
@ghost
Copy link
Author

ghost commented Sep 20, 2021

I discovered that this glob only affects top-level files.
You'll need to use **/*.json instead of *.json.

aeschli added a commit that referenced this pull request Sep 20, 2021
@aeschli
Copy link
Contributor

aeschli commented Sep 20, 2021

Made the change

@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant