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

Add schema info to diagnostics #310

Closed
gorkem opened this issue Aug 21, 2020 · 17 comments · Fixed by #326
Closed

Add schema info to diagnostics #310

gorkem opened this issue Aug 21, 2020 · 17 comments · Fixed by #326
Assignees
Milestone

Comments

@gorkem
Copy link
Collaborator

gorkem commented Aug 21, 2020

when validation against a schema fails also add the schema information to the published diagnostics. This way users can identify the schemas that is affecting their YAML files.

@evidolob
Copy link
Collaborator

evidolob commented Sep 24, 2020

@gorkem @JPinkney @joshuawilson
I have prototype of this, I add scheme URL in source field of Diagnostic
Example URL:
Screenshot 2020-09-24 at 14 52 36
Example local file:
Screenshot 2020-09-24 at 14 53 02

WDYT?

@JPinkney
Copy link
Contributor

That looks good to me

@gorkem
Copy link
Collaborator Author

gorkem commented Sep 24, 2020

Are you sending it as part of the message filed on Diagnostic?

@evidolob
Copy link
Collaborator

I not sure that understand the question, I add URL/path to Diagnostic#source field

@gorkem
Copy link
Collaborator Author

gorkem commented Sep 25, 2020

That is exactly what I was asking.
Can you also provide a screenshot the problem view when you have one of these issues.

@evidolob
Copy link
Collaborator

Screenshot 2020-09-28 at 10 28 25

@gorkem
Copy link
Collaborator Author

gorkem commented Sep 28, 2020

OK this is half bad :) Can we have a format such as yaml-schema: ${schema} and for generic yaml issues just yaml

@evidolob
Copy link
Collaborator

We can, but what is the value of ${schema} ? URI or just name?
We can use schema title if present, if not, than URI.

@gorkem
Copy link
Collaborator Author

gorkem commented Sep 29, 2020

We can use schema title if present, if not, than URI.
+1 Sounds like a plan

evidolob added a commit to evidolob/yaml-language-server that referenced this issue Sep 30, 2020
@evidolob evidolob added this to the Sprint #191 milestone Oct 7, 2020
evidolob added a commit that referenced this issue Oct 15, 2020
* #310 Add schema info to diagnostics

Signed-off-by: Yevhen Vydolob <[email protected]>

* Show schema url if file has multiple schemas

Signed-off-by: Yevhen Vydolob <[email protected]>

* Fix k8s url

Signed-off-by: Yevhen Vydolob <[email protected]>

* Update test/schemaValidation.test.ts

Co-authored-by: Josh Pinkney <[email protected]>

* Remove commented code

Signed-off-by: Yevhen Vydolob <[email protected]>

* Set schema url and title to child schemas

Signed-off-by: Yevhen Vydolob <[email protected]>

Co-authored-by: Josh Pinkney <[email protected]>
@rivy
Copy link

rivy commented Oct 15, 2020

Thanks, nice addition!

@ssbarnea
Copy link
Member

I looked for the entire morning on how to expose this information and I still do not know how to enable the diagnosticst. Was this fixed only in an unreleased version? -- I am currently using the latest YAML 0.15.0 extension, which was released two weeks ago, so I expect to be able to see the diagnostics. Is there a hidden option to enable these?

@evidolob
Copy link
Collaborator

@ssbarnea this works since 0.12.0

@ssbarnea
Copy link
Member

@evidolob So why do I see only this:

@evidolob
Copy link
Collaborator

I discuss that there #310 (comment)
Becouse your schema has title field we show it first and then show shema url.

@evidolob
Copy link
Collaborator

And you can use Jump to schema location CodeAction to open schema in editor.

@ssbarnea
Copy link
Member

ssbarnea commented Feb 23, 2021

The string that is displayed is not really the schema name but is the title of the schema property that generated the error, which does not give any hint about which schema was effectively used. Take a look at one file like https://github.com/ansible-community/ansible-lint/blob/schemas/src/ansiblelint/data/ansible-playbook-schema.json and you will see lots of titles around.

So yet, we do display information from within the schema but not the schema information itself.

Combined with the fact that user can enable the schemaStore, you endup with a plethora of possible sources for schemas, some configured by local patterns and some determined by the schemastore. Still, as a user I have no clue from which file this came.

I am not sure where is the "Jump to schema location" available from, as by pressing F1 (CommandPallete) I am unable to find it and I neither see the light-bulb contextual action available on the error, as documented on https://code.visualstudio.com/docs/editor/refactoring#_code-actions-quick-fixes-and-refactorings -- (i did not disable it in settings).

My yaml related settings look like:

  "[yaml]": {
      "editor.insertSpaces": true,
      "editor.quickSuggestions": {
          "comments": true,
          "other": true,
          "strings": true
      },
      "editor.tabSize": 2
  },
  "yaml.completion": true,
  "yaml.customTags": [
      "!include-raw:"
  ],
  "yaml.format.enable": true,
  "yaml.schemas": {
...
  },
  "yaml.schemaStore.enable": false,
  "yaml.trace.server": "verbose",
  "yaml.validate": true

@evidolob
Copy link
Collaborator

Oh, my bad Jump to schema location is not released yet.

As for

The string that is displayed is not really the schema name but is the title of the schema property that generated the error, which does not give any hint about which schema was effectively used. Take a look at one file like https://github.com/ansible-community/ansible-lint/blob/schemas/src/ansiblelint/data/ansible-playbook-schema.json and you will see lots of titles around.

So yet, we do display information from within the schema but not the schema information itself.

Combined with the fact that user can enable the schemaStore, you endup with a plethora of possible sources for schemas, some configured by local patterns and some determined by the schemastore. Still, as a user I have no clue from which file this came.

Can you create an issue for it? We can fix that pretty fast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants