Duplicate "CompletionDone" events fired with multiple LSPs

[mhartington]

trafficstars

• nvim --version: v0.5.0-dev+1016-gd003497ff-dirty (built from source)
• vim -u DEFAULTS (version: ) behaves differently?
• Operating system/version: macOS 11.1
• Terminal name/version: kitty
• $TERM: xterm-256

Steps to reproduce using nvim -u NORC

nvim -u NORC
# Alternative for shell-related problems:
# env -i TERM=ansi-256color "$(which nvim)"

Actual behaviour

While debugging #13414, it seems if you have multiple LSPs attached to the same buffer, when you trigger omni-completion there will be multiple CompletionDone events fired for each server. This seems to be a similar issue to what @tjdevries fixed in #12655

Expected behaviour

When completion is finished, it should only fire CompletionDone once for all servers attached.

[mjlbach]

Isn't this expected? vim.fn.complete is called once per server completion result in the callback function passed to the buf request, each of which should trigger CompleteDone

https://github.com/mjlbach/neovim/blob/3d25a72a60323c9d6eab2daf9dbd4920b94b8a94/runtime/lua/vim/lsp.lua#L1233-L1239

[mhartington]

yeah, it's to be expected with the current implementation. Though in other environments, this is not the case, like vscode. Requests are sent to the server and queued up before returning a response. So for N number of servers attached to a buffer, the requests will be made, a single table will be created with the results, and the call back will be called with a combined set of results.

[mjlbach]

Right, that's true, but the advantage of our current implementation is that vim.fn.complete is called as soon as the request returns, allowing async population of the return results. If we wait until each completion result returns, and then aggregate the results before populating the completion list, the bottleneck will be the slowest server. I'm not sure if complete_add triggers CompleteDone, but one option would be to open the completion menu, and then call complete_add in the callback functions. The disadvantage being I'm not sure how slow complete_add is, because I believe it can only add a single completion result per call.

[MagicDuck]

Instead of CompleteDone, should it not be CompleteChanged?

CompleteChanged 					*CompleteChanged*
				After each time the Insert mode completion
				menu changed.  Not fired on popup menu hide,
				use |CompleteDonePre| or |CompleteDone| for
				that.

I agree with @mjlbach that async population of results is better. In my case (#14330), I work with a large project and tsserver can be a bit slow to provide code actions sometimes, but would still like to see the code actions from the other language server. I don't think this is clearly documented now though, because every LSP UI I've tried using, like lsp-saga, etc. assumes that there is only one handler call. Even the default textDocument/codeAction handler assumes that as it blocks for user input. I think it would help to clarify this so that users/plugin implementers know what to expect.