Add priority setting for servers handling the same language #588
Conversation
| let servers = this.available_servers.filter( | ||
| server => server.spec.languages.indexOf(language) !== -1 | ||
| let server_ids = this._connection_manager.language_server_manager.getMatchingServers( | ||
| { language: document.language } | ||
| ); | ||
| if (servers.length > 1) { | ||
| console.warn('More than one server per language for', language); | ||
| } | ||
| if (servers.length === 0) { | ||
| if (server_ids.length === 0) { | ||
| continue; | ||
| } | ||
| let server = servers[0]; | ||
| // For now only use the server with the highest priority |
There was a problem hiding this comment.
Should probably use the connection + server Identifier here. Also should we restart all connections if preferred language server changed our just add a note that JupyterLab reload is needed for changes to go into effect? Probably the latter
There was a problem hiding this comment.
i think wherever possible, settings changes should take effect immediately, since have established the pattern... can see how it would be a tad onerous
There was a problem hiding this comment.
Right. But I would prefer not to do this now given the plan to rewrite it later to support multiple connections per document which would invalidate the logic; lets consider the priority setting a tool for us to test multiple servers on CI for now, and a user-facing feature once it starts defining priority between multiple servers running in tandem.
| @@ -47,7 +47,7 @@ Moving Cells Around | |||
| Foreign Extractors | |||
| ${file} = Set Variable Foreign extractors.ipynb | |||
| Configure JupyterLab Plugin | |||
| ... {"language_servers": {"texlab": {"serverSettings": {"latex.lint.onChange": true}}, "bash-langauge-server": {"bashIde.highlightParsingErrors": true}}} | |||
| ... {"language_servers": {"texlab": {"serverSettings": {"latex.lint.onChange": true}}, "bash-langauge-server": {"bashIde.highlightParsingErrors": true}}, "pylsp": {"priority": 1000}} | |||
There was a problem hiding this comment.
getting long... maybe we hoist to a fixture file?
There was a problem hiding this comment.
Actually, could we have Configure JupyterLab Plugin merge the new settings with the default priorities (unless custom priorities are given)? This should simplify tests that have custom config.
There was a problem hiding this comment.
Or just move "pylsp": {"priority": 1000} to a fixture file. I think we used to have a json file for frontend settings but cannot see it.
|
On the main looks good. I don't remember where, but I thought there was an issue where someone wanted the "full deal" on python source and another server on notebooks, but don't remember where. If we're not going to do it now, certainly don't need to talk about it in the code anywhere! |
References
Add initial priority setting as discussed in #587
Code changes
getServerIdwithgetMatchingServerswhich returns names of all matching servers, sorted by priority (even though we only use the first one now, it will come in handy later to have an ordered list of those)TServerKeys = TLanguageServerIdto avoid too broad types (one was a string, the other was a literal union; now both use the literal union and one could be removed but keeping it for now because they have distinct meaning)User-facing changes
Backwards-incompatible changes
Chores