new client capability: defer calculation additionalTextEdit until comletionItem/resolve

[Eskibear]

original discussion in microsoft/vscode#96779

Up to now in LSP v3.15, textEdit/additionalTextEdits are calculated in textDocument/completion, and are not allowed to change during comepltionItem/resolve. This leads to a performance issue of Java language server, because it takes time to calculate additionalTextEdits like "import" statements.

Now in vscode, it offers a pragmatic solution, that additionalTextEdits can be calculated during comepltionItem/resolve, as mentioned in microsoft/vscode#96779 (comment) . Then come the problem, how does the language server know if the client supports the "pragmatic solution", i.e., lazy-loading addtionalTextEdits. So I think a corresponding client capability should be introduced for it.

[yyoncho]

Why there is no flag for the server support?

[dbaeumer]

@yyoncho what would that flag do?

[yyoncho]

@yyoncho what would that flag do?

ATM the clients don't know if the server has deferred the population of additionalTextEdits. In case they are nil and the server does not support this deferred additionalTextEdits calculation the client will have do one redundant call.

[dbaeumer]

This is already the case today for the other properties on the completion item that can be deferred. So we decided to not add this. In addition a single global property will not help since this can be different for every completion item (some will have additional text edits, some don't). So to avoid the second round trip we would need to have a property on every completion item which we decided for now not to do.

The additional round trip will not cause any delays in inserting the completion item or block typing in any way.

[yyoncho]

This is already the case today for the other properties on the completion item that can be deferred.

That is not true. At this point, all properties required for inserting the completion itself are present in textDocument/completion response - even more, check your response here - #924 (comment) .

So to avoid the second round trip we would need to have a property on every completion item which we decided for now not to do.

Again, the property will state if the server supports the new client capability - it is a global property. Without it, the clients that are not VScode will have to perform resolve no matter if the server does not support this new feature.

[dbaeumer]

That is not true. At this point, all properties required for inserting the completion itself are present in textDocument/completion response

And that will stay the same if the client doesn't opt into allow lazy additional text edits.

If a client has support for the resolve (it opts into it) why is it difficult for the client to send the resolve request even if the server doesn't fill in any edits. The only advantage of a server property will be to avoid a second roundtrip. It will not ease the implementation side for clients.

[yyoncho]

The only advantage of a server property will be to avoid a second roundtrip.

Yes - this is the advantage I am looking for.

[dbaeumer]

Just to be clear. The second roundtrip will very likely happen anyways because of other lazy attributes like detail and documentation. If they are not set clients will call resolve when an item is selected.

[yyoncho]

The second roundtrip will very likely happen anyways because of other lazy attributes like detail and documentation.

this depends on client implementation. On our side(Emacs), we call resolve only for documentation(detail apparently is prepopulated by all(?) of the servers) which can be performed even without blocking. This is not the case with additionalTextEdits - after this change, we will be forced block and resolve additionalTextEdits when the item is not resolved for no reason. The server might not support additionalTextEdits at all but we still should call resolve and block. All this could be easily avoided by adding the server capability flag.

[dbaeumer]

@yyoncho now I understand it. IMO you shouldn't force block the resolving of additional text edits. This will result in a bad user experience. In VS Code we do the following: we capture the edit state before the resolve and if the provided text edits don't conflict with what the user has types in the meantime we insert them. In case of a conflict we drop them and let the user know

In general a conflict is VERY unlikely since additional text edits should only modify text unrelated to the cursor position (https://github.com/microsoft/vscode-languageserver-node/blob/master/types/src/main.ts#L1636)

[yyoncho]

@dbaeumer thank you, we implemented your suggestion on Emacs side and it works fine.

[dbaeumer]

Great to hear that.