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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

clearScreen performs a full reset on some terminals #25

Open
Tyriar opened this issue May 5, 2021 · 7 comments
Open

clearScreen performs a full reset on some terminals #25

Tyriar opened this issue May 5, 2021 · 7 comments

Comments

@Tyriar
Copy link

Tyriar commented May 5, 2021

馃憢

We just had this issue reported which said \x1bc (from clearScreen) in xterm.js behaves differently to Terminal.app/iTerm xtermjs/xterm.js#3315. We behave as xterm/VTE does and don't think we should change so I would suggest indicating the fact in this lib that clearScreen may actually clear everything.

Note also that RIS is also more destructive than just clearing the buffer, also resetting modes among other things.

@Qix-
Copy link
Contributor

Qix- commented May 5, 2021

Hey @Tyriar :)

Yeah this is a naming issue. Most people want clearTerminal, which emits the (correct) \x1b[2J\x1b[3J\x1b[H.

clearScreen should be renamed to fullReset IMO, though this would be a major release.

@SamVerschueren
Copy link
Contributor

SamVerschueren commented May 5, 2021

@Qix- The problem with clearTerminal is that it resets the scrollback buffer, which is not always what people want. So from the docs clearScreen should actually clear the screen without resetting the buffer. So what I think what should happen is assign \u001B[H\u001B[2J to clearScreen (which is what the clear command prints when executed) and use \u001Bc as value for a new fullReset command.

Something like this

exports.clearScreen = `${ESC}H${ESC}2J`;
exports.fullReset = '\u001Bc';

Then clearScreen does what it's supposed to do according to the docs

Clear the terminal screen. (Viewport)

What do you think?

@Qix-
Copy link
Contributor

Qix- commented May 5, 2021

Yes sorry, I meant to edit addressing your original ticket but my internet cut out.

You want eraseScreen. The cursor position should reset in that case, but if it doesn't then we should also add ESC[H to it as well (though I don't think that's an issue).

@SamVerschueren
Copy link
Contributor

The problem with eraseScreen is that it seems to erase the screen but the cursor is not moved to the top left.

image

When I print the eraseScreen sequence with printf '\u001B[2J'

image

So I think we have to add ESC[H to move the cursor to the top left first.

@Qix-
Copy link
Contributor

Qix- commented May 5, 2021

Interesting, it seems to be random which terminals reset cursor position and which do not. Fun.

It's @sindresorhus's decision - should eraseScreen be the pure terminal code or should it have the opinion that the cursor also goes to the top-left?

If the latter (which I would agree with), then ESC[H should be appended to eraseScreen and clearTerminal should just have the codes ESC[2JESC[3JESC[H directly instead of re-using eraseScreen.

@jerch
Copy link

jerch commented May 6, 2021

Maybe establish a difference between erase and clear. Idea:

  • eraseLine - erasing line content, leaving the cursor alone
  • eraseLineLeft - erasing line content left of the cursor, leaving the cursor alone
  • eraseLineRight - erasing line content right of the cursor (+ content at cursor position), leaving the cursor alone
  • clearLine - erasing line content + cursor to left
  • eraseScreen - erasing screen content, leaving the cursor alone
  • eraseScreenTop - erasing screen top content up to cursor position, leaving the cursor alone
  • eraseScreenBottom - erasing screen bottom content from cursor position, leaving the cursor alone
  • clearScreen - easing content + cursor to top-left
  • clearScrollback - nullify scrollback
  • clearTerminal - same as clearScreen + clearScrollback
  • resetTerminal - RIS (or DECSTR, which is less "rebooty")

This model tries to resemble the ECMA-48 sequence ideas (VT100+), have not looked up your existing definitions, thus it might collide here or there. Well see it as an inspiration.

@sindresorhus
Copy link
Owner

There is some useful info in https://github.com/watchexec/clearscreen/blob/main/src/lib.rs

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

No branches or pull requests

5 participants