Merge pull request #17275 from bfredl/keysethl

refactor(api): use a keyset for highlight dicts

src/nvim/api/keysets.lua

@@ -78,5 +78,32 @@ return {
   option = {
     "scope";
   };
+  highlight = {
+    "bold";
+    "standout";
+    "underline";
+    "undercurl";
+    "italic";
+    "reverse";
+    "default";
+    "global";
+    "cterm";
+    "foreground"; "fg";
+    "background"; "bg";
+    "ctermfg";
+    "ctermbg";
+    "special"; "sp";
+    "link";
+    "fallback";
+    "temp";
+  };
+  highlight_cterm = {
+    "bold";
+    "standout";
+    "underline";
+    "undercurl";
+    "italic";
+    "reverse";
+  };
 }
 

src/nvim/api/vim.c

@@ -140,22 +140,16 @@ Dictionary nvim__get_hl_defs(Integer ns_id, Error *err)
 /// @param[out] err Error details, if any
 ///
 // TODO(bfredl): val should take update vs reset flag
-void nvim_set_hl(Integer ns_id, String name, Dictionary val, Error *err)
+void nvim_set_hl(Integer ns_id, String name, Dict(highlight) *val, Error *err)
   FUNC_API_SINCE(7)
 {
   int hl_id = syn_check_group(name.data, (int)name.size);
   int link_id = -1;
 
-  HlAttrNames *names = NULL;  // Only used when setting global namespace
-  if (ns_id == 0) {
-    names = xmalloc(sizeof(*names));
-    *names = HLATTRNAMES_INIT;
-  }
-  HlAttrs attrs = dict2hlattrs(val, true, &link_id, names, err);
+  HlAttrs attrs = dict2hlattrs(val, true, &link_id, err);
   if (!ERROR_SET(err)) {
-    ns_hl_def((NS)ns_id, hl_id, attrs, link_id, names);
+    ns_hl_def((NS)ns_id, hl_id, attrs, link_id, val);
   }
-  xfree(names);
 }
 
 /// Set active namespace for highlights.

src/nvim/generators/gen_keysets.lua

@@ -27,7 +27,8 @@ local defspipe = io.open(defs_file, 'wb')
 local keysets = require'api.keysets'
 
 local keywords = {
-  register = true,
+  register = true;
+  default = true;
 }
 
 local function sanitize(key)

src/nvim/highlight.c

@@ -144,16 +144,16 @@ int hl_get_syn_attr(int ns_id, int idx, HlAttrs at_en)
   }
 }
 
-void ns_hl_def(NS ns_id, int hl_id, HlAttrs attrs, int link_id, HlAttrNames *names)
+void ns_hl_def(NS ns_id, int hl_id, HlAttrs attrs, int link_id, Dict(highlight) *dict)
 {
   if ((attrs.rgb_ae_attr & HL_DEFAULT)
       && map_has(ColorKey, ColorItem)(&ns_hl, ColorKey(ns_id, hl_id))) {
     return;
   }
   if (ns_id == 0) {
-    assert(names);
+    assert(dict);
     // set in global (':highlight') namespace
-    set_hl_group(hl_id, attrs, names, link_id);
+    set_hl_group(hl_id, attrs, dict, link_id);
     return;
   }
   DecorProvider *p = get_decor_provider(ns_id, true);
@@ -198,24 +198,18 @@ int ns_get_hl(NS ns_id, int hl_id, bool link, bool nodefault)
     int tmp = false;
     HlAttrs attrs = HLATTRS_INIT;
     if (ret.type == kObjectTypeDictionary) {
-      Dictionary dict = ret.data.dictionary;
       fallback = false;
-      attrs = dict2hlattrs(dict, true, &it.link_id, NULL, &err);
-      for (size_t i = 0; i < dict.size; i++) {
-        char *key = dict.items[i].key.data;
-        Object val = dict.items[i].value;
-        bool truthy = api_object_to_bool(val, key, false, &err);
-
-        if (strequal(key, "fallback")) {
-          fallback = truthy;
-        } else if (strequal(key, "temp")) {
-          tmp = truthy;
-        }
-      }
+      Dict(highlight) dict = { 0 };
+      if (api_dict_to_keydict(&dict, KeyDict_highlight_get_field,
+                              ret.data.dictionary, &err)) {
+        attrs = dict2hlattrs(&dict, true, &it.link_id, &err);
+        fallback = api_object_to_bool(dict.fallback, "fallback", true, &err);
+        tmp = api_object_to_bool(dict.fallback, "tmp", false, &err);
         if (it.link_id >= 0) {
           fallback = true;
         }
       }
+    }
 
     it.attr_id = fallback ? -1 : hl_get_syn_attr((int)ns_id, hl_id, attrs);
     it.version = p->hl_valid-tmp;
@@ -802,116 +796,98 @@ Dictionary hlattrs2dict(HlAttrs ae, bool use_rgb)
   return hl;
 }
 
