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 some search highlights scenarios #17352

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

tusharsnx
Copy link
Contributor

Fixes:

  • Snapping the current match to the current selection doesn't work.
  • Fast closing and re-opening SearchBox would leave search highlights in an inconsistent state. The highlights would be active even when SB is not on the screen, and results are not updated as more text is added to the buffer.
  • Search highlights scroll marks are not cleared when the search box is closed.

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved
Comment on lines +164 to +168
// Stop ongoing close animation if any
if (CloseAnimation().GetCurrentState() == Media::Animation::ClockState::Active)
{
CloseAnimation().Stop();
}
Copy link
Member

@lhecker lhecker Jun 4, 2024

Choose a reason for hiding this comment

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

I'm not sure I understand how us previously ignoring the animation state would result in

Fast closing and re-opening SearchBox would leave search highlights in an inconsistent state.

If you know why, could you explain how that happens?

Copy link
Contributor Author

@tusharsnx tusharsnx Jun 5, 2024

Choose a reason for hiding this comment

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

When searchbox is in focus, if you press ESC and then quickly do a "find next/prev match" action, it will trigger this issue. The trick is to start a search when SB is closing. It's easier if you bind Find match action to a single key, eg. F3

Copy link
Contributor Author

@tusharsnx tusharsnx Jun 5, 2024

Choose a reason for hiding this comment

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

Not to forget, highlights should be active (i.e. one or more search results available) when you're closing SB.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Sorry it took us a while to get to it 😅

@carlos-zamora carlos-zamora added this pull request to the merge queue Jun 13, 2024
Merged via the queue into microsoft:main with commit 7c1e229 Jun 13, 2024
15 checks passed
DHowett pushed a commit that referenced this pull request Jun 20, 2024
Fixes:
- Snapping the current match to the current selection doesn't work.
- Fast closing and re-opening SearchBox would leave search highlights in
an inconsistent state. The highlights would be active even when SB is
not on the screen, and results are not updated as more text is added to
the buffer.
- Search highlights scroll marks are not cleared when the search box is
closed.

(cherry picked from commit 7c1e229)
Service-Card-Id: 92736967
Service-Version: 1.21
@tusharsnx tusharsnx deleted the search-highlights-fixes branch June 27, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants