vim
vim copied to clipboard
feat: extend complete_add function
complete_add support adding matches to list of current completion
What is the benefit of this over calling complete() again with longer list? complete_add() needs to redraw the screen anyway and will cause the same type of flicker as complete(), no? Also, if list gets smaller, there is no complementary complete-remove().
Calling complete again will not redraw? Complete will also call ins_compl_del_pum and then rebuild pum. LSP allows multiple clients to handle completion requests. Currently complete_add will use del_pum false to prevent ins_compl_add from deleting pum. You can check the linked issue to understand the problem. compelte_remove is also needed, but I want to confirm the behavior of PR first
What is this purpose of this PR?
-
First, the LSP specification includes an
isIncompleteflag to indicate whether the returned completion results are complete. If the results are incomplete, the next input will still trigger a completion request that returns additional results, which need to be merged with the previous ones. However, if the complete function is called again, the previous matches will be cleared. -
Second, a buffer may be associated with multiple LSP servers. When a completion request is triggered, the results from multiple servers will not be returned simultaneously, so it is necessary to append the matches.
ref https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#completionList
Shouldn't LSP Clients cache incomplete results in both scenarios?
-
When
'isIncomplete'is set totrue, the LSP specification does not specify whether the next response should be incremental or a full replacement. LSP clients need to handle both cases. -
An LSP client often aggregates results from multiple LSP servers and other sources, such as buffer searches. It may also provide an omnifunc interface. In all these cases, properly handling and presenting partial results requires caching intermediate results.
I would expect modern LSP clients to have already solved this issue. You may verify this with the authors.
How to cache when the characters are different? . complete_add exists to extend matches but is currently too limited. linked issue is sufficient to explain the problem
I read the issue you linked, but it is not clear.
If the LSP client can choose to append to the existing match list instead of replacing it entirely, it can always use complete_info() to retrieve all matches, modify the list (append/insert/delete), and then call complete() again. This approach offers greater flexibility.
The issue you linked looks like a bug in LSP client implementation in neovim. client2 should be triggered on ab, even if b is not a trigger character for client2. When gathering completion list from multiple servers, maybe the correct approach is to trigger all servers for all trigger chars (even if specific trigger char is not associated with a server) and let the server decide.
it can always use complete_info() to retrieve all matches, modify the list (append/insert/delete), and then call complete() again. This approach offers greater flexibility.
It is feasible. but the time complexity here increases with the number of items. Secondly, when the merge process is completed, calling complete again on the C side still requires memory allocation. Isn't this more wasteful? and calling compelte_info also requires memory allocation.
Hm, I am still not sure about this. The use case seems quite niche and a user reading the help file, won't be necessarily clear when to use complete() instead of complete_add(..., 1). Also having to specify the completion column seems a bit restrictive, no? Or other way around, how would the completion engine know what column to use here?
Hm, I am still not sure about this. The use case seems quite niche and a user reading the help file, won't be necessarily clear when to use
complete()instead ofcomplete_add(..., 1). Also having to specify the completion column seems a bit restrictive, no? Or other way around, how would the completion engine know what column to use here?
The documentation states: “The {startcol} must match the current completion column.” In practice, startcol serves more as a placeholder to differentiate between two complete_add() signatures, rather than being meaningfully used. The two signatures serve entirely different purposes — one adds entries to the list before it is displayed, while the other appends to a list that is already visible to the user. There are other issues as well, as I explained above. I'm unsure why it's still being pursued in its current form.
I tend to agree
client2 should be triggered on ab
I think you didn't see the problem clearly. b triggers textDocumention/completion request again because isIncomplete, how to merge the new result with all existing results. Your approach is to get complete_items from complete_info and then get lsp from user_data structure and then merge. How much memory allocation efficiency is there here. This is not a niche problem. There will be many lsp clients in one buffer.
This PR appends the new request results and updates pum, complete_add needs to specify an existing startcol to ensure that it is the same as the current. Or the complete function returns an id and uses this id as the remaining book.
let's not do it.