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

hooks for custom control sequences #1813

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion src/EscapeSequenceParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { ParserState, ParserAction, IParsingState, IDcsHandler, IEscapeSequenceParser } from './Types';
import { IDisposable } from 'xterm';
import { Disposable } from './common/Lifecycle';

/**
Expand Down Expand Up @@ -41,7 +42,7 @@ export class TransitionTable {
* @param action parser action to be done
* @param next next parser state
*/
add(code: number, state: number, action: number | null, next: number | null): void {
add(code: number, state: number, action: number | null, next: number | null): void {
this.table[state << 8 | code] = ((action | 0) << 4) | ((next === undefined) ? state : next);
}

Expand Down Expand Up @@ -303,6 +304,33 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
this._executeHandlerFb = callback;
}

addCsiHandler(flag: string, callback: (params: number[], collect: string) => boolean): IDisposable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yupp this will work without penalty if no other handler is attached 👍
One thing though: can we make this management code more general usable, like in a private method called for CSI, OSC etc. when needed. This way the code is cleaner without duplication and feels less monkey patching, and can also be used for ESC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See commit 8a5a032 "New method _linkHandler used by both addCsiHandler and addOscHandler." I had some problems with the type-checking, and the resulting code is a bit ugly, but not too bad, I think.

const index = flag.charCodeAt(0);
const oldHead = this._csiHandlers[index];
const parser = this;
const newHead =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be documented: Inserting the additional handlers at top. (since it is somewhat surprising compared to many other event systems)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 6b65ebd.
(I added detailed doc comments in xterm.d.ts, with just a "link" in Terminal.ts.)

(params: number[], collect: string): void => {
if (! callback(params, collect)) {
if (newHead.nextHandler) { newHead.nextHandler(params, collect); }
else { this._csiHandlerFb(collect, params, index); }
}
};
newHead.nextHandler = oldHead;
newHead.dispose = function (): void {
let previous = null; let cur = parser._csiHandlers[index];
for (; cur && cur.nextHandler;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this still work with cycles, like someone added a handler twice by accident? Also an cycle might prevent the autoclean up by the .dispose method, imho the dispose chain of additional handlers should be called there, too.
Ah well it creates always a new function object, so cycles should not be an issue (I think).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it should be ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yupp found no issues with it (handler is recreated anyway).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner with this management method. 👍

previous = cur, cur = cur.nextHandler) {
if (cur === newHead) {
if (previous) { previous.nextHandler = cur.nextHandler; }
else { parser._csiHandlers[index] = cur.nextHandler; }
break;
}
}
};
this._csiHandlers[index] = newHead;
return newHead;
}

setCsiHandler(flag: string, callback: (params: number[], collect: string) => void): void {
this._csiHandlers[flag.charCodeAt(0)] = callback;
}
Expand All @@ -323,6 +351,32 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
this._escHandlerFb = callback;
}

addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
const oldHead = this._oscHandlers[ident];
const parser = this;
const newHead =
(data: string): void => {
if (! callback(data)) {
if (newHead.nextHandler) { newHead.nextHandler(data); }
else { this._oscHandlerFb(ident, data); }
}
};
newHead.nextHandler = oldHead;
newHead.dispose =
function (): void {
let previous = null; let cur = parser._oscHandlers[ident];
for (; cur && cur.nextHandler;
previous = cur, cur = cur.nextHandler) {
if (cur === newHead) {
if (previous) { previous.nextHandler = cur.nextHandler; }
else { parser._oscHandlers[ident] = cur.nextHandler; }
break;
}
}
};
this._oscHandlers[ident] = newHead;
return newHead;
}
setOscHandler(ident: number, callback: (data: string) => void): void {
this._oscHandlers[ident] = callback;
}
Expand Down
12 changes: 10 additions & 2 deletions src/InputHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
* @license MIT
*/

import { IInputHandler, IDcsHandler, IEscapeSequenceParser, IBuffer, IInputHandlingTerminal } from './Types';
import { IVtInputHandler, IDcsHandler, IEscapeSequenceParser, IBuffer, IInputHandlingTerminal } from './Types';
import { C0, C1 } from './common/data/EscapeSequences';
import { CHARSETS, DEFAULT_CHARSET } from './core/data/Charsets';
import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CODE_INDEX, DEFAULT_ATTR, NULL_CELL_CHAR, NULL_CELL_WIDTH, NULL_CELL_CODE } from './Buffer';
import { FLAGS } from './renderer/Types';
import { wcwidth } from './CharWidth';
import { EscapeSequenceParser } from './EscapeSequenceParser';
import { ICharset } from './core/Types';
import { IDisposable } from 'xterm';
import { Disposable } from './common/Lifecycle';

