Adding Did Change Configuration feature #802
Conversation
Signed-off-by: Adam Knapp <adam.knapp@ericsson.com>
| * @return {@link Map} containing the requested preference name and its value | ||
| */ | ||
| @SuppressWarnings("null") | ||
| default @NonNull Map<String, Object> getConfiguration(final String prefName) { |
There was a problem hiding this comment.
Some servers asks for concrete preference (for example vue language-server) in this case getConfiguration can be for example boolean
There was a problem hiding this comment.
Sorry I don't get this. What do you mean by "can be"? The return value or the parameter or what do you mean?
| @Override | ||
| public void propertyChange(PropertyChangeEvent event) { | ||
| final IConfigurationHandler configHandler = serverDefinition.getConfigurationHandler(); | ||
| languageServer.getWorkspaceService().didChangeConfiguration( |
There was a problem hiding this comment.
How about configuration sharing? For example HTML LS can ask for CSS or JAVASCRIPT settings or just workspace settings (http)
There was a problem hiding this comment.
I think any LS can ask for any section, however the LS is not aware of what configuration sections are available in/at the client. So by default the configuration is shared, just need to ask the client properly.
|
Please note that WWD manages HTML, CSS, JS settings https://github.com/eclipse-wildwebdeveloper/wildwebdeveloper/blob/bd4d90213bee415549706c875dbe3ce348e26bda/org.eclipse.wildwebdeveloper/src/org/eclipse/wildwebdeveloper/html/HTMLLanguageClient.java#L42 I wonder if this PR will simplify the code of WWD. |
Based on the code you linked, the WWD LS has to ask for the configuration in every case. Contrarily, the didChangeConfiguration message is a notification about the change of the settings. The former mechanism is not influenced by the latter one. So the LS can have both if the developers like. In our case, the user can have such preferences that influence the code analysis at the server side. So we needed the configuration of the client asap, preferably before starting the analysis. Therefore, we "copied" the VSCode behavior by sending the whole client configuration right after the initialization messages. |
|
I'm a bit unsure about this one as there are many many things that could trigger a configuration change, and I'm unsure about whether 1 API could successfully encompass all cases. For example. a configuration can be derived from one or more preference store, with or without translation of preference ids, or can be derived from a configuration file, or can be triggered by some custom menu action, or even by a codeAction that would trigger a client side action... |
|
I'm thinking about something different for eclipse-wildwebdeveloper/wildwebdeveloper#1335
|
|
See my proposition: #835 and example implementation for wildwebdeveloper ;) |
|
The initial target branch |
The intention of this PR is adding the Did Change Configuration feature of the LSP. The initial/last configuration is sent after the initialized notification message is received, similarly to VS Code. Then each preference store change triggers a
didChangeConfigurationmessage to the server. This behavior provides that the language server acquires a meaningful configuration before starting analyzing any file/workspace/project coming from the LSP4E client.The plugin developers can add their own implementation via a new extension point called
configurationHandlerand by implementing theIConfigurationHandlerinterface. This interface comes with a default implementation that basically sends empty messages that can be easily neglected by the language server.I think the idea is understandable from the code, I tried to add meaningful comments as well, however, feedbacks are welcome.