Skip to content

Commit 9b8c060

Browse files
lheckerDHowett
authored andcommitted
Stop scrolling on output when search is open (#17885)
* 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 d9131c6) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgS3elo PVTI_lADOAF3p4s4AmhmQzgTEciM Service-Version: 1.22
1 parent ea60f14 commit 9b8c060

File tree

9 files changed

+32
-48
lines changed

9 files changed

+32
-48
lines changed

src/buffer/out/search.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ bool Search::IsStale(const Microsoft::Console::Render::IRenderData& renderData,
1616
_lastMutationId != renderData.GetTextBuffer().GetLastMutationId();
1717
}
1818

19-
bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse)
19+
void Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse)
2020
{
2121
const auto& textBuffer = renderData.GetTextBuffer();
2222

@@ -30,15 +30,15 @@ bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const st
3030
_results = std::move(result).value_or(std::vector<til::point_span>{});
3131
_index = reverse ? gsl::narrow_cast<ptrdiff_t>(_results.size()) - 1 : 0;
3232
_step = reverse ? -1 : 1;
33-
return true;
34-
}
3533

36-
void Search::MoveToCurrentSelection()
37-
{
3834
if (_renderData->IsSelectionActive())
3935
{
4036
MoveToPoint(_renderData->GetTextBuffer().ScreenToBufferPosition(_renderData->GetSelectionAnchor()));
4137
}
38+
else if (const auto span = _renderData->GetSearchHighlightFocused())
39+
{
40+
MoveToPoint(_step > 0 ? span->start : span->end);
41+
}
4242
}
4343

4444
void Search::MoveToPoint(const til::point anchor) noexcept

src/buffer/out/search.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ class Search final
3636
Search() = default;
3737

3838
bool IsStale(const Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags) const noexcept;
39-
bool Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse);
39+
void Reset(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, SearchFlag flags, bool reverse);
4040

41-
void MoveToCurrentSelection();
4241
void MoveToPoint(til::point anchor) noexcept;
4342
void MovePastPoint(til::point anchor) noexcept;
4443
void FindNext(bool reverse) noexcept;

src/cascadia/TerminalControl/ControlCore.cpp

+16-29
Original file line numberDiff line numberDiff line change
@@ -1688,38 +1688,41 @@ namespace winrt::Microsoft::Terminal::Control::implementation
16881688
SearchResults ControlCore::Search(SearchRequest request)
16891689
{
16901690
const auto lock = _terminal->LockForWriting();
1691+
16911692
SearchFlag flags{};
16921693
WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !request.CaseSensitive);
16931694
WI_SetFlagIf(flags, SearchFlag::RegularExpression, request.RegularExpression);
16941695
const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), request.Text, flags);
16951696

1696-
if (searchInvalidated || !request.Reset)
1697+
if (searchInvalidated || !request.ResetOnly)
16971698
{
16981699
std::vector<til::point_span> oldResults;
1700+
til::point_span oldFocused;
1701+
1702+
if (const auto focused = _terminal->GetSearchHighlightFocused())
1703+
{
1704+
oldFocused = *focused;
1705+
}
16991706

17001707
if (searchInvalidated)
17011708
{
17021709
oldResults = _searcher.ExtractResults();
17031710
_searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward);
1704-
1705-
if (SnapSearchResultToSelection())
1706-
{
1707-
_searcher.MoveToCurrentSelection();
1708-
SnapSearchResultToSelection(false);
1709-
}
1710-
17111711
_terminal->SetSearchHighlights(_searcher.Results());
17121712
}
1713-
else
1713+
1714+
if (!request.ResetOnly)
17141715
{
17151716
_searcher.FindNext(!request.GoForward);
17161717
}
17171718

1718-
if (const auto idx = _searcher.CurrentMatch(); idx >= 0)
1719+
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(std::max<ptrdiff_t>(0, _searcher.CurrentMatch())));
1720+
_renderer->TriggerSearchHighlight(oldResults);
1721+
1722+
if (const auto focused = _terminal->GetSearchHighlightFocused(); focused && *focused != oldFocused)
17191723
{
1720-
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx), request.ScrollOffset);
1724+
_terminal->ScrollToSearchHighlight(request.ScrollOffset);
17211725
}
1722-
_renderer->TriggerSearchHighlight(oldResults);
17231726
}
17241727

17251728
int32_t totalMatches = 0;
@@ -1747,27 +1750,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
17471750
{
17481751
const auto lock = _terminal->LockForWriting();
17491752
_terminal->SetSearchHighlights({});
1750-
_terminal->SetSearchHighlightFocused({}, 0);
1753+
_terminal->SetSearchHighlightFocused(0);
17511754
_renderer->TriggerSearchHighlight(_searcher.Results());
17521755
_searcher = {};
17531756
}
17541757

