Avoid covering current search highlight with search box (#17516)

e82eric · DHowett · commit d1ac37599cd2 · 2024-09-24T14:38:24.000-05:00
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 0bafab9) Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgTTNTE Service-Version: 1.21
diff --git 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:
     // - <none>
-    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<til::point_span> 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<size_t>(idx));
+                _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(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
@@ -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<til::point_span>& SearchResultRows() const noexcept;
         void ClearSearch();
         void SnapSearchResultToSelection(bool snap) noexcept;
diff --git 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
@@ -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<int32_t>(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
@@ -303,6 +303,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         bool _isBackgroundLight{ false };
         bool _detached{ false };
+        til::CoordType _searchScrollOffset = 0;
 
         Windows::Foundation::Collections::IObservableVector<Windows::UI::Xaml::Controls::ICommandBarElement> _originalPrimaryElements{ nullptr };
         Windows::Foundation::Collections::IObservableVector<Windows::UI::Xaml::Controls::ICommandBarElement> _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
@@ -1238,7 +1238,7 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& 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
@@ -231,7 +231,7 @@ class Microsoft::Terminal::Core::Terminal final :
     void SetPlayMidiNoteCallback(std::function<void(const int, const int, const std::chrono::microseconds)> pfn) noexcept;
     void CompletionsChangedCallback(std::function<void(std::wstring_view, unsigned int)> pfn) noexcept;
     void SetSearchHighlights(const std::vector<til::point_span>& 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;

Original file line number	Diff line number	Diff line change
`@@ -1654,22 +1654,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`1654`	`1654`	`// - resetOnly: If true, only Reset() will be called, if anything. FindNext() will never be called.`
`1655`	`1655`	`// Return Value:`
`1656`	`1656`	`// - <none>`
`1657`		`- SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool regularExpression, const bool resetOnly)`
	`1657`	`+ SearchResults ControlCore::Search(SearchRequest request)`
`1658`	`1658`	`{`
`1659`	`1659`	`const auto lock = _terminal->LockForWriting();`
`1660`	`1660`	`SearchFlag flags{};`
`1661`		`- WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !caseSensitive);`
`1662`		`- WI_SetFlagIf(flags, SearchFlag::RegularExpression, regularExpression);`
`1663`		`- const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), text, flags);`
	`1661`	`+ WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !request.CaseSensitive);`
	`1662`	`+ WI_SetFlagIf(flags, SearchFlag::RegularExpression, request.RegularExpression);`
	`1663`	`+ const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), request.Text, flags);`
`1664`	`1664`
`1665`		`- if (searchInvalidated \|\| !resetOnly)`
	`1665`	`+ if (searchInvalidated \|\| !request.Reset)`
`1666`	`1666`	`{`
`1667`	`1667`	`std::vector<til::point_span> oldResults;`
`1668`	`1668`
`1669`	`1669`	`if (searchInvalidated)`
`1670`	`1670`	`{`
`1671`	`1671`	`oldResults = _searcher.ExtractResults();`
`1672`		`- _searcher.Reset(*_terminal.get(), text, flags, !goForward);`
	`1672`	`+ _searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward);`
`1673`	`1673`
`1674`	`1674`	`if (SnapSearchResultToSelection())`
`1675`	`1675`	`{`
`@@ -1681,12 +1681,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`1681`	`1681`	`}`
`1682`	`1682`	`else`
`1683`	`1683`	`{`
`1684`		`- _searcher.FindNext(!goForward);`
	`1684`	`+ _searcher.FindNext(!request.GoForward);`
`1685`	`1685`	`}`
`1686`	`1686`
`1687`	`1687`	`if (const auto idx = _searcher.CurrentMatch(); idx >= 0)`
`1688`	`1688`	`{`
`1689`		`- _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));`
	`1689`	`+ _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx), request.ScrollOffset);`
`1690`	`1690`	`}`
`1691`	`1691`	`_renderer->TriggerSearchHighlight(oldResults);`
`1692`	`1692`	`}`
`@@ -1716,7 +1716,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`1716`	`1716`	`{`
`1717`	`1717`	`const auto lock = _terminal->LockForWriting();`
`1718`	`1718`	`_terminal->SetSearchHighlights({});`
`1719`		`- _terminal->SetSearchHighlightFocused({});`
	`1719`	`+ _terminal->SetSearchHighlightFocused({}, 0);`
