Language-server-protocol: What's the expected behaviour of CompletionList.isIncomplete=false when the user presses backspace?

[DanTup]

My understanding is that if IsIncomplete=true, the client will not cache results and must go back to the server whenever the user types a character (this allows the server to return a partial set of results).

Currently in Dart we use IsIncomplete=false - eg. we give the full list to VS Code and allow it to filter client side.

There has been an assumption that I don't think is well described in the spec, relating to whether the client should supply all possible completions given the current prefix or all possible completions for the current location regardless of prefix.

For example, let's say my completion list simple has:

• one
• two
• three

If the user invokes completion where ^ is here:

print(on^

If the server is using IsIncomplete=false (eg. it's providing the full list), should it include two and three here, on the assumption that the client will not call the server again even if the user hits backspace, or should it assume that the client-side filtering is only for typing forwards?

The spec is a bit vague on this (it's not clear what "Further typing" covers, and "recomputing" is also a little vague - I presume it means "the client should re-request completions from the server").

This list it not complete. Further typing should result in recomputing this list.

If pressing backspace is intended to re-call the server even when IsIncomplete=false, then significant gains could be made for us by filtering based on the prefix on the server.

Thanks!

[DanTup]

@dbaeumer do you know the answer to this? I don't mind sending a PR to clarify the docs as long as someone confirms the correct answer.

[kdvolder]

If pressing backspace is intended to re-call the server even when IsIncomplete=false, then significant gains could be made for us by filtering based on the prefix on the server.

I've always assumed that server is at least free to take the prefix into account. If not, then the number of completions could be 100s or even 1000s for some cases. So filtering based on prefix is almost mandatory for performance/practicality.

However, I do agree with @DanTup, the spec is unclear about it.

[DanTup]

I've always assumed that server is at least free to take the prefix into account

I assumed the opposite, on the assumption this really meant "the client never needs to ask the server again for this completion 'session'" 😄

I think for most cases, the completion opens with no prefix, so the saving is probably not for the majority of invocations, but it'd definitely be nice to save some bytes (especially for things like LiveShare, which sends the entire payload over the internet) if it can be confirmed that clients should re-request from the server when deleting the original prefix.

[kdvolder]

I assumed the opposite,

All the more reason the spec really needs to be clarified. I agree either interpretation is possible with the current wording. I suspect the spec may even be deliberately vague to allow both types of server implemenations.

I think for most cases, the completion opens with no prefix, so the saving is probably not for the majority of invocations,

I think it realy depends a lot on the specifics of the language or even the context in which the completions are being invoked. In some contexts a complete 'no prefix' completion request is going to return unmanageable number of results. (E.g. all the classes on your project's classpath, or all the functions defined in all the code + libraries).

Those kinds of things will tend to return 100s or 1000s of results unless they are somehow 'narrowed' down. I think a typical server implementation would have to be practical and just return a 'incomplete' list of result in such cases, and limit it to some small number of results. Then as user types more, the server takes the prefix into account to return a 'complete' list that is specific to that prefix.

[DanTup]

I suspect the spec may even be deliberately vague to allow both types of server implemenations.

This won't work out though, as the client+server need to have the same understanding. For example if a server assumes the client will re-request on backspace and filters to the prefix, but an editor does not, the user will be missing completions if they backspace.

I think it's likely just accidental, but I do believe it needs to be concrete. I've seen many inconsistencies across LSP clients because of vagueness in the spec, so I think it's in everyones interest to make everything absolutely concrete (though I don't think anyone disagrees with this 🙂)

Those kinds of things will tend to return 100s or 1000s of results unless they are somehow 'narrowed' down. I think a typical server implementation would have to be practical and just return a 'incomplete' list of result in such cases, and limit it to some small number of results. Then as user types more, the server takes the prefix into account to return a 'complete' list that is specific to that prefix.

That only works with IsIncomplete=true though, and that results in a lot of data being round-tripped multiple times and increased latency as the user types. That's why I'd prefer to filter to the prefix (if this can be clarified) than switch to IsIncomplete=true. (That said, I think this API is in real need of supporting partial results - I think the API does, but VS Code doesn't support it.. sending all the imported completions initially and then the huge list of unimported ones would be a big improvement).

[iamcco]

For coc.nvim implement, if isInComplete: false it will cache the result, and will not send request for forward typing and backspace hit. if isInComplete: true it will resend request for forward typing but cancel autocomplete if backspace hit.

[DanTup]

For coc.nvim implement, if isInComplete: false it will cache the result, and will not send request for forward typing and backspace hit.

This is the behaviour I somewhat expect (which unfortunately prevents the optimisation I was hoping to make), but it would be nice to have clarified (and made concrete in the spec to ensure all clients/servers assume the same).

[DanTup]

I just did some testing in VS Code, and I found:

• If you type a single character to trigger completion, then hit backspace, the completion widget closes
• If you type several characters to trigger completion, then backspace - the completion widget does not go back to the server if the original completion request was made before you'd typed the character you deleted (eg. it can be assumed the server had already included all relevant completions)
• If you invoke backspace with an existing prefix and then backspace (such that you now have a shorter prefix than was sent to the server), then it does re-call the server

Therefore I'm inclined to believe that it's always safe for a server to filter results based on the current prefix even if using isIncomplete=false. However it would be nice for the LSP spec (and the VS Code API docs) to be explicit about this so we don't have to hope all clients interpret it this way.

[bluz71]

I've always assumed that server is at least free to take the prefix into account. If not, then the number of completions could be 100s or even 1000s for some cases. So filtering based on prefix is almost mandatory for performance/practicality.

In the case of Dart / Flutter v1.17 we have observed about 27,000 completions being sent from language server to client without server-side prefix filtering.

Needless to say this results in noticeable editor pauses and stutter, at least in Vim + LSC.

[DanTup]

There have been a few bugs in the Dart LSP implementation that resulted in many more items being returned than there should (for example private and/or static members). However, the remaining list is still large when using auto-import completions.

I'm working on a change that filters server-side based on the prefix (I am making the assumption that the behaviour I noted above from VS Code is what the spec intends, even though it's not clear) that should make things significantly better - however the spec really needs updating to be explicit here - clients/servers shouldn't need to make assumptions about what the other expects.

[DanTup]

@dbaeumer the related VS Code issue (https://github.com/microsoft/vscode/issues/99355) was closed as not-enough-votes and then locked with outstanding questions. Could it be re-opened? It doesn't seem like spec ambiguities should require user votes to address - it's important the LSP spec is clear for servers/clients to know how the other will behave, and if LSP is based on VS Code functionality, it stands to reason that VS Code needs to be clear too.

I've made assumptions about "current but unspecified behaviour" in the past, and then VS Code changed (I shouldn't rely on undocumented behaviour) and broke for users 😞