Merge pull request #12621 from Shougo/vim-8.1.1564

[RDY] vim-patch:8.1.1564, 1570, 1623, 1712 : sign column takes up space

runtime/doc/options.txt

@@ -5553,6 +5553,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 	   "yes"    	always
 	   "yes:[1-9]"  always, with fixed space for signs up to the given
 	                number (maximum 9), e.g. "yes:3"
+	   "number"	display signs in the 'number' column. If the number
+			column is not present, then behaves like 'auto'.
 
 
 			*'smartcase'* *'scs'* *'nosmartcase'* *'noscs'*

src/nvim/option.c

@@ -314,7 +314,7 @@ static char *(p_icm_values[]) =       { "nosplit", "split", NULL };
 static char *(p_scl_values[]) =       { "yes", "no", "auto", "auto:1", "auto:2",
   "auto:3", "auto:4", "auto:5", "auto:6", "auto:7", "auto:8", "auto:9",
   "yes:1", "yes:2", "yes:3", "yes:4", "yes:5", "yes:6", "yes:7", "yes:8",
-  "yes:9", NULL };
+  "yes:9", "number", NULL };
 static char *(p_fdc_values[]) =       { "auto:1", "auto:2",
   "auto:3", "auto:4", "auto:5", "auto:6", "auto:7", "auto:8", "auto:9",
   "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", NULL };
@@ -3183,6 +3183,13 @@ ambw_end:
     if (check_opt_strings(*varp, p_scl_values, false) != OK) {
       errmsg = e_invarg;
     }
