feat(completion): add two highlight groups PmenuMatch and PmenuMatchSel

[glepnir]

Add two highlights group PmenuMatch and PmenuMatchSel for the leader matched position in candidate words.

[chrisbra]

Thanks, can you explain what those are for? Also should be documented at :h highlight-groups

[techntools]

Will it work with fuzzy matching ?

[glepnir]

Thanks, can you explain what those are for? Also should be documented at :h highlight-groups

yup but not finish . i test it within mulitple bytes characters..something is wrong.. wait for a while

[glepnir]

Will it work with fuzzy matching ?

Hello . currently the completeopt doesn't have fuzzy option. if vim maintainer thought we can add it. i think it can be support.

[glepnir]

@chrisbra PTAL. works fine for me now. also for before comment i think we can support this completeopt+=fuzzy this is more useful. I can add it if maintainer vote yes

[techntools]

Please vote yes.

Suggested here as well.

[chrisbra]

can you make this interface similar to screen_puts_len()?

pum_screen_put_with_attr(text, textlen, row, col, attr)

[glepnir]

currently can't

[glepnir]

I've thought about this carefully. currently we use strncmp to compare compl_leader which always matches from the beginning of the string. so highlighting this part doesn't make sense. adding completeopt+=fuzzy would make this PR more useful. so I think I'll implement completeopt fuzzy first and then reorganize this PR

[romainl]

If this is merged, please ping:

• @habamax, @neutaaaaan and me (@romainl) so that we can update the built-in colorschemes at https://github.com/vim/colorschemes where and if necessary,
• @lifepillar so that he can update https://github.com/lifepillar/vim-colortemplate.

Thanks

[glepnir]

after a218cc6 ..this should be more useful now and i have do some update to make pr can also handle match highlight when in fuzzy .

[techntools]

@chrisbra Please merge this

[glepnir]

emm lots of windows failed Test_ColonEight_MultiByte() ..is related ?

[chrisbra]

yeah, those are new failures starting on the Windows builds recently. I have no idea why shortening pathnames does no longer work on Github CI Windows runners. But it only fails with multi-byte paths. No idea why, but I assume it's an issue with the Github CI runners, since there weren't any changes in the related code.

[chrisbra]

I think this looks good now. I am just slightly worried this will cause a bit of slowdown, because we are looping over the original text several times, once to determine the match positions and then to set the highlighting attributes.

[chrisbra]

I was just about to merging this, when I noticed the test does not work as expected and doesn't show any popup menus. Can you please fix those and also make sure the screen-dump test run once in fuzzy mode and one in regular completion mode?

In addition to your changes, I had the following changes on top, please add them while fixing the test:

diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index 655fd6c52..1e6f313eb 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -1,4 +1,4 @@
-*options.txt*  For Vim version 9.1.  Last change: 2024 Jun 05
+*options.txt*  For Vim version 9.1.  Last change: 2024 Jun 10


                  VIM REFERENCE MANUAL    by Bram Moolenaar
@@ -4275,6 +4275,7 @@ A jump table for the options with a short description can be found at |Q_op|.
                                     T:DiffText,>:SignColumn,-:Conceal,
                                     B:SpellBad,P:SpellCap,R:SpellRare,
                                     L:SpellLocal,+:Pmenu,=:PmenuSel,
+                                    k:PmenuMatch,<:PmenuMatchSel,
                                     [:PmenuKind,]:PmenuKindSel,
                                     {:PmenuExtra,}:PmenuExtraSel,
                                     x:PmenuSbar,X:PmenuThumb,*:TabLine,
@@ -4341,6 +4342,8 @@ A jump table for the options with a short description can be found at |Q_op|.
        |hl-PmenuExtraSel| }  popup menu "extra" selected line
        |hl-PmenuSbar|   x  popup menu scrollbar
        |hl-PmenuThumb|  X  popup menu scrollbar thumb
+       |hl-PmenuMatch|  k  popup menu matched text
+       |hl-PmenuMatchSel| <  popup menu matched text in selected line

        The display modes are:
                r       reverse         (termcap entry "mr" and "me")
diff --git a/runtime/doc/syntax.txt b/runtime/doc/syntax.txt
index 23de60704..3f684412c 100644
--- a/runtime/doc/syntax.txt
+++ b/runtime/doc/syntax.txt
@@ -1,4 +1,4 @@
-*syntax.txt*   For Vim version 9.1.  Last change: 2024 Jun 09
+*syntax.txt*   For Vim version 9.1.  Last change: 2024 Jun 10


                  VIM REFERENCE MANUAL    by Bram Moolenaar
