refactor(decorations): merge the two different code paths for virt_text

test(bufhl): CHANGE of tested behaviour (inb4 a proper priority mechanism)
test(decoration): change of test; previous behavior was buggy (ghost
buffer text)

7 files changed, 78 insertions, 117 deletions

diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c
index 47216996b4..56c9cfc517 100644
--- a/src/nvim/api/buffer.c
+++ b/src/nvim/api/buffer.c
@@ -1566,7 +1566,8 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id,
                       "virt_text is not an Array");
         goto error;
       }
-      decor.virt_text = parse_virt_text(v->data.array, err);
+      decor.virt_text = parse_virt_text(v->data.array, err,
+                                        &decor.virt_text_width);
       if (ERROR_SET(err)) {
         goto error;
       }
@@ -1949,23 +1950,26 @@ Integer nvim_buf_set_virtual_text(Buffer buffer,
   }
 
   uint64_t ns_id = src2ns(&src_id);
+  int width;
 
-  VirtText virt_text = parse_virt_text(chunks, err);
+  VirtText virt_text = parse_virt_text(chunks, err, &width);
   if (ERROR_SET(err)) {
     return 0;
   }
 
 
-  VirtText *existing = decor_find_virttext(buf, (int)line, ns_id);
+  Decoration *existing = decor_find_virttext(buf, (int)line, ns_id);
 
   if (existing) {
-    clear_virttext(existing);
-    *existing = virt_text;
+    clear_virttext(&existing->virt_text);
+    existing->virt_text = virt_text;
+    existing->virt_text_width = width;
     return src_id;
   }
 
   Decoration *decor = xcalloc(1, sizeof(*decor));
   decor->virt_text = virt_text;
+  decor->virt_text_width = width;
 
   extmark_set(buf, ns_id, 0, (int)line, 0, -1, -1, decor, true,
               false, kExtmarkNoUndo);
diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index e78bd2ea9a..e9a436d3e3 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -1592,9 +1592,10 @@ bool extmark_get_index_from_obj(buf_T *buf, Integer ns_id, Object obj, int
   }
 }
 
-VirtText parse_virt_text(Array chunks, Error *err)
+VirtText parse_virt_text(Array chunks, Error *err, int *width)
 {
   VirtText virt_text = KV_INITIAL_VALUE;
+  int w = 0;
   for (size_t i = 0; i < chunks.size; i++) {
     if (chunks.items[i].type != kObjectTypeArray) {
       api_set_error(err, kErrorTypeValidation, "Chunk is not an array");
@@ -1635,9 +1636,12 @@ VirtText parse_virt_text(Array chunks, Error *err)
     }
 
     char *text = transstr(str.size > 0 ? str.data : "");  // allocates
+    w += (int)mb_string2cells((char_u *)text);
+
     kv_push(virt_text, ((VirtTextChunk){ .text = text, .hl_id = hl_id }));
   }
 
+  *width = w;
   return virt_text;
 
 free_exit:
diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c
index 9d969ada89..74cb9a26b7 100644
--- a/src/nvim/decoration.c
+++ b/src/nvim/decoration.c
@@ -119,7 +119,7 @@ void clear_virttext(VirtText *text)
   *text = (VirtText)KV_INITIAL_VALUE;
 }
 
-VirtText *decor_find_virttext(buf_T *buf, int row, uint64_t ns_id)
+Decoration *decor_find_virttext(buf_T *buf, int row, uint64_t ns_id)
 {
   MarkTreeIter itr[1] = { 0 };
   marktree_itr_get(buf->b_marktree, row, 0,  itr);
@@ -132,7 +132,7 @@ VirtText *decor_find_virttext(buf_T *buf, int row, uint64_t ns_id)
                                                        mark.id, false);
     if (item && (ns_id == 0 || ns_id == item->ns_id)
         && item->decor && kv_size(item->decor->virt_text)) {
-      return &item->decor->virt_text;
+      return item->decor;
     }
     marktree_itr_next(buf->b_marktree, itr);
   }