+    // When changing the 'signcolumn' to or from 'number', recompute the
+    // width of the number column if 'number' or 'relativenumber' is set.
+    if (((*oldval == 'n' && *(oldval + 1) == 'u')
+         || (*curwin->w_p_scl == 'n' && *(curwin->w_p_scl + 1) =='u'))
+        && (curwin->w_p_nu || curwin->w_p_rnu)) {
+      curwin->w_nrwidth_line_count = 0;
+    }
   } else if (varp == &curwin->w_p_fdc || varp == &curwin->w_allbuf_opt.wo_fdc) {
     // 'foldcolumn'
     if (check_opt_strings(*varp, p_fdc_values, false) != OK) {
@@ -7397,7 +7404,10 @@ int win_signcol_count(win_T *wp)
   int maximum = 1, needed_signcols;
   const char *scl = (const char *)wp->w_p_scl;
 
-  if (*scl == 'n') {
+  // Note: It checks "no" or "number" in 'signcolumn' option
+  if (*scl == 'n'
+      && (*(scl + 1) == 'o' || (*(scl + 1) == 'u'
+                                && (wp->w_p_nu || wp->w_p_rnu)))) {
     return 0;
   }
   needed_signcols = buf_signcols(wp->w_buffer);

src/nvim/screen.c

@@ -2816,6 +2816,7 @@ win_line (
   for (;; ) {
     int has_match_conc = 0;  ///< match wants to conceal
     bool did_decrement_ptr = false;
+
     // Skip this quickly when working on the text.
     if (draw_state != WL_LINE) {
       if (draw_state == WL_CMDLINE - 1 && n_extra == 0) {
@@ -2854,47 +2855,12 @@ win_line (
            * buffer or when using Netbeans. */
           int count = win_signcol_count(wp);
           if (count > 0) {
-              int text_sign;
+              get_sign_display_info(
-              // Draw cells with the sign value or blank.
+                  false, wp, lnum, row,
-              c_extra = ' ';
+                  startrow, filler_lines, filler_todo, count,
-              c_final = NUL;
+                  &c_extra, &c_final, extra, sizeof(extra),
-              char_attr = win_hl_attr(wp, HLF_SC);
+                  &p_extra, &n_extra,
-              n_extra = win_signcol_width(wp);
+                  &char_attr, &draw_state, &sign_idx);
-
-              if (row == startrow + filler_lines && filler_todo <= 0) {
-                  text_sign = buf_getsigntype(wp->w_buffer, lnum, SIGN_TEXT,
-                                              sign_idx, count);
-                  if (text_sign != 0) {
-                      p_extra = sign_get_text(text_sign);
-                      if (p_extra != NULL) {
-                          int symbol_blen = (int)STRLEN(p_extra);
-
-                          c_extra = NUL;
-                          c_final = NUL;
-
-                          // TODO(oni-link): Is sign text already extended to
-                          // full cell width?
-                          assert((size_t)win_signcol_width(wp)
-                                 >= mb_string2cells(p_extra));
-                          // symbol(s) bytes + (filling spaces) (one byte each)
-                          n_extra = symbol_blen +
-                            (win_signcol_width(wp) - mb_string2cells(p_extra));
-
-                          assert(sizeof(extra) > (size_t)symbol_blen);
-                          memset(extra, ' ', sizeof(extra));
-                          memcpy(extra, p_extra, symbol_blen);
-
-                          p_extra = extra;
-                          p_extra[n_extra] = NUL;
-                      }
-                      char_attr = sign_get_attr(text_sign, SIGN_TEXT);
-                  }
-              }
-
-              sign_idx++;
-              if (sign_idx < count) {
-                  draw_state = WL_SIGN - 1;
-              }
           }
       }
 
@@ -2903,65 +2869,78 @@ win_line (
         /* Display the absolute or relative line number. After the
          * first fill with blanks when the 'n' flag isn't in 'cpo' */
         if ((wp->w_p_nu || wp->w_p_rnu)
-            && (row == startrow
+            && (row == startrow + filler_lines
-                + filler_lines
                 || vim_strchr(p_cpo, CPO_NUMCOL) == NULL)) {
-          /* Draw the line number (empty space after wrapping). */
+          // If 'signcolumn' is set to 'number' and a sign is present
-          if (row == startrow
+          // in 'lnum', then display the sign instead of the line
-              + filler_lines
+          // number.
-              ) {
+          if (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u'
-            long num;
+              && buf_findsign_id(wp->w_buffer, lnum, (char_u *)"*") != 0) {
-            char *fmt = "%*ld ";
+            int count = win_signcol_count(wp);
-
+            get_sign_display_info(
-            if (wp->w_p_nu && !wp->w_p_rnu)
+                true, wp, lnum, row,
-              /* 'number' + 'norelativenumber' */
+                startrow, filler_lines, filler_todo, count,
-              num = (long)lnum;
+                &c_extra, &c_final, extra, sizeof(extra),
-            else {
+                &p_extra, &n_extra,
-              /* 'relativenumber', don't use negative numbers */
+                &char_attr, &draw_state, &sign_idx);
-              num = labs((long)get_cursor_rel_lnum(wp, lnum));
-              if (num == 0 && wp->w_p_nu && wp->w_p_rnu) {
-                /* 'number' + 'relativenumber' */
-                num = lnum;
-                fmt = "%-*ld ";
-              }
-            }
-
-            sprintf((char *)extra, fmt,
-                number_width(wp), num);
-            if (wp->w_skipcol > 0)
-              for (p_extra = extra; *p_extra == ' '; ++p_extra)
-                *p_extra = '-';
-            if (wp->w_p_rl) {                       // reverse line numbers
-              // like rl_mirror(), but keep the space at the end
-              char_u *p2 = skiptowhite(extra) - 1;
-              for (char_u *p1 = extra; p1 < p2; p1++, p2--) {
-                const int t = *p1;
-                *p1 = *p2;
-                *p2 = t;
-              }
-            }
-            p_extra = extra;
-            c_extra = NUL;
-            c_final = NUL;
           } else {
-            c_extra = ' ';
+            if (row == startrow + filler_lines) {
-            c_final = NUL;
+              // Draw the line number (empty space after wrapping). */
-          }
+              long num;
-          n_extra = number_width(wp) + 1;
+              char *fmt = "%*ld ";
-          char_attr = win_hl_attr(wp, HLF_N);
 
-          int num_sign = buf_getsigntype(wp->w_buffer, lnum, SIGN_NUMHL,
+              if (wp->w_p_nu && !wp->w_p_rnu) {
-                                         0, 1);
+                // 'number' + 'norelativenumber'
-          if (num_sign != 0) {
+                num = (long)lnum;
-            // :sign defined with "numhl" highlight.
+              } else {
-            char_attr = sign_get_attr(num_sign, SIGN_NUMHL);
+                // 'relativenumber', don't use negative numbers
-          } else if ((wp->w_p_cul || wp->w_p_rnu)
+                num = labs((long)get_cursor_rel_lnum(wp, lnum));
-                     && lnum == wp->w_cursor.lnum) {
+                if (num == 0 && wp->w_p_nu && wp->w_p_rnu) {
-            // When 'cursorline' is set highlight the line number of
+                  // 'number' + 'relativenumber'
-            // the current line differently.
+                  num = lnum;
-            // TODO(vim): Can we use CursorLine instead of CursorLineNr
+                  fmt = "%-*ld ";
-            // when CursorLineNr isn't set?
+                }
-            char_attr = win_hl_attr(wp, HLF_CLN);
+              }
+
+              snprintf((char *)extra, sizeof(extra),
+                       fmt, number_width(wp), num);
+              if (wp->w_skipcol > 0) {
+                for (p_extra = extra; *p_extra == ' '; p_extra++) {
+                  *p_extra = '-';
+                }
+              }
+              if (wp->w_p_rl) {                       // reverse line numbers
+                // like rl_mirror(), but keep the space at the end
+                char_u *p2 = skiptowhite(extra) - 1;
+                for (char_u *p1 = extra; p1 < p2; p1++, p2--) {
+                  const int t = *p1;
+                  *p1 = *p2;
+                  *p2 = t;
+                }
+              }
+              p_extra = extra;
+              c_extra = NUL;
+              c_final = NUL;
+            } else {
+              c_extra = ' ';
+              c_final = NUL;
+            }
+            n_extra = number_width(wp) + 1;
+            char_attr = win_hl_attr(wp, HLF_N);
+
+            int num_sign = buf_getsigntype(
+                wp->w_buffer, lnum, SIGN_NUMHL, 0, 1);
+            if (num_sign != 0) {
+              // :sign defined with "numhl" highlight.
+              char_attr = sign_get_attr(num_sign, SIGN_NUMHL);
+            } else if ((wp->w_p_cul || wp->w_p_rnu)
+                       && lnum == wp->w_cursor.lnum) {
+              // When 'cursorline' is set highlight the line number of
+              // the current line differently.
+              // TODO(vim): Can we use CursorLine instead of CursorLineNr
+              // when CursorLineNr isn't set?
+              char_attr = win_hl_attr(wp, HLF_CLN);
+            }
           }
         }
       }
@@ -4497,6 +4476,88 @@ void screen_adjust_grid(ScreenGrid **grid, int *row_off, int *col_off)
   }
 }
 
+// Get information needed to display the sign in line 'lnum' in window 'wp'.
+// If 'nrcol' is TRUE, the sign is going to be displayed in the number column.
+// Otherwise the sign is going to be displayed in the sign column.
+static void get_sign_display_info(
+    bool nrcol,
+    win_T *wp,
+    linenr_T lnum,
+    int row,
+    int startrow,
+    int filler_lines,
+    int filler_todo,
+    int count,
+    int *c_extrap,
+    int *c_finalp,
+    char_u *extra,
+    size_t extra_size,
+    char_u **pp_extra,
+    int *n_extrap,
+    int *char_attrp,
+    int *draw_statep,
+    int *sign_idxp
+)
+{
+  int text_sign;
+
+  // Draw cells with the sign value or blank.
+  *c_extrap = ' ';
+  *c_finalp = NUL;
+  if (nrcol) {
+    *n_extrap = number_width(wp) + 1;
+  } else {
+    *char_attrp = win_hl_attr(wp, HLF_SC);
+    *n_extrap = win_signcol_width(wp);
+  }
+
+  if (row == startrow + filler_lines && filler_todo <= 0) {
+    text_sign = buf_getsigntype(wp->w_buffer, lnum, SIGN_TEXT,
+                                *sign_idxp, count);
+    if (text_sign != 0) {
+      *pp_extra = sign_get_text(text_sign);
+      if (*pp_extra != NULL) {
+        *c_extrap = NUL;
+        *c_finalp = NUL;
+
+        if (nrcol) {
+          int n, width = number_width(wp) - 2;
+          for (n = 0; n < width; n++) {
+            extra[n] = ' ';
+          }
+          extra[n] = NUL;
+          STRCAT(extra, *pp_extra);
+          STRCAT(extra, " ");
+          *pp_extra = extra;
+          *n_extrap = (int)STRLEN(*pp_extra);
+        } else {
+          int symbol_blen = (int)STRLEN(*pp_extra);
+
+          // TODO(oni-link): Is sign text already extended to
+          // full cell width?
+          assert((size_t)win_signcol_width(wp) >= mb_string2cells(*pp_extra));
+          // symbol(s) bytes + (filling spaces) (one byte each)
+          *n_extrap = symbol_blen +
+            (win_signcol_width(wp) - mb_string2cells(*pp_extra));
+
+          assert(extra_size > (size_t)symbol_blen);
+          memset(extra, ' ', extra_size);
+          memcpy(extra, *pp_extra, symbol_blen);
+
+          *pp_extra = extra;
+          (*pp_extra)[*n_extrap] = NUL;
+        }
+      }
+      *char_attrp = sign_get_attr(text_sign, SIGN_TEXT);
+    }
+  }
+
+  (*sign_idxp)++;
+  if (*sign_idxp < count) {
+    *draw_statep = WL_SIGN - 1;
+  }
+}
+
 
 /*
  * Check whether the given character needs redrawing:
@@ -7347,9 +7408,17 @@ int number_width(win_T *wp)
     ++n;
   } while (lnum > 0);
 
-  /* 'numberwidth' gives the minimal width plus one */
+  // 'numberwidth' gives the minimal width plus one
-  if (n < wp->w_p_nuw - 1)
+  if (n < wp->w_p_nuw - 1) {
     n = wp->w_p_nuw - 1;
+  }
+
+  // If 'signcolumn' is set to 'number' and there is a sign to display, then
+  // the minimal width for the number column is 2.
+  if (n < 2 && (wp->w_buffer->b_signlist != NULL)
+      && (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u')) {
+    n = 2;
+  }
 
   wp->w_nrwidth_width = n;
   return n;
@@ -7511,3 +7580,5 @@ win_T *get_win_by_grid_handle(handle_T handle)
   }
   return NULL;
 }
+
+

src/nvim/sign.c

@@ -881,6 +881,17 @@ int sign_undefine_by_name(const char_u *name)
   return OK;
 }
 
+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;
+    }
+}
+
 /// List the signs matching 'name'
 static void sign_list_by_name(char_u *name)
 {
@@ -935,6 +946,10 @@ int sign_place(
   }
   if (lnum > 0) {
     redraw_buf_line_later(buf, lnum);
+
+    // 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 {
     EMSG2(_("E885: Not possible to change sign %s"), sign_name);
     return FAIL;
@@ -964,6 +979,13 @@ int sign_unplace(int sign_id, char_u *sign_group, buf_T *buf, linenr_T atlnum)
     redraw_buf_line_later(buf, lnum);
   }
 
+  // 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->b_signlist == NULL) {
+    may_force_numberwidth_recompute(buf, true);
+  }
+
   return OK;
 }
 

src/nvim/testdir/test_signs.vim

@@ -1742,3 +1742,117 @@ func Test_sign_cursor_position()
   call StopVimInTerminal(buf)
   call delete('XtestSigncolumn')
 endfunc
+
+" Return the 'len' characters in screen starting from (row,col)
+func s:ScreenLine(row, col, len)
+  let s = ''
+  for i in range(a:len)
+    let s .= nr2char(screenchar(a:row, a:col + i))
+  endfor
+  return s
+endfunc
+
+" Test for 'signcolumn' set to 'number'.
+func Test_sign_numcol()
+  new
+  call append(0, "01234")
+  " With 'signcolumn' set to 'number', make sure sign is displayed in the
+  " number column and line number is not displayed.
+  set numberwidth=2
+  set number
+  set signcolumn=number
+  sign define sign1 text==>
+  sign place 10 line=1 name=sign1
+  sign define sign2 text=Ｖ
+  redraw!
+  call assert_equal("=> 01234", s:ScreenLine(1, 1, 8))
+
+  " With 'signcolumn' set to 'number', when there is no sign, make sure line
+  " number is displayed in the number column
+  sign unplace 10
+  redraw!
+  call assert_equal("1 01234", s:ScreenLine(1, 1, 7))
+
+  " Disable number column. Check whether sign is displayed in the sign column
+  set numberwidth=4
+  set nonumber
+  sign place 10 line=1 name=sign1
+  redraw!
+  call assert_equal("=>01234", s:ScreenLine(1, 1, 7))
+
+  " Enable number column. Check whether sign is displayed in the number column
+  set number
+  redraw!
+  call assert_equal(" => 01234", s:ScreenLine(1, 1, 9))
+
+  " Disable sign column. Make sure line number is displayed
+  set signcolumn=no
+  redraw!
+  call assert_equal("  1 01234", s:ScreenLine(1, 1, 9))
+
+  " Enable auto sign column. Make sure both sign and line number are displayed
+  set signcolumn=auto
+  redraw!
+  call assert_equal("=>  1 01234", s:ScreenLine(1, 1, 11))
+
+  " Test displaying signs in the number column with width 1
+  call sign_unplace('*')
+  call append(1, "abcde")
+  call append(2, "01234")
+  " Enable number column with width 1
+  set number numberwidth=1 signcolumn=auto
+  redraw!
+  call assert_equal("3 01234", s:ScreenLine(3, 1, 7))
+  " Place a sign and make sure number column width remains the same
+  sign place 20 line=2 name=sign1
+  redraw!
+  call assert_equal("=>2 abcde", s:ScreenLine(2, 1, 9))
+  call assert_equal("  3 01234", s:ScreenLine(3, 1, 9))
+  " Set 'signcolumn' to 'number', make sure the number column width increases
+  set signcolumn=number
+  redraw!
+  call assert_equal("=> abcde", s:ScreenLine(2, 1, 8))
+  call assert_equal(" 3 01234", s:ScreenLine(3, 1, 8))
+  " Set 'signcolumn' to 'auto', make sure the number column width is 1.
+  set signcolumn=auto
+  redraw!
+  call assert_equal("=>2 abcde", s:ScreenLine(2, 1, 9))
+  call assert_equal("  3 01234", s:ScreenLine(3, 1, 9))
+  " Set 'signcolumn' to 'number', make sure the number column width is 2.
+  set signcolumn=number
+  redraw!
+  call assert_equal("=> abcde", s:ScreenLine(2, 1, 8))
+  call assert_equal(" 3 01234", s:ScreenLine(3, 1, 8))
+  " Disable 'number' column
+  set nonumber
+  redraw!
+  call assert_equal("=>abcde", s:ScreenLine(2, 1, 7))
+  call assert_equal("  01234", s:ScreenLine(3, 1, 7))
+  " Enable 'number' column
+  set number
+  redraw!
+  call assert_equal("=> abcde", s:ScreenLine(2, 1, 8))
+  call assert_equal(" 3 01234", s:ScreenLine(3, 1, 8))
+  " Remove the sign and make sure the width of the number column is 1.
+  call sign_unplace('', {'id' : 20})
+  redraw!
+  call assert_equal("3 01234", s:ScreenLine(3, 1, 7))
+  " When the first sign is placed with 'signcolumn' set to number, verify that
+  " the number column width increases
+  sign place 30 line=1 name=sign1
+  redraw!
+  call assert_equal("=> 01234", s:ScreenLine(1, 1, 8))
+  call assert_equal(" 2 abcde", s:ScreenLine(2, 1, 8))
+  " Add sign with multi-byte text
+  set numberwidth=4
+  sign place 40 line=2 name=sign2
+  redraw!
+  call assert_equal(" => 01234", s:ScreenLine(1, 1, 9))
+  call assert_equal(" Ｖ abcde", s:ScreenLine(2, 1, 9))
+
+  sign unplace * group=*
+  sign undefine sign1
+  set signcolumn&
+  set number&
+  enew!  | close
+endfunc