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

Reopen: vscode terminal disrespects cls command in windows #45137

Closed
mihailik opened this issue Mar 6, 2018 · 10 comments
Closed

Reopen: vscode terminal disrespects cls command in windows #45137

mihailik opened this issue Mar 6, 2018 · 10 comments
Assignees
Labels
*duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster terminal General terminal issues that don't fall under another label

Comments

@mihailik
Copy link
Contributor

mihailik commented Mar 6, 2018

#23227 was closed incorrectly:

I believe this is a duplicate of this: xtermjs/xterm.js#106

First of all, this is not a duplicate of xterm.js issue titled Running clear in terminal removes viewport content from buffer instead of hiding it. It is the precise opposite of the behaviour described in that xterm.js issue: on Windows the content is hidden rather than wiped.

Note that @Tyriar himself (owner of term.js repo) said in that issue:

As for Windows, I'm not sure if there's anything that can be done on our side either.

If this is not something term.js is meant to fix, VSCode should handle the use case.

To clarify how this bug makes a big deal, and unnecessary pain

In the course of the day I've run a build on a large project, which produced a heap of errors — my bad, silly coding error. Upon fixing the error, I've done "CLS" to wipe off all that sea of red, then run the build again.

That produced a couple of minor errors — which in fact looked like a sea of red again, because scrollback buffer was still full of junky errors from my first run.

I've done CLS again, tweaked some files, rebuilt — all the long list is still with me.

When I was doing CLS, I was expecting to get a wipe-clean terminal. Because VSCode didn't do it correctly, my previous build output got mixed up with the new stuff, and I've wasted quite a bit of time to figure out it's VSCode messing with me.

@vscodebot vscodebot bot added the terminal General terminal issues that don't fall under another label label Mar 6, 2018
@mihailik
Copy link
Contributor Author

mihailik commented Mar 6, 2018

Apparently, that also affects tsc --watch behaviour.

The latest release of TypeScript modified its rebuild-on-change behaviour. It wipes the screen before doing incremental rebuild, just to make it easier to tell apart build outputs from consecutive builds.

