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.

Sign up for GitHub

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

hooks for custom control sequences #1813

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
4d660de
hooks for custom control sequences
PerBothner Dec 5, 2018
b689745
Cleanups required by tslink.
PerBothner Dec 5, 2018
0311563
Optimize parsing of OSC_STRING to minimize string concatenation.
PerBothner Dec 9, 2018
30a667c
Revert "Optimize parsing of OSC_STRING to minimize string concatenati…
PerBothner Dec 9, 2018
8ad2d1b
Merge remote-tracking branch 'upstream/master'
PerBothner Dec 12, 2018
ffb2708
Revert "Cleanups required by tslink."
PerBothner Dec 12, 2018
53fd04a
Revert "hooks for custom control sequences"
PerBothner Dec 12, 2018
8ceea11
hooks for custom control sequences
PerBothner Dec 12, 2018
6351a5b
Merge branch 'master' into master
Tyriar Dec 13, 2018
5af4626
Change addCsiHandler/addOscHandler to not use Object.assign.
PerBothner Dec 14, 2018
8a5a032
New method _linkHandler used by both addCsiHandler and addOscHandler.
PerBothner Dec 15, 2018
6b65ebd
Various typing and API fixes, doc comments, typing test etc.
PerBothner Dec 15, 2018
7173577
Revert "Various typing and API fixes, doc comments, typing test etc."
PerBothner Dec 21, 2018
9f603a2
Revert "New method _linkHandler used by both addCsiHandler and addOsc…
PerBothner Dec 21, 2018
a4cc87d
Revert "Change addCsiHandler/addOscHandler to not use Object.assign."
PerBothner Dec 21, 2018
2551a53
Revert "hooks for custom control sequences"
PerBothner Dec 21, 2018
d3d3064
Merge remote-tracking branch 'upstream/master'
PerBothner Dec 21, 2018
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
53 changes: 52 additions & 1 deletion src/EscapeSequenceParser.ts
View file
Open in desktop
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,32 @@ 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 newHead = Object.assign(
(params: number[], collect: string): void => {
if (callback(params, collect)) { }
else if (newHead.nextHandler) { newHead.nextHandler(params, collect); }
else { this._csiHandlerFb(collect, params, index); }
},
{ nextHandler: oldHead,
dispose(): void {
let previous = null; let cur = this._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 { this._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 +350,30 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
this._escHandlerFb = callback;
}

addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
const oldHead = this._oscHandlers[ident];
const newHead = Object.assign(
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
(data: string): void => {
if (callback(data)) { }
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
else if (newHead.nextHandler) { newHead.nextHandler(data); }
else { this._oscHandlerFb(ident, data); }
},
{ nextHandler: oldHead,
dispose(): void {
let previous = null; let cur = this._oscHandlers[ident];
for (; cur && cur.nextHandler;
previous = cur, cur = cur.nextHandler) {
if (cur === newHead) {
if (previous) { previous.nextHandler = cur.nextHandler; }
else { this._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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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