feat: add preserveScrollOnWrite option to lock viewport on new output#5
Merged
Merged
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
When the user has scrolled into the scrollback and new output arrives, the legacy xterm.js-style behaviour auto-scrolls back to the bottom (losing the user's reading position). Modern terminals (kitty, alacritty) instead lock the viewport on the same content so the user keeps reading where they were. This commit ports the upstream fix as a new opt-in option: - Add `preserveScrollOnWrite?: boolean` to `ITerminalOptions` (default: `false`, preserves the current xterm.js-compat behaviour). - When `true`, save `viewportY` and `getScrollbackLength()` before the WASM write, compute the scrollback delta after, and shift `viewportY` by that delta — clamped to the new scrollback length in case old lines were dropped by the scrollback limit. Re-fires `scrollEmitter` and briefly shows the scrollbar when the viewport actually shifts. - Add two regression tests covering both behaviours. This is an adaptation of upstream PR coder#150 (which made the new behaviour unconditional). Resolves coder#127 (request to make auto-scroll configurable). Co-authored-by: Sauyon Lee <git@sjle.co> Inspired-by: coder#150
diegosouzapw
force-pushed
the
feat/port-pr-150-preserve-scroll-opt
branch
from
ce1e9e9 to
6a6824a
Compare
|
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
1 participant
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
Adapts upstream PR #150 into an opt-in option:
preserveScrollOnWrite?: booleanonITerminalOptions(defaultfalse).true, the viewport stays locked on its current scrollback content as new output arrives (kitty/alacritty-style). Viewport shifts by the scrollback delta and clamps to the new scrollback length.Difference from upstream PR
Upstream made the new behaviour unconditional — a breaking change for any embedder relying on xterm.js auto-scroll semantics. This port gates it behind an opt-in flag so downstream consumers can adopt at their own pace.
Attribution
Thanks to @sauyon for the original implementation.
Test plan
bun run fmt && bun run lint && bun run typecheckbun test— 333 tests pass (2 new regression tests for both option values), 0 failbun run build:libbun run build:wasmnot run locally (Zig not installed); no WASM path touchedRisk
Low — additive, opt-in. Default
falsepreserves current behaviour. Two regression tests cover both legacy auto-scroll AND the new locked-viewport path.