vim-patch:9.1.1011: popupmenu internal error with some abbr in completion item (#31988)

Problem:  Popup menu internal error with some abbr in completion item.
Solution: Don't compute attributes when there is no corresponding text.
          Reduce indent in pum_redraw() while at it (zeertzjq).

fixes: vim/vim#16427
closes: vim/vim#16435

https://github.com/vim/vim/commit/3a0cc36c69744a7727ce34311d39d2d9d8ddc6f9

3 files changed, 140 insertions, 70 deletions

diff --git a/src/nvim/popupmenu.c b/src/nvim/popupmenu.c
index 2b1dd22b1a..d1c6f647fd 100644
--- a/src/nvim/popupmenu.c
+++ b/src/nvim/popupmenu.c
@@ -412,7 +412,7 @@ void pum_display(pumitem_T *array, int size, int selected, bool array_changed, i
 /// Returns attributes for every cell, or NULL if all attributes are the same.
 static int *pum_compute_text_attrs(char *text, hlf_T hlf, int user_hlattr)
 {
-  if ((hlf != HLF_PSI && hlf != HLF_PNI)
+  if (*text == NUL || (hlf != HLF_PSI && hlf != HLF_PNI)
       || (win_hl_attr(curwin, HLF_PMSI) == win_hl_attr(curwin, HLF_PSI)
           && win_hl_attr(curwin, HLF_PMNI) == win_hl_attr(curwin, HLF_PNI))) {
     return NULL;
@@ -654,89 +654,87 @@ void pum_redraw(void)
             s = p;
           }
           int w = ptr2cells(p);
+          if (*p != NUL && *p != TAB && totwidth + w <= pum_width) {
+            width += w;
+            continue;
+          }
 
-          if ((*p == NUL) || (*p == TAB) || (totwidth + w > pum_width)) {
-            // Display the text that fits or comes before a Tab.
-            // First convert it to printable characters.
-            char *st;
-            char saved = *p;
+          // Display the text that fits or comes before a Tab.
+          // First convert it to printable characters.
+          char saved = *p;
 
-            if (saved != NUL) {
-              *p = NUL;
-            }
-            st = transstr(s, true);
-            if (saved != NUL) {
-              *p = saved;
-            }
-
-            int *attrs = NULL;
-            if (item_type == CPT_ABBR) {
-              attrs = pum_compute_text_attrs(st, hlf, pum_array[idx].pum_user_abbr_hlattr);
-            }
+          if (saved != NUL) {
+            *p = NUL;
+          }
+          char *st = transstr(s, true);
+          if (saved != NUL) {
+            *p = saved;
+          }
 
-            if (pum_rl) {
-              char *rt = reverse_text(st);
-              char *rt_start = rt;
-              int cells = vim_strsize(rt);
-
-              if (cells > pum_width) {
-                do {
-                  cells -= utf_ptr2cells(rt);
-                  MB_PTR_ADV(rt);
-                } while (cells > pum_width);
-
-                if (cells < pum_width) {
-                  // Most left character requires 2-cells but only 1 cell
-                  // is available on screen.  Put a '<' on the left of the
-                  // pum item
-                  *(--rt) = '<';
-                  cells++;
-                }
-              }
+          int *attrs = NULL;
+          if (item_type == CPT_ABBR) {
+            attrs = pum_compute_text_attrs(st, hlf,
+                                           pum_array[idx].pum_user_abbr_hlattr);
+          }
 
-              if (attrs == NULL) {
-                grid_line_puts(grid_col - cells + 1, rt, -1, attr);
-              } else {
-                pum_grid_puts_with_attrs(grid_col - cells + 1, cells, rt, -1, attrs);
+          if (pum_rl) {
+            char *rt = reverse_text(st);
+            char *rt_start = rt;
+            int cells = vim_strsize(rt);
+
+            if (cells > pum_width) {
+              do {
+                cells -= utf_ptr2cells(rt);
+                MB_PTR_ADV(rt);
+              } while (cells > pum_width);
+
+              if (cells < pum_width) {
+                // Most left character requires 2-cells but only 1 cell is available on
+                // screen.  Put a '<' on the left of the pum item.
+                *(--rt) = '<';
+                cells++;
               }
+            }
 
-              xfree(rt_start);
-              xfree(st);
-              grid_col -= width;
+            if (attrs == NULL) {
+              grid_line_puts(grid_col - cells + 1, rt, -1, attr);
             } else {
-              if (attrs == NULL) {
-                grid_line_puts(grid_col, st, -1, attr);
-              } else {
-                pum_grid_puts_with_attrs(grid_col, vim_strsize(st), st, -1, attrs);
-              }
-
-              xfree(st);
-              grid_col += width;
+              pum_grid_puts_with_attrs(grid_col - cells + 1, cells, rt, -1, attrs);
             }
 
-            if (attrs != NULL) {
-              XFREE_CLEAR(attrs);
+            xfree(rt_start);
+            xfree(st);
+            grid_col -= width;
+          } else {
+            if (attrs == NULL) {
+              grid_line_puts(grid_col, st, -1, attr);
+            } else {
+              pum_grid_puts_with_attrs(grid_col, vim_strsize(st), st, -1, attrs);
             }
 
-            if (*p != TAB) {
-              break;
-            }
+            xfree(st);
+            grid_col += width;
+          }
 
-            // Display two spaces for a Tab.
-            if (pum_rl) {
-              grid_line_puts(grid_col - 1, "  ", 2, attr);
-              grid_col -= 2;
-            } else {
-              grid_line_puts(grid_col, "  ", 2, attr);
-              grid_col += 2;
-            }
-            totwidth += 2;
-            // start text at next char
-            s = NULL;
-            width = 0;
+          if (attrs != NULL) {
+            XFREE_CLEAR(attrs);
+          }
+
+          if (*p != TAB) {
+            break;
+          }
+
+          // Display two spaces for a Tab.
+          if (pum_rl) {
+            grid_line_puts(grid_col - 1, "  ", 2, attr);
+            grid_col -= 2;
           } else {
-            width += w;
+            grid_line_puts(grid_col, "  ", 2, attr);
+            grid_col += 2;
           }
+          totwidth += 2;
+          s = NULL;  // start text at next char
+          width = 0;
         }
       }
 
diff --git a/test/functional/ui/popupmenu_spec.lua b/test/functional/ui/popupmenu_spec.lua
index b763f4ba6c..5e883d1a92 100644
--- a/test/functional/ui/popupmenu_spec.lua
+++ b/test/functional/ui/popupmenu_spec.lua
@@ -5415,6 +5415,45 @@ describe('builtin popupmenu', function()
         feed('<C-E><Esc>')
       end)
 
+      -- oldtest: Test_pum_highlights_match_with_abbr()
+      it('can highlight matched text with abbr', function()
+        exec([[
+          func Omni_test(findstart, base)
+            if a:findstart
+              return col(".")
+            endif
+            return {
+                  \ 'words': [
+                  \ { 'word': 'foobar', 'abbr': "foobar\t\t!" },
+                  \ { 'word': 'foobaz', 'abbr': "foobaz\t\t!" },
+                  \]}
+          endfunc
+
+          set omnifunc=Omni_test
+          set completeopt=menuone,noinsert
+          hi PmenuMatchSel  guifg=Blue guibg=Grey
+          hi PmenuMatch     guifg=Blue guibg=Plum1
+        ]])
+        feed('i<C-X><C-O>')
+        screen:expect([[
+          ^                                |
+          {s:foobar    !    }{1:                 }|
+          {n:foobaz    !    }{1:                 }|
+          {1:~                               }|*16
+          {2:-- }{5:match 1 of 2}                 |
+        ]])
+        feed('foo')
+        screen:expect([[
+          foo^                             |
+          {ms:foo}{s:bar    !    }{1:                 }|
+          {mn:foo}{n:baz    !    }{1:                 }|
+          {1:~                               }|*16
+          {2:-- }{5:match 1 of 2}                 |
+        ]])
+
+        feed('<C-E><Esc>')
+      end)
+
       -- oldtest: Test_pum_user_abbr_hlgroup()
       it('custom abbr_hlgroup override', function()
         exec([[
diff --git a/test/old/testdir/test_popup.vim b/test/old/testdir/test_popup.vim
index 31cfc2d096..e4abf978ab 100644
--- a/test/old/testdir/test_popup.vim
+++ b/test/old/testdir/test_popup.vim
@@ -1519,6 +1519,39 @@ func Test_pum_highlights_match()
   call StopVimInTerminal(buf)
 endfunc
 
+func Test_pum_highlights_match_with_abbr()
+  CheckScreendump
+  let lines =<< trim END
+    func Omni_test(findstart, base)
+      if a:findstart
+        return col(".")
+      endif
+      return {
+            \ 'words': [
+            \ { 'word': 'foobar', 'abbr': "foobar\t\t!" },
+            \ { 'word': 'foobaz', 'abbr': "foobaz\t\t!" },
+            \]}
+    endfunc
+
+    set omnifunc=Omni_test
+    set completeopt=menuone,noinsert
+    hi PmenuMatchSel  ctermfg=6 ctermbg=7
+    hi PmenuMatch     ctermfg=4 ctermbg=225
+  END
+  call writefile(lines, 'Xscript', 'D')
+  let  buf = RunVimInTerminal('-S Xscript', {})
+  call TermWait(buf)
+  call term_sendkeys(buf, "i\<C-X>\<C-O>")
+  call TermWait(buf, 50)
+  call term_sendkeys(buf, "foo")
+  call VerifyScreenDump(buf, 'Test_pum_highlights_19', {})
+
+  call term_sendkeys(buf, "\<C-E>\<Esc>")
+  call TermWait(buf)
+
+  call StopVimInTerminal(buf)
+endfunc
+
 func Test_pum_user_abbr_hlgroup()
   CheckScreendump
   let lines =<< trim END