From 3f5b40cf7553e2ecd6bcd02c69f4af6a6e030e93 Mon Sep 17 00:00:00 2001 From: John Campion Jr Date: Mon, 31 Aug 2026 19:52:03 -0400 Subject: [PATCH 1/2] Keep a 96-set designation out of the 94-set lookup when DECNRCM moves Fixes #137 ESC - A designates ISO Latin-1 and ESC ( A designates the United Kingdom set, and the 96-set designators were given their own path so the two could not be confused. The identifier was then stored without the space it came from, and DECNRCM re-resolves every stored identifier through the 94-set lookup -- so the collision came back the moment the mode moved, and stayed, because resetting the mode re-resolves it the same wrong way. ESC - A, SO, print # -> # correct ESC - A, DECNRCM set, SO -> pound sign, the UK set A designation is now stored with its space and re-resolved in it. The national replacement sets are all 94-set, so a 96-set designation has no DECNRCM-dependent reading at all and is simply left alone. The screen this was filed against renders. vttest's SI/SO test draws three rows after ESC - A and the report says two of them vanish; replayed into a headless terminal, with and without SO, all three are present. What was really wrong is what the title now says, and it is invisible in that screen: the test's text is the ASCII range, where Latin-1 and UK differ in one character, '#'. The regression test invokes G1 with SO in every case, and carries a French designation alongside, so the two cannot agree because nothing is being re-resolved at all. SO and SI are written as escapes: the file had them as literal control bytes, which nothing that touches a source file on the way here preserves. 2143 passed, 1 skipped (DA3, #123), 0 failed. Co-Authored-By: Claude Opus 5 --- src/XTerm.NET.Tests/VtTestBehaviourTests.cs | 47 ++++++++++++++++++++- src/XTerm.NET/InputHandler.Print.cs | 28 +++++++++--- src/XTerm.NET/InputHandler.cs | 7 ++- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/XTerm.NET.Tests/VtTestBehaviourTests.cs b/src/XTerm.NET.Tests/VtTestBehaviourTests.cs index 41981bb..deae18b 100644 --- a/src/XTerm.NET.Tests/VtTestBehaviourTests.cs +++ b/src/XTerm.NET.Tests/VtTestBehaviourTests.cs @@ -21,6 +21,11 @@ public class VtTestBehaviourTests { private const string Esc = "\u001b"; + // Written as escapes rather than as the bytes themselves: a literal control character in a + // source file survives nothing that touches the file on the way here. + private const string ShiftOut = "\u000e"; // SO, invokes G1 into GL + private const string ShiftIn = "\u000f"; // SI, back to G0 + private static Terminal Sized(int cols = 40, int rows = 6) => new(new TerminalOptions { Cols = cols, Rows = rows }); @@ -294,7 +299,47 @@ public void The_96_character_set_designators_are_a_separate_space() Assert.Equal("£@[", uk.GetLine(0)); var latin1 = Sized(30, 3); - latin1.Write($"{Esc}-A#@["); + latin1.Write($"{Esc}-A{ShiftOut}#@[{ShiftIn}"); Assert.Equal("#@[", latin1.GetLine(0)); } + + /// + /// A 96-set designation survives DECNRCM moving under it. + /// + /// + /// DECNRCM re-resolves every designation, because a national set means one thing with the mode + /// set and ASCII without it. That re-resolution has to know which SPACE each identifier came + /// from, and it did not: 'A' designated as a 96-set is Latin-1 and stays Latin-1 however the + /// mode moves, while re-asking the 94-set lookup for it answers United Kingdom. So the + /// collision the 96-set designators were given their own space to avoid came back the first + /// time DECNRCM moved -- and stayed, because resetting the mode re-resolves it the same wrong + /// way. + /// + /// The national replacement sets are all 94-set, so there is no DECNRCM state in which a + /// 96-set designation means anything else; both directions are asserted rather than only the + /// one that was broken. The French half is here so the two cannot agree for the boring reason + /// that nothing is being re-resolved at all. + /// + [Fact] + public void A_96_set_designation_is_not_re_resolved_as_a_94_set() + { + var enabled = Sized(30, 3); + enabled.Write($"{Esc}-A"); + enabled.Write($"{Esc}[?42h"); // DECNRCM set, after the designation + enabled.Write($"{ShiftOut}#@[{ShiftIn}"); + Assert.Equal("#@[", enabled.GetLine(0)); + + var andBack = Sized(30, 3); + andBack.Write($"{Esc}-A"); + andBack.Write($"{Esc}[?42h"); + andBack.Write($"{Esc}[?42l"); // and reset again + andBack.Write($"{ShiftOut}#@[{ShiftIn}"); + Assert.Equal("#@[", andBack.GetLine(0)); + + var french = Sized(30, 3); + french.Write($"{Esc})R"); // French: a 94-set that DOES move + french.Write($"{Esc}[?42h"); + french.Write($"{ShiftOut}#@[{ShiftIn}"); + Assert.Equal("£à°", french.GetLine(0)); + } } diff --git a/src/XTerm.NET/InputHandler.Print.cs b/src/XTerm.NET/InputHandler.Print.cs index c9729eb..e46200b 100644 --- a/src/XTerm.NET/InputHandler.Print.cs +++ b/src/XTerm.NET/InputHandler.Print.cs @@ -715,9 +715,7 @@ private void SetCharset(CharsetMode mode, string charsetId) // with DECNRCM set and ASCII without it, so the designation has to outlive the // resolution -- a program designating French and then enabling NRC mode expects // French, and it never designates again. - _charsetIds[mode] = charsetId; - _charsets[mode] = Charsets.GetCharset(charsetId, _terminal.NationalReplacementCharsets); - RefreshActiveCharset(); + Designate(mode, charsetId, ninetySix: false); } /// @@ -732,16 +730,34 @@ private void SetCharset(CharsetMode mode, string charsetId) /// private void SetNinetySixCharset(CharsetMode mode, string charsetId) { - _charsetIds[mode] = charsetId; - _charsets[mode] = Charsets.ASCII; + Designate(mode, charsetId, ninetySix: true); + } + + private void Designate(CharsetMode mode, string charsetId, bool ninetySix) + { + var designation = (charsetId, ninetySix); + _charsetIds[mode] = designation; + _charsets[mode] = Resolve(designation); RefreshActiveCharset(); } + /// + /// The table a designation resolves to NOW -- which depends on DECNRCM, and so is asked + /// again every time that mode moves rather than being decided once at designation time. + /// + private Dictionary? Resolve((string Id, bool NinetySix) designation) => + // The 96-set space has its own identifiers, and only ASCII-transparent members: routing + // one through the 94-set lookup is what turns a Latin-1 designation into the UK set. + // DECNRCM does not reach here either -- the national replacement sets are all 94-set. + designation.NinetySix + ? Charsets.ASCII + : Charsets.GetCharset(designation.Id, _terminal.NationalReplacementCharsets); + /// Re-resolves every designation, for when DECNRCM changes under them. internal void RefreshDesignatedCharsets() { foreach (var mode in _charsetIds.Keys.ToList()) - _charsets[mode] = Charsets.GetCharset(_charsetIds[mode], _terminal.NationalReplacementCharsets); + _charsets[mode] = Resolve(_charsetIds[mode]); RefreshActiveCharset(); } diff --git a/src/XTerm.NET/InputHandler.cs b/src/XTerm.NET/InputHandler.cs index f3482d0..225e36d 100644 --- a/src/XTerm.NET/InputHandler.cs +++ b/src/XTerm.NET/InputHandler.cs @@ -44,8 +44,13 @@ public partial class InputHandler /// Kept alongside the resolved tables because a designation outlives its resolution: a /// national set resolves to ASCII while DECNRCM is reset and to itself once it is set, and /// the program that designated it does not designate again when the mode changes. + /// + /// The SPACE the identifier came from is kept with it, because the identifier alone does + /// not say which set it names: 'A' is the United Kingdom set after ESC ( and ISO Latin-1 + /// after ESC -. Re-resolving without it turns a Latin-1 designation into UK the first + /// time DECNRCM moves. /// - private readonly Dictionary _charsetIds = new(); + private readonly Dictionary _charsetIds = new(); /// /// The set a SINGLE shift has invoked for the next printed character, or null. From 7dff24425e16612a99ba34c3a08f5f313ec30a51 Mon Sep 17 00:00:00 2001 From: John Campion Jr Date: Mon, 31 Aug 2026 20:09:26 -0400 Subject: [PATCH 2/2] Save the designation at DECSC, not the table it resolved to Both of Copilot's findings on #146, and one they lead to. 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 That half is older than this branch; it arrived with the identifiers themselves. Copilot found the 96-set version of it, where a restored Latin-1 becomes the United Kingdom set, and the 94-set one above is the same fault a designation earlier. DECSC now saves the designation and DECRC resolves it against the mode state as it is then, which is also the right answer when DECNRCM moved between the two. 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. The other finding: the UK half of the separate-space test designated G0 and printed without a shift while the Latin-1 half designated G1 and shifted, so the two differed by two things rather than one. Both now designate G1 and invoke it with SO. The PR body had described the test this way before the test did. 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. 2144 passed, 1 skipped (DA3, #123), 0 failed. Co-Authored-By: Claude Opus 5 --- src/XTerm.NET.Tests/VtTestBehaviourTests.cs | 41 ++++++++++++++++++++- src/XTerm.NET/Buffer/TerminalBuffer.cs | 11 +++++- src/XTerm.NET/InputHandler.Csi.cs | 6 +-- src/XTerm.NET/InputHandler.Print.cs | 37 ++++++++++++++++--- src/XTerm.NET/InputHandler.cs | 12 +++++- 5 files changed, 93 insertions(+), 14 deletions(-) diff --git a/src/XTerm.NET.Tests/VtTestBehaviourTests.cs b/src/XTerm.NET.Tests/VtTestBehaviourTests.cs index deae18b..f0a1f03 100644 --- a/src/XTerm.NET.Tests/VtTestBehaviourTests.cs +++ b/src/XTerm.NET.Tests/VtTestBehaviourTests.cs @@ -295,7 +295,7 @@ public void Special_graphics_maps_the_control_pictures_too() public void The_96_character_set_designators_are_a_separate_space() { var uk = Sized(30, 3); - uk.Write($"{Esc}(A#@["); + uk.Write($"{Esc})A{ShiftOut}#@[{ShiftIn}"); Assert.Equal("£@[", uk.GetLine(0)); var latin1 = Sized(30, 3); @@ -342,4 +342,43 @@ public void A_96_set_designation_is_not_re_resolved_as_a_94_set() french.Write($"{ShiftOut}#@[{ShiftIn}"); Assert.Equal("£à°", french.GetLine(0)); } + + /// + /// DECRC puts back the DESIGNATION, so a later DECNRCM re-resolves what was restored. + /// + /// + /// DECSC saved the table each G-set had resolved to rather than what it was designated as, so + /// the identifier behind a restored slot stayed as whatever had been designated AFTER the + /// save. The screen was right and the state behind it was not, which is why this needs a mode + /// change to show at all: the next DECNRCM re-resolves the restored slot into the wrong set, + /// arbitrarily far from the DECRC that caused it. + /// + /// Both spaces, because they fail differently. The 94-set pair loses line drawing to a + /// national set -- ESC ( 0, DECSC, ESC ( R, DECRC draws borders until the mode moves and then + /// draws letters. The 96-set pair is the identifier collision again: Latin-1 restored, then + /// re-resolved as the United Kingdom set. + /// + [Fact] + public void DECRC_restores_what_was_designated_not_what_it_resolved_to() + { + var graphics = Sized(30, 3); + graphics.Write($"{Esc})0{Esc}7{Esc})R{Esc}8"); // graphics, DECSC, French, DECRC + graphics.Write($"{Esc}[?42h"); // DECNRCM, which re-resolves + graphics.Write($"{ShiftOut}qqq{ShiftIn}"); + Assert.Equal("───", graphics.GetLine(0)); + + var latin1 = Sized(30, 3); + latin1.Write($"{Esc}-A{Esc}7{Esc})A{Esc}8"); // Latin-1, DECSC, UK, DECRC + latin1.Write($"{Esc}[?42h"); + latin1.Write($"{ShiftOut}#@[{ShiftIn}"); + Assert.Equal("#@[", latin1.GetLine(0)); + + // And the restore itself, which was never the broken half: without the mode change both + // of the above already came back right, and a test that stopped there would pass on the + // defect. + var immediate = Sized(30, 3); + immediate.Write($"{Esc})0{Esc}7{Esc})R{Esc}8"); + immediate.Write($"{ShiftOut}qqq{ShiftIn}"); + Assert.Equal("───", immediate.GetLine(0)); + } } diff --git a/src/XTerm.NET/Buffer/TerminalBuffer.cs b/src/XTerm.NET/Buffer/TerminalBuffer.cs index 5532852..2544d85 100644 --- a/src/XTerm.NET/Buffer/TerminalBuffer.cs +++ b/src/XTerm.NET/Buffer/TerminalBuffer.cs @@ -136,10 +136,17 @@ public class SavedCursor /// and reused after: DECSC is not rare -- a full-screen program saves and restores the /// cursor on every redraw -- and copying a dictionary allocated once per save. There are /// exactly four G-slots and the enum numbers them from zero, so the save is four - /// reference writes. Null means DECSC has not run on this screen, which is what tells + /// writes. Null means DECSC has not run on this screen, which is what tells /// DECRC to leave the designations alone. + /// + /// The DESIGNATION each slot held, not the table it had resolved to. The two differ once + /// a mode moves under them: DECNRCM re-resolves every designation, so a slot restored as + /// a table carries whatever identifier was designated AFTER the save, and the next + /// DECNRCM re-resolves the restored slot into that instead. The identifier carries the + /// space it came from for the same reason -- 'A' is the United Kingdom set in the 94-set + /// space and ISO Latin-1 in the 96-set one. /// - public Dictionary?[]? Designations { get; set; } + public (string Id, bool NinetySix)[]? Designations { get; set; } public SavedCursor() { diff --git a/src/XTerm.NET/InputHandler.Csi.cs b/src/XTerm.NET/InputHandler.Csi.cs index c24aafd..2eff0f0 100644 --- a/src/XTerm.NET/InputHandler.Csi.cs +++ b/src/XTerm.NET/InputHandler.Csi.cs @@ -1609,9 +1609,9 @@ private void SaveCursor() // which set is selected, so saving _currentCharset alone restored a pointer to a table // the program had since replaced: a TUI that saved the cursor mid-border finished the box // in letters. - var designations = _buffer.SavedCursorState.Designations ??= new Dictionary?[4]; + var designations = _buffer.SavedCursorState.Designations ??= new (string, bool)[4]; for (var slot = 0; slot < designations.Length; slot++) - designations[slot] = _charsets.GetValueOrDefault((CharsetMode)slot); + designations[slot] = DesignationOf((CharsetMode)slot); _buffer.SavedCursorState.OriginMode = _terminal.OriginMode; _buffer.SavedCursorState.PendingWrap = _buffer.PendingWrap; } @@ -1632,7 +1632,7 @@ private void RestoreCursor() if (designations is not null) { for (var slot = 0; slot < designations.Length; slot++) - _charsets[(CharsetMode)slot] = designations[slot]; + RestoreDesignation((CharsetMode)slot, designations[slot]); } _currentCharset = _buffer.SavedCursorState.Charset; diff --git a/src/XTerm.NET/InputHandler.Print.cs b/src/XTerm.NET/InputHandler.Print.cs index e46200b..b1fc992 100644 --- a/src/XTerm.NET/InputHandler.Print.cs +++ b/src/XTerm.NET/InputHandler.Print.cs @@ -756,12 +756,31 @@ private void Designate(CharsetMode mode, string charsetId, bool ninetySix) /// Re-resolves every designation, for when DECNRCM changes under them. internal void RefreshDesignatedCharsets() { - foreach (var mode in _charsetIds.Keys.ToList()) + foreach (var mode in GSets) _charsets[mode] = Resolve(_charsetIds[mode]); RefreshActiveCharset(); } + /// The designation a G-set is holding, for DECSC to save. + internal (string Id, bool NinetySix) DesignationOf(CharsetMode mode) => _charsetIds[mode]; + + /// + /// Puts a saved designation back, for DECRC, resolving it against the mode state as it is NOW. + /// + /// + /// The DESIGNATION is what DECSC saves, not the table it had resolved to. Saving the table + /// restores the right glyphs and leaves the identifier behind it stale, so the next DECNRCM + /// re-resolves the restored slot from whatever was designated after the save -- and a program + /// doing ESC ( 0, DECSC, ESC ( R, DECRC gets its line drawing back and then loses it again the + /// first time the mode moves, which is further from the fault than any test looks. + /// + internal void RestoreDesignation(CharsetMode mode, (string Id, bool NinetySix) designation) + { + _charsetIds[mode] = designation; + _charsets[mode] = Resolve(designation); + } + /// /// Shift Out - Select G1 character set (SO, 0x0E). /// @@ -815,15 +834,21 @@ public void InvokeSingleShift(CharsetMode mode) /// public void ResetCharsets() { - _charsetIds.Clear(); - _charsets[CharsetMode.G0] = Charsets.ASCII; - _charsets[CharsetMode.G1] = Charsets.ASCII; - _charsets[CharsetMode.G2] = Charsets.ASCII; - _charsets[CharsetMode.G3] = Charsets.ASCII; + foreach (var mode in GSets) + { + // Seeded rather than cleared: US ASCII IS the designation every slot starts with, and + // saying so keeps every walk over the four total. + _charsetIds[mode] = (UsAsciiId, NinetySix: false); + _charsets[mode] = Charsets.ASCII; + } + _currentCharset = CharsetMode.G0; RefreshActiveCharset(); } + /// The identifier US ASCII is designated by, which is where every G-set starts. + private const string UsAsciiId = "B"; + /// /// Prints the payload of an OSC 66 whose metadata could not be parsed, as ordinary text. /// diff --git a/src/XTerm.NET/InputHandler.cs b/src/XTerm.NET/InputHandler.cs index 225e36d..cfa6023 100644 --- a/src/XTerm.NET/InputHandler.cs +++ b/src/XTerm.NET/InputHandler.cs @@ -49,9 +49,17 @@ public partial class InputHandler /// not say which set it names: 'A' is the United Kingdom set after ESC ( and ISO Latin-1 /// after ESC -. Re-resolving without it turns a Latin-1 designation into UK the first /// time DECNRCM moves. + /// + /// All four slots are always present, seeded to US ASCII, so a designation is a value + /// rather than a value-or-absent. Both DECRC and DECNRCM walk every slot, and "never + /// designated" and "designated B" mean the same thing to both. /// private readonly Dictionary _charsetIds = new(); + /// The four G-sets, for the walks that touch all of them. + private static readonly CharsetMode[] GSets = + [CharsetMode.G0, CharsetMode.G1, CharsetMode.G2, CharsetMode.G3]; + /// /// The set a SINGLE shift has invoked for the next printed character, or null. /// @@ -115,8 +123,8 @@ public InputHandler(Terminal terminal) { CharsetMode.G3, Charsets.ASCII } }; - _currentCharset = CharsetMode.G0; // G0 is active by default - RefreshActiveCharset(); + // And the designations behind them, which ResetCharsets seeds alongside the tables. + ResetCharsets(); } ///