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

Sending \u001Bc escape sequence clears scrollback buffer #3315

Closed
SamVerschueren opened this issue May 5, 2021 · 16 comments
Closed

Sending \u001Bc escape sequence clears scrollback buffer #3315

SamVerschueren opened this issue May 5, 2021 · 16 comments

Comments

@SamVerschueren
Copy link

Details

  • Browser and browser version: Google Chrome | 90.0.4430.93 (Official Build) (x86_64)
  • OS version: macOS Big Sur
  • xterm.js version: 4.9.0

Steps to reproduce

  1. Use a regular macOS Terminal (or iTerm if you prefer) and execute ls a couple of times and then execute printf '\u001Bc'. You will notice that the scrollback buffer is not cleared.
  2. Open Visual Studio Code and do the same thing, you will now notice that the scrollback buffer is cleared.

I noticed the difference in behaviour because I was using ansi-escapes#clearscreen which shouldn't clear the buffer but it does in xterm.js.

@jerch
Copy link
Member

jerch commented May 5, 2021

ESC c means "reset to initial state" (RIS). By defintition it resets almost everything on the terminal, including the buffer ("full reset"). Using it for viewport clearing is wrong, thats a bug in the package you are using. (In terms of a blackboard: It is not just wiping the board, it is like putting a brand new board in place).

Why Terminal.app and iTerm keep the buffer on RIS I cant tell (maybe they have a custom setting for this?). It does not resemble the meaning of "reset" at all. Note that RIS is the only sequence with the power to fully reset and reinitialize the terminal, thus we cannot change it.

@Tyriar
Copy link
Member

Tyriar commented May 5, 2021

@jerch do you know if VTE acts like us or the mac terminals?

@jerch
Copy link
Member

jerch commented May 5, 2021

vte 0.52.2 also clears the buffer, and so does xterm.

@Tyriar
Copy link
Member

Tyriar commented May 5, 2021

Ok let's close this as designed then as we want to act the same everywhere, xterm.js doesn't even know the true OS that the backend is on. FWIW VS Code also relies on this reset everything behavior and would need changes if we did this.

@Tyriar
Copy link
Member

Tyriar commented May 5, 2021

Created sindresorhus/ansi-escapes#25

@SamVerschueren
Copy link
Author

Already checking this with @sindresorhus and @Qix-. Thanks for creating the issue and for the feedback @jerch @Tyriar!

@SamVerschueren
Copy link
Author

@jerch @Tyriar I have a question which is somewhat related. I can create a new issue if this seems to be an xterm thing but want to check it with you guys first.

So when sending RIS, the entire terminal resets, as well as the scrollback buffer and what not. So that's pretty clear behaviour.

However, when doing a "clear scrollback buffer", the buffer is cleared, but the scrollbar is not being removed. I screencaptured the behaviour.

Commands used in the screen capture

  1. ls -la (just to make sure we have a scrollbar)
  2. printf '\u001B[2J\u001B[3J\u001B[H' (ESC 2J + ESC 3J + ESC H)
  3. printf '\u001Bc' (RIS)

So ESC 3J should clear the scrollback buffer, which it does. But xterm.js still shows the scrollbar as if the scrollback buffer was not cleared. If you then scroll, nothing happens.

Kapture 2021-05-06 at 08 01 17

@SamVerschueren
Copy link
Author

I ended up using terminal.clear() which seems to behave entirely like I want 🎉 . But still wondering if the above is an xterm bug or not 🤔 .

@jerch
Copy link
Member

jerch commented May 6, 2021

Thx for finding this one. Looks like a bug to me.

@Tyriar Is that a regression around the event rework of the input handler? Seems the onScroll event here gets not caught on / delegated to renderer side:

case 3:
// Clear scrollback (everything not in viewport)
const scrollBackSize = this._bufferService.buffer.lines.length - this._bufferService.rows;
if (scrollBackSize > 0) {
this._bufferService.buffer.lines.trimStart(scrollBackSize);
this._bufferService.buffer.ybase = Math.max(this._bufferService.buffer.ybase - scrollBackSize, 0);
this._bufferService.buffer.ydisp = Math.max(this._bufferService.buffer.ydisp - scrollBackSize, 0);
// Force a scroll event to refresh viewport
this._onScroll.fire(0);
}

while replacing it with the following works again:

(this._bufferService as any)._onScroll.fire(0);

@SamVerschueren
Copy link
Author

So this means the this._onScroll.fire(0) should happen outside the if statement to make sure it always fires?

Because it seems that if you only use ESC 3J (printf '\u001B[3J'), it actually reduces the scrollbar correctly but then you still have 1 line in the buffer so it probably ends up inside the if statement in that case.

@jerch
Copy link
Member

jerch commented May 6, 2021

Imho the onScroll event can stay inside the condition (it just skips things, because we are already scrolled to line 0). But if you enter echo -e '\x1b[3J\x1b[H' after some pages of scrollback, the scrollback bar will not shrink, which is a bug.

@jerch
Copy link
Member

jerch commented May 6, 2021

@SamVerschueren Btw Terminal.clear is a bit dangerous, it can screw up your terminal instance/state. It is ignoring the chunk band position, and will be executed synchronously. Kinda all sync methods on the terminal have that limitation.

@SamVerschueren
Copy link
Author

Do you know what VS Code uses internally when you press Cmd + k? Because I'm implementing that shortcut and want it to behave the same way.

@jerch
Copy link
Member

jerch commented May 6, 2021

Seems it does clear & focus on the terminal instance: https://github.com/microsoft/vscode/blob/2000f36fdefb06321db6ceaa893e9b7e92e0f466/src/vs/workbench/contrib/terminal/browser/terminalActions.ts#L1513-L1537

Edit: Clear is fine to be used if the terminal is at rest waiting for input and the app on the other end is just using it as logging output. Or the other way around - clear will screw up output of ncurses apps or any app, that use line length calculations or rely on cursor positions to update content ("canvas mode").

@Tyriar
Copy link
Member

Tyriar commented May 6, 2021

The scrollbar thing looks like a bug, yes please create an issue 🙂

Seems it does clear & focus on the terminal instance:

Yep VS Code uses clear here, it may do weird things on Windows (due to pty emulation) and in the alt buffer for reasons @jerch called out. This is how most terminals work afaik.

@SamVerschueren
Copy link
Author

Created an issue for this 🔝

Thanks for all the information. Really love the hard work you guys put into this 🙏 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants