Fix a few VI edit mode issues (#1262)

Author: daxian-dbw

PSReadLine/BasicEditing.cs

@@ -164,8 +164,6 @@ public static void BackwardDeleteChar(ConsoleKeyInfo? key = null, object arg = n
 
         private void DeleteCharImpl(int qty, bool orExit)
         {
-            qty = Math.Min(qty, _singleton._buffer.Length + 1 + ViEndOfLineFactor - _singleton._current);
-
             if (_visualSelectionCommandCount > 0)
             {
                 GetRegion(out var start, out var length);
@@ -177,6 +175,8 @@ private void DeleteCharImpl(int qty, bool orExit)
             {
                 if (_current < _buffer.Length)
                 {
+                    qty = Math.Min(qty, _singleton._buffer.Length - _singleton._current);
+
                     SaveEditItem(EditItemDelete.Create(_buffer.ToString(_current, qty), _current, DeleteChar, qty));
                     SaveToClipboard(_current, qty);
                     _buffer.Remove(_current, qty);

PSReadLine/Movement.cs

@@ -114,9 +114,11 @@ private static void ViOffsetCursorPosition(int count)
                     _singleton.MoveCursor(newCurrent);
                 }
             }
-            else
+            else if (_singleton._current < _singleton._buffer.Length)
             {
-                var end = GetEndOfLogicalLinePos(_singleton._current);
+                // when in the VI command mode, 'end' is the position of the last character;
+                // when in the VI insert mode, 'end' is 1 char beyond the last character.
+                var end = GetEndOfLogicalLinePos(_singleton._current) + 1 + ViEndOfLineFactor;
                 var newCurrent = Math.Min(end, _singleton._current + count);
                 if (_singleton._current != newCurrent)
                 {

PSReadLine/Position.cs

@@ -80,7 +80,7 @@ private static int GetEndOfLogicalLinePos(int current)
         {
             var newCurrent = current;
 
-            for (var position = newCurrent; position < _singleton._buffer.Length; position++)
+            for (var position = current; position < _singleton._buffer.Length; position++)
             {
                 if (_singleton._buffer[position] == '\n')
                 {

PSReadLine/ReadLine.vi.cs

@@ -589,14 +589,8 @@ public static void ViInsertWithDelete(ConsoleKeyInfo? key = null, object arg = n
         {
             _singleton._groupUndoHelper.StartGroup(ViInsertWithDelete, arg);
 
-            var isEOL = _singleton._current == _singleton._buffer.Length + ViEndOfLineFactor;
-
-            DeleteChar(key, arg);
-            if(isEOL)
-            {
-                _singleton._current++;
-            }
             ViInsertMode(key, arg);
+            DeleteChar(key, arg);
         }
 
         /// <summary>

PSReadLine/Replace.vi.cs

@@ -213,8 +213,8 @@ private static void ViReplaceEndOfGlob(ConsoleKeyInfo? key, object arg)
         private static void ReplaceChar(ConsoleKeyInfo? key, object arg)
         {
             _singleton._groupUndoHelper.StartGroup(ReplaceChar, arg);
-            DeleteChar(key, arg);
             ViInsertMode(key, arg);
+            DeleteChar(key, arg);
         }
 
         /// <summary>

test/BasicEditingTest.VI.cs

@@ -255,6 +255,13 @@ public void ViDelete()
         {
             TestSetup(KeyMode.Vi);
 
+            Test("bc", Keys(
+                "a", _.Escape,
+                CheckThat(() => AssertLineIs("a")),
+                CheckThat(() => AssertCursorLeftIs(0)),
+                "s", CheckThat(() => AssertLineIs("")),
+                "bc"));
+
             Test("", Keys(
                 "0123456789", _.Escape, CheckThat(() => AssertCursorLeftIs(9)),
                 "x", CheckThat(() => AssertLineIs("012345678")), CheckThat(() => AssertCursorLeftIs(8)),
@@ -881,5 +888,18 @@ public void ViComplete()
                 _.Escape, "Csness"
                 ));
         }
+
+        [SkippableFact]
+        public void ViInsertModeMoveCursor()
+        {
+            TestSetup(KeyMode.Vi);
+
+            Test("abc", Keys(
+                "ab", CheckThat(() => AssertCursorLeftIs(2)),
+                _.LeftArrow, CheckThat(() => AssertCursorLeftIs(1)),
+                _.RightArrow, CheckThat(() => AssertCursorLeftIs(2)),
+                _.RightArrow, // 'RightArrow' again does nothing, but doesn't crash
+                "c"));
+        }
     }
 }

test/BasicEditingTest.cs

@@ -158,6 +158,21 @@ public void DeleteChar()
             Test("ac", Keys("abc", _.Home, _.RightArrow, _.Delete));
         }
 
+        [SkippableFact]
+        public void DeleteCharAfterDigitArgument()
+        {
+            TestSetup(KeyMode.Cmd);
+
+            Test("abc", Keys(
+                "ab", _.LeftArrow, _.Alt_2, _.Delete,
+                CheckThat(() => AssertLineIs("a")),
+                CheckThat(() => AssertCursorLeftIs(1)),
+                _.Delete, // 'Delete' again does nothing, but doesn't crash
+                CheckThat(() => AssertLineIs("a")),
+                CheckThat(() => AssertCursorLeftIs(1)),
+                "bc"));
+        }
+
         [SkippableFact]
         public void DeleteCharOrExit()
         {