fix(marks): revalidate marks whose position did not change

Problem:  Marks whose position did not change with the action that
          invalidated them (right_gravity = false) are not revalidated
          upon undo.
Solution: Remove early return when restoring a marks saved position so
          that it is still revalidated. Add "move" guards instead.

2 files changed, 27 insertions, 28 deletions

diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c
index b592283d92..4ff298cde5 100644
--- a/src/nvim/extmark.c
+++ b/src/nvim/extmark.c
@@ -116,16 +116,10 @@ static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool
 {
   MarkTreeIter itr[1] = { 0 };
   MTKey key = marktree_lookup(buf->b_marktree, mark, itr);
-  if (key.pos.row < 0 || (key.pos.row == row && key.pos.col == col)) {
-    // Does this hold? If it doesn't, we should still revalidate.
-    assert(!invalid || !mt_invalid(key));
-    return;
-  }
-
-  // Key already revalidated(how?) Avoid adding to decor again.
-  if (invalid && !mt_invalid(key)) {
-    invalid = false;
-  }
+  bool move = key.pos.row >= 0 && (key.pos.row != row || key.pos.col != col);
+  // Already valid keys were being revalidated, presumably when encountering a
+  // SavePos from a modified mark. Avoid adding that to the decor again.
+  invalid = invalid && mt_invalid(key);
 
   // Only the position before undo needs to be redrawn here,
   // as the position after undo should be marked as changed.
@@ -137,19 +131,21 @@ static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool
   int row2 = 0;
   if (invalid) {
     mt_itr_rawkey(itr).flags &= (uint16_t) ~MT_FLAG_INVALID;
-  } else if (key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) {
+  } else if (move && key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) {
     MTPos end = marktree_get_altpos(buf->b_marktree, key, NULL);
     row1 = MIN(end.row, MIN(key.pos.row, row));
     row2 = MAX(end.row, MAX(key.pos.row, row));
     buf_signcols_count_range(buf, row1, row2, 0, kTrue);
   }
 
-  marktree_move(buf->b_marktree, itr, row, col);
+  if (move) {
+    marktree_move(buf->b_marktree, itr, row, col);
+  }
 
   if (invalid) {
     row2 = mt_paired(key) ? marktree_get_altpos(buf->b_marktree, key, NULL).row : row;
     buf_put_decor(buf, mt_decor(key), row, row2);
-  } else if (key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) {
+  } else if (move && key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) {
     buf_signcols_count_range(buf, row1, row2, 0, kNone);
   }
 }
diff --git a/test/functional/api/extmark_spec.lua b/test/functional/api/extmark_spec.lua
index 7b2fe209ba..a7f4ba25e0 100644
--- a/test/functional/api/extmark_spec.lua
+++ b/test/functional/api/extmark_spec.lua
@@ -1758,13 +1758,13 @@ describe('API/extmarks', function()
     command('1d 2')
     eq(0, #get_extmarks(-1, 0, -1, {}))
     -- mark is not removed when deleting bytes before the range
-    set_extmark(
-      ns,
-      3,
-      0,
-      4,
-      { invalidate = true, undo_restore = false, hl_group = 'Error', end_col = 7 }
-    )
+    set_extmark(ns, 3, 0, 4, {
+      invalidate = true,
+      undo_restore = true,
+      hl_group = 'Error',
+      end_col = 7,
+      right_gravity = false,
+    })
     feed('dw')
     eq(3, get_extmark_by_id(ns, 3, { details = true })[3].end_col)
     -- mark is not removed when deleting bytes at the start of the range
@@ -1778,15 +1778,18 @@ describe('API/extmarks', function()
     eq(1, get_extmark_by_id(ns, 3, { details = true })[3].end_col)
     -- mark is removed when all bytes in the range are deleted
     feed('hx')
-    eq({}, get_extmark_by_id(ns, 3, {}))
+    eq(true, get_extmark_by_id(ns, 3, { details = true })[3].invalid)
+    -- mark is restored with undo_restore == true if pos did not change
+    command('undo')
+    eq(nil, get_extmark_by_id(ns, 3, { details = true })[3].invalid)
     -- multiline mark is not removed when start of its range is deleted
-    set_extmark(
-      ns,
-      4,
-      1,
-      4,
-      { undo_restore = false, invalidate = true, hl_group = 'Error', end_col = 7, end_row = 3 }
-    )
+    set_extmark(ns, 4, 1, 4, {
+      undo_restore = false,
+      invalidate = true,
+      hl_group = 'Error',
+      end_col = 7,
+      end_row = 3,
+    })
     feed('ddDdd')
     eq({ 0, 0 }, get_extmark_by_id(ns, 4, {}))
     -- multiline mark is removed when entirety of its range is deleted