feat(decoration): allow conceal_char to be a composing char

decor->text.str pointer must go. This removes it for conceal char,
in preparation for a larger PR which will also handle the sign case.

By actually allowing composing chars for a conceal chars, this
becomes a feature and not just a refactor, as a bonus.

runtime/doc/api.txt

@@ -645,6 +645,10 @@ nvim__inspect_cell({grid}, {row}, {col})                *nvim__inspect_cell()*
     NB: if your UI doesn't use hlstate, this will not return hlstate first
     time.
 
+nvim__invalidate_glyph_cache()                *nvim__invalidate_glyph_cache()*
+    For testing. The condition in schar_cache_clear_if_full is hard to reach,
+    so this function can be used to force a cache clear in a test.
+
 nvim__stats()                                                  *nvim__stats()*
     Gets internal stats.
 

runtime/lua/vim/_meta/api.lua

@@ -80,6 +80,9 @@ function vim.api.nvim__id_float(flt) end
 function vim.api.nvim__inspect_cell(grid, row, col) end
 
 --- @private
+--- For testing. The condition in schar_cache_clear_if_full is hard to reach,
+--- so this function can be used to force a cache clear in a test.
+---
 function vim.api.nvim__invalidate_glyph_cache() end
 
 --- @private

src/nvim/api/extmark.c

@@ -18,6 +18,7 @@
 #include "nvim/drawscreen.h"
 #include "nvim/extmark.h"
 #include "nvim/func_attr.h"
+#include "nvim/grid.h"
 #include "nvim/highlight_group.h"
 #include "nvim/marktree.h"
 #include "nvim/mbyte.h"
@@ -503,7 +504,6 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, Integer line, Integer
   DecorVirtText virt_text = DECOR_VIRT_TEXT_INIT;
   DecorVirtText virt_lines = DECOR_VIRT_LINES_INIT;
   bool has_hl = false;
-  String conceal_char_large = STRING_INIT;
 
   buf_T *buf = find_buffer_by_handle(buffer, err);
   if (!buf) {
@@ -593,10 +593,11 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, Integer line, Integer
     has_hl = true;
     String c = opts->conceal;
     if (c.size > 0) {
-      if (c.size <= 4) {
+      int ch;
-        memcpy(hl.conceal_char, c.data, c.size + (c.size < 4 ? 1 : 0));
+      hl.conceal_char = utfc_ptr2schar_len(c.data, (int)c.size, &ch);
-      } else {
+      if (!hl.conceal_char || !vim_isprintc(ch)) {
-        conceal_char_large = c;
+        api_set_error(err, kErrorTypeValidation, "conceal char has to be printable");
+        goto error;
       }
     }
   }
@@ -777,7 +778,7 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, Integer line, Integer
       decor_range_add_virt(&decor_state, r, c, line2, col2, decor_put_vt(virt_lines, NULL), true);
     }
     if (has_hl) {
-      DecorSignHighlight sh = decor_sh_from_inline(hl, conceal_char_large);
+      DecorSignHighlight sh = decor_sh_from_inline(hl);
       decor_range_add_sh(&decor_state, r, c, line2, col2, &sh, true, (uint32_t)ns_id, id);
     }
     if (sign.flags & kSHIsSign) {
@@ -815,9 +816,9 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, Integer line, Integer
     }
 
     DecorInline decor = DECOR_INLINE_INIT;
-    if (decor_alloc || decor_indexed != DECOR_ID_INVALID || conceal_char_large.size) {
+    if (decor_alloc || decor_indexed != DECOR_ID_INVALID || schar_high(hl.conceal_char)) {
       if (has_hl) {
-        DecorSignHighlight sh = decor_sh_from_inline(hl, conceal_char_large);
+        DecorSignHighlight sh = decor_sh_from_inline(hl);
         sh.next = decor_indexed;
         decor_indexed = decor_put_sh(sh);
       }

src/nvim/api/vim.c

@@ -1999,9 +1999,12 @@ void nvim__screenshot(String path)
   ui_call_screenshot(path);
 }
 
+/// For testing. The condition in schar_cache_clear_if_full is hard to
+/// reach, so this function can be used to force a cache clear in a test.
 void nvim__invalidate_glyph_cache(void)
 {
-  schar_cache_clear_force();
+  schar_cache_clear();
+  must_redraw = UPD_CLEAR;
 }
 
 Object nvim__unpack(String str, Error *err)

src/nvim/decoration.c

@@ -13,6 +13,7 @@
 #include "nvim/drawscreen.h"
 #include "nvim/extmark.h"
 #include "nvim/fold.h"
+#include "nvim/grid.h"
 #include "nvim/highlight.h"
 #include "nvim/highlight_group.h"
 #include "nvim/mbyte.h"
@@ -115,7 +116,7 @@ void decor_redraw(buf_T *buf, int row1, int row2, DecorInline decor)
       idx = sh->next;
     }
   } else {
-    decor_redraw_sh(buf, row1, row2, decor_sh_from_inline(decor.data.hl, (String)STRING_INIT));
+    decor_redraw_sh(buf, row1, row2, decor_sh_from_inline(decor.data.hl));
   }
 }
 
