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

Closing a pane does not reset the PowerShell text position #2432

Closed
richardszalay opened this issue Aug 14, 2019 · 10 comments
Closed

Closing a pane does not reset the PowerShell text position #2432

richardszalay opened this issue Aug 14, 2019 · 10 comments
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-External For issues that are outside this codebase Tracking-External This bug isn't resolved, but it's following an external workitem.
Milestone

Comments

@richardszalay
Copy link
Contributor

Environment

Windows build number: Microsoft Windows [Version 10.0.18362.175]
Windows Terminal version (if applicable): 0.3.2171.0

Steps to reproduce

  1. Create a PowerShell tab
  2. Press enter until the text covers most of the window
  3. Split the view horizontally
  4. Close the bottom pane
  5. Start typing

Expected behavior

Text appears at previous cursor line

Actual behavior

Text appears in blank space. Screenshots of process:

pane-text-1

pane-text-2

pane-text-3

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 14, 2019
@zadjii-msft
Copy link
Member

So this is definitely a psreadline bug, though it does seem unique to Windows Terminal. Notably, I can't seem to repro a similar scenario in a conhost window:

  1. Do a wsl.exe powershell.exe to get a conpty running powershell in a conhost window.
  2. fill the buffer
  3. Resize down so the prompt is in the middle of the buffer
  4. Resize up again
  5. type anything

If this was a purely psreadline or conpty bug, then this wouldn't repro in this scenario. We'll need to do something better with the Terminal specifically.

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Aug 15, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 15, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Aug 15, 2019
@DHowett-MSFT
Copy link
Contributor

This is partially related to the fact that PowerShell saves and restores your cursor position across taking prompt input. That's also the cause of #2267.

@DHowett-MSFT DHowett-MSFT added Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 22, 2019
@zadjii-msft zadjii-msft added the Priority-2 A description (P2) label Jan 22, 2020
@zadjii-msft
Copy link
Member

zadjii-msft commented Mar 13, 2020

Okay with #4741 merged, this is still happening. I'm pretty sure PsReadline is saving the cursor position at the start of the prompt, and restores the position when it wants to redraw the terminal. Though, the question I have is "why isn't this happening if you just decrease, then increase the height of the terminal"

This discussion from VTE seemed relevant, but I'm not sure

https://gitlab.gnome.org/GNOME/vte/blob/master/doc/rewrap.txt

Shell prompt
────────────

If you resize the terminal to be narrower than your shell prompt (plus the
command you're entering) while the shell is waiting for your command, you see
weird behavior there. This is not a bug in rewrapping: it's because the shell
redisplays its prompt (and command line) on every resize. There's not much VTE
could do here.

As a long term goal, maybe readline could have an option where it knows that
the terminal rewraps its contents so that it doesn't redisplay the prompt and
the command line, just expects the terminal to do this correctly. It's a bit
risky, since probably all terminals that support rewrapping do this a little
bit differently.


EDIT 3-17-2020

image

Okay this definitely isn't a unique problem to the Windows Terminal, but it might only be trigger-able in the Windows Terminal. The above screenshot shows WT side-by-side with the conpty that's hosting it. As you can see, in that conpty, the cursor position jumped to the bottom of the viewport. So the Terminal is doing exactly what conpty told it to here.

But if you just normally resize the Terminal, it works fine:
image

This also seems to repro with the wsl version of pwsh, which I have 7.0.0 installed. So it's not uniquely Windows/the Win32 API that this is happening with.

When I do the whole split-pane, close pane repro with a breakpoint in ScreenBufferSizeChange in the first conpty, I get the following:

<split pane here>
ScreenBufferSizeChange(79, 11)
ScreenBufferSizeChange(79, 11)
conpty: 24136
ScreenBufferSizeChange(79, 12)
ScreenBufferSizeChange(79, 12)
<close pane here>
ScreenBufferSizeChange(79, 24)
ScreenBufferSizeChange(79, 24)

A bunch of resizes down to the smaller size, then a pair back up to the larger size.

@zadjii-msft zadjii-msft self-assigned this Mar 17, 2020
@zadjii-msft
Copy link
Member

@lzybkr, @SteveL-MSFT

Any ideas what might be going on here? I'm thinking that PSReadline is restoring the cursor relative to the bottom of the viewport, but I don't know where in PSReadline's code I'd go to check that out.

@lzybkr
Copy link
Member

lzybkr commented Mar 18, 2020

The cursor position is saved after the prompt here.

Console resizing has always a bit of problem, but mostly nobody noticed when the code was originally written because resizing used to be awkward. I considered moving away from .Net to INPUT_RECORD to get the resize events early in Win10 when resizing a console became much easier, but that would have made the code much less portable so I'm glad I didn't.

@lzybkr
Copy link
Member

lzybkr commented Mar 18, 2020

BTW, @daxian-dbw maintains PSReadLine now.

@zadjii-msft
Copy link
Member

Okay I'm just pasting here for my future reference code that looks relevant
This issue: PowerShell/PSReadLine#682
and the associated PR: PowerShell/PSReadLine#694

Which pointed me to this function
https://github.com/PowerShell/PSReadLine/blob/198a64390e713214df20ef580f7a546b5c3ac1e8/PSReadLine/Render.cs#L985-L1000

I can't be sure that's actually it, but that's certainly something to look into.

It looks like I've got 2.0.0 on my powershell 5 instance, and 2.0.0-beta3 on my powershell 6.2.3 instance.

Though, reading through that, it looks like _console.CursorTop might already be wrong at that point in the call. Looks like that function is just trying to see how many rows the input line takes, then is setting the _initialY to be that many rows above the cursor. So I'd think the cursor was already wrong at this point.

I'll keep digging

@zadjii-msft
Copy link
Member

zadjii-msft commented Apr 6, 2020

A simpler repro from #5254

Steps to reproduce

  1. Open Terminal in full screen
  2. Type ls
  3. unmaximize the window
  4. maximize the window
  5. type ls again and see that ls is not positioned correctly

displaybug


More investigation from today

With a plain old conhost:

  • Maximize it
  • Run ls
  • Restore down
  • Change the buffer size to match the window size
  • Maximize again

this will repro without conpty. I'm struggling to figure this out on our side so I'm going to move this upstream to PowerShell/PSReadLine#1456

@zadjii-msft
Copy link
Member

This may have been fixed in an updated PsReadline. Just need to confirm.

🎉 This issue was addressed in 3074, which has now been successfully released in v2.2.0-beta5. 🎉

@zadjii-msft
Copy link
Member

Just confirmed over here that this looks like it was fixed in PSReadline 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-External For issues that are outside this codebase Tracking-External This bug isn't resolved, but it's following an external workitem.
Projects
None yet
Development

No branches or pull requests

6 participants