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

e82eric · web-flow · commit 0bafab9a0ff8 · 2024-08-07T08:32:16.000+02:00
## Summary of the Pull Request 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
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -1689,22 +1689,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())
                 {
@@ -1716,12 +1716,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);
         }
@@ -1751,7 +1751,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         const auto lock = _terminal->LockForWriting();
         _terminal->SetSearchHighlights({});
-        _terminal->SetSearchHighlightFocused({});
+        _terminal->SetSearchHighlightFocused({}, 0);
         _renderer->TriggerSearchHighlight(_searcher.Results());
         _searcher = {};
     }
@@ -2938,5 +2938,4 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         _terminal->PreviewText(input);
     }
-
 }
diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h
@@ -225,7 +225,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;
@@ -136,7 +146,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
@@ -584,6 +584,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();
                     }
@@ -605,7 +606,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));
         }
     }
 
@@ -639,7 +641,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));
         }
     }
 
@@ -660,7 +663,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);
         }
     }
@@ -3572,6 +3576,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             QuickFixIcon().FontSize(static_cast<double>(args.Width() / dpiScale));
             RefreshQuickFixMenu();
         }
+
+        _searchScrollOffset = _calculateSearchScrollOffset();
     }
 
     void TermControl::_coreRaisedNotice(const IInspectable& /*sender*/,
@@ -3702,7 +3708,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)
@@ -3982,6 +3989,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         SearchMissingCommand.raise(*this, args);
     }
 
+    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::ClearQuickFix()
     {
         _core.ClearQuickFix();
diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h
@@ -289,6 +289,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 };
@@ -409,6 +410,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _showContextMenuAt(const til::point& controlRelativePos);
 
         void _bubbleSearchMissingCommand(const IInspectable& sender, const Control::SearchMissingCommandEventArgs& args);
+        til::CoordType _calculateSearchScrollOffset() const;
 
         void _PasteCommandHandler(const IInspectable& sender, const IInspectable& args);
         void _CopyCommandHandler(const IInspectable& sender, const IInspectable& args);
diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
@@ -1251,7 +1251,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;
@@ -1260,7 +1260,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
@@ -232,7 +232,7 @@ class Microsoft::Terminal::Core::Terminal final :
     void SetSearchMissingCommandCallback(std::function<void(std::wstring_view)> pfn) noexcept;
     void SetClearQuickFixCallback(std::function<void()> 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
`@@ -1689,22 +1689,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`1689`	`1689`	`// - resetOnly: If true, only Reset() will be called, if anything. FindNext() will never be called.`
`1690`	`1690`	`// Return Value:`
`1691`	`1691`	`// - <none>`
`1692`		`- SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool regularExpression, const bool resetOnly)`
	`1692`	`+ SearchResults ControlCore::Search(SearchRequest request)`
`1693`	`1693`	`{`
`1694`	`1694`	`const auto lock = _terminal->LockForWriting();`
`1695`	`1695`	`SearchFlag flags{};`
`1696`		`- WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !caseSensitive);`
`1697`		`- WI_SetFlagIf(flags, SearchFlag::RegularExpression, regularExpression);`
`1698`		`- const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), text, flags);`
	`1696`	`+ WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !request.CaseSensitive);`
	`1697`	`+ WI_SetFlagIf(flags, SearchFlag::RegularExpression, request.RegularExpression);`
	`1698`	`+ const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), request.Text, flags);`
`1699`	`1699`
`1700`		`- if (searchInvalidated \|\| !resetOnly)`
	`1700`	`+ if (searchInvalidated \|\| !request.Reset)`
`1701`	`1701`	`{`
`1702`	`1702`	`std::vector<til::point_span> oldResults;`
`1703`	`1703`
`1704`	`1704`	`if (searchInvalidated)`
`1705`	`1705`	`{`
`1706`	`1706`	`oldResults = _searcher.ExtractResults();`
`1707`		`- _searcher.Reset(*_terminal.get(), text, flags, !goForward);`
	`1707`	`+ _searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward);`
`1708`	`1708`
`1709`	`1709`	`if (SnapSearchResultToSelection())`
`1710`	`1710`	`{`
`@@ -1716,12 +1716,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`1716`	`1716`	`}`
`1717`	`1717`	`else`
`1718`	`1718`	`{`
`1719`		`- _searcher.FindNext(!goForward);`
	`1719`	`+ _searcher.FindNext(!request.GoForward);`
`1720`	`1720`	`}`
`1721`	`1721`
`1722`	`1722`	`if (const auto idx = _searcher.CurrentMatch(); idx >= 0)`
`1723`	`1723`	`{`
`1724`		`- _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));`
	`1724`	`+ _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx), request.ScrollOffset);`
`1725`	`1725`	`}`
`1726`	`1726`	`_renderer->TriggerSearchHighlight(oldResults);`
`1727`	`1727`	`}`
`@@ -1751,7 +1751,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`1751`	`1751`	`{`
`1752`	`1752`	`const auto lock = _terminal->LockForWriting();`
`1753`	`1753`	`_terminal->SetSearchHighlights({});`
`1754`		`- _terminal->SetSearchHighlightFocused({});`
	`1754`	`+ _terminal->SetSearchHighlightFocused({}, 0);`
`1755`	`1755`	`_renderer->TriggerSearchHighlight(_searcher.Results());`
`1756`	`1756`	`_searcher = {};`
`1757`	`1757`	`}`
`@@ -2938,5 +2938,4 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`2938`	`2938`	`{`
`2939`	`2939`	`_terminal->PreviewText(input);`
`2940`	`2940`	`}`
`2941`		`-`
`2942`	`2941`	`}`
Original file line number	Diff line number	Diff line change
`@@ -584,6 +584,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`584`	`584`	`_searchBox->Open([weakThis = get_weak()]() {`
`585`	`585`	`if (const auto self = weakThis.get(); self && !self->_IsClosing())`
`586`	`586`	`{`
	`587`	`+ self->_searchScrollOffset = self->_calculateSearchScrollOffset();`
