Merge pull request #173 from tig/tznind-ansi-parser

Code review comments and cleanup

Author: tznind

Terminal.Gui/ConsoleDrivers/AnsiEscapeSequenceRequest.cs

@@ -16,6 +16,7 @@ public class AnsiEscapeSequenceRequest
     /// </summary>
     public required string Request { get; init; }
 
+    // BUGBUG: Nullable issue
     /// <summary>
     ///     Invoked when the console responds with an ANSI response code that matches the
     ///     <see cref="Terminator"/>

Terminal.Gui/ConsoleDrivers/AnsiResponseParser/AnsiRequestScheduler.cs

@@ -14,8 +14,8 @@ public class AnsiRequestScheduler
     private readonly IAnsiResponseParser _parser;
 
     /// <summary>
-    /// Function for returning the current time. Use in unit tests to
-    /// ensure repeatable tests.
+    ///     Function for returning the current time. Use in unit tests to
+    ///     ensure repeatable tests.
     /// </summary>
     internal Func<DateTime> Now { get; set; }
 
@@ -54,6 +54,11 @@ public class AnsiRequestScheduler
 
     private readonly DateTime _lastRun;
 
+    /// <summary>
+    ///     Creates a new instance.
+    /// </summary>
+    /// <param name="parser"></param>
+    /// <param name="now"></param>
     public AnsiRequestScheduler (IAnsiResponseParser parser, Func<DateTime>? now = null)
     {
         _parser = parser;
@@ -67,11 +72,9 @@ public AnsiRequestScheduler (IAnsiResponseParser parser, Func<DateTime>? now = n
     /// </summary>
     /// <param name="request"></param>
     /// <returns><see langword="true"/> if request was sent immediately. <see langword="false"/> if it was queued.</returns>
-    public bool SendOrSchedule (AnsiEscapeSequenceRequest request)
-    {
-        return SendOrSchedule (request, true);
-    }
-    private bool SendOrSchedule (AnsiEscapeSequenceRequest request,bool addToQueue)
+    public bool SendOrSchedule (AnsiEscapeSequenceRequest request) { return SendOrSchedule (request, true); }
+
+    private bool SendOrSchedule (AnsiEscapeSequenceRequest request, bool addToQueue)
     {
         if (CanSend (request, out ReasonCannotSend reason))
         {
@@ -105,13 +108,13 @@ private bool SendOrSchedule (AnsiEscapeSequenceRequest request,bool addToQueue)
 
     private void EvictStaleRequests ()
     {
-        foreach (var stale in _lastSend.Where (v => IsStale (v.Value)).Select (k => k.Key))
+        foreach (string stale in _lastSend.Where (v => IsStale (v.Value)).Select (k => k.Key))
         {
             EvictStaleRequests (stale);
         }
     }
 
-    private bool IsStale (DateTime dt) => Now () - dt > _staleTimeout;
+    private bool IsStale (DateTime dt) { return Now () - dt > _staleTimeout; }
 
     /// <summary>
     ///     Looks to see if the last time we sent <paramref name="withTerminator"/>
@@ -155,14 +158,15 @@ public bool RunSchedule (bool force = false)
         }
 
         // Get oldest request
-        Tuple<AnsiEscapeSequenceRequest, DateTime>? opportunity = _queuedRequests.MinBy (r=>r.Item2);
+        Tuple<AnsiEscapeSequenceRequest, DateTime>? opportunity = _queuedRequests.MinBy (r => r.Item2);
 
         if (opportunity != null)
         {
             // Give it another go
             if (SendOrSchedule (opportunity.Item1, false))
             {
                 _queuedRequests.Remove (opportunity);
+
                 return true;
             }
         }
@@ -172,7 +176,6 @@ public bool RunSchedule (bool force = false)
         return false;
     }
 
-
     private void Send (AnsiEscapeSequenceRequest r)
     {
         _lastSend.AddOrUpdate (r.Terminator, _ => Now (), (_, _) => Now ());
@@ -210,23 +213,4 @@ private bool ShouldThrottle (AnsiEscapeSequenceRequest r)
 
         return false;
     }
-}
-
-internal enum ReasonCannotSend
-{
-    /// <summary>
-    ///     No reason given.
-    /// </summary>
-    None = 0,
-
-    /// <summary>
-    ///     The parser is already waiting for a request to complete with the given terminator.
-    /// </summary>
-    OutstandingRequest,
-
-    /// <summary>
-    ///     There have been too many requests sent recently, new requests will be put into
-    ///     queue to prevent console becoming unresponsive.
-    /// </summary>
-    TooManyRequests
-}
+}