Merge PR #1883 'More work on highlight tests + update documentation'

src/nvim/api/vim.c

@@ -552,6 +552,18 @@ Integer vim_name_to_color(String name)
   return name_to_color((uint8_t *)name.data);
 }
 
+Dictionary vim_get_color_map(void)
+{
+  Dictionary colors = ARRAY_DICT_INIT;
+
+  for (int i = 0; color_name_table[i].name != NULL; i++) {
+    PUT(colors, color_name_table[i].name,
+        INTEGER_OBJ(color_name_table[i].color));
+  }
+  return colors;
+}
+
+
 Array vim_get_api_info(uint64_t channel_id)
 {
   Array rv = ARRAY_DICT_INIT;

src/nvim/syntax.c

@@ -7645,14 +7645,8 @@ char_u *get_highlight_name(expand_T *xp, int idx)
   return HL_TABLE()[idx].sg_name;
 }
 
-
-RgbValue name_to_color(uint8_t *name)
-{
 #define RGB(r, g, b) ((r << 16) | (g << 8) | b)
-  static struct {
+color_name_table_T color_name_table[] = {
-    char *name;
-    RgbValue color;
-  } color_name_table[] = {
   // Color names taken from
   // http://www.rapidtables.com/web/color/RGB_Color.htm
   {"Maroon", RGB(0x80, 0x00, 0x00)},
@@ -7824,6 +7818,9 @@ RgbValue name_to_color(uint8_t *name)
   {NULL, 0},
 };
 
+RgbValue name_to_color(uint8_t *name)
+{
+
   if (name[0] == '#' && isxdigit(name[1]) && isxdigit(name[2])
       && isxdigit(name[3]) && isxdigit(name[4]) && isxdigit(name[5])
       && isxdigit(name[6]) && name[7] == NUL) {

src/nvim/syntax.h

@@ -38,6 +38,12 @@
 #define HL_CONCEAL     0x20000 /* can be concealed */
 #define HL_CONCEALENDS 0x40000 /* can be concealed */
 
+typedef struct {
+  char *name;
+  RgbValue color;
+} color_name_table_T;
+extern color_name_table_T color_name_table[];
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "syntax.h.generated.h"
 #endif

test/functional/ui/highlight_spec.lua

@@ -29,19 +29,19 @@ end)
 describe('Default highlight groups', function()
   -- Test the default attributes for highlight groups shown by the :highlight
   -- command
-  local screen, hlgroup_colors
+  local screen
 
-  setup(function()
+  local hlgroup_colors = {
-    hlgroup_colors = {
+    NonText = Screen.colors.Blue,
-      NonText = nvim('name_to_color', 'Blue'),
+    Question = Screen.colors.SeaGreen
-      Question = nvim('name_to_color', 'SeaGreen')
   }
-  end)
 
   before_each(function()
     clear()
     screen = Screen.new()
     screen:attach()
+    --ignore highligting of ~-lines
+    screen:set_default_attr_ignore( {{bold=true, foreground=hlgroup_colors.NonText}} )
   end)
 
   after_each(function()
@@ -52,8 +52,6 @@ describe('Default highlight groups', function()
       [1] = {reverse = true, bold = true},  -- StatusLine
       [2] = {reverse = true}                -- StatusLineNC
     })
-    --ignore highligting of ~-lines
-    screen:set_default_attr_ignore( {{}, {bold=true, foreground=hlgroup_colors.NonText}} )
     execute('sp', 'vsp', 'vsp')
     screen:expect([[
       ^                   {2:|}                {2:|}               |

test/functional/ui/mouse_spec.lua

@@ -4,13 +4,12 @@ local clear, feed, nvim = helpers.clear, helpers.feed, helpers.nvim
 local insert, execute = helpers.insert, helpers.execute
 
 describe('Mouse input', function()
-  local screen, hlgroup_colors
+  local screen
 
-  setup(function()
+  local hlgroup_colors = {
-    hlgroup_colors = {
+    NonText = Screen.colors.Blue,
-      Visual = nvim('name_to_color', 'LightGrey'),
+    Visual = Screen.colors.LightGrey
   }
-  end)
 
   before_each(function()
     clear()
@@ -21,8 +20,10 @@ describe('Mouse input', function()
     screen = Screen.new(25, 5)
     screen:attach()
     screen:set_default_attr_ids({
-      [1] = {background = hlgroup_colors.Visual}
+      [1] = {background = hlgroup_colors.Visual},
+      [2] = {bold = true}
     })
+    screen:set_default_attr_ignore( {{bold=true, foreground=hlgroup_colors.NonText}} )
     feed('itesting<cr>mouse<cr>support and selection<esc>')
     screen:expect([[
       testing                  |
@@ -72,7 +73,7 @@ describe('Mouse input', function()
       mo{1:us}^                    |
       support and selection    |
       ~                        |
-      -- VISUAL --             |
+      {2:-- VISUAL --}             |
     ]])
     feed('<LeftDrag><2,2>')
     screen:expect([[
@@ -80,7 +81,7 @@ describe('Mouse input', function()
       mo{1:use }                   |
       {1:su}^port and selection    |
       ~                        |
-      -- VISUAL --             |
+      {2:-- VISUAL --}             |
     ]])
     feed('<LeftDrag><0,0>')
     screen:expect([[
@@ -88,7 +89,7 @@ describe('Mouse input', function()
       {1:mou}se                    |
       support and selection    |
       ~                        |
-      -- VISUAL --             |
+      {2:-- VISUAL --}             |
     ]])
   end)
 
@@ -99,7 +100,7 @@ describe('Mouse input', function()
       mouse                    |
       {1:suppor}^ and selection    |
       ~                        |
-      -- VISUAL --             |
+      {2:-- VISUAL --}             |
     ]])
   end)
 
@@ -110,7 +111,7 @@ describe('Mouse input', function()
       mouse                    |
       {1:su}^{1:port and selection }   |
       ~                        |
-      -- VISUAL LINE --        |
+      {2:-- VISUAL LINE --}        |
     ]])
   end)
 
@@ -121,7 +122,7 @@ describe('Mouse input', function()
       mouse                    |
       su^port and selection    |
       ~                        |
-      -- VISUAL BLOCK --       |
+      {2:-- VISUAL BLOCK --}       |
     ]])
   end)
 
@@ -140,7 +141,7 @@ describe('Mouse input', function()
       {1:mouse }                   |
       {1:su}^port and selection    |
       ~                        |
-      -- VISUAL --             |
+      {2:-- VISUAL --}             |
     ]])
   end)
 
@@ -153,7 +154,7 @@ describe('Mouse input', function()
       ing                      |
       Press ENTER or type comma|
       nd to continue^          |
-    ]])
+    ]],nil,true)
     feed('<cr>')
   end)
 
