microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-10 18:43:54 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Fix search constantly triggering a scroll (#17132)

Browse Source 
This addresses a review comment left by tusharsnx in #17092 which I
forgot to fix before merging the PR. The fix itself is somewhat simple:
`Terminal::SetSearchHighlightFocused` triggers a scroll if the target
is outside of the current (scrolled) viewport and avoiding the call
unless necessary fixes it. To do it properly though, I've split up
`Search::ResetIfStale` into `IsStale` and `Reset`. Now we can properly
detect staleness in advance and branch out the search reset cleanly.

Additionally, I've taken the liberty to replace the `IVector` in
`SearchResultRows` with a direct `const std::vector&` into `Searcher`.
This removes a bunch of code and makes it faster to boot.

## Validation Steps Performed
* Print lots of text
* Search a common letter
* Scroll up
* Doesn't scroll back down ✅
* Hold enter to search more occurrences scrolls up as needed ✅
* `showMarksOnScrollbar` still works ✅
This commit is contained in:
Leonard Hecker 2024-04-30 22:48:05 +02:00 committed by GitHub
parent 6bc7b9e68b
commit 32fbb16d43
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 86 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/buffer/out/search.cpp
View File

@ -8,29 +8,22 @@
using namespace Microsoft::Console::Types;
bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive, std::vector<til::point_span>* prevResults)
bool Search::IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive) const noexcept
{
return _renderData != &renderData ||
_needle != needle ||
_caseInsensitive != caseInsensitive ||
_lastMutationId != renderData.GetTextBuffer().GetLastMutationId();
}
bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive, bool reverse)
{
const auto& textBuffer = renderData.GetTextBuffer();
const auto lastMutationId = textBuffer.GetLastMutationId();
if (_renderData == &renderData &&
_needle == needle &&
_caseInsensitive == caseInsensitive &&
_lastMutationId == lastMutationId)
{
_step = reverse ? -1 : 1;
return false;
}
if (prevResults)
{
*prevResults = std::move(_results);
}
_renderData = &renderData;
_needle = needle;
_caseInsensitive = caseInsensitive;
_lastMutationId = lastMutationId;
_lastMutationId = textBuffer.GetLastMutationId();
_results = textBuffer.SearchText(needle, caseInsensitive);
_index = reverse ? gsl::narrow_cast<ptrdiff_t>(_results.size()) - 1 : 0;
@ -98,8 +91,9 @@ void Search::MovePastPoint(const til::point anchor) noexcept
_index = (index + count) % count;
}
void Search::FindNext() noexcept
void Search::FindNext(bool reverse) noexcept
{
_step = reverse ? -1 : 1;
if (const auto count{ gsl::narrow_cast<ptrdiff_t>(_results.size()) })
{
_index = (_index + _step + count) % count;
@ -141,6 +135,11 @@ const std::vector<til::point_span>& Search::Results() const noexcept
return _results;
}
std::vector<til::point_span>&& Search::ExtractResults() noexcept
{
return std::move(_results);
}
ptrdiff_t Search::CurrentMatch() const noexcept
{
return _index;

10
src/buffer/out/search.h
View File

@ -25,21 +25,19 @@ class Search final
public:
Search() = default;
bool ResetIfStale(Microsoft::Console::Render::IRenderData& renderData,
const std::wstring_view& needle,
bool reverse,
bool caseInsensitive,
std::vector<til::point_span>* prevResults = nullptr);
bool IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive) const noexcept;
bool Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool caseInsensitive, bool reverse);
void MoveToCurrentSelection();
void MoveToPoint(til::point anchor) noexcept;
void MovePastPoint(til::point anchor) noexcept;
void FindNext() noexcept;
void FindNext(bool reverse) noexcept;
const til::point_span* GetCurrent() const noexcept;
bool SelectCurrent() const;
const std::vector<til::point_span>& Results() const noexcept;
std::vector<til::point_span>&& ExtractResults() noexcept;
ptrdiff_t CurrentMatch() const noexcept;
private:

65
src/cascadia/TerminalControl/ControlCore.cpp
View File

