support contributes.jsonValidation VSCode API. #7560

bd82 · 2020-04-13T16:34:06Z

What it does

Support VSCode contributes.jsonValidation API.

Note that jsonValidation contribution have a higher priority than SchemaStore schemas, and a lower priority than registered schemas from the preferences
(In case of identical fileMatch strings).

How to test

Create a dummy VSCode ext
Add in its package.json contributes.jsonValidation section pointing to some schema from the schema store with an appropriate fileMatch property`.
Bundle your VSCode ext into a VSIX and place it the plugins folder.
Start Theia
Inspect that the JSON schema is used from the given

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

bd82 · 2020-04-13T16:39:03Z

~~Hmm I need to figure out how to sign the commit~~

I guess i need smaller case -s instead of -S which tries using gpg keys...

packages/json/src/browser/json-validation-registry.ts

akosyakov · 2020-04-14T11:15:49Z

packages/json/src/browser/json-validation-registry.ts

+
+    registerJsonValidation(jsonValidationEntry: JsonSchemaConfiguration): Disposable {
+        this.registry.push(jsonValidationEntry);
+        return Disposable.NULL;


Disposable.NULL is bogus, please use JsonSchemaStore.registerSchema instead. it has a proper logic to unregster configuration

@bd82 It was not addressed, this should remove entry from this.registry and json language server should be notified about it as well

akosyakov · 2020-04-14T11:18:31Z

packages/plugin-ext/package.json

@@ -11,6 +11,7 @@
    "@theia/editor": "^1.0.0",
    "@theia/file-search": "^1.0.0",
    "@theia/filesystem": "^1.0.0",
+    "@theia/json": "^1.0.0",


we don't need this dependency, we only need to register configuration in core json schema storage

@benoitf @tolusha It goes against #6647, are you fine with it?

@azatsarynnyy Do you know whether introducing such dependency will cause issues in Che? i.e. do you use json vscode built-in extensions already?

packages/plugin-ext/src/common/plugin-protocol.ts

packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts

akosyakov · 2020-04-15T07:55:46Z

I'm not sure about this PR. The issue is that we should drop @theia/json and switch to json-language-features vscode extension, after that contributes.jsonValidation should be implemented to work with the built-in extension.

But we can accept is as a temporary solution? cc @eclipse-theia/ecd-theia-committers

@bd82 please remove after closing of #3983 from the commit and PR descriptions. It does not resolve it without switching to the built-in.

akosyakov · 2020-04-15T08:00:52Z

Switching to the built-in extension will automatically resolves the issue: https://github.com/theia-ide/vscode/blob/d24b5f70c69b3e75cd10c6b5247a071265ccdd38/extensions/json-language-features/client/src/jsonMain.ts#L327-L360

bd82 · 2020-04-15T11:50:37Z

@akosyakov are there any estimated timelines for switching out @theia/json?

akosyakov · 2020-04-15T11:55:27Z

@bd82 I hope within next 2 months. We are planning to work on removing monaco-languageclient and getting rid of @theia/json is a prerequisite. cc @marcdumais-work @svenefftinge

I think we can accept this PR for time being, but there are still 2 issues:

whether it is fine to pull @theia/json via @theia/plugin-ext. I remember there were a PR which removed a dependency to it. @thegecko sorry if a wrong call but I remember you were concerned
contribution shapes are bogus: support contributes.jsonValidation VSCode API. #7560 (comment)

bd82 · 2020-04-15T21:34:26Z

I will fix the "wrong shapes" in the next few days/

I hope within next 2 months.

Hmm, probably best if we can merge this PR as a temp solution (once review passes).
As there is also additional delay for consumers of Theia until they can upgrade version due
to other products life-cycles concerns...

Signed-off-by: I060847 <shachar.soel@sap.com>

bd82 · 2020-04-20T13:12:24Z

Hi @akosyakov I've fixed the wrong shape and another relevant CR comment.
I will now do more manual testing.

bd82 · 2020-04-20T14:34:05Z

O.k. I've tested manually for both scenarios of fileMatch as a single string and as an array of strings. works well.

akosyakov · 2020-04-22T09:37:37Z

CHANGELOG.md

@@ -14,6 +16,7 @@ Use `PluginManager.configStorage` property instead. [#7265](https://github.com/e

 ## v1.0.0

+- [core] `CommandRegistry#getAllHandlers` returns with the reversed order of handlers, so if a command has multiple handlers, any handler is considered to be more specific than the other subsequent handlers in the array. In other words, if `@theia/core` contributes a handler for a particular command and your extension also contributes a handler for the same command, the handler from your extension will have `0` index, and the handler from `@theia/core` will have `1` index when calling `getAllHandlers`.


It should not be here. How did you get it?

oy probably via a rebase to update my local branch

akosyakov · 2020-04-22T09:44:21Z

packages/json/src/browser/json-client-contribution.ts

-            allConfigs.push(...config);
-        }
+        const schemaStoreConfigs = this.jsonSchemaStore.getJsonSchemaConfigurations();
+        const pluginContributionConfigs = this.jsonValidationContributionsRegistry.getJsonValidations();


such configs can be registered dynamically, i.e. a user install or uninstall new extension. registry should fire an event and contribution should trigger updateSchemas then

akosyakov · 2020-04-22T09:45:32Z

@bd82 please see comments

thegecko · 2020-05-07T12:17:30Z

whether it is fine to pull @theia/json via @theia/plugin-ext.

I think that's OK. I seem to remember the debug package requires json anyway to correctly show the UI for configuring debug configs.