/**
Expand Down Expand Up @@ -112,7 +113,7 @@ class DECRQSS implements IDcsHandler {
* Refer to http://invisible-island.net/xterm/ctlseqs/ctlseqs.html to understand
* each function's header comment.
*/
export class InputHandler extends Disposable implements IInputHandler {
export class InputHandler extends Disposable implements IVtInputHandler {
private _surrogateFirst: string;

constructor(
Expand Down Expand Up @@ -465,6 +466,13 @@ export class InputHandler extends Disposable implements IInputHandler {
this._terminal.updateRange(buffer.y);
}

addCsiHandler(flag: string, callback: (params: number[], collect: string) => boolean): IDisposable {
return this._parser.addCsiHandler(flag, callback);
}
addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
return this._parser.addOscHandler(ident, callback);
}

/**
* BEL
* Bell (Ctrl-G).
Expand Down
6 changes: 5 additions & 1 deletion src/Terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
* http://linux.die.net/man/7/urxvt
*/

import { IInputHandlingTerminal, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, CharacterJoinerHandler, IBufferLine } from './Types';
import { IInputHandlingTerminal, IInputHandler, IViewport, ICompositionHelper, ITerminalOptions, ITerminal, IBrowser, ILinkifier, ILinkMatcherOptions, CustomKeyEventHandler, LinkMatcherHandler, CharData, CharacterJoinerHandler, IBufferLine } from './Types';
import { IMouseZoneManager } from './ui/Types';
import { IRenderer } from './renderer/Types';
import { BufferSet } from './BufferSet';
Expand Down Expand Up @@ -1287,6 +1287,10 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II
this.refresh(0, this.rows - 1);
}

public get inputHandler(): IInputHandler {
return this._inputHandler;
}

/**
* Scroll the display of the terminal by a number of pages.
* @param pageCount The number of pages to scroll (negative scrolls up).
Expand Down
10 changes: 10 additions & 0 deletions src/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ export interface IInputHandler {
ESC ~ */ setgLevel(level: number): void;
}

/*
* An InputHandler for VT-style terminals
*/
export interface IVtInputHandler extends IInputHandler {
addCsiHandler(flag: string, callback: (params: number[], collect: string) => boolean): IDisposable;
addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable;
}

export interface ILinkMatcher {
id: number;
regex: RegExp;
Expand Down Expand Up @@ -492,6 +500,8 @@ export interface IEscapeSequenceParser extends IDisposable {
setCsiHandler(flag: string, callback: (params: number[], collect: string) => void): void;
clearCsiHandler(flag: string): void;
setCsiHandlerFallback(callback: (collect: string, params: number[], flag: number) => void): void;
addCsiHandler(flag: string, callback: (params: number[], collect: string) => boolean): IDisposable;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerch are the strings here future proof with your upcoming changes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tyriar, @PerBothner: Not future proof, its still a subject to change, but imho good to go for now. I will address this with one of the typed array transitions to come.
Not sure about including this in public API yet, might be better to go unofficial until the transition is done (will not before 3.11 though due to the JS Array buffer).

addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable;

setEscHandler(collectAndFlag: string, callback: () => void): void;
clearEscHandler(collectAndFlag: string): void;
Expand Down
5 changes: 4 additions & 1 deletion src/public/Terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Terminal as ITerminalApi, ITerminalOptions, IMarker, IDisposable, ILinkMatcherOptions, ITheme, ILocalizableStrings } from 'xterm';
import { ITerminal } from '../Types';
import { ITerminal, IInputHandler } from '../Types';
import { Terminal as TerminalCore } from '../Terminal';
import * as Strings from '../Strings';

Expand All @@ -15,6 +15,9 @@ export class Terminal implements ITerminalApi {
this._core = new TerminalCore(options);
}

public get inputHandler(): IInputHandler {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this to expose the following interface (pending @jerch's comments on whether the types are future proof):

class Terminal {
  addCsiHandler(flag: string, callback: (params: number[], collect: string) => boolean): IDisposable;
  addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable;
}

You'll then call through to _core, see addDisposableListener for what I mean here.

You should also merge IVtInputHandler back into IInputHandler after doing this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, they will most likely change type to a borrowed typed array, but not until 3.11 since I have to wait for the JS Array buffer to be gone. So not sure if we should include it yet in the public API, well marking them "experimental" or "unstable" would work for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified things as suggested (as I understand it).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this becomes API we'll need an entry also in xterm.d.ts as well as a test in fixtures/typings-test/typings-test.ts. The .d.ts file is a TypeScript declaration file and that defines the entire API we want to expose, the test file just makes sure we don't regress the declaration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 6b65ebd.

return (this._core as TerminalCore).inputHandler;
}
public get element(): HTMLElement { return this._core.element; }
public get textarea(): HTMLTextAreaElement { return this._core.textarea; }
public get rows(): number { return this._core.rows; }
Expand Down