This is a draft PR with a proposal of a way of addressing an issue we have with how we handle the initializationOptions. Currently, the value we send on initializationOptions on the initialize request is the value for the key initializationOptions on the settings file, but that doesn't seem correct to me for two reasons:
it assumes that it's ok to send the same initialization options to two different servers (if you are working on a multi-language project with two different servers)
it makes things difficult for the workspace/configuration implementation, as the servers (at least rust-analyzer, gopls and pyls) seem to assume that the root section is a section with the name of the server. So for example if a server wants the configuration for rust-analyzer.inlayHints.chainingHints it would fail to get that now as there is no section named rust-analyzer (as we use a section named initializationOptions instead).
Fixing this issue is a little tricky, because we don't know the name of the server prior to issuing the initialize request, so we wouldn't know what section to get from the settings file. Other clients don't have this problem because they either are a client for a specific server (for example rust-analyzer's VSCode extension, it knows it should send the section rust-analyzer in the settings file), or because they implement some other way to populate the initialization options (vim-lsc uses a pre-request hook for that).
Fixing this will also enable users to use a single global settings file, as each server's settings will be under it's own section. For example, users will be able to have a settings.json file like this one:
{
"gopls": {
},
"rust-analyzer": {
}
}
The above is currently impossible to configure, as we don't differentiate settings for different servers.
To fix this breaking as little as possible, I propose that we add a new format for specifying the serverCommands. So the idea is to accept what we accept now (a list of strings with the command and arguments) or an object with a commands key which is the same list of strings we accept now and an optional server_name key in that object which will be used to identify the root section for that language server. For example something like this would be a valid config for serverCommands:
To avoid breaking too many things, in the case where the user specifies the command in a list of strings like we do now, I attempted to make a best effort guess of the name of the server by assuming the first item in the list is a binary (or a path to one) with the name of the server (/path/to/rust-analyzer would yield rust-analyzer as server name). This is a little nasty, but to remove this I think we would have to introduce a breaking change in the configuration (maybe that's better?).
Note that the server command now also includes an optional initializationOptions key, that acts as global settings for that specific server and will be merged with the default server settings we define in code (java only) and with the workspace settings defined by the user. The order in which they are merged are default options -> server options -> workspace options, so the workspace local options will be preferred over the server ones and the server ones over the default ones.
Also, to aid existing users in identifying issues with their current configuration, an error message will be shown to the user when we detect a settings file with an initializationOptions key, the message will direct them to see the help for serverCommands which will indicate the proper configuration.
I considered a few other alternatives to how we specify the server commands. There was one that I was very tempted to implement, which was basically to have servers keyed by their server name instead of the filetype they apply to.
let g:LanguageClient_servers = {}
let g:LanguageClient_servers.gopls = {
\ 'command': ['gopls'],
\ 'filetypes': ['go', 'gomod'],
\ }
This would enable running more than one server for the same filetype, which maybe is something we want to do in the future? I didn't go through with it because it seemed like it would introduce a whole new set of problems to solve that I wasn't planning to address with this PR, but maybe it's something that we can look into after this one.
This is a draft PR with a proposal of a way of addressing an issue we have with how we handle the
initializationOptions. Currently, the value we send oninitializationOptionson theinitializerequest is the value for the keyinitializationOptionson the settings file, but that doesn't seem correct to me for two reasons:workspace/configurationimplementation, as the servers (at leastrust-analyzer,goplsandpyls) seem to assume that the root section is a section with the name of the server. So for example if a server wants the configuration forrust-analyzer.inlayHints.chainingHintsit would fail to get that now as there is no section namedrust-analyzer(as we use a section namedinitializationOptionsinstead).Fixing this issue is a little tricky, because we don't know the name of the server prior to issuing the
initializerequest, so we wouldn't know what section to get from the settings file. Other clients don't have this problem because they either are a client for a specific server (for example rust-analyzer's VSCode extension, it knows it should send the sectionrust-analyzerin the settings file), or because they implement some other way to populate the initialization options (vim-lsc uses a pre-request hook for that).Fixing this will also enable users to use a single global settings file, as each server's settings will be under it's own section. For example, users will be able to have a settings.json file like this one:
The above is currently impossible to configure, as we don't differentiate settings for different servers.
To fix this breaking as little as possible, I propose that we add a new format for specifying the
serverCommands. So the idea is to accept what we accept now (a list of strings with the command and arguments) or an object with acommandskey which is the same list of strings we accept now and an optionalserver_namekey in that object which will be used to identify the root section for that language server. For example something like this would be a valid config forserverCommands:To avoid breaking too many things, in the case where the user specifies the command in a list of strings like we do now, I attempted to make a best effort guess of the name of the server by assuming the first item in the list is a binary (or a path to one) with the name of the server (/path/to/rust-analyzer would yield rust-analyzer as server name). This is a little nasty, but to remove this I think we would have to introduce a breaking change in the configuration (maybe that's better?).
Note that the server command now also includes an optional
initializationOptionskey, that acts as global settings for that specific server and will be merged with the default server settings we define in code (java only) and with the workspace settings defined by the user. The order in which they are merged aredefault options -> server options -> workspace options, so the workspace local options will be preferred over the server ones and the server ones over the default ones.Also, to aid existing users in identifying issues with their current configuration, an error message will be shown to the user when we detect a settings file with an
initializationOptionskey, the message will direct them to see the help forserverCommandswhich will indicate the proper configuration.I considered a few other alternatives to how we specify the server commands. There was one that I was very tempted to implement, which was basically to have servers keyed by their server name instead of the filetype they apply to.
This would enable running more than one server for the same filetype, which maybe is something we want to do in the future? I didn't go through with it because it seemed like it would introduce a whole new set of problems to solve that I wasn't planning to address with this PR, but maybe it's something that we can look into after this one.
Related: #1090 Closes #1107