fix(drawline): combine extmark highligh with area hl correctly

fixes #23734

Get rid of the weird attr_pri dance which always seemed like a kludge:

    if (!attr_pri) {
      wlv.char_attr = hl_combine_attr(wlv.char_attr, extmark_attr);
    } else {
      wlv.char_attr = hl_combine_attr(extmark_attr, wlv.char_attr);
    }

Instead combine extmark attrs with (old-skool) syntax attrs in a consistent way and then combine that with attr_pri and the rest in an _unified_ code path

fixes #23722

Co-authored-by: luukvbaal <luukvbaal@gmail.com>
Co-authored-by: zeertzjq <zeertzjq@outlook.com>

src/nvim/drawline.c

@@ -1073,7 +1073,7 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
   int saved_search_attr = 0;            // search_attr to be used when n_extra
                                         // goes to zero
   int vcol_save_attr = 0;               // saved attr for 'cursorcolumn'
-  int syntax_attr = 0;                  // attributes desired by syntax
+  int decor_attr = 0;                   // attributes desired by syntax and extmarks
   bool has_syntax = false;              // this buffer has syntax highl.
   int save_did_emsg;
   int eol_hl_off = 0;                   // 1 if highlighted char after EOL
@@ -1902,11 +1902,7 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
         wlv.char_attr = wlv.line_attr;
       } else {
         attr_pri = false;
-        if (has_syntax) {
+        wlv.char_attr = decor_attr;
-          wlv.char_attr = syntax_attr;
-        } else {
-          wlv.char_attr = 0;
-        }
       }
     }
 
@@ -2105,11 +2101,12 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
       }
       ptr++;
 
