fix(column): clear "b_signcols" before moving saved marks

Problem:  Marks moved by undo may be lost to "b_signcols.count".
Solution: Count signs for each undo object separately instead of
          once for the entire undo.

src/nvim/decoration.c

@@ -684,15 +684,16 @@ static const uint32_t sign_filter[4] = {[kMTMetaSignText] = kMTFilterSelect,
 void decor_redraw_signs(win_T *wp, buf_T *buf, int row, SignTextAttrs sattrs[], int *line_id,
                         int *cul_id, int *num_id)
 {
-  MarkTreeIter itr[1];
+  if (!buf_has_signs(buf)) {
-  if (!marktree_itr_get_overlap(buf->b_marktree, row, 0, itr)) {
     return;
   }
 
   MTPair pair;
   int num_text = 0;
+  MarkTreeIter itr[1];
   kvec_t(SignItem) signs = KV_INITIAL_VALUE;
   // TODO(bfredl): integrate with main decor loop.
+  marktree_itr_get_overlap(buf->b_marktree, row, 0, itr);
   while (marktree_itr_step_overlap(buf->b_marktree, itr, &pair)) {
     if (!mt_invalid(pair.start) && mt_decor_sign(pair.start)) {
       DecorSignHighlight *sh = decor_find_sign(mt_decor(pair.start));
@@ -773,12 +774,6 @@ void buf_signcols_count_range(buf_T *buf, int row1, int row2, int add, TriState
     return;
   }
 
-  static int nested = 0;
-  // An undo/redo may trigger subsequent calls before its own kNone call.
-  if ((nested += clear) > (0 + (clear == kTrue))) {
-    return;  // Avoid adding signs more than once.
-  }
-
   // Allocate an array of integers holding the number of signs in the range.
   assert(row2 >= row1);
   int *count = xcalloc(sizeof(int), (size_t)(row2 + 1 - row1));

src/nvim/drawscreen.c

@@ -1199,7 +1199,7 @@ void comp_col(void)
   set_vim_var_nr(VV_ECHOSPACE, sc_col - 1);
 }
 
-/// Redraw entire window "wp" if configured 'signcolumn' width changes.
+/// Redraw entire window "wp" if "auto" 'signcolumn' width has changed.
 static bool win_redraw_signcols(win_T *wp)
 {
   buf_T *buf = wp->w_buffer;

src/nvim/extmark.c

@@ -107,20 +107,33 @@ revised:
   }
 }
 
-static bool extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col)
+static void extmark_setraw(buf_T *buf, uint64_t mark, int row, colnr_T col, bool invalid)
 {
   MarkTreeIter itr[1] = { 0 };
   MTKey key = marktree_lookup(buf->b_marktree, mark, itr);
-  if (key.pos.row == -1) {
+  if (key.pos.row < 0 || (key.pos.row == row && key.pos.col == col)) {
-    return false;
+    return;
   }
 
-  if (key.pos.row == row && key.pos.col == col) {
+  int row1 = 0;
-    return true;
+  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) {
+    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);
-  return true;
+
+  if (invalid) {
+    MTPos end = marktree_get_altpos(buf->b_marktree, key, NULL);
+    buf_put_decor(buf, mt_decor(key), row, end.row);
+  } else if (key.flags & MT_FLAG_DECOR_SIGNTEXT && buf->b_signcols.autom) {
+    buf_signcols_count_range(buf, row1, row2, 0, kNone);
+  }
 }
 
 /// Remove an extmark in "ns_id" by "id"
@@ -401,16 +414,8 @@ void extmark_apply_undo(ExtmarkUndoObject undo_info, bool undo)
     // kExtmarkSavePos
   } else if (undo_info.type == kExtmarkSavePos) {
     ExtmarkSavePos pos = undo_info.data.savepos;
-    if (undo) {
+    if (undo && pos.old_row >= 0) {
-      if (pos.old_row >= 0
+      extmark_setraw(curbuf, pos.mark, pos.old_row, pos.old_col, pos.invalidated);
-          && extmark_setraw(curbuf, pos.mark, pos.old_row, pos.old_col)
-          && pos.invalidated) {
-        MarkTreeIter itr[1] = { 0 };
-        MTKey mark = marktree_lookup(curbuf->b_marktree, pos.mark, itr);
-        mt_itr_rawkey(itr).flags &= (uint16_t) ~MT_FLAG_INVALID;
-        MTPos end = marktree_get_altpos(curbuf->b_marktree, mark, itr);
-        buf_put_decor(curbuf, mt_decor(mark), mark.pos.row, end.row);
-      }
     }
     // No Redo since kExtmarkSplice will move marks back
   } else if (undo_info.type == kExtmarkMove) {
@@ -527,7 +532,7 @@ void extmark_splice_impl(buf_T *buf, int start_row, colnr_T start_col, bcount_t
 
   // Remove signs inside edited region from "b_signcols.count", add after splicing.
   if (old_row > 0 || new_row > 0) {
-    buf_signcols_count_range(buf, start_row, start_row + old_row + 1, 0, kTrue);
+    buf_signcols_count_range(buf, start_row, start_row + old_row, 0, kTrue);
   }
 
   marktree_splice(buf->b_marktree, (int32_t)start_row, start_col,
@@ -535,7 +540,7 @@ void extmark_splice_impl(buf_T *buf, int start_row, colnr_T start_col, bcount_t
                   new_row, new_col);
 
   if (old_row > 0 || new_row > 0) {
-    buf_signcols_count_range(buf, start_row, start_row + new_row + 1, 0, kNone);
+    buf_signcols_count_range(buf, start_row, start_row + new_row, 0, kNone);
   }
 
   if (undo == kExtmarkUndo) {

src/nvim/undo.c

@@ -2427,39 +2427,15 @@ static void u_undoredo(bool undo, bool do_buf_event)
     curbuf->b_op_end.lnum = curbuf->b_ml.ml_line_count;
   }
 
-  int row1 = MAXLNUM;
-  int row2 = -1;
-  int row3 = -1;
-  // Tricky: ExtmarkSavePos may come after ExtmarkSplice which does call
-  // buf_signcols_count_range() but then misses the yet unrestored marks.
-  if (curbuf->b_signcols.autom && buf_meta_total(curbuf, kMTMetaSignText)) {
-    for (int i = 0; i < (int)kv_size(curhead->uh_extmark); i++) {
-      ExtmarkUndoObject undo_info = kv_A(curhead->uh_extmark, i);
-      if (undo_info.type == kExtmarkSplice) {
-        ExtmarkSplice s = undo_info.data.splice;
-        if (s.old_row > 0 || s.new_row > 0) {
-          row1 = MIN(row1, s.start_row);
-          row2 = MAX(row2, s.start_row + (undo ? s.new_row : s.old_row) + 1);
-          row3 = MAX(row3, s.start_row + (undo ? s.old_row : s.new_row) + 1);
-        }
-      }
-    }
-    if (row2 != -1) {
-      // Remove signs inside edited region from "b_signcols.count".
-      buf_signcols_count_range(curbuf, row1, row2, 0, kTrue);
-    }
-  }
   // Adjust Extmarks
   if (undo) {
     for (int i = (int)kv_size(curhead->uh_extmark) - 1; i > -1; i--) {
-      ExtmarkUndoObject undo_info = kv_A(curhead->uh_extmark, i);
+      extmark_apply_undo(kv_A(curhead->uh_extmark, i), undo);
-      extmark_apply_undo(undo_info, undo);
     }
     // redo
   } else {
     for (int i = 0; i < (int)kv_size(curhead->uh_extmark); i++) {
-      ExtmarkUndoObject undo_info = kv_A(curhead->uh_extmark, i);
+      extmark_apply_undo(kv_A(curhead->uh_extmark, i), undo);
-      extmark_apply_undo(undo_info, undo);
     }
   }
   if (curhead->uh_flags & UH_RELOAD) {
@@ -2467,10 +2443,7 @@ static void u_undoredo(bool undo, bool do_buf_event)
     // should have all info to send a buffer-reloaing on_lines/on_bytes event
     buf_updates_unload(curbuf, true);
   }
-  // Finish adjusting extmarks: add signs inside edited region to "b_signcols.count".
+  // Finish adjusting extmarks
-  if (row2 != -1) {
-    buf_signcols_count_range(curbuf, row1, row3, 0, kNone);
-  }
 
   curhead->uh_entry = newlist;
   curhead->uh_flags = new_flags;

test/functional/ui/decorations_spec.lua

@@ -5058,6 +5058,33 @@ l5
     ]]}
   end)
 
+  it('correct width with moved marks before undo savepos', function()
+    screen:try_resize(20, 4)
+    insert(example_test3)
+    feed('gg')
+    exec_lua([[
+      local ns = vim.api.nvim_create_namespace('')
+      vim.api.nvim_buf_set_extmark(0, ns, 0, 0, { sign_text = 'S1' })
+      vim.api.nvim_buf_set_extmark(0, ns, 1, 0, { sign_text = 'S2' })
+      local s3 = vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { sign_text = 'S3' })
+      local s4 = vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { sign_text = 'S4' })
+      vim.schedule(function()
+        vim.cmd('silent d3')
+        vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { id = s3, sign_text = 'S3' })
+        vim.api.nvim_buf_set_extmark(0, ns, 2, 0, { id = s4, sign_text = 'S4' })
+        vim.cmd('silent undo')
+        vim.api.nvim_buf_del_extmark(0, ns, s3)
+      end)
+    ]])
+
+    screen:expect{grid=[[
+      S1^l1                |
+      S2l2                |
+      S4l3                |
+                          |
+    ]]}
+  end)
+
   it('no crash with sign after many marks #27137', function()
     screen:try_resize(20, 4)
     insert('a')