From fbe9e8cdbc3d93fcd13cd9e8e0bcbb8a63b15b2c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Magnus=20Gro=C3=9F?= Date: Fri, 13 Oct 2023 01:08:06 +0200 Subject: [PATCH] Avoid moving the cursor when applying textedits In 33c9dc37bd354391579f669a142699801b13d571 we removed a workaround that resets the cursor to the previous location after applying an workspaceEdit, because it interfered with auto-imports. Turns out the workaround was there for a reason: When we apply general textedits (e.g. when we rename a variable), then we are always moving the cursor by accident, because internally (to apply a textedit) we delete the entire text range and then append the new range. Obviously this loses the cursor position, hence the reason for the original manual cursor reset workaround. Instead of reintroducing that workaround, we now avoid moving the cursor altogether: We accomplish this by no longer deleting the entire range and inserting it again. Instead we just remove lines that actually need to be deleted, add lines that actually need to be added and then handle the rest with a normal setbufline() call. This will cause the cursor to never lose the correct position, because we never remove the entire region where the cursor is located in. We have to be careful with off-by-one errors, therefore this also adds an extra test for the scenario where we have to delete some lines. The two other scenarios already had comprehensive tests (adding lines, and keeping the amount of lines). --- autoload/lsp/textedit.vim | 33 ++++++++++++++++++++++++++++----- test/clangd_tests.vim | 7 +++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/autoload/lsp/textedit.vim b/autoload/lsp/textedit.vim index 2fe2ad0..abea145 100644 --- a/autoload/lsp/textedit.vim +++ b/autoload/lsp/textedit.vim @@ -166,9 +166,6 @@ export def ApplyTextEdits(bnr: number, text_edits: list>): void #echomsg $'ApplyTextEdits: start_line = {start_line}, finish_line = {finish_line}' #echomsg $'lines = {string(lines)}' - # Delete all the lines that need to be modified - bnr->deletebufline(start_line + 1, finish_line + 1) - # if the buffer is empty, appending lines before the first line adds an # extra empty line at the end. Delete the empty line after appending the # lines. @@ -178,8 +175,34 @@ export def ApplyTextEdits(bnr: number, text_edits: list>): void dellastline = true endif - # Append the updated lines - appendbufline(bnr, start_line, lines) + # Now we apply the textedits to the actual buffer. + # In theory we could just delete all old lines and append the new lines. + # This would however cause the cursor to change position: It will always be + # on the last line added. + # + # Luckily there is an even simpler solution, that has no cursor sideeffects. + # + # Logically this method is split into the following three cases: + # + # 1. The number of new lines is equal to the number of old lines: + # Just replace the lines inline with setbufline() + # + # 2. The number of new lines is greater than the old ones: + # First append the missing lines at the **end** of the range, then use + # setbufline() again. This does not cause the cursor to change position. + # + # 3. The number of new lines is less than before: + # First use setbufline() to replace the lines that we can replace. + # Then remove superfluous lines. + # + # Luckily, the three different cases exist only logically, we can reduce + # them to a single case practically, because appendbufline() does not append + # anything if an empty list is passed just like deletebufline() does not + # delete anything, if the last line of the range is before the first line. + # We just need to be careful with all indices. + appendbufline(bnr, finish_line + 1, lines[finish_line - start_line + 1 : -1]) + setbufline(bnr, start_line + 1, lines) + deletebufline(bnr, start_line + 1 + lines->len(), finish_line + 1) if dellastline bnr->deletebufline(bnr->getbufinfo()[0].linecount) diff --git a/test/clangd_tests.vim b/test/clangd_tests.vim index 59811f6..97da941 100644 --- a/test/clangd_tests.vim +++ b/test/clangd_tests.vim @@ -118,6 +118,13 @@ def g:Test_LspFormat() } END assert_equal(expected, getline(1, '$')) + + deletebufline('', 1, '$') + # shrinking multiple lines into a single one works + setline(1, ['int \', 'i \', '= \', '42;']) + :redraw! + :4LspFormat + assert_equal(['int i = 42;'], getline(1, '$')) bw! # empty file -- 2.48.1