@ -1654,30 +1654,41 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - text: the text to search
// - goForward: boolean that represents if the current search direction is forward
// - caseSensitive: boolean that represents if the current search is case sensitive
// - 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 reset)
SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool resetOnly)
{
const auto lock = _terminal->LockForWriting();
const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), text, !caseSensitive);
bool searchInvalidated = false;
std::vector<til::point_span> oldResults;
if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive, &oldResults))
if (searchInvalidated || !resetOnly)
{
searchInvalidated = true;
std::vector<til::point_span> oldResults;
_cachedSearchResultRows = {};
if (SnapSearchResultToSelection())
if (searchInvalidated)
{
_searcher.MoveToCurrentSelection();
SnapSearchResultToSelection(false);
oldResults = _searcher.ExtractResults();
_searcher.Reset(*_terminal.get(), text, !caseSensitive, !goForward);
if (SnapSearchResultToSelection())
{
_searcher.MoveToCurrentSelection();
SnapSearchResultToSelection(false);
}
_terminal->SetSearchHighlights(_searcher.Results());
}
else
{
_searcher.FindNext(!goForward);
}
_terminal->SetSearchHighlights(_searcher.Results());
}
else if (!reset)
{
_searcher.FindNext();
if (const auto idx = _searcher.CurrentMatch(); idx >= 0)
{
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));
}
_renderer->TriggerSearchHighlight(oldResults);
}
int32_t totalMatches = 0;
@ -1686,11 +1697,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
totalMatches = gsl::narrow<int32_t>(_searcher.Results().size());
currentMatch = gsl::narrow<int32_t>(idx);
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));
}
_renderer->TriggerSearchHighlight(oldResults);
return {
.TotalMatches = totalMatches,
.CurrentMatch = currentMatch,
@ -1698,27 +1706,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
};
}
Windows::Foundation::Collections::IVector<int32_t> ControlCore::SearchResultRows()
const std::vector<til::point_span>& ControlCore::SearchResultRows() const noexcept
{
if (!_cachedSearchResultRows)
{
auto results = std::vector<int32_t>();
auto lastRow = til::CoordTypeMin;
for (const auto& match : _searcher.Results())
{
const auto row{ match.start.y };
if (row != lastRow)
{
results.push_back(row);
lastRow = row;
}
}
_cachedSearchResultRows = winrt::single_threaded_vector<int32_t>(std::move(results));
}
return _cachedSearchResultRows;
return _searcher.Results();
}
void ControlCore::ClearSearch()
@ -1728,7 +1718,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_terminal->SetSearchHighlightFocused({});
_renderer->TriggerSearchHighlight(_searcher.Results());
_searcher = {};
_cachedSearchResultRows = {};
}
// Method Description:

5
src/cascadia/TerminalControl/ControlCore.h
View File

