Address PR comments

runtime/autoload/man.vim

@@ -161,7 +161,7 @@ function! s:put_page(page) abort
   while getline(1) =~# '^\s*$'
     silent keepjumps 1delete _
   endwhile
-  call man#highlight_formatted_text()
+  lua require("man").highlight_man_page()
   setlocal filetype=man
 endfunction
 
@@ -375,7 +375,7 @@ function! man#init_pager() abort
   else
     keepjumps 1
   endif
-  call man#highlight_formatted_text()
+  lua require("man").highlight_man_page()
   " This is not perfect. See `man glDrawArraysInstanced`. Since the title is
   " all caps it is impossible to tell what the original capitilization was.
   let ref = substitute(matchstr(getline(1), '^[^)]\+)'), ' ', '_', 'g')
@@ -387,14 +387,4 @@ function! man#init_pager() abort
   execute 'silent file man://'.fnameescape(ref)
 endfunction
 
-function! man#highlight_formatted_text() abort
-  let l:modifiable = &modifiable
-  set modifiable
-
-  lua man = require("man")
-  luado return man.highlight_formatted(line, linenr)
-
-  let &modifiable = l:modifiable
-endfunction
-
 call s:init()

runtime/lua/man.lua

@@ -1,8 +1,10 @@
-local function highlight_formatted(line, linenr)
+local buf_hls = {}
+
+local function highlight_line(line, linenr)
   local chars = {}
   local prev_char = ''
   local overstrike, escape = false, false
-  local hls = {} -- Store highlight groups as { attr, start, end }
+  local hls = {} -- Store highlight groups as { attr, start, final }
   local NONE, BOLD, UNDERLINE, ITALIC = 0, 1, 2, 3
   local hl_groups = {[BOLD]="manBold", [UNDERLINE]="manUnderline", [ITALIC]="manItalic"}
   local attr = NONE
@@ -10,40 +12,40 @@ local function highlight_formatted(line, linenr)
 
   local function end_attr_hl(attr)
     for i, hl in ipairs(hls) do
-      if hl[1] == attr and hl[3] == -1 then
+      if hl.attr == attr and hl.final == -1 then
-        hl[3] = byte
+        hl.final = byte
         hls[i] = hl
       end
     end
   end
 
   local function add_attr_hl(code)
-    local on = true
+    local continue_hl = true
     if code == 0 then
       attr = NONE
-      on = false
+      continue_hl = false
     elseif code == 1 then
       attr = BOLD
-    elseif code == 21 or code == 22 then
+    elseif code == 22 then
       attr = BOLD
-      on = false
+      continue_hl = false
     elseif code == 3 then
       attr = ITALIC
     elseif code == 23 then
       attr = ITALIC
-      on = false
+      continue_hl = false
     elseif code == 4 then
       attr = UNDERLINE
     elseif code == 24 then
       attr = UNDERLINE
-      on = false
+      continue_hl = false
     else
       attr = NONE
       return
     end
 
