fix(api): make nvim_get_hl return 'cterm' attrs properly

6 files changed, 64 insertions, 87 deletions

diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index 201989aee5..5dd858a6a9 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -727,8 +727,8 @@ void remote_ui_hl_attr_define(UI *ui, Integer id, HlAttrs rgb_attrs, HlAttrs cte
   ADD_C(args, INTEGER_OBJ(id));
   MAXSIZE_TEMP_DICT(rgb, HLATTRS_DICT_SIZE);
   MAXSIZE_TEMP_DICT(cterm, HLATTRS_DICT_SIZE);
-  hlattrs2dict(&rgb, rgb_attrs, true, false);
-  hlattrs2dict(&cterm, rgb_attrs, false, false);
+  hlattrs2dict(&rgb, NULL, rgb_attrs, true, false);
+  hlattrs2dict(&cterm, NULL, rgb_attrs, false, false);
   ADD_C(args, DICTIONARY_OBJ(rgb));
   ADD_C(args, DICTIONARY_OBJ(cterm));
 
@@ -751,7 +751,7 @@ void remote_ui_highlight_set(UI *ui, int id)
   }
   data->hl_id = id;
   MAXSIZE_TEMP_DICT(dict, HLATTRS_DICT_SIZE);
-  hlattrs2dict(&dict, syn_attr2entry(id), ui->rgb, false);
+  hlattrs2dict(&dict, NULL, syn_attr2entry(id), ui->rgb, false);
   ADD_C(args, DICTIONARY_OBJ(dict));
   push_call(ui, "highlight_set", args);
 }
@@ -950,7 +950,7 @@ static Array translate_contents(UI *ui, Array contents, Arena *arena)
     int attr = (int)item.items[0].data.integer;
     if (attr) {
       Dictionary rgb_attrs = arena_dict(arena, HLATTRS_DICT_SIZE);
-      hlattrs2dict(&rgb_attrs, syn_attr2entry(attr), ui->rgb, false);
+      hlattrs2dict(&rgb_attrs, NULL, syn_attr2entry(attr), ui->rgb, false);
       ADD(new_item, DICTIONARY_OBJ(rgb_attrs));
     } else {
       ADD(new_item, DICTIONARY_OBJ((Dictionary)ARRAY_DICT_INIT));
diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c
index 8f4c7b1d80..04ecdd46a8 100644
--- a/src/nvim/hashtab.c
+++ b/src/nvim/hashtab.c
@@ -207,7 +207,7 @@ int hash_add(hashtab_T *ht, char *key)
   hash_T hash = hash_hash(key);
   hashitem_T *hi = hash_lookup(ht, key, strlen(key), hash);
   if (!HASHITEM_EMPTY(hi)) {
-    internal_error("hash_add()");
+    siemsg(_("E685: Internal error: hash_add(): duplicate key \"%s\""), key);
     return FAIL;
   }
   hash_add_item(ht, hi, key, hash);
diff --git a/src/nvim/highlight.c b/src/nvim/highlight.c
index a97f8f4142..36da057ddc 100644
--- a/src/nvim/highlight.c
+++ b/src/nvim/highlight.c
@@ -821,7 +821,7 @@ Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Arena *arena, Error *
     return dic;
   }
   Dictionary retval = arena_dict(arena, HLATTRS_DICT_SIZE);
-  hlattrs2dict(&retval, syn_attr2entry((int)attr_id), rgb, false);
+  hlattrs2dict(&retval, NULL, syn_attr2entry((int)attr_id), rgb, false);
   return retval;
 }
 
@@ -832,99 +832,98 @@ Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Arena *arena, Error *
 /// @param use_rgb use 'gui*' settings if true, else resorts to 'cterm*'
 /// @param short_keys change (foreground, background, special) to (fg, bg, sp) for 'gui*' settings
 ///                          (foreground, background) to (ctermfg, ctermbg) for 'cterm*' settings
