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

API to check if alternate screen is active #2694

Closed
Eugeny opened this issue Feb 5, 2020 · 22 comments
Closed

API to check if alternate screen is active #2694

Eugeny opened this issue Feb 5, 2020 · 22 comments
Labels
area/api help wanted type/enhancement Features or improvements to existing features

Comments

@Eugeny
Copy link
Member

Eugeny commented Feb 5, 2020

Could we have an API to check whether xterm is currently showing the primary or the alternate screen?

@Tyriar
Copy link
Member

Tyriar commented Feb 5, 2020

@Eugeny what do you need it for out of curiosity?

@Tyriar Tyriar added area/api type/enhancement Features or improvements to existing features labels Feb 5, 2020
@Eugeny
Copy link
Member Author

Eugeny commented Feb 5, 2020

I've got automatic "progress detection" in Terminus that scans for percentage-like strings in terminal output. However if a text editor like vi or nano is open (usually in alternate screen mode) and the text contains "12%" or similar, it's getting redrawn by the application again and again, triggering progress detection code again.

I'm looking to just disable it as long as alternate screen is active

@mofux
Copy link
Contributor

mofux commented Feb 5, 2020

I can also see a lot of scenarios where this is useful. Things like prompt line detection algorithms usually need this kind of information too.

Maybe adding isNormalBuffer and isAltBuffer to IBuffer would be sufficient?

It would also be nice to have an event like terminal.onBufferActivate((buffer: IBuffer) => void ) to dynamically react to buffer changes.

@Tyriar
Copy link
Member

Tyriar commented Feb 5, 2020

Perhaps we should also take this opportunity to namespace buffer like we did with parser, something like this?

interface buffer {
  normal: IBuffer
  alt: IBuffer
  active: IBuffer
  onDidChangeBuffer: IEvent<IBuffer>
}

@mofux
Copy link
Contributor

mofux commented Feb 5, 2020

@Tyriar Yes that sounds good. So checking if the normal buffer is active would be an expression like terminal.buffer.active === terminal.buffer.normal? Or should we also introduce a convenience method for it either on the buffer or IBuffer interface?

@Tyriar
Copy link
Member

Tyriar commented Feb 5, 2020

I guess having a IBuffer.type: 'normal' | 'alt' would be the best of both worlds?

@jerch
Copy link
Member

jerch commented Feb 6, 2020

Just a minor headsup - we should take care with offering too much synchronous state introspection for a simple reason - ppl will end up trying to do things based on that but mostly ignore the current input buffer state, which might change things at any time again. (@Tyriar Remember that issue about someone trying to write a local lisp interface? Very hard to spot de-sync issues will result from that.)

Or maybe we should be more clear about that in the API doc, which part is sync/async...

@Tyriar
Copy link
Member

Tyriar commented Feb 7, 2020

@jerch I guess a note on write would help those problems. Trying to address that problem with docs would be better than not offering something like this.

Open to PRs, we might want to tweak the naming a bit but I think the basic shape of the API is there.

@JavaCS3
Copy link
Contributor

JavaCS3 commented Feb 11, 2020

@Tyriar serialize-addon also need a way to read alternative buffer. Otherwise it's very hard to implement terminal session playback player even terminal session sharing tool that allow anyone can join at anytime(I found vscode's live share ext already support terminal sharing, I don't know how it works instead of sending all the terminal output from the beginning)

@jerch
Copy link
Member

jerch commented Feb 11, 2020

@JavaCS3 Is that really an issue? Imho the serializer would always get the same buffer the user sees as screen output at the time of serialization. Furthermore apps typically use DECSET 1049 these days, which always would clean the altbuffer. Thus even if the altbuffer still holds data, it would be seen as remnant from a previous altbuffer session.

Imho the serializer only needs to care about one buffer at a given time. Note that real playback will not be possible with the serialize plugin, for that you will have to record the input data. (Serialize plugin - snapshot at a given time, playback - restore states over time; big difference).

Edit (offtopic): Sorry for mocking about the playback idea with the serialize plugin, still I have the feeling that there is a misconception. The serialize plugin can snapshot the terminal buffer at a given time. For a smooth playback thats not enough, as it only gives you isolated snapshots. What you need for real playback are "incremental diffs" from snapshot to snapshot. To save you some work - you dont have to calculate the incremental diffs from two snapshots yourself, it is already there in a nice condensed form - its the input data since the first snapshot. Simply take one snapshot, then start recording input data. Note that you will have to save a few more internal states with the initial snapshot to correctly replay it later on. At least you need all options, DEC private flags, buffer/tab settings, intercept resize and so on.

@mofux
Copy link
Contributor

mofux commented Feb 11, 2020

@jerch For me the use-case is the other way around. I only want to serialize the normal buffer, so when restoring a terminal after the app restarts, I only want to restore the history of the normal buffer. At the moment I'm using a hack by calling terminal._core.buffers.activateNormalBuffer() just before running the serialize addon. Having a public api for this would certainly be better.

@jerch
Copy link
Member

jerch commented Feb 11, 2020

@mofux So you want to give serialize the buffer as argument? But isn't that already possible with @Tyriar proposal above (after extending serialize that way)? Maybe I miss some point...

Edit (slightly offtopic again): I think we have different ideas what shall/can be serialized atm. As it is implemented now, serialize is "only" a snapshot tool of the current buffer state. Ofc thats not enough to get a full replay working starting at a certain time in a terminal session. Maybe we should discuss first, what more advanced serialize features are needed (maybe in a different issue though).

@JavaCS3
Copy link
Contributor

JavaCS3 commented Feb 11, 2020

@jerch The basic idea of terminal playback player is like what u're saying,(create a snapshot one by one and fill the gap between snapshots with incremental diffs) But there's a problem if the snapshot creation time is during alternative is active and then following incremental diffs contains DEC 1049 (Use Normal Screen Buffer and restore cursor). There will be a mess because the normal buffer is not being serialized yet. that's my point. Otherwise, I have to make sure the snapshot creation is just before DECSET 1049. But I think it would it helpful if I can directly access both normal buffer and alternative buffer .

(offtopic)
I will use asciinema to help me record terminal buffer instead of adding a new addon into xterm.js. The recording file of asciinema is like

[0.031006, "o", "\u001b]0;mat@mat-mint: ~/Documents/term-tris\u0007\u001b[01;32mmat@mat-mint\u001b[00m:\u001b[01;34m~/Documents/term-tris\u001b[00m$ "]
[0.739447, "o", "s"]
[0.843387, "o", "u"]
[0.939383, "o", "d"]
[1.051685, "o", "o"]
[1.147569, "o", " "]
[1.291866, "o", "p"]
[1.419392, "o", "y"]
[1.578995, "o", "t"]
[1.635077, "o", "h"]
[1.699073, "o", "o"]
[1.810955, "o", "n"]
[1.891268, "o", "3"]
[1.962887, "o", " "]
[2.083119, "o", "m"]
[2.202915, "o", "a"]
[2.315119, "o", "i"]
[2.394943, "o", "n"]
[2.603245, "o", "."]
[2.810757, "o", "p"]
[2.906992, "o", "y"]
[3.034495, "o", "\r\n"]
[3.04091, "o", "[sudo] password for mat: "]
[5.266162, "o", "\r\n"]
[5.369433, "o", "\u001b[?1049h\u001b[22;0;0t\u001b[1;24r\u001b(B\u001b[m\u001b[4l\u001b[?7h\u001b[?1h\u001b="]
[5.402342, "o", "\u001b[39;49m\u001b[?1h\u001b=\u001b[?25l"]
[5.402543, "o", "\u001b[?1000h\u001b[39;49m\u001b[37m\u001b[40m\u001b[H\u001b[2J"]

The purpose of creating a terminal playback player is the asciinema's terminal player is written in ClojureScript which I think is hard for people to contribute. So I decided to write my own version with xterm.js

@jerch
Copy link
Member

jerch commented Feb 11, 2020

But I think it would it helpful if I can directly access both normal buffer and alternative buffer .

But thats what @Tyriar's suggestion above is about, so it should be possible with that? So I am still not getting whats missing there.

Seems asciinema does a timetrace of the pty output. Yepp, imho the only way to get a correct replay done. The advantage of the serialize plugin here would be, that you could start the replay from any snapshot omitting the older pty output. Btw you can create such a timetrace pretty easy yourself, schematically:

let recording = false;
const recordedData = [];
function overloadedWrite(chunk, cb) {
  term.write(chunk, () => {
    if (recording) recordedData.push([Date.now(), 'input', chunk]);
    if (cb) cb();
  });
}
// replace term.write invocation with overloaded function
...
// initial snapshot, also activate recording
const initialData = addon.serialize(normal_buffer_as_arg);
// maybe 2nd call with altbuffer (if thats the active atm)
recording = true;
// more states need to be serialized here, like initial DEC private flags
const internalStates = ...
...
// stop recording
recording = false;

// --> final replay from snapshot is initialData + internalStates + recordedData

This is just the rough idea, its still missing bits like resize recording. And if you repeat the snapshotting in between and collect the recording under a snapshot, you basically get "sync frames" like in mpeg codecs, which later allows skipping/seeking.

@JavaCS3
Copy link
Contributor

JavaCS3 commented Feb 11, 2020

@jerch I think my first reply #2694 (comment) is a little bit misleading. @Tyriar‘s suggestion can solve my problem as well. I want to explain why we need access alternate buffer in my user case. Sorry for my poor expression.

@jerch
Copy link
Member

jerch commented Feb 11, 2020

@JavaCS3 All good, no need to apologize. I think we should discuss further serialize ideas in another thread. It seems, @mofux needs something similar to the replay thingy as well (reading between his lines).

@mofux
Copy link
Contributor

mofux commented Feb 11, 2020

@jerch I guess there was some misunderstanding 🤗 All I need is clean way to tell the serialize addon which buffer to serialize. I think the API proposed by @Tyriar will provide it with the API required to do so.

@Tyriar
Copy link
Member

Tyriar commented Feb 12, 2020

All I need is clean way to tell the serialize addon which buffer to serialize.

This is what I want eventually as well, consider the following scenario:

  1. Open VS Code
  2. Open a terminal
  3. Open vim
  4. Restart VS Code
  5. A terminal gets opened, restoring the normal and alt buffers

I know there is other state/modes in the terminal which may cause problems with this restore but it should cover most cases for the time being and we can always improve as needed later.

@Tyriar
Copy link
Member

Tyriar commented Feb 20, 2020

Just came up with my own use case for checking if the alt buffer is active: microsoft/vscode#90838

@anatoly314
Copy link

I can already be achieved by this code terminal.buffer.active.type or I miss something?

@JavaCS3
Copy link
Contributor

JavaCS3 commented Aug 23, 2020

I can already be achieved by this code terminal.buffer.active.type or I miss something?

It's because #2713 was merged

@Tyriar Tyriar closed this as completed Aug 24, 2020
@Tyriar
Copy link
Member

Tyriar commented Aug 24, 2020

Thanks for the bump 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api help wanted type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

6 participants