Simplify and correctness ANSI escape sequences.

Terminal.Gui/ConsoleDrivers/EscSeqUtils/EscSeqReqStatus.cs

@@ -18,10 +18,10 @@ public class EscSeqReqStatus
     }
 
     /// <summary>Gets the number of unfinished requests.</summary>
-    public int NumOutstanding { get; set; }
+    public int NumOutstanding { get; internal set; }
 
     /// <summary>Gets the number of requests.</summary>
-    public int NumRequests { get; }
+    public int NumRequests { get; internal set; }
 
     /// <summary>Gets the Escape Sequence Terminator (e.g. ESC[8t ... t is the terminator).</summary>
     public string Terminator { get; }

Terminal.Gui/ConsoleDrivers/EscSeqUtils/EscSeqRequests.cs

@@ -1,7 +1,5 @@
 #nullable enable
 
-using System.Collections.Concurrent;
-
 namespace Terminal.Gui;
 
 /// <summary>
@@ -12,27 +10,32 @@ namespace Terminal.Gui;
 public static class EscSeqRequests
 {
     /// <summary>Gets the <see cref="EscSeqReqStatus"/> list.</summary>
-    public static List<EscSeqReqStatus> Statuses { get; } = new ();
+    public static List<EscSeqReqStatus> Statuses { get; } = [];
 
     /// <summary>
     ///     Adds a new request for the ANSI Escape Sequence defined by <paramref name="terminator"/>. Adds a
     ///     <see cref="EscSeqReqStatus"/> instance to <see cref="Statuses"/> list.
     /// </summary>
     /// <param name="terminator">The terminator.</param>
-    /// <param name="numReq">The number of requests.</param>
-    public static void Add (string terminator, int numReq = 1)
+    /// <param name="numRequests">The number of requests.</param>
+    public static void Add (string terminator, int numRequests = 1)
     {
+        ArgumentException.ThrowIfNullOrEmpty (terminator);
+
+        int numReq = Math.Max (numRequests, 1);
+
         lock (Statuses)
         {
             EscSeqReqStatus? found = Statuses.Find (x => x.Terminator == terminator);
 
             if (found is null)
             {
-                Statuses.Add (new EscSeqReqStatus (terminator, numReq));
+                Statuses.Add (new (terminator, numReq));
             }
-            else if (found.NumOutstanding < found.NumRequests)
+            else
             {
-                found.NumOutstanding = Math.Min (found.NumOutstanding + numReq, found.NumRequests);
+                found.NumRequests += numReq;
+                found.NumOutstanding += numReq;
             }
         }
     }
@@ -60,21 +63,7 @@ public static class EscSeqRequests
         {
             EscSeqReqStatus? found = Statuses.Find (x => x.Terminator == terminator);
 
-            if (found is null)
-            {
-                return false;
-            }
-
-            if (found is { NumOutstanding: > 0 })
-            {
-                return true;
-            }
-
-            // BUGBUG: Why does an API that returns a bool remove the entry from the list?
-            // NetDriver and Unit tests never exercise this line of code. Maybe Curses does?
-            Statuses.Remove (found);
-
-            return false;
+            return found is { };
         }
     }
 
@@ -87,6 +76,8 @@ public static class EscSeqRequests
     /// <param name="terminator">The terminating string.</param>
     public static void Remove (string terminator)
     {
+        ArgumentException.ThrowIfNullOrEmpty (terminator);
+
         lock (Statuses)
         {
             EscSeqReqStatus? found = Statuses.Find (x => x.Terminator == terminator);

Terminal.Gui/ConsoleDrivers/EscSeqUtils/EscSeqUtils.cs

@@ -204,7 +204,7 @@ public static class EscSeqUtils
         out List<MouseFlags> buttonState,
         out Point pos,
         out bool isResponse,
-        Action<MouseFlags, Point> continuousButtonPressedHandler
+        Action<MouseFlags, Point>? continuousButtonPressedHandler
     )
     {
         char [] kChars = GetKeyCharArray (cki);

UnitTests/Input/EscSeqRequestsTests.cs

@@ -5,7 +5,6 @@ public class EscSeqRequestsTests
     [Fact]
     public void Add_Tests ()
     {
-        EscSeqRequests.Clear ();
         EscSeqRequests.Add ("t");
         Assert.Single (EscSeqRequests.Statuses);
         Assert.Equal ("t", EscSeqRequests.Statuses [^1].Terminator);
@@ -15,8 +14,8 @@ public class EscSeqRequestsTests
         EscSeqRequests.Add ("t", 2);
         Assert.Single (EscSeqRequests.Statuses);
         Assert.Equal ("t", EscSeqRequests.Statuses [^1].Terminator);
-        Assert.Equal (1, EscSeqRequests.Statuses [^1].NumRequests);
-        Assert.Equal (1, EscSeqRequests.Statuses [^1].NumOutstanding);
+        Assert.Equal (3, EscSeqRequests.Statuses [^1].NumRequests);
+        Assert.Equal (3, EscSeqRequests.Statuses [^1].NumOutstanding);
 
         EscSeqRequests.Clear ();
         EscSeqRequests.Add ("t", 2);
@@ -28,14 +27,15 @@ public class EscSeqRequestsTests
         EscSeqRequests.Add ("t", 3);
         Assert.Single (EscSeqRequests.Statuses);
         Assert.Equal ("t", EscSeqRequests.Statuses [^1].Terminator);
-        Assert.Equal (2, EscSeqRequests.Statuses [^1].NumRequests);
-        Assert.Equal (2, EscSeqRequests.Statuses [^1].NumOutstanding);
+        Assert.Equal (5, EscSeqRequests.Statuses [^1].NumRequests);
+        Assert.Equal (5, EscSeqRequests.Statuses [^1].NumOutstanding);
+
+        EscSeqRequests.Clear ();
     }
 
     [Fact]
     public void Constructor_Defaults ()
     {
-        EscSeqRequests.Clear ();
         Assert.NotNull (EscSeqRequests.Statuses);
         Assert.Empty (EscSeqRequests.Statuses);
     }
@@ -43,7 +43,6 @@ public class EscSeqRequestsTests
     [Fact]
     public void Remove_Tests ()
     {
-        EscSeqRequests.Clear ();
         EscSeqRequests.Add ("t");
         EscSeqRequests.Remove ("t");
         Assert.Empty (EscSeqRequests.Statuses);
@@ -57,16 +56,133 @@ public class EscSeqRequestsTests
 
         EscSeqRequests.Remove ("t");
         Assert.Empty (EscSeqRequests.Statuses);
+
+        EscSeqRequests.Clear ();
     }
 
     [Fact]
-    public void Requested_Tests ()
+    public void HasResponse_Tests ()
     {
-        EscSeqRequests.Clear ();
         Assert.False (EscSeqRequests.HasResponse ("t"));
 
         EscSeqRequests.Add ("t");
         Assert.False (EscSeqRequests.HasResponse ("r"));
         Assert.True (EscSeqRequests.HasResponse ("t"));
+        Assert.Single (EscSeqRequests.Statuses);
+        Assert.Equal ("t", EscSeqRequests.Statuses [^1].Terminator);
+        Assert.Equal (1, EscSeqRequests.Statuses [^1].NumRequests);
+        Assert.Equal (1, EscSeqRequests.Statuses [^1].NumOutstanding);
+
+        EscSeqRequests.Remove ("t");
+        Assert.Empty (EscSeqRequests.Statuses);
+    }
+
+    [Theory]
+    [InlineData (null)]
+    [InlineData ("")]
+    public void Add_Null_Or_Empty_Terminator_Throws (string terminator)
+    {
+        if (terminator is null)
+        {
+            Assert.Throws<ArgumentNullException> (() => EscSeqRequests.Add (terminator));
+        }
+        else
+        {
+            Assert.Throws<ArgumentException> (() => EscSeqRequests.Add (terminator));
+        }
+    }
+
+    [Theory]
+    [InlineData (null)]
+    [InlineData ("")]
+    public void HasResponse_Null_Or_Empty_Terminator_Does_Not_Throws (string terminator)
+    {
+        EscSeqRequests.Add ("t");
+
+        Assert.False (EscSeqRequests.HasResponse (terminator));
+
+        EscSeqRequests.Clear ();
+    }
+
+    [Theory]
+    [InlineData (null)]
+    [InlineData ("")]
+    public void Remove_Null_Or_Empty_Terminator_Throws (string terminator)
+    {
+        EscSeqRequests.Add ("t");
+
+        if (terminator is null)
+        {
+            Assert.Throws<ArgumentNullException> (() => EscSeqRequests.Remove (terminator));
+        }
+        else
+        {
+            Assert.Throws<ArgumentException> (() => EscSeqRequests.Remove (terminator));
+        }
+
+        EscSeqRequests.Clear ();
+    }
+
+    [Fact]
+    public void Requests_Responses_Tests ()
+    {
+        // This is simulated response from a CSI_ReportTerminalSizeInChars
+        ConsoleKeyInfo [] cki =
+        [
+            new ('\u001b', 0, false, false, false),
+            new ('[', 0, false, false, false),
+            new ('8', 0, false, false, false),
+            new (';', 0, false, false, false),
+            new ('1', 0, false, false, false),
+            new ('0', 0, false, false, false),
+            new (';', 0, false, false, false),
+            new ('2', 0, false, false, false),
+            new ('0', 0, false, false, false),
+            new ('t', 0, false, false, false)
+        ];
+        ConsoleKeyInfo newConsoleKeyInfo = default;
+        ConsoleKey key = default;
+        ConsoleModifiers mod = default;
+
+        Assert.Empty (EscSeqRequests.Statuses);
+
+        EscSeqRequests.Add ("t");
+        Assert.Single (EscSeqRequests.Statuses);
+        Assert.Equal ("t", EscSeqRequests.Statuses [^1].Terminator);
+        Assert.Equal (1, EscSeqRequests.Statuses [^1].NumRequests);
+        Assert.Equal (1, EscSeqRequests.Statuses [^1].NumOutstanding);
+
+        EscSeqUtils.DecodeEscSeq (
+                                  ref newConsoleKeyInfo,
+                                  ref key,
+                                  cki,
+                                  ref mod,
+                                  out string c1Control,
+                                  out string code,
+                                  out string [] values,
+                                  out string terminating,
+                                  out bool isKeyMouse,
+                                  out List<MouseFlags> mouseFlags,
+                                  out Point pos,
+                                  out bool isResponse,
+                                  null
+                                 );
+
+        Assert.Empty (EscSeqRequests.Statuses);
+        Assert.Equal (default, newConsoleKeyInfo);
+        Assert.Equal (default, key);
+        Assert.Equal (10, cki.Length);
+        Assert.Equal (default, mod);
+        Assert.Equal ("CSI", c1Control);
+        Assert.Null (code);
+        // ReSharper disable once HeuristicUnreachableCode
+        Assert.Equal (3, values.Length);
+        Assert.Equal ("8", values [0]);
+        Assert.Equal ("t", terminating);
+        Assert.False (isKeyMouse);
+        Assert.Single (mouseFlags);
+        Assert.Equal (default, mouseFlags [^1]);
+        Assert.Equal (Point.Empty, pos);
+        Assert.True (isResponse);
     }
 }