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

Add a way to plugin a custom control sequence handler #1176

Closed
ztmr opened this issue Jan 1, 2018 · 8 comments · Fixed by #1853
Closed

Add a way to plugin a custom control sequence handler #1176

ztmr opened this issue Jan 1, 2018 · 8 comments · Fixed by #1853
Labels
area/api area/parser help wanted type/enhancement Features or improvements to existing features

Comments

@ztmr
Copy link

ztmr commented Jan 1, 2018

Story:

  • I want to implement a DEC VT420+ sequence that is not implemented in the original xterm (= we don't want it to be a part of xterm.js by default) -- in theory, any of the sequences mentioned here
  • I want to implement a custom sequence that is supported only by the application displayed on the terminal: links Support hyperlink ansi escapes #1134, displaying pictures, custom svg graphics, maybe some kind of buttons (a [styled?] link that sends something back to the terminal, rather than opening in a new tab) and eventually being able to execute some code (being careful about security, so probably doing so in a sandbox and having to explicitly enable this and only for whitelisted functions)

Notes:

  • it is possible to do these things by hacking Parser and InputProcessor but I think most of it should be pluggable from outside
  • I am not sure if the parser is implemented correctly as it is missing some prefixes and suffixes (*, %, at minimum) and it is somehow expecting that these can be only prefix or only suffix, never both (should we check this? Not sure how this is related to broken escape sequence parser states #145.
  • some of these handlers would need to access the internals (terminal, canvas, etc.) -- although i can imagine some kind of abstraction (renderSvg(...), renderWhatever(...), setStateProp(prop, value), ensureBuffer(id), ...) the question is if these would be enough (not sure how to cleanly handle Implement double-height/width sequences (DECSWL/DECDWL/DECDHL) #1175 for example)
  • for example, there are things like DECSASD/DECSCPP/DECSNLS that can be implemented by using Terminal.resize or DECSSDT to set a new terminal "state" property (status line in this case) that can be displayed outside of terminal below the canvas if enabled in terminal properties

I can do some work on this eventually but need to agree on the approach to take.

@Tyriar
Copy link
Member

Tyriar commented Jan 2, 2018

I think @parisk's approach to enabling #576 was going to be allowing custom control sequence handling. It would definitely be cool to allow this to be pluggable imo but we need to come up with the right API.

Related: #808, #1128

@Tyriar
Copy link
Member

Tyriar commented Jun 8, 2018

@jerch how easy is this now that we have the new parser?

@Tyriar Tyriar added type/enhancement Features or improvements to existing features help wanted area/parser labels Jun 8, 2018
@jerch
Copy link
Member

jerch commented Jun 8, 2018

Well as long as the custom escape sequence follows the CSI, OSC or DCS scheme it is a matter of writing a handler and registering it. At the current state it would still need some mechanism to expose the registering functionality since it is only propagated to InputHandler internally.

@Tyriar Tyriar changed the title Add a way to plugin a custom control sequence handling Add a way to plugin a custom control sequence handler Jun 27, 2018
@PerBothner
Copy link
Contributor

It seems there are a couple of different ways to do it, none difficult, but we need to agree on the right API. For concreteness, let's assume we want to expose setOscHandlerFallback to addons.

One way is to add a setOscHandlerFallback method to Terminal. The implementation is trivial: Add setOscHandlerFallback to InputHandler, and have Terminal call the InputHandler method which calls the method in EscapeSequenceParser.

The problem with this approach is it hardwires the concepts of VT-style terminals (specifically OSC and CSI commands) into Terminal.

How about something like the following. The idea is to add a new IVtInputHandler nterface for "VT-style terminals":

export interface IVtInputHandler extends IInputHandler {
  setOscHandlerFallback(callback: (identifier: number, data: string) => void): void;
  ...
}
export interface IInputHandlingTerminal extends IEventEmitter {
  ...
   getInputHandler(): IInputHandler;
}
export class InputHandler extends Disposable implements IVtInputHandler {
  ...
  public setOscHandlerFallback(callback: (identifier: number, data: string) => void): void {
    this._parser.setOscHandlerFallback(callback);
  }
}
export class Terminal extends EventEmitter implements ITerminal, IDisposable, IInputHandlingTerminal {
   public getInputHandler(): IInputHandler { return _inputHandler; }
}

So instead of calling term.setOscHandlerFallback(...) directly, an addon would call term.getInputHandler().setOscHandlerFallback(...) instead.

(As may be clear, I don't have a lot of TypeScript experience, so this may be all wrong.)

@PerBothner
Copy link
Contributor

Note my previous comment focuses on the classes and interfaces. Using setOscHandlerFallback for addons may be undesirable because it is awkward if multiple addons want to add handlers. More useful would be setting a specific handler:

setOscHandler(ident: number, callback: (data: string) => void)

Also, rather than having setOscHandlerFallback in IVtInputHandler it might be more useful to "add":

addOscHandlerFallback(callback: (identifier: number, data: string) => boolean): void

This would loop through a stack of fallback handlers until one returns true.

@jerch
Copy link
Member

jerch commented Nov 26, 2018

@PerBothner Yes I think directly exposing set... would be enough, not sure about support for multiple handlers for the same "event" at once, we had the discussion before and decided to go with just one at a time. (If one really needs this it still can be done by hooking in an event dispatch handler.)
The fallback variants are meant to catch all ESC/CSI/OSC "events" that are not covered by a real handler, I did this more for debug purpose than for real usage logic.

Imho the tricky part starts in the handler logic, as it would need to have access to Terminal internals (most handlers will change some state of it). We have not decided how to do this, atm we only expose an easy to use terminal API for integrators, but not yet a programmer's API. We currently soft migrate the internals into better separated submodules/folders (we have several issues for that, e.g. #1515, also the way addons work will be a big part of the deal), main goal is to get the core parts separated from DOM/browser engine related stuff to be able to run the inner terminal parts w'o any DOM association (easier testing, ability to run the core in headless envs etc). This is a burden we carry around from the original implementation. We lift it step by step, still it hinders us from going fully API modularized yet.

@PerBothner
Copy link
Contributor

The attached patch works, in that it lets me do things like:

        xterm.inputHandler.setOscHandler(8, function(str) {
            console.log("saw OSC 8 "+str);
        });

Does this seems like a good way to go? Is having a separate IVtInputHandler desirable, or would it be better to just add setOscHandler to plain IInputHandler?

osc-hook.txt

@PerBothner
Copy link
Contributor

Having to setXxx functions in the API may not be enough. For example, DomTerm has many custom escapes of the form CSI+param+u. There is also a standard/builtin handler for plain CSI+'u'. Suppose we want to add support for one of the DomTerm escapes without breaking the builtin handler. The simplest way to do this is if the API exports both setCsiHandler and getCsiHandler:

let oldHandler = inputHandler.getCsiHandler('u');
inputHandler.setCsiHandler('u', function(params, collect) {
    if (myAddedEscape(params, collect)) myAddedAction();
    else oldHandler(paramas, collect);
});

This has the advantage that it doesn't add any builtin overhead. However, it's a little clunky to use. Worse, it doesn't handler _removing) handlers well, especially if multiple addons may be added and removed independently. That argues for a traditional addHandler/removeHandler API:

 inputHandler.addCsiHandler('u', function(params, collect) {
    if (myAddedEscape(params, collect)) {
        myAddedAction();
        return true; // handled
     }
    else return false; // fall back to previous handler
});

PerBothner added a commit to PerBothner/xterm.js that referenced this issue Dec 5, 2018
This fixes (at least partially) issue xtermjs#1176
"Add a way to plugin a custom control sequence handler".
@Tyriar Tyriar added the area/api label Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/parser help wanted type/enhancement Features or improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants