204 support schema associations in file #280

apupier · 2020-07-15T15:38:01Z

Choose to use modeline with pattern # yaml-language-server: $schema=<urlToTheSchema>

Why?

needs to make a choice :-)
using directive likes using #SCHEMA for instance is creating an invalid file with current specifications state, so need to move to a comment-like #
use $schema because it is what is used by VS Code for JSON schema

big advantages of allowing defining the schema in the file is that the filename can be whatever is needed. It avoids the requirement of specific file name pattern. It ensures users can use a schema in all IDEs, no need for a specific IDE integration (apart from relying on Yaml Language Server of course).

For instance in OpenShift dev console, there is no way for user to choose the filename. it would allow to add have completion/validation there.

apupier · 2020-07-21T12:04:40Z

@JPinkney @gorkem Can you review it and tell what you think please about this approach please?

JPinkney

It looks good to me, just a few things to address.

I've also discovered: #286 when testing this PR but its a completely seperate issue

README.md

test/parser.test.ts

src/languageservice/services/yamlSchemaService.ts

JPinkney · 2020-07-21T14:31:08Z

test/autoCompletion.withschemainfile.test.ts

+                }).then(done, done);
+            });
+
+            it('Provide completion from schema declared in file with several attributes', done => {


Is there any way you can rebase on master and integrate these tests into the main autoCompletion.test.ts file? And also check the actual values of result.items. Also can you add another test for completion with yaml-language-server modelline in multiple documents. I've been trying to harden the tests lately

I added a test with multiple documents. it seems there is no "textEdit" which is added:
(node:25674) UnhandledPromiseRejectionWarning: AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{ documentation: '', insertText: 'prop1: $1', insertTextFormat: 2, kind: 10, label: 'prop1' } should equal { documentation: '', insertText: 'prop1: $1', insertTextFormat: 2, kind: 10, label: 'prop1', textEdit: { newText: 'prop1: $1', range: { end: { character: 2, line: 0 }, start: { character: 2, line: 0 } } } }

I don't get why it woudl be specific to what I changed. I will have a look tomorrow.

this is an existing bug on master #288

I think we can go without checking for the specific completion in this PR given that the completion is well provided and inserted in VS Code yaml anyway.

- association done using modeline "# yaml-language-server: $schema=<mySchemaURL"> Signed-off-by: Aurélien Pupier <[email protected]>

JPinkney

LGTM! Only one question, can you set the modelline for each specific yaml doc or does setting it once set it globally for the entire file?

gorkem · 2020-07-22T01:02:01Z

@apupier Is there a reason why we can not use a pattern such as # $schema=<urlToTheSchema>

gorkem · 2020-07-22T01:21:02Z

src/languageservice/services/yamlSchemaService.ts

+     */
+    private getSchemaFromModeline(doc: any) : string{
+        if (doc instanceof SingleYAMLDocument) {
+            const modelineDeclaration = '# yaml-language-server:';


What if I put 2 spaces between # and yaml-language-server:? Do we want it to be this strict?

I wonder if tokenizing the line with something like string.split("/\b\s+(?!$)/"); would result on a more flexible implementation.

I went to the easiest. it is also easier to read code. (for instance, I don't see the regexp that you provided to fit what is expected but i'm not sure if it is just an example or not)
That's possible to use regular expression. It allows to be more flexible for the users.
Not sure it is really needed.
I propose to report an issue to handle it in a separate PR. because there is the case of the double-space between # and yaml-language-server but there is also tabs. and if it is several spaces between the modeline attributes. and surely a bunch of other cases.

string.match(/\S+/g) would be the way to get non-whitespace and it is a loop from there.

We should build this flexibility because at the current implementation if somebody tries to use the feature and misses a single space we fail silently. Or we can build a way to warn the users of such "syntax" errors when we detect the intention but that is essentially the same implementation.

as always not so trivial with regexp. \S is matching also new line and page break Which we don't want here.

that said, given that we are providingline by line, I think we won't have the false-positives thanks to that.

i'm not very concerned for extra-spaces for yaml users, they already choose a language where each space has a meaning.

updated to cover more cases with extra spaces

src/languageservice/services/yamlSchemaService.ts

apupier · 2020-07-22T06:27:16Z

Is there a reason why we can not use a pattern such as # $schema=<urlToTheSchema>

it is to better match modeline like way of writing it. It allows to separate concerns, to clarify in which context it is expected to be used and also it will be easier to add other possible parameters.

That said, technically it is absolutely possible to use directly the notation # $schema=<urlToTheSchema>.

From a functional point of view, # $schema=<urlToTheSchema> is shorter.

apupier · 2020-07-22T06:32:50Z

can you set the modelline for each specific yaml doc or does setting it once set it globally for the entire file?

you can set one per yaml doc.

and it is joining some limitations I thought during the night:

it doesn't work if there is only the comment for completion because there is no document created. I will have a look but this might be not trivial as it seems that we will need to retrieve the content of the fiel from the provided resource uri or to have an empty yamldocument provided when the file is empty instead of no yamldocument but I fear a lot of side effects. it is working, there is a document created when there is just a comment! it is working already. Added a test in the parser
I think that if the comment is in a directive section, it should apply to all documents. unless there is another one defined in document itself. As a first iteration I think it sounds fine to say that using the comment in the directive section is not supported. Maybe worth precising in readme.

…veloper#204 it requires to cast the parameter in 2 places. Signed-off-by: Aurélien Pupier <[email protected]>

Signed-off-by: Aurélien Pupier <[email protected]>

…at-developer#204 it ensures that the schema is well-applied also on documents with only the schema declaration Signed-off-by: Aurélien Pupier <[email protected]>

the "textEdit" is not provided due to an existing bug on master redhat-developer#288 Signed-off-by: Aurélien Pupier <[email protected]>

…veloper#204 Signed-off-by: Aurélien Pupier <[email protected]>

apupier · 2020-07-23T08:14:01Z

src/languageservice/services/yamlSchemaService.ts

+                const schemaMatchs = yamlLanguageServerModeline.match(/\$schema=\S+/g);
+                if(schemaMatchs !== null && schemaMatchs.length >= 1) {
+                    if (schemaMatchs.length >= 2) {
+                        console.log('Several $schema attributes has been found on the yaml-language-server modeline. The first one will be picked.');


How can I send a notification to the client?
(can be done in a different PR)

This can be fine for now. server.ts binds console.log and console.error to the connection.

...have been found...

shadycuz · 2023-08-03T11:40:17Z

@apupier Does this support local schema files? I'm trying this: # yaml-language-server: $schema= "./stack_schema.json" but its not working.

Figured it out # yaml-language-server: $schema=./stack_schema.json

apupier force-pushed the 204-supportSchemaAssociationsInFile branch 5 times, most recently from 4427479 to f145387 Compare July 21, 2020 11:58

apupier requested review from JPinkney and gorkem July 21, 2020 12:04

apupier changed the title ~~[Work In progress / Proof of Concept] 204 support schema associations in file~~ 204 support schema associations in file Jul 21, 2020

JPinkney suggested changes Jul 21, 2020

View reviewed changes

Support schema associations in yaml files redhat-developer#204

f21d6fc

- association done using modeline "# yaml-language-server: $schema=<mySchemaURL"> Signed-off-by: Aurélien Pupier <[email protected]>

apupier force-pushed the 204-supportSchemaAssociationsInFile branch from f145387 to f21d6fc Compare July 21, 2020 15:12

JPinkney approved these changes Jul 21, 2020

View reviewed changes

gorkem reviewed Jul 22, 2020

View reviewed changes

apupier added 2 commits July 22, 2020 08:35

Use type for getSchemaForResource parameter from superclass redhat-de…

6672d2d

…veloper#204 it requires to cast the parameter in 2 places. Signed-off-by: Aurélien Pupier <[email protected]>

Reuse variable redhat-developer#204

a13c6e3

Signed-off-by: Aurélien Pupier <[email protected]>

apupier force-pushed the 204-supportSchemaAssociationsInFile branch from 5368bf9 to a13c6e3 Compare July 22, 2020 06:35

apupier added 2 commits July 22, 2020 09:41

Add test that a yaml document is created when there is a comment redh…

b3ae1c5

…at-developer#204 it ensures that the schema is well-applied also on documents with only the schema declaration Signed-off-by: Aurélien Pupier <[email protected]>

Remove check for precise completion redhat-developer#204

73567b2

the "textEdit" is not provided due to an existing bug on master redhat-developer#288 Signed-off-by: Aurélien Pupier <[email protected]>

apupier mentioned this pull request Jul 22, 2020

Missing textEdit in CompletionItem when there is a newline #288

Open

Support writing modeline with several spaces and characters redhat-de…

de14434

…veloper#204 Signed-off-by: Aurélien Pupier <[email protected]>

apupier force-pushed the 204-supportSchemaAssociationsInFile branch from b87981e to de14434 Compare July 23, 2020 08:12

apupier commented Jul 23, 2020

View reviewed changes

apupier requested a review from gorkem July 23, 2020 09:02

gorkem approved these changes Jul 23, 2020

View reviewed changes

JPinkney approved these changes Jul 23, 2020

View reviewed changes

JPinkney merged commit 4b33c79 into redhat-developer:master Jul 23, 2020

apupier deleted the 204-supportSchemaAssociationsInFile branch July 24, 2020 06:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

204 support schema associations in file #280

204 support schema associations in file #280

apupier commented Jul 15, 2020 •

edited

Loading

apupier commented Jul 21, 2020

JPinkney left a comment

JPinkney Jul 21, 2020

apupier Jul 21, 2020 •

edited

Loading

apupier Jul 22, 2020 •

edited

Loading

JPinkney left a comment

gorkem commented Jul 22, 2020

gorkem Jul 22, 2020

apupier Jul 22, 2020

gorkem Jul 22, 2020

apupier Jul 23, 2020 •

edited

Loading

apupier Jul 23, 2020

apupier commented Jul 22, 2020

apupier commented Jul 22, 2020 •

edited

Loading

apupier Jul 23, 2020 •

edited

Loading

gorkem Jul 23, 2020

fbricon Aug 20, 2020

apupier Aug 20, 2020

shadycuz commented Aug 3, 2023 •

edited

Loading

204 support schema associations in file #280

204 support schema associations in file #280

Conversation

apupier commented Jul 15, 2020 • edited Loading

apupier commented Jul 21, 2020

JPinkney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apupier Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

apupier Jul 22, 2020 • edited Loading

Choose a reason for hiding this comment

JPinkney left a comment

Choose a reason for hiding this comment

gorkem commented Jul 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apupier Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apupier commented Jul 22, 2020

apupier commented Jul 22, 2020 • edited Loading

apupier Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadycuz commented Aug 3, 2023 • edited Loading

apupier commented Jul 15, 2020 •

edited

Loading

apupier Jul 21, 2020 •

edited

Loading

apupier Jul 22, 2020 •

edited

Loading

apupier Jul 23, 2020 •

edited

Loading

apupier commented Jul 22, 2020 •

edited

Loading

apupier Jul 23, 2020 •

edited

Loading

shadycuz commented Aug 3, 2023 •

edited

Loading