From b32d5fc599d5a1f1f7a109609baf9912bb57724c Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 19 Sep 2023 17:50:34 +0200 Subject: [PATCH] Avoid moving the selection while typing a search query --- src/buffer/out/search.cpp | 52 +++++++++++++------- src/buffer/out/search.h | 8 ++- src/cascadia/TerminalControl/ControlCore.cpp | 38 +++++++------- src/interactivity/win32/find.cpp | 2 +- 4 files changed, 55 insertions(+), 45 deletions(-) diff --git a/src/buffer/out/search.cpp b/src/buffer/out/search.cpp index eef5fac17cb..9707e8fdeaa 100644 --- a/src/buffer/out/search.cpp +++ b/src/buffer/out/search.cpp @@ -8,30 +8,21 @@ using namespace Microsoft::Console::Types; -bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData) -{ - return ResetIfStale(renderData, - _needle, - _step == -1, // this is the opposite of the initializer below - _caseInsensitive); -} - bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive) { const auto& textBuffer = renderData.GetTextBuffer(); const auto lastMutationId = textBuffer.GetLastMutationId(); if (_needle == needle && - _reverse == reverse && _caseInsensitive == caseInsensitive && _lastMutationId == lastMutationId) { + _step = reverse ? -1 : 1; return false; } _renderData = &renderData; _needle = needle; - _reverse = reverse; _caseInsensitive = caseInsensitive; _lastMutationId = lastMutationId; @@ -42,12 +33,38 @@ bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, c return true; } -void Search::MovePastCurrentSelection() +void Search::MoveToCurrentSelection() { if (_renderData->IsSelectionActive()) { - MovePastPoint(_renderData->GetTextBuffer().ScreenToBufferPosition(_renderData->GetSelectionAnchor())); + MoveToPoint(_renderData->GetTextBuffer().ScreenToBufferPosition(_renderData->GetSelectionAnchor())); + } +} + +void Search::MoveToPoint(const til::point anchor) noexcept +{ + if (_results.empty()) + { + return; + } + + const auto count = gsl::narrow_cast(_results.size()); + ptrdiff_t index = 0; + + if (_step < 0) + { + for (index = count - 1; index >= 0 && til::at(_results, index).start > anchor; --index) + { + } + } + else + { + for (index = 0; index < count && til::at(_results, index).start < anchor; ++index) + { + } } + + _index = (index + count) % count; } void Search::MovePastPoint(const til::point anchor) noexcept @@ -58,18 +75,17 @@ void Search::MovePastPoint(const til::point anchor) noexcept } const auto count = gsl::narrow_cast(_results.size()); - const auto highestIndex = count - 1; - auto index = _reverse ? highestIndex : 0; + ptrdiff_t index = 0; - if (_reverse) + if (_step < 0) { - for (; index >= 0 && til::at(_results, index).start >= anchor; --index) + for (index = count - 1; index >= 0 && til::at(_results, index).start >= anchor; --index) { } } else { - for (; index <= highestIndex && til::at(_results, index).start <= anchor; ++index) + for (index = 0; index < count && til::at(_results, index).start <= anchor; ++index) { } } @@ -119,7 +135,7 @@ const std::vector& Search::Results() const noexcept return _results; } -size_t Search::CurrentMatch() const noexcept +ptrdiff_t Search::CurrentMatch() const noexcept { return _index; } diff --git a/src/buffer/out/search.h b/src/buffer/out/search.h index 2c6bc80a724..c2d035e3e9c 100644 --- a/src/buffer/out/search.h +++ b/src/buffer/out/search.h @@ -25,10 +25,10 @@ class Search final public: Search() = default; - bool ResetIfStale(Microsoft::Console::Render::IRenderData& renderData); bool ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive); - void MovePastCurrentSelection(); + void MoveToCurrentSelection(); + void MoveToPoint(til::point anchor) noexcept; void MovePastPoint(til::point anchor) noexcept; void FindNext() noexcept; @@ -36,14 +36,12 @@ class Search final bool SelectCurrent() const; const std::vector& Results() const noexcept; - size_t CurrentMatch() const noexcept; - bool CurrentDirection() const noexcept; + ptrdiff_t CurrentMatch() const noexcept; private: // _renderData is a pointer so that Search() is constexpr default constructable. Microsoft::Console::Render::IRenderData* _renderData = nullptr; std::wstring _needle; - bool _reverse = false; bool _caseInsensitive = false; uint64_t _lastMutationId = 0; diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 52dc7d4087f..f1fc672aa73 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1569,7 +1569,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive)) { - _searcher.MovePastCurrentSelection(); + _searcher.MoveToCurrentSelection(); + _cachedSearchResultRows = {}; } else { @@ -1600,12 +1601,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation Windows::Foundation::Collections::IVector ControlCore::SearchResultRows() { - auto lock = _terminal->LockForWriting(); - if (_searcher.ResetIfStale(*GetRenderData())) + const auto lock = _terminal->LockForReading(); + + if (!_cachedSearchResultRows) { auto results = std::vector(); - auto lastRow = til::CoordTypeMin; + for (const auto& match : _searcher.Results()) { const auto row{ match.start.y }; @@ -1615,6 +1617,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation lastRow = row; } } + _cachedSearchResultRows = winrt::single_threaded_vector(std::move(results)); } @@ -2245,27 +2248,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation Windows::Foundation::Collections::IVector ControlCore::ScrollMarks() const { - auto internalMarks{ _terminal->GetScrollMarks() }; - auto v = winrt::single_threaded_observable_vector(); - for (const auto& mark : internalMarks) - { - Control::ScrollMark m{}; + const auto& internalMarks = _terminal->GetScrollMarks(); + std::vector v; - // sneaky: always evaluate the color of the mark to a real value - // before shoving it into the optional. If the mark doesn't have a - // specific color set, we'll use the value from the color table - // that's appropriate for this category of mark. If we do have a - // color set, then great we'll use that. The TermControl can then - // always use the value in the Mark regardless if it was actually - // set or not. - m.Color = OptionalFromColor(_terminal->GetColorForMark(mark)); - m.Start = mark.start.to_core_point(); - m.End = mark.end.to_core_point(); + v.reserve(internalMarks.size()); - v.Append(m); + for (const auto& mark : internalMarks) + { + v.emplace_back( + mark.start.to_core_point(), + mark.end.to_core_point(), + OptionalFromColor(_terminal->GetColorForMark(mark))); } - return v; + return winrt::single_threaded_vector(std::move(v)); } void ControlCore::AddMark(const Control::ScrollMark& mark) diff --git a/src/interactivity/win32/find.cpp b/src/interactivity/win32/find.cpp index 93c4d55a8b4..4e7cf36a589 100644 --- a/src/interactivity/win32/find.cpp +++ b/src/interactivity/win32/find.cpp @@ -54,7 +54,7 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l if (searcher.ResetIfStale(gci.renderData, lastFindString, reverse, caseInsensitive)) { - searcher.MovePastCurrentSelection(); + searcher.MoveToCurrentSelection(); } else {