-void hlattrs2dict(Dictionary *dict, HlAttrs ae, bool use_rgb, bool short_keys)
+void hlattrs2dict(Dictionary *hl, Dictionary *hl_attrs, HlAttrs ae, bool use_rgb, bool short_keys)
 {
-  assert(dict->capacity >= HLATTRS_DICT_SIZE);  // at most 16 items
-  Dictionary hl = *dict;
+  hl_attrs = hl_attrs ? hl_attrs : hl;
+  assert(hl->capacity >= HLATTRS_DICT_SIZE);  // at most 16 items
+  assert(hl_attrs->capacity >= HLATTRS_DICT_SIZE);  // at most 16 items
   int mask  = use_rgb ? ae.rgb_ae_attr : ae.cterm_ae_attr;
 
   if (mask & HL_INVERSE) {
-    PUT_C(hl, "reverse", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "reverse", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_BOLD) {
-    PUT_C(hl, "bold", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "bold", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_ITALIC) {
-    PUT_C(hl, "italic", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "italic", BOOLEAN_OBJ(true));
   }
 
   switch (mask & HL_UNDERLINE_MASK) {
   case HL_UNDERLINE:
-    PUT_C(hl, "underline", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "underline", BOOLEAN_OBJ(true));
     break;
 
   case HL_UNDERCURL:
-    PUT_C(hl, "undercurl", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "undercurl", BOOLEAN_OBJ(true));
     break;
 
   case HL_UNDERDOUBLE:
-    PUT_C(hl, "underdouble", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "underdouble", BOOLEAN_OBJ(true));
     break;
 
   case HL_UNDERDOTTED:
-    PUT_C(hl, "underdotted", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "underdotted", BOOLEAN_OBJ(true));
     break;
 
   case HL_UNDERDASHED:
-    PUT_C(hl, "underdashed", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "underdashed", BOOLEAN_OBJ(true));
     break;
   }
 
   if (mask & HL_STANDOUT) {
-    PUT_C(hl, "standout", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "standout", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_STRIKETHROUGH) {
-    PUT_C(hl, "strikethrough", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "strikethrough", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_ALTFONT) {
-    PUT_C(hl, "altfont", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "altfont", BOOLEAN_OBJ(true));
   }
 
   if (mask & HL_NOCOMBINE) {
-    PUT_C(hl, "nocombine", BOOLEAN_OBJ(true));
+    PUT_C(*hl_attrs, "nocombine", BOOLEAN_OBJ(true));
   }
 
   if (use_rgb) {
     if (ae.rgb_fg_color != -1) {
-      PUT_C(hl, short_keys ? "fg" : "foreground", INTEGER_OBJ(ae.rgb_fg_color));
+      PUT_C(*hl, short_keys ? "fg" : "foreground", INTEGER_OBJ(ae.rgb_fg_color));
     }
 
     if (ae.rgb_bg_color != -1) {
-      PUT_C(hl, short_keys ? "bg" : "background", INTEGER_OBJ(ae.rgb_bg_color));
+      PUT_C(*hl, short_keys ? "bg" : "background", INTEGER_OBJ(ae.rgb_bg_color));
     }
 
     if (ae.rgb_sp_color != -1) {
-      PUT_C(hl, short_keys ? "sp" : "special", INTEGER_OBJ(ae.rgb_sp_color));
+      PUT_C(*hl, short_keys ? "sp" : "special", INTEGER_OBJ(ae.rgb_sp_color));
     }
 
     if (!short_keys) {
       if (mask & HL_FG_INDEXED) {
-        PUT_C(hl, "fg_indexed", BOOLEAN_OBJ(true));
+        PUT_C(*hl, "fg_indexed", BOOLEAN_OBJ(true));
       }
 
       if (mask & HL_BG_INDEXED) {
-        PUT_C(hl, "bg_indexed", BOOLEAN_OBJ(true));
+        PUT_C(*hl, "bg_indexed", BOOLEAN_OBJ(true));
       }
     }
   } else {
     if (ae.cterm_fg_color != 0) {
-      PUT_C(hl, short_keys ? "ctermfg" : "foreground", INTEGER_OBJ(ae.cterm_fg_color - 1));
+      PUT_C(*hl, short_keys ? "ctermfg" : "foreground", INTEGER_OBJ(ae.cterm_fg_color - 1));
     }
 
     if (ae.cterm_bg_color != 0) {
-      PUT_C(hl, short_keys ? "ctermbg" : "background", INTEGER_OBJ(ae.cterm_bg_color - 1));
+      PUT_C(*hl, short_keys ? "ctermbg" : "background", INTEGER_OBJ(ae.cterm_bg_color - 1));
     }
   }
 
-  if (ae.hl_blend > -1) {
-    PUT_C(hl, "blend", INTEGER_OBJ(ae.hl_blend));
+  if (ae.hl_blend > -1 && (use_rgb || !short_keys)) {
+    PUT_C(*hl, "blend", INTEGER_OBJ(ae.hl_blend));
   }
-
-  *dict = hl;
 }
 
 HlAttrs dict2hlattrs(Dict(highlight) *dict, bool use_rgb, int *link_id, Error *err)
diff --git a/src/nvim/highlight_group.c b/src/nvim/highlight_group.c
index 96b55b5abb..38af2a708a 100644
--- a/src/nvim/highlight_group.c
+++ b/src/nvim/highlight_group.c
@@ -1536,8 +1536,12 @@ static bool hlgroup2dict(Dictionary *hl, NS ns_id, int hl_id, Arena *arena)
     PUT_C(*hl, "link", STRING_OBJ(cstr_as_string(hl_table[link - 1].sg_name)));
   } else {
     *hl = arena_dict(arena, HLATTRS_DICT_SIZE);
-    hlattrs2dict(hl, attr, true, true);
-    hlattrs2dict(hl, attr, false, true);
+    Dictionary hl_cterm = arena_dict(arena, HLATTRS_DICT_SIZE);
+    hlattrs2dict(hl, NULL, attr, true, true);
+    hlattrs2dict(hl, &hl_cterm, attr, false, true);
+    if (kv_size(hl_cterm)) {
+      PUT_C(*hl, "cterm", DICTIONARY_OBJ(hl_cterm));
+    }
   }
   return true;
 }
diff --git a/test/functional/api/highlight_spec.lua b/test/functional/api/highlight_spec.lua
index de3eb4e798..65b13bebf7 100644
--- a/test/functional/api/highlight_spec.lua
+++ b/test/functional/api/highlight_spec.lua
@@ -380,14 +380,14 @@ describe('API: get highlight', function()
   local highlight1 = {
     bg = highlight_color.bg,
     fg = highlight_color.fg,
-    bold = true,
-    italic = true,
+    bold = true, italic = true,
+    cterm = {bold = true, italic = true},
   }
   local highlight2 = {
     ctermbg = highlight_color.ctermbg,
     ctermfg = highlight_color.ctermfg,
-    underline = true,
-    reverse = true,
+    underline = true, reverse = true,
+    cterm = {underline = true, reverse = true},
   }
   local highlight3_config = {
     bg = highlight_color.bg,
@@ -413,13 +413,8 @@ describe('API: get highlight', function()
     fg = highlight_color.fg,
     ctermbg = highlight_color.ctermbg,
     ctermfg = highlight_color.ctermfg,
-    bold = true,
-    italic = true,
-    nocombine = true,
-    reverse = true,
-    underdashed = true,
-    strikethrough = true,
-    altfont = true,
+    bold = true, italic = true, reverse = true, underdashed = true, strikethrough = true, altfont = true,
+    cterm = {italic = true, nocombine = true, reverse = true, strikethrough = true, altfont = true}
   }
 
   local function get_ns()
@@ -500,7 +495,8 @@ describe('API: get highlight', function()
         undercurl = true,
       },
     })
-    eq({ undercurl = true, underdotted = true }, meths.get_hl(ns, { name = 'Test_hl' }))
+    eq({ underdotted = true, cterm = { undercurl = true} },
+       meths.get_hl(ns, { name = 'Test_hl' }))
   end)
 
   it('can get a highlight in the global namespace', function()
@@ -524,43 +520,15 @@ describe('API: get highlight', function()
     }, meths.get_hl(0, { name = 'Test_hl3' }))
   end)
 
-  local expected_rgb = {
-    altfont = true,
-    bg = 16776960,
-    bold = true,
-    ctermbg = 10,
-    fg = 16711680,
-    italic = true,
-    nocombine = true,
-    reverse = true,
-    sp = 255,
-    strikethrough = true,
-    underline = true,
-  }
-  local expected = {
-    bg = 16776960,
-    bold = true,
-    ctermbg = 10,
-    fg = 16711680,
-    sp = 255,
-    underline = true,
-  }
-  local expected_undercurl = {
-    bg = 16776960,
-    ctermbg = 10,
-    fg = 16711680,
-    sp = 255,
-    undercurl = true,
-    underline = true,
-  }
-
   it('nvim_get_hl by id', function()
     local hl_id = meths.get_hl_id_by_name('NewHighlight')
 
     command(
       'hi NewHighlight cterm=underline ctermbg=green guifg=red guibg=yellow guisp=blue gui=bold'
     )
-    eq(expected, meths.get_hl(0, { id = hl_id }))
+    eq({ fg = 16711680, bg = 16776960, sp = 255, bold = true,
+         ctermbg = 10, cterm = { underline = true },
+    }, meths.get_hl(0, { id = hl_id }))
 
     -- Test 0 argument
     eq('Highlight id out of bounds', pcall_err(meths.get_hl, 0, { id = 0 }))
@@ -572,11 +540,17 @@ describe('API: get highlight', function()
 
     -- Test all highlight properties.
     command('hi NewHighlight gui=underline,bold,italic,reverse,strikethrough,altfont,nocombine')
-    eq(expected_rgb, meths.get_hl(0, { id = hl_id }))
+    eq({ fg = 16711680, bg = 16776960, sp = 255,
+         altfont = true, bold = true, italic = true, nocombine = true, reverse = true, strikethrough = true, underline = true,
+         ctermbg = 10, cterm = {underline = true},
+    }, meths.get_hl(0, { id = hl_id }))
 
     -- Test undercurl
     command('hi NewHighlight gui=undercurl')
-    eq(expected_undercurl, meths.get_hl(0, { id = hl_id }))
+    eq({ fg = 16711680, bg = 16776960, sp = 255, undercurl = true,
+         ctermbg = 10,
+         cterm = {underline = true},
+    }, meths.get_hl(0, { id = hl_id }))
   end)
 
   it('can correctly detect links', function()
diff --git a/test/unit/eval/typval_spec.lua b/test/unit/eval/typval_spec.lua
index 825377813d..ecee773ba8 100644
--- a/test/unit/eval/typval_spec.lua
+++ b/test/unit/eval/typval_spec.lua
@@ -1653,7 +1653,7 @@ describe('typval.c', function()
           eq(OK, lib.tv_dict_add(d, di))
           alloc_log:check({})
           eq(FAIL, check_emsg(function() return lib.tv_dict_add(d, di) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key ""'))
           alloc_log:clear()
           lib.tv_dict_item_remove(d, di)
           alloc_log:check({
@@ -1947,7 +1947,7 @@ describe('typval.c', function()
           alloc_log:check({})
           eq({test=10, ['t-est']=int(42)}, dct2tbl(d))
           eq(FAIL, check_emsg(function() return lib.tv_dict_add(d, di) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "t-est"'))
         end)
       end)
       describe('list()', function()
@@ -1964,7 +1964,7 @@ describe('typval.c', function()
           eq({test=10, tes={1, 2, 3}}, dct2tbl(d))
           eq(2, l.lv_refcount)
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_list(d, 'testt', 3, l) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "tes"'))
           eq(2, l.lv_refcount)
           alloc_log:clear()
           lib.emsg_skip = lib.emsg_skip + 1
@@ -1990,7 +1990,7 @@ describe('typval.c', function()
           eq({test=10, tes={foo=42}}, dct2tbl(d))
           eq(2, d2.dv_refcount)
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_dict(d, 'testt', 3, d2) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "tes"'))
           eq(2, d2.dv_refcount)
           alloc_log:clear()
           lib.emsg_skip = lib.emsg_skip + 1
@@ -2012,7 +2012,7 @@ describe('typval.c', function()
           alloc_log:check({a.di(dis.tes, 'tes')})
           eq({test=10, tes=int(2)}, dct2tbl(d))
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_nr(d, 'testt', 3, 2) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "tes"'))
           alloc_log:clear()
           lib.emsg_skip = lib.emsg_skip + 1
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_nr(d, 'testt', 3, 2) end,
@@ -2032,7 +2032,7 @@ describe('typval.c', function()
           alloc_log:check({a.di(dis.tes, 'tes')})
           eq({test=10, tes=1.5}, dct2tbl(d))
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_float(d, 'testt', 3, 1.5) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "tes"'))
           alloc_log:clear()
           lib.emsg_skip = lib.emsg_skip + 1
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_float(d, 'testt', 3, 1.5) end,
@@ -2055,7 +2055,7 @@ describe('typval.c', function()
           })
           eq({test=10, tes='TEST'}, dct2tbl(d))
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_str(d, 'testt', 3, 'TEST') end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "tes"'))
           alloc_log:clear()
           lib.emsg_skip = lib.emsg_skip + 1
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_str(d, 'testt', 3, 'TEST') end,
@@ -2085,7 +2085,7 @@ describe('typval.c', function()
           })
           eq({test=10, tes='TEST'}, dct2tbl(d))
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_allocated_str(d, 'testt', 3, s2) end,
-                              'E685: Internal error: hash_add()'))
+                              'E685: Internal error: hash_add(): duplicate key "tes"'))
           alloc_log:clear()
           lib.emsg_skip = lib.emsg_skip + 1
           eq(FAIL, check_emsg(function() return lib.tv_dict_add_allocated_str(d, 'testt', 3, s3) end,