`1720`	`1720`	`_renderer->TriggerSearchHighlight(_searcher.Results());`
`1721`	`1721`	`_searcher = {};`
`1722`	`1722`	`}`
Original file line number	Diff line number	Diff line change
`@@ -547,6 +547,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`547`	`547`	`_searchBox->Open([weakThis = get_weak()]() {`
`548`	`548`	`if (const auto self = weakThis.get(); self && !self->_IsClosing())`
`549`	`549`	`{`
	`550`	`+ self->_searchScrollOffset = self->_calculateSearchScrollOffset();`
`550`	`551`	`self->_searchBox->SetFocusOnTextbox();`
`551`	`552`	`self->_refreshSearch();`
`552`	`553`	`}`
`@@ -568,7 +569,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`568`	`569`	`}`
`569`	`570`	`else`
`570`	`571`	`{`
`571`		`- _handleSearchResults(_core.Search(_searchBox->Text(), goForward, _searchBox->CaseSensitive(), _searchBox->RegularExpression(), false));`
	`572`	`+ const auto request = SearchRequest{ _searchBox->Text(), goForward, _searchBox->CaseSensitive(), _searchBox->RegularExpression(), false, _searchScrollOffset };`
	`573`	`+ _handleSearchResults(_core.Search(request));`
`572`	`574`	`}`
`573`	`575`	`}`
`574`	`576`
`@@ -602,7 +604,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`602`	`604`	`{`
`603`	`605`	`if (_searchBox && _searchBox->IsOpen())`
`604`	`606`	`{`
`605`		`- _handleSearchResults(_core.Search(text, goForward, caseSensitive, regularExpression, false));`
	`607`	`+ const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, false, _searchScrollOffset };`
	`608`	`+ _handleSearchResults(_core.Search(request));`
`606`	`609`	`}`
`607`	`610`	`}`
`608`	`611`
`@@ -623,7 +626,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`623`	`626`	`{`
`624`	`627`	`// We only want to update the search results based on the new text. Set`
`625`	`628`	// `resetOnly` to true so we don't accidentally update the current match index.
`626`		`- const auto result = _core.Search(text, goForward, caseSensitive, regularExpression, true);`
	`629`	`+ const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset };`
	`630`	`+ const auto result = _core.Search(request);`
`627`	`631`	`_handleSearchResults(result);`
`628`	`632`	`}`
`629`	`633`	`}`
`@@ -3629,6 +3633,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`3629`	`3633`	`};`
`3630`	`3634`	`scaleMarker(SelectionStartMarker());`
`3631`	`3635`	`scaleMarker(SelectionEndMarker());`
	`3636`	`+`
	`3637`	`+ _searchScrollOffset = _calculateSearchScrollOffset();`
`3632`	`3638`	`}`
`3633`	`3639`
`3634`	`3640`	`void TermControl::_coreRaisedNotice(const IInspectable& /sender/,`
`@@ -3750,7 +3756,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`3750`	`3756`	`const auto goForward = _searchBox->GoForward();`
`3751`	`3757`	`const auto caseSensitive = _searchBox->CaseSensitive();`
`3752`	`3758`	`const auto regularExpression = _searchBox->RegularExpression();`
`3753`		`- _handleSearchResults(_core.Search(text, goForward, caseSensitive, regularExpression, true));`
	`3759`	`+ const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() };`
	`3760`	`+ _handleSearchResults(_core.Search(request));`
`3754`	`3761`	`}`
`3755`	`3762`
`3756`	`3763`	`void TermControl::_handleSearchResults(SearchResults results)`
`@@ -3919,6 +3926,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`3919`	`3926`	`_showContextMenuAt(_toControlOrigin(cursorPos));`
`3920`	`3927`	`}`
`3921`	`3928`
	`3929`	`+ til::CoordType TermControl::_calculateSearchScrollOffset() const`
	`3930`	`+ {`
	`3931`	`+ auto result = 0;`
	`3932`	`+ if (_searchBox)`
	`3933`	`+ {`
	`3934`	`+ const auto displayInfo = DisplayInformation::GetForCurrentView();`
	`3935`	`+ const auto scaleFactor = _core.FontSize().Height / displayInfo.RawPixelsPerViewPixel();`
	`3936`	`+ const auto searchBoxRows = _searchBox->ActualHeight() / scaleFactor;`
	`3937`	`+ result = static_cast<int32_t>(std::ceil(searchBoxRows));`
	`3938`	`+ }`
	`3939`	`+ return result;`
	`3940`	`+ }`
	`3941`	`+`
`3922`	`3942`	`void TermControl::_PasteCommandHandler(const IInspectable& /sender/,`
`3923`	`3943`	`const IInspectable& /args/)`
`3924`	`3944`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1238,7 +1238,7 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& highlight`
`1238`	`1238`	`// Method Description:`
`1239`	`1239`	`// - Stores the focused search highlighted region in the terminal`
`1240`	`1240`	`// - If the region isn't empty, it will be brought into view`
`1241`		`-void Terminal::SetSearchHighlightFocused(const size_t focusedIdx)`
	`1241`	`+void Terminal::SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset)`
`1242`	`1242`	`{`
`1243`	`1243`	`_assertLocked();`
`1244`	`1244`	`_searchHighlightFocused = focusedIdx;`
`@@ -1247,7 +1247,9 @@ void Terminal::SetSearchHighlightFocused(const size_t focusedIdx)`
`1247`	`1247`	`if (focusedIdx < _searchHighlights.size())`
`1248`	`1248`	`{`
`1249`	`1249`	`const auto focused = til::at(_searchHighlights, focusedIdx);`
`1250`		`- _ScrollToPoints(focused.start, focused.end);`
	`1250`	`+ const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) };`
	`1251`	`+ const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) };`
	`1252`	`+ _ScrollToPoints(adjustedStart, adjustedEnd);`
`1251`	`1253`	`}`
`1252`	`1254`	`}`
`1253`	`1255`