refactor(signs): handle non-sign attrs separately (#19784)

5 files changed, 149 insertions, 136 deletions

diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c
index 8258f2afd5..ef4162a143 100644
--- a/src/nvim/decoration.c
+++ b/src/nvim/decoration.c
@@ -358,7 +358,8 @@ next_mark:
   return attr;
 }
 
-void decor_redraw_signs(buf_T *buf, int row, int *num_signs, sign_attrs_T sattrs[])
+void decor_redraw_signs(buf_T *buf, int row, int *num_signs, SignTextAttrs sattrs[],
+                        HlPriAttr *num_attrs, HlPriAttr *line_attrs, HlPriAttr *cul_attrs)
 {
   if (!buf->b_signs) {
     return;
@@ -383,30 +384,37 @@ void decor_redraw_signs(buf_T *buf, int row, int *num_signs, sign_attrs_T sattrs
       goto next_mark;
     }
 
-    int j;
-    for (j = (*num_signs); j > 0; j--) {
-      if (sattrs[j - 1].sat_prio >= decor->priority) {
-        break;
-      }
-      sattrs[j] = sattrs[j - 1];
-    }
-    if (j < SIGN_SHOW_MAX) {
-      CLEAR_FIELD(sattrs[j]);
-      sattrs[j].sat_text = decor->sign_text;
-      if (decor->sign_hl_id != 0) {
-        sattrs[j].sat_texthl = syn_id2attr(decor->sign_hl_id);
-      }
-      if (decor->number_hl_id != 0) {
-        sattrs[j].sat_numhl = syn_id2attr(decor->number_hl_id);
+    if (decor->sign_text) {
+      int j;
+      for (j = (*num_signs); j > 0; j--) {
+        if (sattrs[j - 1].priority >= decor->priority) {
+          break;
+        }
+        sattrs[j] = sattrs[j - 1];
       }
-      if (decor->line_hl_id != 0) {
-        sattrs[j].sat_linehl = syn_id2attr(decor->line_hl_id);
+      if (j < SIGN_SHOW_MAX) {
+        sattrs[j] = (SignTextAttrs) {
+          .text = decor->sign_text,
+          .hl_attr_id = decor->sign_hl_id == 0 ? 0 : syn_id2attr(decor->sign_hl_id),
+          .priority = decor->priority
+        };
+        (*num_signs)++;
       }
-      if (decor->cursorline_hl_id != 0) {
-        sattrs[j].sat_culhl = syn_id2attr(decor->cursorline_hl_id);
+    }
+
+    struct { HlPriAttr *dest; int hl; } cattrs[] = {
+      { line_attrs, decor->line_hl_id        },
+      { num_attrs,  decor->number_hl_id      },
+      { cul_attrs,  decor->cursorline_hl_id  },
+      { NULL, -1 },
+    };
+    for (int i = 0; cattrs[i].dest; i++) {
+      if (cattrs[i].hl != 0 && decor->priority >= cattrs[i].dest->priority) {
+        *cattrs[i].dest = (HlPriAttr) {
+          .attr_id = syn_id2attr(cattrs[i].hl),
+          .priority = decor->priority
+        };
       }
-      sattrs[j].sat_prio = decor->priority;
-      (*num_signs)++;
     }
 
 next_mark:
diff --git a/src/nvim/highlight_defs.h b/src/nvim/highlight_defs.h
index f41f980054..da6d5284c2 100644
--- a/src/nvim/highlight_defs.h
+++ b/src/nvim/highlight_defs.h
@@ -223,4 +223,10 @@ typedef struct {
 #define COLOR_ITEM_INITIALIZER { .attr_id = -1, .link_id = -1, \
                                  .version = -1, .is_default = false }
 
+/// highlight attributes with associated priorities
+typedef struct {
+  int attr_id;
+  int priority;
+} HlPriAttr;
+
 #endif  // NVIM_HIGHLIGHT_DEFS_H
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index 8cc597fa99..e3b7fff1c4 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -1943,6 +1943,44 @@ static inline void get_line_number_str(win_T *wp, linenr_T lnum, char_u *buf, si
   snprintf((char *)buf, buf_len, fmt, number_width(wp), num);
 }
 
+static void apply_cursorline_highlight(win_T *wp, linenr_T lnum, int *line_attr, int *cul_attr,
+                                       int *line_attr_lowprio)
+{
+  *cul_attr = win_hl_attr(wp, HLF_CUL);
+  HlAttrs ae = syn_attr2entry(*cul_attr);
+  // We make a compromise here (#7383):
+  //  * low-priority CursorLine if fg is not set
+  //  * high-priority ("same as Vim" priority) CursorLine if fg is set
+  if (ae.rgb_fg_color == -1 && ae.cterm_fg_color == 0) {
+    *line_attr_lowprio = *cul_attr;
+  } else {
+    if (!(State & MODE_INSERT) && bt_quickfix(wp->w_buffer)
+        && qf_current_entry(wp) == lnum) {
+      *line_attr = hl_combine_attr(*cul_attr, *line_attr);
+    } else {
+      *line_attr = *cul_attr;
+    }
+  }
+}
+
+static int get_sign_attrs(buf_T *buf, linenr_T lnum, SignTextAttrs *sattrs, int *line_attr,
+                          int *num_attr, int *cul_attr)
+{
+  HlPriAttr line_attrs = { *line_attr, 0 };
+  HlPriAttr num_attrs  = { *num_attr,  0 };
+  HlPriAttr cul_attrs  = { *cul_attr,  0 };
+
+  // TODO(bfredl, vigoux): line_attr should not take priority over decoration!
+  int num_signs = buf_get_signattrs(buf, lnum, sattrs, &num_attrs, &line_attrs, &cul_attrs);
+  decor_redraw_signs(buf, lnum - 1, &num_signs, sattrs, &num_attrs, &line_attrs, &cul_attrs);
+
+  *line_attr = line_attrs.attr_id;
+  *num_attr = num_attrs.attr_id;
+  *cul_attr = cul_attrs.attr_id;
+
+  return num_signs;
+}
+
 /// Display line "lnum" of window 'wp' on the screen.
 /// wp->w_virtcol needs to be valid.
 ///
@@ -2049,8 +2087,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
   bool in_multispace = false;           // in multiple consecutive spaces
   int multispace_pos = 0;               // position in lcs-multispace string
   bool need_showbreak = false;          // overlong line, skip first x chars
-  sign_attrs_T sattrs[SIGN_SHOW_MAX];   // attributes for signs
-  int num_signs;                        // number of signs for line
   int line_attr = 0;                    // attribute for the whole line
   int line_attr_save;
   int line_attr_lowprio = 0;            // low-priority attribute for the line
@@ -2320,21 +2356,7 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
       cul_screenline = (wp->w_p_wrap
                         && (wp->w_p_culopt_flags & CULOPT_SCRLINE));
       if (!cul_screenline) {
-        cul_attr = win_hl_attr(wp, HLF_CUL);
-        HlAttrs ae = syn_attr2entry(cul_attr);
-        // We make a compromise here (#7383):
-        //  * low-priority CursorLine if fg is not set
-        //  * high-priority ("same as Vim" priority) CursorLine if fg is set
-        if (ae.rgb_fg_color == -1 && ae.cterm_fg_color == 0) {
-          line_attr_lowprio = cul_attr;
-        } else {
-          if (!(State & MODE_INSERT) && bt_quickfix(wp->w_buffer)
-              && qf_current_entry(wp) == lnum) {
-            line_attr = hl_combine_attr(cul_attr, line_attr);
-          } else {
-            line_attr = cul_attr;
-          }
-        }
+        apply_cursorline_highlight(wp, lnum, &line_attr, &cul_attr, &line_attr_lowprio);
       } else {
         margin_columns_win(wp, &left_curline_col, &right_curline_col);
       }
@@ -2342,16 +2364,11 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
     }
   }
 
+  SignTextAttrs sattrs[SIGN_SHOW_MAX];  // sign attributes for the sign column
+  int sign_num_attr = 0;                // sign attribute for the number column
+  int sign_cul_attr = 0;                // sign attribute for cursorline
   CLEAR_FIELD(sattrs);
-  num_signs = buf_get_signattrs(wp->w_buffer, lnum, sattrs);
-  decor_redraw_signs(buf, lnum - 1, &num_signs, sattrs);
-
-  // If this line has a sign with line highlighting set line_attr.
-  // TODO(bfredl, vigoux): this should not take priority over decoration!
-  sign_attrs_T *sattr = sign_get_attr(SIGN_LINEHL, sattrs, 0, 1);
-  if (sattr != NULL) {
-    line_attr = sattr->sat_linehl;
-  }
+  int num_signs = get_sign_attrs(buf, lnum, sattrs, &line_attr, &sign_num_attr, &sign_cul_attr);
 
   // Highlight the current line in the quickfix window.
   if (bt_quickfix(wp->w_buffer) && qf_current_entry(wp) == lnum) {
@@ -2623,13 +2640,13 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
       // sign column, this is hit until sign_idx reaches count
       if (draw_state == WL_SIGN - 1 && n_extra == 0) {
         draw_state = WL_SIGN;
-        // Show the sign column when there are any signs in this
-        // buffer or when using Netbeans.
+        // Show the sign column when there are any signs in this buffer
         if (wp->w_scwidth > 0) {
           get_sign_display_info(false, wp, lnum, sattrs, row,
                                 startrow, filler_lines, filler_todo,
                                 &c_extra, &c_final, extra, sizeof(extra),
-                                &p_extra, &n_extra, &char_attr, sign_idx);
+                                &p_extra, &n_extra, &char_attr, sign_idx,
+                                sign_cul_attr);
           sign_idx++;
           if (sign_idx < wp->w_scwidth) {
             draw_state = WL_SIGN - 1;
@@ -2649,12 +2666,12 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
           // If 'signcolumn' is set to 'number' and a sign is present
           // in 'lnum', then display the sign instead of the line
           // number.
-          if (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u'
-              && num_signs > 0 && sign_get_attr(SIGN_TEXT, sattrs, 0, 1)) {
+          if (*wp->w_p_scl == 'n' && *(wp->w_p_scl + 1) == 'u' && num_signs > 0) {
             get_sign_display_info(true, wp, lnum, sattrs, row,
                                   startrow, filler_lines, filler_todo,
                                   &c_extra, &c_final, extra, sizeof(extra),
-                                  &p_extra, &n_extra, &char_attr, sign_idx);
+                                  &p_extra, &n_extra, &char_attr, sign_idx,
+                                  sign_cul_attr);
           } else {
             // Draw the line number (empty space after wrapping).
             if (row == startrow + filler_lines) {
@@ -2681,7 +2698,11 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
             }
             c_final = NUL;
             n_extra = number_width(wp) + 1;
-            char_attr = get_line_number_attr(wp, lnum, row, startrow, filler_lines, sattrs);
+            if (sign_num_attr > 0) {
+              char_attr = sign_num_attr;
+            } else {
+              char_attr = get_line_number_attr(wp, lnum, row, startrow, filler_lines);
+            }
           }
         }
       }
@@ -2810,18 +2831,7 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
     if (cul_screenline && draw_state == WL_LINE
         && vcol >= left_curline_col
         && vcol < right_curline_col) {
-      cul_attr = win_hl_attr(wp, HLF_CUL);
-      HlAttrs ae = syn_attr2entry(cul_attr);
-      if (ae.rgb_fg_color == -1 && ae.cterm_fg_color == 0) {
-        line_attr_lowprio = cul_attr;
-      } else {
-        if (!(State & MODE_INSERT) && bt_quickfix(wp->w_buffer)
-            && qf_current_entry(wp) == lnum) {
-          line_attr = hl_combine_attr(cul_attr, line_attr);
-        } else {
-          line_attr = cul_attr;
-        }
-      }
+      apply_cursorline_highlight(wp, lnum, &line_attr, &cul_attr, &line_attr_lowprio);
     }
 
     // When still displaying '$' of change command, stop at cursor
@@ -4349,15 +4359,8 @@ static bool use_cursor_line_nr(win_T *wp, linenr_T lnum, int row, int startrow,
                  && (wp->w_p_culopt_flags & CULOPT_LINE)));
 }
 
-static int get_line_number_attr(win_T *wp, linenr_T lnum, int row, int startrow, int filler_lines,
-                                sign_attrs_T *sattrs)
+static int get_line_number_attr(win_T *wp, linenr_T lnum, int row, int startrow, int filler_lines)
 {
-  sign_attrs_T *num_sattr = sign_get_attr(SIGN_NUMHL, sattrs, 0, 1);
-  if (num_sattr != NULL) {
-    // :sign defined with "numhl" highlight.
-    return num_sattr->sat_numhl;
-  }
-
   if (wp->w_p_rnu) {
     if (lnum < wp->w_cursor.lnum) {
       // Use LineNrAbove
@@ -4385,10 +4388,11 @@ static int get_line_number_attr(win_T *wp, linenr_T lnum, int row, int startrow,
 // @param count max number of signs
 // @param[out] n_extrap number of characters from pp_extra to display
 // @param sign_idxp Index of the displayed sign
-static void get_sign_display_info(bool nrcol, win_T *wp, linenr_T lnum, sign_attrs_T sattrs[],
+static void get_sign_display_info(bool nrcol, win_T *wp, linenr_T lnum, SignTextAttrs sattrs[],
                                   int row, int startrow, int filler_lines, int filler_todo,
                                   int *c_extrap, int *c_finalp, char_u *extra, size_t extra_size,
-                                  char_u **pp_extra, int *n_extrap, int *char_attrp, int sign_idx)
+                                  char_u **pp_extra, int *n_extrap, int *char_attrp, int sign_idx,
+                                  int cul_attr)
 {
   // Draw cells with the sign value or blank.
   *c_extrap = ' ';
@@ -4405,9 +4409,9 @@ static void get_sign_display_info(bool nrcol, win_T *wp, linenr_T lnum, sign_att
   }
 
   if (row == startrow + filler_lines && filler_todo <= 0) {
-    sign_attrs_T *sattr = sign_get_attr(SIGN_TEXT, sattrs, sign_idx, wp->w_scwidth);
+    SignTextAttrs *sattr = sign_get_attr(sign_idx, sattrs, wp->w_scwidth);
     if (sattr != NULL) {
-      *pp_extra = sattr->sat_text;
+      *pp_extra = sattr->text;
       if (*pp_extra != NULL) {
         *c_extrap = NUL;
         *c_finalp = NUL;
@@ -4423,28 +4427,28 @@ static void get_sign_display_info(bool nrcol, win_T *wp, linenr_T lnum, sign_att
           *pp_extra = extra;
           *n_extrap = (int)STRLEN(*pp_extra);
         } else {
-          int symbol_blen = (int)STRLEN(*pp_extra);
+          size_t symbol_blen = STRLEN(*pp_extra);
 
           // TODO(oni-link): Is sign text already extended to
           // full cell width?
           assert((size_t)win_signcol_width(wp) >= mb_string2cells((char *)(*pp_extra)));
           // symbol(s) bytes + (filling spaces) (one byte each)
-          *n_extrap = symbol_blen + win_signcol_width(wp) -
+          *n_extrap = (int)symbol_blen + win_signcol_width(wp) -
                       (int)mb_string2cells((char *)(*pp_extra));
 
-          assert(extra_size > (size_t)symbol_blen);
+          assert(extra_size > symbol_blen);
           memset(extra, ' ', extra_size);
-          memcpy(extra, *pp_extra, (size_t)symbol_blen);
+          memcpy(extra, *pp_extra, symbol_blen);
 
           *pp_extra = extra;
           (*pp_extra)[*n_extrap] = NUL;
         }
       }
 
-      if (use_cursor_line_sign(wp, lnum) && sattr->sat_culhl > 0) {
-        *char_attrp = sattr->sat_culhl;
+      if (use_cursor_line_sign(wp, lnum) && cul_attr > 0) {
+        *char_attrp = cul_attr;
       } else {
-        *char_attrp = sattr->sat_texthl;
+        *char_attrp = sattr->hl_attr_id;
       }
     }
   }
diff --git a/src/nvim/sign.c b/src/nvim/sign.c
index a1e61a4d8c..9447f32258 100644
--- a/src/nvim/sign.c
+++ b/src/nvim/sign.c
@@ -442,27 +442,24 @@ static linenr_T buf_change_sign_type(buf_T *buf, int markId, const char_u *group
 /// @param max_signs the number of signs, with priority for the ones
 ///        with the highest Ids.
 /// @return Attrs of the matching sign, or NULL
-sign_attrs_T *sign_get_attr(SignType type, sign_attrs_T sattrs[], int idx, int max_signs)
+SignTextAttrs *sign_get_attr(int idx, SignTextAttrs sattrs[], int max_signs)
 {
-  sign_attrs_T *matches[SIGN_SHOW_MAX];
-  int nr_matches = 0;
+  SignTextAttrs *matches[SIGN_SHOW_MAX];
+  int sattr_matches = 0;
 
   for (int i = 0; i < SIGN_SHOW_MAX; i++) {
-    if ((type == SIGN_TEXT && sattrs[i].sat_text != NULL)
-        || (type == SIGN_LINEHL && sattrs[i].sat_linehl != 0)
-        || (type == SIGN_NUMHL && sattrs[i].sat_numhl != 0)) {
-      matches[nr_matches] = &sattrs[i];
-      nr_matches++;
+    if (sattrs[i].text != NULL) {
+      matches[sattr_matches++] = &sattrs[i];
       // attr list is sorted with most important (priority, id), thus we
       // may stop as soon as we have max_signs matches
-      if (nr_matches >= max_signs) {
+      if (sattr_matches >= max_signs) {
         break;
       }
     }
   }
 
-  if (nr_matches > idx) {
-    return matches[nr_matches - idx - 1];
+  if (sattr_matches > idx) {
+    return matches[sattr_matches - idx - 1];
   }
 
   return NULL;
@@ -474,12 +471,12 @@ sign_attrs_T *sign_get_attr(SignType type, sign_attrs_T sattrs[], int idx, int m
 /// @param lnum Line in which to search
 /// @param sattrs Output array for attrs
 /// @return Number of signs of which attrs were found
-int buf_get_signattrs(buf_T *buf, linenr_T lnum, sign_attrs_T sattrs[])
+int buf_get_signattrs(buf_T *buf, linenr_T lnum, SignTextAttrs sattrs[], HlPriAttr *num_attrs,
+                      HlPriAttr *line_attrs, HlPriAttr *cul_attrs)
 {
   sign_entry_T *sign;
-  sign_T *sp;
 
-  int nr_matches = 0;
+  int sattr_matches = 0;
 
   FOR_ALL_SIGNS_IN_BUF(buf, sign) {
     if (sign->se_lnum > lnum) {
@@ -488,37 +485,39 @@ int buf_get_signattrs(buf_T *buf, linenr_T lnum, sign_attrs_T sattrs[])
       break;
     }
 
-    if (sign->se_lnum == lnum) {
-      sign_attrs_T sattr;
-      CLEAR_FIELD(sattr);
-      sattr.sat_typenr = sign->se_typenr;
-      sp = find_sign_by_typenr(sign->se_typenr);
-      if (sp != NULL) {
-        sattr.sat_text = sp->sn_text;
-        if (sattr.sat_text != NULL && sp->sn_text_hl != 0) {
-          sattr.sat_texthl = syn_id2attr(sp->sn_text_hl);
-        }
-        if (sp->sn_line_hl != 0) {
-          sattr.sat_linehl = syn_id2attr(sp->sn_line_hl);
-        }
-        if (sp->sn_cul_hl != 0) {
-          sattr.sat_culhl = syn_id2attr(sp->sn_cul_hl);
-        }
-        if (sp->sn_num_hl != 0) {
-          sattr.sat_numhl = syn_id2attr(sp->sn_num_hl);
-        }
-        // Store the priority so we can mesh in extmark signs later
-        sattr.sat_prio = sign->se_priority;
-      }
+    if (sign->se_lnum < lnum) {
+      continue;
+    }
 
-      sattrs[nr_matches] = sattr;
-      nr_matches++;
-      if (nr_matches == SIGN_SHOW_MAX) {
-        break;
+    sign_T *sp = find_sign_by_typenr(sign->se_typenr);
+    if (sp == NULL) {
+      continue;
+    }
+
+    if (sp->sn_text != NULL && sattr_matches < SIGN_SHOW_MAX) {
+      sattrs[sattr_matches++] = (SignTextAttrs) {
+        .text = sp->sn_text,
+        .hl_attr_id = sp->sn_text_hl == 0 ? 0 : syn_id2attr(sp->sn_text_hl),
+        .priority = sign->se_priority
+      };
+    }
+
+    struct { HlPriAttr *dest; int hl; } cattrs[] = {
+      { line_attrs, sp->sn_line_hl },
+      { num_attrs,  sp->sn_num_hl  },
+      { cul_attrs,  sp->sn_cul_hl  },
+      { NULL, -1 },
+    };
+    for (int i = 0; cattrs[i].dest; i++) {
+      if (cattrs[i].hl != 0 && sign->se_priority >= cattrs[i].dest->priority) {
+        *cattrs[i].dest = (HlPriAttr) {
+          .attr_id = syn_id2attr(cattrs[i].hl),
+          .priority = sign->se_priority
+        };
       }
     }
   }
-  return nr_matches;
+  return sattr_matches;
 }
 
 /// Delete sign 'id' in group 'group' from buffer 'buf'.
diff --git a/src/nvim/sign_defs.h b/src/nvim/sign_defs.h
index e4ece71846..a4fb325ec8 100644
--- a/src/nvim/sign_defs.h
+++ b/src/nvim/sign_defs.h
@@ -33,15 +33,11 @@ struct sign_entry {
 };
 
 /// Sign attributes. Used by the screen refresh routines.
-typedef struct sign_attrs_S {
-  int sat_typenr;
-  char_u *sat_text;
-  int sat_texthl;
-  int sat_linehl;
-  int sat_culhl;
-  int sat_numhl;
-  int sat_prio;  // Used for inserting extmark signs
-} sign_attrs_T;
+typedef struct {
+  char_u *text;
+  int hl_attr_id;
+  int priority;
+} SignTextAttrs;
 
 #define SIGN_SHOW_MAX 9