-HlAttrs dict2hlattrs(Dictionary dict, bool use_rgb, int *link_id, HlAttrNames *names, Error *err)
+HlAttrs dict2hlattrs(Dict(highlight) *dict, bool use_rgb, int *link_id, Error *err)
 {
   HlAttrs hlattrs = HLATTRS_INIT;
-
   int32_t fg = -1, bg = -1, ctermfg = -1, ctermbg = -1, sp = -1;
   int16_t mask = 0;
   int16_t cterm_mask = 0;
   bool cterm_mask_provided = false;
 
-  for (size_t i = 0; i < dict.size; i++) {
-    char *key = dict.items[i].key.data;
-    Object val = dict.items[i].value;
-
-    struct {
-      const char *name;
-      int16_t flag;
-    } flags[] = {
-      { "bold", HL_BOLD },
-      { "standout", HL_STANDOUT },
-      { "underline", HL_UNDERLINE },
-      { "undercurl", HL_UNDERCURL },
-      { "italic", HL_ITALIC },
-      { "reverse", HL_INVERSE },
-      { "default", HL_DEFAULT },
-      // { "global", HL_GLOBAL },
-      { NULL, 0 },
-    };
-
-    int j;
-    for (j = 0; flags[j].name; j++) {
-      if (strequal(flags[j].name, key)) {
-        if (api_object_to_bool(val, key, false, err)) {
-          mask = mask | flags[j].flag;
+#define CHECK_FLAG(d, m, name, extra, flag) \
+    if (api_object_to_bool(d->name ## extra, #name, false, err)) { \
+      m = m | flag; \
     }
-        break;
+
+  CHECK_FLAG(dict, mask, bold, , HL_BOLD);
+  CHECK_FLAG(dict, mask, standout, , HL_STANDOUT);
+  CHECK_FLAG(dict, mask, underline, , HL_UNDERLINE);
+  CHECK_FLAG(dict, mask, undercurl, , HL_UNDERCURL);
+  CHECK_FLAG(dict, mask, italic, , HL_ITALIC);
+  CHECK_FLAG(dict, mask, reverse, , HL_INVERSE);
+  CHECK_FLAG(dict, mask, default, _, HL_DEFAULT);
+  CHECK_FLAG(dict, mask, global, , HL_GLOBAL);
+
+  if (HAS_KEY(dict->fg)) {
+    fg = object_to_color(dict->fg, "fg", err);
+  } else if (HAS_KEY(dict->foreground)) {
+    fg = object_to_color(dict->foreground, "foreground", err);
+  }
+  if (ERROR_SET(err)) {
+    return hlattrs;
+  }
+
+  if (HAS_KEY(dict->bg)) {
+    bg = object_to_color(dict->bg, "bg", err);
+  } else if (HAS_KEY(dict->background)) {
+    bg = object_to_color(dict->background, "background", err);
+  }
+  if (ERROR_SET(err)) {
+    return hlattrs;
+  }
+
+  if (HAS_KEY(dict->sp)) {
+    sp = object_to_color(dict->sp, "sp", err);
+  } else if (HAS_KEY(dict->special)) {
+    sp = object_to_color(dict->special, "special", err);
+  }
+  if (ERROR_SET(err)) {
+    return hlattrs;
+  }
+
+  if (HAS_KEY(dict->link)) {
+    if (link_id) {
+      *link_id = object_to_hl_id(dict->link, "link", err);
+      if (ERROR_SET(err)) {
+        return hlattrs;
+      }
+    } else {
+      api_set_error(err, kErrorTypeValidation, "Invalid Key: 'link'");
     }
   }
 
   // Handle cterm attrs
-    if (strequal(key, "cterm") && val.type == kObjectTypeDictionary) {
+  if (dict->cterm.type == kObjectTypeDictionary) {
+    Dict(highlight_cterm) cterm[1] = { 0 };
+    if (!api_dict_to_keydict(cterm, KeyDict_highlight_cterm_get_field,
+                             dict->cterm.data.dictionary, err)) {
+      return hlattrs;
+    }
+
     cterm_mask_provided = true;
-      Dictionary cterm_dict = val.data.dictionary;
-      for (size_t l = 0; l < cterm_dict.size; l++) {
-        char *cterm_dict_key = cterm_dict.items[l].key.data;
-        Object cterm_dict_val = cterm_dict.items[l].value;
-        for (int m = 0; flags[m].name; m++) {
-          if (strequal(flags[m].name, cterm_dict_key)) {
-            if (api_object_to_bool(cterm_dict_val, cterm_dict_key, false,
-                                   err)) {
-              cterm_mask |= flags[m].flag;
-            }
-            break;
-          }
-        }
-      }
-    }
+    CHECK_FLAG(cterm, cterm_mask, bold, , HL_BOLD);
+    CHECK_FLAG(cterm, cterm_mask, standout, , HL_STANDOUT);
+    CHECK_FLAG(cterm, cterm_mask, underline, , HL_UNDERLINE);
+    CHECK_FLAG(cterm, cterm_mask, undercurl, , HL_UNDERCURL);
+    CHECK_FLAG(cterm, cterm_mask, italic, , HL_ITALIC);
+    CHECK_FLAG(cterm, cterm_mask, reverse, , HL_INVERSE);
 
-    struct {
-      const char *name;
-      const char *shortname;
-      int *dest;
-      char **dest_name;
-    } colors[] = {
-      { "foreground", "fg", &fg, names ? &names->fg_name : NULL },
-      { "background", "bg", &bg, names ? &names->bg_name : NULL },
-      { "ctermfg", NULL, &ctermfg, NULL },
-      { "ctermbg", NULL, &ctermbg, NULL },
-      { "special", "sp", &sp, names ? &names->sp_name : NULL },
-      { NULL, NULL, NULL, NULL },
-    };
-
-    int k;
-    for (k = 0; (!flags[j].name) && colors[k].name; k++) {
-      if (strequal(colors[k].name, key) || strequal(colors[k].shortname, key)) {
-        if (val.type == kObjectTypeInteger) {
-          *colors[k].dest = (int)val.data.integer;
-        } else if (val.type == kObjectTypeString) {
-          String str = val.data.string;
-          // TODO(bfredl): be more fancy with "bg", "fg" etc
-          if (str.size) {
-            *colors[k].dest = name_to_color(str.data);
-            if (colors[k].dest_name) {
-              *colors[k].dest_name = str.data;
-            }
-          }
-        } else {
-          api_set_error(err, kErrorTypeValidation,
-                        "'%s' must be string or integer", key);
-        }
-        break;
-      }
-    }
-
-    if (flags[j].name || colors[k].name) {
-      // handled above
-    } else if (link_id && strequal(key, "link")) {
-      if (val.type == kObjectTypeString) {
-        String str = val.data.string;
-        *link_id = syn_check_group(str.data, (int)str.size);
-      } else if (val.type == kObjectTypeInteger) {
-        // TODO(bfredl): validate range?
-        *link_id = (int)val.data.integer;
-      } else {
-        api_set_error(err, kErrorTypeValidation,
-                      "'link' must be string or integer");
-      }
-    }
+  } else if (HAS_KEY(dict->cterm)) {
+    api_set_error(err, kErrorTypeValidation, "'cterm' must be a Dictionary.");
+  }
+#undef CHECK_FLAG
 
+  if (HAS_KEY(dict->ctermfg)) {
+    ctermfg = object_to_color(dict->ctermfg, "ctermfg", err);
     if (ERROR_SET(err)) {
-      return hlattrs;  // error set, caller should not use retval
+      return hlattrs;
+    }
+  }
+
+  if (HAS_KEY(dict->ctermbg)) {
+    ctermbg = object_to_color(dict->ctermbg, "ctermbg", err);
+    if (ERROR_SET(err)) {
+      return hlattrs;
     }
   }
 
@@ -937,6 +913,21 @@ HlAttrs dict2hlattrs(Dictionary dict, bool use_rgb, int *link_id, HlAttrNames *n
 
   return hlattrs;
 }
+
+int object_to_color(Object val, char *key, Error *err)
+{
+  if (val.type == kObjectTypeInteger) {
+    return (int)val.data.integer;
+  } else if (val.type == kObjectTypeString) {
+    String str = val.data.string;
+    // TODO(bfredl): be more fancy with "bg", "fg" etc
+    return str.size ? name_to_color(str.data) : 0;
+  } else {
+    api_set_error(err, kErrorTypeValidation, "'%s' must be string or integer", key);
+    return 0;
+  }
+}
+
 Array hl_inspect(int attr)
 {
   Array ret = ARRAY_DICT_INIT;

src/nvim/highlight_defs.h

@@ -46,18 +46,6 @@ typedef struct attr_entry {
   .hl_blend = -1, \
 }
 
-typedef struct {
-  char *bg_name;
-  char *fg_name;
-  char *sp_name;
-} HlAttrNames;
-
-#define HLATTRNAMES_INIT (HlAttrNames) { \
-  .bg_name = NULL, \
-  .fg_name = NULL, \
-  .sp_name = NULL, \
-}
-
 /// Values for index in highlight_attr[].
 /// When making changes, also update hlf_names below!
 typedef enum {

src/nvim/syntax.c

@@ -6714,7 +6714,7 @@ int lookup_color(const int idx, const bool foreground, TriState *const boldp)
   return color;
 }
 
-void set_hl_group(int id, HlAttrs attrs, HlAttrNames *names, int link_id)
+void set_hl_group(int id, HlAttrs attrs, Dict(highlight) *dict, int link_id)
 {
   int idx = id - 1;  // Index is ID minus one.
 
@@ -6750,19 +6750,19 @@ void set_hl_group(int id, HlAttrs attrs, HlAttrNames *names, int link_id)
   g->sg_rgb_sp = attrs.rgb_sp_color;
 
   struct {
-    char **dest; RgbValue val; char *name;
+    char **dest; RgbValue val; Object name;
   } cattrs[] = {
-    { &g->sg_rgb_fg_name, g->sg_rgb_fg, names->fg_name },
-    { &g->sg_rgb_bg_name, g->sg_rgb_bg, names->bg_name },
-    { &g->sg_rgb_sp_name, g->sg_rgb_sp, names->sp_name },
-    { NULL, -1, NULL },
+    { &g->sg_rgb_fg_name, g->sg_rgb_fg, HAS_KEY(dict->fg) ? dict->fg : dict->foreground },
+    { &g->sg_rgb_bg_name, g->sg_rgb_bg, HAS_KEY(dict->bg) ? dict->bg : dict->background },
+    { &g->sg_rgb_sp_name, g->sg_rgb_sp, HAS_KEY(dict->sp) ? dict->sp : dict->special },
+    { NULL, -1, NIL },
   };
 
   for (int j = 0; cattrs[j].dest; j++) {
     if (cattrs[j].val != -1) {
       xfree(*cattrs[j].dest);
-      if (cattrs[j].name) {
-        *cattrs[j].dest = xstrdup(cattrs[j].name);
+      if (cattrs[j].name.type == kObjectTypeString && cattrs[j].name.data.string.size) {
+        *cattrs[j].dest = xstrdup(cattrs[j].name.data.string.data);
       } else {
         char hex_name[8];
         snprintf(hex_name, sizeof(hex_name), "#%06x", cattrs[j].val);