@@ -171,6 +172,8 @@ describe('Mouse input', function()
     ]])
     screen:try_resize(53, 14)
     execute('sp', 'vsp')
+    screen:set_default_attr_ignore( {{bold=true, foreground=hlgroup_colors.NonText},
+            {reverse=true}, {bold=true, reverse=true}} )
     screen:expect([[
       lines                     |lines                     |
       to                        |to                        |

test/functional/ui/screen.lua

@@ -54,12 +54,18 @@
 -- having the exact same set of attributes will be substituted by "{K:S}",
 -- where K is a key associated the attribute set via the second argument of
 -- "expect".
+-- If a transformation table is present, unexpected attribute sets in the final
+-- state is considered an error. To make testing simpler, a list of attribute
+-- sets that should be ignored can be passed as a third argument. Alternatively,
+-- this third argument can be "true" to indicate that all unexpected attribute
+-- sets should be ignored.
 --
--- Too illustrate how this works, let's say that in the above example we wanted
+-- To illustrate how this works, let's say that in the above example we wanted
 -- to assert that the "-- INSERT --" string is highlighted with the bold
 -- attribute(which normally is), here's how the call to "expect" should look
 -- like:
 --
+--     NonText = Screen.colors.Blue
 --     screen:expect([[
 --       hello screen             \
 --       ~                        \
@@ -71,11 +77,34 @@
 --       ~                        \
 --       ~                        \
 --       {b:-- INSERT --}             \
---     ]], {b = {bold = true}})
+--     ]], {b = {bold = true}}, {{bold = true, foreground = NonText}})
 --
 -- In this case "b" is a string associated with the set composed of one
 -- attribute: bold. Note that since the {b:} markup is not a real part of the
--- screen, the delimiter(|) had to be moved right
+-- screen, the delimiter(|) had to be moved right. Also, the highlighting of the
+-- NonText markers (~) is ignored in this test.
+--
+-- Multiple expect:s will likely share a group of attribute sets to test.
+-- Therefore these could be specified at the beginning of a test like this:
+--    NonText = Screen.colors.Blue
+--    screen:set_default_attr_ids( {
+--      [1] = {reverse = true, bold = true},
+--      [2] = {reverse = true}
+--    })
+--    screen:set_default_attr_ignore( {{}, {bold=true, foreground=NonText}} )
+-- These can be overridden for a specific expect expression, by passing
+-- different sets as parameters.
+--
+-- To help writing screen tests, there is a utility function
+-- "screen:snapshot_util()", that can be placed in a test file at any point an
+-- "expect(...)" should be. It will wait a short amount of time and then dump
+-- the current state of the screen, in the form of an "expect(..)" expression
+-- that would match it exactly. "snapshot_util" optionally also take the
+-- transformation and ignore set as parameters, like expect, or uses the default
+-- set. It will generate a larger attribute transformation set, if needed.
+-- To generate a text-only test without highlight checks,
+-- use `screen:snapshot_util({},true)`
+
 local helpers = require('test.functional.helpers')
 local request, run, stop = helpers.request, helpers.run, helpers.stop
 local eq, dedent = helpers.eq, helpers.dedent
