fix(column): crash with 'signcolumn' set to "number" (#29003)

Problem:  Numberwidth may depend on number of signs with text in the
          buffer and is not handled correctly for extmark signs.
Solution: Move legacy sign code for changed numberwidth so that it is
          handled properly for legacy and extmark signs alike.

src/nvim/decoration.c

@@ -15,6 +15,7 @@
 #include "nvim/drawscreen.h"
 #include "nvim/extmark.h"
 #include "nvim/fold.h"
+#include "nvim/globals.h"
 #include "nvim/grid.h"
 #include "nvim/grid_defs.h"
 #include "nvim/highlight.h"
@@ -184,6 +185,21 @@ void buf_put_decor(buf_T *buf, DecorInline decor, int row, int row2)
   }
 }
 
+/// When displaying signs in the 'number' column, if the width of the number
+/// column is less than 2, then force recomputing the width after placing or
+/// unplacing the first sign in "buf".
+static void may_force_numberwidth_recompute(buf_T *buf, bool unplace)
+{
+  FOR_ALL_TAB_WINDOWS(tp, wp) {
+    if (wp->w_buffer == buf
+        && wp->w_minscwidth == SCL_NUM
+        && (wp->w_p_nu || wp->w_p_rnu)
+        && (unplace || wp->w_nrwidth_width < 2)) {
+      wp->w_nrwidth_line_count = 0;
+    }
+  }
+}
+
 static int sign_add_id = 0;
 void buf_put_decor_sh(buf_T *buf, DecorSignHighlight *sh, int row1, int row2)
 {
@@ -191,6 +207,7 @@ void buf_put_decor_sh(buf_T *buf, DecorSignHighlight *sh, int row1, int row2)
     sh->sign_add_id = sign_add_id++;
     if (sh->text[0]) {
       buf_signcols_count_range(buf, row1, row2, 1, kFalse);
+      may_force_numberwidth_recompute(buf, false);
     }
   }
 }
@@ -218,6 +235,7 @@ void buf_remove_decor_sh(buf_T *buf, int row1, int row2, DecorSignHighlight *sh)
       if (buf_meta_total(buf, kMTMetaSignText)) {
         buf_signcols_count_range(buf, row1, row2, -1, kFalse);
       } else {
+        may_force_numberwidth_recompute(buf, true);
         buf->b_signcols.resized = true;
         buf->b_signcols.max = buf->b_signcols.count[0] = 0;
       }

src/nvim/drawline.c

@@ -465,6 +465,7 @@ static void draw_sign(bool nrcol, win_T *wp, winlinevars_T *wlv, int sign_idx, i
     int fill = nrcol ? number_width(wp) + 1 : SIGN_WIDTH;
     draw_col_fill(wlv, schar_from_ascii(' '), fill, attr);
     int sign_pos = wlv->off - SIGN_WIDTH - (int)nrcol;
+    assert(sign_pos >= 0);
     linebuf_char[sign_pos] = sattr.text[0];
     linebuf_char[sign_pos + 1] = sattr.text[1];
   } else {

src/nvim/drawscreen.c

@@ -1548,6 +1548,7 @@ static void win_update(win_T *wp)
   // Force redraw when width of 'number' or 'relativenumber' column changes.
   if (wp->w_nrwidth != nrwidth_new) {
     type = UPD_NOT_VALID;
+    changed_line_abv_curs_win(wp);
     wp->w_nrwidth = nrwidth_new;
   } else {
     // Set mod_top to the first line that needs displaying because of

src/nvim/sign.c

@@ -246,12 +246,6 @@ static int buf_delete_signs(buf_T *buf, char *group, int id, linenr_T atlnum)
     return FAIL;
   }
 
-  // When deleting the last sign need to redraw the windows to remove the
-  // sign column. Not when curwin is NULL (this means we're exiting).
-  if (!buf_meta_total(buf, kMTMetaSignText) && curwin != NULL) {
-    changed_line_abv_curs();
-  }
-
   return OK;
 }
 
@@ -499,17 +493,6 @@ static void sign_list_by_name(char *name)
   }
 }
 
-static void may_force_numberwidth_recompute(buf_T *buf, int unplace)
-{
-  FOR_ALL_TAB_WINDOWS(tp, wp)
-  if (wp->w_buffer == buf
-      && (wp->w_p_nu || wp->w_p_rnu)
-      && (unplace || wp->w_nrwidth_width < 2)
-      && (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u')) {
-    wp->w_nrwidth_line_count = 0;
-  }
-}
-
 /// Place a sign at the specified file location or update a sign.
 static int sign_place(uint32_t *id, char *group, char *name, buf_T *buf, linenr_T lnum, int prio)
 {
@@ -531,11 +514,7 @@ static int sign_place(uint32_t *id, char *group, char *name, buf_T *buf, linenr_
     // ":sign place {id} file={fname}": change sign type and/or priority
     lnum = buf_mod_sign(buf, id, group, prio, sp);
   }
-  if (lnum > 0) {
+  if (lnum <= 0) {
-    // When displaying signs in the 'number' column, if the width of the
-    // number column is less than 2, then force recomputing the width.
-    may_force_numberwidth_recompute(buf, false);
-  } else {
     semsg(_("E885: Not possible to change sign %s"), name);
     return FAIL;
   }
@@ -562,13 +541,6 @@ static int sign_unplace_inner(buf_T *buf, int id, char *group, linenr_T atlnum)
     }
   }
 
-  // When all the signs in a buffer are removed, force recomputing the
-  // number column width (if enabled) in all the windows displaying the
-  // buffer if 'signcolumn' is set to 'number' in that window.
-  if (!buf_meta_total(buf, kMTMetaSignText)) {
-    may_force_numberwidth_recompute(buf, true);
-  }
-
   return OK;
 }
 

test/functional/ui/decorations_spec.lua

@@ -5497,6 +5497,26 @@ l5
 
     api.nvim_buf_clear_namespace(0, ns, 0, -1)
   end)
+
+  it([[correct numberwidth with 'signcolumn' set to "number" #28984]], function()
+    command('set number numberwidth=1 signcolumn=number')
+    api.nvim_buf_set_extmark(0, ns, 0, 0, { sign_text = 'S1' })
+    screen:expect({
+      grid = [[
+        S1 ^                                               |
+        {1:~                                                 }|*8
+                                                          |
+      ]]
+    })
+    api.nvim_buf_del_extmark(0, ns, 1)
+    screen:expect({
+      grid = [[
+        {8:1 }^                                                |
+        {1:~                                                 }|*8
+                                                          |
+      ]]
+    })
+  end)
 end)
 
 describe('decorations: virt_text', function()