Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

wants to merge 1 commit into from

Conversation

glepnir
Copy link
Contributor

@glepnir glepnir commented May 1, 2024 •
edited

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

image

@chrisbra
Copy link
Member

chrisbra commented May 1, 2024

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

@techntools
Copy link

Will it work with fuzzy matching ?

@glepnir
Copy link
Contributor Author

glepnir commented May 2, 2024 •
edited

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
Copy link
Contributor Author

glepnir commented May 2, 2024 •
edited

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 glepnir force-pushed the p_hl branch 4 times, most recently from 09bd0d1 to 05c71e0 Compare May 2, 2024 10:39
@glepnir
Copy link
Contributor Author

glepnir commented May 3, 2024 •
edited

@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
Copy link

Please vote yes.

Suggested here as well.

@glepnir
Copy link
Contributor Author

glepnir commented May 6, 2024 •
edited

tried some work on fuzzy 😄

fuzzy

chrisbra
chrisbra reviewed May 6, 2024
View reviewed changes
src/highlight.c
@@ -257,6 +257,8 @@ static char *(highlight_init_both[]) = {
"default link CursorLineFold FoldColumn",
"default link CurSearch Search",
"default link PmenuKind Pmenu",
"default link PmenuMatch Pmenu",
"default link PmenuMatchSel PmenuSel",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep that sorted please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no please then :)

src/popupmenu.c
while (*ptr != NUL && ptr < text + textlen)
{
char_len = mb_ptr2len(ptr);
cells = utf_ptr2cells(ptr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mb_ptr2cells()

src/popupmenu.c
{
char_len = mb_ptr2len(ptr);
cells = utf_ptr2cells(ptr);
int new_attr = attr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to the beginning of the block please

src/popupmenu.c
@@ -596,7 +660,8 @@ pum_redraw(void)
else
--cells;
}
screen_puts_len(st, size, row, col, attr);
pum_screen_put_with_attr(row, col, st, size, attr);
//screen_puts_len(st, size, row, col, attr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented out line please

src/popupmenu.c
* displays text on the popup menu with specific attributes.
*/
static void
pum_screen_put_with_attr(int row, int col, char_u *text, int textlen, int attr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make this interface similar to screen_puts_len()?

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

src/testdir/test_popup.vim
@@ -1339,4 +1339,32 @@ func Test_pum_highlights_custom()
call StopVimInTerminal(buf)
endfunc

"Test match relate highlight group in pmenu
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space. Also the comment is not clear

src/optiondefs.h
@@ -300,7 +300,7 @@ struct vimoption
# define ISP_LATIN1 (char_u *)"@,161-255"
#endif

# define HIGHLIGHT_INIT "8:SpecialKey,~:EndOfBuffer,@:NonText,d:Directory,e:ErrorMsg,i:IncSearch,l:Search,y:CurSearch,m:MoreMsg,M:ModeMsg,n:LineNr,a:LineNrAbove,b:LineNrBelow,N:CursorLineNr,G:CursorLineSign,O:CursorLineFold,r:Question,s:StatusLine,S:StatusLineNC,c:VertSplit,t:Title,v:Visual,V:VisualNOS,w:WarningMsg,W:WildMenu,f:Folded,F:FoldColumn,A:DiffAdd,C:DiffChange,D:DiffDelete,T:DiffText,>:SignColumn,-:Conceal,B:SpellBad,P:SpellCap,R:SpellRare,L:SpellLocal,+:Pmenu,=:PmenuSel,[:PmenuKind,]:PmenuKindSel,{:PmenuExtra,}:PmenuExtraSel,x:PmenuSbar,X:PmenuThumb,*:TabLine,#:TabLineSel,_:TabLineFill,!:CursorColumn,.:CursorLine,o:ColorColumn,q:QuickFixLine,z:StatusLineTerm,Z:StatusLineTermNC,g:MsgArea"
# define HIGHLIGHT_INIT "8:SpecialKey,~:EndOfBuffer,@:NonText,d:Directory,e:ErrorMsg,i:IncSearch,l:Search,y:CurSearch,m:MoreMsg,M:ModeMsg,n:LineNr,a:LineNrAbove,b:LineNrBelow,N:CursorLineNr,G:CursorLineSign,O:CursorLineFold,r:Question,s:StatusLine,S:StatusLineNC,c:VertSplit,t:Title,v:Visual,V:VisualNOS,w:WarningMsg,W:WildMenu,f:Folded,F:FoldColumn,A:DiffAdd,C:DiffChange,D:DiffDelete,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,#:TabLineSel,_:TabLineFill,!:CursorColumn,.:CursorLine,o:ColorColumn,q:QuickFixLine,z:StatusLineTerm,Z:StatusLineTermNC,g:MsgArea"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those added entries should probably move to the end of the old entries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also requires to update :h 'hl'

@glepnir
Copy link
Contributor Author

glepnir commented May 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisbra chrisbra chrisbra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@glepnir @chrisbra @techntools