ex_getln: Add some more tests, fix some found errors

4 files changed, 320 insertions, 23 deletions

diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c
index b08407233f..e4914d6ebc 100644
--- a/src/nvim/ex_getln.c
+++ b/src/nvim/ex_getln.c
@@ -2178,6 +2178,8 @@ void free_cmdline_buf(void)
 
 # endif
 
+enum { MAX_CB_ERRORS = 5 };
+
 /// Color command-line
 ///
 /// Should use built-in command parser or user-specified one. Currently only the 
@@ -2186,50 +2188,77 @@ void free_cmdline_buf(void)
 /// Operates on ccline, saving results to ccline_colors.
 ///
 /// Always colors the whole cmdline.
-static void color_cmdline(void)
+///
+/// @return true if draw_cmdline may proceed, false if it does not need anything
+///         to do.
+static bool color_cmdline(void)
 {
+  bool ret = true;
   kv_size(ccline_colors) = 0;
   if (ccline.cmdfirstc != ':') {
-    return;
+    return ret;
   }
+
+  const int saved_force_abort = force_abort;
+  force_abort = true;
+  bool arg_allocated = false;
+  typval_T arg = {
+    .v_type = VAR_STRING,
+    .vval.v_string = ccline.cmdbuff,
+  };
+  typval_T tv = { .v_type = VAR_UNKNOWN };
+
   static Callback prev_cb = { .type = kCallbackNone };
   static int prev_cb_errors = 0;
   Callback color_cb;
   if (!tv_dict_get_callback(&globvardict, S_LEN("Nvim_color_cmdline"),
                             &color_cb)) {
-    return;
+    goto color_cmdline_error;
   }
   if (color_cb.type == kCallbackNone) {
-    return;
+    goto color_cmdline_end;
   }
-  if (prev_cb_errors == 5 && tv_callback_equal(&prev_cb, &color_cb)) {
-    return;
+  if (!tv_callback_equal(&prev_cb, &color_cb)) {
+    prev_cb_errors = 0;
+  } else if (prev_cb_errors >= MAX_CB_ERRORS) {
+    callback_free(&color_cb);
+    goto color_cmdline_end;
   }
   callback_free(&prev_cb);
   prev_cb = color_cb;
-  bool arg_allocated;
-  typval_T arg = {
-    .v_type = VAR_STRING,
-  };
-  if (ccline.cmdbuff[ccline.cmdlen] == NUL) {
-    arg_allocated = false;
-    arg.vval.v_string = ccline.cmdbuff;
-  } else {
+  if (ccline.cmdbuff[ccline.cmdlen] != NUL) {
     arg_allocated = true;
     arg.vval.v_string = xmemdupz((const char *)ccline.cmdbuff,
                                  (size_t)ccline.cmdlen);
   }
-  typval_T tv;
+  // msg_start() called by e.g. :echo may shift command-line to the first column
+  // even though msg_silent is here. Two ways to workaround this problem without
+  // altering message.c: use full_screen or save and restore msg_col.
+  //
+  // Saving and restoring full_screen does not work well with :redraw!. Saving
+  // and restoring msg_col is neither ideal, but while with full_screen it
+  // appears shifted one character to the right and cursor position is no longer
+  // correct, with msg_col it just misses leading `:`. Since `redraw!` in
+  // callback lags this is least of the user problems.
+  const int saved_msg_col = msg_col;
   msg_silent++;
   if (!callback_call(&color_cb, 1, &arg, &tv)) {
     msg_silent--;
-    goto color_cmdline_end;
+    msg_col = saved_msg_col;
+    goto color_cmdline_error;
   }
   msg_silent--;
-  if (tv.v_type != VAR_LIST || tv.vval.v_list == NULL) {
+  msg_col = saved_msg_col;
+  if (got_int || did_emsg) {
+    goto color_cmdline_error;
+  }
+  if (tv.v_type != VAR_LIST) {
     emsgf(_("E5400: Callback should return list"));
     goto color_cmdline_error;
   }
+  if (tv.vval.v_list == NULL) {
+    goto color_cmdline_end;
+  }
   varnumber_T prev_end = 0;
   int i = 0;
   for (const listitem_T *li = tv.vval.v_list->lv_first;
@@ -2248,11 +2277,15 @@ static void color_cmdline(void)
     const varnumber_T start = tv_get_number_chk(&l->lv_first->li_tv, &error);
     if (error) {
       goto color_cmdline_error;
-    } else if (!(prev_end <= start && start <= ccline.cmdlen)) {
+    } else if (!(prev_end <= start && start < ccline.cmdlen)) {
       emsgf(_("E5403: Chunk %i start %" PRIdVARNUMBER " not in range "
-              "[%" PRIdVARNUMBER ", %i]"),
+              "[%" PRIdVARNUMBER ", %i)"),
             i, start, prev_end, ccline.cmdlen);
       goto color_cmdline_error;
+    } else if (utf8len_tab_zero[(uint8_t)ccline.cmdbuff[start]] == 0) {
+      emsgf(_("E5405: Chunk %i start %" PRIdVARNUMBER " splits multibyte "
+              "character"), i, start);
+      goto color_cmdline_error;
     }
     if (start != prev_end) {
       kv_push(ccline_colors, ((ColoredCmdlineChunk) {
@@ -2270,6 +2303,11 @@ static void color_cmdline(void)
               "(%" PRIdVARNUMBER ", %i]"),
             i, end, start, ccline.cmdlen);
       goto color_cmdline_error;
+    } else if (end < ccline.cmdlen
+               && utf8len_tab_zero[(uint8_t)ccline.cmdbuff[end]] == 0) {
+      emsgf(_("E5406: Chunk %i end %" PRIdVARNUMBER " splits multibyte "
+              "character"), i, end);
+      goto color_cmdline_error;
     }
     prev_end = end;
     const char *const group = tv_get_string_chk(&l->lv_last->li_tv);
@@ -2293,13 +2331,16 @@ static void color_cmdline(void)
   }
   prev_cb_errors = 0;
 color_cmdline_end:
+  force_abort = saved_force_abort;
   if (arg_allocated) {
     tv_clear(&arg);
   }
   tv_clear(&tv);
-  return;
+  return ret;
 color_cmdline_error:
   prev_cb_errors++;
+  const bool do_redraw = (did_emsg || got_int);
+  got_int = false;
   did_emsg = false;
   if (did_throw) {
     discard_current_exception();
@@ -2308,6 +2349,12 @@ color_cmdline_error:
     free_global_msglist();
   }
   kv_size(ccline_colors) = 0;
+  if (do_redraw) {
+    prev_cb_errors += MAX_CB_ERRORS;
+    redrawcmdline();
+    prev_cb_errors -= MAX_CB_ERRORS;
+    ret = false;
+  }
   goto color_cmdline_end;
 }
 
@@ -2317,6 +2364,10 @@ color_cmdline_error:
  */
 static void draw_cmdline(int start, int len)
 {
+  if (!color_cmdline()) {
+    return;
+  }
+
   if (cmdline_star > 0) {
     for (int i = 0; i < len; i++) {
       msg_putchar('*');
@@ -2415,7 +2466,6 @@ static void draw_cmdline(int start, int len)
     msg_outtrans_len(arshape_buf, newlen);
   } else {
 draw_cmdline_no_arabicshape:
-    color_cmdline();
     if (kv_size(ccline_colors)) {
       for (size_t i = 0; i < kv_size(ccline_colors); i++) {
         ColoredCmdlineChunk chunk = kv_A(ccline_colors, i);
diff --git a/src/nvim/mbyte.c b/src/nvim/mbyte.c
index d5907da2ed..862ca7ea1a 100644
--- a/src/nvim/mbyte.c
+++ b/src/nvim/mbyte.c
@@ -75,7 +75,7 @@ struct interval {
 /*
  * Like utf8len_tab above, but using a zero for illegal lead bytes.
  */
-static uint8_t utf8len_tab_zero[256] =
+const uint8_t utf8len_tab_zero[256] =
 {
   1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
   1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
diff --git a/src/nvim/mbyte.h b/src/nvim/mbyte.h
index ad9e38004c..02c6065672 100644
--- a/src/nvim/mbyte.h
+++ b/src/nvim/mbyte.h
@@ -1,6 +1,7 @@
 #ifndef NVIM_MBYTE_H
 #define NVIM_MBYTE_H
 
+#include <stdint.h>
 #include <stdbool.h>
 #include <string.h>
 
@@ -67,6 +68,8 @@ typedef struct {
                  ///< otherwise use '?'.
 } vimconv_T;
 
+extern const uint8_t utf8len_tab_zero[256];
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "mbyte.h.generated.h"
 #endif
diff --git a/test/functional/ui/cmdline_highlight_spec.lua b/test/functional/ui/cmdline_highlight_spec.lua
index 812841ab7e..eb0a65bea7 100644
--- a/test/functional/ui/cmdline_highlight_spec.lua
+++ b/test/functional/ui/cmdline_highlight_spec.lua
@@ -1,16 +1,18 @@
 local helpers = require('test.functional.helpers')(after_each)
 local Screen = require('test.functional.ui.screen')
 
+local eq = helpers.eq
 local feed = helpers.feed
 local clear = helpers.clear
 local meths = helpers.meths
+local funcs = helpers.funcs
 local source = helpers.source
 
 local screen
 
 before_each(function()
   clear()
-  screen = Screen.new(40, 2)
+  screen = Screen.new(40, 8)
   screen:attach()
   source([[
     highlight RBP1 guifg=Red
@@ -39,12 +41,74 @@ before_each(function()
       endwhile
       return ret
     endfunction
+    function SplittedMultibyteStart(cmdline)
+      let ret = []
+      let i = 0
+      while i < len(a:cmdline)
+        let char = nr2char(char2nr(a:cmdline[i:]))
+        if a:cmdline[i:i +  len(char) - 1] is# char
+          if len(char) > 1
+            call add(ret, [i + 1, i + len(char), 'RBP2'])
+          endif
+          let i += len(char)
+        else
+          let i += 1
+        endif
+      endwhile
+      return ret
+    endfunction
+    function SplittedMultibyteEnd(cmdline)
+      let ret = []
+      let i = 0
+      while i < len(a:cmdline)
+        let char = nr2char(char2nr(a:cmdline[i:]))
+        if a:cmdline[i:i +  len(char) - 1] is# char
+          if len(char) > 1
+            call add(ret, [i, i + 1, 'RBP1'])
+          endif
+          let i += len(char)
+        else
+          let i += 1
+        endif
+      endwhile
+      return ret
+    endfunction
+    function Echoing(cmdline)
+      echo 'HERE'
+      return v:_null_list
+    endfunction
+    function Echoning(cmdline)
+      echon 'HERE'
+      return v:_null_list
+    endfunction
+    function Echomsging(cmdline)
+      echomsg 'HERE'
+      return v:_null_list
+    endfunction
+    function Echoerring(cmdline)
+      echoerr 'HERE'
+      return v:_null_list
+    endfunction
+    function Redrawing(cmdline)
+      redraw!
+      return v:_null_list
+    endfunction
+    function Throwing(cmdline)
+      throw "ABC"
+      return v:_null_list
+    endfunction
+    function Halting(cmdline)
+      while 1
+      endwhile
+    endfunction
   ]])
   screen:set_default_attr_ids({
     RBP1={foreground = Screen.colors.Red},
     RBP2={foreground = Screen.colors.Yellow},
     RBP3={foreground = Screen.colors.Green},
     RBP4={foreground = Screen.colors.Blue},
+    EOB={bold = true, foreground = Screen.colors.Blue1},
+    ERR={foreground = Screen.colors.Grey100, background = Screen.colors.Red},
   })
 end)
 
@@ -55,47 +119,227 @@ describe('Command-line coloring', function()
     feed(':')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :^                                       |
     ]])
     feed('e')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :e^                                      |
     ]])
     feed('cho ')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo ^                                  |
     ]])
     feed('(')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo {RBP1:(}^                                 |
     ]])
     feed('(')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo {RBP1:(}{RBP2:(}^                                |
     ]])
     feed('42')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo {RBP1:(}{RBP2:(}42^                              |
     ]])
     feed('))')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo {RBP1:(}{RBP2:(}42{RBP2:)}{RBP1:)}^                            |
     ]])
     feed('<BS>')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo {RBP1:(}{RBP2:(}42{RBP2:)}^                             |
     ]])
     feed('{REDRAW}')
     screen:expect([[
                                               |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
       :echo {RBP1:(}{RBP2:(}42{RBP2:)}^                             |
     ]])
   end)