@ -220,12 +220,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void SetEndSelectionPoint(const til::point position);
SearchResults Search(const std::wstring_view& text, bool goForward, bool caseSensitive, bool reset);
const std::vector<til::point_span>& SearchResultRows() const noexcept;
void ClearSearch();
void SnapSearchResultToSelection(bool snap) noexcept;
bool SnapSearchResultToSelection() const noexcept;
Windows::Foundation::Collections::IVector<int32_t> SearchResultRows();
void LeftClickOnTerminal(const til::point terminalPosition,
const int numberOfClicks,
const bool altEnabled,
@ -353,8 +352,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
til::point _contextMenuBufferPosition{ 0, 0 };
Windows::Foundation::Collections::IVector<int32_t> _cachedSearchResultRows{ nullptr };
void _setupDispatcherAndCallbacks();
bool _setFontSizeUnderLock(float fontSize);

1
src/cascadia/TerminalControl/ControlCore.idl
View File

@ -136,7 +136,6 @@ namespace Microsoft.Terminal.Control
SearchResults Search(String text, Boolean goForward, Boolean caseSensitive, Boolean reset);
void ClearSearch();
IVector<Int32> SearchResultRows { get; };
Boolean SnapSearchResultToSelection;
Microsoft.Terminal.Core.Color ForegroundColor { get; };

16
src/cascadia/TerminalControl/TermControl.cpp
View File

@ -495,14 +495,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
if (_searchBox && _searchBox->Visibility() == Visibility::Visible)
{
if (const auto searchMatches = _core.SearchResultRows())
{
const til::color color{ _core.ForegroundColor() };
const auto rightAlignedOffset = (scrollBarWidthInPx - pipWidth) * sizeof(til::color);
const auto core = winrt::get_self<ControlCore>(_core);
const auto& searchMatches = core->SearchResultRows();
const auto color = core->ForegroundColor();
const auto rightAlignedOffset = (scrollBarWidthInPx - pipWidth) * sizeof(til::color);
til::CoordType lastRow = til::CoordTypeMin;
for (const auto row : searchMatches)
for (const auto& span : searchMatches)
{
if (lastRow != span.start.y)
{
const auto base = dataAt(row) + rightAlignedOffset;
lastRow = span.start.y;
const auto base = dataAt(lastRow) + rightAlignedOffset;
drawPip(base, color);
}
}

40
src/host/ut_host/SearchTests.cpp
View File

@ -57,7 +57,7 @@ class SearchTests
return true;
}
static void DoFoundChecks(Search& s, til::point coordStartExpected, til::CoordType lineDelta)
static void DoFoundChecks(Search& s, til::point coordStartExpected, til::CoordType lineDelta, bool reverse)
{
const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
@ -70,7 +70,7 @@ class SearchTests
coordStartExpected.y += lineDelta;
coordEndExpected.y += lineDelta;
s.FindNext();
s.FindNext(reverse);
VERIFY_IS_TRUE(s.SelectCurrent());
VERIFY_ARE_EQUAL(coordStartExpected, gci.renderData.GetSelectionAnchor());
@ -78,7 +78,7 @@ class SearchTests
coordStartExpected.y += lineDelta;
coordEndExpected.y += lineDelta;
s.FindNext();
s.FindNext(reverse);
VERIFY_IS_TRUE(s.SelectCurrent());
VERIFY_ARE_EQUAL(coordStartExpected, gci.renderData.GetSelectionAnchor());
@ -86,7 +86,7 @@ class SearchTests
coordStartExpected.y += lineDelta;
coordEndExpected.y += lineDelta;
s.FindNext();
s.FindNext(reverse);
VERIFY_IS_TRUE(s.SelectCurrent());
VERIFY_ARE_EQUAL(coordStartExpected, gci.renderData.GetSelectionAnchor());
@ -98,16 +98,16 @@ class SearchTests
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"AB", false, false);
DoFoundChecks(s, {}, 1);
s.Reset(gci.renderData, L"AB", false, false);
DoFoundChecks(s, {}, 1, false);
}
TEST_METHOD(ForwardCaseSensitiveJapanese)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"\x304b", false, false);
DoFoundChecks(s, { 2, 0 }, 1);
s.Reset(gci.renderData, L"\x304b", false, false);
DoFoundChecks(s, { 2, 0 }, 1, false);
}
TEST_METHOD(ForwardCaseInsensitive)
@ -115,47 +115,47 @@ class SearchTests
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"ab", false, true);
DoFoundChecks(s, {}, 1);
s.Reset(gci.renderData, L"ab", true, false);
DoFoundChecks(s, {}, 1, false);
}
TEST_METHOD(ForwardCaseInsensitiveJapanese)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"\x304b", false, true);
DoFoundChecks(s, { 2, 0 }, 1);
s.Reset(gci.renderData, L"\x304b", true, false);
DoFoundChecks(s, { 2, 0 }, 1, false);
}
TEST_METHOD(BackwardCaseSensitive)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"AB", true, false);
DoFoundChecks(s, { 0, 3 }, -1);
s.Reset(gci.renderData, L"AB", false, true);
DoFoundChecks(s, { 0, 3 }, -1, true);
}
TEST_METHOD(BackwardCaseSensitiveJapanese)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"\x304b", true, false);
DoFoundChecks(s, { 2, 3 }, -1);
s.Reset(gci.renderData, L"\x304b", false, true);
DoFoundChecks(s, { 2, 3 }, -1, true);
}
TEST_METHOD(BackwardCaseInsensitive)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"ab", true, true);
DoFoundChecks(s, { 0, 3 }, -1);
s.Reset(gci.renderData, L"ab", true, true);
DoFoundChecks(s, { 0, 3 }, -1, true);
}
TEST_METHOD(BackwardCaseInsensitiveJapanese)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
Search s;
s.ResetIfStale(gci.renderData, L"\x304b", true, true);
DoFoundChecks(s, { 2, 3 }, -1);
s.Reset(gci.renderData, L"\x304b", true, true);
DoFoundChecks(s, { 2, 3 }, -1, true);
}
};

5
src/interactivity/win32/find.cpp
View File

@ -52,13 +52,14 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l
LockConsole();
auto Unlock = wil::scope_exit([&] { UnlockConsole(); });
if (searcher.ResetIfStale(gci.renderData, lastFindString, reverse, caseInsensitive))
if (searcher.IsStale(gci.renderData, lastFindString, caseInsensitive))
{
searcher.Reset(gci.renderData, lastFindString, caseInsensitive, reverse);
searcher.MoveToCurrentSelection();
}
else
{
searcher.FindNext();
searcher.FindNext(reverse);
}
if (searcher.SelectCurrent())

5
src/types/UiaTextRangeBase.cpp
View File

@ -620,7 +620,10 @@ try
// -> We need to turn [_beg,_end) into (_beg,_end).
exclusiveBegin.x--;
_searcher.ResetIfStale(*_pData, queryText, searchBackward, ignoreCase);
if (_searcher.IsStale(*_pData, queryText, ignoreCase))
{
_searcher.Reset(*_pData, queryText, ignoreCase, searchBackward);
}
_searcher.MovePastPoint(searchBackward ? _end : exclusiveBegin);
til::point hitBeg{ til::CoordTypeMax, til::CoordTypeMax };