@@ -218,6 +218,7 @@ bool decor_redraw_line(buf_T *buf, int row, DecorState *state)
   }
   state->row = row;
   state->col_until = -1;
+  state->eol_col = -1;
   return true;  // TODO(bfredl): be more precise
 }
 
@@ -230,10 +231,6 @@ static void decor_add(DecorState *state, int start_row, int start_col,
                        *decor, attr_id,
                     kv_size(decor->virt_text) && owned, -1 };
 
-  if (decor->virt_text_pos == kVTEndOfLine) {
-    range.win_col = -2;  // handled separately
-  }
-
   kv_pushp(state->active);
   size_t index;
   for (index = kv_size(state->active)-1; index > 0; index--) {
@@ -345,29 +342,22 @@ void decor_redraw_end(DecorState *state)
   state->buf = NULL;
 }
 
-VirtText decor_redraw_eol(buf_T *buf, DecorState *state, int *eol_attr,
-                          bool *aligned)
+bool decor_redraw_eol(buf_T *buf, DecorState *state, int *eol_attr, int eol_col)
 {
   decor_redraw_col(buf, MAXCOL, MAXCOL, false, state);
-  VirtText text = VIRTTEXT_EMPTY;
+  state->eol_col = eol_col;
+  bool has_virttext = false;
   for (size_t i = 0; i < kv_size(state->active); i++) {
     DecorRange item = kv_A(state->active, i);
     if (item.start_row == state->row && kv_size(item.decor.virt_text)) {
-      if (!kv_size(text) && item.decor.virt_text_pos == kVTEndOfLine) {
-        text = item.decor.virt_text;
-      } else if (item.decor.virt_text_pos == kVTRightAlign
-                 || item.decor.virt_text_pos == kVTWinCol) {
-        *aligned = true;
-      }
+      has_virttext = true;
     }
 
-
     if (item.decor.hl_eol && item.start_row <= state->row) {
       *eol_attr = hl_combine_attr(*eol_attr, item.attr_id);
     }
   }
-
-  return text;
+  return has_virttext;
 }
 
 void decor_add_ephemeral(int start_row, int start_col, int end_row, int end_col,
diff --git a/src/nvim/decoration.h b/src/nvim/decoration.h
index 4cebc0b731..28dabeeada 100644
--- a/src/nvim/decoration.h
+++ b/src/nvim/decoration.h
@@ -34,19 +34,20 @@ typedef enum {
 
 struct Decoration
 {
-  int hl_id;  // highlight group
   VirtText virt_text;
+  int hl_id;  // highlight group
   VirtTextPos virt_text_pos;
-  bool virt_text_hide;
   HlMode hl_mode;
+  bool virt_text_hide;
   bool hl_eol;
+  bool shared;  // shared decoration, don't free
   // TODO(bfredl): style, signs, etc
   DecorPriority priority;
-  bool shared;  // shared decoration, don't free
   int col;  // fixed col value, like win_col
+  int virt_text_width;  // width of virt_text
 };
-#define DECORATION_INIT { 0, KV_INITIAL_VALUE, kVTEndOfLine, false, \
-                          kHlModeUnknown, false, DECOR_PRIORITY_BASE, false, 0 }
+#define DECORATION_INIT { KV_INITIAL_VALUE, 0, kVTEndOfLine, kHlModeUnknown, \
+                          false, false, false, DECOR_PRIORITY_BASE, 0, 0 }
 
 typedef struct {
   int start_row;
@@ -67,6 +68,8 @@ typedef struct {
   int row;
   int col_until;
   int current;
+
+  int eol_col;
   VirtText *virt_text;
 } DecorState;
 
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index 586aeff1e2..d8f5f1077c 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -165,7 +165,7 @@ static bool resizing = false;
 #endif
 #define SEARCH_HL_PRIORITY 0
 
-static char * provider_first_error = NULL;
+static char * provider_err = NULL;
 
 static bool provider_invoke(NS ns_id, const char *name, LuaRef ref,
                             Array args, bool default_true)
@@ -187,10 +187,10 @@ static bool provider_invoke(NS ns_id, const char *name, LuaRef ref,
     const char *ns_name = describe_ns(ns_id);
     ELOG("error in provider %s:%s: %s", ns_name, name, err.msg);
     bool verbose_errs = true;  // TODO(bfredl):
-    if (verbose_errs && provider_first_error == NULL) {
+    if (verbose_errs && provider_err == NULL) {
       static char errbuf[IOSIZE];
       snprintf(errbuf, sizeof errbuf, "%s: %s", ns_name, err.msg);
-      provider_first_error = xstrdup(errbuf);
+      provider_err = xstrdup(errbuf);
     }
   }
 
@@ -2103,7 +2103,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
 
   bool search_attr_from_match = false;  // if search_attr is from :match
   bool has_decor = false;               // this buffer has decoration
-  bool do_virttext = false;             // draw virtual text for this line
   int win_col_offset = 0;               // offset for window columns
 
   char_u buf_fold[FOLD_TEXT_LEN + 1];   // Hold value returned by get_foldtext
@@ -2148,8 +2147,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
 
   row = startrow;
 
-  char *err_text = NULL;
-
   buf_T *buf = wp->w_buffer;
 
   if (!number_only) {
@@ -2194,14 +2191,20 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
       }
     }
 
-    if (has_decor) {
-      extra_check = true;
+    if (provider_err) {
+      Decoration err_decor = DECORATION_INIT;
+      int hl_err = syn_check_group((char_u *)S_LEN("ErrorMsg"));
+      kv_push(err_decor.virt_text,
+              ((VirtTextChunk){ .text = provider_err,
+                                .hl_id = hl_err }));
+      err_decor.virt_text_width = mb_string2cells((char_u *)provider_err);
+      decor_add_ephemeral(lnum-1, 0, lnum-1, 0, &err_decor);
+      provider_err = NULL;
+      has_decor = true;
     }
 
-    if (provider_first_error) {
-      err_text = provider_first_error;
-      provider_first_error = NULL;
-      do_virttext = true;
+    if (has_decor) {
+      extra_check = true;
     }
 
     // Check for columns to display for 'colorcolumn'.
@@ -3953,19 +3956,15 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
       if (draw_color_col)
         draw_color_col = advance_color_col(VCOL_HLC, &color_cols);
 
-      VirtText virt_text = KV_INITIAL_VALUE;
-      bool has_aligned = false;
-      if (err_text) {
-        int hl_err = syn_check_group((char_u *)S_LEN("ErrorMsg"));
-        kv_push(virt_text, ((VirtTextChunk){ .text = err_text,
-                                             .hl_id = hl_err }));
-        do_virttext = true;
-      } else if (has_decor) {
-        virt_text = decor_redraw_eol(wp->w_buffer, &decor_state, &line_attr,
-                                     &has_aligned);
-        if (kv_size(virt_text)) {
-          do_virttext = true;
-        }
+      bool has_virttext = false;
+      // Make sure alignment is the same regardless
+      // if listchars=eol:X is used or not.
+      int eol_skip = (wp->w_p_lcs_chars.eol == lcs_eol_one && eol_hl_off == 0
+                      ? 1 : 0);
+
+      if (has_decor) {
+        has_virttext = decor_redraw_eol(wp->w_buffer, &decor_state, &line_attr,
+                                        col + eol_skip);
       }
 
       if (((wp->w_p_cuc
@@ -3974,20 +3973,10 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
             grid->Columns * (row - startrow + 1) + v
             && lnum != wp->w_cursor.lnum)
            || draw_color_col || line_attr_lowprio || line_attr
-           || diff_hlf != (hlf_T)0 || do_virttext
-           || has_aligned)) {
+           || diff_hlf != (hlf_T)0 || has_virttext)) {
         int rightmost_vcol = 0;
         int i;
 
-        size_t virt_pos = 0;
-        LineState s = LINE_STATE("");
-        int virt_attr = 0;
-
-        // Make sure alignment is the same regardless
-        // if listchars=eol:X is used or not.
-        bool delay_virttext = wp->w_p_lcs_chars.eol == lcs_eol_one
-                              && eol_hl_off == 0;
-
         if (wp->w_p_cuc) {
           rightmost_vcol = wp->w_virtcol;
         }
@@ -4013,38 +4002,15 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
         }
 
         int base_attr = hl_combine_attr(line_attr_lowprio, diff_attr);
-        if (base_attr || line_attr || has_aligned) {
+        if (base_attr || line_attr || has_virttext) {
           rightmost_vcol = INT_MAX;
         }
 
         int col_stride = wp->w_p_rl ? -1 : 1;
 
         while (wp->w_p_rl ? col >= 0 : col < grid->Columns) {
-          int cells = -1;
-          // TODO: integrate this whith the other virt_text impl already
-          if (do_virttext && !delay_virttext) {
-            if (*s.p == NUL) {
-              if (virt_pos < virt_text.size) {
-                s.p = kv_A(virt_text, virt_pos).text;
-                int hl_id = kv_A(virt_text, virt_pos).hl_id;
-                virt_attr = hl_id > 0 ? syn_id2attr(hl_id) : 0;
-                virt_pos++;
-              } else {
-               do_virttext = false;
-              }
-            }
-            if (*s.p != NUL) {
-              cells = line_putchar(&s, &linebuf_char[off], grid->Columns - col,
-                                   false);
-            }
-          }
-          delay_virttext = false;
-
-          if (cells == -1) {
-            schar_from_ascii(linebuf_char[off], ' ');
-            cells = 1;
-          }
-          col += cells * col_stride;
+          schar_from_ascii(linebuf_char[off], ' ');
+          col += col_stride;
           if (draw_color_col) {
             draw_color_col = advance_color_col(VCOL_HLC, &color_cols);
           }
@@ -4057,24 +4023,16 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
             col_attr = mc_attr;
           }
 
-          if (do_virttext) {
-            col_attr = hl_combine_attr(col_attr, virt_attr);
-          }
-
           col_attr = hl_combine_attr(col_attr, line_attr);
 
           linebuf_attr[off] = col_attr;
-          if (cells == 2) {
-            linebuf_attr[off+1] = col_attr;
-          }
-          off += cells * col_stride;
+          off += col_stride;
 
-          if (VCOL_HLC >= rightmost_vcol && *s.p == NUL
-              && virt_pos >= virt_text.size) {
+          if (VCOL_HLC >= rightmost_vcol) {
             break;
           }
 
-          vcol += cells;
+          vcol += 1;
         }
       }
 
@@ -4393,7 +4351,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow,
   }
 
   xfree(p_extra_free);
-  xfree(err_text);
   return row;
 }
 
@@ -4401,13 +4358,17 @@ void draw_virt_text(buf_T *buf, int col_off, int *end_col, int max_col)
 {
   DecorState *state = &decor_state;
   int right_pos = max_col;
+  bool do_eol = state->eol_col > -1;
   for (size_t i = 0; i < kv_size(state->active); i++) {
     DecorRange *item = &kv_A(state->active, i);
     if (item->start_row == state->row && kv_size(item->decor.virt_text)) {
       if (item->win_col == -1) {
         if (item->decor.virt_text_pos == kVTRightAlign) {
-          right_pos -= item->decor.col;
+          right_pos -= item->decor.virt_text_width;
           item->win_col = right_pos;
+        } else if (item->decor.virt_text_pos == kVTEndOfLine && do_eol) {
+          item->win_col = state->eol_col;
+          state->eol_col += item->decor.virt_text_width;
         } else if (item->decor.virt_text_pos == kVTWinCol) {
           item->win_col = MAX(item->decor.col+col_off, 0);
         }
@@ -4436,7 +4397,6 @@ void draw_virt_text(buf_T *buf, int col_off, int *end_col, int max_col)
                                         hl_id > 0 ? syn_id2attr(hl_id) : 0);
             virt_pos++;
           } while (!s.p && virt_pos < kv_size(vt));
-          // TODO: y w0t m8
           if (!s.p) {
             break;
           }
diff --git a/test/functional/ui/bufhl_spec.lua b/test/functional/ui/bufhl_spec.lua
index af709cd521..16ed3b9486 100644
--- a/test/functional/ui/bufhl_spec.lua
+++ b/test/functional/ui/bufhl_spec.lua
@@ -858,8 +858,8 @@ describe('Buffer highlighting', function()
     it('works with cursorline', function()
       command("set cursorline")
 
-      screen:expect([[
-        {14:^1 + 2 }{15:=}{16: 3}{14:                               }|
+      screen:expect{grid=[[
+        {14:^1 + 2 }{3:=}{2: 3}{14:                               }|
         3 + {11:ERROR:} invalid syntax               |
         5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5|
         , 5, 5, 5, 5, 5, 5,  Lorem ipsum dolor s|
@@ -867,32 +867,32 @@ describe('Buffer highlighting', function()
         {1:~                                       }|
         {1:~                                       }|
                                                 |
-      ]])
+      ]]}
 
       feed('j')
-      screen:expect([[
+      screen:expect{grid=[[
         1 + 2 {3:=}{2: 3}                               |
-        {14:^3 + }{11:ERROR:}{14: invalid syntax               }|
+        {14:^3 + }{11:ERROR:} invalid syntax{14:               }|
         5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5|
         , 5, 5, 5, 5, 5, 5,  Lorem ipsum dolor s|
         x = 4                                   |
         {1:~                                       }|
         {1:~                                       }|
                                                 |
-      ]])
+      ]]}
 
 
       feed('j')
-      screen:expect([[
+      screen:expect{grid=[[
         1 + 2 {3:=}{2: 3}                               |
         3 + {11:ERROR:} invalid syntax               |
         {14:^5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5}|
-        {14:, 5, 5, 5, 5, 5, 5,  Lorem ipsum dolor s}|
+        {14:, 5, 5, 5, 5, 5, 5,  }Lorem ipsum dolor s|
         x = 4                                   |
         {1:~                                       }|
         {1:~                                       }|
                                                 |
-      ]])
+      ]]}
     end)
 
     it('works with color column', function()
@@ -910,11 +910,11 @@ describe('Buffer highlighting', function()
 
       command("set colorcolumn=9")
       screen:expect{grid=[[
-        ^1 + 2 {3:=}{2: }{17:3}                               |
+        ^1 + 2 {3:=}{2: 3}                               |
         3 + {11:ERROR:} invalid syntax               |
         5, 5, 5,{18: }5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5|
         , 5, 5, 5, 5, 5, 5,  Lorem ipsum dolor s|
-        x = 4 {12:暗}{19:x}{12:事}                             |
+        x = 4 {12:暗x事}                             |
         {1:~                                       }|
         {1:~                                       }|
                                                 |
diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua
index 96d9eab065..4373d17890 100644
--- a/test/functional/ui/decorations_spec.lua
+++ b/test/functional/ui/decorations_spec.lua
@@ -562,7 +562,7 @@ end]]
           {5:l}{8:blen}{7:dy}{10:e}{7:text}{10:h}{7:-}{10:_}{7:here}ell, count = unpack(item)  |
           {5:i}{12:c}{11:ombining color} {13:nil} {5:then}                     |
            {11:replacing color}d_cell                        |
-          {5:e}{8:bl}{14:endy}{15:i}{14:text}{15:o}{14:-}{15:o}{14:h}{7:ere}                           |
+          {5:e}{8:bl}{7:endy}{10: }{7:text}{10: }{7:-}{10: }{7:here}                           |
           {5:f}{12:co}{11:mbini}{16:n}{11:g color}t {5:or} {13:1}) {5:do}                    |
            {11:replacing color} line[colpos]                 |
               cell.text = text                          |