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

Perform terminal resizes synchronously #83558

Closed
octref opened this issue Oct 29, 2019 · 10 comments
Closed

Perform terminal resizes synchronously #83558

octref opened this issue Oct 29, 2019 · 10 comments
Assignees
Labels
*as-designed Described behavior is as designed feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label
Milestone

Comments

@octref
Copy link
Contributor

octref commented Oct 29, 2019

Version: 1.37.0-insider
Commit: f0058ed
Date: 2019-07-30T16:57:58.561Z
Electron: 4.2.7
Chrome: 69.0.3497.128
Node.js: 10.11.0
V8: 6.9.427.31-electron.0
OS: Linux x64 4.15.0-55-generic

Make sure there's enough space for 1 terminal, not 2:

image

Split:

image

@Tyriar
Copy link
Member

Tyriar commented Oct 29, 2019

Can you share the output of echo $PS1?

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Oct 29, 2019
@octref
Copy link
Contributor Author

octref commented Oct 29, 2019

\[\e]0;\u@\h: \w\a\]${debian_chroot:+($debian_chroot)}\[\033[01;32m\]\u@\h\[\033[00m\]:\[\033[01;34m\]\w\[\033[00m\]\$

@Tyriar
Copy link
Member

Tyriar commented Oct 29, 2019

So things will never be perfect when resizing because of the way that shells reprint the prompt and sometimes resizing will insert lines that the shell doesn't know how to handle, for example in Terminal.app after resizing a bit:

Screen Shot 2019-10-29 at 12 14 58 PM

However I think we can do better than we are currently by making resizes synchronous, to do this we would need flow control (#74620) and then do something like this when resizing:

  1. renderer: Tell node-pty to block for a resize
  2. node-pty: block the pty and resize
  3. node-pty: Tell renderer that it's been blocked and resized
  4. renderer: Ensure messages from pty have been parsed
  5. renderer: Resize terminal
  6. renderer: Tell node-pty to unblock

@Tyriar Tyriar added terminal General terminal issues that don't fall under another label polish Cleanup and polish issue bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Oct 29, 2019
@Tyriar Tyriar added this to the Backlog milestone Oct 29, 2019
@ayyash
Copy link

ayyash commented Nov 27, 2019

not sure if my issue is related to that, when I split the terminal, the statements start piling up instead of appearing on the same line... have a look: https://drive.google.com/file/d/1u8tVLEcXXF2ckh4OvN55x1zggga9Dn7X/view

@octref
Copy link
Contributor Author

octref commented Feb 5, 2020

No longer an issue for me. I'm not even sure now what exact behavior I'd be asking for. So closing.

@octref octref closed this as completed Feb 5, 2020
@ayyash
Copy link

ayyash commented Feb 6, 2020

lucky you, it still does that on my end

@octref
Copy link
Contributor Author

octref commented Feb 6, 2020

OK, I'll leave this open then

@octref octref reopened this Feb 6, 2020
@Tyriar Tyriar changed the title Splitting terminal causes terminal to show bad prompt Perform terminal resizes synchronously Apr 7, 2020
@Tyriar Tyriar removed the polish Cleanup and polish issue label Apr 7, 2020
@Tyriar Tyriar added feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug labels Jul 22, 2021
@Tyriar
Copy link
Member

Tyriar commented Jul 22, 2021

Here's one of the resize problems below.

Resize down:

Screen Shot 2021-07-22 at 9 14 06 AM

Resize up:

Screen Shot 2021-07-22 at 9 14 11 AM

Incoming data from bash:

Screen Shot 2021-07-22 at 9 15 06 AM

Resize events:
Screen Shot 2021-07-22 at 9 15 43 AM

@Tyriar
Copy link
Member

Tyriar commented Jul 22, 2021

In the above example it goes [K [A [A which is clear (bottom) line, move cursor up, move cursor up. xterm.js looks to be behaving as expected here.

Resizing Terminal.app by contrast ends up keeping the prompt on the same line which results in this. Might be because it's not unwrapping the lines?

Screen Shot 2021-07-22 at 9 19 45 AM

@Tyriar
Copy link
Member

Tyriar commented Jul 22, 2021

I updated bash (3->5) and it didn't make a difference. I think this is acting as expected and we seem to do as good of a job if not better than other terminals.

I remember thinking Windows Terminal did a better job than us in this regard but that appears to only be when it's behind conpty in a Windows environment, when wsl is used it can really mess up easily as well.

@Tyriar Tyriar closed this as completed Jul 22, 2021
@Tyriar Tyriar added the *as-designed Described behavior is as designed label Jul 22, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label
Projects
None yet
Development

No branches or pull requests

3 participants