fix(vt): handle SCO restore cursor (CSI u) - #921
Open
PeterSR wants to merge 1 commit into
Open
Conversation
vt registers a CSI 's' handler that saves the cursor for SCO console mode (ansi.SCOSC) when DECSLRM is not active, but there was no matching CSI 'u' handler for the SCO restore (ansi.SCORC). The sequence parsed fine and simply matched no handler, so ESC[u was silently dropped: the cursor never moved back and subsequent writes landed at the wrong column instead of overwriting. Register a CSI 'u' handler that calls scr.RestoreCursor(), mirroring the existing DECSC/DECRC pair (ESC 7 / ESC 8) already wired up in the same file. This does not collide with the kitty keyboard protocol sequences on the same final byte (CSI > u, CSI = ... u, CSI < u): those carry a prefix byte, which is packed into the command int separately from a bare final byte, so they resolve to distinct handler keys. Adds a table-driven test asserting that SCOSC/SCORC repositions the cursor, next to an equivalent DECSC/DECRC case for symmetry.
PeterSR
added a commit
to PeterSR/pupptyeer
that referenced
this pull request
Jul 22, 2026
…-close race (#5) Two independent daemon-side fixes in pkg/ptysession, no wire-protocol change. SCO restore-cursor (fixes #4): the vt emulator handles ESC[s (SCOSC) but has no ESC[u (SCORC) handler, so restore was silently dropped and later writes landed at the wrong column. Rewrite ESC[u into ESC8 (DECRC) on the way into the emulator only; ring and OnOutput keep the child's bytes verbatim. Upstream fix filed at charmbracelet/x#921; scorc.go carries a TODO to delete once it lands. Data race: Emulator.Close writes an unsynchronised closed bool that Emulator.Read checks while drainTerm is parked in it. closeTerm closes the reply pipe directly via InputPipe instead, so the flag is never written. Go tests now run under -race in CI and make test.
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.
What's broken
vtimplements the SCO save-cursor sequence (CSI s,ansi.SCOSC) but not thematching restore (
CSI u,ansi.SCORC). Invt/handlers.go,registerDefaultCsiHandlersregisters a handler for
's'that either sets DECSLRM margins (whenansi.ModeLeftRightMarginis set) or callse.scr.SaveCursor()for the SCO case.There is no corresponding
e.RegisterCsiHandler('u', ...)anywhere in the package, soCSI uis parsed correctly by the sequence parser but matches no handler and issilently dropped.
The visible effect: after
ESC[s, a followingESC[udoes not move the cursor. Thecursor stays wherever it happened to end up, so the next write lands at the wrong
column and gets appended instead of overwriting the saved position.
Minimal repro
Write the following to the emulator and inspect row 0:
ESC[2JESC[1;1HESC[sAAAAESC[uBBExpected row 0:
"BBAA"(steps 5-6 return to the saved position and overwrite).Actual row 0 (before this patch):
"AAAABB"(step 5 is a no-op, step 6 appends afterthe existing text instead of overwriting it).
Every other repositioning mechanism already works
Only the SCO pair is affected. Every other way of getting the cursor back to the start
of the line correctly produces
"BBAA":\r"BBAA"(correct)ESC[1G"BBAA"(correct)ESC[4D"BBAA"(correct)ESC[1;1H"BBAA"(correct)ESC7/ESC8"BBAA"(correct)ESC[s/ESC[u"AAAABB"(bug: restore is dropped)xterm and vte both implement
CSI uas SCO restore-cursor; this emulator should match.The fix
Register a
CSI uhandler that callse.scr.RestoreCursor(), mirroring the existingDECRC handler (
ESC 8) already registered in the same file:Placed immediately after the
's'handler registration inregisterDefaultCsiHandlers, with a comment mirroring the neighbouring// Save Current Cursor Position [ansi.SCOSC]style.ansi.SCORCalready exists as anamed constant (
ansi/cursor.go), same asansi.SCOSC.Added a table-driven case to
vt/emulator_test.goasserting SCOSC/SCORC repositionsthe cursor, next to an equivalent DECSC/DECRC (
ESC7/ESC8) case so the symmetrybetween the two save/restore pairs is explicit.
Kitty keyboard protocol is unaffected
The kitty keyboard protocol also ends sequences in
u(CSI > flags u,CSI = flags ; mode u,CSI < number u), which raised the question of whether a plain'u'handler could swallow them. It cannot: those sequences carry a prefix byte(
>,=,<), andansi.Command(prefix, inter, final)packs the prefix into higherbits of the command int, separate from the final byte.
handleCsidispatches on thatfull packed int (
h.csiHandlers[int(cmd)]), so a handler registered for bare'u'(prefix
0) occupies a different map key than one registered foransi.Command('>', 0, 'u'),ansi.Command('=', 0, 'u'), oransi.Command('<', 0, 'u').The new handler only ever fires for unprefixed
CSI u, i.e. SCORC.Testing
gofmt -l vt go vet ./vt/... go test ./vt/...All clean. Confirmed the new SCOSC/SCORC test fails without the handler (and that the
DECSC/DECRC case passes either way), then passes with the one-hunk fix applied.
Where this turned up
Found downstream in pupptyeer, which renders PTY output with this emulator: PeterSR/pupptyeer#5 currently carries a byte-rewrite workaround (
ESC[u->ESC8on the way into the emulator) that this patch makes unnecessary.