add SerializeAddon #2298

JavaCS3 · 2019-07-10T13:52:54Z

This PR is refer to #2213
Color and Wrap mode are NOT supported in serialize function. Will continue improve.

Part of #595

Tyriar

Nice work 👏

addons/xterm-addon-serialize/src/SerializeAddon.ts

addons/xterm-addon-serialize/typings/xterm-addon-serialize.d.ts

package.json

…ist join instead and some code clean up

addons/xterm-addon-serialize/src/SerializeAddon.ts

Tyriar · 2019-07-12T15:38:54Z

addons/xterm-addon-serialize/typings/xterm-addon-serialize.d.ts

+
+    /**
+     * Serializes terminal rows into a string that can be written back to the terminal
+     * to restore the state. The cursor will also be positioned to the correct cell.


I think we should add an explicit cursor move sequence (\x1b[y;xH) if it's not where we expect it, otherwise this statement is false.

Same as with my note above - absolute positioning of the cursor would have to be corrected later during restore, if the target terminal has other rows X rows width, thus serialize needs to export the viewport size to not lose this important bit of information.

jerch

Yeah great PR 👍. I am a bit unsure about the final goal of the addon, whether you want exact cell restoration later on or not. See my notes for further details.

jerch · 2019-07-12T23:24:47Z

addons/xterm-addon-serialize/src/SerializeAddon.ts

+
+    for (let i = 0; i < length; i++) {
+      const line = buffer.getLine(i);
+      lines[i] = line ? line.translateToString() : '';


If the goal of the addon is to get the closest possible match to be replayed on other terminals it should also serialize/restore gaps in lines (empty cells up to trim border). Those can happen if the cursor jumped over cells (e.g. some cursor command or TAB). Note that translateToString fills those with whitespaces to get a close string representation, that does not rely on terminal output (cannot use cursor commands for that).

To also grab those gaps imho its better to walk over all cells and check explicitly for content. If a real gap was found (watch out for empty cells after wide chars, those dont count here), the cursor should be moved forward. Doing this will ensure that the content is really the same after serialize/restore (beside the attrs that are not exported yet).

But as always, the devil is in the details. For this to work reliably, serialize would also have to export the viewport grid size, and a later restore action would have to take this into account.

My goal is to write a terminal session player which is the same as asciinema but with audio support. examples: https://asciinema.org/a/138296

But asciinema is written in ClosureScript that I believe rare people is familiar with. So I have to implement it in my own way.

To answer your question, Yes, I need to get the closest possible match to the original terminal session

Ah ok, then I think you cannot go with translateToString directly and need a more advanced buffer scraping method instead. Also there is no simple replacement character to keep the null cells in the output (you cannot use NUL as it will be skipped by emulators as stream padding byte).

Hmm, do you want the data to be re-applicable to other targets (other emulators beside xterm.js)? If so, then the serializer would have to know about the features of that target. Note that there are dumb terminals, that cannot deal with cursor movements at all and would map CUF to whitespace (basically translateToString assumes this dumb control char capabilities). Same with text attrs once you expose them - not all targets understand them.

So if you want to support those "other targets", imho serialize needs options to be eval'ed during export:

public serialize(rows?: number, opts?: ExportOptions): string { ... if (!cell.hasContent) { // empty cell for (let i = 0; i < cell.width; ++i) { output += opts.CUF; // ' ' for dumb, '\x1b[C' for smart terminals } ... }

Not sure if we expose enough buffer state yet in API to let you do it this fine-grained.

Later on, this would look likewise for text attributes.

Edit: A different approach would be to serialize as much as possible information into a not directly applicable format, and do the features switch during restore. This has one advantage: you still have all the needed bits of information and can apply the same data to HTML, dumb, xterm.js (to be fed back). The major disadvantage is the fact, that this will need a restore component, while doing it during serialize would not.
Not sure where the tradeoff is, imho a directly applicable format is nicer to deal with.

IMO xterm.js is enough for me and I don't need to support some special terminal emulator currently. All I want is to "copy" all the features that asciinema already have by Typescript.

Another thing is asciinema is based on VT terminal (see https://github.com/asciinema/vt), I believe it's the same as xterm.js

And I count this commit as a beginning, it doesn't need to cover all that cases for now. Continuously add features steps by steps is a good choice for me.

This commit based on the discussion of #2213 (comment)

Yeah lets not over complicate things. If you only want to go with xterm.js compatibility, you can take any escape sequence in the final output thats listed in InputHandler. (We dont have a list of supported sequences yet.)

jerch · 2019-07-12T23:40:13Z

addons/xterm-addon-serialize/typings/xterm-addon-serialize.d.ts

+
+    /**
+     * Serializes terminal rows into a string that can be written back to the terminal
+     * to restore the state. The cursor will also be positioned to the correct cell.


Same as with my note above - absolute positioning of the cursor would have to be corrected later during restore, if the target terminal has other rows X rows width, thus serialize needs to export the viewport size to not lose this important bit of information.

JavaCS3 · 2019-07-16T12:11:50Z

Can I merge to master now?

mofux · 2019-07-16T16:38:54Z

Hi @JavaCS3, thanks for your contribution, we think your code quality is really good and we appreciate your effort!

We've just had an internal discussion within the xterm.js maintainer team, and we came to the conclusion that this addon would be better off living in a separate repository outside the responsibility of our core maintainer team.

We feel that at the current state, this addon is very specific to a certain use-case (extracting plain text from the buffer), and it won't benefit many users.

I've got some ideas on how you can achieve #2213 with a different technique, so let's continue the discussion over there.

Tyriar · 2019-07-16T19:37:37Z

@JavaCS3 we're still discussing, I think we do want this. Hold tight 😃

I suggest checking out @mofux's suggestion though in #2213 (comment) as that's probably a better solution to your problem in the end.

JavaCS3 · 2019-07-22T03:58:58Z

Any update?

maxhora · 2019-07-22T11:00:28Z

addons/xterm-addon-serialize/src/SerializeAddon.ts

+    if (!this._terminal) {
+      throw new Error('Cannot use addon until it has been loaded');
+    }
+    const terminalRows = this._terminal.rows;


Does it make sense to serialize whole buffer, but not only visible lines?
( const terminalRows = this._terminal.buffer.length; )

Yes, It used to be like that.
But after I took @Tyriar 's suggestion into account, I changed to this.

Doing the whole buffer could be pretty expensive so opting into more than the minimum is the thinking here.

Tyriar · 2019-08-02T18:37:04Z

@mofux @jerch where do you guys stand on this now? Some form of serialization is needed when we want to retain processes across reloads in VS Code (microsoft/vscode#20013). I think it's a fine compromise to only serialize the active single buffer.

For serializing terminal modes we could add APIs down the track to do that, for example Terminal.modes or something and then this extension could read that is toggle all the relevant modes.

I don't really see how @JavaCS3 could seek backwards without such a feature.

jerch · 2019-08-02T19:18:04Z

@Tyriar Well up to you, I still think that we dont need an addon in the repo base that does not yet add any real value. The PR still feels like a stub to me with much more things to come.

The serializing idea is promising, if we could get more relevant data serialized. Thus I was hoping the discussion would go into this direction with API extensions and implementation here.

Well we can do that in the future and add real value to this code later on. So again, up to you.

Tyriar · 2019-08-02T21:33:41Z

Ok I've merged this into feat/serialize-addon for the time being, @JavaCS3 if you need this to unblock you I suggest to publish the package yourself under an ID until it's more fleshed out. You could also inline the addon into your application which might be easier.

JavaCS3 · 2019-08-03T14:19:26Z

@Tyriar Thanks for your understanding. I almost want to give up. Is there any existing APIs can I use to capture buffers color?

Tyriar · 2019-08-03T15:18:53Z

@JavaCS3 not yet, we just need to agree on the shape of them and add them though.

maxhora · 2019-08-03T16:45:31Z

Hey, thank you @JavaCS3 for your work on serialization plugin and Xterm.JS's team for addons feature. Fluent already started usage of buffer serialization to allow tabs drag and drop (new xterm.js instance should be created and restored to the state before user started tab drag).
What is missed now for the best users experience, it's possibility to serialize and restore colors for each cell.

JavaCS3 · 2019-08-04T03:19:50Z

@Maxris We can progressively improve it!

ZedYeung · 2019-11-05T18:28:45Z

how to deserialize? term.write('what is serialize')?

Tyriar · 2019-11-05T19:21:34Z

@ZedYeung yes deserializing is just writing the serialized result to a new terminal. The idea was to leverage the existing data format and our optimized parser (write) instead of coming up with a new format and a bunch of new APIs

JavaCS3 and others added 3 commits July 10, 2019 21:46

add SerializeAddon

2571a06

fix lint error

3ba6ffe

Merge branch 'master' into master

a71ed7b

Tyriar requested changes Jul 11, 2019

View reviewed changes

JavaCS3 added 2 commits July 12, 2019 23:23

add test files from addons folder

57a8255

add missing api docs and refactor serialize implementation by using l…

31f812c

…ist join instead and some code clean up

Tyriar requested changes Jul 12, 2019

View reviewed changes

jerch reviewed Jul 12, 2019

View reviewed changes

JavaCS3 added 4 commits July 13, 2019 17:25

change exception message

fe3520c

serialize from the bottom of the terminal

bca9024

fix lint error

f859bff

Merge branch 'master' into master

09ede02

maxhora reviewed Jul 22, 2019

View reviewed changes

Merge branch 'master' into master

bb73470

Tyriar changed the base branch from master to feat/serialize-addon August 2, 2019 21:31

Tyriar approved these changes Aug 2, 2019

View reviewed changes

Tyriar added this to the 4.0.0 milestone Aug 2, 2019

Tyriar merged commit 2e237cc into xtermjs:feat/serialize-addon Aug 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add SerializeAddon #2298

add SerializeAddon #2298

JavaCS3 commented Jul 10, 2019 •

edited by Tyriar

Tyriar left a comment

Tyriar Jul 12, 2019

jerch Jul 12, 2019

jerch left a comment

jerch Jul 12, 2019

JavaCS3 Jul 13, 2019

jerch Jul 13, 2019 •

edited

jerch Jul 13, 2019 •

edited

JavaCS3 Jul 13, 2019 •

edited

jerch Jul 13, 2019

jerch Jul 12, 2019

JavaCS3 commented Jul 16, 2019

mofux commented Jul 16, 2019

Tyriar commented Jul 16, 2019

JavaCS3 commented Jul 22, 2019

maxhora Jul 22, 2019 •

edited

JavaCS3 Jul 23, 2019

Tyriar Aug 2, 2019

Tyriar commented Aug 2, 2019 •

edited

jerch commented Aug 2, 2019

Tyriar commented Aug 2, 2019 •

edited

JavaCS3 commented Aug 3, 2019

Tyriar commented Aug 3, 2019

maxhora commented Aug 3, 2019

JavaCS3 commented Aug 4, 2019

ZedYeung commented Nov 5, 2019

Tyriar commented Nov 5, 2019

add SerializeAddon #2298

add SerializeAddon #2298

Conversation

JavaCS3 commented Jul 10, 2019 • edited by Tyriar

Tyriar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerch Jul 13, 2019 • edited

Choose a reason for hiding this comment

jerch Jul 13, 2019 • edited

Choose a reason for hiding this comment

JavaCS3 Jul 13, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JavaCS3 commented Jul 16, 2019

mofux commented Jul 16, 2019

Tyriar commented Jul 16, 2019

JavaCS3 commented Jul 22, 2019

maxhora Jul 22, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tyriar commented Aug 2, 2019 • edited

jerch commented Aug 2, 2019

Tyriar commented Aug 2, 2019 • edited

JavaCS3 commented Aug 3, 2019

Tyriar commented Aug 3, 2019

maxhora commented Aug 3, 2019

JavaCS3 commented Aug 4, 2019

ZedYeung commented Nov 5, 2019

Tyriar commented Nov 5, 2019

JavaCS3 commented Jul 10, 2019 •

edited by Tyriar

jerch Jul 13, 2019 •

edited

jerch Jul 13, 2019 •

edited

JavaCS3 Jul 13, 2019 •

edited

maxhora Jul 22, 2019 •

edited

Tyriar commented Aug 2, 2019 •

edited

Tyriar commented Aug 2, 2019 •

edited