diff --git a/runtime/doc/tags b/runtime/doc/tags
index 96be83ab0..3085ccc4c 100644
--- a/runtime/doc/tags
+++ b/runtime/doc/tags
@@ -8090,6 +8090,8 @@ hl-PmenuExtra     syntax.txt      /*hl-PmenuExtra*
 hl-PmenuExtraSel       syntax.txt      /*hl-PmenuExtraSel*
 hl-PmenuKind   syntax.txt      /*hl-PmenuKind*
 hl-PmenuKindSel        syntax.txt      /*hl-PmenuKindSel*
+hl-PmenuMatch  syntax.txt      /*hl-PmenuMatch*
+hl-PmenuMatchSel       syntax.txt      /*hl-PmenuMatchSel*
 hl-PmenuSbar   syntax.txt      /*hl-PmenuSbar*
 hl-PmenuSel    syntax.txt      /*hl-PmenuSel*
 hl-PmenuThumb  syntax.txt      /*hl-PmenuThumb*
diff --git a/runtime/doc/version9.txt b/runtime/doc/version9.txt
index 6edb8891b..71e95554e 100644
--- a/runtime/doc/version9.txt
+++ b/runtime/doc/version9.txt
@@ -1,4 +1,4 @@
-*version9.txt*  For Vim version 9.1.  Last change: 2024 Jun 05
+*version9.txt*  For Vim version 9.1.  Last change: 2024 Jun 10


                  VIM REFERENCE MANUAL    by Bram Moolenaar
@@ -41596,6 +41596,9 @@ Autocommands: ~
 Highlighting: ~

 |hl-MsgArea|           highlighting of the Command-line and messages area
+|hl-PmenuMatch|                Popup menu: highlighting of matched text
+|hl-PmenuMatchSel|     Popup menu: highlighting of matched text in selected
+                       line

 Commands: ~

diff --git a/src/popupmenu.c b/src/popupmenu.c
index 1c8c91c28..f3726f6ef 100644
--- a/src/popupmenu.c
+++ b/src/popupmenu.c
@@ -434,6 +434,12 @@ pum_screen_put_with_attr(int row, int col, char_u *text, int textlen, int attr)
     char_u     *leader = ins_compl_leader();
     int                in_fuzzy = (get_cot_flags() & COT_FUZZY) != 0;

+    if (highlight_attr[HLF_PMSI] == highlight_attr[HLF_PSI] &&
+       highlight_attr[HLF_PMNI] == highlight_attr[HLF_PNI])
+    {
+       screen_puts_len(text, textlen, row, col, attr);
+       return;
+    }
 #ifdef FEAT_RIGHTLEFT
     if (leader != NULL && curwin->w_p_rl)
         rt_leader = reverse_text(leader);
diff --git a/src/testdir/test_popup.vim b/src/testdir/test_popup.vim
index 434d86e2d..db7556d95 100644
--- a/src/testdir/test_popup.vim
+++ b/src/testdir/test_popup.vim
@@ -1357,7 +1357,7 @@ func Test_pum_highlights_match()
     hi PmenuMatch     ctermfg=4 ctermbg=225
   END
   call writefile(lines, 'Xscript', 'D')
-  let buf = RunVimInTerminal('-S Xscript', {})
+  let  buf = RunVimInTerminal('-S Xscript', {})
   call TermWait(buf)
   call term_sendkeys(buf, "i\<C-X>\<C-O>\<BS>\<BS>\<BS>fi")
   call TermWait(buf, 50)
diff --git a/src/vim.h b/src/vim.h
index d32d661d1..e70323937 100644
--- a/src/vim.h
+++ b/src/vim.h
@@ -1501,7 +1501,7 @@ typedef enum
     , HLF_PNI      // popup menu normal item
     , HLF_PSI      // popup menu selected item
     , HLF_PMNI     // popup menu matched text in normal item
-    , HLF_PMSI     // popup menu mathced text in selected item
+    , HLF_PMSI     // popup menu matched text in selected item
     , HLF_PNK      // popup menu normal item "kind"
     , HLF_PSK      // popup menu selected item "kind"
     , HLF_PNX      // popup menu normal item "menu" (extra text)

[chrisbra]

I think we should also skip this, if the HLF_PMSI and HLF_PMNI highlightings are not setup?

[glepnir]

yup also need compare attr if same as PSI also need run into this if.

[chrisbra]

so PmenuMatch and PmenuMatchSel highlighting only works for when fuzzy completeopt is enabled? I think it should also work when fuzzy has not been enabled yet.

[glepnir]

emm if you want . I can restore some codes of before commit to make it rework. honestly it not make sense when there is no fuzzy. because we only use prefix match (strncmp(item ,leader, leader_len) that mean it always highlight the first few letters of each word.

[chrisbra]

True, but the expectation from those newly introduced highlighting groups is, that they highlight the matching pattern, even if it is only a substring highlight I would think. So yes please enable a simple substring highlighting for the non-fuzzy case then

[glepnir]

this is picture is the first implementation . the point is item2 have two fi position. they both should highlight then or only highlight the first fi ?

[chrisbra]

I think only the first pattern match should be fine, so a simple subtring match is fine. I don't see the point of highlighting additional matches further down in the matches, because those are not considered when filtering the candidates. Default completion only works with single submatch filtering.

[glepnir]

done @chrisbra PTAL

[neutaaaaan]

I'll try and test drive this during the week.
I agree with @glepnir that there is little to no point in highlighting partial matches outside of fuzzy matching, and I'm somewhat wary of how we will have to tweak the 8 and 16c versions of the colorschemes to handle this properly, if possible at all, but the feature itself is definitely pertinent.

[chrisbra]

Nice, thanks for updating the test!