statusline: Add search position #4635
Conversation
sigmaSd
commented
Nov 7, 2022
sigmaSd
commented
kirawi
added
A-gui
helix-term/src/commands.rs
Outdated
| // Find all matches in the document | ||
| // Then find the current match position inside all matches | ||
| // We then render that to the status line by setting editor.search_matches | ||
| let all_matches: Vec<_> = regex.find_iter(contents).collect(); |
There was a problem hiding this comment.
I'm a little reluctant about this feature in general because it requires collecting all matches in the document. n only needs to find the first match after the start point and N works on a subslice of the document in most cases but with this change, all invocations of n and N find all matches of the pattern across the doc
There was a problem hiding this comment.
Maybe all_matches should be cached somewhere so consecutive n N don't need to do a full search
There was a problem hiding this comment.
It can be cached like this 07c5e9a (I can clanup the code) between consecutive n or N usages.
What do you think? I personally find this feature very useful it feels weird when I'm searching a document and I don't know how many elements ahead there is
There was a problem hiding this comment.
Storing the matches on the doc (preferrably within SearchPosition) looks good. We should be able to avoid running any regex if we already have the set of matches, and we may want to perform the search async so that we don't block the UI when there are many matches #2811 (comment)
There was a problem hiding this comment.
I merged them in one struct. I'll probably need some time to figure out how to make the search async and to use only one regex search, probably we'll need to keep a state in the search like a cursor for where we are at.
There was a problem hiding this comment.
@the-mikedavis can you review/test efdb69a This commit only makes one regex search, if that works and I can clean it up I'll probably tackle async next
There was a problem hiding this comment.
I just noticed a bug when the register / changes the search needs to be also reset, I can fix that later after your next review
There was a problem hiding this comment.
Bug fixed https://github.com/helix-editor/helix/compare/master...sigmaSd:helix:reg?expand=1 I'll keep pushing updates to that branch
|
Resolves #2811 |
|
I think I need to add this #2811 (comment) but I wonder how is that implemented should I put a limit based on contents number of chars: (need to figure how to not land inside Unicode surrogate because that panics) regex.find_iter(contents [..99999]) and then cap the max number of found items as well just to guarantee thats it's always the same |
|
Maybe instead we could have it reflect |
|
Limiting the number of chars found seems unnecessary. To limit to a certain number of results you can use |
|
What if we instead added an element that showed the selection position? |
|
Misclick. |
Do you mean the line-number/column of the current match? |
|
I would like to have this feature. Sometimes I need it really bad, so bad that I've implemented it in my fork hoping to open a PR but then noticed this :) Differences are that in my implementation I don't keep state and use the status line to display current match and total number of matches. Works fast on sane documents, haven't checked it on big. |
|
@zummenix feel free to open another PR, I don't think I''m going to have time to finish this |
|
The point of keeping state is to not search the whole document |
I understand this is an optimization. But maybe it works fast enough that it doesn't matter. One problem I see with keeping state is that there should be clear invalidation points like changing a search query or changes in a document, perhaps something else. |
|
closing this one out as stale, thank you for contributing! |
|
Just saw this, I have a more recent implementation that doesn't need to add a bunch of extra code: |
