fix: implement CBT/CHT and 1-indexed CPR (fixes #58, #94)#223
Open
falki3nec wants to merge 5 commits into
Open
fix: implement CBT/CHT and 1-indexed CPR (fixes #58, #94)#223falki3nec wants to merge 5 commits into
falki3nec wants to merge 5 commits into
Conversation
added 5 commits
April 13, 2026 21:22
EscapeEmitter.cursorPosition() was emitting raw 0-indexed buffer coordinates as the CPR (Cursor Position Report) reply, but VT100 requires 1-indexed values (CSI row ; col R). Every DSR (CSI 6n) query thus returned a position off by one in both axes. ncurses applications like nano and vim that sync their internal cursor model from CPR ended up editing at the wrong visible position. The bug is invisible inside a tmux session because tmux answers CPR from its own state and never forwards the query to the underlying emulator — which is why the bug only manifests on raw SSH. Adds regression tests under test/src/regression/nano_vim_cursor_test.dart covering CPR at home, CPR after CUP, and CPR round-trip fidelity, plus sanity checks for the other scroll-region / save-restore / autowrap hypotheses that proved to be implemented correctly. Fixes TerminalStudio#58, TerminalStudio#94
CSI Pn Z (Cursor Backward Tab) and CSI Pn I (Cursor Horizontal Tab) were missing from the CSI handler table. The parser dropped them into unknownCSI and the cursor stayed put. nano uses CBT aggressively. When cursor is past a tab stop and nano wants to re-render a syntax-highlighted region starting at an earlier tab stop, it emits CSI Z to get there in one hop instead of a BS run. With the emulator ignoring the sequence, nano's internal cursor model diverges from the buffer's by whatever distance the skipped CBT would have covered. Subsequent character writes then land to the right of where nano expects and overwrite unrelated cells — the user-visible symptom is long syntax-highlighted lines morphing into garbled text during plain arrow-key navigation. Bug is invisible inside tmux (tmux handles CBT in its own emulator layer and forwards nothing to xterm.dart) and with TERM=vt100 (nano's vt100 terminfo path avoids CBT). Also implements CHT for symmetry. Depends on the preceding CPR off-by-one fix; both combined restore correct nano/vim behaviour in raw SSH. Fixes TerminalStudio#58, TerminalStudio#94
RenderTerminal._onTerminalChange was calling markNeedsLayout on every write from the Terminal, which triggered a full Flutter layout pass (walking the entire RenderObject tree, re-measuring every child). For pure content writes the widget geometry is unchanged — only the scrollback buffer grew. Layout was the hot path in TUI-heavy scenarios (Claude Code spinners + counters, vim/nano cursor movement) and made xterm.dart render 5–10x slower than xterm.js on the web, because web's DOM/canvas renderer doesn't re-layout on every write. Update scroll bounds (applyContentDimensions + stick-to-bottom correction) directly and trigger only a repaint. Geometry changes (widget resize, font change, viewport resize) still go through the normal performLayout path via markNeedsLayout calls elsewhere. Measured against a Claude Code session with ~98 small output chunks (spinner animation + counter), this removes ~98 layout passes per burst, which was the dominant cost on desktop. Fixes desktop terminal feeling sluggish during TUI output.
eraseLineToCursor called eraseRange(0, _cursorX) but eraseRange end is exclusive, so column == _cursorX was never cleared. Per VT100 / xterm docs, "Erase in Line from start to cursor" must include the cell at the cursor position. Symptom: Claude Code's "Do you want to…" confirmation menu on Android and desktop Flutter apps rendered garbled, with residual characters bleeding through each re-drawn line. Web (xterm.js) worked correctly. Claude Code's menu layout relies on repeated `CSI 1 K` + `CUF` + text patterns to blit one row at a time — each frame issues ~36 CSI 1 K calls, and each one left exactly one residual cell of stale text. Regression tests added in test/src/regression/csi_1k_erase_test.dart cover mid-line, first-column, last-column erase, and confirm that CSI 0 K semantics are unchanged by the fix.
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
Two independent fixes restoring correct behaviour of ncurses applications (nano, vim, htop) over raw SSH without tmux.
1. Implement
CSI Pn Z(CBT) andCSI Pn I(CHT)Both sequences were absent from
_csiHandlers; the parser dropped them intounknownCSIand the cursor stayed in place. nano uses CBT aggressively while redrawing syntax-highlighted portions of the current line — when ignored, nano's internal cursor model diverges from the buffer by the skipped tab-stop distance, and subsequent writes land on the wrong cells. Visible symptom: long syntax-highlighted lines morph into garbled text during plain arrow-key navigation.Invisible inside tmux (tmux handles CBT in its own emulator layer and forwards nothing to us) and with
TERM=vt100(nano's vt100 terminfo path avoids CBT).2. Off-by-one in CPR reply
EscapeEmitter.cursorPosition()emitted raw 0-indexed buffer coordinates, but VT100 CPR (CSI row;col R) is 1-indexed. EveryCSI 6nquery returned a position off by one in both axes, throwing off ncurses apps that sync from CPR.Root cause analysis
Diagnosed by capturing raw stdout bytes during a reproducible nano session and replaying them through a headless
Terminalin a regression harness, narrowing the first divergence to aCSI Zthat leftcursorXunchanged.Tests
test/src/regression/nano_vim_cursor_test.dart— 12 hypothesis-driven tests covering scroll region, save/restore, alt-screen, autowrap, CUP, CPRtest/src/regression/replay_test.dart— replays a captured debug log viaREPLAY_LOGenv varflutter testpasses except for 2 pre-existingtextScalergolden-image failures unrelated to these changes.Fixes
Happy to split into two PRs if preferred, or adjust style.