Skip to content

Fix arrow-up when the cursor is at the end of a wrapped line in a multiple-line text #1028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 12, 2019
Merged

Fix arrow-up when the cursor is at the end of a wrapped line in a multiple-line text #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
42c0686
Fix arrow-up when the cursor is at the end of a wrapped line in a mul…
daxian-dbw Sep 9, 2019
309a855
Rename the parameter name of 'MoveToLine' to 'lineOffset'
daxian-dbw Sep 11, 2019
0109d7d
Address Carl's comment and avoid converting offset to point when not …
daxian-dbw Sep 12, 2019
3e3daa7
Add comment about its behavior
daxian-dbw Sep 12, 2019
7254391
Update comment a bit
daxian-dbw Sep 12, 2019
a5de2b3
Add Test for the UpArrow/DownArrow fix
daxian-dbw Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 63 additions & 14 deletions PSReadLine/Movement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,39 +76,88 @@ public static void BackwardChar(ConsoleKeyInfo? key = null, object arg = null)
}
}

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);
}
}
Expand Down
72 changes: 72 additions & 0 deletions test/MovementTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down