fix(highlight): avoid ORing underline flags (#22372)

When combining attributes use the one that takes priority.
For :highlight command use the last one specified.
For API use a hard-coded order same as the order in docs.

5 files changed, 120 insertions, 4 deletions

diff --git a/src/nvim/highlight.c b/src/nvim/highlight.c
index 72da66cfc2..305e84276c 100644
--- a/src/nvim/highlight.c
+++ b/src/nvim/highlight.c
@@ -507,6 +507,16 @@ void hl_invalidate_blends(void)
   update_window_hl(curwin, true);
 }
 
+/// Combine HlAttrFlags.
+/// The underline attribute in "prim_ae" overrules the one in "char_ae" if both are present.
+static int16_t hl_combine_ae(int16_t char_ae, int16_t prim_ae)
+{
+  int16_t char_ul = char_ae & HL_UNDERLINE_MASK;
+  int16_t prim_ul = prim_ae & HL_UNDERLINE_MASK;
+  int16_t new_ul = prim_ul ? prim_ul : char_ul;
+  return (char_ae & ~HL_UNDERLINE_MASK) | (prim_ae & ~HL_UNDERLINE_MASK) | new_ul;
+}
+
 // Combine special attributes (e.g., for spelling) with other attributes
 // (e.g., for syntax highlighting).
 // "prim_attr" overrules "char_attr".
