Update to work with xtermjs 4

[coolreader18]

This should still be compatible with xterm v3, though I haven't actually tested it. The onData and onResize methods have been there for a while (typescript typings for xtermjs 3.14), and in theory even the new addon system could be used in xtermjs 3.14, though the old way of passing a terminal to the constructor still works.

[coolreader18]

@wavesoft would you be able to take a look at this?

[wavesoft]

Hello and thanks for the PR @coolreader18 ! Will have a look now

[wavesoft]

Thanks for the PR @coolreader18 !
I tried this with the v3 API and it's not working, so I proposed a few changes. Plus, what do you think about maintaining the old LocalEchoController constructor interface?

[wavesoft]

As a good practice towards V8 optimisation it's a good idea to describe the object shape in the constructor and avoid mutating it later. So I would vote for keeping some defaults here, like so:

    this._termSize = {
      cols: 0,
      rows: 0
    }

[wavesoft]

What do you think instead of breaking the API (in order to maintain some seamless migration from v3 to v4) to move this detection in the constructor?

For instance:

if (!term.onData) {
   // We are dealing with v4 API
} else {
   // We are dealing with v3 API
}

[coolreader18]

I'd think that people using this library would be the ones specifying the version of xterm that they're using, and even if they aren't, passing the terminal to the constructor should still work even in v4.

[arthur5005]

It looks as if the reviewer suggestions have been applied in the latest commit. Can we expect this PR to be merged soon?

[ppacory]

is it ready for xtermjs 4.x ? can't find a release working with xtermjs 4.x

[pre]

How about ditching support for older xtermjs versions altogether? An older version of local-echo can be used with an oder xtermjs, but there is currently no version of local-echo that works with the current version of xtermjs.

[RangerMauve]

I just installed the branch from github and it's working great.

[pre]

I concur - master does not work but the branch of this PR does work.

I suggest a merge.

[jasonolmstead33]

@wavesoft @coolreader18 any update on this? looking to get a local-echo working with xterm >= v4

[RangerMauve]

@jasonolmstead33 I suggest installing from the branch instead of the main repo. 😁

[jasonolmstead33]

@RangerMauve thats what ive done for now, thanks, hoping we can get it as part of master :)

[wavesoft]

Hey guys! Sorry for the long absence, going to land this, run some checks and cut a new release 👍

[RReverser]

@wavesoft It looks like release hasn't been cut yet?

[RReverser]

@wavesoft Ping. Can you please at least add a prebuilt version to the repo? For now I can do npm install github:wavesoft/local-echo, but then need to cd node_modules && npm install && npm run build, which is quite tedious and error-prone.