mark mode: begin selection at focused search result#19550
Merged
carlos-zamora merged 1 commit intomainfrom Nov 18, 2025
Merged
Conversation
carlos-zamora
commented
Nov 11, 2025
Comment on lines
+1791
to
+1797
| // GH #19358: select the focused search result before clearing search | ||
| if (const auto focusedSearchResult = _terminal->GetSearchHighlightFocused()) | ||
| { | ||
| _terminal->SetSelectionAnchor(focusedSearchResult->start); | ||
| _terminal->SetSelectionEnd(focusedSearchResult->end); | ||
| _renderer->TriggerSelection(); | ||
| } |
Member
Author
There was a problem hiding this comment.
Personally, I think leaving behind selected text after performing a search feels a little weird, but I think it's necessary. This enables the user to begin a selection at the focused search result using only the keyboard. If we get rid of this, the user would need to manually click on the terminal or transfer focus over somehow.
Member
|
Hmmmm. I don't love that we now have three search states: active result, inactive result, and selected. I'm not sure how I feel all-up |
lhecker
approved these changes
Nov 17, 2025
DHowett
pushed a commit
that referenced
this pull request
Dec 2, 2025
## Summary of the Pull Request Fixes a bug where the dangling selection from a search would be applied to the wrong position. Specifically, the issue is that `SetSelectionAnchor()` and `SetSelectionEnd()` expect viewport positions whereas the searcher outputs buffer positions. This PR simply applies the scroll offset to the search result before calling the functions. In a separate iteration, I changed the functions to allow for viewport-relative vs buffer-relative positions. However, that ended up feeling a bit odd because this is the only scenario where the functions were receiving buffer-relative positions. I chose this approach instead because it's smaller/cleaner, even though we convert to viewport-relative before the call just to change it to buffer-relative in the function. Bug introduced in #19550 ## Validation Steps Performed The correct region is selected in the following scenarios: ✅ no scrollback ✅ with scrollback, at bottom ✅ with scrollback, not at bottom (selection isn't scrolled to, but I think that's ok. Can be fixed easily if requested) ✅ alt buffer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Searching in terminal highlights all search results. However, those results are considered separate from a selection. In the past, the highlighted result would be selected, resulting in it being the initial position for mark mode. Now that it's separate, mark mode doesn't start there.
To fix this, there's 2 changes here:
With this change, mark mode's initial position is determined in this order:
Validation Steps Performed
Entering mark mode in scenario X results in a starting position of Y:
✅ selected text during a search --> selected text
✅ performed a search and results are available -->focused search result
✅ performed a search and no results are available
✅ performed a search, got results, then closed search --> focused search result
Closes #19358