`587`	`588`	`self->_searchBox->SetFocusOnTextbox();`
`588`	`589`	`self->_refreshSearch();`
`589`	`590`	`}`
`@@ -605,7 +606,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`605`	`606`	`}`
`606`	`607`	`else`
`607`	`608`	`{`
`608`		`- _handleSearchResults(_core.Search(_searchBox->Text(), goForward, _searchBox->CaseSensitive(), _searchBox->RegularExpression(), false));`
	`609`	`+ const auto request = SearchRequest{ _searchBox->Text(), goForward, _searchBox->CaseSensitive(), _searchBox->RegularExpression(), false, _searchScrollOffset };`
	`610`	`+ _handleSearchResults(_core.Search(request));`
`609`	`611`	`}`
`610`	`612`	`}`
`611`	`613`
`@@ -639,7 +641,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`639`	`641`	`{`
`640`	`642`	`if (_searchBox && _searchBox->IsOpen())`
`641`	`643`	`{`
`642`		`- _handleSearchResults(_core.Search(text, goForward, caseSensitive, regularExpression, false));`
	`644`	`+ const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, false, _searchScrollOffset };`
	`645`	`+ _handleSearchResults(_core.Search(request));`
`643`	`646`	`}`
`644`	`647`	`}`
`645`	`648`
`@@ -660,7 +663,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`660`	`663`	`{`
`661`	`664`	`// We only want to update the search results based on the new text. Set`
`662`	`665`	// `resetOnly` to true so we don't accidentally update the current match index.
`663`		`- const auto result = _core.Search(text, goForward, caseSensitive, regularExpression, true);`
	`666`	`+ const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset };`
	`667`	`+ const auto result = _core.Search(request);`
`664`	`668`	`_handleSearchResults(result);`
`665`	`669`	`}`
`666`	`670`	`}`
`@@ -3572,6 +3576,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`3572`	`3576`	`QuickFixIcon().FontSize(static_cast<double>(args.Width() / dpiScale));`
`3573`	`3577`	`RefreshQuickFixMenu();`
`3574`	`3578`	`}`
	`3579`	`+`
	`3580`	`+ _searchScrollOffset = _calculateSearchScrollOffset();`
`3575`	`3581`	`}`
`3576`	`3582`
`3577`	`3583`	`void TermControl::_coreRaisedNotice(const IInspectable& /sender/,`
`@@ -3702,7 +3708,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`3702`	`3708`	`const auto goForward = _searchBox->GoForward();`
`3703`	`3709`	`const auto caseSensitive = _searchBox->CaseSensitive();`
`3704`	`3710`	`const auto regularExpression = _searchBox->RegularExpression();`
`3705`		`- _handleSearchResults(_core.Search(text, goForward, caseSensitive, regularExpression, true));`
	`3711`	`+ const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() };`
	`3712`	`+ _handleSearchResults(_core.Search(request));`
`3706`	`3713`	`}`
`3707`	`3714`
`3708`	`3715`	`void TermControl::_handleSearchResults(SearchResults results)`
`@@ -3982,6 +3989,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation`
`3982`	`3989`	`SearchMissingCommand.raise(*this, args);`
`3983`	`3990`	`}`
`3984`	`3991`
	`3992`	`+ til::CoordType TermControl::_calculateSearchScrollOffset() const`
	`3993`	`+ {`
	`3994`	`+ auto result = 0;`
	`3995`	`+ if (_searchBox)`
	`3996`	`+ {`
	`3997`	`+ const auto displayInfo = DisplayInformation::GetForCurrentView();`
	`3998`	`+ const auto scaleFactor = _core.FontSize().Height / displayInfo.RawPixelsPerViewPixel();`
	`3999`	`+ const auto searchBoxRows = _searchBox->ActualHeight() / scaleFactor;`
	`4000`	`+ result = static_cast<int32_t>(std::ceil(searchBoxRows));`
	`4001`	`+ }`
	`4002`	`+ return result;`
	`4003`	`+ }`
	`4004`	`+`
`3985`	`4005`	`void TermControl::ClearQuickFix()`
`3986`	`4006`	`{`
`3987`	`4007`	`_core.ClearQuickFix();`
Original file line number	Diff line number	Diff line change
`@@ -1251,7 +1251,7 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& highlight`
`1251`	`1251`	`// Method Description:`
`1252`	`1252`	`// - Stores the focused search highlighted region in the terminal`
`1253`	`1253`	`// - If the region isn't empty, it will be brought into view`
`1254`		`-void Terminal::SetSearchHighlightFocused(const size_t focusedIdx)`
	`1254`	`+void Terminal::SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset)`
`1255`	`1255`	`{`
`1256`	`1256`	`_assertLocked();`
`1257`	`1257`	`_searchHighlightFocused = focusedIdx;`
`@@ -1260,7 +1260,9 @@ void Terminal::SetSearchHighlightFocused(const size_t focusedIdx)`
`1260`	`1260`	`if (focusedIdx < _searchHighlights.size())`
`1261`	`1261`	`{`
`1262`	`1262`	`const auto focused = til::at(_searchHighlights, focusedIdx);`
`1263`		`- _ScrollToPoints(focused.start, focused.end);`
	`1263`	`+ const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) };`
	`1264`	`+ const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) };`
	`1265`	`+ _ScrollToPoints(adjustedStart, adjustedEnd);`
`1264`	`1266`	`}`
`1265`	`1267`	`}`
`1266`	`1268`