Skip to content

Commit 99df9e9

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_lADOAF3p4s4AmhmszgS3elk PVTI_lADOAF3p4s4AmhmszgTEciI Service-Version: 1.21
1 parent d1ac375 commit 99df9e9

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
@@ -1657,38 +1657,41 @@ namespace winrt::Microsoft::Terminal::Control::implementation
16571657
SearchResults ControlCore::Search(SearchRequest request)
16581658
{
16591659
const auto lock = _terminal->LockForWriting();
1660+
16601661
SearchFlag flags{};
16611662
WI_SetFlagIf(flags, SearchFlag::CaseInsensitive, !request.CaseSensitive);
16621663
WI_SetFlagIf(flags, SearchFlag::RegularExpression, request.RegularExpression);
16631664
const auto searchInvalidated = _searcher.IsStale(*_terminal.get(), request.Text, flags);
16641665

1665-
if (searchInvalidated || !request.Reset)
1666+
if (searchInvalidated || !request.ResetOnly)
16661667
{
16671668
std::vector<til::point_span> oldResults;
1669+
til::point_span oldFocused;
1670+
1671+
if (const auto focused = _terminal->GetSearchHighlightFocused())
1672+
{
1673+
oldFocused = *focused;
1674+
}
16681675

16691676
if (searchInvalidated)
16701677
{
16711678
oldResults = _searcher.ExtractResults();
16721679
_searcher.Reset(*_terminal.get(), request.Text, flags, !request.GoForward);
1673-
1674-
if (SnapSearchResultToSelection())
1675-
{
1676-
_searcher.MoveToCurrentSelection();
1677-
SnapSearchResultToSelection(false);
1678-
}
1679-
16801680
_terminal->SetSearchHighlights(_searcher.Results());
16811681
}
1682-
else
1682+
1683+
if (!request.ResetOnly)
16831684
{
16841685
_searcher.FindNext(!request.GoForward);
16851686
}
16861687

1687-
if (const auto idx = _searcher.CurrentMatch(); idx >= 0)
1688+
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(std::max<ptrdiff_t>(0, _searcher.CurrentMatch())));
1689+
_renderer->TriggerSearchHighlight(oldResults);
1690+
1691+
if (const auto focused = _terminal->GetSearchHighlightFocused(); focused && *focused != oldFocused)
16881692
{
1689-
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx), request.ScrollOffset);
1693+
_terminal->ScrollToSearchHighlight(request.ScrollOffset);
16901694
}
1691-
_renderer->TriggerSearchHighlight(oldResults);
16921695
}
16931696

16941697
int32_t totalMatches = 0;
@@ -1716,27 +1719,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
17161719
{
17171720
const auto lock = _terminal->LockForWriting();
17181721
_terminal->SetSearchHighlights({});
1719-
_terminal->SetSearchHighlightFocused({}, 0);
1722+
_terminal->SetSearchHighlightFocused(0);
17201723
_renderer->TriggerSearchHighlight(_searcher.Results());
17211724
_searcher = {};
17221725
}
17231726

1724-
// Method Description:
1725-
// - Tells ControlCore to snap the current search result index to currently
1726-
// selected text if the search was started using it.
1727-
void ControlCore::SnapSearchResultToSelection(bool shouldSnap) noexcept
1728-
{
1729-
_snapSearchResultToSelection = shouldSnap;
1730-
}
1731-
1732-
// Method Description:
1733-
// - Returns true, if we should snap the current search result index to
1734-
// the currently selected text after a new search is started, else false.
1735-
bool ControlCore::SnapSearchResultToSelection() const noexcept
1736-
{
1737-
return _snapSearchResultToSelection;
1738-
}
1739-
17401727
void ControlCore::Close()
17411728
{
17421729
if (!_IsClosing())

src/cascadia/TerminalControl/ControlCore.h

-2
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
222222
SearchResults Search(SearchRequest request);
223223
const std::vector<til::point_span>& SearchResultRows() const noexcept;
224224
void ClearSearch();
225-
void SnapSearchResultToSelection(bool snap) noexcept;
226-
bool SnapSearchResultToSelection() const noexcept;
227225

228226
void LeftClickOnTerminal(const til::point terminalPosition,
229227
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

@@ -147,7 +147,6 @@ namespace Microsoft.Terminal.Control
147147

148148
SearchResults Search(SearchRequest request);
149149
void ClearSearch();
150-
Boolean SnapSearchResultToSelection;
151150

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

src/cascadia/TerminalControl/TermControl.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
538538
// but since code paths differ, extra work is required to ensure correctness.
539539
if (!_core.HasMultiLineSelection())
540540
{
541-
_core.SnapSearchResultToSelection(true);
542541
const auto selectedLine{ _core.SelectedText(true) };
543542
_searchBox->PopulateTextbox(selectedLine);
544543
}
@@ -3756,7 +3755,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
37563755
const auto goForward = _searchBox->GoForward();
37573756
const auto caseSensitive = _searchBox->CaseSensitive();
37583757
const auto regularExpression = _searchBox->RegularExpression();
3759-
const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _calculateSearchScrollOffset() };
3758+
const auto request = SearchRequest{ text, goForward, caseSensitive, regularExpression, true, _searchScrollOffset };
37603759
_handleSearchResults(_core.Search(request));
37613760
}
37623761

src/cascadia/TerminalCore/Terminal.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -1238,15 +1238,17 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& highlight
12381238
// Method Description:
12391239
// - Stores the focused search highlighted region in the terminal
12401240
// - If the region isn't empty, it will be brought into view
1241-
void Terminal::SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset)
1241+
void Terminal::SetSearchHighlightFocused(const size_t focusedIdx) noexcept
12421242
{
12431243
_assertLocked();
12441244
_searchHighlightFocused = focusedIdx;
1245+
}
12451246

1246-
// bring the focused region into the view if the index is in valid range
1247-
if (focusedIdx < _searchHighlights.size())
1247+
void Terminal::ScrollToSearchHighlight(til::CoordType searchScrollOffset)
1248+
{
1249+
if (_searchHighlightFocused < _searchHighlights.size())
12481250
{
1249-
const auto focused = til::at(_searchHighlights, focusedIdx);
1251+
const auto focused = til::at(_searchHighlights, _searchHighlightFocused);
12501252
const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) };
12511253
const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) };
12521254
_ScrollToPoints(adjustedStart, adjustedEnd);

src/cascadia/TerminalCore/Terminal.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@ class Microsoft::Terminal::Core::Terminal final :
231231
void SetPlayMidiNoteCallback(std::function<void(const int, const int, const std::chrono::microseconds)> pfn) noexcept;
232232
void CompletionsChangedCallback(std::function<void(std::wstring_view, unsigned int)> pfn) noexcept;
233233
void SetSearchHighlights(const std::vector<til::point_span>& highlights) noexcept;
234-
void SetSearchHighlightFocused(const size_t focusedIdx, til::CoordType searchScrollOffset);
234+
void SetSearchHighlightFocused(size_t focusedIdx) noexcept;
235+
void ScrollToSearchHighlight(til::CoordType searchScrollOffset);
235236

236237
void BlinkCursor() noexcept;
237238
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)