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

mitigate prompt issue #1178

Merged

Conversation

TylerLeonhardt
Copy link
Member

I needed to remove the check of PSReadLine because in the PSRL experience, whether you used ENTER or Ctrl-C, we'd still only get empty string...

This means that it would work with ENTER as ENTER would cause the cursor to go down... but Ctrl-C didn't.

I'm not sure if this is a PSRL bug that only we are exposing or actually our bug... but this makes it less ugly for sure.

The downside is that once the screen starts scrolling, an extra new line shows up.

cc @daxian-dbw if you're curious.

Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The downside is that once the screen starts scrolling, an extra new line shows up.

Note that this used to work at some point (on Windows). Somewhere down the line (probably with the switch to ConPTY) the cursor API's started working more like Unix and now only reports based on the current view port instead of the whole buffer. The check if the cursor position is the same is now effectively useless.

@TylerLeonhardt
Copy link
Member Author

@SeeminglyScience so I removed that cursor check and that actually gives us the extra new line all the time now rather than just when the buffer can scroll... hmmm

@SeeminglyScience
Copy link
Collaborator

@SeeminglyScience so I removed that cursor check and that actually gives us the extra new line all the time now rather than just when the buffer can scroll... hmmm

Yeah. The cursor position reported is based on the view port instead of the buffer. The cursor check was designed assuming (because it was at one point) that it would be based on the buffer. That's why once the buffer is larger than the view port, that check no longer works as initially designed. Removing the check makes the behavior consistent, but an extra line still needs to be removed somewhere.

@TylerLeonhardt
Copy link
Member Author

Yeah I think consistency sounds better here. Plus we'd be removing 2 Cursor position queries which I'm happy about. Removed those @SeeminglyScience.

@TylerLeonhardt TylerLeonhardt merged commit 40380f6 into PowerShell:master Feb 10, 2020
@TylerLeonhardt TylerLeonhardt deleted the mitigate-prompt-issue branch February 10, 2020 21:56
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.

3 participants