Discard LSP requests/notifcation that target outdated versions #6282
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a slight followup to #6173
I noticed that similar bugs could occur basically anytime the LS sends an edit to the client. The user can always add some new text which makes the received text invalid. However, for these edits the LSP standard actually includes a mechanism to deal with the problem: Whenever we send a change a notification to the server, the client must attach a unique version to the resulting document state.
If the LSP server sends an edit to the client, that edit includes the document version it applies to and if that version doesn't match the current version then that change can not be applied/should be discarded.
For workspace edits specifically there are actually many capabilities which include the exact behavior here, but we didn't set any of them. Even worse the client is supposed to return that applying the edits failed but we always returned a success. I have fixed that by implemented the
abort
failure mechanism for now (and setting the capabilities appropriately) as it was the easiest to implement (basically just propagating aResult
with?
). In the future we likely want to check whether aWorkspaceEdit
is valid before we start apply it and switch to transactional failure handling. I think that is how VSCode handles it. This way we won't end-up with half-applied changes (a good LS would resend the missing edits, but I suspect many don't). Implementing this is a bit more work so I left that to future work. The current failure handling strategy is spec compliant and avoids apply incorrect edits/crashes so I wanted a quick fix for that first.PublishDiagnostic
notifications also send a document version and this PR also adds a check to drop outdatedPublishDiagnostic
notifications. This fixes diagnostics "lagging behind" while typing.Future Work
I noticed that we don't normalize line endings while applying LSP edits. We currently do this for snippets but not anywhere else. Furthermore, we also need to normalize indentation (replace tabs with spaces and vice versa). I will create an issue for that. Once we do that we should also set the appropriate capability to true (I set it to
false
for now to reflect what we are doing