vidispine / hull

The incredible HULL - Helm Uniform Layer Library - is a Helm library chart to improve Helm chart based workflows
https://github.com/vidispine/hull
Apache License 2.0
230 stars 13 forks source link

Is JSON schema supposed to work with yaml-language-server ? #340

Closed eshepelyuk closed 2 weeks ago

eshepelyuk commented 3 weeks ago

I am trying to use HULL values.yaml JSON schema with yaml-language-server. In particular using NeoVim 's LSP client.

So I've added it to LSP setting, but I am experiencing weird behaviour.

  1. Opening empty document - it's working in the beginnig, I can see LSP suggestions (ones with Property) image
  2. But when I put something under hull - LSP suggestions are not provided anymore image
  3. Schema validation is not working.
  4. But the object/tag information\help from JSON schema is working, even in existing YAML document. image

I do believe it's something related to the yaml-language-server - since it's working OK on other schemas. I am not asking for fixes or investigation, just trying to understand if anyone from maintainers tried to use HULL yamls with yaml-language-server, is this maybe known issue etc.

Thanks in advance.

gre9ory commented 2 weeks ago

Out of curiosity, I have played around a little with schema validation and auto-completion in VSCode. Haven't used it in VSCode for a while. Setup is the documented one...

While I get those popups on mouseover do work consistently, the UI support for auto-completion is definitively shaky. However, one thing I noticed and can seemingly reproduce in my tests:

At the root of the schema (very bottom of values.schema.json) is an anyOf switch:

  "anyOf": [
    {
      "$ref": "#/definitions/root"
    },
    {
      "$ref": "#/definitions/hull"
    }
  ]

The first case directly loads the "#/definitions/root". This has the version, config and objects sub keys.

The second case defines the hull root key as start of the tree and then points to the same "#/definitions/root" for its possible children.

The reason why there are two is that the schema should both work in the HULL's internal values.yaml itself (where there is no hull root key) and of course in all charts using HULL and thus having root key hull.

Now in the current order it seems that - at least in VSCode - I don't get auto completion loaded for the hull root key. I only get suggestions for version, config and objects though.

If I switch the order in values.yaml.json it looks as if it processes the hull key based subpath of the schema first (or maybe only then?). In this order I get auto-completion support for hull subkeys (I do have to make a dummy change to the schema file path for the schema change to take effect in VCode).

I made a little video where I switch from current order to changed order and dummy trigger a schema reload. You can see that in the "hull first" order it gives me a proper auto-completion and when I switch order back to loading root first it is gone again.

https://github.com/user-attachments/assets/b584f3b2-e39e-4673-824e-2105fe6067be

Logically I would argue the order of the anyOf elements should not matter. Practically it is maybe an issue with the schema size and for whatever reason it is not the same outcome if it processes the root schema path first.

To me it looks as if the VSCode also relies on this yaml-language-server even though I am no expert on the subject matter:

image

If possible, could you do a local test where you switch the order as explained in the values.yaml.json? Think you can easily do that outside of the Helm scope since it just involves the schema file and your editor. If it improves the auto-completion I would integrate this change into the next version if no other issues arise.

PS: For what its worth I also changed that value in VSCode settings from 5000 to 50000. Don't know if it plays a role or not.

"yaml.maxItemsComputed": 50000,

eshepelyuk commented 2 weeks ago

Yes, swapping those sections fixed an issue in my NeoVim ( downloaded schema and using it from local disk )

gre9ory commented 2 weeks ago

So all good now?

If yes, will add it to next release. For now, I think it is ok if you just point to your local copy of values.yaml.json?

eshepelyuk commented 2 weeks ago

It's much much better then before :) So, all users would be grateful if you can include this fix into a mainstream sources.

Thanks for your efforts on researching the core issue.