@@ -537,12 +547,12 @@ int hl_combine_attr(int char_attr, int prim_attr)
   if (prim_aep.cterm_ae_attr & HL_NOCOMBINE) {
     new_en.cterm_ae_attr = prim_aep.cterm_ae_attr;
   } else {
-    new_en.cterm_ae_attr |= prim_aep.cterm_ae_attr;
+    new_en.cterm_ae_attr = hl_combine_ae(new_en.cterm_ae_attr, prim_aep.cterm_ae_attr);
   }
   if (prim_aep.rgb_ae_attr & HL_NOCOMBINE) {
     new_en.rgb_ae_attr = prim_aep.rgb_ae_attr;
   } else {
-    new_en.rgb_ae_attr |= prim_aep.rgb_ae_attr;
+    new_en.rgb_ae_attr = hl_combine_ae(new_en.rgb_ae_attr, prim_aep.rgb_ae_attr);
   }
 
   if (prim_aep.cterm_fg_color > 0) {
@@ -664,7 +674,7 @@ int hl_blend_attrs(int back_attr, int front_attr, bool *through)
   } else {
     cattrs = fattrs;
     if (ratio >= 50) {
-      cattrs.rgb_ae_attr |= battrs.rgb_ae_attr;
+      cattrs.rgb_ae_attr = hl_combine_ae(battrs.rgb_ae_attr, cattrs.rgb_ae_attr);
     }
     cattrs.rgb_fg_color = rgb_blend(ratio/2, battrs.rgb_fg_color,
                                     fattrs.rgb_fg_color);
@@ -924,7 +934,10 @@ HlAttrs dict2hlattrs(Dict(highlight) *dict, bool use_rgb, int *link_id, Error *e
 
 #define CHECK_FLAG(d, m, name, extra, flag) \
   if (api_object_to_bool(d->name##extra, #name, false, err)) { \
-    m = m | flag; \
+    if (flag & HL_UNDERLINE_MASK) { \
+      m &= ~HL_UNDERLINE_MASK; \
+    } \
+    m |= flag; \
   }
 
   CHECK_FLAG(dict, mask, reverse, , HL_INVERSE);
diff --git a/src/nvim/highlight_group.c b/src/nvim/highlight_group.c
index e34c13abc1..d2f5b60dc6 100644
--- a/src/nvim/highlight_group.c
+++ b/src/nvim/highlight_group.c
@@ -1124,6 +1124,9 @@ void do_highlight(const char *line, const bool forceit, const bool init)
           for (i = ARRAY_SIZE(hl_attr_table); --i >= 0;) {
             int len = (int)strlen(hl_name_table[i]);
             if (STRNICMP(arg + off, hl_name_table[i], len) == 0) {
+              if (hl_attr_table[i] & HL_UNDERLINE_MASK) {
+                attr &= ~HL_UNDERLINE_MASK;
+              }
               attr |= hl_attr_table[i];
               off += len;
               break;
diff --git a/test/functional/api/highlight_spec.lua b/test/functional/api/highlight_spec.lua
index e1b3562329..eb7d0f7b47 100644
--- a/test/functional/api/highlight_spec.lua
+++ b/test/functional/api/highlight_spec.lua
@@ -278,6 +278,20 @@ describe("API: set highlight", function()
     eq(highlight3_result_cterm, meths.get_hl_by_name('Test_hl', false))
   end)
 
+  it("only allows one underline attribute #22371", function()
+    local ns = get_ns()
+    meths.set_hl(ns, 'Test_hl', {
+      underdouble = true,
+      underdotted = true,
+      cterm = {
+        underline = true,
+        undercurl = true,
+      },
+    })
+    eq({ undercurl = true }, meths.get_hl_by_name('Test_hl', false))
+    eq({ underdotted = true }, meths.get_hl_by_name('Test_hl', true))
+  end)
+
   it("can set a highlight in the global namespace", function()
     meths.set_hl(0, 'Test_hl', highlight2_config)
     eq('Test_hl        xxx cterm=underline,reverse ctermfg=8 ctermbg=15 gui=underline,reverse',
diff --git a/test/functional/ex_cmds/highlight_spec.lua b/test/functional/ex_cmds/highlight_spec.lua
index 1cd6759a53..18f215cf75 100644
--- a/test/functional/ex_cmds/highlight_spec.lua
+++ b/test/functional/ex_cmds/highlight_spec.lua
@@ -36,4 +36,13 @@ describe(':highlight', function()
     command('highlight normal ctermbg=red')
     eq('9', eval('synIDattr(hlID("Normal"), "bg", "cterm")'))
   end)
+
+  it('only the last underline style takes effect #22371', function()
+    command('highlight NonText gui=underline,undercurl')
+    eq('', eval('synIDattr(hlID("NonText"), "underline", "gui")'))
+    eq('1', eval('synIDattr(hlID("NonText"), "undercurl", "gui")'))
+    command('highlight NonText gui=undercurl,underline')
+    eq('', eval('synIDattr(hlID("NonText"), "undercurl", "gui")'))
+    eq('1', eval('synIDattr(hlID("NonText"), "underline", "gui")'))
+  end)
 end)
diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua
index 6ed32a8cd4..4759d68200 100644
--- a/test/functional/ui/decorations_spec.lua
+++ b/test/functional/ui/decorations_spec.lua
@@ -1045,6 +1045,83 @@ end]]
 
   end)
 
+  it('underline attribute with higher priority takes effect #22371', function()
+    screen:try_resize(50, 3)
+    insert('aaabbbaaa')
+    exec([[
+      hi TestUL gui=underline guifg=Blue
+      hi TestUC gui=undercurl guisp=Red
+      hi TestBold gui=bold
+    ]])
+    screen:set_default_attr_ids({
+      [0] = {bold = true, foreground = Screen.colors.Blue};
+      [1] = {underline = true, foreground = Screen.colors.Blue};
+      [2] = {undercurl = true, special = Screen.colors.Red};
+      [3] = {underline = true, foreground = Screen.colors.Blue, special = Screen.colors.Red};
+      [4] = {undercurl = true, foreground = Screen.colors.Blue, special = Screen.colors.Red};
+      [5] = {bold = true, underline = true, foreground = Screen.colors.Blue};
+      [6] = {bold = true, undercurl = true, special = Screen.colors.Red};
+    })
+
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUL', priority = 20 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestUC', priority = 30 })
+    screen:expect([[
+      {1:aaa}{4:bbb}{1:aa^a}                                         |
+      {0:~                                                 }|
+                                                        |
+    ]])
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUC', priority = 20 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestUL', priority = 30 })
+    screen:expect([[
+      {2:aaa}{3:bbb}{2:aa^a}                                         |
+      {0:~                                                 }|
+                                                        |
+    ]])
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUL', priority = 30 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestUC', priority = 20 })
+    screen:expect([[
+      {1:aaa}{3:bbb}{1:aa^a}                                         |
+      {0:~                                                 }|
+                                                        |
+    ]])
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUC', priority = 30 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestUL', priority = 20 })
+    screen:expect([[
+      {2:aaa}{4:bbb}{2:aa^a}                                         |
+      {0:~                                                 }|
+                                                        |
+    ]])
+
+    -- When only one highlight group has an underline attribute, it should always take effect.
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUL', priority = 20 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestBold', priority = 30 })
+    screen:expect([[
+      {1:aaa}{5:bbb}{1:aa^a}                                         |
+      {0:~                                                 }|
+                                                        |
+    ]])
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUL', priority = 30 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestBold', priority = 20 })
+    screen:expect_unchanged(true)
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUC', priority = 20 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestBold', priority = 30 })
+    screen:expect([[
+      {2:aaa}{6:bbb}{2:aa^a}                                         |
+      {0:~                                                 }|
+                                                        |
+    ]])
+    meths.buf_clear_namespace(0, ns, 0, -1)
+    meths.buf_set_extmark(0, ns, 0, 0, { end_col = 9, hl_group = 'TestUC', priority = 30 })
+    meths.buf_set_extmark(0, ns, 0, 3, { end_col = 6, hl_group = 'TestBold', priority = 20 })
+    screen:expect_unchanged(true)
+  end)
+
 end)
 
 describe('decorations: virtual lines', function()