VSCode sticks those back together however :-(

@mihailik
Copy link
Contributor Author

mihailik commented Mar 6, 2018

When VSCode runs on Windows, it should override term.js handling of [wipe] signal.

It could simple destroy old terminal instance, create new and then continue with that reflecting the same actual console that's been running behind the scenes.

@Tyriar
Copy link
Member

Tyriar commented Mar 6, 2018

@mihailik you're right I think I closed #23227 in error.

Apparently, that also affects tsc --watch behaviour.

I was just fixing an issue caused by this and the clearing worked just fine for me under Windows 10 FCU.

It could simple destroy old terminal instance, create new and then continue with that reflecting the same actual console that's been running behind the scenes.

This isn't feasible, firstly we don't know at the VS Code level, or even from the xterm.js level that cls was run, only the shell process knows that. Secondly it would lose a huge amount of information and listeners which would break the terminal if we reconstructed it.


As a workaround I suggest you use ctrl+k to clear the terminal instead of cls.

What version of Windows are you on and what's in your settings.json file?

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Mar 6, 2018
@mihailik
Copy link
Contributor Author

mihailik commented Mar 7, 2018

OK, investigated a bit more — it's a slippery one to catch.

VSCode About box:

Version 1.20.1
Commit f88bbf9137d24d36d968ea6b2911786bfe103002
Date 2018-02-13T15:34:36.336Z
Shell 1.7.9
Renderer 58.0.3029.110
Node 7.9.0
Architecture x64

Windows 7
ver output: Microsoft Windows [Version 6.1.7601]
about box: Version 6.1 (Build 7601: Service Pack 1)


When I start new VSCode instance and do some simple terminal operations - CLS works fine.

It's not too hard to bump into the bug though. I've done a bit in the terminal (a couple of screens worth of interaction), then I've started TSC with --watch and --pretty tags — and at that point it flipped and stopped cleaning the buffer.

Note that the terminal still ate up a bit of old content, just not all of it. My command to run tsc --watch --pretty is cut off, and few other lines. But the rest of history is there. And I can see the output from TSC watch below that:

Terminate batch job (Y/N)? y

C:\code\headless>cd ../runner
The system cannot find the path specified.

C:\code\headless>cd ..\runner
The system cannot find the path specified.

C:\code\headless>cd ..
<------------------------------------------ - - - HERE ABOUT ~10 LINES WAS WIPED
[10:20:20 AM] Starting compilation in watch mode...


[10:20:23 AM] Compilation complete. Watching for file changes.


Looking in VSCode DevTools, nothing seems suspect.


To be honest, it feels like VSCode+term.js wipe a chunk of buffer after screen clear event, rather than the whole of it. The chunk could be proportional to the visible height of the terminal — i.e. the visible area of the buffer is wiped, rather than reducing the whole buffer.

And to add a crucial point: Ctrl-K does clear the buffer fully.
And one more thing: after doing Ctrl-K once, the bug disappears. Which only adds to the suspicion this is to do with whole buffer height versus visible terminal height.

Wonder if it's something in VSCode handling the of Windows-specific hidden console, before passing the updates to term.js?

@mihailik
Copy link
Contributor Author

mihailik commented Mar 7, 2018

@Tyriar please let me know if more info needed, or I can do some digging at my side?

And much thanks for looking into that!

@Tyriar
Copy link
Member

Tyriar commented Mar 7, 2018

To be honest, it feels like VSCode+term.js wipe a chunk of buffer after screen clear event, rather than the whole of it. The chunk could be proportional to the visible height of the terminal — i.e. the visible area of the buffer is wiped, rather than reducing the whole buffer.

This is as designed currently as mentioned in xtermjs/xterm.js#106

And to add a crucial point: Ctrl-K does clear the buffer fully.

ctrl+k uses a different mechanism to clear, there vscode tells the terminal to clear the entire buffer and move the line with the cursor on it to the top. When the shell tells the terminal to clear it will send several instructions to the terminal such as to clear the viewport and/or clear the scrollback (everything above the viewport). As noted in xtermjs/xterm.js#106 (comment), this is an issue with winpty not sending back the correct escape sequences.

@mihailik
Copy link
Contributor Author

mihailik commented Mar 8, 2018

Yes, but that's not as console is designed and intended to work on the target platform.

If xterm.js can't/won't fix it, VSCode should have a workaround. All the communication from remote console process to VSCode shell is clearly visible.

For example, VSCode can intercept 'clear screen' sequence and replace it with Ctrl-K when running on a platform where that behaviour is required.

@Tyriar
Copy link
Member

Tyriar commented Mar 9, 2018

VSCode can intercept 'clear screen' sequence

This is easier said than done, there isn't really a reliable way to do this without introducing other bugs. This is an upstream issue in winpty that is blocked on either the bug being fixed or something like microsoft/terminal#57 becoming available.

@Tyriar Tyriar closed this as completed Mar 9, 2018
@Tyriar Tyriar added the *duplicate Issue identified as a duplicate of another issue(s) label Mar 9, 2018
@mihailik
Copy link
Contributor Author

mihailik commented Mar 9, 2018

@Tyriar duplicate of which issue is it?

This is an unexpected attitude from Microsoft project such as VSCode: to close a very clear, reproduceable bug for a second time by the same person. And both times without a justification, on an excuse that makes no sense in the context of the issue.

Two closures with no explanation

The first time the bug was closed as "a duplicate of xtermjs/xterm.js#106". The behaviour described in referenced issue is a complete opposite of what the bug is about.

This second time the bug is closed as "a duplicate" with no issue referenced whatsoever. The comment makes an impression that issue is simply brushed off.

  • "This is easier said than done" — that won't do for any professional project, never mind of this size.
  • "there isn't really a reliable way to do this without introducing other bugs" — why not put forward the potential troubles and let the volunteers decide if they want to help?
  • "This is an upstream issue in winpty" — which upstream issue?
  • "that is blocked on either the bug being fixed or something like Official API to create a third-party console host (like pty in Unix) terminal#57 becoming available" — given that in the matter of last few days the issue was attributed to very different causes, it's only natural to be dubious on that count.

Would really help to see elaboration on that please!

@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2018

@mihailik winpty is the cause of an enormous amount of issues on Windows because of how it works, it's a big box of black magic attempting to convert the very verbose Windows API into a Unix-like API. I've accumulated the issues here #45693 (the xterm.js mention is called out there specifically).

My responses are typically terse because I close these winpty issues off several times a week and explaining in detail on every single issue would affect my productivity even more than it already does. Basically it's not worth the time investment on our side to fix this because we should be getting microsoft/terminal#57 soon which will allow us to use winpty only as a legacy solution as a fallback for older versions of Windows.

TL;DR: The root cause of this is the pty emulation from winpty which, while is an impressive feat that it works at all, has many flaws #45693. The fix is getting microsoft/terminal#57 and then integrating the native Windows pty API into VS Code. It shall be fixed.

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster terminal General terminal issues that don't fall under another label
Projects
None yet
Development

No branches or pull requests

2 participants