Closed vimpostor closed 1 year ago
@yegappan not sure if had outage, but so far i felt this is a good refine, if you had no objection, perhaps good to merge.
// @vimpostor // if you have time, perhaps continue to test or think some if there other concern cases if there were. // and thanks, this should be a good enhancement if there no side outage.
Thanks for the patch and the detailed description.
Currently the
ApplyTextEdits()
code loses the cursor position, because it applies the text edits by deleting the entire range withdeletebufline()
and then adding the new range withappendbufline()
. Obviously the cursor will move, if it happens to be inside this range, which is often the case (e.g. when renaming a variable).This is also apparently the reason, why in the past there was a workaround that saved the cursor position before applying any edits, and then restored it afterwards. That workaround was already (correctly) removed with: https://github.com/yegappan/lsp/pull/389#issuecomment-1705501598
Instead of reintroducing the workaround, this patch avoids losing the cursor position altogether by not deleting the entire range, but only deleting lines that actually need to be deleted (and adding new lines that actually need to be added).
In my opinion this solution is even simpler code-wise than it was before where we removed the entire region.
I also added a new test case for the scenario where lines have to be removed, all other cases already have test cases.
Demo
The old behaviour is on the left side: After renaming the variable, the cursor jumps to the end of the range. The new behaviour is on the right side: After renaming the variable, the cursor does not move.
https://github.com/yegappan/lsp/assets/21310755/e17a7be1-a0cb-4597-9ab1-4edb12905d02