Stream: t-compiler/wg-rls-2.0

Topic: textDocument/rename response


Martin Asquino (Jun 15 2020 at 21:24, on Zulip):

Hi! I noticed that the response that comes back from rust-analyzer when issuing a textDocument/rename on a document that needs N edits in it is a list of N TextDocumentEdits with a single TextEdit in it.

A TextDocumentEdit describes all changes on a version Si and after they are applied move the document to version Si+1. So the creator of a TextDocumentEdit doesn’t need to sort the array of edits or do any kind of ordering

The docs there imply to me that applying a TextDocumentEdit "changes" the version of the document, so applying subsequent TextDocumentEdits on it without going back to the server is potentially invalid. This is in fact an issue right now in LanguageClient-neovim, and I suppose we could solve it by grouping the edits by the text document's uri, but I feel like that is not the way it's intended to be used.

Is there any reason that that is not a single TextDocumentEdit with N TextEdits instead of the current response?

(btw, I can open an issue, just wanted to check if there's some reason that I'm missing for it to reply in that way)

Jeremy Kolb (Jun 15 2020 at 21:37, on Zulip):

I think that's just the way it's currently implemented. I don't think we check workspace.workspaceEdit.documentChanges either

Martin Asquino (Jun 15 2020 at 21:40, on Zulip):

Not sure what you mean by check there, but it seems like VSCode handles this rename correctly, so I suspect there is some additional handling of the results there. I haven't checked this myself though, I just have checked the vim side of this, but the person who raised this issue claims it works in VSCode with the rust-analyzer extension.

In any case, do you think is something that needs fixing then? Should I open an issue about it?

Jeremy Kolb (Jun 15 2020 at 21:40, on Zulip):

I'm going off this here: https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#workspaceEdit

Jeremy Kolb (Jun 15 2020 at 21:44, on Zulip):

I'm wondering if this affects "undo" behavior. Are you seeing any weirdness? It may be worth filing an issue as it sounds like suboptimal behavior

Martin Asquino (Jun 15 2020 at 21:44, on Zulip):

It kinda does yeah. To undo a rename in vim you have to press u once for each change in the document, whereas in other servers you just press it once and it rolls back that rename.

Martin Asquino (Jun 15 2020 at 21:46, on Zulip):

But more importantly, it's messing up the edits themselves when there are two edits in the same line, because if the leftmost edit is applied first then the range for the rightmost one is incorrect.

Martin Asquino (Jun 15 2020 at 21:47, on Zulip):

LanguageClient-neovim handles this when there are multiple edits in a single TextDocumentEdit by sorting the edits in a document by character, and applying them from right to left. But it can't be done as it is now because there's effectively a single edit for each TextDocumentEdit.

Martin Asquino (Jun 15 2020 at 21:58, on Zulip):

filed https://github.com/rust-analyzer/rust-analyzer/issues/4901

Martin Asquino (Jun 15 2020 at 21:59, on Zulip):

Btw, forgive me if any of that comes as a criticism, my english sometimes fails in correctness, but I have nothing but admiration by the work you folks are doing with rust-analyzer. :bow:

Jeremy Kolb (Jun 16 2020 at 15:14, on Zulip):

Good catch! thanks for filing the issue :)

Last update: Sep 27 2020 at 14:45UTC