-    if on then
+    if continue_hl then
-      hls[#hls + 1] = {attr, byte, -1}
+      hls[#hls + 1] = {attr=attr, start=byte, final=-1}
     else
       if attr == NONE then
         for a, _ in pairs(hl_groups) do
@@ -55,12 +57,15 @@ local function highlight_formatted(line, linenr)
     end
   end
 
-  -- Break input into UTF8 characters
+  -- Break input into UTF8 code points. ASCII code points (from 0x00 to 0x7f)
+  -- can be represented in one byte. Any code point above that is represented by
+  -- a leading byte (0xc0 and above) and continuation bytes (0x80 to 0xbf, or
+  -- decimal 128 to 191).
   for char in line:gmatch("[^\128-\191][\128-\191]*") do
     if overstrike then
       local last_hl = hls[#hls]
       if char == prev_char then
-        if char == '_' and attr == UNDERLINE and last_hl and last_hl[3] == byte then
+        if char == '_' and attr == UNDERLINE and last_hl and last_hl.final == byte then
           -- This underscore is in the middle of an underlined word
           attr = UNDERLINE
         else
@@ -72,21 +77,21 @@ local function highlight_formatted(line, linenr)
       elseif prev_char == '+' and char == 'o' then
         -- bullet (overstrike text '+^Ho')
         attr = BOLD
-        char = [[·]]
+        char = '·'
-      elseif prev_char == [[·]] and char == 'o' then
+      elseif prev_char == '·' and char == 'o' then
         -- bullet (additional handling for '+^H+^Ho^Ho')
         attr = BOLD
-        char = [[·]]
+        char = '·'
       else
         -- use plain char
         attr = NONE
       end
 
       -- Grow the previous highlight group if possible
-      if last_hl and last_hl[1] == attr and last_hl[3] == byte then
+      if last_hl and last_hl.attr == attr and last_hl.final == byte then
-        last_hl[3] = byte + #char
+        last_hl.final = byte + #char
       else
-        hls[#hls + 1] = {attr, byte, byte + #char}
+        hls[#hls + 1] = {attr=attr, start=byte, final=byte + #char}
       end
 
       overstrike = false
@@ -96,15 +101,19 @@ local function highlight_formatted(line, linenr)
     elseif escape then
       -- Use prev_char to store the escape sequence
       prev_char = prev_char .. char
-      local sgr = prev_char:match("^%[([\020-\063]*)m$")
+      -- We only want to match against SGR sequences, which consist of ESC
+      -- followed by '[', then a series of parameter and intermediate bytes in
+      -- the range 0x20 - 0x3f, then 'm'. (See ECMA-48, sections 5.4 & 8.3.117)
+      local sgr = prev_char:match("^%[([\032-\063]*)m$")
       if sgr then
         local match = ''
         while sgr and #sgr > 0 do
+          -- Match against SGR parameters, which may be separated by ';'
           match, sgr = sgr:match("^(%d*);?(.*)")
           add_attr_hl(match + 0) -- coerce to number
         end
         escape = false
-      elseif not prev_char:match("^%[[\020-\063]*$") then
+      elseif not prev_char:match("^%[[\032-\063]*$") then
         -- Stop looking if this isn't a partial CSI sequence
         escape = false
       end
@@ -122,20 +131,38 @@ local function highlight_formatted(line, linenr)
     end
   end
 
-  for i, hl in ipairs(hls) do
+  for _, hl in ipairs(hls) do
-    if hl[1] ~= NONE then
+    if hl.attr ~= NONE then
-      vim.api.nvim_buf_add_highlight(
+      buf_hls[#buf_hls + 1] = {
         0,
         -1,
-        hl_groups[hl[1]],
+        hl_groups[hl.attr],
         linenr - 1,
-        hl[2],
+        hl.start,
-        hl[3]
+        hl.final
-      )
+      }
     end
   end
 
   return table.concat(chars, '')
 end
 
-return { highlight_formatted = highlight_formatted }
+local function highlight_man_page()
+  local mod = vim.api.nvim_eval("&modifiable")
+  vim.api.nvim_command("set modifiable")
+
+  local lines = vim.api.nvim_buf_get_lines(0, 0, -1, false)
+  for i, line in ipairs(lines) do
+    lines[i] = highlight_line(line, i)
+  end
+  vim.api.nvim_buf_set_lines(0, 0, -1, false, lines)
+
+  for _, args in ipairs(buf_hls) do
+    vim.api.nvim_buf_add_highlight(unpack(args))
+  end
+  buf_hls = {}
+
+  vim.api.nvim_command("let &modifiable = "..mod)
+end
+
+return { highlight_man_page = highlight_man_page }

runtime/syntax/man.vim

@@ -18,18 +18,9 @@ highlight default link manOptionDesc     Constant
 highlight default link manReference      PreProc
 highlight default link manSubHeading     Function
 
-function! s:init_highlight_groups()
+highlight default manUnderline cterm=underline gui=underline
-  highlight default manUnderline cterm=underline gui=underline
+highlight default manBold      cterm=bold      gui=bold
-  highlight default manBold      cterm=bold      gui=bold
+highlight default manItalic    cterm=italic    gui=italic
-  highlight default manItalic    cterm=italic    gui=italic
-endfunction
-
-augroup man_init_highlight_groups
-  autocmd!
-  autocmd ColorScheme * call s:init_highlight_groups()
-augroup END
-
-call s:init_highlight_groups()
 
 if &filetype != 'man'
   " May have been included by some other filetype.

test/functional/plugin/man_spec.lua

@@ -3,7 +3,7 @@ local plugin_helpers = require('test.functional.plugin.helpers')
 
 local Screen = require('test.functional.ui.screen')
 
-local buffer, command, eval = helpers.buffer, helpers.command, helpers.eval
+local command, eval, rawfeed = helpers.command, helpers.eval, helpers.rawfeed
 
 before_each(function()
   plugin_helpers.reset()
@@ -19,6 +19,17 @@ describe('In autoload/man.vim', function()
     before_each(function()
       command('syntax off') -- Ignore syntax groups
       screen = Screen.new(52, 5)
+      screen:set_default_attr_ids({
+        b = { bold = true },
+        i = { italic = true },
+        u = { underline = true },
+        bi = { bold = true, italic = true },
+        biu = { bold = true, italic = true, underline = true },
+      })
+      screen:set_default_attr_ignore({
+        { foreground = Screen.colors.Blue }, -- control chars
+        { bold = true, foreground = Screen.colors.Blue } -- empty line '~'s
+      })
       screen:attach()
     end)
 
@@ -26,84 +37,22 @@ describe('In autoload/man.vim', function()
       screen:detach()
     end)
 
-    local function expect(string)
+    it('clears backspaces from text and adds highlights', function()
-      screen:expect(string,
+      rawfeed([[
-      {
+        ithis i<C-v><C-h>is<C-v><C-h>s a<C-v><C-h>a test
-        b = { bold = true },
+        with _<C-v><C-h>o_<C-v><C-h>v_<C-v><C-h>e_<C-v><C-h>r_<C-v><C-h>s_<C-v><C-h>t_<C-v><C-h>r_<C-v><C-h>u_<C-v><C-h>c_<C-v><C-h>k text<ESC>]])
-        i = { italic = true },
-        u = { underline = true },
-        bi = { bold = true, italic = true },
-        biu = { bold = true, italic = true, underline = true },
-      },
-      {{ bold = true, foreground = Screen.colors.Blue }})
-    end
 
-    local function expect_without_highlights(string)
+      screen:expect([[
-      screen:expect(string, nil, true)
+      this i^His^Hs a^Ha test                             |
-    end
+      with _^Ho_^Hv_^He_^Hr_^Hs_^Ht_^Hr_^Hu_^Hc_^Hk tex^t  |
-
-    local function insert_lines(...)
-      buffer('set_lines', 0, 0, 1, false, { ... })
-    end
-
-    it('clears backspaces from text', function()
-      insert_lines(
-        "this i\bis\bs a\ba test",
-        "with _\bo_\bv_\be_\br_\bs_\bt_\br_\bu_\bc_\bk text"
-      )
-
-      expect_without_highlights([[
-      ^this i^His^Hs a^Ha test                             |
-      with _^Ho_^Hv_^He_^Hr_^Hs_^Ht_^Hr_^Hu_^Hc_^Hk text  |
       ~                                                   |
       ~                                                   |
                                                           |
       ]])
 
-      eval('man#highlight_formatted_text()')
+      eval('man#init_pager()')
 
-      expect_without_highlights([[
+      screen:expect([[
-      ^this is a test                                      |
-      with overstruck text                                |
-      ~                                                   |
-      ~                                                   |
-                                                          |
-      ]])
-    end)
-
-    it('clears escape sequences from text', function()
-      insert_lines(
-        "this \027[1mis \027[3ma \027[4mtest\027[0m",
-        "\027[4mwith\027[24m \027[4mescaped\027[24m \027[4mtext\027[24m"
-      )
-
-      expect_without_highlights([[
-      ^this ^[[1mis ^[[3ma ^[[4mtest^[[0m                  |
-      ^[[4mwith^[[24m ^[[4mescaped^[[24m ^[[4mtext^[[24m  |
-      ~                                                   |
-      ~                                                   |
-                                                          |
-      ]])
-
-      eval('man#highlight_formatted_text()')
-
-      expect_without_highlights([[
-      ^this is a test                                      |
-      with escaped text                                   |
-      ~                                                   |
-      ~                                                   |
-                                                          |
-      ]])
-    end)
-
-    it('highlights overstruck text', function()
-      insert_lines(
-        "this i\bis\bs a\ba test",
-        "with _\bo_\bv_\be_\br_\bs_\bt_\br_\bu_\bc_\bk text"
-      )
-      eval('man#highlight_formatted_text()')
-
-      expect([[
       ^this {b:is} {b:a} test                                      |
       with {u:overstruck} text                                |
       ~                                                   |
@@ -112,14 +61,22 @@ describe('In autoload/man.vim', function()
       ]])
     end)
 
-    it('highlights escape sequences in text', function()
+    it('clears escape sequences from text and adds highlights', function()
-      insert_lines(
+      rawfeed([[
-        "this \027[1mis \027[3ma \027[4mtest\027[0m",
+        ithis <C-v><ESC>[1mis <C-v><ESC>[3ma <C-v><ESC>[4mtest<C-v><ESC>[0m
-        "\027[4mwith\027[24m \027[4mescaped\027[24m \027[4mtext\027[24m"
+        <C-v><ESC>[4mwith<C-v><ESC>[24m <C-v><ESC>[4mescaped<C-v><ESC>[24m <C-v><ESC>[4mtext<C-v><ESC>[24m<ESC>]])
-      )
-      eval('man#highlight_formatted_text()')
 
-      expect([[
+      screen:expect([[
+      this ^[[1mis ^[[3ma ^[[4mtest^[[0m                  |
+      ^[[4mwith^[[24m ^[[4mescaped^[[24m ^[[4mtext^[[24^m  |
+      ~                                                   |
+      ~                                                   |
+                                                          |
+      ]])
+
+      eval('man#init_pager()')
+
+      screen:expect([[
       ^this {b:is }{bi:a }{biu:test}                                      |
       {u:with} {u:escaped} {u:text}                                   |
       ~                                                   |
@@ -129,15 +86,14 @@ describe('In autoload/man.vim', function()
     end)
 
     it('highlights multibyte text', function()
-      insert_lines(
+      rawfeed([[
-        "this i\bis\bs あ\bあ test",
+        ithis i<C-v><C-h>is<C-v><C-h>s あ<C-v><C-h>あ test
-        "with _\bö_\bv_\be_\br_\bs_\bt_\br_\bu_\bc_\bk te\027[3mxt¶\027[0m"
+        with _<C-v><C-h>ö_<C-v><C-h>v_<C-v><C-h>e_<C-v><C-h>r_<C-v><C-h>s_<C-v><C-h>t_<C-v><C-h>r_<C-v><C-h>u_<C-v><C-h>̃_<C-v><C-h>c_<C-v><C-h>k te<C-v><ESC>[3mxt¶<C-v><ESC>[0m<ESC>]])
-      )
+      eval('man#init_pager()')
-      eval('man#highlight_formatted_text()')
 
-      expect([[
+      screen:expect([[
       ^this {b:is} {b:あ} test                                     |
-      with {u:överstruck} te{i:xt¶}                               |
+      with {u:överstrũck} te{i:xt¶}                               |
       ~                                                   |
       ~                                                   |
                                                           |
@@ -145,14 +101,13 @@ describe('In autoload/man.vim', function()
     end)
 
     it('highlights underscores based on context', function()
-      insert_lines(
+      rawfeed([[
-        "_\b_b\bbe\beg\bgi\bin\bns\bs",
+        i_<C-v><C-h>_b<C-v><C-h>be<C-v><C-h>eg<C-v><C-h>gi<C-v><C-h>in<C-v><C-h>ns<C-v><C-h>s
-        "m\bmi\bid\bd_\b_d\bdl\ble\be",
+        m<C-v><C-h>mi<C-v><C-h>id<C-v><C-h>d_<C-v><C-h>_d<C-v><C-h>dl<C-v><C-h>le<C-v><C-h>e
-        "_\bm_\bi_\bd_\b__\bd_\bl_\be"
+        _<C-v><C-h>m_<C-v><C-h>i_<C-v><C-h>d_<C-v><C-h>__<C-v><C-h>d_<C-v><C-h>l_<C-v><C-h>e<ESC>]])
-      )
+      eval('man#init_pager()')
-      eval('man#highlight_formatted_text()')
 
-      expect([[
+      screen:expect([[
       {b:^_begins}                                             |
       {b:mid_dle}                                             |
       {u:mid_dle}                                             |
@@ -162,14 +117,13 @@ describe('In autoload/man.vim', function()
     end)
 
     it('highlights various bullet formats', function()
-      insert_lines(
+      rawfeed([[
-        "· ·\b·",
+        i· ·<C-v><C-h>·
-        "+\bo",
+        +<C-v><C-h>o
-        "+\b+\bo\bo double"
+        +<C-v><C-h>+<C-v><C-h>o<C-v><C-h>o double<ESC>]])
-      )
+      eval('man#init_pager()')
-      eval('man#highlight_formatted_text()')
 
-      expect([[
+      screen:expect([[
       ^· {b:·}                                                 |
       {b:·}                                                   |
       {b:·} double                                            |