[RFC] Implement SearchCurrent for current match hl

[certik]

Uses SearchCurrent to highlight the current match under cursor.

How to use: put this into your init.vim:

hi Search ctermbg=blue ctermfg=red
hi SearchCurrent ctermbg=green ctermfg=red

Then the current match will be highlighted using SearchCurrent, while the rest will be highlighted using Search. An example:

Issues to resolve:

• When "n" is pressed, the colors do not get updated. One has to press /int<CR> (in the screenshot above) when the cursor is at some other place, and it will correctly highlight that new match. One has to figure how to redraw when "n" or "N" is pressed.
• Update tests for the new behavior (a few currently fail)
• document the new option SearchCurrent

Fixes #4600.

[blueyed]

For reference: related Vim issue: vim/vim#2819

[justinmk]

would it make sense to re-use IncSearch for this instead of adding another highlight group?

One has to figure how to redraw when "n" or "N" is pressed.

In nv_next() try adding redraw_buf_line_later().

[certik]

@justinmk I reused IncSearch, I think that's better. I now redraw the screen in nv_next(), and the problem is fixed! Both "n" and "N" now works. Thanks for the suggestion. Here is how it looks like, with default nvim settings:

And here is how it looks like with hi IncSearch ctermfg=green:

Which I think looks better. I wonder if we should change the default IncSearch settings?

(In order for the background to be green, I had to set the ctermfg (i.e, foreground), which seems weird.)

---

At this point, I think the basic prototype works exactly as expected. I have a few questions:

• How does nvim redraw the screen? When a redraw is asked, is the whole screen always redrawn from scratch, and then nvim compares to the old screen and figures out an efficient diff to be sent to the terminal (much like ncurses works)? Or does nvim have some elaborate update mechanism to only draw things that changed?
• Above I call redraw_later(NOT_VALID), which I assume redraws the whole screen. Is it a waste to always redraw everything in nv_next?
• Should I figure out how to call the redraw_buf_line_later on the old line and the new line?
• For efficiency reasons, should this feature in this PR be even considered, since it seems we now have to redraw the screen in nv_next, one way or another.

[certik]

This simplification can be applied:

diff --git a/src/nvim/buffer_defs.h b/src/nvim/buffer_defs.h
index dc0e718d6..16c7804be 100644
--- a/src/nvim/buffer_defs.h
+++ b/src/nvim/buffer_defs.h
@@ -940,7 +940,6 @@ typedef struct {
   buf_T       *buf;     // the buffer to search for a match
   linenr_T lnum;        // the line to search for a match
   int attr;             // attributes to be used for a match
-  int attr_cur2;        // attributes to be used for a current match
   int attr_cur;         // attributes currently active in win_line()
   linenr_T first_lnum;  // first lnum to search for multi-line pat
   colnr_T startcol;     // in win_line() points to char where HL starts
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index baa100333..766ad4657 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -3001,7 +3001,7 @@ win_line (
               if (wp->w_cursor.lnum == lnum
                   && wp->w_cursor.col >= (long)shl->startcol
                   && wp->w_cursor.col < (long)shl->endcol) {
-                shl->attr_cur = shl->attr_cur2;
+                shl->attr_cur = win_hl_attr(wp, HLF_I);
               } else {
                 shl->attr_cur = shl->attr;
               }
@@ -5613,7 +5613,6 @@ static void init_search_hl(win_T *wp)
   search_hl.lnum = 0;
   search_hl.first_lnum = 0;
   search_hl.attr = win_hl_attr(wp, HLF_L);
-  search_hl.attr_cur2 = win_hl_attr(wp, HLF_I);
 
   // time limit is set at the toplevel, for all windows
 }

and things still work. @justinmk should I do that? I don't know what style is preferred.

[justinmk]

should I do that? I don't know what style is preferred

Sure.

Note that Bram seems to prefer the SearchCurrent approach, so we should probably stick with that. It can be linked with IncSearch by default.

[certik]

Sure, I'll revert it. It should probably be called something like SearchLastMatch, per Bram's comment.

[certik]

I updated the PR. Interesting is that before one did:

hi IncSearch ctermfg=green

but now one has to do:

hi SearchLastFoundMatch ctermbg=green ctermfg=black

to get the same result. It looks like the IncSearch is doing some inverting.

Anyway, the PR is now really simple. Just four very little changes.

@justinmk do you think that this change looks good? If so, I'll start working on updating the tests to pass.

[janlazo]

Cannot merge without tests.

[clason]

Bump -- that would be quite nice to have! @certik would you be interested in finishing this (rebase and fix build, docs, and test)?

@bfredl is this approach still fine (especially the forced redraw in nv_next)?

[zeertzjq]

A change in cursor_spec.lua should be needed:

diff --git a/test/functional/ui/cursor_spec.lua b/test/functional/ui/cursor_spec.lua
index 03cd4bfd0..4c51547e2 100644
--- a/test/functional/ui/cursor_spec.lua
+++ b/test/functional/ui/cursor_spec.lua
@@ -212,10 +212,10 @@ describe('ui/cursor', function()
         if m.blinkwait then m.blinkwait = 700 end
       end
       if m.hl_id then
-          m.hl_id = 60
+          m.hl_id = 61
           m.attr = {background = Screen.colors.DarkGray}
       end
-      if m.id_lm then m.id_lm = 61 end
+      if m.id_lm then m.id_lm = 62 end
     end
 
     -- Assert the new expectation.

BTW the SearchLastFoundMatch name is really too long.

[numToStr]

I think setting the highlight group somehow affects the matchit highlighting

2021-12-29.16-54-00.mp4

Note: I changed the hl-group name to SearchCurrent

[certik]

This would be very nice to have, but I need help from neovim developers to help me get this over the finish line. I need help with writing tests, as well as feedback if any changes are needed.

[clason]

Before any of that, we might want to bikeshed on the name; I agree that SearchLastFoundMatch is a mouthful and would prefer SearchCurrentMatch or even SearchCurrent. (I know Bram has issues with that name as it's not technically descriptive -- but that's what the documentation is for!)

---

Then, I think the first thing would be to add the new highlight group here:

neovim/src/nvim/option.c

Line 263 in c46f7ca

"i:IncSearch,l:Search,m:MoreMsg,M:ModeMsg,n:LineNr,a:LineNrAbove,b:LineNrBelow,N:CursorLineNr," \

and here:

neovim/src/nvim/syntax.c

Line 6164 in c46f7ca

"IncSearch cterm=reverse gui=reverse",

(probably easiest to link it to IncSearch by default -- or Search, in the hope that this would require fewer changes).

Documentation goes here:

neovim/runtime/doc/syntax.txt

Lines 5170 to 5172 in c46f7ca

	*hl-Search*
	Search Last search pattern highlighting (see 'hlsearch').
	Also used for similar items that need to stand out.

For tests, a good starting point would be to adapt one of the tests here: https://github.com/neovim/neovim/blob/master/test/functional/ui/searchhl_spec.lua
(I'm not a test expert, so I hope someone else will step in here, as that can indeed be rough the first time ;))

[famiu]

Do you mind if I take over this PR? I think I might be able to finish it if you don't have time

[clason]

@famiu I think you can take the silence for a "yes".

[famiu]

@famiu I think you can take the silence for a "yes".

Will work on it once I get the PC

[leungbk]

An initial attempt at this has been merged upstream (vim/vim#10133 (comment)), with some small issues.

[certik]

@famiu yes, please take over the PR. I don't have the time to learn enough about neovim to bring it over the finish line, it looks like tests have to be written etc.

[zeertzjq]

Superseded by #18081