Make edit group more stable in VI mode (#1526)

Author: daxian-dbw

PSReadLine/History.cs

@@ -85,6 +85,7 @@ public class HistoryItem
             internal bool _sensitive;
             internal List<EditItem> _edits;
             internal int _undoEditIndex;
+            internal int _editGroupStart;
         }
 
         // History state
@@ -119,6 +120,7 @@ private void ClearSavedCurrentLine()
             _savedCurrentLine.CommandLine = null;
             _savedCurrentLine._edits = null;
             _savedCurrentLine._undoEditIndex = 0;
+            _savedCurrentLine._editGroupStart = -1;
         }
 
         private AddToHistoryOption GetAddToHistoryOption(string line)
@@ -194,6 +196,7 @@ private string MaybeAddToHistory(
                     CommandLine = result,
                     _edits = edits,
                     _undoEditIndex = undoEditIndex,
+                    _editGroupStart = -1,
                     _saved = fromHistoryFile,
                     FromOtherSession = fromDifferentSession,
                     FromHistoryFile = fromInitialRead,
@@ -477,12 +480,14 @@ private void UpdateFromHistory(HistoryMoveCursor moveCursor)
                 line = _savedCurrentLine.CommandLine;
                 _edits = new List<EditItem>(_savedCurrentLine._edits);
                 _undoEditIndex = _savedCurrentLine._undoEditIndex;
+                _editGroupStart = _savedCurrentLine._editGroupStart;
             }
             else
             {
                 line = _history[_currentHistoryIndex].CommandLine;
                 _edits = new List<EditItem>(_history[_currentHistoryIndex]._edits);
                 _undoEditIndex = _history[_currentHistoryIndex]._undoEditIndex;
+                _editGroupStart = _history[_currentHistoryIndex]._editGroupStart;
             }
             _buffer.Clear();
             _buffer.Append(line);
@@ -519,6 +524,7 @@ private void SaveCurrentLine()
                 _savedCurrentLine.CommandLine = _buffer.ToString();
                 _savedCurrentLine._edits = _edits;
                 _savedCurrentLine._undoEditIndex = _undoEditIndex;
+                _savedCurrentLine._editGroupStart = _editGroupStart;
             }
         }
 

PSReadLine/UndoRedo.cs

@@ -18,10 +18,10 @@ private void RemoveEditsAfterUndo()
             if (removeCount > 0)
             {
                 _edits.RemoveRange(_undoEditIndex, removeCount);
-                if (_editGroupStart >= 0)
+                if (_edits.Count < _editGroupStart)
                 {
-                    // Adjust the edit group start if we are started a group.
-                    _editGroupStart -= removeCount;
+                    // Reset the group start index if any edits before setting the start mark were undone.
+                    _editGroupStart = -1;
                 }
             }
         }
@@ -54,10 +54,27 @@ private void StartEditGroup()
 
         private void EndEditGroup(Action<ConsoleKeyInfo?, object> instigator = null, object instigatorArg = null)
         {
+            // Remove the undone edits when closing an edit group, so the generated group
+            // doesn't contain those edits that were already undone.
+            RemoveEditsAfterUndo();
+
+            // If any edits before the start mark were done, the start mark will be reset
+            // and no need to generate the edit group.
+            if (_editGroupStart < 0)
+            {
+                return;
+            }
+
             var groupEditCount = _edits.Count - _editGroupStart;
-            var groupedEditItems = _edits.GetRange(_editGroupStart, groupEditCount);
-            _edits.RemoveRange(_editGroupStart, groupEditCount);
-            SaveEditItem(GroupedEdit.Create(groupedEditItems, instigator, instigatorArg));
+
+            // It's possible that just enough edits were undone and now 'groupEditCount' is 0.
+            // We don't generate the edit group in that case.
+            if (groupEditCount > 0)
+            {
+                var groupedEditItems = _edits.GetRange(_editGroupStart, groupEditCount);
+                _edits.RemoveRange(_editGroupStart, groupEditCount);
+                SaveEditItem(GroupedEdit.Create(groupedEditItems, instigator, instigatorArg));
+            }
             _editGroupStart = -1;
         }
 

test/UndoRedoTest.cs

@@ -1,8 +1,60 @@
-namespace Test
+using Xunit;
+
+namespace Test
 {
-    // Disgusting language hack to make it easier to read a sequence of keys.
-    
     public partial class ReadLine
     {
+        [SkippableFact]
+        public void UndoneEditsShouldBeRemovedBeforeEndingEditGroup()
+        {
+            TestSetup(KeyMode.Vi);
+
+            Test("yuiogh", Keys(
+                "yuiogh", _.Escape, CheckThat(() => AssertLineIs("yuiogh")),
+                _.C, CheckThat(() => AssertLineIs("yuiog")),
+                "897", CheckThat(() => AssertLineIs("yuiog897")),
+                _.Ctrl_z, CheckThat(() => AssertLineIs("yuiog89")),
+                _.Escape, _.u, CheckThat(() => AssertLineIs("yuiogh"))
+            ));
+        }
+
+        [SkippableFact]
+        public void ProperlyUpdateEditGroupStartMarkWhenRemovingUndoneEdits()
+        {
+            TestSetup(KeyMode.Vi);
+
+            Test("yuiogh", Keys(
+                "yuiogh", _.Escape, CheckThat(() => AssertLineIs("yuiogh")),
+                _.LeftArrow, _.C, CheckThat(() => AssertLineIs("yuio")),
+                "789", CheckThat(() => AssertLineIs("yuio789")),
+                _.Ctrl_z, CheckThat(() => AssertLineIs("yuio78")),
+                '1', _.Escape, _.u, CheckThat(() => AssertLineIs("yuiogh"))
+            ));
+        }
+
+        [SkippableFact]
+        public void ProperlySetEditGroupStartMarkWhenLoopInHistory()
+        {
+            TestSetup(KeyMode.Vi);
+
+            Test("ok", Keys("ok"));
+
+            // The _editGroupStart mark for an history entry should always be -1, meaning no start mark.
+            Test("ok", Keys(
+                "yuiogh", _.Escape, CheckThat(() => AssertLineIs("yuiogh")),
+                _.C, CheckThat(() => AssertLineIs("yuiog")),
+                _.UpArrow, CheckThat(() => AssertLineIs("ok")),
+                _.Escape));
+
+            // The _editGroupStart mark should be restored when loop back to the current command line.
+            Test("yuiogh", Keys(
+                "yuiogh", _.Escape, CheckThat(() => AssertLineIs("yuiogh")),
+                _.C, CheckThat(() => AssertLineIs("yuiog")),
+                _.UpArrow, CheckThat(() => AssertLineIs("ok")),
+                _.DownArrow, CheckThat(() => AssertLineIs("yuiog")),
+                "123", CheckThat(() => AssertLineIs("yuiog123")),
+                _.Escape, _.u, CheckThat(() => AssertLineIs("yuiogh"))
+            ));
+        }
     }
 }