Fix AcceptAndGetNext skipping history problem

PSReadLine/History.cs

@@ -90,8 +90,8 @@ public class HistoryItem
         }
 
         // History state
-        private HistoryQueue<HistoryItem> _history;
-        private HistoryQueue<string> _recentHistory;
+        private RingBuffer<HistoryItem> _history;
+        private RingBuffer<string> _recentHistory;
         private HistoryItem _previousHistoryItem;
         private Dictionary<string, int> _hashedHistory;
         private int _currentHistoryIndex;
@@ -827,6 +827,10 @@ public static HistoryItem[] GetHistoryItems()
 
         enum HistoryMoveCursor { ToEnd, ToBeginning, DontMove }
 
+        /// <summary>
+        /// Set current line from the history item `_currentHistoryIndex` pointing to.
+        /// </summary>
+        /// <param name="moveCursor">How to move cursor after line being updated</param>
         private void UpdateFromHistory(HistoryMoveCursor moveCursor)
         {
             string line;

PSReadLine/Options.cs

@@ -47,7 +47,7 @@ private void SetOptionsInternal(SetPSReadLineOption options)
                 Options.MaximumHistoryCount = options.MaximumHistoryCount;
                 if (_history != null)
                 {
-                    var newHistory = new HistoryQueue<HistoryItem>(Options.MaximumHistoryCount);
+                    var newHistory = new RingBuffer<HistoryItem>(Options.MaximumHistoryCount);
                     while (_history.Count > Options.MaximumHistoryCount)
                     {
                         _history.Dequeue();

PSReadLine/ReadLine.cs

@@ -79,7 +79,7 @@ public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods
         private static readonly Stopwatch _readkeyStopwatch = new Stopwatch();
 
         // Save a fixed # of keys so we can reconstruct a repro after a crash
-        private static readonly HistoryQueue<PSKeyInfo> _lastNKeys = new HistoryQueue<PSKeyInfo>(200);
+        private static readonly RingBuffer<PSKeyInfo> _lastNKeys = new RingBuffer<PSKeyInfo>(200);
 
         // Tokens etc.
         private Token[] _tokens;
@@ -818,7 +818,11 @@ private void Initialize(Runspace runspace, EngineIntrinsics engineIntrinsics)
 
             if (_getNextHistoryIndex > 0)
             {
-                _currentHistoryIndex = _getNextHistoryIndex;
+                // This branch is specifically reached after AcceptAndGetNext and the command execution finished,
+                // a new history item will be enqueued into `_history`.
+                // If `_history` ring buffer is full, the original history item `_currentHistoryIndex` pointed to
+                // will be pushed forward after new item is enqueued, so we need to decrement the index by 1 here
+                _currentHistoryIndex = _getNextHistoryIndex - (_history.Count == _history.Capacity ? 1 : 0);
                 UpdateFromHistory(HistoryMoveCursor.ToEnd);
                 _getNextHistoryIndex = 0;
                 if (_searchHistoryCommandCount > 0)
@@ -891,8 +895,8 @@ private void DelayedOneTimeInitialize()
 
             _historyFileMutex = new Mutex(false, GetHistorySaveFileMutexName());
 
-            _history = new HistoryQueue<HistoryItem>(Options.MaximumHistoryCount);
-            _recentHistory = new HistoryQueue<string>(capacity: 5);
+            _history = new RingBuffer<HistoryItem>(Options.MaximumHistoryCount);
+            _recentHistory = new RingBuffer<string>(capacity: 5);
             _currentHistoryIndex = 0;
 
             bool readHistoryFile = true;

PSReadLine/Render.cs

@@ -171,6 +171,10 @@ struct LineInfoForRendering
 
         private ConsoleColor _initialForeground;
         private ConsoleColor _initialBackground;
+
+        /// <summary>
+        /// Current cursor position
+        /// </summary>
         private int _current;
         private int _emphasisStart;
         private int _emphasisLength;

PSReadLine/HistoryQueue.cs → PSReadLine/RingBuffer.cs

@@ -3,7 +3,6 @@
 --********************************************************************/
 
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 
@@ -12,63 +11,41 @@ namespace Microsoft.PowerShell
     [ExcludeFromCodeCoverage]
     internal sealed class QueueDebugView<T>
     {
-        private readonly HistoryQueue<T> _queue;
+        private readonly RingBuffer<T> _queue;
 
         [DebuggerBrowsable(DebuggerBrowsableState.RootHidden)]
         public T[] Items => this._queue.ToArray();
 
-        public QueueDebugView(HistoryQueue<T> queue)
+        public QueueDebugView(RingBuffer<T> queue)
         {
             this._queue = queue ?? throw new ArgumentNullException(nameof(queue));
         }
     }
 
     [DebuggerDisplay("Count = {" + nameof(Count) + "}")]
     [DebuggerTypeProxy(typeof(QueueDebugView<>))]
-    internal class HistoryQueue<T>
+    internal class RingBuffer<T>
     {
         private readonly T[] _array;
         private int _head;
         private int _tail;
 
-        public HistoryQueue(int capacity)
+        public int Capacity => _array.Length;
+        public int Count { get; private set; }
+
+        public RingBuffer(int capacity)
         {
-            Debug.Assert(capacity > 0);
+            ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(capacity, 0);
             _array = new T[capacity];
             _head = _tail = Count = 0;
         }
 
         public void Clear()
         {
-            for (int i = 0; i < Count; i++)
-            {
-                this[i] = default(T);
-            }
+            Array.Clear(_array);
             _head = _tail = Count = 0;
         }
 
-        public bool Contains(T item)
-        {
-            return IndexOf(item) != -1;
-        }
-
-        public int Count { get; private set; }
-
-        public int IndexOf(T item)
-        {
-            // REVIEW: should we use case insensitive here?
-            var eqComparer = EqualityComparer<T>.Default;
-            for (int i = 0; i < Count; i++)
-            {
-                if (eqComparer.Equals(this[i], item))
-                {
-                    return i;
-                }
-            }
-
-            return -1;
-        }
-
         public void Enqueue(T item)
         {
             if (Count == _array.Length)
@@ -82,10 +59,10 @@ public void Enqueue(T item)
 
         public T Dequeue()
         {
-            Debug.Assert(Count > 0);
+            if (Count == 0) throw new InvalidOperationException("RingBuffer is empty");
 
             T obj = _array[_head];
-            _array[_head] = default(T);
+            _array[_head] = default;
             _head = (_head + 1) % _array.Length;
             Count -= 1;
             return obj;
@@ -113,14 +90,10 @@ public T[] ToArray()
         public T this[int index]
         {
             get
-            { 
-                Debug.Assert(index >= 0 && index < Count);
-                return _array[(_head + index) % _array.Length];
-            }
-            set
             {
-                Debug.Assert(index >= 0 && index < Count);
-                _array[(_head + index) % _array.Length] = value;
+                ArgumentOutOfRangeException.ThrowIfNegative(index);
+                ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual(index, Count);
+                return _array[(_head + index) % _array.Length];
             }
         }
     }

test/BasicEditingTest.cs

@@ -312,6 +312,12 @@ public void AcceptAndGetNext()
             SetHistory("echo 1", "echo 2");
             Test("echo 1", Keys("e", _.UpArrow, _.UpArrow, _.Ctrl_o, InputAcceptedNow));
             Test("eee", Keys(_.DownArrow, _.DownArrow, "ee", _.Enter));
+
+            // if _history ring buffer is full, should work properly as well
+            PSConsoleReadLine.SetOptions(new() { MaximumHistoryCount = 3 });
+            SetHistory("echo 1", "echo 2", "echo 3");
+            Test("echo 1", Keys("e", _.UpArrow, _.UpArrow, _.UpArrow, _.Ctrl_o, InputAcceptedNow));
+            Test("echo 2", Keys(_.Enter));
         }
 
         [SkippableFact]