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.

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

Fix several minor search bugs relating to incsearch highlighting and Visual mode #1080

Merged
merged 8 commits into from
Jan 22, 2025

Conversation

citizenmatt
Copy link
Member

@citizenmatt citizenmatt commented Jan 4, 2025

This PR fixes a number of bugs related to 'incsearch' highlighting while searching in Visual mode, e.g. v/foo

  • Fixes an issue where 'incsearch' would not correctly update an empty selection, when 'selection' was set to "exclusive". So something like ve/foo would work but v/foo would not update the selection. The selection would be correctly set when completing the search with <CR>
  • This also fixed an issue with correctly updating the selection when the search pattern was changed, such as editing it from /foo to /faa
  • Don't show 'incsearch' highlighting when 'nohlsearch' is set and Visual mode is active. Normally, 'incsearch' highlighting is shown for the current match, even when 'hlsearch' is disabled, but it's not shown if Visual mode is active - the selection is enough
  • Exit Visual mode when 'incsearch' needs to move the caret for a Command-line command, rather than a search. E.g. something like ve:<C-U>normal 3j should not maintain the Visual selection, but ve/foo should

It also removes some obsolete tests using the no longer used 'usenewregex' option, and splits up the search related tests into SearchGroupTest.kt, IncsearchTests.kt, SearchHighlightsTest.kt, etc.

This PR depends on the Mode refactoring in #1078 and includes all of those commits. To make it easier for me to manage, it's also based on #1079. Once these PRs are merged, I will rebase and force push and mark this PR as no longer draft. Here are the lists of commits for this PR: faeaafd2...f229c5d0

@citizenmatt citizenmatt force-pushed the bugfix/incsearch-and-visual branch from f31fde9 to f229c5d Compare January 10, 2025 21:11
@citizenmatt citizenmatt force-pushed the bugfix/incsearch-and-visual branch from f229c5d to 93cc47c Compare January 13, 2025 19:40
@citizenmatt citizenmatt marked this pull request as ready for review January 13, 2025 19:42
@citizenmatt
Copy link
Member Author

Rebased and ready for review

Copy link
Member

@AlexPl292 AlexPl292 left a comment

Choose a reason for hiding this comment

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

Looks good for me. Can you add a small doc for my comment?

@AlexPl292 AlexPl292 merged commit 33b34b8 into JetBrains:master Jan 22, 2025
4 checks passed
@AlexPl292
Copy link
Member

Thank you!

@citizenmatt citizenmatt deleted the bugfix/incsearch-and-visual branch January 23, 2025 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants