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

src/XTerm.NET.Tests/VtTestBehaviourTests.cs

@@ -20,6 +20,8 @@ namespace XTerm.Tests;
 public class VtTestBehaviourTests
 {
     private const string Esc = "\u001b";
+    private const string ShiftOut = "\u000e";   // SO -- invoke 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 });
@@ -289,12 +291,15 @@ public void Special_graphics_maps_the_control_pictures_too()
     [Fact]
     public void The_96_character_set_designators_are_a_separate_space()
     {
+        // Both halves designate G1 and invoke it with SO, so they differ by ONE thing: the
+        // space the designator came from. Designating G0 for one and G1 for the other left
+        // the comparison carrying a second difference it was not trying to test.
         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));
     }
 
@@ -389,4 +394,59 @@ public void A_national_set_answers_to_both_of_its_designators()
             Assert.Equal("£à°", terminal.GetLine(0));
         }
     }
+
+    /// <summary>
+    /// DECRC puts back the DESIGNATION, so a later DECNRCM re-resolves what was restored.
+    /// </summary>
+    /// <remarks>
+    /// <para>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>
+    ///
+    /// <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));
+
+        // DECNRCM moving BETWEEN the save and the restore, both directions. This is the half the
+        // doc comment claims and the three cases above do not reach: they move the mode after the
+        // DECRC, so replaying a saved TABLE would satisfy them. Here the table saved and the table
+        // wanted are different, and only re-resolving the designation produces the second one.
+        var modeOnAfterSave = Sized(30, 3);
+        modeOnAfterSave.Write($"{Esc})R{Esc}7");              // French designated with NRC OFF
+        modeOnAfterSave.Write($"{Esc}[?42h{Esc}8");           // NRC on, then restore
+        modeOnAfterSave.Write($"{ShiftOut}@{ShiftIn}");
+        Assert.Equal("à", modeOnAfterSave.GetLine(0));        // French, not the ASCII it saved
+
+        var modeOffAfterSave = Sized(30, 3);
+        modeOffAfterSave.Write($"{Esc}[?42h{Esc})R{Esc}7");   // French designated with NRC ON
+        modeOffAfterSave.Write($"{Esc}[?42l{Esc}8");          // NRC off, then restore
+        modeOffAfterSave.Write($"{ShiftOut}@{ShiftIn}");
+        Assert.Equal("@", modeOffAfterSave.GetLine(0));       // ASCII, not the French it saved
+    }
 }

src/XTerm.NET/Buffer/TerminalBuffer.cs

@@ -136,10 +136,23 @@ 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
-        /// DECRC to leave the designations alone.
+        /// 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.
+        ///
+        /// Internal, unlike the rest of this class: it is DECSC scratch that a consumer cannot do
+        /// anything with, and it was public only by having been written that way. Narrowing it now
+        /// costs nothing -- the property arrived in #93, after v1.2, so no released package has
+        /// ever carried it -- and it stops the shape of an internal detail being an API question
+        /// every time DECSC learns something new.
         /// </summary>
-        public Dictionary<char, string>?[]? Designations { get; set; }
+        internal (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

@@ -709,17 +709,8 @@ private bool TryPairRegionalIndicator(string data, int cellX)
         return true;
     }
 
-    private void SetCharset(CharsetMode mode, string charsetId)
-    {
-        // The ID is kept, not just the table it resolves to. A national set means one thing
-        // 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;
-        _ninetySixSets.Remove(mode);
-        _charsets[mode] = Charsets.GetCharset(charsetId, _terminal.NationalReplacementCharsets);
-        RefreshActiveCharset();
-    }
+    private void SetCharset(CharsetMode mode, string charsetId) =>
+        Designate(mode, charsetId, ninetySix: false);
 
     /// <summary>
     /// Designates a 96-character set: <c>ESC - Ps</c> (G1), <c>ESC . Ps</c> (G2),
@@ -731,14 +722,40 @@ private void SetCharset(CharsetMode mode, string charsetId)
     /// 96-set space and the United Kingdom set in the 94-set one. Anything else is left as
     /// ASCII rather than guessed at.
     /// </remarks>
-    private void SetNinetySixCharset(CharsetMode mode, string charsetId)
+    private void SetNinetySixCharset(CharsetMode mode, string charsetId) =>
+        Designate(mode, charsetId, ninetySix: true);
+
+    /// <summary>Records a designation and resolves it, for both spaces.</summary>
+    /// <remarks>
+    /// The ID is kept, not just the table it resolves to. A national set means one thing 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.
+    /// </remarks>
+    private void Designate(CharsetMode mode, string charsetId, bool ninetySix)
     {
         _charsetIds[mode] = charsetId;
-        _ninetySixSets.Add(mode);
-        _charsets[mode] = Charsets.ASCII;
+        if (ninetySix)
+            _ninetySixSets.Add(mode);
+        else
+            _ninetySixSets.Remove(mode);
+
+        _charsets[mode] = Resolve(charsetId, ninetySix);
         RefreshActiveCharset();
     }
 
+    /// <summary>What a designation means RIGHT NOW, given the mode state.</summary>
+    /// <remarks>
+    /// One answer for the three callers that need it -- designation, DECRC and DECNRCM -- because
+    /// the question is the same one and they got different answers while each resolved separately.
+    /// The space is half the question: 'A' is ISO Latin-1 after ESC - and the United Kingdom set
+    /// after ESC (, so an identifier without the space it came from cannot be resolved at all.
+    /// </remarks>
+    private Dictionary<char, string>? Resolve(string charsetId, bool ninetySix) =>
+        ninetySix
+            ? Charsets.ASCII
+            : Charsets.GetCharset(charsetId, _terminal.NationalReplacementCharsets);
+
     /// <summary>Re-resolves every designation, for when DECNRCM changes under them.</summary>
     /// <remarks>
     /// Through the space each was designated in. 'A' is ISO Latin-1 after ESC - and the
@@ -749,16 +766,40 @@ private void SetNinetySixCharset(CharsetMode mode, string charsetId)
     /// </remarks>
     internal void RefreshDesignatedCharsets()
     {
-        foreach (var mode in _charsetIds.Keys.ToList())
-        {
-            _charsets[mode] = _ninetySixSets.Contains(mode)
-                ? Charsets.ASCII
-                : Charsets.GetCharset(_charsetIds[mode], _terminal.NationalReplacementCharsets);
-        }
+        foreach (var mode in GSets)
+            _charsets[mode] = Resolve(_charsetIds[mode], _ninetySixSets.Contains(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], _ninetySixSets.Contains(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, arbitrarily far from the DECRC that caused it.
+    ///
+    /// Resolving at restore time rather than replaying the saved table is also the right answer
+    /// when DECNRCM moved BETWEEN the save and the restore.
+    /// </remarks>
+    internal void RestoreDesignation(CharsetMode mode, (string Id, bool NinetySix) designation)
+    {
+        _charsetIds[mode] = designation.Id;
+        if (designation.NinetySix)
+            _ninetySixSets.Add(mode);
+        else
+            _ninetySixSets.Remove(mode);
+
+        _charsets[mode] = Resolve(designation.Id, designation.NinetySix);
+    }
+
     /// <summary>
     /// Shift Out - Select G1 character set (SO, 0x0E).
     /// </summary>
@@ -812,16 +853,22 @@ public void InvokeSingleShift(CharsetMode mode)
     /// </summary>
     public void ResetCharsets()
     {
-        _charsetIds.Clear();
         _ninetySixSets.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 is what keeps every walk over the four total.
+            _charsetIds[mode] = UsAsciiId;
+            _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

@@ -41,12 +41,20 @@ public partial class InputHandler
 
     /// <summary>What each G-set was DESIGNATED as, by its escape identifier.</summary>
     /// <remarks>
-    /// Kept alongside the resolved tables because a designation outlives its resolution: a
+    /// <para>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 program that designated it does not designate again when the mode changes.</para>
+    ///
+    /// <para>All four slots are always present, seeded to US ASCII, so a designation is a value
+    /// rather than a value-or-absent. DECSC, DECRC and DECNRCM each walk every slot, and "never
+    /// designated" and "designated B" mean the same thing to all three.</para>
     /// </remarks>
     private readonly Dictionary<CharsetMode, string> _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>Which G-sets were designated as 96-character sets.</summary>
     /// <remarks>
     /// The identifier alone does not say: 'A' is the UK set in one space and ISO Latin-1 in the
@@ -117,8 +125,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>