From ba7277cfb4e2556f246446d06b53f3427f28130f Mon Sep 17 00:00:00 2001
From: Matthieu Coudron <mattator@gmail.com>
Date: Wed, 26 Jul 2017 23:28:26 +0200
Subject: [PATCH 1/5] Adds nvim_get_hl_by_name/by_id

...in order to retrieve highlights.

Added test/functional/api/highlight_spec.lua
HL_NORMAL is not really a good name, since it's more like an empty attribute than the normal's one.
If one pays attention, syn_cterm_attr2entry is never called with attr=0 because it's always special cased before.
I suggest in subsequent PRs we remove the ATTR_OFF and just insert an EMPTY ATTR/RESET_ATTR/UNINITIALIZED for id 0.
---
 src/nvim/api/ui.c                      |  34 +-----
 src/nvim/api/vim.c                     |  37 ++++++
 src/nvim/syntax.c                      |  22 ++--
 src/nvim/tui/tui.c                     |   6 +-
 src/nvim/ugrid.c                       |   4 +-
 src/nvim/ugrid.h                       |   2 -
 src/nvim/ui.c                          | 149 +++++++++++++++++++------
 src/nvim/ui.h                          |   3 +
 test/functional/api/highlight_spec.lua |  54 +++++++++
 9 files changed, 220 insertions(+), 91 deletions(-)
 create mode 100644 test/functional/api/highlight_spec.lua

diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index bbbd5ab2dc..afbee09c1c 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -242,39 +242,7 @@ static void push_call(UI *ui, char *name, Array args)
 static void remote_ui_highlight_set(UI *ui, HlAttrs attrs)
 {
   Array args = ARRAY_DICT_INIT;
-  Dictionary hl = ARRAY_DICT_INIT;
-
-  if (attrs.bold) {
-    PUT(hl, "bold", BOOLEAN_OBJ(true));
-  }
-
-  if (attrs.underline) {
-    PUT(hl, "underline", BOOLEAN_OBJ(true));
-  }
-
-  if (attrs.undercurl) {
-    PUT(hl, "undercurl", BOOLEAN_OBJ(true));
-  }
-
-  if (attrs.italic) {
-    PUT(hl, "italic", BOOLEAN_OBJ(true));
-  }
-
-  if (attrs.reverse) {
-    PUT(hl, "reverse", BOOLEAN_OBJ(true));
-  }
-
-  if (attrs.foreground != -1) {
-    PUT(hl, "foreground", INTEGER_OBJ(attrs.foreground));
-  }
-
-  if (attrs.background != -1) {
-    PUT(hl, "background", INTEGER_OBJ(attrs.background));
-  }
-
-  if (attrs.special != -1) {
-    PUT(hl, "special", INTEGER_OBJ(attrs.special));
-  }
+  Dictionary hl = hlattrs2dict(attrs);
 
   ADD(args, DICTIONARY_OBJ(hl));
   push_call(ui, "highlight_set", args);
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index ab893a4c0f..0459d9235d 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -33,6 +33,7 @@
 #include "nvim/syntax.h"
 #include "nvim/getchar.h"
 #include "nvim/os/input.h"
+#include "nvim/ui.h"
 
 #define LINE_BUFFER_SIZE 4096
 
@@ -55,6 +56,42 @@ void nvim_command(String command, Error *err)
   try_end(err);
 }
 
+/// Retrieves highlight description from its name
+///
+/// @param name Highlight group name
+/// @return a highlight description e.g. {'bold': true, 'bg': 123, 'fg': 42}
+/// @see nvim_get_hl_by_id
+Dictionary nvim_get_hl_by_name(String name, Error *err)
+  FUNC_API_SINCE(3)
+{
+  Dictionary result = ARRAY_DICT_INIT;
+  int id = syn_name2id((const char_u *)name.data);
+
+  if (id == 0) {
+    api_set_error(err, kErrorTypeException, "Invalid highlight name %s",
+                  name.data);
+    return result;
+  }
+  result = nvim_get_hl_by_id(id, err);
+  return result;
+}
+
+/// Retrieves highlight description from its id
+///
+/// @param hl_id highlight id as returned by hlID()
+/// @see nvim_get_hl_by_name
+Dictionary nvim_get_hl_by_id(Integer hl_id, Error *err)
+  FUNC_API_SINCE(3)
+{
+  Dictionary dic = ARRAY_DICT_INIT;
+  if (syn_get_final_id((int)hl_id) == 0) {
+    api_set_error(err, kErrorTypeException, "Invalid highlight id %d", hl_id);
+    return dic;
+  }
+  int attrcode = syn_id2attr((int)hl_id);
+  return get_attr_by_id(attrcode, err);
+}
+
 /// Passes input keys to Nvim.
 /// On VimL error: Does not fail, but updates v:errmsg.
 ///
diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c
index 65c0e2464a..fdb7196fc4 100644
--- a/src/nvim/syntax.c
+++ b/src/nvim/syntax.c
@@ -42,6 +42,7 @@
 #include "nvim/ui.h"
 #include "nvim/os/os.h"
 #include "nvim/os/time.h"
+#include "nvim/api/private/helpers.h"
 
 static bool did_syntax_onoff = false;
 
@@ -6622,7 +6623,6 @@ do_highlight(char_u *line, int forceit, int init) {
     syn_unadd_group();
   } else {
     if (is_normal_group) {
-      HL_TABLE()[idx].sg_attr = 0;
       // Need to update all groups, because they might be using "bg" and/or
       // "fg", which have been changed now.
       highlight_attr_set_all();
@@ -6826,7 +6826,7 @@ int hl_combine_attr(int char_attr, int prim_attr)
     // Copy all attributes from char_aep to the new entry
     new_en = *char_aep;
   } else {
-    memset(&new_en, 0, sizeof(new_en));
+    new_en = (attrentry_T)ATTRENTRY_INIT;
   }
 
   spell_aep = syn_cterm_attr2entry(prim_attr);
@@ -6859,6 +6859,7 @@ int hl_combine_attr(int char_attr, int prim_attr)
 
 /// \note this function does not apply exclusively to cterm attr contrary
 /// to what its name implies
+/// \warn don't call it with attr 0 (i.e., the null attribute)
 attrentry_T *syn_cterm_attr2entry(int attr)
 {
   attr -= ATTR_OFF;
@@ -7103,22 +7104,15 @@ syn_list_header(int did_header, int outlen, int id)
   return newline;
 }
 
-/*
- * Set the attribute numbers for a highlight group.
- * Called after one of the attributes has changed.
- */
-static void 
-set_hl_attr (
-    int idx                    /* index in array */
-)
+/// Set the attribute numbers for a highlight group.
+/// Called after one of the attributes has changed.
+/// @param idx corrected highlight index
+static void
+set_hl_attr(int idx)
 {
   attrentry_T at_en = ATTRENTRY_INIT;
   struct hl_group     *sgp = HL_TABLE() + idx;
 
-  // The "Normal" group doesn't need an attribute number
-  if (sgp->sg_name_u != NULL && STRCMP(sgp->sg_name_u, "NORMAL") == 0) {
-    return;
-  }
 
   at_en.cterm_ae_attr = sgp->sg_cterm;
   at_en.cterm_fg_color = sgp->sg_cterm_fg;
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
index 256772489d..8e0e905bcd 100644
--- a/src/nvim/tui/tui.c
+++ b/src/nvim/tui/tui.c
@@ -288,7 +288,7 @@ static void terminfo_stop(UI *ui)
 static void tui_terminal_start(UI *ui)
 {
   TUIData *data = ui->data;
-  data->print_attrs = EMPTY_ATTRS;
+  data->print_attrs = HLATTRS_INIT;
   ugrid_init(&data->grid);
   terminfo_start(ui);
   update_size(ui);
@@ -628,7 +628,7 @@ static void clear_region(UI *ui, int top, int bot, int left, int right)
   if (grid->bg == -1 && right == ui->width -1) {
     // Background is set to the default color and the right edge matches the
     // screen end, try to use terminal codes for clearing the requested area.
-    HlAttrs clear_attrs = EMPTY_ATTRS;
+    HlAttrs clear_attrs = HLATTRS_INIT;
     clear_attrs.foreground = grid->fg;
     clear_attrs.background = grid->bg;
     update_attrs(ui, clear_attrs);
@@ -926,7 +926,7 @@ static void tui_scroll(UI *ui, Integer count)
     cursor_goto(ui, grid->top, grid->left);
     // also set default color attributes or some terminals can become funny
     if (scroll_clears_to_current_colour) {
-      HlAttrs clear_attrs = EMPTY_ATTRS;
+      HlAttrs clear_attrs = HLATTRS_INIT;
       clear_attrs.foreground = grid->fg;
       clear_attrs.background = grid->bg;
       update_attrs(ui, clear_attrs);
diff --git a/src/nvim/ugrid.c b/src/nvim/ugrid.c
index 7a0a16687e..2b5e96ee60 100644
--- a/src/nvim/ugrid.c
+++ b/src/nvim/ugrid.c
@@ -16,7 +16,7 @@
 
 void ugrid_init(UGrid *grid)
 {
-  grid->attrs = EMPTY_ATTRS;
+  grid->attrs = HLATTRS_INIT;
   grid->fg = grid->bg = -1;
   grid->cells = NULL;
 }
@@ -118,7 +118,7 @@ UCell *ugrid_put(UGrid *grid, uint8_t *text, size_t size)
 
 static void clear_region(UGrid *grid, int top, int bot, int left, int right)
 {
-  HlAttrs clear_attrs = EMPTY_ATTRS;
+  HlAttrs clear_attrs = HLATTRS_INIT;
   clear_attrs.foreground = grid->fg;
   clear_attrs.background = grid->bg;
   UGRID_FOREACH_CELL(grid, top, bot, left, right, {
diff --git a/src/nvim/ugrid.h b/src/nvim/ugrid.h
index 268362bf1b..1cf047502d 100644
--- a/src/nvim/ugrid.h
+++ b/src/nvim/ugrid.h
@@ -21,8 +21,6 @@ struct ugrid {
   UCell **cells;
 };
 
-#define EMPTY_ATTRS ((HlAttrs){ false, false, false, false, false, -1, -1, -1 })
-
 #define UGRID_FOREACH_CELL(grid, top, bot, left, right, code) \
   do { \
     for (int row = top; row <= bot; row++) { \
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
index 01d3604159..184ae56cf2 100644
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -166,6 +166,115 @@ void ui_event(char *name, Array args)
   }
 }
 
+/// Retrieves attribute description from its id
+///
+/// @param attr_id attribute id
+Dictionary get_attr_by_id(Integer attr_id, Error *err)
+{
+  HlAttrs attrs = HLATTRS_INIT;
+  Dictionary dic = ARRAY_DICT_INIT;
+
+  if (attr_id == 0) {
+    goto end;
+  }
+
+  attrentry_T *aep = syn_cterm_attr2entry((int)attr_id);
+  if (!aep) {
+    api_set_error(err, kErrorTypeException,
+                  "Invalid attribute id %d", attr_id);
+    return dic;
+  }
+
+  attrs = attrentry2hlattrs(aep, p_tgc);
+
+end:
+  return hlattrs2dict(attrs);
+}
+
+
+/// Converts an attrentry_T into an HlAttrs
+///
+/// @param[in] aep data to convert
+/// @param use_rgb use 'gui*' settings if true, else resorts to 'cterm*'
+HlAttrs attrentry2hlattrs(const attrentry_T *aep, bool use_rgb)
+{
+  assert(aep);
+
+  HlAttrs attrs = HLATTRS_INIT;
+  int mask = 0;
+
+  mask = use_rgb ? aep->rgb_ae_attr : aep->cterm_ae_attr;
+
+  attrs.bold = mask & HL_BOLD;
+  attrs.underline = mask & HL_UNDERLINE;
+  attrs.undercurl = mask & HL_UNDERCURL;
+  attrs.italic = mask & HL_ITALIC;
+  attrs.reverse = mask & (HL_INVERSE | HL_STANDOUT);
+
+  if (use_rgb) {
+    if (aep->rgb_fg_color != -1) {
+      attrs.foreground = aep->rgb_fg_color;
+    }
+
+    if (aep->rgb_bg_color != -1) {
+      attrs.background = aep->rgb_bg_color;
+    }
+
+    if (aep->rgb_sp_color != -1) {
+      attrs.special = aep->rgb_sp_color;
+    }
+  } else {
+    if (cterm_normal_fg_color != aep->cterm_fg_color) {
+      attrs.foreground = aep->cterm_fg_color - 1;
+    }
+
+    if (cterm_normal_bg_color != aep->cterm_bg_color) {
+        attrs.background = aep->cterm_bg_color - 1;
+    }
+  }
+
+  return attrs;
+}
+
+Dictionary hlattrs2dict(HlAttrs attrs)
+{
+  Dictionary hl = ARRAY_DICT_INIT;
+
+  if (attrs.bold) {
+    PUT(hl, "bold", BOOLEAN_OBJ(true));
+  }
+
+  if (attrs.underline) {
+    PUT(hl, "underline", BOOLEAN_OBJ(true));
+  }
+
+  if (attrs.undercurl) {
+    PUT(hl, "undercurl", BOOLEAN_OBJ(true));
+  }
+
+  if (attrs.italic) {
+    PUT(hl, "italic", BOOLEAN_OBJ(true));
+  }
+
+  if (attrs.reverse) {
+    PUT(hl, "reverse", BOOLEAN_OBJ(true));
+  }
+
+  if (attrs.foreground != -1) {
+    PUT(hl, "foreground", INTEGER_OBJ(attrs.foreground));
+  }
+
+  if (attrs.background != -1) {
+    PUT(hl, "background", INTEGER_OBJ(attrs.background));
+  }
+
+  if (attrs.special != -1) {
+    PUT(hl, "special", INTEGER_OBJ(attrs.special));
+  }
+
+  return hl;
+}
+
 void ui_refresh(void)
 {
   if (!ui_active()) {
@@ -405,54 +514,20 @@ void ui_flush(void)
 
 static void set_highlight_args(int attr_code)
 {
-  HlAttrs rgb_attrs = { false, false, false, false, false, -1, -1, -1 };
+  HlAttrs rgb_attrs = HLATTRS_INIT;
   HlAttrs cterm_attrs = rgb_attrs;
 
   if (attr_code == HL_NORMAL) {
     goto end;
   }
-
-  int rgb_mask = 0;
-  int cterm_mask = 0;
   attrentry_T *aep = syn_cterm_attr2entry(attr_code);
 
   if (!aep) {
     goto end;
   }
 
-  rgb_mask = aep->rgb_ae_attr;
-  cterm_mask = aep->cterm_ae_attr;
-
-  rgb_attrs.bold = rgb_mask & HL_BOLD;
-  rgb_attrs.underline = rgb_mask & HL_UNDERLINE;
-  rgb_attrs.undercurl = rgb_mask & HL_UNDERCURL;
-  rgb_attrs.italic = rgb_mask & HL_ITALIC;
-  rgb_attrs.reverse = rgb_mask & (HL_INVERSE | HL_STANDOUT);
-  cterm_attrs.bold = cterm_mask & HL_BOLD;
-  cterm_attrs.underline = cterm_mask & HL_UNDERLINE;
-  cterm_attrs.undercurl = cterm_mask & HL_UNDERCURL;
-  cterm_attrs.italic = cterm_mask & HL_ITALIC;
-  cterm_attrs.reverse = cterm_mask & (HL_INVERSE | HL_STANDOUT);
-
-  if (aep->rgb_fg_color != normal_fg) {
-    rgb_attrs.foreground = aep->rgb_fg_color;
-  }
-
-  if (aep->rgb_bg_color != normal_bg) {
-    rgb_attrs.background = aep->rgb_bg_color;
-  }
-
-  if (aep->rgb_sp_color != normal_sp) {
-    rgb_attrs.special = aep->rgb_sp_color;
-  }
-
-  if (cterm_normal_fg_color != aep->cterm_fg_color) {
-    cterm_attrs.foreground = aep->cterm_fg_color - 1;
-  }
-
-  if (cterm_normal_bg_color != aep->cterm_bg_color) {
-    cterm_attrs.background = aep->cterm_bg_color - 1;
-  }
+  rgb_attrs = attrentry2hlattrs(aep, true);
+  cterm_attrs = attrentry2hlattrs(aep, false);
 
 end:
   UI_CALL(highlight_set, (ui->rgb ? rgb_attrs : cterm_attrs));
diff --git a/src/nvim/ui.h b/src/nvim/ui.h
index 064f77fee1..f1ea0716e6 100644
--- a/src/nvim/ui.h
+++ b/src/nvim/ui.h
@@ -21,6 +21,9 @@ typedef struct {
   int foreground, background, special;
 } HlAttrs;
 
+#define HLATTRS_INIT \
+  ((HlAttrs){ false, false, false, false, false, -1, -1, -1 })
+
 typedef struct ui_t UI;
 
 struct ui_t {
diff --git a/test/functional/api/highlight_spec.lua b/test/functional/api/highlight_spec.lua
new file mode 100644
index 0000000000..1aacfaed03
--- /dev/null
+++ b/test/functional/api/highlight_spec.lua
@@ -0,0 +1,54 @@
+
+local helpers = require('test.functional.helpers')(after_each)
+local clear, nvim = helpers.clear, helpers.nvim
+local Screen = require('test.functional.ui.screen')
+local eq, eval = helpers.eq, helpers.eval
+local command = helpers.command
+local ok = helpers.ok
+local meths = helpers.meths
+
+
+describe('highlight api', function()
+
+  before_each(function()
+    clear('--cmd', 'set termguicolors')
+  end)
+
+  it("nvim_get_hl_by_id", function()
+    local expected_hl = { background = Screen.colors.Yellow,
+                          foreground = Screen.colors.Red,
+                          special = Screen.colors.Blue
+                        }
+
+    command('hi NewHighlight guifg=red guibg=yellow guisp=blue')
+
+    local hl_id = eval("hlID('NewHighlight')")
+    eq(expected_hl, nvim("get_hl_by_id", hl_id))
+
+    -- assume there is no hl with 30000
+    local err, emsg = pcall(meths.get_hl_by_id, 30000)
+    eq(false, err)
+    ok(string.find(emsg, 'Invalid highlight id') ~= nil)
+  end)
+
+  it("nvim_get_hl_by_name", function()
+    local expected_hl = { background = Screen.colors.Yellow,
+                          foreground = Screen.colors.Red }
+
+    -- test "Normal" hl defaults
+    eq({}, nvim("get_hl_by_name", 'Normal'))
+
+    command('hi NewHighlight guifg=red guibg=yellow')
+    eq(expected_hl, nvim("get_hl_by_name", 'NewHighlight'))
+
+    command('hi Normal guifg=red guibg=yellow')
+    eq(expected_hl, nvim("get_hl_by_name", 'Normal'))
+    local err, emsg = pcall(meths.get_hl_by_name , 'unknown_highlight')
+    eq(false, err)
+    ok(string.find(emsg, 'Invalid highlight name') ~= nil)
+  end)
+
+
+
+end)
+

From e3a2cca3878f44252eccdc1918cc8854145de860 Mon Sep 17 00:00:00 2001
From: Matthieu Coudron <mattator@gmail.com>
Date: Sun, 27 Aug 2017 00:33:36 +0200
Subject: [PATCH 2/5] Increased test coverage for RGB and cterm

---
 src/nvim/api/vim.c                     |  4 +--
 src/nvim/syntax.c                      | 26 +++++++++++++++
 src/nvim/ui.c                          | 25 ---------------
 test/functional/api/highlight_spec.lua | 44 +++++++++++++++-----------
 4 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 0459d9235d..bf3e4bc6a0 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -78,7 +78,7 @@ Dictionary nvim_get_hl_by_name(String name, Error *err)
 
 /// Retrieves highlight description from its id
 ///
-/// @param hl_id highlight id as returned by hlID()
+/// @param hl_id highlight id as returned by |hlID()|
 /// @see nvim_get_hl_by_name
 Dictionary nvim_get_hl_by_id(Integer hl_id, Error *err)
   FUNC_API_SINCE(3)
@@ -89,7 +89,7 @@ Dictionary nvim_get_hl_by_id(Integer hl_id, Error *err)
     return dic;
   }
   int attrcode = syn_id2attr((int)hl_id);
-  return get_attr_by_id(attrcode, err);
+  return hl_get_attr_by_id(attrcode, err);
 }
 
 /// Passes input keys to Nvim.
diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c
index fdb7196fc4..2f48cc8757 100644
--- a/src/nvim/syntax.c
+++ b/src/nvim/syntax.c
@@ -8221,6 +8221,32 @@ RgbValue name_to_color(const uint8_t *name)
   return -1;
 }
 
+/// Retrieves attribute description from its id
+///
+/// @param attr_id attribute id
+Dictionary hl_get_attr_by_id(Integer attr_id, Error *err)
+{
+  HlAttrs attrs = HLATTRS_INIT;
+  Dictionary dic = ARRAY_DICT_INIT;
+
+  if (attr_id == 0) {
+    goto end;
+  }
+
+  attrentry_T *aep = syn_cterm_attr2entry((int)attr_id);
+  if (!aep) {
+    api_set_error(err, kErrorTypeException,
+                  "Invalid attribute id %d", attr_id);
+    return dic;
+  }
+
+  attrs = attrentry2hlattrs(aep, p_tgc);
+
+end:
+  return hlattrs2dict(attrs);
+}
+
+
 /**************************************
 *  End of Highlighting stuff	      *
 **************************************/
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
index 184ae56cf2..afe7a51d43 100644
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -166,31 +166,6 @@ void ui_event(char *name, Array args)
   }
 }
 
-/// Retrieves attribute description from its id
-///
-/// @param attr_id attribute id
-Dictionary get_attr_by_id(Integer attr_id, Error *err)
-{
-  HlAttrs attrs = HLATTRS_INIT;
-  Dictionary dic = ARRAY_DICT_INIT;
-
-  if (attr_id == 0) {
-    goto end;
-  }
-
-  attrentry_T *aep = syn_cterm_attr2entry((int)attr_id);
-  if (!aep) {
-    api_set_error(err, kErrorTypeException,
-                  "Invalid attribute id %d", attr_id);
-    return dic;
-  }
-
-  attrs = attrentry2hlattrs(aep, p_tgc);
-
-end:
-  return hlattrs2dict(attrs);
-}
-
 
 /// Converts an attrentry_T into an HlAttrs
 ///
diff --git a/test/functional/api/highlight_spec.lua b/test/functional/api/highlight_spec.lua
index 1aacfaed03..a5d3871d13 100644
--- a/test/functional/api/highlight_spec.lua
+++ b/test/functional/api/highlight_spec.lua
@@ -8,47 +8,53 @@ local ok = helpers.ok
 local meths = helpers.meths
 
 
-describe('highlight api', function()
+describe('highlight api',function()
+  local expected_rgb = { background = Screen.colors.Yellow,
+                          foreground = Screen.colors.Red,
+                          special = Screen.colors.Blue,
+                          bold = true,
+                        }
+
+  local expected_cterm = { background = 10,
+                          underline = true,
+                        }
 
   before_each(function()
-    clear('--cmd', 'set termguicolors')
+    clear()
+    command("hi NewHighlight cterm=underline ctermbg=green guifg=red guibg=yellow guisp=blue gui=bold")
   end)
 
   it("nvim_get_hl_by_id", function()
-    local expected_hl = { background = Screen.colors.Yellow,
-                          foreground = Screen.colors.Red,
-                          special = Screen.colors.Blue
-                        }
-
-    command('hi NewHighlight guifg=red guibg=yellow guisp=blue')
-
     local hl_id = eval("hlID('NewHighlight')")
-    eq(expected_hl, nvim("get_hl_by_id", hl_id))
 
-    -- assume there is no hl with 30000
+    eq(expected_cterm, nvim("get_hl_by_id", hl_id))
+
+    command('set termguicolors')
+    hl_id = eval("hlID('NewHighlight')")
+    eq(expected_rgb, nvim("get_hl_by_id", hl_id))
+
+    -- assume there is no hl with id 30000
     local err, emsg = pcall(meths.get_hl_by_id, 30000)
     eq(false, err)
     ok(string.find(emsg, 'Invalid highlight id') ~= nil)
   end)
 
   it("nvim_get_hl_by_name", function()
-    local expected_hl = { background = Screen.colors.Yellow,
+    local expected_normal = { background = Screen.colors.Yellow,
                           foreground = Screen.colors.Red }
 
     -- test "Normal" hl defaults
     eq({}, nvim("get_hl_by_name", 'Normal'))
 
-    command('hi NewHighlight guifg=red guibg=yellow')
-    eq(expected_hl, nvim("get_hl_by_name", 'NewHighlight'))
+    eq(expected_cterm, nvim("get_hl_by_name", 'NewHighlight'))
+    command('set termguicolors')
+    eq(expected_rgb, nvim("get_hl_by_name", 'NewHighlight'))
 
     command('hi Normal guifg=red guibg=yellow')
-    eq(expected_hl, nvim("get_hl_by_name", 'Normal'))
+    eq(expected_normal, nvim("get_hl_by_name", 'Normal'))
+
     local err, emsg = pcall(meths.get_hl_by_name , 'unknown_highlight')
     eq(false, err)
     ok(string.find(emsg, 'Invalid highlight name') ~= nil)
   end)
-
-
-
 end)
-

From 3a006486397d611234abd9b429bce0b44d6b7747 Mon Sep 17 00:00:00 2001
From: Matthieu Coudron <mattator@gmail.com>
Date: Sun, 3 Sep 2017 05:25:57 +0200
Subject: [PATCH 3/5] Changed prototypes to accept a boolean "rgb"

---
 src/nvim/api/vim.c                     | 10 ++++++----
 src/nvim/syntax.c                      |  4 ++--
 test/functional/api/highlight_spec.lua | 20 +++++++++-----------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index bf3e4bc6a0..0c3c497533 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -59,9 +59,10 @@ void nvim_command(String command, Error *err)
 /// Retrieves highlight description from its name
 ///
 /// @param name Highlight group name
+/// @param rgb True to export GUI values
 /// @return a highlight description e.g. {'bold': true, 'bg': 123, 'fg': 42}
 /// @see nvim_get_hl_by_id
-Dictionary nvim_get_hl_by_name(String name, Error *err)
+Dictionary nvim_get_hl_by_name(String name, Boolean rgb, Error *err)
   FUNC_API_SINCE(3)
 {
   Dictionary result = ARRAY_DICT_INIT;
@@ -72,15 +73,16 @@ Dictionary nvim_get_hl_by_name(String name, Error *err)
                   name.data);
     return result;
   }
-  result = nvim_get_hl_by_id(id, err);
+  result = nvim_get_hl_by_id(id, rgb, err);
   return result;
 }
 
 /// Retrieves highlight description from its id
 ///
 /// @param hl_id highlight id as returned by |hlID()|
+/// @param rgb True to export GUI values
 /// @see nvim_get_hl_by_name
-Dictionary nvim_get_hl_by_id(Integer hl_id, Error *err)
+Dictionary nvim_get_hl_by_id(Integer hl_id, Boolean rgb, Error *err)
   FUNC_API_SINCE(3)
 {
   Dictionary dic = ARRAY_DICT_INIT;
@@ -89,7 +91,7 @@ Dictionary nvim_get_hl_by_id(Integer hl_id, Error *err)
     return dic;
   }
   int attrcode = syn_id2attr((int)hl_id);
-  return hl_get_attr_by_id(attrcode, err);
+  return hl_get_attr_by_id(attrcode, rgb, err);
 }
 
 /// Passes input keys to Nvim.
diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c
index 2f48cc8757..32567a63de 100644
--- a/src/nvim/syntax.c
+++ b/src/nvim/syntax.c
@@ -8224,7 +8224,7 @@ RgbValue name_to_color(const uint8_t *name)
 /// Retrieves attribute description from its id
 ///
 /// @param attr_id attribute id
-Dictionary hl_get_attr_by_id(Integer attr_id, Error *err)
+Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Error *err)
 {
   HlAttrs attrs = HLATTRS_INIT;
   Dictionary dic = ARRAY_DICT_INIT;
@@ -8240,7 +8240,7 @@ Dictionary hl_get_attr_by_id(Integer attr_id, Error *err)
     return dic;
   }
 
-  attrs = attrentry2hlattrs(aep, p_tgc);
+  attrs = attrentry2hlattrs(aep, rgb);
 
 end:
   return hlattrs2dict(attrs);
diff --git a/test/functional/api/highlight_spec.lua b/test/functional/api/highlight_spec.lua
index a5d3871d13..4082b0daf9 100644
--- a/test/functional/api/highlight_spec.lua
+++ b/test/functional/api/highlight_spec.lua
@@ -27,14 +27,13 @@ describe('highlight api',function()
   it("nvim_get_hl_by_id", function()
     local hl_id = eval("hlID('NewHighlight')")
 
-    eq(expected_cterm, nvim("get_hl_by_id", hl_id))
+    eq(expected_cterm, nvim("get_hl_by_id", hl_id, false))
 
-    command('set termguicolors')
     hl_id = eval("hlID('NewHighlight')")
-    eq(expected_rgb, nvim("get_hl_by_id", hl_id))
+    eq(expected_rgb, nvim("get_hl_by_id", hl_id, true))
 
-    -- assume there is no hl with id 30000
-    local err, emsg = pcall(meths.get_hl_by_id, 30000)
+    -- assume there is no hl with id = 30000
+    local err, emsg = pcall(meths.get_hl_by_id, 30000, false)
     eq(false, err)
     ok(string.find(emsg, 'Invalid highlight id') ~= nil)
   end)
@@ -44,16 +43,15 @@ describe('highlight api',function()
                           foreground = Screen.colors.Red }
 
     -- test "Normal" hl defaults
-    eq({}, nvim("get_hl_by_name", 'Normal'))
+    eq({}, nvim("get_hl_by_name", 'Normal', true))
 
-    eq(expected_cterm, nvim("get_hl_by_name", 'NewHighlight'))
-    command('set termguicolors')
-    eq(expected_rgb, nvim("get_hl_by_name", 'NewHighlight'))
+    eq(expected_cterm, nvim("get_hl_by_name", 'NewHighlight', false))
+    eq(expected_rgb, nvim("get_hl_by_name", 'NewHighlight', true))
 
     command('hi Normal guifg=red guibg=yellow')
-    eq(expected_normal, nvim("get_hl_by_name", 'Normal'))
+    eq(expected_normal, nvim("get_hl_by_name", 'Normal', true))
 
-    local err, emsg = pcall(meths.get_hl_by_name , 'unknown_highlight')
+    local err, emsg = pcall(meths.get_hl_by_name , 'unknown_highlight', false)
     eq(false, err)
     ok(string.find(emsg, 'Invalid highlight name') ~= nil)
   end)

From 481e40cc8ca061f5c3a68f56f30ee96e9086da4d Mon Sep 17 00:00:00 2001
From: Matthieu Coudron <mattator@gmail.com>
Date: Sat, 30 Sep 2017 11:44:50 +0900
Subject: [PATCH 4/5] Remove duplicate ATTRENTRY_INIT

---
 src/nvim/syntax.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c
index 32567a63de..933baad901 100644
--- a/src/nvim/syntax.c
+++ b/src/nvim/syntax.c
@@ -6825,8 +6825,6 @@ int hl_combine_attr(int char_attr, int prim_attr)
   if (char_aep != NULL) {
     // Copy all attributes from char_aep to the new entry
     new_en = *char_aep;
-  } else {
-    new_en = (attrentry_T)ATTRENTRY_INIT;
   }
 
   spell_aep = syn_cterm_attr2entry(prim_attr);

From 52517321d1859c31fef14aa75d784615693fcecb Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Sun, 8 Oct 2017 20:23:11 +0200
Subject: [PATCH 5/5] test: nvim_get_hl_by_name/by_id #7082

- test all properties
- test failure modes
---
 src/nvim/api/vim.c                     |  1 -
 src/nvim/syntax.c                      |  9 +--
 test/functional/api/highlight_spec.lua | 77 +++++++++++++++++++++-----
 3 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index bc89ffefe6..98f4410347 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -33,7 +33,6 @@
 #include "nvim/syntax.h"
 #include "nvim/getchar.h"
 #include "nvim/os/input.h"
-#include "nvim/ui.h"
 
 #define LINE_BUFFER_SIZE 4096
 
diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c
index f59495f660..0224b28c2a 100644
--- a/src/nvim/syntax.c
+++ b/src/nvim/syntax.c
@@ -7127,8 +7127,7 @@ syn_list_header(int did_header, int outlen, int id)
 /// Set the attribute numbers for a highlight group.
 /// Called after one of the attributes has changed.
 /// @param idx corrected highlight index
-static void
-set_hl_attr(int idx)
+static void set_hl_attr(int idx)
 {
   attrentry_T at_en = ATTRENTRY_INIT;
   struct hl_group     *sgp = HL_TABLE() + idx;
@@ -8241,9 +8240,7 @@ RgbValue name_to_color(const uint8_t *name)
   return -1;
 }
 
-/// Retrieves attribute description from its id
-///
-/// @param attr_id attribute id
+/// Gets highlight description for id `attr_id` as a map.
 Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Error *err)
 {
   HlAttrs attrs = HLATTRS_INIT;
@@ -8256,7 +8253,7 @@ Dictionary hl_get_attr_by_id(Integer attr_id, Boolean rgb, Error *err)
   attrentry_T *aep = syn_cterm_attr2entry((int)attr_id);
   if (!aep) {
     api_set_error(err, kErrorTypeException,
-                  "Invalid attribute id %d", attr_id);
+                  "Invalid attribute id: %d", attr_id);
     return dic;
   }
 
diff --git a/test/functional/api/highlight_spec.lua b/test/functional/api/highlight_spec.lua
index c482128a31..2297a0760f 100644
--- a/test/functional/api/highlight_spec.lua
+++ b/test/functional/api/highlight_spec.lua
@@ -6,15 +6,26 @@ local command = helpers.command
 local meths = helpers.meths
 
 describe('highlight api',function()
-  local expected_rgb = { background = Screen.colors.Yellow,
-                         foreground = Screen.colors.Red,
-                         special = Screen.colors.Blue,
-                         bold = true,
-                       }
-
-  local expected_cterm = { background = 10,
-                           underline = true,
-                         }
+  local expected_rgb = {
+    background = Screen.colors.Yellow,
+    foreground = Screen.colors.Red,
+    special = Screen.colors.Blue,
+    bold = true,
+  }
+  local expected_cterm = {
+    background = 10,
+    underline = true,
+  }
+  local expected_rgb2 = {
+    background = Screen.colors.Yellow,
+    foreground = Screen.colors.Red,
+    special = Screen.colors.Blue,
+    bold = true,
+    italic = true,
+    reverse = true,
+    undercurl = true,
+    underline = true,
+  }
 
   before_each(function()
     clear()
@@ -26,31 +37,67 @@ describe('highlight api',function()
     eq(expected_cterm, nvim("get_hl_by_id", hl_id, false))
 
     hl_id = eval("hlID('NewHighlight')")
+    -- Test valid id.
     eq(expected_rgb, nvim("get_hl_by_id", hl_id, true))
 
-    local expected_error = 'Invalid highlight id'
-    -- Assume there is no hl id 30000.
+    -- Test invalid id.
     local err, emsg = pcall(meths.get_hl_by_id, 30000, false)
     eq(false, err)
-    eq(expected_error, string.match(emsg, expected_error))
+    eq('Invalid highlight id: 30000', string.match(emsg, 'Invalid.*'))
+
+    -- Test all highlight properties.
+    command('hi NewHighlight gui=underline,bold,undercurl,italic,reverse')
+    eq(expected_rgb2, nvim("get_hl_by_id", hl_id, true))
+
+    -- Test nil argument.
+    err, emsg = pcall(meths.get_hl_by_id, { nil }, false)
+    eq(false, err)
+    eq('Wrong type for argument 1, expecting Integer',
+       string.match(emsg, 'Wrong.*'))
+
+    -- Test 0 argument.
+    err, emsg = pcall(meths.get_hl_by_id, 0, false)
+    eq(false, err)
+    eq('Invalid highlight id: 0',
+       string.match(emsg, 'Invalid.*'))
+
+    -- Test -1 argument.
+    err, emsg = pcall(meths.get_hl_by_id, -1, false)
+    eq(false, err)
+    eq('Invalid highlight id: -1',
+       string.match(emsg, 'Invalid.*'))
   end)
 
   it("nvim_get_hl_by_name", function()
     local expected_normal = { background = Screen.colors.Yellow,
                               foreground = Screen.colors.Red }
 
-    -- Test "Normal" hl defaults.
+    -- Test `Normal` default values.
     eq({}, nvim("get_hl_by_name", 'Normal', true))
 
     eq(expected_cterm, nvim("get_hl_by_name", 'NewHighlight', false))
     eq(expected_rgb, nvim("get_hl_by_name", 'NewHighlight', true))
 
+    -- Test `Normal` modified values.
     command('hi Normal guifg=red guibg=yellow')
     eq(expected_normal, nvim("get_hl_by_name", 'Normal', true))
 
-    local expected_error = 'Invalid highlight name'
+    -- Test invalid name.
     local err, emsg = pcall(meths.get_hl_by_name , 'unknown_highlight', false)
     eq(false, err)
-    eq(expected_error, string.match(emsg, expected_error))
+    eq('Invalid highlight name: unknown_highlight',
+       string.match(emsg, 'Invalid.*'))
+
+    -- Test nil argument.
+    err, emsg = pcall(meths.get_hl_by_name , { nil }, false)
+    eq(false, err)
+    eq('Wrong type for argument 1, expecting String',
+       string.match(emsg, 'Wrong.*'))
+
+    -- Test empty string argument.
+    err, emsg = pcall(meths.get_hl_by_name , '', false)
+    eq(false, err)
+    eq('Invalid highlight name: ',
+       string.match(emsg, 'Invalid.*'))
   end)
 end)