client: restore scrollback search (Ctrl+Shift+F)#72
Open
dolonet wants to merge 1 commit into
Open
Conversation
Reverts #48. That PR dropped the search addon believing browser-native Ctrl+F covers the use case — but browser find only searches the rendered DOM, and the xterm DOM renderer virtualizes rows: only the visible viewport is in the DOM, not scrollback. Browser find searches the current screen; the removed feature searched terminal history. No equivalent was left behind. Restores SearchAddon, the per-pane search bar, the toolbar search button, the Ctrl+Shift+F binding and the test mock — the exact inverse of #48. The restored toolbar button addresses the "undocumented hotkey" concern. The default highlightLimit (1000) is well inside the addon's freeze-free range; the upstream slowness (xtermjs/xterm.js#5176) only bites above ~5k highlights, which websh never configures.
Owner
Author
Self-reviewStraight revert of #48 — restored code is identical to what #48 removed; no hand-written code. Checked that it still integrates after the changes since the drop (mostly the vault work — #66/#67/#69/#70/#71). Verified:
Not verified — manual smoke needed before merge:
Pre-existing behaviour, restored verbatim (not a regression introduced here):
@gorevds — heads up, this reverts your #48. Rationale is in the description: browser Ctrl+F only sees the virtualized DOM viewport, not scrollback, so it isn't an equivalent for the removed scrollback search. Happy to discuss if you read it differently. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverts #48. That PR removed
@xterm/addon-searchscrollback search onthe premise that browser-native Cmd+F / Ctrl+F covers the use case.
It doesn't. Browser find searches the rendered DOM, and the xterm DOM
renderer virtualizes rows — only the visible viewport is in the DOM, not
scrollback. Browser find searches the current screen; the removed
feature searched terminal history. PR #48 kept server-side
tmux capture-paneexport as the "full history" path, but that's anon-interactive dump, not in-terminal search — the capability had no
equivalent left.
What's restored (exact inverse of #48)
SearchAddon+ per-pane instance + the@xterm/addon-search@0.15.0tag.search-barCSSclient: drop scrollback search (Ctrl+Shift+F) #48, since the binding is now discoverable in the UI
Ctrl+Shift+Fbinding + the delegated Enter/Escape handlerSearchAddonPerformance note
The slowness in xtermjs/xterm.js#5176 only appears once
highlightLimitis raised to ~5k+. websh uses the addon default (1000), well inside the
freeze-free range (~22 ms highlight-all, 60 fps scroll per published
benchmarks). No local patch or pinned fork needed.
Test plan
suite mocks
SearchAddon, so the real addon path needs a live check):python3 server.py→ localhost SSH, non-persistent pane, ~3000 lines ofscrollback. Ctrl+Shift+F opens the bar (positioned correctly), matches
highlight, Enter / Shift+Enter cycle next/prev through deep scrollback,
Esc closes. 0 console errors.