Highlight all search matches in document

[wes-adams]

Altering document clears search_matches highlights

(PS: the extra whitespace being highlighted in the demo gif is a glitch in asciicinema capture)

Updated display of matching text to use ui.cursor.match. this is the result:

References: https://github.com/helix-editor/helix/issues/4798

[the-mikedavis]

I haven't looked at this yet but also see https://github.com/helix-editor/helix/pull/4635 which I think will have some overlap implementation-wise

[wes-adams]

I haven't looked at this yet but also see #4635 which I think will have some overlap implementation-wise

Thanks. I'll definitely look into that work. This PR is 100% not ready. I'm still finding my way in this project and Rust in general, so any feedback and/or reviews I can get will be greatly appreciated!

[wes-adams]

i think the only optimization left is to search only the visible text for matches versus the entire document i've given this a little effort, but would welcome any feedback i can get

EDIT: the above is fixed, but a bug is introduced: if scrolling through matches moves current view, the search range is not updated, so highlights remain in the same place, but are wrong

[pascalkuthe]

Just a couple comments:

• The matches should not be stored on the editor, they should be stored on the document or (even better) on the view
• Right now the matches stick around forever which causes a host of problems if the text is edited. To fix this you either need to update the matches on every edit or hide them as soon as the document is edited. Regex matches has quite a bit of overhead because we need to convert Rope -> String (see #185) so that would not be ideal. I also dislike how long the highlights stick around in nvim (even tough I like the basic feature of search highlights) so I think it's actually a great solution to remove the search matches on edits.
• Updating the matches while scrolling is too slow. I think you could recompute the matches on idle timeout if the view position changed.
• This potentially should be rebased on #5420 too as the way the "visible area" is calculated changes there (the view offset changes), this PR should also not be merged before that because I would have to update this command as part of #5420 then (which is already very large)

[wes-adams]

Just a couple comments:

• The matches should not be stored on the editor, they should be stored on the document or (even better) on the view
• Right now the matches stick around forever which causes a host of problems if the text is edited. To fix this you either need to update the matches on every edit or hide them as soon as the document is edited. Regex matches has quite a bit of overhead because we need to convert Rope -> String (see Regex Automata #185) so that would not be ideal. I also dislike how long the highlights stick around in nvim (even tough I like the basic feature of search highlights) so I think it's actually a great solution to remove the search matches on edits.
• Updating the matches while scrolling is too slow. I think you could recompute the matches on idle timeout if the view position changed.
• This potentially should be rebased on rework positioning/rendering and enable softwrap/virtual text #5420 too as the way the "visible area" is calculated changes there (the view offset changes), this PR should also not be merged before that because I would have to update this command as part of rework positioning/rendering and enable softwrap/virtual text #5420 then (which is already very large)

i have this PR in the following state:

• search_matches are now stored in View

• search_matches highlights are updated with each new key pressed (in search_impl)

  • if n(or others) is used to traverse searches, search_matches are not updated

• ; can be used to clear highlights, but highlights will resume if n is used to continue match traversal

• highlights do not go away or update when doc updates

  • i tried to make this happen with Document::is_modified() but i had troubles
  • if is_modified() == true and the document is never saved, the search_matches highlights would never be rendered
  • i think i need help with this (either updating search_matches on edit, or clearing search_matches on edit)

  (also, FWIW, i tried all sorts of solutions using idle timeout, but it seems to be dependent on key presses. so i had some decent solutions in place, but they required a keystroke or two before highlights would update, etc)

  @the-mikedavis @pascalkuthe

[pathwave]

Is the color here the same as selections? Might need to difference between those.

A feature I would like is to highlight words that match the current primary selection, and that seems to overlap with this (since when you search the search term gets selected)

[wes-adams]

Is the color here the same as selections? Might need to difference between those.

A feature I would like is to highlight words that match the current primary selection, and that seems to overlap with this (since when you search the search term gets selected)

the search_matches highlight color is dictated by ui-selection:

    pub fn get_search_matches(
        &self,
        theme: &Theme,
    ) -> Option<Vec<(usize, std::ops::Range<usize>)>> {
        let selection_scope = theme
            .find_scope_index("ui.selection")
            .expect("could not find `ui.selection` scope in the theme!");
. . . 

an easy update or change to have a different highlight for selection and search_matches, but there would have to be a candidate definition in the theme configs.

[wes-adams]

this one is ready for a closer look guys

i think the behavior is close to what @pascalkuthe outlined in the comment above

@the-mikedavis @pascalkuthe

[wes-adams]

would either of you please take a look? i'd be very grateful for some feedback on this one. @the-mikedavis @pascalkuthe

[tmpm697]

What's status of this PR? this is really helpful to have all matched words highlighted.

[lpchaim]

I've been looking into adding a search index/total matches statusline as I was used to back in nvim (e.g., indicating you're on match 2 out of 5 with something like [2/5] on the bottom) and it does look like I should wait until this gets merged in the interest of not duplicating the work made here so that the context holds search match information and whatnot, so I'll be keeping an eye on this.

[wes-adams]

@the-mikedavis @archseer @pascalkuthe @sudormrfbin sorry to blindly tag you guys again, but i'm still hoping to get some sort of review on this PR would anyone kindly help me gain some traction on this?

[pascalkuthe]

my concern from https://github.com/helix-editor/helix/pull/5702#issuecomment-1408507220 still apply/are unadressed.

Running a regex over document contents synchronously on every keypress is unacceotable. Particularly of the rope -> string conversion is very non-ideal and should be done rarely (or rather not at all, the current implementation is a workaround). I am working on a streaming regex engine but that still takes some time and even with that solved (which would improve pref a lot) running a regex on every keypress is stijj not something I am ok with. Using the transactional word mapping in #6447 could allow you to update the ranges without actually running a regex and then you could run the actual regex with a 5s debounce or similar (with #8021). I don't think regularly covering to string would be ok tough so that has to wait until we have a streaming regex engine... Which will be a while.

You could keep all of this simpler by dismissing changes when transactions are applied just as I said in my first comment. Store the highlights in a HashMap<ViewId, SerachMatches> and then clear the entire map in apply_impl. Then you just need some sort of view position changed hook (with #8021) to update the matches when scrolling (similar to inlay hints but not based on the idle timeout like the current implementation. I don't want to merge new stuff that uses the idle timeout).