Support relative paths for JSON schema associations (Fix #7140) #7225

ndoschek · 2020-02-26T17:05:47Z

What it does

JSON preferences now also accept relative paths inside a theia workspace to associate JSON schemas to JSON files.
Absolute paths and URLs still work as expected.

Remark

As mentioned in #7140, I came across a minor problem, namely the jsonSchemaService (vscode-json-languageservice) returns the following warning:
Problems loading reference 'file:///test/my-schema-definition.json': Unable to load schema from '/test/my-schema-definition.json': ENOENT: no such file or directory, open '/test/my-schema-definition.json'.

The JSON schema support works as expected now also for relative paths, but this warning occurs in files that are associated with JSON schemas via relative paths.

How to test

Set up a theia-workspace and include a plain JSON file and a JSON schema file,
e.g. /testing.json and /.schemas/json-schema.json
Define a schema association for those JSON files in your settings,
e.g.

"json.schemas": [
  {
    "fileMatch": [
      "/testing.json"
    ],
    "url": "/.schemas/json-schema.json"
  }
]

JSON schema support for the matched file is supported for associations via relative workspace paths.

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

Signed-off-by: Nina Doschek ndoschek@eclipsesource.com

ndoschek · 2020-02-26T17:17:57Z

@akosyakov
As mentioned in #7140 I opened this PR with my proposed changes.

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

akosyakov · 2020-02-27T08:02:24Z

/.schemas/json-schema.json that's an absolute path, have you tried with ./.schemas/json-schema.json or .schemas/json-schema.json?

ndoschek · 2020-02-27T10:09:24Z

/.schemas/json-schema.json that's an absolute path, have you tried with ./.schemas/json-schema.json or .schemas/json-schema.json?

Yes, those cases are also covered.

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

akosyakov · 2020-02-28T04:50:51Z

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

+                    const fileUri = new URI(schemaConfig.url);
+                    if (fileUri.scheme === 'file') {
+                        const filePath = fileUri.path.toString();
+                        const workspaceRootPath = this.workspace.rootPath;


It does not handle multi root case. We should fetch json.schemas scoped by resource URI from each root and then apply such schemas only in the scope of a corresponding root.

ndoschek · 2020-04-08T17:44:51Z

Thank you for your review!
I adjusted the behavior accordingly and tested with different workspaces, as well as User, Workspace and Folder settings.

Still, I have the problem (as described earlier in the first comment, section 'Remark') with loading the reference for a JSON schema that was defined with an absolute path. But, this problem only seems to exist in a single-root-workspace.
Do you have any idea on this?

akosyakov · 2020-04-09T07:25:07Z

An error seems to come from here: https://github.com/microsoft/vscode-json-languageservice/blob/2ea5ad3d2ffbbe40dea11cfe764a502becf113ce/src/services/jsonSchemaService.ts#L437-L452

Maybe you can debug it somehow or read through the code assuming your path.

ndoschek · 2020-04-29T11:01:49Z

I have tracked down the problem by debugging the vscode json language server. It listens additionally to changes of the workspace settings, and hence there the relative paths were not resolved properly.
I revised my earlier code and tested different scenarios (e.g. single and multi root workspaces absolute, relative paths, URLs and so on) and am glad to hear your opinion on this.

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com>

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com> Rebase on master

ndoschek · 2020-05-25T07:28:21Z

Hi @akosyakov !
I rebased my change to master and tested the functionality also with the new preference editor.
Do you have time to take a look at it?
Thanks in advance!

akosyakov · 2020-06-02T07:13:51Z

@ndoschek sorry that it takes so long, I'm looking at it this week.

akosyakov

I think it is the right direction but resolutions should happen via Language Client middleware as in VS Code json extension: https://github.com/microsoft/vscode/blob/45aa0bf525b08d7e23c9b10ca65f1de55949e601/extensions/json-language-features/client/src/jsonMain.ts#L137-L140 You basically can copy code from there.

Before you continue working on it: we are going to delete this extension by mid of July and use VS Code extension instead which will automatically resolve the issue.

akosyakov · 2020-06-02T07:25:28Z

packages/preferences/src/browser/folder-preference-provider.ts

+
+    resolve<T>(preferenceName: string, resourceUri?: string): PreferenceResolveResult<T> {
+        // resolve relative paths for json schema settings in workspace scope
+        if (preferenceName === 'json.schemas' && this.getScope() === PreferenceScope.Workspace) {


Can it be done by clients? It does not look good to introduce handling of individual preferences into the preference service logic.

akosyakov · 2020-06-02T07:39:03Z

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

@@ -119,6 +149,24 @@ export class JsonClientContribution extends BaseLanguageClientContribution {
        }
    }

+    protected initializeJsonPreferencesAssociations(): void {
+        const userSettings = this.preferenceService.inspect<JsonSchemaConfiguration[]>('json.schemas');


I am a bit confused why we need to worry about json.schemas. We send complete json configuration to JSON language server whenever it is changed. I don't think we need to pass it additionally as part of json/schemaAssociations.

ndoschek · 2020-06-05T07:01:06Z

Thank you for your answer! Do I understand you correctly, that the Theia json extension is being replaced by the vscode json extension in July?
So I will close this PR for now and see if the issues with resolving relative schema paths in single root workspaces are resolved then.
Thank you anyway for your support! :)

akosyakov · 2020-06-05T07:31:24Z

Thank you for your answer! Do I understand you correctly, that the Theia json extension is being replaced by the vscode json extension in July?

Yes, we plan to deliver it latest mid July.

vince-fugnitto reviewed Feb 26, 2020

View reviewed changes

packages/json/src/browser/json-client-contribution.ts Outdated Show resolved Hide resolved

packages/json/src/browser/json-client-contribution.ts Outdated Show resolved Hide resolved

vince-fugnitto added the json issues related to the json language label Feb 26, 2020

ndoschek force-pushed the fix7140 branch from 4e05282 to 4b04a4a Compare February 27, 2020 10:07

ndoschek requested a review from vince-fugnitto February 27, 2020 10:12

akosyakov reviewed Feb 28, 2020

View reviewed changes

packages/json/src/browser/json-client-contribution.ts Outdated Show resolved Hide resolved

akosyakov reviewed Feb 28, 2020

View reviewed changes

ndoschek force-pushed the fix7140 branch from 4b04a4a to 4239592 Compare April 8, 2020 17:38

ndoschek requested a review from akosyakov April 8, 2020 17:44

ndoschek force-pushed the fix7140 branch 2 times, most recently from f5547ef to f134faf Compare April 29, 2020 10:59

ndoschek added 3 commits May 13, 2020 10:19

Support relative paths for JSON schema associations

7f78f1e

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com>

Support JSON schema associations for multi-root-workspaces

57472fb

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com>

Ensure resolving of relative paths in JSON schema settings

bbde831

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com> Rebase on master

ndoschek force-pushed the fix7140 branch from f134faf to bbde831 Compare May 13, 2020 08:54

akosyakov reviewed Jun 2, 2020

View reviewed changes

ndoschek closed this Jun 5, 2020

akosyakov mentioned this pull request Jun 29, 2020

replace json theia extension with vscode one #8112

Merged

4 tasks

ndoschek deleted the fix7140 branch July 13, 2020 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support relative paths for JSON schema associations (Fix #7140) #7225

Support relative paths for JSON schema associations (Fix #7140) #7225

ndoschek commented Feb 26, 2020 •

edited by akosyakov

Loading

ndoschek commented Feb 26, 2020

akosyakov commented Feb 27, 2020

ndoschek commented Feb 27, 2020

akosyakov Feb 28, 2020

ndoschek commented Apr 8, 2020

akosyakov commented Apr 9, 2020

ndoschek commented Apr 29, 2020

ndoschek commented May 25, 2020

akosyakov commented Jun 2, 2020

akosyakov left a comment •

edited

Loading

akosyakov Jun 2, 2020

akosyakov Jun 2, 2020

ndoschek commented Jun 5, 2020

akosyakov commented Jun 5, 2020

Support relative paths for JSON schema associations (Fix #7140) #7225

Support relative paths for JSON schema associations (Fix #7140) #7225

Conversation

ndoschek commented Feb 26, 2020 • edited by akosyakov Loading

What it does

Remark

How to test

Review checklist

Reminder for reviewers

ndoschek commented Feb 26, 2020

akosyakov commented Feb 27, 2020

ndoschek commented Feb 27, 2020

akosyakov Feb 28, 2020

Choose a reason for hiding this comment

ndoschek commented Apr 8, 2020

akosyakov commented Apr 9, 2020

ndoschek commented Apr 29, 2020

ndoschek commented May 25, 2020

akosyakov commented Jun 2, 2020

akosyakov left a comment • edited Loading

Choose a reason for hiding this comment

akosyakov Jun 2, 2020

Choose a reason for hiding this comment

akosyakov Jun 2, 2020

Choose a reason for hiding this comment

ndoschek commented Jun 5, 2020

akosyakov commented Jun 5, 2020

ndoschek commented Feb 26, 2020 •

edited by akosyakov

Loading

akosyakov left a comment •

edited

Loading