Skip to content

Commit

Permalink
Stop scrolling on output when search is open (microsoft#17885)
Browse files Browse the repository at this point in the history
* 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 microsoft#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. ✅
  • Loading branch information
lhecker authored Sep 24, 2024
1 parent 0ce654e commit d9131c6
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 48 deletions.
10 changes: 5 additions & 5 deletions src/buffer/out/search.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -30,15 +30,15 @@ bool Search::Reset(Microsoft::Console::Render::IRenderData& renderData, const st
_results = std::move(result).value_or(std::vector<til::point_span>{});
_index = reverse ? gsl::narrow_cast<ptrdiff_t>(_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
Expand Down
3 changes: 1 addition & 2 deletions src/buffer/out/search.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
45 changes: 16 additions & 29 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1697,38 +1697,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<til::point_span> 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<size_t>(std::max<ptrdiff_t>(0, _searcher.CurrentMatch())));
_renderer->TriggerSearchHighlight(oldResults);

if (const auto focused = _terminal->GetSearchHighlightFocused(); focused && *focused != oldFocused)
{
_terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx), request.ScrollOffset);
_terminal->ScrollToSearchHighlight(request.ScrollOffset);
}
_renderer->TriggerSearchHighlight(oldResults);
}

int32_t totalMatches = 0;
Expand Down Expand Up @@ -1756,27 +1759,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())
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
SearchResults Search(SearchRequest request);
const std::vector<til::point_span>& SearchResultRows() const noexcept;
void ClearSearch();
void SnapSearchResultToSelection(bool snap) noexcept;
bool SnapSearchResultToSelection() const noexcept;

void LeftClickOnTerminal(const til::point terminalPosition,
const int numberOfClicks,
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ namespace Microsoft.Terminal.Control
Boolean GoForward;
Boolean CaseSensitive;
Boolean RegularExpression;
Boolean Reset;
Boolean ResetOnly;
Int32 ScrollOffset;
};

Expand Down Expand Up @@ -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; };
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,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);
}
Expand Down Expand Up @@ -3861,7 +3860,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));
}

Expand Down
10 changes: 6 additions & 4 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1262,15 +1262,17 @@ void Terminal::SetSearchHighlights(const std::vector<til::point_span>& 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);
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ class Microsoft::Terminal::Core::Terminal final :
void SetClearQuickFixCallback(std::function<void()> pfn) noexcept;
void SetWindowSizeChangedCallback(std::function<void(int32_t, int32_t)> pfn) noexcept;
void SetSearchHighlights(const std::vector<til::point_span>& 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;
Expand Down
1 change: 0 additions & 1 deletion src/interactivity/win32/find.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit d9131c6

Please sign in to comment.