Closed feamsr00 closed 5 years ago
jerch
commented
6 years ago somewhat offtopic: For custom escape codes there is imho no way to hook into the default parser atm. You can still get those custom codes working with your own parser beforehand. The ANSI spec allows custom escape codes for OSC and DCS commands. Unless you know what you are doing - stick with those to stay compatible with the default parser.
parisk
commented
6 years ago Hello @feamsr00; good points.
loadAddon in favor of a new cleaner API. Hopefully I will open a PR today 😄 
feamsr00
commented
6 years ago @jerch Thanks for the insight. I was worried about that. This kinda goes back to the API question. Even without a special API for add-ons per se, It's not clear how one could merely call a "Terminal.setParser()" (especially considering no such method seems to exist) to set that dependency. Moreover, it would be helpful if there was some interface contract for Parser. Are there any custom parsers in the wild, or other examples on how to move forward? Might I suggest a feature to at least emit parser/ctrl events?
Tyriar
commented
6 years ago @feamsr00 currently there are no real external addons, everything is built in. The main reason we have not actively encouraged the community building addons right now is all they would be able to do is call the standard API of which you can't really do much interesting stuff that would call for an addon.
An addon could of course call into private members but it will likely break in the future as we don't commit to keeping private APIs stable. The addons packaged with the repo have a better chance of being stable at any given time.
It seems to be just asking for collisions. Is there any other namespace registration or dic facility?
100%, I really want to isolate the addons in a better and more consistent way.
Indeed it's not documented how to create an add-on, but we can fix this in 3.0.
I'd prefer we didn't rush in to this but instead fully think out https://github.com/xtermjs/xterm.js/issues/808 and make that the standard way to build an addon. We're also mid way through the discussions on what the future of loadAddon is, best let that bake for a while to see if there are any bumps in the road.
Are there any custom parsers in the wild, or other examples on how to move forward?
Swapping out renderers is something we've talked about before, not parsers though as the parser is quite complex. I think there are plans to allow extending of the parser with https://github.com/xtermjs/xterm.js/issues/576
@feamsr00 as you actually have some things you want to build, your feedback would be invaluable in https://github.com/xtermjs/xterm.js/issues/808. The hope here was to move a bunch of built in functionality to use this nice and modular component API and then make it available for use externally in addons.
Tyriar
commented
6 years ago Let's defer this so we can get v3 out
feamsr00
commented
6 years ago And so, v3 is out now (🎉). What are the plans for the addon system?
Tyriar
commented
6 years ago No update, we still want to do it eventually though. I'm particularly keen on separating the built-in addons from the core repo.
Tyriar
commented
6 years ago Find below a proposal for proper addons that I wrote up while in the air ✈️ 😄. Any feedback would be greatly appreciated as if go ahead and make mistakes they'll be hard to change.
/cc @xtermjs/core @jerch @vincentwoo @chabou @amejia1 @jluk
xterm.js addons are components that use the xterm.js API to provide additional functionality. They follow a particular structure to make developing an addon convenient.
The difference between writing functionality in an addon and just using the API is that the Terminal is aware of addons and can provides additional hooks/functionality that you don't normally get when just programming against the API. It's also easy to develop and share your work in a consistent way with the community.
While it's certainly possible to write addons in JavaScript, TypeScript is encouraged due to the additional compiler type checks and first-class TS support in the core library.
Since xterm.js is a living project and breakages do occur (on major versions), there's a chance an addon could break.
Instead of what was done previously, registering addons in the static Terminal.applyAddon function, addons are now passed into Terminal in the constructor as an optional 2nd argument:
interface ITerminalAddonConstructor<T> {
// Used to make sure the same addon isn't instantiated twice
readonly NAME: string;
new(terminal: number): T;
}
class Terminal {
constructor(
options: ITerminalOptions,
addons?: ITerminalAddonConstructor[]
)
}This allows different terminals to have a different set of addons and provides a convenient way to load the addons for each of them. Also note that the typeof an ITerminalAddon is provided
import { Addon1 } from '...';
import { Addon2 } from '...';
const addons = [Addon1, Addon2];
const terminal = new Terminal({}, addons);This module contains declaration files that define the interface of an addon. The only reason this module exists is so addons do not need to depend on the xterm module (and create a circular dependency).
xterm.d.ts
^
|
-------------------
^ ^ ^
| | |
xterm xterm-fit ...This can be published to npm in a similar way that the vscode API is published, the source of truth remains in the xterm.js repo (and is referenced directly by xterm.js repo), but is published as a separate module for addons to consume.
Addons define a constructor which is triggered during Terminal's constructor, other hooks should be added using Terminal.on.
interface ITerminalAddon {
/**
* The minimum version of xterm.js that this addon is compatible with, in the format
* `[major, minor, patch]`. if this is higher than the version being used it will throw an
* `Error` when the addon is loaded.
*/
minimumVersion: [number, number, number];
/**
* The maximum version of xterm.js that this addon is compatible with, in the format
* `[major, minor, patch]`. If this is defined and lower than the version being used it will
* throw an `Error` when the addon is loaded.
* TODO: Should we bother with this? Are people going to bother updating the addon to add this?
*/
maximumVersion?: [number, number, number];
// This can be a starting point, with more hooks added later
constructor(terminal: ITerminal);
// Terminal will call this when Terminal.dispose is called, this can clean up any
// references/elements it needs to to shut down gracefully and eventually be
// used to turn addons off without disposing the terminal
dispose(): void;
// We could add more hooks here if we want, or just let addons listen in on internals using
// `Terminal.on` (which wouldn't be as nicely typed). See xtermjs/xterm.js#808
}To actually call functions on the addon you need to acquire the addon instance of a terminal. This can be done by leveraging the relatively complex Terminal.getAddon which takes an addon constructor and returns the addon instance. The internals of getAddon should be easy to implement:
interface ITerminalAddonConstructor<T> {
new(terminal: number): T;
}
interface ITerminalAddon {
a(): void;
}
class SearchAddon implements ITerminalAddon {
findNext(): void {}
a(): void {}
}
class FitAddon implements ITerminalAddon {
fit(): void {}
a(): void {}
}
class Terminal {
getAddon<T extends ITerminalAddon>(addonConstructor: ITerminalAddonConstructor<T>): T {
// Search internal addons list for that which matches addonConstructor
return <T><any>1;
}
}
const term = new Terminal();
const search = term.getAddon(SearchAddon);
search.findNext(); // Strongly typed simply by using the ctor
const fit = term.getAddon(FitAddon);
fit.fit(); // Strongly typed simply by using the ctor
term.getAddon({}); // errorBundled addons will each move to the following repos and have published npm modules:
This will have many benefits like:
Moving these into their own repos should also encourage us to open up the API so these core addons are no longer leveraging private API that could break. This would also be a good opportunity to add an API test suite which is a set of tests which use only the public API.
"engines": { "xterm": "..." } in package.json? We probably can't pull this out in a nice way without enforcing how things are built.Tyriar
commented
6 years ago Had a chat with @mofux on Slack, we could do dependencies between addons by declaring the dependency names on ITerminalAddon/ITerminalAddonConstructor that would be used to block activation until deps are met. A peer dependency could be used to help ensure the dependency is installed.
ghost
commented
6 years ago The min and/or max version checks are redundant when using the package.json peer dependency option? I'd prefer to let npm handle semver ranges i.e. 3.4.0 - 3.5.0 over adding { min: [3,4,0], max: [3,5,0] } to my add-on's class. Doing dependency mangement seems to be at odds with the unix philosophy of do one thing and do it well.
ghost
commented
6 years ago How would you disable an add-on?
Tyriar
commented
6 years ago The min and/or max version checks are redundant when using the package.json peer dependency option
Further chats with @mofux came to the same conclusion 😉
How would you disable an add-on?
You can't do that now, but if we wanted to support that it would probably look something like this:
class Terminal {
disposeAddon<T extends ITerminalAddon>(addonConstructor: ITerminalAddonConstructor<T>);
}
interface ITerminalAddon {
dispose?(): void;
}If I wanted to develop an add-on with optional dependency on other add-ons, it would be nice to be able to call getAddon('someOptionalAddon') to get an add-on instance. With that in mind, I'd prefer the following for registering.
class FitAddon implements ITerminalAddon {
name: 'fit'
fit(): void
}
xterm.useAddon(FitAddon, function optionalCallback(fit) {
fit.fit();
});
// And maybe allow an optional name argument
xterm.useAddon('myfit', FitAddon); // Using the name 'fit' would throw here
const fit = xterm.getAddon('myfit');An overloaded getAddon would work just as well.
Tyriar
commented
6 years ago @pro-src this is how I'm imagining inter-dependencies working:
// fit.ts
import { ITerminal, ITerminalAddon } from 'xterm-base';
class FitAddon implements ITerminalAddon {
// Still not 100% sure if we need this
name: 'fit'
constructor(private _terminal: ITerminal) {
}
fit(): void {
// Do stuff with this._terminal
}
}
// myFit.ts
import { ITerminal, ITerminalAddon } from 'xterm-base';
import { FitAddon } from 'xterm-addon-fit';
class MyFitAddon implements ITerminalAddon {
name: 'myFit'
// Will peerDependencies just work here and allow access to them?
dependencies: [ FitAddon ]
constructor(private _terminal: ITerminal) {
}
myFit(): void {
const fit = this._terminal.getAddon(FitAddon)
}
}
// app.ts
import { ITerminal, ITerminalAddon } from 'xterm-base';
import { FitAddon } from 'xterm-addon-fit';
import { MyFitAddon } from 'xterm-addon-myfit';
// FitAddon will initialize first, MyFitAddon will follow since it's
// dependencies are met
const term = new Terminal({}, [MyFitAddon, FitAddon]);
term.getAddon(FitAddon).fit();
term.getAddon(MyFitAddon).myFit();LGTM :+1:
Just a few questions that pop into my mind:
class XYAddon extends ZAddon ... still register correctly? If so under which name?fitAddon and not breaking with the dependencies of others? Not sure if we really need this, I've seen plugin systems that allow "injections" while keeping everything else intact.ghost
commented
6 years ago @Tyriar
this is how I'm imagining inter-dependencies working: ...
cool, that does seem to work for optional deps as well.
const deps = [FitAddon];
Try { deps.push(require('optionalDep')); } catch(e) {}Would a class XYAddon extends ZAddon ... still register correctly? If so under which name?
Yeah I don't see why that wouldn't work, I was thinking the name property idea was needed to be used as an identifier so the name would be overridden by XYAddon.
Is an addon extensible in place, thus changing lets say fitAddon and not breaking with the dependencies of others? Not sure if we really need this, I've seen plugin systems that allow "injections" while keeping everything else intact.
Not totally clear to me what you mean here?
chabou
commented
6 years ago Really better for Hyper to specify add-ons for an instance :+1:
Can we imagine to have an addAddon(MyFitAddon) function or better a setAddons([MyFitAddon, FitAddon]) to change add-ons at runtime? It would be awesome to set them like options. We really want to ensure that our hyper plugins are hot-(un)loadable.
In our case, when a plugin is added/removed, we'll ask each plugin its xterm's add-on array, merge them and pass it to xterm existing instances (and futur new ones).
parisk
commented
6 years ago @Tyriar's proposal (https://github.com/xtermjs/xterm.js/issues/1128#issuecomment-394142177) covers me completely.
Also, deferring compatibility checks for xterm.js or other add-ons to peerDependencies is ideal.
Last, my opinion about disabling add-ons and hot loading/unloading should be left for a later stage, so we can focus on the basics first.
Updating the add-on system as @Tyriar proposed and standardising the structure of add-ons and the process of creating and publishing them is already a great step forward. Unloading add-ons and hot loading is a plus, but we can have this conversation after we put something out to the world 😄.
jerch
commented
6 years ago @Tyriar My second question is also covered by the name property thing.
Regarding interdependent addons - how are dependencies resolved? Do we get a MRO like or cycling problem here? I dont think people will start to create tons of dependent addons so this might be only an academical problem lol.
Tyriar
commented
6 years ago Can we imagine to have an addAddon(MyFitAddon) function or better a setAddons([MyFitAddon, FitAddon]) to change add-ons at runtime? It would be awesome to set them like options. We really want to ensure that our hyper plugins are hot-(un)loadable.
@chabou I haven't considered removal/disposal of addons much in this examples but that could be added after an MVP, which would then naturally lead to a method to be able to remove/add to an existing terminal. If we want to be able to add an addon after new Terminal, that will add some complexity we will need to deal with as the open event may have fired which I imagine will be a critical to many addons.
This could be as simple as Terminal.isAttached boolean or Terminal.state properties.
Regarding interdependent addons - how are dependencies resolved? Do we get a MRO like or cycling problem here? I dont think people will start to create tons of dependent addons so this might be only an academical problem lol.
@jerch I was thinking at the "activation time" do this:
do
Iterate over addons, activating any with dependencies matched
while (an addon was activated)I think this simple algorithm should cover all the bases, circular dependent addons would never activate but that's desired 😄
Tyriar
commented
6 years ago I added a dispose method to the proposal above:
interface ITerminalAddon {
// Terminal will call this when Terminal.dispose is called, this can clean up any
// references/elements it needs to to shut down gracefully and eventually be
// used to turn addons off without disposing the terminal
dispose(): void;
}I realized we needed this after seeing attach hold on to resources after calling term.dispose() in the demo.
Tyriar
commented
6 years ago Another idea:
Consider that an ITerminalAddonApi is passed into the addon instead of ITerminal:
class FitAddon implements ITerminalAddon {
name: 'fit'
constructor(private _api: ITerminalAddonApi) {
}
fit(): void {
this._api.terminal.doStuff();
}
}The actual API implementation could look something like this:
interface ITerminalAddonApi {
readonly terminal: ITerminal
}
class TerminalAddonApi implements ITerminalAddonApi {
constructor(private _terminal: ITerminal) {
}
public get terminal(): ITerminal {
return this._terminal;
}
public dispose(): void {
this._terminal = null;
}
}This will discourage the addon holding onto references of ITerminal, allowing them to be freed when the terminal is disposed (as Terminal.dispose will trigger TerminalAddonApi.dispose).
Tyriar
commented
6 years ago I renamed the https://github.com/xtermjs/xterm-addon-ligatures repo to align with the naming scheme mentioned here
princjef
commented
6 years ago Sorry for being a little late to the game :smile: @Tyriar's proposal overall is a great step toward making xterm.js more easily extensible in a structured way.
A few thoughts/considerations as I've read through the proposal and discussion (in no particular order). None of these are a huge deal or dealbreakers for me, but I wanted to get my thoughts and impressions out there:
The xterm-base package seems unnecessary to me. Taking systems such as grunt/gulp as an example, it's already common for plugins to take a peerDependency on the main package that they are extending. This is useful because it serves as a guard against addons being loaded for a version of xterm that won't work with them without actually introducing another copy of the package in the dependency tree. When I need to depend on types for a package listed as a peer dependency (or want to test with it), I will generally also include a compatible version of that package as a devDependency so that there is a copy available for development. This takes care of importing the types at development and runtime without the introduction of a new package.
The two main drawbacks to having an xterm-base package that everyone depends on are (1) that it forces a two step publishing chain whenever the types change and (2) it loses the version alignment guarantees that a peerDependency provides (since an addon may depend on xterm-base@4 while the version of xterm used aligns with xterm-base@5).
The maximumVersion and minimumVersion are redundant with using a peerDependency and require extra maintenance. A peerDependency alone should be sufficient here. This seems to have already been agreed upon but not yet updated in the proposal so I figured I'd mention it as well.
Regarding circular references between addons and the terminal, is the holding of circular references really a problem here? As long as any underlying timers/listeners/etc. are shut down as part of the disposal, the garbage collector should be able to find and clean up the isolated group of circular references. Even if the circular reference is a concern, is it not sufficient to simply remove the references to the addons on the terminal side after they have had their dispose() called? Then there is still a reference on the terminal, but the addon referencing it has no references itself, so the chain is cleaned up.
I'm not wild about mandating that addons be classes that are added/referenced by the class itself, but it seems like the best/only way to get Typescript to automatically give back the right type information when using something string-based without passing addons in as an object instead of an array (which itself runs into problem if/when dynamic registration is possible). With that said, I don't think the name field is providing any value with the current design since names aren't used to access the addon and would be susceptible to naming conflicts (especially if they aren't used as the primary identifier for the addon). I'd vote to remove them for now.
The issue of dependencies between addons and load order is interesting. The specification of the dependencies of an addon is deceptive because it doesn't absolve the user of having to installing the depended-upon package (dependencies between addons would need to be peer dependencies to avoid duplication problems).
At that point, I think it makes sense to just make the user specify all of the addons they want to load and the order they want them to load (via the ordering in the array). It makes the behavior much easier to understand from the outside and gives users maximum flexibility. It also makes things simpler when people want to do hot unloading of addons, as the current proposal seems like it would require xterm to forbid or silently ignore the removal of an addon which still has dependents in the addon list. It's easier for users to shoot themselves in the foot but also easier to get themselves out of it. This approach is typically how I typically see things done in shell plugin systems, for instance.
In this case, addons could still potentially specify their dependencies as part of their interface (for potential future capabilities), but with this approach there is no issue with circular dependencies between addons causing an infinite loop or addons being silently dropped since xterm would no longer need to build up a dependency tree out of a graph. If an addon is unable to function because its dependencies were loaded out of order, it should fail loudly anyway.
Tyriar
commented
5 years ago @princjef thanks a bunch of the feedback, very valuable!
ITerminalAddonConstructor<T> methodTyriar
commented
5 years ago I've put together a branch that implements the proposal and converts web links and attach to the new format, you can view it here (the demo works! 😮): https://github.com/xtermjs/xterm.js/compare/master...Tyriar:1128_addons?expand=1
Here are the modules:
Here's the API:
class Terminal {
loadAddon<T extends ITerminalAddon>(addonConstructor: ITerminalAddonConstructor<T>): T;
disposeAddon<T extends ITerminalAddon>(addonConstructor: ITerminalAddonConstructor<T>): void;
getAddon<T extends ITerminalAddon>(addonConstructor: ITerminalAddonConstructor<T>): T;
}
export interface ITerminalAddonConstructor<T extends ITerminalAddon> {
new(terminal: Terminal): T;
}
export interface ITerminalAddon {
/**
* This function includes anything that needs to happen to clean up when
* the addon is being disposed.
*/
dispose(): void;
}You might notice the Terminal.disposeAddon API, I think one of the best things about doing this is managing an addons lifecycle so that they can be swapped in and out without recreating the Terminal. One idea I had was to move the WebGL renderer to an addon, primarily as a means to test it in a really low risk way (opt-in and monkey patching private APIs), and potentially keep it in that form as there are big benefits in not loading the WebGL renderer if you're not using it.
Let me know what you think 😃
mofux
commented
5 years ago I like the API in general 👍
(1) I'm wondering - how would I pass a configuration to an addon? As far as I can see the only way is by using a separate call to the addon instance to some custom method like init:
const myAddon = term.loadAddon(MyAddon);
myAddon.init({ /*...config*/ })IMO passing a configuration to a plugin needs a better story than this. Maybe adding an optional config as the second parameter to the term.loadAddon method, which then passes it to the addon constructor?
// addon definition
class MyAddon implements ITerminalAddon {
constructor(terminal, config) {
}
dispose() {
}
}
// register addon
const myAddon = term.loadAddon(MyAddon, { /*...config*/ });which for the attach addon could look like this:
term.loadAddon(AttachAddon, { socket: mySocket });and you are done.
(2) Would it make sense to add a lifecycle hook to when the terminal got opened (term.open)? I think addons that need access to screen dimensions or the DOM would benefit from that hook.
Tyriar
commented
5 years ago @mofux
(2) Would it make sense to add a lifecycle hook to when the terminal got opened (term.open)? I think addons that need access to screen dimensions or the DOM would benefit from that hook.
Great idea, onOpen, onDomAttach? (after change from https://github.com/xtermjs/xterm.js/issues/1505)
(1) I'm wondering - how would I pass a configuration to an addon?
I thought about this a little bit but thought it might get too complicated. I just put together a prototype:
The API looks like this:
export class Terminal {
loadAddon<T extends ITerminalAddon>(addonConstructor: ITerminalAddonConstructor<T>): T;
loadAddonWithConfig<T extends ITerminalAddonWithConfig<K>, K>(addonConstructor: ITerminalAddonWithConfigConstructor<T, K>, config: K): T;
}
export interface ITerminalAddonConstructor<T extends ITerminalAddon> {
new(terminal: Terminal): T;
}
export interface ITerminalAddonWithConfigConstructor<T extends ITerminalAddonWithConfig<K>, K> {
new(terminal: Terminal, config: K): T;
}
export interface ITerminalAddon {
dispose(): void;
}
// The types must be duplicated, extending ITerminalAddon means this could
// be passed into Terminal.loadAddon
export interface ITerminalAddonWithConfig<K> {
dispose(): void;
}The addon impl:
export interface IWebLinksAddonConfig {
handler?: (event: MouseEvent, uri: string) => void;
options?: ILinkMatcherOptions;
}
export class WebLinksAddonWithConfig implements ITerminalAddonWithConfig<IWebLinksAddonConfig> {
// config can be omitted here without warning
constructor(private _terminal: Terminal, config: IWebLinksAddonConfig) {
}
...
}Diff: https://github.com/Tyriar/xterm.js/commit/9822953d012ab167a0ad17a557595aea124ac97a
Thoughts?
jerch
commented
5 years ago Yupp really nice API so far. :+1:
I like the idea of lifecycle hooks, this way ppl can act on specific states of the terminal (and even cover the offscreen terminal case when we support it someday in the future). Things that come to my mind (Ive never written an addon myself, so the usefulness of the list is questionable):
dispose might be already to late...Tyriar
commented
5 years ago afterInit
Well this is essentially the constructor, the initial plan was to give an array of addons to Terminal.ctor but that seems to be overcomplicating things, especially since nothing will be rendered until open is called anyway.
Hmm - does an addon need cleanup room before the whole object tree gets disposed? dispose might be already to late...
Can you give an example?
Tyriar
commented
5 years ago Also I've been thinking maybe we don't even need Terminal.disposeAddon and Terminal.getAddon. If you want to manage the addon's lifecycle yourself (disabling at runtime) or need to interact with it later (eg. search), just keep a reference from loadAddon, Wrapping the dispose function on load makes disposeAddon redundant imo.
jerch
commented
5 years ago Can you give an example?
Well I have currently no idea of the dispose order, so this is quite theoretical. What if an addon relies on another component state (something internal or another addon), and has to cleanup things, but the other component is already gone? Example that comes to my mind is an addon that serializes the current terminal state to be able to resume from later on.
With a beforeDispose (could be triggered as first task of terminal.dispose) any addon can safely prepare for the upcoming dispose while the env is still intact.
Tyriar
commented
5 years ago Example that comes to my mind is an addon that serializes the current terminal state to be able to resume from later on.
Hmm, let's add if someone requests 🙂, that might be needed if we add addon dependencies
mofux
commented
5 years ago @Tyriar Sorry, I forget about the convenience of typed config options in my previous post. Couldn't we let the user instantiate the addon, and then let him pass the addon instance to term.loadAddon?
// addon definition
class MyAddon implements ITerminalAddon {
// addon constructor
constructor(config: IMyAddonConfig) {
}
// called when initiating the plugin
onLoad(terminal: Terminal) {
}
// called when term.open() was called
onOpen(terminal: Terminal) {
}
// called on term.dispose()
dispose(terminal: Terminal) {
}
}
// create addon instance (note: the consumer instanciates the addon, not us)
const myAddon = new MyAddon({ /*config*/ });
term.loadAddon(myAddon);So onLoad would essentially be the hook that would handle what was previously done in the constructor. This would also have the advantage that one could create a class-less addon at runtime (not sure if it's useful though):
term.loadAddon({
onLoad(terminal) {
},
dispose(terminal) {
}
});Update
Another advantage with this approach is that one could use the same Addon instance for multiple terminals. One example would be addons that maintain an internal cache, which could be easily reused with this approach (constructor called once, onLoad called for every instance)
What do you think?
Tyriar
commented
5 years ago Couldn't we let the user instantiate the addon, and then let him pass the addon instance to term.loadAddon?
I suppose we could do away with all the constructor stuff if we're only going to have a single Terminal.loadAddon API. Certainly seems simpler.
Another advantage with this approach is that one could use the same Addon instance for multiple terminals. One example would be addons that maintain an internal cache, which could be easily reused with this approach (constructor called once, onLoad called for every instance)
This is something I was trying to avoid, this would mean that there are some addons that work with multiple terminals and some that don't. It might also encourage doing this:
Take the future webgl addon for example, it's all ready big enough without adding support to manage multiple terminals within the same addon, especially when it just works when there is only every a single terminal loaded.
How about we do something like this to disallow it?
loadAddon(addon: ITerminalAddon): void {
if (addon.__isLoaded) {
throw ...
}
...
addon.__isLoaded = true;
}Idea for bundled addons: https://github.com/xtermjs/xterm.js/pull/1714#issuecomment-454898319
Tyriar
commented
5 years ago New model is much simpler:
class Terminal {
/**
* Loads an addon into this instance of xterm.js.
* @param addon The addon to load.
*/
loadAddon(addon: ITerminalAddon): void;
}
export interface ITerminalAddon {
/**
* This is called when the addon is activated within xterm.js.
*/
activate(terminal: Terminal): void;
/**
* This function includes anything that needs to happen to clean up when
* the addon is being disposed.
*/
dispose(): void;
}This lets the embedder construct the addon however they wish, and we do away with the complexity of the ctor system and the additional methods. If someone wants to keep a reference around just hold on to the addon after you register:
term.loadAddon(new WebLinksAddon());
const attachAddon = new AttachAddon();
term.loadAddon(attachAddon);
// ...
attachAddon.attach(...);I'm leaning towards not exposing a bunch of events on the addons themselves but instead allow addons to register their own events during the activate event. The addon author may need to check the state of the terminal before doing it's thing is all:
const addon = {
activate(term: Terminal): void {
if (term.element) {
// it's open
} else {
// handle open event
term.onOpen(() => ...);
}
}
}It's worth pointing out that I wasn't planning on exporting addons on window (see https://github.com/xtermjs/xterm.js/issues/2015).
Hello all!
I'm trying to create a new addon and the documentation doesn't seem to be terribly clear on what it takes to go from 0-60.
For example, 1) exactly how does the API for add-ons differ from the nominal Terminal API? Does it at all? Will it?* Additionally, 2) is directly modifying Terminals' prototype the most appropriate way for add-ons to register functionality? It seems to be just asking for collisions. Is there any other namespace registration or dic facility? (I suppose maybe even just adding the addon object eg Terminal.MyAddon.method() for the simplest way, but surely Terminal.addon('MyAddon').method() is much more sound). 3) Also, it doesn't seem to be clear how to actually add a third party addon, as the names have all been hardcoded... (I have taken to extending Terminal and widening loadAddon (
static loadAddon(String): void;)) I am making assumptions, since the docs don't say if loadAddon is only for private (non 3rd party) use.I have reviewed some of the existing add-ons, but they don't seem to have the most consistent implementations.
I'm creating a couple add-ons (at least one for custom escape codes) because I wish to add discrete functionality to xTerm.js. It seems far more architecturally sound to do that instead of just making a big abstraction layer on top* of xTerm.
Details