VTTEST alignment by tomlm · Pull Request #141 · tomlm/XTerm.NET

tomlm · 2026-08-31T22:07:25Z

Everything from the vttest sweep in one place: the harness that found the problems, the tests that pin them, and the fixes that are settled.

Fixes #124
Fixes #125
Fixes #126
Fixes #129
Fixes #130
Fixes #131
Fixes #132
Fixes #143
Fixes #144
Fixes #136

Fixed

issue
#125	DECRQM silent for unrecognised modes	now `CSI ? 2 ; 0 $ y` — vttest reads `unknown` where it read `failed`
#126	pixel-geometry reports need a host handler	now `CSI 4 ; 0 ; 0 t`, matching the position report next door
#129	erased lines stayed double-width/height	full non-selective erases reset the attribute — the "stuck in double-size mode" report
#131	G2 and G3 unreachable	SS2, SS3, LS2, LS3 dispatched; single shifts expire by being consumed
#132	national replacement sets advertised but missing	nine sets, gated on DECNRCM, designations remembered by ID
#143	DECNCSM missing, so a column change always cleared	mode 95 suppresses the erase
#138	DECSCPP ignored	implemented; the DECSLPP half of that issue is withdrawn, see below
#144	CHT cancelled a pending wrap where HT preserved it	one shared tab motion
#124	DECREQTPARM unimplemented	answers with xterm's values; vttest marks both arguments `-- OK`
#130	S7C1T/S8C1T ignored	replies convert at one exit point; see the limit below

Three of these touched decisions someone had made on purpose, and all three are called out rather than buried:

DECRQM stays silent for unrecognized modes instead of reporting Ps=0 #125 changes an existing test. RequestModeTests pinned the silence, reasoning that reporting "reset" for an untracked mode would be a guess. That reasoning survives — Ps=0 is not that guess, it is "I do not recognise this mode", and DECRQSS in this same emulator already answers its unsupported requests that way. A missing parameter still says nothing, because mode 0 is not a mode.
National replacement character sets are advertised in the DA but not implemented #132 keeps designations by ID, not just the table they resolved to. A national set means one thing with DECNRCM set and ASCII without it, so a program that designates French and then enables NRC expects French — and never designates again.
S7C1T/S8C1T are ignored, so replies never use 8-bit C1 controls #130 converts at RaiseDataReceived, not at the ~55 places that build a reply. Only the leading introducer and a trailing ST are rewritten: a payload can contain anything, and rewriting every ESC pair would corrupt one that merely looked like an introducer. RIS puts the form back to 7-bit.

Two corrections to my own reports