@@ -90,6 +119,16 @@ if os.getenv('VALGRIND') then
   default_screen_timeout = 7500
 end
 
+local colors = request('vim_get_color_map')
+local colornames = {}
+for name, rgb in pairs(colors) do
+    -- we disregard the case that colornames might not be unique, as
+    -- this is just a helper to get any canonical name of a color
+    colornames[rgb] = name
+end
+
+Screen.colors = colors
+
 function Screen.debug(command)
   if not command then
     command = 'pynvim -n -g -c '
@@ -158,19 +197,11 @@ function Screen:expect(expected, attr_ids, attr_ignore)
     table.insert(expected_rows, row)
   end
   local ids = attr_ids or self._default_attr_ids
-  if attr_ignore == nil and self._default_attr_ignore ~= nil then
+  local ignore = attr_ignore or self._default_attr_ignore
-    attr_ignore = {}
-    -- don't ignore something we specified in attr_ids
-    for i,a in pairs(self._default_attr_ignore) do
-      if attr_index(ids, a) == nil then
-        table.insert(attr_ignore, a)
-      end
-    end
-  end
   self:wait(function()
     for i = 1, self._height do
       local expected_row = expected_rows[i]
-      local actual_row = self:_row_repr(self._rows[i], ids, attr_ignore)
+      local actual_row = self:_row_repr(self._rows[i], ids, ignore)
       if expected_row ~= actual_row then
         return 'Row '..tostring(i)..' didnt match.\nExpected: "'..
                expected_row..'"\nActual:   "'..actual_row..'"'
@@ -417,16 +448,20 @@ function Screen:snapshot_util(attrs, ignore)
       end
     end
 
+    if ignore ~= true then
       for i = 1, self._height do
         local row = self._rows[i]
         for j = 1, self._width do
           local attr = row[j].attrs
           if attr_index(attrs, attr) == nil and attr_index(ignore, attr) == nil then
+            if not equal_attrs(attr, {}) then
               table.insert(attrs, attr)
             end
           end
         end
       end
+    end
+  end
 
   local rv = {}
   for i = 1, self._height do
@@ -438,11 +473,7 @@ function Screen:snapshot_util(attrs, ignore)
     if self._default_attr_ids == nil or self._default_attr_ids[i] ~= a then
       alldefault = false
     end
-    local items = {}
+    local dict = "{"..pprint_attrs(a).."}"
-    for f, v in pairs(a) do
-      table.insert(items, f.." = "..tostring(v))
-    end
-    local dict = "{"..table.concat(items, ", ").."}"
     table.insert(attrstrs, "["..tostring(i).."] = "..dict)
   end
   local attrstr = "{"..table.concat(attrstrs, ", ").."}"
@@ -455,6 +486,19 @@ function Screen:snapshot_util(attrs, ignore)
   end
 end
 
+function pprint_attrs(attrs)
+    local items = {}
+    for f, v in pairs(attrs) do
+      local desc = tostring(v)
+      if f == "foreground" or f == "background" then
+        if colornames[v] ~= nil then
+          desc = "Screen.colors."..colornames[v]
+        end
+      end
+      table.insert(items, f.." = "..desc)
+    end
+    return table.concat(items, ", ")
+end
 
 function backward_find_meaningful(tbl, from)
   for i = from or #tbl, 1, -1 do
@@ -465,7 +509,7 @@ function backward_find_meaningful(tbl, from)
   return from
 end
 
-function get_attr_id(attr_ids, attr_ignore, attrs)
+function get_attr_id(attr_ids, ignore, attrs)
   if not attr_ids then
     return
   end
@@ -474,11 +518,12 @@ function get_attr_id(attr_ids, attr_ignore, attrs)
        return id
      end
   end
-  if attr_ignore == nil or attr_index(attr_ignore, attrs) ~= nil then
+  if equal_attrs(attrs, {}) or
+      ignore == true or attr_index(ignore, attrs) ~= nil then
     -- ignore this attrs
     return nil
   end
-  return "UNEXPECTED"
+  return "UNEXPECTED "..pprint_attrs(attrs)
 end
 
 function equal_attrs(a, b)