From 9b8c0602a2bcf12d23371930db5e3178ba52590c Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 24 Sep 2024 21:06:36 +0200 Subject: [PATCH] Stop scrolling on output when search is open (#17885) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don't reset the position entirely when changing the needle * Don't change the scroll position when output arrives * Don't interfere with the search when output arrives constantly Closes #17301 ## Validation Steps Performed * In pwsh, run `10000..20000 | % { sleep 0.25; $_ }` * You can search for e.g. `1004` and it'll find 10 results. ✅ * You can scroll up and down past it and it won't snap back when new output arrives. ✅ * `while ($true) { Write-Host -NoNewline "`e[Ha"; sleep 0.0001; }` * You can cycle between the hits effortlessly. ✅ (This tests that the constantly reset `OutputIdle` event won't interfere.) * On input change, the focused result is near the previous one. ✅ (cherry picked from commit d9131c68893fe114a6091d262c23a5a26199d023) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgS3elo PVTI_lADOAF3p4s4AmhmQzgTEciM Service-Version: 1.22 --- src/buffer/out/search.cpp | 10 ++--- src/buffer/out/search.h | 3 +- src/cascadia/TerminalControl/ControlCore.cpp | 45 +++++++------------- src/cascadia/TerminalControl/ControlCore.h | 2 - src/cascadia/TerminalControl/ControlCore.idl | 3 +- src/cascadia/TerminalControl/TermControl.cpp | 3 +- src/cascadia/TerminalCore/Terminal.cpp | 10 +++-- src/cascadia/TerminalCore/Terminal.hpp | 3 +- src/interactivity/win32/find.cpp | 1 - 9 files changed, 32 insertions(+), 48 deletions(-) diff --git a/src/buffer/out/search.cpp b/src/buffer/out/search.cpp index 31c7081b2b9..75143f047ca 100644 --- a/src/buffer/out/search.cpp +++ b/src/buffer/out/search.cpp @@ -16,7 +16,7 @@ bool Search::IsStale(const Microsoft::Console::Render::IRenderData& renderData, _lastMutationId != renderData.GetTextBuffer().GetLastMutationId(); } -bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse) +void Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse) { const auto& textBuffer = renderData.GetTextBuffer(); @@ -30,15 +30,15 @@ bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const st _results = std::move(result).value_or(std::vector{}); _index = reverse ? gsl::narrow_cast(_results.size()) - 1 : 0; _step = reverse ? -1 : 1; - return true; -} -void Search::MoveToCurrentSelection() -{ if (_renderData->IsSelectionActive()) { MoveToPoint(_renderData->GetTextBuffer().ScreenToBufferPosition(_renderData->GetSelectionAnchor())); } + else if (const auto span = _renderData->GetSearchHighlightFocused()) + { + MoveToPoint(_step > 0 ? span->start : span->end); + } } void Search::MoveToPoint(const til::point anchor) noexcept diff --git a/src/buffer/out/search.h b/src/buffer/out/search.h index 763b5d40c73..304de1ab889 100644 --- a/src/buffer/out/search.h +++ b/src/buffer/out/search.h @@ -36,9 +36,8 @@ class Search final Search() = default; bool IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags) const noexcept; - bool Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse); + void Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse); - void MoveToCurrentSelection(); void MoveToPoint(til::point anchor) noexcept; void MovePastPoint(til::point anchor) noexcept; void FindNext(bool reverse) noexcept; diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 9be62aad0c3..3880b3188de 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1688,38 +1688,41 @@ namespace winrt::Microsoft::Terminal::Control::implementation SearchResults ControlCore::Search(SearchRequest request) { const auto lock = _terminal->LockForWriting(); + SearchFlag 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 || !request.Reset) + if (searchInvalidated || !request.ResetOnly) { std::vector oldResults; + til::point_span oldFocused; + + if (const auto focused = _terminal->GetSearchHighlightFocused()) + { + oldFocused = *focused; + } if (searchInvalidated) { oldResults = _searcher.ExtractResults(); _searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward); - - if (SnapSearchResultToSelection()) - { - _searcher.MoveToCurrentSelection(); - SnapSearchResultToSelection(false); - } - _terminal->SetSearchHighlights(_searcher.Results()); } - else + + if (!request.ResetOnly) { _searcher.FindNext(!request.GoForward); } - if (const auto idx = _searcher.CurrentMatch(); idx >= 0) + _terminal->SetSearchHighlightFocused(gsl::narrow(std::max(0, _searcher.CurrentMatch()))); + _renderer->TriggerSearchHighlight(oldResults); + + if (const auto focused = _terminal->GetSearchHighlightFocused(); focused && *focused != oldFocused) { - _terminal->SetSearchHighlightFocused(gsl::narrow(idx), request.ScrollOffset); + _terminal->ScrollToSearchHighlight(request.ScrollOffset); } - _renderer->TriggerSearchHighlight(oldResults); } int32_t totalMatches = 0; @@ -1747,27 +1750,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation { const auto lock = _terminal->LockForWriting(); _terminal->SetSearchHighlights({}); - _terminal->SetSearchHighlightFocused({}, 0); + _terminal->SetSearchHighlightFocused(0); _renderer->TriggerSearchHighlight(_searcher.Results()); _searcher = {}; } - // Method Description: - // - Tells ControlCore to snap the current search result index to currently - // selected text if the search was started using it. - void ControlCore::SnapSearchResultToSelection(bool shouldSnap) noexcept - { - _snapSearchResultToSelection = shouldSnap; - } - - // Method Description: - // - Returns true, if we should snap the current search result index to - // the currently selected text after a new search is started, else false. - bool ControlCore::SnapSearchResultToSelection() const noexcept - { - return _snapSearchResultToSelection; - } - void ControlCore::Close() { if (!_IsClosing()) diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 648cee15915..3c0d8e30a1a 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -228,8 +228,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation SearchResults Search(SearchRequest request); const std::vector& SearchResultRows() const noexcept; void ClearSearch(); - void SnapSearchResultToSelection(bool snap) noexcept; - bool SnapSearchResultToSelection() const noexcept; void LeftClickOnTerminal(const til::point terminalPosition, const int numberOfClicks, diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index db1d314b23c..ce9f3572ab0 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -55,7 +55,7 @@ namespace Microsoft.Terminal.Control Boolean GoForward; Boolean CaseSensitive; Boolean RegularExpression; - Boolean Reset; + Boolean ResetOnly; Int32 ScrollOffset; }; @@ -148,7 +148,6 @@ namespace Microsoft.Terminal.Control SearchResults Search(SearchRequest request); void ClearSearch(); - Boolean SnapSearchResultToSelection; Microsoft.Terminal.Core.Color ForegroundColor { get; }; Microsoft.Terminal.Core.Color BackgroundColor { get; }; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index d8a6f82e802..74c9a679ca6 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -575,7 +575,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // but since code paths differ, extra work is required to ensure correctness. if (!_core.HasMultiLineSelection()) { - _core.SnapSearchResultToSelection(true); const auto selectedLine{ _core.SelectedText(true) }; _searchBox->PopulateTextbox(selectedLine); } @@ -3824,7 +3823,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto goForward = _searchBox->GoForward(); const auto caseSensitive = _searchBox->CaseSensitive(); const auto regularExpression = _searchBox->RegularExpression(); - const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() }; + const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset }; _handleSearchResults(_core.Search(request)); } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index b745d66f704..6603d91c57e 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1254,15 +1254,17 @@ 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, til::CoordType searchScrollOffset) +void Terminal::SetSearchHighlightFocused(const size_t focusedIdx) noexcept { _assertLocked(); _searchHighlightFocused = focusedIdx; +} - // bring the focused region into the view if the index is in valid range - if (focusedIdx < _searchHighlights.size()) +void Terminal::ScrollToSearchHighlight(til::CoordType searchScrollOffset) +{ + if (_searchHighlightFocused < _searchHighlights.size()) { - const auto focused = til::at(_searchHighlights, focusedIdx); + const auto focused = til::at(_searchHighlights, _searchHighlightFocused); 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 34d92213d82..89340fab016 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -233,7 +233,8 @@ class Microsoft::Terminal::Core::Terminal final : void SetSearchMissingCommandCallback(std::function pfn) noexcept; void SetClearQuickFixCallback(std::function pfn) noexcept; void SetSearchHighlights(const std::vector& highlights) noexcept; - void SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset); + void SetSearchHighlightFocused(size_t focusedIdx) noexcept; + void ScrollToSearchHighlight(til::CoordType searchScrollOffset); void BlinkCursor() noexcept; void SetCursorOn(const bool isOn) noexcept; diff --git a/src/interactivity/win32/find.cpp b/src/interactivity/win32/find.cpp index 033918e3ae5..07996c47767 100644 --- a/src/interactivity/win32/find.cpp +++ b/src/interactivity/win32/find.cpp @@ -57,7 +57,6 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l if (searcher.IsStale(gci.renderData, lastFindString, flags)) { searcher.Reset(gci.renderData, lastFindString, flags, reverse); - searcher.MoveToCurrentSelection(); } else {