Allow multiple language server with lsp-workspace-command #10176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fix allows for multiple language servers at once which support workspace commands. This was previously broken as just the first language server supporting workspace commands was queried when listing allowed worspace commands. The fix is in two parts. Firstly, querying all workspace commands from all language servers available and using them when actually running the command in `lsp_workspace_command`. Secondly, doing the same in `completers::lsp_workspace_command` such that completion still works as expected. The fix has one remaining issue, which I am unsure how to handle in the best way possible, but which I also don't think should happen often: Multiple language servers may register commands with the same name. This will lead to that command being listed in the popup menu and in the completion list multiple times, which can be possibly confusing. One could disambigue them in the popover menu, but I am not sure the same can be done for completion. When running `lsp-workspace-command` with parameters, this behavior is "fixed" by displaying an error in that case. I am unsure if this is the best fix for this issue in that case, but could not find a better one.
Schmiddiii
commented
Apr 6, 2024
| @@ -1406,21 +1420,32 @@ fn lsp_workspace_command( | |||
| cx.jobs.callback(callback); | |||
| } else { | |||
| let command = args.join(" "); | |||
There was a problem hiding this comment.
I am unsure about this one. Is there some LSP which has a space in the command name? I would personally like this to be such that the first argument is the command and the remaining ones are arguments to that command. But I have kept it as it was for now.
the-mikedavis
added
A-language-server
pascalkuthe
approved these changes
Apr 8, 2024
There was a problem hiding this comment.
lgtm but may want to delay after #9647 to avoid stalling that pr with endless conflicts
|
That is fine by me. I will keep an eye on the PR and rebase when it is merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fix allows for multiple language servers at once which support workspace commands. This was previously broken as just the first language server supporting workspace commands was queried when listing allowed worspace commands.
The fix is in two parts. Firstly, querying all workspace commands from all language servers available and using them when actually running the command in
lsp_workspace_command. Secondly, doing the same incompleters::lsp_workspace_commandsuch that completion still works as expected.The fix has one remaining issue, which I am unsure how to handle in the best way possible, but which I also don't think should happen often: Multiple language servers may register commands with the same name. This will lead to that command being listed in the popup menu and in the completion list multiple times, which can be possibly confusing. One could disambigue them in the popover menu, but I am not sure the same can be done for completion. When running
lsp-workspace-commandwith parameters, this behavior is "fixed" by displaying an error in that case. I am unsure if this is the best fix for this issue in that case, but could not find a better one.Fixes #9017.