From 42c0686a478774d61493aea9638fa1137d3aea9b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 9 Sep 2019 14:47:41 -0700 Subject: [PATCH 1/6] Fix arrow-up when the cursor is at the end of a wrapped line in a multiple-line text --- PSReadLine/Movement.cs | 64 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/PSReadLine/Movement.cs b/PSReadLine/Movement.cs index a964a2965..f7ace8b61 100644 --- a/PSReadLine/Movement.cs +++ b/PSReadLine/Movement.cs @@ -80,6 +80,18 @@ private void MoveToLine(int numericArg) { const int endOfLine = int.MaxValue; + // Actually not moving the line. + if (numericArg == 0) + { + return; + } + + // Moving the line down when it's actually at the end of the last line. + if (numericArg > 0 && _current == _buffer.Length) + { + return; + } + _moveToLineCommandCount += 1; var point = ConvertOffsetToPoint(_current); if (_moveToLineCommandCount == 1) @@ -90,25 +102,53 @@ private void MoveToLine(int numericArg) : point.X; } - var topLine = _initialY; + int newCurrent; + if (_moveToLineDesiredColumn == endOfLine) + { + newCurrent = _current; + + if (numericArg > 0) + { + // Moving to a subsequent line. + for (int i = 0; i < numericArg; i++) + { + for (newCurrent++; newCurrent < _buffer.Length && _buffer[newCurrent] != '\n'; newCurrent++) ; + + if (newCurrent == _buffer.Length) + { + break; + } + } + } + else + { + // Moving to a previous line. + int lastEndOfLineIndex = _current; + for (int i = 0; i < -numericArg; i++) + { + for (newCurrent--; newCurrent >= 0 && _buffer[newCurrent] != '\n'; newCurrent--) ; + + if (newCurrent < 0) + { + newCurrent = lastEndOfLineIndex; + break; + } - var newY = point.Y + numericArg; - point.Y = Math.Max(newY, topLine); - if (_moveToLineDesiredColumn != endOfLine) + lastEndOfLineIndex = newCurrent; + } + } + } + else { + int newY = point.Y + numericArg; + point.Y = Math.Max(newY, _initialY); point.X = _moveToLineDesiredColumn; + + newCurrent = ConvertLineAndColumnToOffset(point); } - var newCurrent = ConvertLineAndColumnToOffset(point); if (newCurrent != -1) { - if (_moveToLineDesiredColumn == endOfLine) - { - while (newCurrent < _buffer.Length && _buffer[newCurrent] != '\n') - { - newCurrent += 1; - } - } MoveCursor(newCurrent); } } From 309a855e58ee10bb94922bc9f34df789c03f2dbd Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 11 Sep 2019 10:40:46 -0700 Subject: [PATCH 2/6] Rename the parameter name of 'MoveToLine' to 'lineOffset' --- PSReadLine/Movement.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/PSReadLine/Movement.cs b/PSReadLine/Movement.cs index f7ace8b61..bd1d30da8 100644 --- a/PSReadLine/Movement.cs +++ b/PSReadLine/Movement.cs @@ -76,18 +76,18 @@ public static void BackwardChar(ConsoleKeyInfo? key = null, object arg = null) } } - private void MoveToLine(int numericArg) + private void MoveToLine(int lineOffset) { const int endOfLine = int.MaxValue; // Actually not moving the line. - if (numericArg == 0) + if (lineOffset == 0) { return; } // Moving the line down when it's actually at the end of the last line. - if (numericArg > 0 && _current == _buffer.Length) + if (lineOffset > 0 && _current == _buffer.Length) { return; } @@ -107,10 +107,10 @@ private void MoveToLine(int numericArg) { newCurrent = _current; - if (numericArg > 0) + if (lineOffset > 0) { - // Moving to a subsequent line. - for (int i = 0; i < numericArg; i++) + // Moving to the end of a subsequent logical line. + for (int i = 0; i < lineOffset; i++) { for (newCurrent++; newCurrent < _buffer.Length && _buffer[newCurrent] != '\n'; newCurrent++) ; @@ -122,9 +122,9 @@ private void MoveToLine(int numericArg) } else { - // Moving to a previous line. + // Moving to the end of a previous logical line. int lastEndOfLineIndex = _current; - for (int i = 0; i < -numericArg; i++) + for (int i = 0; i < -lineOffset; i++) { for (newCurrent--; newCurrent >= 0 && _buffer[newCurrent] != '\n'; newCurrent--) ; @@ -140,7 +140,7 @@ private void MoveToLine(int numericArg) } else { - int newY = point.Y + numericArg; + int newY = point.Y + lineOffset; point.Y = Math.Max(newY, _initialY); point.X = _moveToLineDesiredColumn; From 0109d7d638e5df54f7515070014ef1494410c807 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 12 Sep 2019 11:03:01 -0700 Subject: [PATCH 3/6] Address Carl's comment and avoid converting offset to point when not needed --- PSReadLine/Movement.cs | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/PSReadLine/Movement.cs b/PSReadLine/Movement.cs index bd1d30da8..e0d86e585 100644 --- a/PSReadLine/Movement.cs +++ b/PSReadLine/Movement.cs @@ -80,26 +80,24 @@ private void MoveToLine(int lineOffset) { const int endOfLine = int.MaxValue; - // Actually not moving the line. - if (lineOffset == 0) - { - return; - } - - // Moving the line down when it's actually at the end of the last line. - if (lineOffset > 0 && _current == _buffer.Length) - { - return; - } - + 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; + } + + // 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)) + { + return; } int newCurrent; @@ -140,11 +138,15 @@ private void MoveToLine(int lineOffset) } else { - int newY = point.Y + lineOffset; - point.Y = Math.Max(newY, _initialY); - point.X = _moveToLineDesiredColumn; + point = point ?? ConvertOffsetToPoint(_current); + int newY = point.Value.Y + lineOffset; + + Point newPoint = new Point() { + X = _moveToLineDesiredColumn, + Y = Math.Max(newY, _initialY) + }; - newCurrent = ConvertLineAndColumnToOffset(point); + newCurrent = ConvertLineAndColumnToOffset(newPoint); } if (newCurrent != -1) From 3e3daa7c87e6c911a8fc943d1f35eeee3cf96e5d Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 12 Sep 2019 11:11:59 -0700 Subject: [PATCH 4/6] Add comment about its behavior --- PSReadLine/Movement.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/PSReadLine/Movement.cs b/PSReadLine/Movement.cs index e0d86e585..3392d414c 100644 --- a/PSReadLine/Movement.cs +++ b/PSReadLine/Movement.cs @@ -78,6 +78,12 @@ public static void BackwardChar(ConsoleKeyInfo? key = null, object arg = null) 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. + const int endOfLine = int.MaxValue; Point? point = null; From 72543913c75b1e149364063030877b41504345b1 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 12 Sep 2019 11:21:42 -0700 Subject: [PATCH 5/6] Update comment a bit --- PSReadLine/Movement.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PSReadLine/Movement.cs b/PSReadLine/Movement.cs index 3392d414c..8099cc549 100644 --- a/PSReadLine/Movement.cs +++ b/PSReadLine/Movement.cs @@ -82,7 +82,8 @@ private void MoveToLine(int lineOffset) // - 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. + // '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; From a5de2b3bb9ff0e57b8242b2f6b3a30e54e3ec499 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 12 Sep 2019 16:36:54 -0700 Subject: [PATCH 6/6] Add Test for the UpArrow/DownArrow fix --- test/MovementTest.cs | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/test/MovementTest.cs b/test/MovementTest.cs index e5cf6faaa..0f38d4694 100644 --- a/test/MovementTest.cs +++ b/test/MovementTest.cs @@ -31,6 +31,78 @@ public void EndOfLine() )); } + [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() {