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

do not show errors for trailing commas in web JSON editors #4100

Merged
merged 1 commit into from
May 17, 2019

Conversation

sqs
Copy link
Member

@sqs sqs commented May 17, 2019

fix #4008

All JSON documents we use (for settings, site config, external services, and extension manifests) support trailing commas on the frontend and backend. However, an upgrade to monaco-editor unexpectedly caused the Monaco JSON editors in our UI to show red squiggly errors for trailing commas.

The (undocumented) way to do this is to add allowComments to the root of the JSON Schema. It is not sufficient to set allowComments: true in a monaco.languages.json.jsonDefaults.setDiagnosticsOptions call. I found this secret fix by finding microsoft/vscode#19992 (comment) and then inferring what changes that might entail, to discover allowComments in the JSON Schema is responsible for ignoring trailing comma errors.

Verification (the trailing comma has no error, and validation/completion still works):

image

@sqs sqs added this to the 3.4 milestone May 17, 2019
@sqs sqs added release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases webapp labels May 17, 2019
@@ -3,6 +3,7 @@
"$id": "https://raw.githubusercontent.com/sourcegraph/sourcegraph/master/shared/src/schema/extension.schema.json#",
"title": "Sourcegraph extension manifest",
"description": "The Sourcegraph extension manifest describes the extension and the features it provides.",
"allowComments": true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, a Sourcegraph manifest is a package.json file, so it does not support comments

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point. I will remove it from this file.

fix #4008

All JSON documents we use (for settings, site config, external services, and extension manifests) support trailing commas on the frontend and backend. However, an upgrade to monaco-editor unexpectedly caused the Monaco JSON editors in our UI to show red squiggly errors for trailing commas.

The (undocumented) way to do this is to add `allowComments` to the root of the JSON Schema. It is not sufficient to set `allowComments: true` in a `monaco.languages.json.jsonDefaults.setDiagnosticsOptions` call. I found this secret fix by finding microsoft/vscode#19992 (comment) and then inferring what changes that might entail, to discover `allowComments` in the JSON Schema is responsible for ignoring trailing comma errors.
@sqs sqs force-pushed the allow-trailing-commas-json-editors branch from 93b4ff5 to 04c90c5 Compare May 17, 2019 06:27
@sqs sqs merged commit 82776ae into master May 17, 2019
sqs added a commit that referenced this pull request May 17, 2019
fix #4008

All JSON documents we use (for settings, site config, external services, and extension manifests) support trailing commas on the frontend and backend. However, an upgrade to monaco-editor unexpectedly caused the Monaco JSON editors in our UI to show red squiggly errors for trailing commas.

The (undocumented) way to do this is to add `allowComments` to the root of the JSON Schema. It is not sufficient to set `allowComments: true` in a `monaco.languages.json.jsonDefaults.setDiagnosticsOptions` call. I found this secret fix by finding microsoft/vscode#19992 (comment) and then inferring what changes that might entail, to discover `allowComments` in the JSON Schema is responsible for ignoring trailing comma errors.
@sqs sqs deleted the allow-trailing-commas-json-editors branch May 17, 2019 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Monaco editor shows red squigglies on trailing commas
2 participants