Auto-import causes cursor position to jump

[hasali19]

When using the auto-import function, the cursor seems to jump backwards by the number of characters that have been inserted.

Before auto-import:

After auto-import:

This is annoying as you then have to manually move it back to the original position to continue typing.

[matklad]

Yeah, I've noticied this as well, we should fix it.

The entry point is here: https://github.com/rust-analyzer/rust-analyzer/blob/d5fcedb38eec33e2eb12ed550a9b90f6950855fe/crates/assists/src/handlers/auto_import.rs#L59-L61

I have not investigated it, but looks like the problem is that we replace the text of the whole file, so the cursor position gets lost. I think we need to do one of the following:

• replace "replace the whole file" with a fine-grained diff
• replace the whole file, but make use of ast diffing functionality

The code for replacement is here:

https://github.com/rust-analyzer/rust-analyzer/blob/d5fcedb38eec33e2eb12ed550a9b90f6950855fe/crates/assists/src/assist_context.rs#L257-L273

[SomeoneToIgnore]

I think we'd better to go with the

replace "replace the whole file" with a fine-grained diff

approach, if we want to reuse the same method for other things like completions: LSP requires all edits related to the same action to be disjoint, same is enforced by us in the TextEdit struct methods.
Obviously, when we replace the whole file, it's not disjoint with any other edit in this file.

My stab at #1062 is now facing this issue, since I have to both insert the new use statement and replace the text used for the completion with something more meaningful.

[Veykril]

I looked into this a bit yesterday to understand how diffing and such works in RA and I can tell that the first approach will definitely be the better choice due to the fact that insert_use either replaces or inserts a new UseTree. This is relevant should the action insert a new tree, ast diffing(via algo::diff) will just give you back the entire node as a diff which in turn will not solve the problem.

So one solution I would see is that insert_use should probably return something like an InsertResult/InsertAction to tell the callee how to apply this? As in whether to do a specific insert or a replacement.