Fix dangling search selection using viewport pos#19603
Merged
Conversation
DHowett
approved these changes
Dec 2, 2025
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
Fixes a bug where the dangling selection from a search would be applied to the wrong position. Specifically, the issue is that
SetSelectionAnchor()andSetSelectionEnd()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