bd82 · 2020-05-07T18:30:28Z

I will try to find time to get back to this PR next week.

bd82 · 2020-06-10T08:17:39Z

Hello @akosyakov.

I am having difficulty finding the time to complete this...
I wonder if this is still relevant?

considering the upcoming switch to json-language-features vscode extension?

Is there any update /time estimates on said switch?

Thanks.
Shahar.

akosyakov · 2020-06-10T09:46:18Z

We've already started looking into it. The latest is mid of next month.

bd82 · 2020-06-10T09:55:51Z

We've already started looking into it. The latest is mid of next month.

Thanks for the update @akosyakov

bd82 · 2020-06-10T09:56:22Z

I think this PR is now redundant so I'm closing it.

akosyakov added the vscode issues related to VSCode compatibility label Apr 14, 2020

akosyakov requested review from amiramw, azatsarynnyy, elaihau, RomanNikitenko and vince-fugnitto April 14, 2020 11:10

akosyakov reviewed Apr 14, 2020

View reviewed changes

packages/json/src/browser/json-validation-registry.ts Show resolved Hide resolved

akosyakov reviewed Apr 14, 2020

View reviewed changes

packages/plugin-ext/src/common/plugin-protocol.ts Show resolved Hide resolved

akosyakov reviewed Apr 14, 2020

View reviewed changes

packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts Outdated Show resolved Hide resolved

akosyakov added the json issues related to the json language label Apr 15, 2020

fix: 3983 support contributes.jsonValidation VSCode API.

f1c48e5

Signed-off-by: I060847 <shachar.soel@sap.com>

akosyakov reviewed Apr 22, 2020

View reviewed changes

akosyakov changed the title ~~fix: 3983 support contributes.jsonValidation VSCode API.~~ support contributes.jsonValidation VSCode API. Apr 22, 2020

akosyakov reviewed Apr 22, 2020

View reviewed changes

bd82 closed this Jun 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support contributes.jsonValidation VSCode API. #7560

support contributes.jsonValidation VSCode API. #7560

bd82 commented Apr 13, 2020 •

edited by akosyakov

Loading

bd82 commented Apr 13, 2020 •

edited

Loading

akosyakov Apr 14, 2020

akosyakov Apr 22, 2020 •

edited

Loading

akosyakov Apr 14, 2020 •

edited

Loading

akosyakov Apr 22, 2020

akosyakov May 11, 2020

akosyakov commented Apr 15, 2020 •

edited

Loading

akosyakov commented Apr 15, 2020

bd82 commented Apr 15, 2020

akosyakov commented Apr 15, 2020

bd82 commented Apr 15, 2020 •

edited

Loading

bd82 commented Apr 20, 2020

bd82 commented Apr 20, 2020

akosyakov Apr 22, 2020

bd82 Apr 22, 2020

akosyakov Apr 22, 2020

akosyakov commented Apr 22, 2020

thegecko commented May 7, 2020

bd82 commented May 7, 2020

bd82 commented Jun 10, 2020 •

edited

Loading

akosyakov commented Jun 10, 2020

bd82 commented Jun 10, 2020

bd82 commented Jun 10, 2020

		@@ -14,6 +16,7 @@ Use `PluginManager.configStorage` property instead. [#7265](https://github.com/e

		## v1.0.0

		- [core] `CommandRegistry#getAllHandlers` returns with the reversed order of handlers, so if a command has multiple handlers, any handler is considered to be more specific than the other subsequent handlers in the array. In other words, if `@theia/core` contributes a handler for a particular command and your extension also contributes a handler for the same command, the handler from your extension will have `0` index, and the handler from `@theia/core` will have `1` index when calling `getAllHandlers`.

support contributes.jsonValidation VSCode API. #7560

support contributes.jsonValidation VSCode API. #7560

Conversation

bd82 commented Apr 13, 2020 • edited by akosyakov Loading

What it does

How to test

Review checklist

Reminder for reviewers

bd82 commented Apr 13, 2020 • edited Loading

akosyakov Apr 14, 2020

Choose a reason for hiding this comment

akosyakov Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

akosyakov Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

akosyakov Apr 22, 2020

Choose a reason for hiding this comment

akosyakov May 11, 2020

Choose a reason for hiding this comment

akosyakov commented Apr 15, 2020 • edited Loading

akosyakov commented Apr 15, 2020

bd82 commented Apr 15, 2020

akosyakov commented Apr 15, 2020

bd82 commented Apr 15, 2020 • edited Loading

bd82 commented Apr 20, 2020

bd82 commented Apr 20, 2020

akosyakov Apr 22, 2020

Choose a reason for hiding this comment

bd82 Apr 22, 2020

Choose a reason for hiding this comment

akosyakov Apr 22, 2020

Choose a reason for hiding this comment

akosyakov commented Apr 22, 2020

thegecko commented May 7, 2020

bd82 commented May 7, 2020

bd82 commented Jun 10, 2020 • edited Loading

akosyakov commented Jun 10, 2020

bd82 commented Jun 10, 2020

bd82 commented Jun 10, 2020

bd82 commented Apr 13, 2020 •

edited by akosyakov

Loading

bd82 commented Apr 13, 2020 •

edited

Loading

akosyakov Apr 22, 2020 •

edited

Loading

akosyakov Apr 14, 2020 •

edited

Loading

akosyakov commented Apr 15, 2020 •

edited

Loading

bd82 commented Apr 15, 2020 •

edited

Loading

bd82 commented Jun 10, 2020 •

edited

Loading