1755-
// Method Description:
1756-
// - Tells ControlCore to snap the current search result index to currently
1757-
// selected text if the search was started using it.
1758-
void ControlCore::SnapSearchResultToSelection(bool shouldSnap) noexcept
1759-
{
1760-
_snapSearchResultToSelection = shouldSnap;
1761-
}
1762-
1763-
// Method Description:
1764-
// - Returns true, if we should snap the current search result index to
1765-
// the currently selected text after a new search is started, else false.
1766-
bool ControlCore::SnapSearchResultToSelection() const noexcept
1767-
{
1768-
return _snapSearchResultToSelection;
1769-
}
1770-
17711758
void ControlCore::Close()
17721759
{
17731760
if (!_IsClosing())

src/cascadia/TerminalControl/ControlCore.h

-2
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
228228
SearchResults Search(SearchRequest request);
229229
const std::vector<til::point_span>& SearchResultRows() const noexcept;
230230
void ClearSearch();
231-
void SnapSearchResultToSelection(bool snap) noexcept;
232-
bool SnapSearchResultToSelection() const noexcept;
233231

234232
void LeftClickOnTerminal(const til::point terminalPosition,
235233
const int numberOfClicks,

src/cascadia/TerminalControl/ControlCore.idl

+1-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ namespace Microsoft.Terminal.Control
5555
Boolean GoForward;
5656
Boolean CaseSensitive;
5757
Boolean RegularExpression;
58-
Boolean Reset;
58+
Boolean ResetOnly;
5959
Int32 ScrollOffset;
6060
};
6161

@@ -148,7 +148,6 @@ namespace Microsoft.Terminal.Control
148148

149149
SearchResults Search(SearchRequest request);
150150
void ClearSearch();
151-
Boolean SnapSearchResultToSelection;
152151

153152
Microsoft.Terminal.Core.Color ForegroundColor { get; };
154153
Microsoft.Terminal.Core.Color BackgroundColor { get; };

src/cascadia/TerminalControl/TermControl.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
575575
// but since code paths differ, extra work is required to ensure correctness.
576576
if (!_core.HasMultiLineSelection())
577577
{
578-
_core.SnapSearchResultToSelection(true);
579578
const auto selectedLine{ _core.SelectedText(true) };
580579
_searchBox->PopulateTextbox(selectedLine);
581580
}
@@ -3824,7 +3823,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
38243823
const auto goForward = _searchBox->GoForward();
38253824
const auto caseSensitive = _searchBox->CaseSensitive();
38263825
const auto regularExpression = _searchBox->RegularExpression();
3827-
const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() };
3826+
const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset };
38283827
_handleSearchResults(_core.Search(request));
38293828
}
38303829

src/cascadia/TerminalCore/Terminal.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -1254,15 +1254,17 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& highlight
12541254
// Method Description:
12551255
// - Stores the focused search highlighted region in the terminal
12561256
// - If the region isn't empty, it will be brought into view
1257-
void Terminal::SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset)
1257+
void Terminal::SetSearchHighlightFocused(const size_t focusedIdx) noexcept
12581258
{
12591259
_assertLocked();
12601260
_searchHighlightFocused = focusedIdx;
1261+
}
12611262

1262-
// bring the focused region into the view if the index is in valid range
1263-
if (focusedIdx < _searchHighlights.size())
1263+
void Terminal::ScrollToSearchHighlight(til::CoordType searchScrollOffset)
1264+
{
1265+
if (_searchHighlightFocused < _searchHighlights.size())
12641266
{
1265-
const auto focused = til::at(_searchHighlights, focusedIdx);
1267+
const auto focused = til::at(_searchHighlights, _searchHighlightFocused);
12661268
const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) };
12671269
const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) };
12681270
_ScrollToPoints(adjustedStart, adjustedEnd);

src/cascadia/TerminalCore/Terminal.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ class Microsoft::Terminal::Core::Terminal final :
233233
void SetSearchMissingCommandCallback(std::function<void(std::wstring_view, const til::CoordType)> pfn) noexcept;
234234
void SetClearQuickFixCallback(std::function<void()> pfn) noexcept;
235235
void SetSearchHighlights(const std::vector<til::point_span>& highlights) noexcept;
236-
void SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset);
236+
void SetSearchHighlightFocused(size_t focusedIdx) noexcept;
237+
void ScrollToSearchHighlight(til::CoordType searchScrollOffset);
237238

238239
void BlinkCursor() noexcept;
239240
void SetCursorOn(const bool isOn) noexcept;

src/interactivity/win32/find.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ INT_PTR CALLBACK FindDialogProc(HWND hWnd, UINT Message, WPARAM wParam, LPARAM l
5757
if (searcher.IsStale(gci.renderData, lastFindString, flags))
5858
{
5959
searcher.Reset(gci.renderData, lastFindString, flags, reverse);
60-
searcher.MoveToCurrentSelection();
6160
}
6261
else
6362
{

0 commit comments

Comments
 (0)