+  for _, func_part in ipairs({'', 'n', 'msg'}) do
+    it('disables :echo' .. func_part .. ' messages', function()
+      meths.set_var('Nvim_color_cmdline', 'Echo' .. func_part .. 'ing')
+      feed(':echo')
+      screen:expect([[
+                                                |
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        :echo^                                   |
+      ]])
+    end)
+  end
+  it('does the right thing when hl start appears to split multibyte char',
+  function()
+    meths.set_var('Nvim_color_cmdline', 'SplittedMultibyteStart')
+    feed(':echo "«')
+    screen:expect([[
+      {EOB:~                                       }|
+      :echo "                                 |
+      {ERR:E5405: Chunk 0 start 7 splits multibyte }|
+      {ERR:character}                               |
+      :echo "«                                |
+      {ERR:E5405: Chunk 0 start 7 splits multibyte }|
+      {ERR:character}                               |
+      :echo "«^                                |
+    ]])
+    feed('»')
+    -- FIXME Does not work well with too much error messages: they overwrite
+    --       cmdline.
+  end)
+  it('does the right thing when hl end appears to split multibyte char',
+  function()
+    meths.set_var('Nvim_color_cmdline', 'SplittedMultibyteEnd')
+    feed(':echo "«')
+    screen:expect([[
+      {EOB:~                                       }|
+      :echo "                                 |
+      {ERR:E5406: Chunk 0 end 7 splits multibyte ch}|
+      {ERR:aracter}                                 |
+      :echo "«                                |
+      {ERR:E5406: Chunk 0 end 7 splits multibyte ch}|
+      {ERR:aracter}                                 |
+      :echo "«^                                |
+    ]])
+  end)
+  it('does the right thing when errorring', function()
+    meths.set_var('Nvim_color_cmdline', 'Echoerring')
+    feed(':e')
+    -- FIXME Does not work well with :echoerr: error message overwrites cmdline.
+  end)
+  it('does the right thing when throwing', function()
+    meths.set_var('Nvim_color_cmdline', 'Throwing')
+    feed(':e')
+    -- FIXME Does not work well with :throw: error message overwrites cmdline.
+  end)
+  it('still executes command-line even if errored out', function()
+    meths.set_var('Nvim_color_cmdline', 'SplittedMultibyteStart')
+    feed(':let x = "«"\n')
+    eq('«', meths.get_var('x'))
+    local msg = 'E5405: Chunk 0 start 10 splits multibyte character'
+    eq('\n'..msg..'\n'..msg, funcs.execute('messages'))
+  end)
+  it('stops executing callback after a number of errors', function()
+    meths.set_var('Nvim_color_cmdline', 'SplittedMultibyteStart')
+    feed(':let x = "«»«»«»«»«»"\n')
+    eq('«»«»«»«»«»', meths.get_var('x'))
+    local msg = '\nE5405: Chunk 0 start 10 splits multibyte character'
+    eq(msg:rep(5), funcs.execute('messages'))
+  end)
+  it('allows interrupting callback with <C-c>', function()
+    meths.set_var('Nvim_color_cmdline', 'Halting')
+    feed(':echo 42')
+    for i = 1, 6 do
+      screen:expect([[
+        ^                                        |
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+        {EOB:~                                       }|
+                                                |
+      ]])
+      feed('<C-c>')
+    end
+    screen:expect([[
+      ^                                        |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      Type  :quit<Enter>  to exit Nvim        |
+    ]])
+    feed(':echo 42<CR>')
+    screen:expect([[
+      ^                                        |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      42                                      |
+    ]])
+  end)
+  it('works fine with NUL, NL, CR', function()
+    meths.set_var('Nvim_color_cmdline', 'RainBowParens')
+    feed(':echo ("<C-v><CR><C-v><Nul><C-v><NL>")')
+    screen:expect([[
+                                              |
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      {EOB:~                                       }|
+      :echo {RBP1:(}"{RBP4:^M^@^@}"{RBP1:)}^                        |
+    ]])
+  end)
+  -- TODO Check for all other errors
 end)