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

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 });
 
@@ -290,11 +295,90 @@ 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);
-        latin1.Write($"{Esc}-A#@[");
+        latin1.Write($"{Esc}-A{ShiftOut}#@[{ShiftIn}");
         Assert.Equal("#@[", latin1.GetLine(0));
     }
+
+    /// <summary>
+    /// A 96-set designation survives DECNRCM moving under it.
+    /// </summary>
+    /// <remarks>
+    /// 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.
+    ///
+    /// <para>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.</para>
+    /// </remarks>
+    [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));
+    }
+
+    /// <summary>
+    /// DECRC puts back the DESIGNATION, so a later DECNRCM re-resolves what was restored.
+    /// </summary>
+    /// <remarks>
+    /// 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.
+    ///
+    /// <para>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.</para>
+    /// </remarks>
+    [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));
+    }
 }

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.
         /// </summary>
-        public Dictionary<char, string>?[]? Designations { get; set; }
+        public (string Id, bool NinetySix)[]? Designations { get; set; }
 
         public SavedCursor()
         {

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<char, string>?[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;

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);
     }
 
     /// <summary>
@@ -732,20 +730,57 @@ private void SetCharset(CharsetMode mode, string charsetId)
     /// </remarks>
     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();
     }
 
+    /// <summary>
+    /// 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.
+    /// </summary>
+    private Dictionary<char, string>? 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);
+
     /// <summary>Re-resolves every designation, for when DECNRCM changes under them.</summary>
     internal void RefreshDesignatedCharsets()
     {
-        foreach (var mode in _charsetIds.Keys.ToList())
-            _charsets[mode] = Charsets.GetCharset(_charsetIds[mode], _terminal.NationalReplacementCharsets);
+        foreach (var mode in GSets)
+            _charsets[mode] = Resolve(_charsetIds[mode]);
 
         RefreshActiveCharset();
     }
 
+    /// <summary>The designation a G-set is holding, for DECSC to save.</summary>
+    internal (string Id, bool NinetySix) DesignationOf(CharsetMode mode) => _charsetIds[mode];
+
+    /// <summary>
+    /// Puts a saved designation back, for DECRC, resolving it against the mode state as it is NOW.
+    /// </summary>
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    internal void RestoreDesignation(CharsetMode mode, (string Id, bool NinetySix) designation)
+    {
+        _charsetIds[mode] = designation;
+        _charsets[mode] = Resolve(designation);
+    }
+
     /// <summary>
     /// Shift Out - Select G1 character set (SO, 0x0E).
     /// </summary>
@@ -799,15 +834,21 @@ public void InvokeSingleShift(CharsetMode mode)
     /// </summary>
     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();
     }
 
+    /// <summary>The identifier US ASCII is designated by, which is where every G-set starts.</summary>
+    private const string UsAsciiId = "B";
+
     /// <summary>
     /// Prints the payload of an OSC 66 whose metadata could not be parsed, as ordinary text.
     /// </summary>

src/XTerm.NET/InputHandler.cs

@@ -44,8 +44,21 @@ 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.
+    ///
+    /// <para>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 <c>ESC (</c> and ISO Latin-1
+    /// after <c>ESC -</c>. Re-resolving without it turns a Latin-1 designation into UK the first
+    /// time DECNRCM moves.</para>
+    ///
+    /// <para>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.</para>
     /// </remarks>
-    private readonly Dictionary<CharsetMode, string> _charsetIds = new();
+    private readonly Dictionary<CharsetMode, (string Id, bool NinetySix)> _charsetIds = new();
+
+    /// <summary>The four G-sets, for the walks that touch all of them.</summary>
+    private static readonly CharsetMode[] GSets =
+        [CharsetMode.G0, CharsetMode.G1, CharsetMode.G2, CharsetMode.G3];
 
     /// <summary>
     /// The set a SINGLE shift has invoked for the next printed character, or null.
@@ -110,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();
     }
 
     /// <summary>