Fix UpArrow when the cursor is at the end of a wrapped line in a multiple-line text (#1028)

PSReadLine/Movement.cs

@@ -76,39 +76,88 @@ namespace Microsoft.PowerShell
             }
         }
 
-        private void MoveToLine(int numericArg)
+        private void MoveToLine(int lineOffset)
         {
+            // Behavior description:
+            //  - If the cursor is at the end of a logical line, then 'UpArrow' (or 'DownArrow') moves the cursor up (or down)
+            //    'lineOffset' numbers of logical lines, and the cursor is always put at the end of the new logical line.
+            //  - If the cursor is NOT at the end of a logical line, then 'UpArrow' (or 'DownArrow') moves the cursor up (or down)
+            //    'lineOffset' numbers of physical lines, and the cursor is always placed at the same column as is now, or at the
+            //    end of line if that physical line is shorter than the targeted column.
+
             const int endOfLine = int.MaxValue;
 
+            Point? point = null;
             _moveToLineCommandCount += 1;
-            var point = ConvertOffsetToPoint(_current);
+
             if (_moveToLineCommandCount == 1)
             {
+                point = ConvertOffsetToPoint(_current);
                 _moveToLineDesiredColumn =
                     (_current == _buffer.Length || _buffer[_current] == '\n')
                         ? endOfLine
-                        : point.X;
+                        : point.Value.X;
             }
 
-            var topLine = _initialY;
-
-            var newY = point.Y + numericArg;
-            point.Y = Math.Max(newY, topLine);
-            if (_moveToLineDesiredColumn != endOfLine)
+            // Nothing needs to be done when:
+            //  - actually not moving the line, or
+            //  - moving the line down when it's at the end of the last line.
+            if (lineOffset == 0 || (lineOffset > 0 && _current == _buffer.Length))
             {
-                point.X = _moveToLineDesiredColumn;
+                return;
             }
 
-            var newCurrent = ConvertLineAndColumnToOffset(point);
-            if (newCurrent != -1)
+            int newCurrent;
+            if (_moveToLineDesiredColumn == endOfLine)
             {
-                if (_moveToLineDesiredColumn == endOfLine)
+                newCurrent = _current;
+
+                if (lineOffset > 0)
                 {
-                    while (newCurrent < _buffer.Length && _buffer[newCurrent] != '\n')
+                    // Moving to the end of a subsequent logical line.
+                    for (int i = 0; i < lineOffset; i++)
                     {
-                        newCurrent += 1;
+                        for (newCurrent++; newCurrent < _buffer.Length && _buffer[newCurrent] != '\n'; newCurrent++) ;
+
+                        if (newCurrent == _buffer.Length)
+                        {
+                            break;
+                        }
                     }
                 }
+                else
+                {
+                    // Moving to the end of a previous logical line.
+                    int lastEndOfLineIndex = _current;
+                    for (int i = 0; i < -lineOffset; i++)
+                    {
+                        for (newCurrent--; newCurrent >= 0 && _buffer[newCurrent] != '\n'; newCurrent--) ;
+
+                        if (newCurrent < 0)
+                        {
+                            newCurrent = lastEndOfLineIndex;
+                            break;
+                        }
+
+                        lastEndOfLineIndex = newCurrent;
+                    }
+                }
+            }
+            else
+            {
+                point = point ?? ConvertOffsetToPoint(_current);
+                int newY = point.Value.Y + lineOffset;
+
+                Point newPoint = new Point() {
+                    X = _moveToLineDesiredColumn,
+                    Y = Math.Max(newY, _initialY)
+                };
+
+                newCurrent = ConvertLineAndColumnToOffset(newPoint);
+            }
+
+            if (newCurrent != -1)
+            {
                 MoveCursor(newCurrent);
             }
         }

test/MovementTest.cs

@@ -31,6 +31,78 @@ namespace Test
                 ));
         }
 
+        [SkippableFact]
+        public void MultilineCursorMovement_WithWrappedLines()
+        {
+            TestSetup(KeyMode.Cmd);
+
+            int continutationPromptLength = PSConsoleReadLineOptions.DefaultContinuationPrompt.Length;
+            string line_0 = "4444";
+            string line_1 = "33";
+            string line_2 = "666666";
+            string line_3 = "777";
+
+            int wrappedLength_1 = 9;
+            int wrappedLength_2 = 2;
+            string wrappedLine_1 = new string('8', _console.BufferWidth - continutationPromptLength + wrappedLength_1); // Take 2 physical lines
+            string wrappedLine_2 = new string('6', _console.BufferWidth - continutationPromptLength + wrappedLength_2); // Take 2 physical lines
+
+            Test("", Keys(
+                "",     _.Shift_Enter,        // physical line 0
+                line_0, _.Shift_Enter,        // physical line 1
+                line_1, _.Shift_Enter,        // physical line 2
+                line_2, _.Shift_Enter,        // physical line 3
+                wrappedLine_1, _.Shift_Enter, // physical line 4,5
+                wrappedLine_2, _.Shift_Enter, // physical line 6,7
+                line_3,                       // physical line 8
+
+                // Starting at the end of the last line.
+                // Verify that UpArrow goes to the end of the previous logical line.
+                CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_3.Length, 8)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_3.Length, 8)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_3.Length, 8)),
+                // Press Up/Down/Up
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(wrappedLength_2, 7)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_3.Length, 8)),
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(wrappedLength_2, 7)),
+                // Press Up/Down/Up
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(wrappedLength_1, 5)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(wrappedLength_2, 7)),
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(wrappedLength_1, 5)),
+                // Press Up/Down/Up
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_2.Length, 3)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(wrappedLength_1, 5)),
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_2.Length, 3)),
+                // Press Up/Up
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_1.Length, 2)),
+                _.UpArrow,   CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length, 1)),
+
+                // Move to left for 1 character, so the cursor now is not at the end of line.
+                // Verify that DownArrow/UpArrow goes to the previous logical line at the same column.
+                _.LeftArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 1)),
+                // Press Down all the way to the end
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_1.Length, 2)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 3)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 4)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 5)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 6)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(wrappedLength_2, 7)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_3.Length, 8)),
+                _.DownArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_3.Length, 8)),
+                // Press Up all the way to the physical line 1
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(wrappedLength_2, 7)),
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 6)),
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 5)),
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 4)),
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 3)),
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_1.Length, 2)),
+                _.UpArrow, CheckThat(() => AssertCursorLeftTopIs(continutationPromptLength + line_0.Length - 1, 1)),
+
+                // Clear the input, we were just testing movement
+                _.Escape
+                ));
+        }
+
         [SkippableFact]
         public void MultilineCursorMovement()
         {