Add support for CompletionItem commands #389
Conversation
When we resolve a completion item with a "completionItem/resolve"
request, the LSP server returns a CompletionItem object [0], which may
contain an optional "command" field. This field describes a command that
must be called **after** inserting the completion.
For example the haskell-language-server may respond with the following
command after autocompleting an unimported function (arguments are left
out for brevity):
{'command': '10619:ghcide-extend-import-action:extendImport', 'arguments': [], 'title': 'extend import'}
Then when we send the matching "workspace/executeCommand" request, the
server will respond with a "workspace/applyEdit" request, that
autoimports the function.
Technically the specification dictates that servers should prefer to set
"additionalTextEdits" for that usecase, but there is still some use for
doing it this way, e.g. haskell-language-server also wants to display an
info message to the user, which doesn't happen with
"additionalTextEdits".
In any case it is important to support the "command" field, as it may
also contain actions unrelated to "additionalTextEdits".
[0] https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#completionItem
The workspace edit may add or delete lines, so resetting it to the last linenumber and column will very likely not match the last logical line before the workspaceEdit operation. For example an action might add an auto-import statement at the top of the file, effectively adding one line in-between. If we now reset the cursor, we will end up one logical line before the one where we actually started at. This behaviour is automatically fixed if we just don't reset the cursor at all.
vimpostor
force-pushed
the
completionitem_command
branch
from
f9bf2c4 to
33c9dc3
Compare
|
@yegappan I have also removed the cursor-reset after an workspaceEdit action. It caused problems, when a workspaceEdit inserts or deletes line, as we will then likely end up on a different logical line than it was before. If there was any reason for the cursor-reset, let me know. |
|
As long as the cursor is restored to the previous insertion end point so that the user can continue to enter text after a completion, it is fine to remove the cursor-reset code. I have also seen some problems in correctly restoring the cursor if an import is automatically added or some lines are removed. I will merge this and if we see any additional issues with cursor restoration, we can then address them. |
it actually was the/my concern about cursor etc above (in the review) when/if there were |
|
and the logic seems wrong like before for textadditionaledits : // BTW: @yegappan looks you're a bit hurry to merge recently except my PR. 😄 |
I don't really understand what you are trying to say (the English is hard to understand tbh), but if you know about a specific language server that provides |
if !completionData->has_key('additionalTextEdits')
# Some language servers (e.g. typescript) delay the computation of theit would NOT apply the additionaltextedits when it do had it (vs delayed) now. |
Regression was introduced in 6f4fdc7. The logic was still fine if the LSP server delayed the additionalTextEdits field, but if we already got it, we ended up ignoring it. Fix this by checking for additionalTextEdits outside the delayed-resolve condition. Also see yegappan#389
I understand your point now, sry that if-case somehow slipped through. I fixed it in #390 |
|
simple way should be just keeping additionalTextEdits not touch and leave it alone,
but just adding `command` before it, like following:
// if so, you above removed cursor save/restore code perhaps just leave it there too.
```vim
diff --git a/autoload/lsp/completion.vim b/autoload/lsp/completion.vim
index 0d64e2b..b7480e6 100644
--- a/autoload/lsp/completion.vim
+++ b/autoload/lsp/completion.vim
@@ -583,28 +583,29 @@ def LspCompleteDone(bnr: number)
var completionData: any = v:completed_item->get('user_data', '')
if completionData->type() != v:t_dict
|| !opt.lspOptions.completionTextEdit
return
endif
+ if completionData->has_key('command')
+ # Some language servers (e.g. haskell-language-server) want to apply
+ # additional commands after completion.
+ codeaction.DoCommand(lspserver, completionData.command)
+ endif
+
if !completionData->has_key('additionalTextEdits')
# Some language servers (e.g. typescript) delay the computation of the
# additional text edits. So try to resolve the completion item now to get
# the text edits.
completionData = lspserver.resolveCompletion(completionData, true)
- if !completionData->get('additionalTextEdits', {})->empty()
- textedit.ApplyTextEdits(bnr, completionData.additionalTextEdits)
+ if completionData->get('additionalTextEdits', {})->empty()
+ return
endif
endif
- if completionData->has_key('command')
- # Some language servers (e.g. haskell-language-server) want to apply
- # additional commands after completion.
- codeaction.DoCommand(lspserver, completionData.command)
- endif
-
+ textedit.ApplyTextEdits(bnr, completionData.additionalTextEdits)
enddef
# Initialize buffer-local completion options and autocmds
export def BufferInit(lspserver: dict<any>, bnr: number, ftype: string)
if !lspserver.isCompletionProvider
# no support for completion
```
// but anyway, if they (additionalTextEdits and command) were both there, i am curious
// cursor would be correct? since one of them may changed the pos stored in 'user_data'
//
// or perhaps generally they actually not possible do the both at same time.
…--
shane.xb.qian
|
|
all in all, your PR was just adding following before additionaltextedits : + if completionData->has_key('command')
+ # Some language servers (e.g. haskell-language-server) want to apply
+ # additional commands after completion.
+ codeaction.DoCommand(lspserver, completionData.command)
+ endifwould you mind to change it to this way? it would be the less change and less impact. |
It was intentionally removed to avoid the logical line changing after the user confirmed the completion. That was an actual problem that was happening with haskell-language-server and other LSPs as well. Can you describe any scenario where there is buggy behaviour because the cursor is not manually reset? Personally I don't know any such scenario. But if you know one, I am happy to find something that works in both cases.
I am not entirely sure, but to me it sounds like |
This patch implements support for the optional "command" field in the
CompletionItemresponse to a "completionItem/resolve" request: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#completionItemSome language servers (e.g. haskell-language-server) use this field to apply other edits to the document (e.g. auto-importing functions).
While the spec technically says that "additionalTextEdits" should be used for that usecase (also see #367), apparently not all servers do and I can see the reason, because haskell-language-server also wants to show an "window/showMessage" info message to the user about the auto-import.