DECSLPP was never broken (half of #138, withdrawn on the issue). CSI Ps t with Ps ≥ 24 has always resized. I had quoted vttest's Screen should be filled (24 of 25 rows) as evidence, but that is vttest's own view of a size it samples once per test and never re-measures — it reads identically whether the resize worked or not. Two mistakes of mine fed it: the harness dumped a hardcoded 24 rows, hiding every row a resize added, and I had not checked that a pty resize reaches a program inside WSL before blaming the emulator. It does. Both fixed here.

#130 is only half fixable from this side. vttest still marks its 8-bit case failed, reading <194> <155> — U+009B encoded as UTF-8 by the harness before it reaches the pty. The emulator's reply carries the C1 character; whether it hits the wire as a raw 0x9B is the host's encoding decision, and a raw 0x9B is not valid UTF-8, which is why xterm ties 8-bit controls to non-UTF-8 operation.

Tests

VtTestConformanceTests — one test per finding, skipped against its issue while open and unskipped as each landed. Unskipped, every one fails on the defect itself. Each ships with the neighbour that already works (DECSWL still resets a line, G0 still reaches the graphics set, the UK set still remaps), because a skipped test alone cannot tell "missing" from "misrouted". One skip remains: DA3 (#123).

VtTestBehaviourTests — guards for what vttest exercises and this emulator gets right, none of which a screen dump can see: BCE fill colours, SGR 22 clearing bold and dim, DECSCA protection against DECSEL with ECH ignoring it, margins clipping and wrapping, reverse wraparound on a genuinely wrapped line, IRM, tab stops, DECRQCRA's negated sum, the three page-size controls, and tabbing off the line the same way however it is spelled.

Three of those started as false alarms — margins made a dump look like DCH ate text outside them; reverse wraparound looked missing because the setup never wrapped a line. They are written down so the next reader does not re-derive the same wrong conclusion.

All string assertions are ordinal: culture-sensitive comparison treats C1 controls as ignorable, and one test passed against a 7-bit reply while asserting an 8-bit prefix.

Tooling

tools/vttest/ — VtDrive spawns vttest through a pty into a headless Terminal; vtsweep.py runs the same keys into tmux and diffs by screen content. Two wirings are load-bearing and were mistakes first: DataReceived back to the pty, or every report test times out; and Resized to connection.Resize, or DECCOLM resizes the grid while vttest still believes it has 80 columns.

The README says plainly what tmux cannot referee — no VT52, DECCOLM ignored, untranslated characters in capture-pane, no DECRQM or DECRQCRA, no attributes at all. It nominates; vttest's own verdict text judges. Several differences it found were tmux's bugs, not ours.

Still open, deliberately

#123 DA3 — the code argues silence is right for a terminal with no unit ID, and that is a decision to revisit on purpose. #128 DECRQCRA (needs the xterm blank-cell question answered), #139 three VT320 state reports, #140 the status line, #142 DECCARA/DECRARA, #145 OSC 50.

Verification

2140 passed, 1 skipped, 0 failed. Every fix re-checked against real vttest through the harness, not only in unit tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

Four unanswered queries found by running vttest 2.7 through a pty against this emulator, restated as sequences and expected replies. Each is skipped against its issue and flips to passing when the issue is fixed; unskipped, all four fail with an empty reply collection, which is the defect exactly. DA3 #123 no entry in the CSI identifier table DECREQTPARM #124 no handler for the final character 'x' DECRQM #125 bare return for a mode not in the table CSI 14 t #126 replies only when a host handler sets Handled Only cases vttest judges ITSELF are ported. It prints the reply it got and says what it expected, so the oracle travels with the case. Its visual tests carry no such thing -- the expectation is a sentence describing the screen to a human, and porting one means writing the expected buffer yourself, which asserts today's behaviour while looking like a conformance test. Three passing tests come with them. Two are the neighbours the fixes should match -- CSI 18 t and a DECRQM mode the table carries both answer today, and they are what the missing replies are missing against. The third is DECCOLM clearing, which passes and nearly did not get written: driving vttest without propagating the emulator's resize to the pty leaves the application drawing at 80 columns into a 132-column grid, which reads as a clearing bug and is not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

Four more skipped tests, one per open issue: ED leaving line attributes set (#129), S8C1T not changing the reply prefix (#130), a locking shift never reaching G2 (#131), and a national replacement set that does not remap (#132). Unskipped, all eight fail on the defect itself. Each ships with the neighbour that already works, because a skipped test alone cannot show whether the mechanism is missing or merely misrouted: DECSWL still resets a line, G0 still reaches the graphics set, and the UK set still remaps its one position. VtTestBehaviourTests is the other half -- behaviour vttest exercises that this emulator gets right, none of which a screen dump can see: BCE fill colours, SGR 22 clearing bold and dim together, DECSCA protection against DECSEL with ECH ignoring it, left/right margins clipping and wrapping, reverse wraparound on a line that actually wrapped, IRM, tab stops, and DECRQCRA's negated sum. Three of those were false alarms first. Margins made a screen dump look like DCH had eaten text outside them; reverse wraparound looked missing because the setup never wrapped a line. They are written down so the next reader does not re-derive the same wrong conclusion. The string assertions are ORDINAL. Culture-sensitive comparison treats C1 controls as ignorable, so the S8C1T test passed against a 7-bit reply -- asserting a prefix of ESC or 0x9B linguistically asserts nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

Three of the unanswered-query findings from the vttest sweep. DECRQM (#125) answers Ps=0 -- "mode not recognised" -- where it used to return without replying. vttest showed the cost as "failed" against modes 2 and 10-13: no report arrives and a client waiting for one waits forever. It now reads "unknown" there. RequestModeTests asserted that silence, and its reasoning is not contradicted: reporting "reset" for an untracked mode WOULD be a guess. Ps=0 is not that guess, it is the defined value for "I do not recognise this mode", and it is what DECRQSS in this same emulator already answers for settings it does not support. The test now pins that instead. A missing parameter still says nothing. Mode 0 is not a mode, and the distinction is worth keeping: an unrecognised mode was asked about, a missing one was not. CSI 14/15/16 t (#126) answer with zeroes when no host handler does. An emulator has no pixels of its own, but silence is not an answer and enabling the report is a statement that the question will be answered -- the position report next door already worked this way. Erasing a line in full puts it back to single width (#129). The attribute says how a line is DRAWN, and an erased line has nothing left to draw at double size, so carrying it forward only doubles what is written next. Reported from the Avalonia terminal as vttest leaving the terminal stuck in double-size mode; the main menu now comes back clean. Full and non-selective only: a partial erase leaves text that is still meant to be double, and a selective erase exists to leave protected text standing -- resizing the line under it would undo the one thing it was asked to preserve. DA3 (#123) is deliberately NOT included. The code says silence is the right outcome for a terminal with no unit ID to report, and that is a decision to revisit on purpose rather than in passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

Two halves of the same gap: a character set the emulator could be told about and then had no way to use. SS2, SS3, LS2 and LS3 (#131). G2 and G3 could be designated and never invoked, so whatever a program put in them printed as ASCII. SO and SI were already the same operation for G1 and G0; the locking shifts join them. The single shifts are held PENDING rather than swapped in, so they expire by being consumed instead of by something remembering to put the old set back, and a shift with no character after it simply never fires. The run-printing fast path is skipped while one is pending, since it prints without translating. The national replacement sets (#132). Nine of them -- French, German, Swedish, Italian, Spanish, Norwegian/Danish, Dutch, Swiss, Finnish -- each remapping a handful of ASCII positions, gated on DECNRCM as the standard requires. Designations are now remembered BY ID, not only as the table they resolved to. A national set means one thing with DECNRCM set and ASCII without it, so a program that designates French and then enables NRC mode expects French -- and it never designates again. Changing the mode re-resolves what is already designated. The DA has advertised feature 9 all along, which is what made this worth implementing rather than removing from the DA: the sets were promised. Verified: G0, G1, LS2 and LS3 all reach the graphics set; SS2 and SS3 shift exactly one character; French reads #@[ with NRC off and the pound, a-grave and degree with it on, in either order of designation and mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

The infrastructure the last week of conformance findings came from, rather than leaving it in a scratch directory where the next question about vttest starts by rebuilding it. VtDrive spawns vttest through a pty into a headless Terminal and dumps the screen after each step. Two wirings in it are load-bearing and were both learned the hard way: DataReceived goes back to the pty, or every report test times out against a terminal with no voice; and Resized calls connection.Resize, or DECCOLM resizes the grid while vttest still believes it has 80 columns, and every screen after the 132-column tests looks like a wrapping bug that is not there. vtsweep.py runs the same key script into tmux and diffs the screens, matching them by CONTENT rather than step number -- the two take different numbers of screens through a menu, so counting desynchronises everything after the first divergence. The README says plainly what tmux cannot referee: no VT52, DECCOLM ignored, untranslated characters in capture-pane, no DECRQM or DECRQCRA, and no visibility of attributes at all. Several differences this tool found were tmux's bugs rather than ours, so it nominates and vttest's own verdict text judges. References the package for Porta.Pty rather than a sibling checkout, so it builds from a clean clone, and is IsPackable=false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

github-actions · 2026-08-31T22:12:14Z

Perf comparison — this change, against its base

3 run(s) of each side, alternating on one machine. Allocation is a count and is gated exactly. Time is a measurement, so its gate is derived from the spread this job just observed in itself rather than fixed in advance.

corpus	bytes/char	gen0/Mchar	ns/char	Δ time	noise	gate
scroll-ascii	0.00 → 0.00	0.00 → 0.00	3.18 → 3.17	-0.3%	±4%	12%
sgr-churn	0.00 → 0.00	0.00 → 0.00	7.47 → 7.26	-2.9%	±2%	7%
truecolor	0.00 → 0.00	0.00 → 0.00	6.97 → 6.95	-0.2%	±2%	7%
alt-redraw	0.00 → 0.00	0.00 → 0.00	9.54 → 9.58	+0.4%	±1%	4%
unicode	7.66 → 7.66	0.09 → 0.09	24.80 → 24.85	+0.2%	±4%	12%
flood	0.00 → 0.00	0.00 → 0.00	98.70 → 98.92	+0.2%	±1%	4%

Each corpus is gated at max(4%, 3 × its own noise). A wide noise column means this runner was busy and the timing half of the table should be read as advisory; the allocation half is exact either way.

assemblies measured

base: XTerm.NET 2.0.0.0 mvid:08abef64-5db5-4cbe-99ab-424ba9832e2d
head: XTerm.NET 2.0.0.0 mvid:110806fe-930f-446e-92d3-6fbe60dab697

Perf comparison — cumulative, everything since 2.0.0-rc002

3 run(s) of each side, alternating on one machine. Allocation is a count and is gated exactly. Time is a measurement, so its gate is derived from the spread this job just observed in itself rather than fixed in advance.

corpus	bytes/char	gen0/Mchar	ns/char	Δ time	noise	gate
scroll-ascii	0.00 → 0.00	0.00 → 0.00	3.23 → 3.17	-2.0%	±3%	9%
sgr-churn	0.00 → 0.00	0.00 → 0.00	7.27 → 7.26	-0.1%	±3%	10%
truecolor	0.00 → 0.00	0.00 → 0.00	7.02 → 6.95	-1.0%	±9%	27%
alt-redraw	0.00 → 0.00	0.00 → 0.00	9.14 → 9.58	+4.7%	±1%	5%
unicode	7.66 → 7.66	0.09 → 0.09	23.59 → 24.85	+5.3% 👀	±4%	12%
flood	0.00 → 0.00	0.00 → 0.00	98.16 → 98.92	+0.8%	±1%	5%

Each corpus is gated at max(5%, 3 × its own noise). A wide noise column means this runner was busy and the timing half of the table should be read as advisory; the allocation half is exact either way.

assemblies measured

base: XTerm.NET 2.0.0.0 mvid:1b573f7c-2562-4a81-9dc9-0cdd9bd9fb1d
head: XTerm.NET 2.0.0.0 mvid:110806fe-930f-446e-92d3-6fbe60dab697

Worth a look — over the floor, under this run's gate, so not failed:

unicode time 23.59 → 24.85 ns/char (5.3%, under its 12.1% gate but over the 5% floor)

Re-run on a quieter machine, or with more --chars, to tell a real change from a busy runner. Both narrow the noise column, which tightens the gate.

DECCOLM erases, DECNCSM (mode 95) turns that off, and DECSCPP never erases -- three callers of one resize that differ in exactly one thing, so the erase is a parameter of SetPageWidth rather than something each repeats. DECNCSM did not exist (#143), so a column change always cleared and there was no way to change width and keep the screen. It is real state now, so DECRQM answers for it too. DECSCPP did not exist (#138): no entry in the CSI table, so CSI Ps $ | was parsed and dropped. DECSLPP was NOT broken, and the other half of #138 is withdrawn. CSI Ps t with Ps >= 24 has always resized, and a probe confirms it: 80x24 -> 80x25 -> 80x48, raising Resized each time. What misled me was vttest reporting "24 of 25 rows" -- that is vttest's own view of a size it samples when a test starts and does not re-measure, not the terminal's. A pty resize does reach the child; a separate probe running stty inside WSL shows 24 80 then 48 132 across one. Two things came out of that. The harness dumped a hardcoded 24 rows, which hid every row a resize added and made a working resize look like a missing one -- it follows terminal.Rows now. And the tests pin all three controls directly, since vttest cannot check any of them through a pty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

CHT moved the cursor without checking for a pending wrap, which cancelled it, so a mark printed after tabbing to the last stop overwrote the last column instead of wrapping onto the next row (#144). HT checked, CHT did not, and vttest draws the same row of marks both ways and expects them to look the same. They share one motion now rather than gaining a second copy of the guard. These two have drifted twice: first on the stop set, when HT hardcoded 8 while CHT honoured Options.TabStopWidth, and now on the phantom column. Backspace moved into the input handler for the same reason, so HT follows it there. DECREQTPARM answers (#124). Parity, bit count, line speeds and a clock multiplier mean nothing to an emulator, and the values are the ones xterm sends for that reason -- no parity, 8 bits, the highest speed code. It is answered anyway because silence is not a decline: vttest's screen paints nothing at all against a terminal that says nothing. Ps 0 reports sol=2, Ps 1 sol=3, and anything else is ignored so a REPORT arriving on the input is not answered as though it were a request. Verified through vttest: it marks both DECREQTPARM arguments -- OK, and the CHT row of marks now wraps like the plain-tab row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

S8C1T asked for 8-bit C1 controls and got 7-bit ones, because the two announcement escapes were never dispatched: HandleEsc's intermediate switch covered charset designation and DEC line attributes, and ESC SP F and ESC SP G fell through it (#130). Converted at RaiseDataReceived rather than at the fifty-odd places that build a reply. They all write the 7-bit form, which stays readable and is what the tests are written against, and this is the one point where the choice can be applied without every one of them remembering it exists. Only the LEADING introducer and a trailing ST are rewritten. A reply payload can hold anything -- base64 in OSC 52, a settings body in DECRQSS -- and rewriting every ESC pair anywhere would corrupt one that merely looked like an introducer. RIS puts it back to 7-bit. S8C1T is a choice a program made about this session, and leaving it set would send 8-bit C1 controls to whatever runs next, which never asked for them. Verified across the families, not just the one vttest tests: DSR, primary DA, DECRQM and the window reports all switch from <1B>[ to <9B>, and the OSC title reply switches both ends, <9D> ... <9C>. One honest limit. vttest still marks its 8-bit case failed, and the reason is not here: it reads <194> <155>, which is U+009B encoded as UTF-8 by the harness before it reaches the pty. The emulator's reply carries the C1 character; whether it reaches the wire as a raw 0x9B is the host's encoding decision, and a raw 0x9B is not representable in UTF-8 -- which is why xterm ties 8-bit controls to non-UTF-8 operation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

…n space Fixes #136 DEC Special Graphics was missing b, c, d and e -- the control pictures for HT, FF, CR and LF -- so they printed as plain letters in the middle of a row of symbols that were all correct. `abcdefghi now reads diamond, stipple, HT, FF, CR, LF, degree, plus-minus, NL, VT. ESC - , ESC . and ESC / were not dispatched at all, so a 96-character set could be designated and was silently ASCII. Their identifiers are a DIFFERENT space, which is the part worth being careful about: 'A' is ISO Latin-1 here and the United Kingdom set after ESC (. Routing these through the 94-set lookup would designate UK for a program that asked for Latin-1 and turn its '#' into a pound sign -- invisible in any test whose text avoids that one character, which is how a bug like this survives. Verified both ways: ESC ( A gives the pound, ESC - A leaves '#' alone. Latin-1 is a pass-through, so the sets themselves need no tables; an unknown 96-set identifier stays ASCII rather than being guessed at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

tomlm · 2026-08-31T23:45:09Z

@claude review these changes and flag high/medium or documentation issues.

claude · 2026-08-31T23:45:23Z

Claude encountered an error after 0s —— View job

I'll analyze this and get back to you.

Copilot

🟡 Changes recommended

Reset-state leaks, charset re-resolution errors, protected-erasure behavior, and paste mutation can produce incorrect terminal output.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds VTTEST-driven conformance tooling, regression tests, and terminal compatibility fixes.

Changes:

Adds a headless VTTEST/tmux comparison harness.
Implements missing DEC reports, modes, page sizing, shifts, and character sets.
Adds regression coverage for discovered and verified behavior.

File summaries

File	Description
`tools/vttest/vtsweep.py`	Compares VTTEST screens against tmux.
`tools/vttest/VtDrive/VtDrive.csproj`	Defines the headless PTY harness project.
`tools/vttest/VtDrive/Program.cs`	Drives VTTEST through XTerm.NET.
`tools/vttest/.gitignore`	Excludes harness build artifacts.
`tools/README.md`	Documents VTTEST tooling.
`src/XTerm.NET/Terminal.cs`	Adds page-width, mode, and C1 reply handling.
`src/XTerm.NET/InputHandler.Protection.cs`	Resets attributes on full-line erasure.
`src/XTerm.NET/InputHandler.Print.cs`	Adds charset designation and shift behavior.
`src/XTerm.NET/InputHandler.Modes.cs`	Updates mode querying and DECNRCM handling.
`src/XTerm.NET/InputHandler.Csi.cs`	Adds tabs, reports, and pixel fallbacks.
`src/XTerm.NET/InputHandler.cs`	Dispatches new CSI/ESC features.
`src/XTerm.NET/Common/TerminalMode.cs`	Defines DECNCSM.
`src/XTerm.NET/Common/CsiCommand.cs`	Defines new CSI commands.
`src/XTerm.NET/Common/CommandExtensions.cs`	Maps new CSI sequences.
`src/XTerm.NET/Common/Charsets.cs`	Adds graphics and national mappings.
`src/XTerm.NET.Tests/VtTestConformanceTests.cs`	Adds conformance regressions.
`src/XTerm.NET.Tests/VtTestBehaviourTests.cs`	Adds established-behavior coverage.
`src/XTerm.NET.Tests/RequestModeTests.cs`	Updates unknown-mode expectations.

Review details

Suppressed comments (4)

src/XTerm.NET/InputHandler.Print.cs:808

Resetting charsets leaves a pending SS2/SS3 and its old translation table intact. Thus an SS2 followed by DECSTR or RIS still translates the first subsequent character using the pre-reset G2 set. Clear _singleShiftCharset and _singleShiftPending here as part of restoring default charset state.

        _charsetIds.Clear();
        _charsets[CharsetMode.G0] = Charsets.ASCII;
        _charsets[CharsetMode.G1] = Charsets.ASCII;
        _charsets[CharsetMode.G2] = Charsets.ASCII;
        _charsets[CharsetMode.G3] = Charsets.ASCII;
        _currentCharset = CharsetMode.G0;
        RefreshActiveCharset();

src/XTerm.NET.Tests/VtTestConformanceTests.cs:217

The assertion contains a raw C1 control byte. Spell it as \u009b; otherwise tooling can reject or silently alter the test source while the escaped form asserts the same ordinal value.

        Assert.StartsWith("�", Assert.Single(replies), StringComparison.Ordinal);

src/XTerm.NET.Tests/VtTestBehaviourTests.cs:297

This test string embeds literal SO and SI control bytes. Use escaped code points so source tooling does not interpret or strip the controls.

        latin1.Write($"{Esc}-A�#@[�");

tools/README.md:49

These arguments contain raw carriage returns rather than the documented \r marker. Keep the marker visible so the command can be copied reliably and VtDrive performs the intended replacement.

dotnet run --project vttest/VtDrive -- - "6
" "3
"

Files reviewed: 18/18 changed files
Comments generated: 15
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    /// <summary>
+    /// DECNCSM (mode 95): whether a DECCOLM column change keeps the screen instead of erasing it.
+    /// </summary>
+    public bool NoClearOnColumnChange { get; set; }


+    /// A designated national set behaves as ASCII while this is false, so changing it re-resolves
+    /// what is already designated rather than only affecting the next designation.
+    /// </remarks>
+    public bool NationalReplacementCharsets { get; set; }


+    internal void RaiseDataReceived(string data) =>
+        DataReceived?.Invoke(this, new TerminalEvents.DataEventArgs(
+            EightBitControls ? ToEightBitControls(data) : data));


+        if (!selective && start == 0 && end >= _terminal.Cols)
+            line.LineAttribute = Buffer.LineAttribute.Normal;


+        _charsetIds[mode] = charsetId;
+        _charsets[mode] = Charsets.ASCII;


+    [InlineData("	")]
+    [InlineData("[1I")]
+    [InlineData("[I")]


    /// <summary>DECCRA -- copy a rectangular area (CSI ... $ v).</summary>
+    SetColumnsPerPage,
    CopyRectangularArea,


+    out = subprocess.run(
+        ["dotnet", "run", "-c", "Debug", "--no-build", "--", *keys],
+        cwd=VTDRIVE, capture_output=True, text=True, timeout=900).stdout


+            literal = key.replace("\\r", "")
+            if literal:
+                wsl(f"tmux send-keys -t vt '{literal}'")


JohnCampionJr

Reviewed at e0edfd5. Built clean, 2142 passed / 1 skipped / 0 failed, and I re-checked each issue this closes against the code rather than the description -- #124, #125, #126, #129, #130, #131, #132, #138, #143 and #144 all do what the body says they do. The three places where the body flags that it is overturning an earlier decision (#125's test, #132's designation-by-ID, #130's single conversion point) are the right calls and are argued rather than buried, and the two self-corrections are worth more than the fixes.

Everything below is verified against this branch with a probe, not read off the diff. Nothing here is an objection to the approach.

Four worth fixing before merge, in order:

Bracketed paste comes out half 8-bit and half 7-bit under S8C1T -- an application never sees the end of a paste.
DECNCSM and DECNRCM survive RIS, which is the exact failure the Sixel comment twenty lines above describes.
A 96-set designation is re-resolved through the 94-set table when DECNRCM moves -- the one collision e0edfd5's own commit message says it was careful about.
A pending single shift survives RIS and SO, holding a table that has been reset out from under it.

Then two smaller behavioural ones (an ISO-guarded line loses its double-width attribute; DECSCPP resets the scrolling region and homes the cursor), one coverage gap (the NRCS alternate designators), and three nits.

On the harness: keeping it is right, and tools/README.md saying plainly what tmux cannot referee is the part that will still be useful in a year. It is Windows-only as written, which is the one thing stopping any of its findings being re-checked where the rest of CI runs -- see the comment on Program.cs.

JohnCampionJr · 2026-09-01T00:02:29Z

+    /// rewriting every ESC pair found anywhere would corrupt one that happened to look like an
+    /// introducer.</para>
+    /// </remarks>
+    internal void RaiseDataReceived(string data) =>


Paste goes through the reply converter, and comes out with a mismatched frame.

Paste (Terminal.cs:1991) raises through this same method, so S8C1T rewrites its leading introducer and -- correctly, by this method's own rule -- leaves the rest alone. The result:

ESC[?2004h ESC SP G then paste "hi" -> <9B>200~hi<1B>[201~

The open bracket is 8-bit and the close is 7-bit. An application matching ESC[201~ for the end of a paste never finds it, and one matching CSI 201~ in either form still sees a frame whose two halves disagree.

The deeper point is that paste is not a reply at all. EightBitControls' own doc says "Output only. Mode 1034 is the other direction -- what the KEYBOARD sends -- and the two are independent" -- and paste is keyboard-direction traffic. S8C1T should not touch it whichever encoding it lands in.

Cheapest fix that keeps the single conversion point: have Paste (and anything else that is input rather than a report) bypass the converter -- raise the event directly, or add an internal void RaiseInputData(string) next to this one. The report sites are all fine as they are; I checked :849 and the three 5522 replies at :2056-2058, which each end in their own ST and convert correctly.

JohnCampionJr · 2026-09-01T00:02:29Z

+        // Replies go back to 7-bit. S8C1T is a choice a program made about this session, and
+        // RIS is where a session's choices end -- leaving it set would send 8-bit C1 controls
+        // to whatever runs next, which never asked for them.
+        EightBitControls = false;


EightBitControls joined this list; DECNCSM and DECNRCM did not.

Both new modes survive RIS:

ESC[?95h ESC[?42h ESC c then ESC[?95$p ESC[?42$p -> ESC[?95;1$y ESC[?42;1$y

and the DECNRCM one is visible, not just a reporting error:

ESC[?42h ESC c ESC(R then print "@[" -> a-grave, degree (French, with NRC never re-enabled)

ResetCharsets() clears the designations, so nothing is wrong until the next program designates -- and then it gets a national set it never asked for. NoClearOnColumnChange is the same shape: after RIS the next ESC[?3h silently skips the clean slate DEC guarantees.

This is the failure the comment fifteen lines below already writes down for Sixel -- "The three Sixel modes survived RIS, so DECRQM went on reporting mode 80 as set after a reset and the next image drew with the previous program's geometry." Two lines here, and the DECNRCM one wants _inputHandler.RefreshDesignatedCharsets() after it so the resolved tables follow the flag down.

JohnCampionJr · 2026-09-01T00:02:29Z

+    internal void RefreshDesignatedCharsets()
+    {
+        foreach (var mode in _charsetIds.Keys.ToList())
+            _charsets[mode] = Charsets.GetCharset(_charsetIds[mode], _terminal.NationalReplacementCharsets);


This walks every designation through the 94-set lookup, including the ones SetNinetySixCharset put there.

e0edfd5's commit message calls out exactly this collision -- "'A' is ISO Latin-1 here and the United Kingdom set after ESC (. Routing these through the 94-set lookup would designate UK for a program that asked for Latin-1 and turn its '#' into a pound sign" -- and the designation path is careful about it. This path is not, so the bug arrives one mode change later:

ESC-A G1 = ISO Latin-1, correctly stored as ASCII pass-through ESC[?42h DECNRCM on -> RefreshDesignatedCharsets SO then "#" -> pound sign

_charsetIds cannot distinguish the two spaces, so it needs to carry which one the ID came from -- a second dictionary, a flag alongside the ID, or a sentinel prefix. Whichever, this loop then has to skip or separately resolve the 96-set entries.

Worth noting the commit message's own standard here: "invisible in any test whose text avoids that one character, which is how a bug like this survives." The new test designates ESC - A and checks #, which is the right check -- it just never moves DECNRCM afterwards.

JohnCampionJr · 2026-09-01T00:02:29Z

    /// </summary>
    public void ResetCharsets()
    {
+        _charsetIds.Clear();


A pending single shift survives this, and keeps the table that was just reset.

_singleShiftPending / _singleShiftCharset are not cleared here, and _singleShiftCharset holds a reference to the old table rather than the mode, so it still resolves after _charsets has gone back to ASCII:

ESC*0 ESC N ESC c then "q" -> box-drawing horizontal

RIS is how someone recovers from a program that died mid-sequence, and this is the state most likely to be left dangling by one.

Same gap in the shift handlers, asymmetrically: ShiftIn() clears both fields, ShiftOut() and LockingShift() do not.

ESC*0 ESC N SO then "q" -> box-drawing horizontal

Whether an intervening SI/SO should cancel a pending single shift is arguable, but the three cannot each answer it differently. ShiftIn already picked "yes"; the other two should agree with it, and ResetCharsets should clear regardless.

JohnCampionJr · 2026-09-01T00:02:29Z

+        // meant to be double, and a selective erase exists precisely to leave protected text
+        // standing -- resizing the line under it would undo that in the one case it was asked
+        // to preserve.
+        if (!selective && start == 0 && end >= _terminal.Cols)


This runs before the protection branch, so it fires on erases that leave text standing.

Under ProtectionIso a guarded cell survives even a non-selective erase -- that is the documented xterm deviation the loop below implements. So a full ED reaches this line, resets the attribute, and then declines to erase the text:

ESC#6 ESC V GUARDED ESC W ESC[2J -> text "GUARDED" still there, LineAttribute DoubleWidth -> Normal

The comment two lines up is the argument against it: "a selective erase exists precisely to leave protected text standing -- resizing the line under it would undo that in the one case it was asked to preserve." That reasoning does not depend on the erase being selective, only on text surviving it, and here text survives a non-selective one.

Cleanest fix is to move the reset after the erase and condition it on nothing having been skipped -- the fast line.Fill branch always qualifies, and the loop can carry a skipped flag. That also makes the rule easier to state: the attribute goes when the line is actually empty.

JohnCampionJr · 2026-09-01T00:02:29Z

+    /// <para>They apply only while DECNRCM (mode 42) is set. With it reset the designation is
+    /// still remembered, but the set behaves as ASCII -- which is what the mode is for.</para>
+    /// </remarks>
+    public static readonly Dictionary<string, Dictionary<char, string>> National = new()


The alternate designators are missing, and the DA advertises attribute 9.

Each of these sets has more than one spelling in the DEC scheme, and only one of each is here. Designating each in turn and printing @[:

R = a-grave, degree f = @[ (French) H = E-acute, A-uml 7 = @[ (Swedish) C = @, A-uml 5 = @[ (Finnish) E = A-uml, AE 6 = @[ (Norwegian/Danish) Q = @[ 9 = @[ (French Canadian -- neither spelling)

PrimaryDeviceAttributes lists 9, so a program is entitled to send any of them. Falling through to ASCII is the failure mode this type's own remarks describe -- "why a missing one is invisible: the text comes out almost right".

Cheap fix for five of them: alias the identifiers into National (f->R, 7->H, 5->C, 6 and backtick ->E). French Canadian and Portuguese (%6) are new tables and can reasonably wait -- but if they do, it is worth saying so here, since the DA does not distinguish.

JohnCampionJr · 2026-09-01T00:02:30Z

@@ -0,0 +1,300 @@
+using XTerm.Options;


Line endings: this is the only file in the diff with CRLF, and 80 of its 300 lines end CR CR LF.

Its sibling VtTestConformanceTests.cs and all sixteen other files in this PR are LF. The doubled CR will show as a whole-file change the next time anyone's editor normalises it, and it makes every future diff of this file unreadable.

sed -i '' 's/\r$//' over the file, or let .gitattributes handle it if the repo grows one.

JohnCampionJr · 2026-09-01T00:02:30Z

+            return;
+
+        var sol = request == 0 ? 2 : 3;
+        _terminal.RaiseDataReceived($"[{sol};1;1;128;128;1;0x");


Literal ESC byte in the source.

This is the only string in this file that carries a raw 0x1B instead of \u001b -- every other reply, including the three winop reports in this same diff, spells it out. CLAUDE.md is explicit about it: "A literal ESC byte written into a source file or a shell command corrupts it, and the tooling rejects control characters outright."

It works today; it is the kind of thing that survives until something in the pipeline eats it. Same for VtTestConformanceTests.cs:23 and the two InlineData strings at VtTestBehaviourTests.cs:246-247 -- the note there suggests ((char)0x1B).ToString(), which nothing can mangle in transit.

JohnCampionJr · 2026-09-01T00:02:30Z

    SoftReset,

    /// <summary>DECCRA -- copy a rectangular area (CSI ... $ v).</summary>
+    SetColumnsPerPage,


This member was inserted between DECCRA's doc comment and DECCRA.

SetColumnsPerPage now carries /// <summary>DECCRA -- copy a rectangular area (CSI ... $ v).</summary>, and CopyRectangularArea has no doc at all. Swapping the two lines fixes both.

RequestTerminalParameters at :142 is the other one -- it is the only member in the enum with no summary.

JohnCampionJr · 2026-09-01T00:02:30Z

+    Cols = 80,
+    Rows = 24,
+    Cwd = Environment.CurrentDirectory,
+    App = "wsl.exe",


The harness is Windows-only, which bounds what it can ever settle.

wsl.exe here and wsl(...) throughout vtsweep.py mean no finding this produces can be re-checked on macOS or Linux -- where the rest of CI runs, and where the next person reading one of these issues probably is. That matters more than usual for this PR, because two of its findings turned out to be harness artifacts rather than emulator defects; being able to reproduce on a second platform is how the next one gets caught before it becomes an issue.

It is a couple of lines:

var vttest = Environment.GetEnvironmentVariable("VTTEST"); var onWindows = OperatingSystem.IsWindows(); App = onWindows ? "wsl.exe" : vttest ?? "vttest", CommandLine = onWindows ? ["-e", vttest ?? "/usr/bin/vttest"] : [],

I tried this locally against Homebrew's vttest 2.7 (20251205). It spawns and paints the menu correctly, so the emulator and the pty wiring are fine -- but keystrokes written to the master after startup never reach vttest (the bytes go out; the menu does not advance), so it starts vttest without being able to drive it. Worth knowing before someone else spends the same hour on it. vtsweep.py additionally needs tmux, which is a second dependency rather than a portability problem.

Behaviour: Paste no longer goes through the reply converter. S8C1T rewrote the LEADING introducer of a bracketed paste and left the closing bracket 7-bit, so the two halves of the frame disagreed and an application watching for ESC [ 201 ~ never saw the paste end. Paste is keyboard-direction traffic; RaiseInputData carries it untouched. DECNCSM and DECNRCM survived both resets. Cleared in each, with RefreshDesignatedCharsets after the DECNRCM one so the resolved tables follow the flag down -- otherwise the next program to designate a national set got one it never asked for. RIS also calls ResetStoredModes, whose own summary says "for RIS" and which only the SOFT reset was calling, so DECRQM reported DECNRCM as set after RIS had already put the behaviour back. A full erase now resets the line attribute only when nothing survived it. Under ISO protection a guarded cell survives a plain erase, and shrinking the line under surviving double-width text is the same mistake in reverse. RefreshDesignatedCharsets re-resolves each designation in the space it came from. 'A' is ISO Latin-1 after ESC - and the United Kingdom set after ESC (, so walking every designation through the 94-set lookup handed a program that asked for Latin-1 the UK set at the first mode change -- the collision the designation path was already careful about, arriving one toggle later. Five national sets answer to both their designators (f, 7, 5, 6 and the backtick). An unregistered spelling fell through to ASCII, which is the failure this file's own remarks describe. French Canadian and Portuguese are tables this terminal does not have, not spellings of ones it does, and are named as such. DECSCPP ignores a width it does not define. Coercing turned CSI 81 $ | into a resize to 80 and CSI 999 $ | into one to 132, so a malformed request moved the screen instead of being declined. Tooling and hygiene: vtsweep.py checks the harness exit code -- a failed build otherwise returned no screens, and no screens reads as "the terminals differ" -- and quotes keys through shlex before handing them to bash -lc. Literal control bytes are escaped in every source they had crept into, and VtTestBehaviourTests.cs is back to LF: 80 of its lines ended CR CR LF, from scripted writes that added a CR to content that already had one. The README examples show the visible \r marker vtsweep.py expects rather than carrying real carriage returns. DECSCPP and DECREQTPARM have their own enum summaries; DECSCPP had inherited DECCRA's and left DECCRA undocumented. The harness being Windows-only is written down in tools/README.md, including that a native spawn is easy and driving it is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015XLGxUGrQiUnsqz3yneHsy

tomlm · 2026-09-01T00:14:48Z

Review addressed in 1f08d48. Suite 2149 passed, 1 skipped.

Behaviour

Paste no longer goes through the reply converter. The sharpest of these — it corrupted user data, not just a report. S8C1T rewrote the leading introducer and left the closing bracket 7-bit, so the frame's halves disagreed. RaiseInputData now carries keyboard-direction traffic untouched:

ESC[?2004h  ESC SP G  paste "hi"   ->  <1B>[200~hi<1B>[201~

Both new modes come down on both resets, in behaviour and in what DECRQM reports. That turned up something wider while checking it: ResetStoredModes says "Puts every stored toggle back to its default, for RIS" and only the soft reset was calling it — so every stored mode survived the hard one. RIS calls it now.

after RIS      <1B>[?95;2$y   <1B>[?42;2$y      French after RIS: '@['
after DECSTR   <1B>[?95;2$y   <1B>[?42;2$y      French after DECSTR: '@['

A full erase resets the line attribute only when nothing survived. Under ISO protection a guarded cell survives a plain erase, and shrinking the line under surviving double-width text is the same mistake in reverse — ESC#6, SPA KEEP EPA, ED now leaves KEEP at DoubleWidth.

RefreshDesignatedCharsets re-resolves in the space each designation came from. Exactly the collision the designation path was careful about, arriving one mode-toggle later; the 96-set class is remembered alongside the ID.

Five national sets answer to both designators (f, 7, 5, 6, `). French Canadian and Portuguese are named in the remarks as tables this terminal does not have, rather than spellings of ones it does.

DECSCPP declines widths it does not define — CSI 81 $ | and CSI 999 $ | both leave the screen alone instead of resizing to 80 and 132.

Tooling and hygiene

vtsweep.py checks the harness exit code (a failed build returned no screens, which reads downstream as "the terminals differ") and quotes keys with shlex.quote. Literal control bytes escaped everywhere they had crept in. VtTestBehaviourTests.cs is back to LF — the committed blob had 80 lines ending CR CR LF, from scripted writes adding a CR to content that already had one; verified 0 CR in the new blob. README examples show the visible \r marker the script expects. DECSCPP and DECREQTPARM have their own enum summaries.

Not done, deliberately

The harness stays Windows-only, with the limitation written into tools/README.md including your finding that a native spawn is easy and driving it is not. Worth having in the file rather than only in this thread — the next person to try it would otherwise spend the same hour.

Five regression tests cover the review findings themselves, so none of them can come back quietly.

The file was saved as CP1252 at some point, and the transcode was lossy in two different ways at once. The characters Latin-1 has -- degree, plus-minus, pound, middle dot -- survived as raw high bytes, which is what makes the file invalid UTF-8. The twenty-three that Latin-1 does not have did not survive at all: they were written as literal question marks, so the comment naming each line-drawing glyph named nothing. The practical cost is that grep treats the file as binary and returns silently. Not an error, no output -- so a search for a character set by name looks like the character set does not exist. It cost me a few minutes during a review of #141, which is how it was noticed. The four control pictures added by that PR had no glyph either, having been written where none could be. They gain one here, so every entry in the table reads the same way. Comments only: the code outside them is byte-identical, and every glyph is derived from the escape on its own line rather than typed, so a comment cannot disagree with the entry it describes. The rest of the repository is clean -- the only other files that fail a UTF-8 decode are the PNGs, which is as it should be. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Recovered from tomlm#146, which was closed unmerged when the branch it was stacked on went away with tomlm#141. The 96-set half of that PR landed independently in 1f08d48; this half did not, and still reproduces on 6d32504. DECSC saved the TABLE each G-set had resolved to. The screen after a DECRC was therefore right and the state behind it was not: the identifier stayed as whatever had been designated after the save, so the next DECNRCM re-resolved the restored slot into that instead. ESC ( 0, DECSC, ESC ( R, DECRC line drawing back, correct ...then DECNRCM letters -- re-resolved as French ESC - A, DECSC, ESC ) A, DECRC Latin-1 back, correct ...then DECNRCM a pound sign -- the UK set The first is older than the 96-set work; it arrived with the identifiers themselves. The second is the identifier collision again, one save and restore later: A is ISO Latin-1 in the 96-set space and the United Kingdom set in the 94-set one, and the saved table records neither. DECSC now saves the designation with the space it came from, and DECRC resolves it against the mode state as it is then -- which is also the right answer when DECNRCM moved between the two. One Resolve answers "what does this designation mean now" for all three callers that ask: the designation path, DECRC, and the DECNRCM refresh. They resolved separately before, which is how two of them disagreed. Every G-set is seeded to B rather than left absent, so a designation is a value rather than a value-or-absent and the walks over the four -- the save, the restore, the DECNRCM refresh -- are all total. Also from Copilot's review on tomlm#146: the separate-space test designated G0 for its UK half and G1 for its Latin-1 half, so the two differed by two things rather than one. Both designate G1 and invoke it with SO now, which also takes the last two literal control bytes out of the file. The new test asserts the restore WITHOUT a mode change as well. That is the part that was never broken, and a test that stopped there would pass on the defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

From the review on tomlm#141. Four ways in and three answers: SI cancelled a pending single shift, SO and the locking shifts left it standing, and RIS could not reach it at all. ESC * 0, SS2, RIS, then 'q' a line-drawing dash, on a terminal that had just been reset to ASCII RIS is the sharpest of the three because the pending state held the TABLE G2 had resolved to. ResetCharsets put the tables back and the shift went on pointing at the one it had captured, so the reset was undone by a character arriving after it. The pending state is now the G-SET rather than a table, resolved when the character arrives. That fixes a third case the same way: a designation between the shift and the character it shifts belongs to that character, because SS2 invokes G2 and what G2 holds is a question with an answer at print time. ESC N, ESC * 0, then 'q' printed a letter; the shift had captured G2 before it was designated SI no longer cancels. The VT510 manual scopes a single shift to "the next graphic character", and a locking shift is not one -- so the three that left it standing were right and the one that did not was wrong. Nothing in the suite depended on the old behaviour. The test carries the three that already worked -- SO, LS2, and a shift spending itself on one character rather than two -- alongside the three that did not, because a test of only the broken cases cannot tell "missing" from "misrouted". Verified against the defect: with the production files reverted the test fails on the RIS case, and the earlier probe fails on the SI and designate-after-shift cases too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Save the designation at DECSC, not the table it resolved to Recovered from #146, which was closed unmerged when the branch it was stacked on went away with #141. The 96-set half of that PR landed independently in 1f08d48; this half did not, and still reproduces on 6d32504. DECSC saved the TABLE each G-set had resolved to. The screen after a DECRC was therefore right and the state behind it was not: the identifier stayed as whatever had been designated after the save, so the next DECNRCM re-resolved the restored slot into that instead. ESC ( 0, DECSC, ESC ( R, DECRC line drawing back, correct ...then DECNRCM letters -- re-resolved as French ESC - A, DECSC, ESC ) A, DECRC Latin-1 back, correct ...then DECNRCM a pound sign -- the UK set The first is older than the 96-set work; it arrived with the identifiers themselves. The second is the identifier collision again, one save and restore later: A is ISO Latin-1 in the 96-set space and the United Kingdom set in the 94-set one, and the saved table records neither. DECSC now saves the designation with the space it came from, and DECRC resolves it against the mode state as it is then -- which is also the right answer when DECNRCM moved between the two. One Resolve answers "what does this designation mean now" for all three callers that ask: the designation path, DECRC, and the DECNRCM refresh. They resolved separately before, which is how two of them disagreed. Every G-set is seeded to B rather than left absent, so a designation is a value rather than a value-or-absent and the walks over the four -- the save, the restore, the DECNRCM refresh -- are all total. Also from Copilot's review on #146: the separate-space test designated G0 for its UK half and G1 for its Latin-1 half, so the two differed by two things rather than one. Both designate G1 and invoke it with SO now, which also takes the last two literal control bytes out of the file. The new test asserts the restore WITHOUT a mode change as well. That is the part that was never broken, and a test that stopped there would pass on the defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Address Copilot's review: narrow the property, cover the mode change Two findings, both worth acting on, one for a different reason than the one given. The API break is real as a type change and cannot break anyone: the property arrived in #93 on 2026-08-29, and the latest release is v1.2 from 2026-08-26, so no published package has ever carried it. That makes the answer easier rather than harder -- it is DECSC scratch a consumer cannot do anything with, so it is internal now, and the shape of an internal detail stops being an API question every time DECSC learns something more. Nothing outside the assembly referenced it; the solution builds, demos included. The untested case was the sharper find. The three cases in the test move DECNRCM AFTER the DECRC, so replaying a saved table would satisfy all three -- the very contract the doc comment claims, that a restore resolves against the mode as it is at restore time, had no test at all. Both directions now, because they fail oppositely: French designated with NRC off, DECSC, NRC on, DECRC -> a-grave French designated with NRC on, DECSC, NRC off, DECRC -> @ On the pre-fix code those give @ and a-grave respectively -- each the table that was saved rather than what the designation means at the moment it is put back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

From the review on tomlm#141. Four ways in and three answers: SI cancelled a pending single shift, SO and the locking shifts left it standing, and RIS could not reach it at all. ESC * 0, SS2, RIS, then 'q' a line-drawing dash, on a terminal that had just been reset to ASCII RIS is the sharpest of the three because the pending state held the TABLE G2 had resolved to. ResetCharsets put the tables back and the shift went on pointing at the one it had captured, so the reset was undone by a character arriving after it. The pending state is now the G-SET rather than a table, resolved when the character arrives. That fixes a third case the same way: a designation between the shift and the character it shifts belongs to that character, because SS2 invokes G2 and what G2 holds is a question with an answer at print time. ESC N, ESC * 0, then 'q' printed a letter; the shift had captured G2 before it was designated SI no longer cancels. The VT510 manual scopes a single shift to "the next graphic character", and a locking shift is not one -- so the three that left it standing were right and the one that did not was wrong. Nothing in the suite depended on the old behaviour. The test carries the three that already worked -- SO, LS2, and a shift spending itself on one character rather than two -- alongside the three that did not, because a test of only the broken cases cannot tell "missing" from "misrouted". Verified against the defect: with the production files reverted the test fails on the RIS case, and the earlier probe fails on the SI and designate-after-shift cases too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…150) * Spend a single shift on the next graphic character, and nothing else From the review on #141. Four ways in and three answers: SI cancelled a pending single shift, SO and the locking shifts left it standing, and RIS could not reach it at all. ESC * 0, SS2, RIS, then 'q' a line-drawing dash, on a terminal that had just been reset to ASCII RIS is the sharpest of the three because the pending state held the TABLE G2 had resolved to. ResetCharsets put the tables back and the shift went on pointing at the one it had captured, so the reset was undone by a character arriving after it. The pending state is now the G-SET rather than a table, resolved when the character arrives. That fixes a third case the same way: a designation between the shift and the character it shifts belongs to that character, because SS2 invokes G2 and what G2 holds is a question with an answer at print time. ESC N, ESC * 0, then 'q' printed a letter; the shift had captured G2 before it was designated SI no longer cancels. The VT510 manual scopes a single shift to "the next graphic character", and a locking shift is not one -- so the three that left it standing were right and the one that did not was wrong. Nothing in the suite depended on the old behaviour. The test carries the three that already worked -- SO, LS2, and a shift spending itself on one character rather than two -- alongside the three that did not, because a test of only the broken cases cannot tell "missing" from "misrouted". Verified against the defect: with the production files reverted the test fails on the RIS case, and the earlier probe fails on the SI and designate-after-shift cases too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Spend the shift on a supplementary character too From Copilot's review. The invariant this branch sets out -- a single shift is spent by the next graphic character -- held only for characters that fit in one UTF-16 code unit. A supplementary character reaches Print as two, and the clear that spends the shift sat inside the single-code-unit branch. That does not skip the shift, it MOVES it. ESC * 0, SS2, an emoji, then 'q' drew the q as a box-drawing glyph on a terminal whose G2 the program had finished with -- the same shape as the RIS case this branch already fixes, reached by a different route. The emoji itself stays untranslated: a 94-character set has no entry outside the BMP, and TranslateChar takes a single code unit. Spending the shift and translating through it are separate things, and only the first applies here. The clear goes AFTER the length test rather than before it. The review suggested consuming the shift ahead of the graphic-character branches, which would put a null check in front of the test every ordinary character takes; CLAUDE.md opens with that. This branch runs for a surrogate pair only, and only to clear a field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

tomlm and others added 6 commits August 31, 2026 13:11

Merge branch 'main' into test/vttest-conformance

4eaa398

This was referenced Aug 31, 2026

Reach G2 and G3, and implement the national replacement sets #135

Closed

Answer DECRQM and the pixel reports, and un-double an erased line #134

Closed

Keep the vttest sweep harness #133

Closed

Port the vttest cases that judge themselves #127

Closed

tomlm and others added 4 commits August 31, 2026 15:22

JohnCampionJr requested a balanced review from Copilot August 31, 2026 23:49

Copilot started reviewing on behalf of JohnCampionJr August 31, 2026 23:50 View session

This was referenced Aug 31, 2026

Keep a 96-set designation out of the 94-set lookup when DECNRCM moves #146

Closed

Rows vanish from vttest's SI/SO screen after a 96-character G1 designation (ESC - A) #137

Closed

Copilot AI reviewed Aug 31, 2026

View reviewed changes

JohnCampionJr reviewed Sep 1, 2026

View reviewed changes

tomlm merged commit 6d32504 into main Sep 1, 2026
4 checks passed

tomlm deleted the vttest-alignment branch September 1, 2026 00:20

tomlm mentioned this pull request Sep 1, 2026

Page-format controls DECSCPP and DECSLPP are ignored #138

Closed

JohnCampionJr mentioned this pull request Sep 1, 2026

Write Charsets.cs as UTF-8, and put back the glyphs it lost #147

Merged

JohnCampionJr mentioned this pull request Sep 1, 2026

Save the designation at DECSC, not the table it resolved to #149

Merged

JohnCampionJr mentioned this pull request Sep 1, 2026

Spend a single shift on the next graphic character, and nothing else #150

Merged

		if (!selective && start == 0 && end >= _terminal.Cols)
		line.LineAttribute = Buffer.LineAttribute.Normal;

		_charsetIds[mode] = charsetId;
		_charsets[mode] = Charsets.ASCII;

Conversation

tomlm commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixed

Two corrections to my own reports

Tests

Tooling

Still open, deliberately

Verification

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf comparison — this change, against its base

Perf comparison — cumulative, everything since 2.0.0-rc002

Uh oh!

tomlm commented Aug 31, 2026

Uh oh!

claude Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Uh oh!

JohnCampionJr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomlm commented Sep 1, 2026

Behaviour

Tooling and hygiene

Not done, deliberately

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomlm commented Aug 31, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading

claude Bot commented Aug 31, 2026 •

edited

Loading