-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Custom addons #1128
Comments
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. |
Hello @feamsr00; good points.
|
@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? |
@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.
100%, I really want to isolate the addons in a better and more consistent way.
I'd prefer we didn't rush in to this but instead fully think out #808 and make that the standard way to build an addon. We're also mid way through the discussions on what the future of
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 #576 @feamsr00 as you actually have some things you want to build, your feedback would be invaluable in #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. |
Let's defer this so we can get v3 out |
And so, v3 is out now (🎉). What are the plans for the addon system? |
No update, we still want to do it eventually though. I'm particularly keen on separating the built-in addons from the core repo. |
Find below a proposal for proper addons that I wrote up while in the air /cc @xtermjs/core @jerch @vincentwoo @chabou @amejia1 @jluk Better Addons Proposalxterm.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. VersioningSince xterm.js is a living project and breakages do occur (on major versions), there's a chance an addon could break. Loading addonsInstead of what was done previously, registering addons in the static 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 import { Addon1 } from '...';
import { Addon2 } from '...';
const addons = [Addon1, Addon2];
const terminal = new Terminal({}, addons); xterm-base npm moduleThis 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
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. InterfacesAddons define a constructor which is triggered during 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 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({}); // error The current "bundled" addonsBundled 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. Thoughts
|
Had a chat with @mofux on Slack, we could do dependencies between addons by declaring the dependency names on |
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. |
How would you disable an add-on? |
Further chats with @mofux came to the same conclusion 😉
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 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 |
@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 👍 Just a few questions that pop into my mind:
|
cool, that does seem to work for optional deps as well.
|
Yeah I don't see why that wouldn't work, I was thinking the
Not totally clear to me what you mean here? |
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.
Can you give an example? |
Also I've been thinking maybe we don't even need |
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. |
Hmm, let's add if someone requests 🙂, that might be needed if we add addon dependencies |
@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 // 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 term.loadAddon({
onLoad(terminal) {
},
dispose(terminal) {
}
}); Update What do you think? |
I suppose we could do away with all the constructor stuff if we're only going to have a single
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: xterm.js/src/addons/attach/attach.ts Line 23 in 509ce5f
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: #1714 (comment) |
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 |
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
The text was updated successfully, but these errors were encountered: