workspace/didChangeConfiguration not sending correct params to YAML language server when deployed by plugin ID

[JPinkney]

Hi,

I'm trying to get the YAML language server to work when deploying a plugin by id but there seems to be an issue with what's being sent in the didChangeConfiguration. From what I can see Theia is sending

[Trace - 00:11:22] Sending notification 'workspace/didChangeConfiguration'.
Params: {
    "settings": {
        "yaml": {}
    }
}

request to the YAML language server, even when changing a value in the preferences. However, I would expect to see something like

[Trace - 00:11:22] Sending notification 'workspace/didChangeConfiguration'.
Params: {
    "settings": {
        "yaml": {
            "completion": true
        }
    }
}

Steps to reproduce:
Use the deploy by plugin id with "vscode:extension/redhat.vscode-yaml"

More info:
Right after loading if you look in the output you should see an error saying something like "Notification handler 'workspace/didChangeConfiguration' failed with message: Cannot read property 'singleQuote' of undefined"". This is probably because the language server expects settings.yaml.format.singleQuote to be sent but since settings.yaml.format is not sent the property singleQuote is not found for an undefined. This will be fixed on YAML LS side with relevant discussion here

[akosyakov]

@JPinkney are you talking about https://github.com/theia-ide/theia-yaml-extension?

YAMLClientContribution does not seem to define configurationSection. Could you have a look into it?

[JPinkney]

Its using vscode-yaml via the deploy by plugin ID command not the YAML extension. I was able to get it loaded using these instructions eclipse-che/che#12142 (comment) but couldn't get much past that because of this issue

[akosyakov]

ok, sounds like a bug in the plugin extension then, I would start by looking around PreferenceRegistryExtImpl

BTW looking closely, plugins and language extensions use different code to listen to configuration changes. Language extension is using MonacoConfigurations and as far as I know, they receive all events given that configuration section provided. Plugins are using PreferenceRegistryExtImpl. It would be good to merge them somehow to remove all duplicate code and fix bugs only once.

[JPinkney]

Is there a way to see the subscribers to an event? I'm able to follow events from preference-service.ts to preference-registry-main.ts which then goes to preference-registry.ts but upon entering this method I have no idea where this._onDidChangeConfiguration.fire is being sent to. I've verified that this.toConfigurationChangeEvent(eventData) is working correctly but after that I'm not sure what piece of code is being hit that will eventually lead to the workspace/didChangeConfiguration notification. Using find all references on onDidChangeConfiguration leads to https://github.com/theia-ide/theia/blob/master/packages/plugin-ext/src/plugin/plugin-context.ts#L354 but whats never hit.

[JPinkney]

Report I've what I've found so far:
Looking at the onDidChangeConfiguration from vscode-languageserver-node it seems that when the onDidChangeConfiguration is hit for the YAML language server the configurationSection variable is set to undefined. This causes that part of the code to send

[Trace - 00:11:22] Sending notification 'workspace/didChangeConfiguration'.
Params: {
    "settings": {
        "yaml": {}
    }
}

My theory was that the initialize portion where the section is registered was not being hit and thus causing the registerOptions.section to not be set here. This would then lead to data.registerOptions.section to be undefined (since it hasn't been registered to YAML section). This lead me to believe that the registration of languages is somehow wrong for the plugin system. To me it seems like the issue has to do with something related to the difference registration/initialization of the plugins vs how its regularly done through extensions in Theia.

The issue is also reproducible with VSCode-Java as well.

[JPinkney]

This does not seem to be an issue anymore so I'm going to close.