+      decor_attr = 0;
       if (extra_check) {
         bool no_plain_buffer = (wp->w_s->b_p_spo_flags & SPO_NPBUFFER) != 0;
         bool can_spell = !no_plain_buffer;
 
-        // Get syntax attribute, unless still at the start of the line
+        // Get extmark and syntax attributes, unless still at the start of the line
         // (double-wide char that doesn't fit).
         v = (ptr - line);
         if (has_syntax && v > 0) {
@@ -2118,8 +2115,8 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
           save_did_emsg = did_emsg;
           did_emsg = false;
 
-          syntax_attr = get_syntax_attr((colnr_T)v - 1,
+          decor_attr = get_syntax_attr((colnr_T)v - 1,
-                                        has_spell ? &can_spell : NULL, false);
+                                       has_spell ? &can_spell : NULL, false);
 
           if (did_emsg) {  // -V547
             wp->w_s->b_syn_error = true;
@@ -2137,17 +2134,6 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
           line = ml_get_buf(wp->w_buffer, lnum, false);
           ptr = line + v;
 
-          if (!attr_pri) {
-            if (wlv.cul_attr) {
-              wlv.char_attr = 0 != wlv.line_attr_lowprio
-                ? hl_combine_attr(wlv.cul_attr, syntax_attr)
-                : hl_combine_attr(syntax_attr, wlv.cul_attr);
-            } else {
-              wlv.char_attr = syntax_attr;
-            }
-          } else {
-            wlv.char_attr = hl_combine_attr(syntax_attr, wlv.char_attr);
-          }
           // no concealing past the end of the line, it interferes
           // with line highlighting.
           if (c == NUL) {
@@ -2155,27 +2141,35 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
           } else {
             syntax_flags = get_syntax_info(&syntax_seqnr);
           }
-        } else if (!attr_pri) {
-          wlv.char_attr = 0;
         }
 
         if (has_decor && v > 0) {
-          if (extmark_attr != 0) {
+          // extmarks take preceedence over syntax.c
-            if (!attr_pri) {
+          decor_attr = hl_combine_attr(decor_attr, extmark_attr);
-              wlv.char_attr = hl_combine_attr(wlv.char_attr, extmark_attr);
-            } else {
-              wlv.char_attr = hl_combine_attr(extmark_attr, wlv.char_attr);
-            }
-          }
 
           decor_conceal = decor_state.conceal;
           if (decor_conceal && decor_state.conceal_char) {
             decor_conceal = 2;  // really??
           }
-
           can_spell = TRISTATE_TO_BOOL(decor_state.spell, can_spell);
         }
 
+        if (decor_attr) {
+          if (!attr_pri) {
+            if (wlv.cul_attr) {
+              wlv.char_attr = 0 != wlv.line_attr_lowprio
+                ? hl_combine_attr(wlv.cul_attr, decor_attr)
+                : hl_combine_attr(decor_attr, wlv.cul_attr);
+            } else {
+              wlv.char_attr = decor_attr;
+            }
+          } else {
+            wlv.char_attr = hl_combine_attr(decor_attr, wlv.char_attr);
+          }
+        } else if (!attr_pri) {
+          wlv.char_attr = 0;
+        }
+
         // Check spelling (unless at the end of the line).
         // Only do this when there is no syntax highlighting, the
         // @Spell cluster is not used or the current syntax item
@@ -2186,8 +2180,7 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool nochange,
           char *prev_ptr = ptr - mb_l;
           // do not calculate cap_col at the end of the line or when
           // only white space is following
-          if (c != 0 && (*skipwhite(prev_ptr) != NUL)
+          if (c != 0 && (*skipwhite(prev_ptr) != NUL) && can_spell) {
-              && ((!has_syntax && !no_plain_buffer) || can_spell)) {
             char *p;
             hlf_T spell_hlf = HLF_COUNT;
             v -= mb_l - 1;

test/functional/ui/decorations_spec.lua

@@ -659,6 +659,8 @@ describe('extmark decorations', function()
       [25] = {background = Screen.colors.LightRed};
       [26] = {background=Screen.colors.DarkGrey, foreground=Screen.colors.LightGrey};
       [27] = {background = Screen.colors.Plum1};
+      [28] = {underline = true, foreground = Screen.colors.SlateBlue};
+      [29] = {foreground = Screen.colors.SlateBlue, background = Screen.colors.LightGray, underline = true};
     }
 
     ns = meths.create_namespace 'test'
@@ -1233,8 +1235,53 @@ describe('extmark decorations', function()
     meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestBold', priority = 20 })
     screen:expect_unchanged(true)
   end)
-end)
 
+  it('highlights the beginning of a TAB char correctly', function()
+    screen:try_resize(50, 3)
+    meths.buf_set_lines(0, 0, -1, true, {'this is the\ttab'})
+    meths.buf_set_extmark(0, ns, 0, 11, { end_col = 15, hl_group = 'ErrorMsg' })
+    screen:expect{grid=[[
+      ^this is the{4:     tab}                               |
+      {1:~                                                 }|
+                                                        |
+    ]]}
+
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 12, { end_col = 15, hl_group = 'ErrorMsg' })
+    screen:expect{grid=[[
+      ^this is the     {4:tab}                               |
+      {1:~                                                 }|
+                                                        |
+    ]]}
+  end)
+
+  pending('highlight applies to a full Tab in visual block mode #23734', function()
+    screen:try_resize(50, 8)
+    meths.buf_set_lines(0, 0, -1, true, {'asdf', '\tasdf', '\tasdf', '\tasdf', 'asdf'})
+    meths.buf_set_extmark(0, ns, 0, 0, {end_row = 5, end_col = 0, hl_group = 'Underlined'})
+    screen:expect([[
+      {28:^asdf}                                              |
+      {28:        asdf}                                      |
+      {28:        asdf}                                      |
+      {28:        asdf}                                      |
+      {28:asdf}                                              |
+      {1:~                                                 }|
+      {1:~                                                 }|
+                                                        |
+    ]])
+    feed('<C-V>Gll')
+    screen:expect([[
+      {29:asd}{28:f}                                              |
+      {29:   }{28:     asdf}                                      |
+      {29:   }{28:     asdf}                                      |
+      {29:   }{28:     asdf}                                      |
+      {29:as}{28:^df}                                              |
+      {1:~                                                 }|
+      {1:~                                                 }|
+      {24:-- VISUAL BLOCK --}                                |
+    ]])
+  end)
+end)
 
 describe('decorations: inline virtual text', function()
   local screen, ns

test/functional/ui/highlight_spec.lua

@@ -526,6 +526,41 @@ describe('highlight', function()
     })
 
   end)
+
+  it("'diff', syntax and extmark", function()
+    local screen = Screen.new(25,10)
+    screen:attach()
+    exec([[
+      new
+      call setline(1, ['', '01234 6789'])
+      windo diffthis
+      wincmd w
+      syn match WarningMsg "^.*$"
+      call nvim_buf_add_highlight(0, -1, 'ErrorMsg', 1, 2, 8)
+    ]])
+    screen:expect([[
+      {1:  }^                       |
+      {1:  }{2:01}{3:234 67}{2:89}{5:             }|
+      {4:~                        }|
+      {4:~                        }|
+      {7:[No Name] [+]            }|
+      {1:  }                       |
+      {1:  }{6:-----------------------}|
+      {4:~                        }|
+      {8:[No Name]                }|
+                               |
+    ]],{
+      [0] = {Screen.colors.WebGray, foreground = Screen.colors.DarkBlue},
+      [1] = {background = Screen.colors.Grey, foreground = Screen.colors.Blue4},
+      [2] = {foreground = Screen.colors.Red, background = Screen.colors.LightBlue},
+      [3] = {foreground = Screen.colors.Grey100, background = Screen.colors.LightBlue},
+      [4] = {bold = true, foreground = Screen.colors.Blue},
+      [5] = {background = Screen.colors.LightBlue},
+      [6] = {bold = true, background = Screen.colors.LightCyan, foreground = Screen.colors.Blue1},
+      [7] = {reverse = true, bold = true},
+      [8] = {reverse = true},
+    })
+  end)
 end)
 
 describe("'listchars' highlight", function()