@@ -153,14 +154,14 @@ DecorVirtText *decor_put_vt(DecorVirtText vt, DecorVirtText *next)
   return decor_alloc;
 }
 
-DecorSignHighlight decor_sh_from_inline(DecorHighlightInline item, String conceal_large)
+DecorSignHighlight decor_sh_from_inline(DecorHighlightInline item)
 {
   // TODO(bfredl): Eventually simple signs will be inlinable as well
   assert(!(item.flags & kSHIsSign));
   DecorSignHighlight conv = {
     .flags = item.flags,
     .priority = item.priority,
-    .text.data = { 0 },
+    .text.sc[0] = item.conceal_char,
     .hl_id = item.hl_id,
     .number_hl_id = 0,
     .line_hl_id = 0,
@@ -168,19 +169,6 @@ DecorSignHighlight decor_sh_from_inline(DecorHighlightInline item, String concea
     .next = DECOR_ID_INVALID,
   };
 
-  // TODO(bfredl): 'tis a little bullshit. Won't need it once conceals and signs to use schar_T
-  if (conceal_large.size) {
-    String c = conceal_large;
-    if (c.size <= 8) {
-      memcpy(conv.text.data, c.data, c.size + (c.size < 8 ? 1 : 0));
-    } else {
-      conv.flags |= kSHConcealAlloc;
-      conv.text.ptr = xstrdup(conceal_large.data);
-    }
-  } else {
-    memcpy(conv.text.data, item.conceal_char, 4);
-    conv.text.data[4] = NUL;
-  }
   return conv;
 }
 
@@ -326,12 +314,13 @@ void decor_free_inner(DecorVirtText *vt, uint32_t first_idx)
   uint32_t idx = first_idx;
   while (idx != DECOR_ID_INVALID) {
     DecorSignHighlight *sh = &kv_A(decor_items, idx);
-    if (sh->flags & (kSHIsSign | kSHConcealAlloc)) {
+    if (sh->flags & kSHIsSign) {
       xfree(sh->text.ptr);
     }
     if (sh->flags & kSHIsSign) {
       xfree(sh->sign_name);
     }
+    sh->flags = 0;
     if (sh->next == DECOR_ID_INVALID) {
       sh->next = decor_freelist;
       decor_freelist = first_idx;
@@ -372,6 +361,16 @@ void clear_virtlines(VirtLines *lines)
   *lines = (VirtLines)KV_INITIAL_VALUE;
 }
 
+void decor_check_invalid_glyphs(void)
+{
+  for (size_t i = 0; i < kv_size(decor_items); i++) {
+    DecorSignHighlight *it = &kv_A(decor_items, i);
+    if ((it->flags & kSHConceal) && schar_high(it->text.sc[0])) {
+      it->text.sc[0] = schar_from_char(schar_get_first_codepoint(it->text.sc[0]));
+    }
+  }
+}
+
 /// Get the next chunk of a virtual text item.
 ///
 /// @param[in]     vt    The virtual text item
@@ -503,7 +502,7 @@ static void decor_range_add_from_inline(DecorState *state, int start_row, int st
       idx = sh->next;
     }
   } else {
-    DecorSignHighlight sh = decor_sh_from_inline(decor.data.hl, (String)STRING_INIT);
+    DecorSignHighlight sh = decor_sh_from_inline(decor.data.hl);
     decor_range_add_sh(state, start_row, start_col, end_row, end_col, &sh, owned, ns, mark_id);
   }
 }
@@ -628,7 +627,7 @@ next_mark:
   int attr = 0;
   size_t j = 0;
   int conceal = 0;
-  int conceal_char = 0;
+  schar_T conceal_char = 0;
   int conceal_attr = 0;
   TriState spell = kNone;
 
@@ -661,10 +660,7 @@ next_mark:
       if (item.start_row == state->row && item.start_col == col) {
         DecorSignHighlight *sh = &item.data.sh;
         conceal = 2;
-        char *text = (sh->flags & kSHConcealAlloc) ? sh->text.ptr : sh->text.data;
+        conceal_char = sh->text.sc[0];
-        // TODO(bfredl): kSHConcealAlloc is obviously a waste unless we change
-        // `conceal_char` to schar_T
-        conceal_char = utf_ptr2char(text);
         state->col_until = MIN(state->col_until, item.start_col);
         conceal_attr = item.attr_id;
       }
@@ -964,20 +960,16 @@ void decor_to_dict_legacy(Dictionary *dict, DecorInline decor, bool hl_name)
       idx = sh->next;
     }
   } else {
-    sh_hl = decor_sh_from_inline(decor.data.hl, (String)STRING_INIT);
+    sh_hl = decor_sh_from_inline(decor.data.hl);
   }
 
   if (sh_hl.hl_id) {
     PUT(*dict, "hl_group", hl_group_name(sh_hl.hl_id, hl_name));
     PUT(*dict, "hl_eol", BOOLEAN_OBJ(sh_hl.flags & kSHHlEol));
     if (sh_hl.flags & kSHConceal) {
-      String name;
+      char buf[MAX_SCHAR_SIZE];
-      if (sh_hl.flags & kSHConcealAlloc) {
+      schar_get(buf, sh_hl.text.sc[0]);
-        name = cstr_to_string(sh_hl.text.ptr);
+      PUT(*dict, "conceal", CSTR_TO_OBJ(buf));
-      } else {
-        name = cbuf_to_string(sh_hl.text.data, strnlen(sh_hl.text.data, 8));
-      }
-      PUT(*dict, "conceal", STRING_OBJ(name));
     }
 
     if (sh_hl.flags & kSHSpellOn) {

src/nvim/decoration.h

@@ -67,7 +67,7 @@ typedef struct {
   int eol_col;
 
   int conceal;
-  int conceal_char;
+  schar_T conceal_char;
   int conceal_attr;
 
   TriState spell;

src/nvim/decoration_defs.h

@@ -3,6 +3,7 @@
 #include <stdint.h>
 
 #include "klib/kvec.h"
+#include "nvim/types_defs.h"
 
 #define DECOR_ID_INVALID UINT32_MAX
 
@@ -42,29 +43,24 @@ enum {
   kSHSpellOn = 16,
   kSHSpellOff = 32,
   kSHConceal = 64,
-  kSHConcealAlloc = 128,
 };
 
 typedef struct {
   uint16_t flags;
   DecorPriority priority;
   int hl_id;
-  char conceal_char[4];
+  schar_T conceal_char;
 } DecorHighlightInline;
 
-#define DECOR_HIGHLIGHT_INLINE_INIT { 0, DECOR_PRIORITY_BASE, 0, { 0 } }
+#define DECOR_HIGHLIGHT_INLINE_INIT { 0, DECOR_PRIORITY_BASE, 0,  0 }
 typedef struct {
   uint16_t flags;
   DecorPriority priority;
   int hl_id;  // if sign: highlight of sign text
-  // TODO(bfredl): Later this should be schar_T[2], but then it needs to handle
+  // TODO(bfredl): Later signs should use sc[2] as well.
-  // invalidations of the cache
   union {
-    // for now:
+    char *ptr;  // sign
-    // 1. sign is always allocated (drawline.c expects a `char *` for a sign)
+    schar_T sc[2];  // conceal text (only sc[0] used)
-    // 2. conceal char is allocated if larger than 8 bytes.
-    char *ptr;  // sign or conceal text
-    char data[8];
   } text;
   // NOTE: if more functionality is added to a Highlight these should be overloaded
   // or restructured

src/nvim/drawline.c

@@ -2459,6 +2459,7 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool number_onl
           && ((syntax_flags & HL_CONCEAL) != 0 || has_match_conc > 0 || decor_conceal > 0)
           && !(lnum_in_visual_area && vim_strchr(wp->w_p_cocu, 'v') == NULL)) {
         wlv.char_attr = conceal_attr;
+        bool is_conceal_char = false;
         if (((prev_syntax_id != syntax_seqnr && (syntax_flags & HL_CONCEAL) != 0)
              || has_match_conc > 1 || decor_conceal > 1)
             && (syn_get_sub_char() != NUL
@@ -2471,7 +2472,9 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool number_onl
           if (has_match_conc && match_conc) {
             mb_c = match_conc;
           } else if (decor_conceal && decor_state.conceal_char) {
-            mb_c = decor_state.conceal_char;
+            mb_schar = decor_state.conceal_char;
+            mb_c = schar_get_first_codepoint(mb_schar);
+            is_conceal_char = true;
             if (decor_state.conceal_attr) {
               wlv.char_attr = decor_state.conceal_attr;
             }
@@ -2499,7 +2502,9 @@ int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool number_onl
           is_concealing = true;
           wlv.skip_cells = 1;
         }
-        mb_schar = schar_from_char(mb_c);
+        if (!is_conceal_char) {
+          mb_schar = schar_from_char(mb_c);
+        }
       } else {
         prev_syntax_id = 0;
         is_concealing = false;

src/nvim/grid.c

@@ -18,7 +18,7 @@
 #include "nvim/arabic.h"
 #include "nvim/ascii.h"
 #include "nvim/buffer_defs.h"
-#include "nvim/drawscreen.h"
+#include "nvim/decoration.h"
 #include "nvim/globals.h"
 #include "nvim/grid.h"
 #include "nvim/highlight.h"
@@ -111,18 +111,16 @@ bool schar_cache_clear_if_full(void)
   // note: critical max is really (1<<24)-1. This gives us some marginal
   // until next time update_screen() is called
   if (glyph_cache.h.n_keys > (1<<21)) {
-    set_clear(glyph, &glyph_cache);
+    schar_cache_clear();
     return true;
   }
   return false;
 }
 
-/// For testing. The condition in schar_cache_clear_force is hard to
+void schar_cache_clear(void)
-/// reach, so this function can be used to force a cache clear in a test.
-void schar_cache_clear_force(void)
 {
+  decor_check_invalid_glyphs();
   set_clear(glyph, &glyph_cache);
-  must_redraw = UPD_CLEAR;
 }
 
 bool schar_high(schar_T sc)
@@ -159,6 +157,13 @@ static char schar_get_first_byte(schar_T sc)
   return schar_high(sc) ? glyph_cache.keys[schar_idx(sc)] : *(char *)&sc;
 }
 
+int schar_get_first_codepoint(schar_T sc)
+{
+  char sc_buf[MAX_SCHAR_SIZE];
+  schar_get(sc_buf, sc);
+  return utf_ptr2char(sc_buf);
+}
+
 /// @return ascii char or NUL if not ascii
 char schar_get_ascii(schar_T sc)
 {

src/nvim/grid_defs.h

@@ -11,14 +11,6 @@
 // ensures we can fit all composed chars which did fit before.
 #define MAX_SCHAR_SIZE 32
 
-// if data[0] is 0xFF, then data[1..4] is a 24-bit index (in machine endianness)
-// otherwise it must be a UTF-8 string of length maximum 4 (no NUL when n=4)
-
-typedef uint32_t schar_T;
-typedef int32_t sattr_T;
-// must be at least as big as the biggest of schar_T, sattr_T, col_T
-typedef int32_t sscratch_T;
-
 enum {
   kZIndexDefaultGrid = 0,
   kZIndexFloatDefault = 50,

src/nvim/types_defs.h

@@ -6,8 +6,12 @@
 // dummy to pass an ACL to a function
 typedef void *vim_acl_T;
 
-// Can hold one decoded UTF-8 character.
+// if data[0] is 0xFF, then data[1..4] is a 24-bit index (in machine endianness)
-typedef uint32_t u8char_T;
+// otherwise it must be a UTF-8 string of length maximum 4 (no NUL when n=4)
+typedef uint32_t schar_T;
+typedef int32_t sattr_T;
+// must be at least as big as the biggest of schar_T, sattr_T, colnr_T
+typedef int32_t sscratch_T;
 
 // Opaque handle used by API clients to refer to various objects in vim
 typedef int handle_T;

test/functional/ui/decorations_spec.lua

@@ -13,6 +13,7 @@ local curbufmeths = helpers.curbufmeths
 local command = helpers.command
 local eq = helpers.eq
 local assert_alive = helpers.assert_alive
+local pcall_err = helpers.pcall_err
 
 describe('decorations providers', function()
   local screen
@@ -1650,6 +1651,34 @@ describe('extmark decorations', function()
     ]])
     command('set conceallevel=1')
     screen:expect_unchanged()
+
+    eq("conceal char has to be printable", pcall_err(meths.buf_set_extmark, 0, ns, 0, 0, {end_col=0, end_row=2, conceal='\255'}))
+  end)
+
+  it('conceal with composed conceal char', function()
+    screen:try_resize(50, 5)
+    insert('foo\n')
+    meths.buf_set_extmark(0, ns, 0, 0, {end_col=0, end_row=2, conceal='ẍ̲'})
+    command('set conceallevel=2')
+    screen:expect([[
+      {26:ẍ̲}                                                 |
+      ^                                                  |
+      {1:~                                                 }|
+      {1:~                                                 }|
+                                                        |
+    ]])
+    command('set conceallevel=1')
+    screen:expect_unchanged()
+
+    -- this is rare, but could happen. Save at least the first codepoint
+    meths._invalidate_glyph_cache()
+    screen:expect{grid=[[
+      {26:x}                                                 |
+      ^                                                  |
+      {1:~                                                 }|
+      {1:~                                                 }|
+                                                        |
+    ]]}
   end)
 
   it('conceal without conceal char #24782', function()