-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Add findNext
, findPrev
actions
#8917
Conversation
…t away with only dismissing selection on key _down_. Otherwise keyups will dismiss the selection you just made
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want a little bit of discussion on the keybinding arg thing before approving. The code is solid. Don't feel strongly enough on these topics to block though.
src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw
Outdated
Show resolved
Hide resolved
Don't forget to update the docs plz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I only have one concern. This is great otherwise.
@@ -102,6 +102,8 @@ namespace Microsoft.Terminal.TerminalControl | |||
|
|||
void CreateSearchBoxControl(); | |||
|
|||
void SearchMatch(Boolean goForward); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a weird public API, but i suppose that this is fine
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
🎉 Handy links: |
This PR is a resurrection of #8522. @Hegunumo has apparently deleted
their account, but the contribution was still valuable. I'm just here to
get it across the finish line.
This PR adds new action for navigating to the next & previous search
results. These actions are unbound by default. These actions can be used
from directly within the search dialog also, to immediately navigate the
results.
Furthermore, if you have a search started, and close the search box,
then press this keybinding, it will still perform the search. So you
can just hit F3 repeatedly with the dialog closed to keep
searching new results. Neat!
If you dispatch the action on the key down, then dismiss a selection on
a key up, we'll end up immediately destroying the selection when you
release the bound key. That's annoying. It also bothers @carlos-zamora
in #3758. However, I think we can just only dismiss the selection on a
key up. I think that's fine. It seems fine so far. We've got an
entire release cycle to futz with it.
Validation Steps Performed
I've played with it all day and it seems crisp.
Closes #7695
Co-authored-by: Kiminori Kaburagi [email protected]