Skip to content
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.

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

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

Merged
merged 6 commits into from
Sep 12, 2019

Conversation

daxian-dbw
Copy link
Member

Fix #1020

When the cursor is at the end of a wrapped line, Up key sets the potential new cursor position to the previous physical line, and then calculates the end of the logical line rendered in that physical line, and eventually ends up in the same old cursor position.
That's why Up at the end of a warped line doesn't work.
The fix is to not depend on the physical line position when the original cursor is at the end of line, but directly search back (for Up) and search forward (for Down) for new line characters.

@daxian-dbw
Copy link
Member Author

daxian-dbw commented Sep 9, 2019

/cc @msftrncs

@daxian-dbw
Copy link
Member Author

I will try adding a test.

@daxian-dbw
Copy link
Member Author

@msftrncs Would you mind review the changes? Thanks!

PSReadLine/Movement.cs Outdated Show resolved Hide resolved
PSReadLine/Movement.cs Show resolved Hide resolved
PSReadLine/Movement.cs Show resolved Hide resolved
@msftrncs
Copy link
Collaborator

msftrncs commented Sep 12, 2019

This does resolve the problem.

When the cursor has been intentionally moved to the end of the wrapped line (or came from being intentionally placed at the end of a line below or a line above), pressing up arrow will take the cursor up to the end of the next full line up.

On the other hand, if the cursor landed at the end of a wrapped line with a up-arrow not from the end of the line below, or down-arrow not from the end of the line above, the cursor resumes its normal place when going up or down another line (when it is able to).

I checked this behavior with multiple wrapped lines in a row, with both existing history from my session and with some jibberish here-strings quickly formulated.

I did find a new behavior that seemed not to exist before (I cannot repeat it with Windows PowerShell which I still have running PSReadLine 2.0.0 beta 12): if the cursor is logically not at the end of the line, and down arrow is pressed on last line but yet the cursor's physical position is at the end of that line, the cursor becomes logically assigned to the end of the line, so then moving up takes the cursor through the end of the logical lines again. This does not require any lines that wrap. Simple example:

@'
hello
`@

With the cursor sitting on 'o' (not after it), pressing down and back up work as expected, but pressing down twice, then back up takes the cursor to past the 'o', as now it moves to the logical line ends.

@daxian-dbw
Copy link
Member Author

did find a new behavior that seemed not to exist before

Good catch @msftrncs, thank you!
The behavior change is because of the line of _moveToLineCommandCount += 1; won't always be hit after my previous change. I have fixed it and now it should behave the same as before -- pressing down twice in the repro above won't change the targeted column when pressing up again.
I also made a change to avoid converting offset to point when it's not necessary, also added comments to describe the behavior clearly.

@daxian-dbw
Copy link
Member Author

Thanks all for the review!

@daxian-dbw daxian-dbw merged commit 7d779cf into PowerShell:master Sep 12, 2019
@daxian-dbw daxian-dbw deleted the arrowUp branch September 12, 2019 23:42
@daxian-dbw daxian-dbw added this to the 2.0.0-beta5 milestone Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

editing wrapped line in history, up-arrow doesn't function at end of wrapped line.
4 participants