Search fixes/improvements (show match count)#1466
Search fixes/improvements (show match count)#1466davidot wants to merge 3 commits intoJetBrains:masterfrom
Conversation
This manifests in the highlighting of search results, when * on a word with capitals smartcase was (correctly) ignored for the actual search but the highlights did use smartcase.
This can otherwise lead to great slowdown particularly during initial typed letters. Because with incsearch highlighting is triggered and block, this means starting your search will highlight up to thousands of the same character only refined later. With this new setting this skips highlighting (during incsearch only) when above threshold, or keeps old behavior if set to -1.
Controlled by new option `showmatchcount`. If enabled shows current/total matches in the main file being edited. For example [2/10] means 10 totals matches and your past match 1 and in or before match 2 in the file. The count is follows the active focussed editor around.
|
|
||
| @TestWithoutNeovim(reason = SkipNeovimReason.NOT_VIM_TESTING) | ||
| @Test | ||
| fun testFindAllIgnoreCaseOverwritesSmartCase() { |
There was a problem hiding this comment.
Can you also add a test in SearchWholeWordForwardActionTest for * action?
|
|
||
| val shouldIgnoreCase = pattern == null || shouldIgnoreCase(pattern, shouldIgnoreSmartCase) | ||
|
|
||
| var maxhlduringincsearch = injector.globalOptions().maxhlduringincsearch |
There was a problem hiding this comment.
@AlexPl292 Please confirm we want to add this custom property.
NeoVim is using it like this:
- 'redrawtime' ('rdt', default 2000ms) — limits time spent on search highlighting. If highlighting all matches takes longer than this, highlighting is disabled for that pattern.
But his solution with just using count is much simpler imho
There was a problem hiding this comment.
I would also vote for using standard Vim options. I don't think IdeaVim should introduce its own (the custom ones we have start with "idea"). I think we end up in a good situation where googling a solution for a problem in Vim can also fix it in IdeaVim.
The same goes for the other option introduced in this PR.
There was a problem hiding this comment.
Very good point it is good to be searchable.
So would it be okay if I implement the options like:
set shortmess-=S for showing search count (mention in docs that no other options are supported at the moment.
And set redrawtime=200 -> But we don't actually do redraw time but use the count? Personally I would prefer to not freeze even for 2 seconds, unless I really want to search for something that occurs 1000s of times.
I had a quick try but stopping halfway (after 2000ms of adding highlights) feels very non intuitive to me.
Now you don't know if there are no other matches or it simply timed out.
Let me know your preference!
There was a problem hiding this comment.
We can pass the deadline to injector.searchHelper.findAlland check periodically if time elapsed every N matches found.
That would reproduce how it's implemented in nvim
| // If any of the following hold we are still searching: | ||
| // - We just highlighted some search results (countMatches is not MaybeClear | ||
| // - We did not clear highlights ( | ||
| // - We are moving towards to a new position | ||
| if (countMatches == CountMatchesState.MaybeClearCount && clearHighlights && newCaretPosition == null) { | ||
| ExEntryPanel.getOrCreatePanelInstance().clearStatusText() | ||
| return@forEach | ||
| } |
There was a problem hiding this comment.
Could you make this part about countMatches more self-explanatory?
It's hard to follow what countMatches is actually representing.
For example it could single method shouldClearStatus and inside justHighlighted, highlightedNotCleared, MovingTowardsNextPositions.
Also, not making countMatches mutable would help, as it's hard to follow what it actually represents
3 participants
Content:
This PR fixes one small bug in highlighting and adds two new features.
Fixes a case where highlighting does not match actual search.
How to reproduce: Have a word with capital letters and one all lower case, use * on the capitalized word to search for that one. The search correctly ignores case, but if smartcase is enabled the highlighting incorrectly does not highlight the lower case word.
Maybe related: VIM-3459 Regression: 'smartcase' should not affect star command
Add an option to disable highlighting many matches during incsearch
This otherwise can massively slow down searching as the IDE tries to highlight all 0s in a csv for example.
This is protected behind an option ``, but for now I have set the default to 100, WOULD BE CHANGE IN DEFAULT BEHAVIOR (setting it to -1 gets old behavior)
Related:
Add option to show matches to search in file
showmatchcountthis will display a counter and index into the matches while searching, e.g.[13/159]means 159 matches and your at or before match 13.Related:
Example:
Notice here that I'm searching for "0" in the left editor, the left file show matches because there are only 8 (see right part of command bar). The right file has 1000+ matches so shows nothing, meaning no hiccups while typing "01" for example.
Also visible in the middle bottom is the counter for matches in the files and the current index in the matches for that file.
Once you press enter the "protection" is gone, meaning to only a single slowdown in this case
After that I clicked on the right side and you can see what the search counter looks like when the entry is not active (maybe it should have a border on the left side?)
Review
Please feel free to school me on anything Kotlin, this is my first time using the languages, just tried to match the patterns I saw in other files.
Also I really tried adding tests for option 2 & 3 but I had to mock so much I gave up, if there is a good/easy way to do it would love to hear it.