-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Use more universal ANSI sequence for 'clear screen and clear buffer' #57701
Conversation
For what it’s worth, the
I am pointing to |
If we're going to do this, I'm somewhat inclined to take the more popular approach of using the same sequence that ansi-escapes/jest take, though that appears to be platform specific which may make our tests unhappy. I'll have to ask for feedback on this from the team. |
I wonder if we even need to make a distinction between mac and windows echo -n -e "\x1B[2J\x1B[3J\x1BH\x1B[0f" Works on all my terminals (Terminal, iTerm, & VSCode). I don't have a Windows machine to test on, though. (I updated the PR to use this sequence) |
The passage of time made me forget the ANSI sequence I had settled on, but I remembered this morning and updated this PR one more time. So, the sequence in there is based on the one from ansi-sequences. It's just a combination of the two, without the OS check. |
I believe that this also fixes #29829 (which I found via people commenting duplicates on the related issues to this PR's). Planning on doing some local testing, though, it's just a lot of terminal emulators to test... If we are doing this, I hope nobody relies on our specific escape sequence. I think not, but rather they depend on other watch messages. |
I did some testing of this PR and #59315. old behavior:
We emit
No
No I have not tested on something like Windows 10 with cmd, though; but I'm fairly confident that this PR is good. |
Using this launch task in our repo:
Shows that the problem matcher in VS Code also works fine: Then undo the bad code: |
Out of curiosity: what if |
That's exactly what this PR does, if I'm understanding you correctly. diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts
index 32d7af09c03e7..cf9318afdd199 100644
--- a/src/compiler/sys.ts
+++ b/src/compiler/sys.ts
@@ -1619,7 +1619,7 @@ export let sys: System = (() => {
setTimeout,
clearTimeout,
clearScreen: () => {
- process.stdout.write("\x1Bc");
+ process.stdout.write("\x1B[2J\x1B[3J\x1BH\x1B[0f");
},
setBlocking: () => {
const handle = (process.stdout as any)?._handle as { setBlocking?: (value: boolean) => void; }; |
Almost. I am suggesting to skip |
I can test that, but what effect are you trying to get by omitting that? |
Tested, and omitting that seems to leave a bunch of space above the log, not actually clearing the screen (and the extra empty lines are scrollable). So it seems to be required. |
Thanks. That’s interesting. I thought that |
Sorry, me again. Seems like |
If I'm reading this correctly, Edit: Ah, the rules for the omission are noted below: "If Edit 2: I think I was misreading the bit about the Home key. It's the other way around - pressing the Home key in "Normal Mode" generates the CUP sequence Edit 3: And going full circle, "If |
To add to the above,
But maybe there are differences between terminals that mean you need more. |
I'm going through all of the combos now (fixing the potential typo, trying those two shorter sequences). |
Ah, that page is incomplete -
|
According to this, |
#57701 but with
Just
Just
However, not including |
That was the next question, which one to choose! Seems like we should be good with |
For completeness:
|
Fixes #57700
Fixes #29829
Fixes #57894
Closes #59315
Tiny tiny patch to bring just a little more terminal support to
tsc --watch
. In my case, iTerm2 needs thisESC [ 3J
sequence in order to clear screen and remove scroll history.If there's a setting in iTerm2 to change this, I would happily do that, but I didn't see anything.