From d1ac37599cd28c9355d3142688c26ea8a34d10e5 Mon Sep 17 00:00:00 2001 From: e82eric Date: Wed, 7 Aug 2024 02:32:16 -0400 Subject: [PATCH] Avoid covering current search highlight with search box (#17516) Adds a scroll offset to avoid hiding the current search highlight with the search box. - Offset is based on the number of rows that the search box takes up. (I am not totally sure I am calculating this right) - This won't help when the current highlight is in the first couple rows of the buffer. Fixes: #4407 (cherry picked from commit 0bafab9a0ff881548bb1356dcaeed21aba1f6474) Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgTTNTE Service-Version: 1.21 --- src/cascadia/TerminalControl/ControlCore.cpp | 18 ++++++------- src/cascadia/TerminalControl/ControlCore.h | 2 +- src/cascadia/TerminalControl/ControlCore.idl | 12 ++++++++- src/cascadia/TerminalControl/TermControl.cpp | 28 +++++++++++++++++--- src/cascadia/TerminalControl/TermControl.h | 3 +++ src/cascadia/TerminalCore/Terminal.cpp | 6 +++-- src/cascadia/TerminalCore/Terminal.hpp | 2 +- 7 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 1fc430575e8..3b56c093e12 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1654,22 +1654,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - resetOnly: If true, only Reset() will be called, if anything. FindNext() will never be called. // Return Value: // - - SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool regularExpression, const bool resetOnly) + SearchResults ControlCore::Search(SearchRequest request) { const auto lock = _terminal->LockForWriting(); SearchFlag flags{}; - WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !caseSensitive); - WI_SetFlagIf(flags, SearchFlag::RegularExpression, regularExpression); - const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), text, flags); + WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !request.CaseSensitive); + WI_SetFlagIf(flags, SearchFlag::RegularExpression, request.RegularExpression); + const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), request.Text, flags); - if (searchInvalidated || !resetOnly) + if (searchInvalidated || !request.Reset) { std::vector oldResults; if (searchInvalidated) { oldResults = _searcher.ExtractResults(); - _searcher.Reset(*_terminal.get(), text, flags, !goForward); + _searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward); if (SnapSearchResultToSelection()) { @@ -1681,12 +1681,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else { - _searcher.FindNext(!goForward); + _searcher.FindNext(!request.GoForward); } if (const auto idx = _searcher.CurrentMatch(); idx >= 0) { - _terminal->SetSearchHighlightFocused(gsl::narrow(idx)); + _terminal->SetSearchHighlightFocused(gsl::narrow(idx), request.ScrollOffset); } _renderer->TriggerSearchHighlight(oldResults); } @@ -1716,7 +1716,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { const auto lock = _terminal->LockForWriting(); _terminal->SetSearchHighlights({}); - _terminal->SetSearchHighlightFocused({}); + _terminal->SetSearchHighlightFocused({}, 0); _renderer->TriggerSearchHighlight(_searcher.Results()); _searcher = {}; } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index f12d76bcc0b..1b20be8840b 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -219,7 +219,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SetSelectionAnchor(const til::point position); void SetEndSelectionPoint(const til::point position); - SearchResults Search(const std::wstring_view& text, bool goForward, bool caseSensitive, bool regularExpression, bool reset); + SearchResults Search(SearchRequest request); const std::vector& SearchResultRows() const noexcept; void ClearSearch(); void SnapSearchResultToSelection(bool snap) noexcept; diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index 4ded4e2ed89..933997a2341 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -49,6 +49,16 @@ namespace Microsoft.Terminal.Control Boolean EndAtRightBoundary; }; + struct SearchRequest + { + String Text; + Boolean GoForward; + Boolean CaseSensitive; + Boolean RegularExpression; + Boolean Reset; + Int32 ScrollOffset; + }; + struct SearchResults { Int32 TotalMatches; @@ -135,7 +145,7 @@ namespace Microsoft.Terminal.Control void ResumeRendering(); void BlinkAttributeTick(); - SearchResults Search(String text, Boolean goForward, Boolean caseSensitive, Boolean regularExpression, Boolean reset); + SearchResults Search(SearchRequest request); void ClearSearch(); Boolean SnapSearchResultToSelection; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index ed714e004e0..986071ae014 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -547,6 +547,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _searchBox->Open([weakThis = get_weak()]() { if (const auto self = weakThis.get(); self && !self->_IsClosing()) { + self->_searchScrollOffset = self->_calculateSearchScrollOffset(); self->_searchBox->SetFocusOnTextbox(); self->_refreshSearch(); } @@ -568,7 +569,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else { - _handleSearchResults(_core.Search(_searchBox->Text(), goForward, _searchBox->CaseSensitive(), _searchBox->RegularExpression(), false)); + const auto request = SearchRequest{ _searchBox->Text(), goForward, _searchBox->CaseSensitive(), _searchBox->RegularExpression(), false, _searchScrollOffset }; + _handleSearchResults(_core.Search(request)); } } @@ -602,7 +604,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { if (_searchBox && _searchBox->IsOpen()) { - _handleSearchResults(_core.Search(text, goForward, caseSensitive, regularExpression, false)); + const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, false, _searchScrollOffset }; + _handleSearchResults(_core.Search(request)); } } @@ -623,7 +626,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // We only want to update the search results based on the new text. Set // `resetOnly` to true so we don't accidentally update the current match index. - const auto result = _core.Search(text, goForward, caseSensitive, regularExpression, true); + const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset }; + const auto result = _core.Search(request); _handleSearchResults(result); } } @@ -3629,6 +3633,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation }; scaleMarker(SelectionStartMarker()); scaleMarker(SelectionEndMarker()); + + _searchScrollOffset = _calculateSearchScrollOffset(); } void TermControl::_coreRaisedNotice(const IInspectable& /*sender*/, @@ -3750,7 +3756,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto goForward = _searchBox->GoForward(); const auto caseSensitive = _searchBox->CaseSensitive(); const auto regularExpression = _searchBox->RegularExpression(); - _handleSearchResults(_core.Search(text, goForward, caseSensitive, regularExpression, true)); + const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() }; + _handleSearchResults(_core.Search(request)); } void TermControl::_handleSearchResults(SearchResults results) @@ -3919,6 +3926,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation _showContextMenuAt(_toControlOrigin(cursorPos)); } + til::CoordType TermControl::_calculateSearchScrollOffset() const + { + auto result = 0; + if (_searchBox) + { + const auto displayInfo = DisplayInformation::GetForCurrentView(); + const auto scaleFactor = _core.FontSize().Height / displayInfo.RawPixelsPerViewPixel(); + const auto searchBoxRows = _searchBox->ActualHeight() / scaleFactor; + result = static_cast(std::ceil(searchBoxRows)); + } + return result; + } + void TermControl::_PasteCommandHandler(const IInspectable& /*sender*/, const IInspectable& /*args*/) { diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 5fe1a7b2be1..b0c3d942de3 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -303,6 +303,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _isBackgroundLight{ false }; bool _detached{ false }; + til::CoordType _searchScrollOffset = 0; Windows::Foundation::Collections::IObservableVector _originalPrimaryElements{ nullptr }; Windows::Foundation::Collections::IObservableVector _originalSecondaryElements{ nullptr }; @@ -420,6 +421,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void _contextMenuHandler(IInspectable sender, Control::ContextMenuRequestedEventArgs args); void _showContextMenuAt(const til::point& controlRelativePos); + til::CoordType _calculateSearchScrollOffset() const; + void _PasteCommandHandler(const IInspectable& sender, const IInspectable& args); void _CopyCommandHandler(const IInspectable& sender, const IInspectable& args); void _SearchCommandHandler(const IInspectable& sender, const IInspectable& args); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 7df91d033ef..2c4279470e6 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1238,7 +1238,7 @@ void Terminal::SetSearchHighlights(const std::vector& highlight // Method Description: // - Stores the focused search highlighted region in the terminal // - If the region isn't empty, it will be brought into view -void Terminal::SetSearchHighlightFocused(const size_t focusedIdx) +void Terminal::SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset) { _assertLocked(); _searchHighlightFocused = focusedIdx; @@ -1247,7 +1247,9 @@ void Terminal::SetSearchHighlightFocused(const size_t focusedIdx) if (focusedIdx < _searchHighlights.size()) { const auto focused = til::at(_searchHighlights, focusedIdx); - _ScrollToPoints(focused.start, focused.end); + const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) }; + const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) }; + _ScrollToPoints(adjustedStart, adjustedEnd); } } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 75b822b1386..db5cf3d027a 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -231,7 +231,7 @@ class Microsoft::Terminal::Core::Terminal final : void SetPlayMidiNoteCallback(std::function pfn) noexcept; void CompletionsChangedCallback(std::function pfn) noexcept; void SetSearchHighlights(const std::vector& highlights) noexcept; - void SetSearchHighlightFocused(const size_t focusedIdx); + void SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset); void BlinkCursor() noexcept; void SetCursorOn(const bool isOn) noexcept;