Skip to content

async parser handler support #3222

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

Merged
jerch merged 30 commits into from
Mar 17, 2021
Merged

async parser handler support #3222

Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bc10480
early version of async support for CSI and ESC handlers
jerch Jan 20, 2021
0ad9f70
Merge branch 'master' into async_handlers
jerch Jan 22, 2021
4a1bdad
fix tests for async handlers in inputhandler
jerch Jan 22, 2021
adfeffe
deprecate writeSync
jerch Jan 22, 2021
22d777c
make linter happy
jerch Jan 22, 2021
b07ca8b
fix timeout issue on macos
jerch Jan 22, 2021
2f07ed9
simplify stack save in parser
jerch Jan 23, 2021
4ba7aef
Merge branch 'master' into async_handlers
jerch Jan 23, 2021
a9eb1de
Merge branch 'master' into async_handlers
jerch Jan 24, 2021
68b7f88
async parser tests for CSI and ESC
jerch Jan 24, 2021
898a18d
Merge branch 'master' into async_handlers
jerch Feb 7, 2021
d59f4d2
async impl for DCS, DcsParser tests
jerch Feb 8, 2021
041e259
OSC impl, OscParser tests
jerch Feb 8, 2021
d0b5556
update parser hooks api and tests
jerch Feb 8, 2021
5db479c
update public API
jerch Feb 8, 2021
2650211
minor tweaks, API docs update
jerch Feb 15, 2021
bce3269
async OSC/DCS registering/dispose tests on parser instance
jerch Feb 25, 2021
9985ae2
fix linter warning
jerch Feb 25, 2021
d500b0f
inputhandler tests
jerch Feb 25, 2021
6b57921
async handler API tests
jerch Feb 25, 2021
b4635ed
make reset async aware
jerch Feb 25, 2021
26bf1ab
Merge branch 'master' into async_handlers
jerch Feb 25, 2021
cc005a2
multiple changes:
jerch Feb 27, 2021
48a93c3
log a warning if an async handler takes too long
jerch Feb 27, 2021
cdd238d
do not timeout on faulty async handlers, continue with false to give …
jerch Feb 28, 2021
c19cc9b
Merge branch 'master' into async_handlers
jerch Mar 15, 2021
a748845
Merge branch 'master' into async_handlers
jerch Mar 17, 2021
76fdf9d
Merge branch 'async_handlers' of github.com:jerch/xterm.js into async…
jerch Mar 17, 2021
8e7cdd2
interface for inputhandler parse stack, proper typing of LogService.l…
jerch Mar 17, 2021
869c11f
interface for subparser stack saves
jerch Mar 17, 2021
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
474 changes: 244 additions & 230 deletions src/browser/Terminal.test.ts
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/browser/Terminal2.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe('Escape Sequence Files', function(): void {
let content = '';
const OSC_CODE = 12345;
await new Promise(resolve => {
customHandler = term.addOscHandler(OSC_CODE, () => {
customHandler = term.registerOscHandler(OSC_CODE, () => {
// grab terminal viewport content
content = terminalToString(term);
resolve();
Expand Down
11 changes: 7 additions & 4 deletions src/browser/TestUtils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export class TestTerminal extends Terminal {
public get curAttrData(): IAttributeData { return (this as any)._inputHandler._curAttrData; }
public keyDown(ev: any): boolean | undefined { return this._keyDown(ev); }
public keyPress(ev: any): boolean { return this._keyPress(ev); }
public writeP(data: string | Uint8Array): Promise<void> {
return new Promise(r => this.write(data, r));
}
}

export class MockTerminal implements ITerminal {
Expand Down Expand Up @@ -75,16 +78,16 @@ export class MockTerminal implements ITerminal {
public attachCustomKeyEventHandler(customKeyEventHandler: (event: KeyboardEvent) => boolean): void {
throw new Error('Method not implemented.');
}
public addCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean): IDisposable {
public registerCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean | Promise<boolean>): IDisposable {
throw new Error('Method not implemented.');
}
public addDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: IParams) => boolean): IDisposable {
public registerDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: IParams) => boolean | Promise<boolean>): IDisposable {
throw new Error('Method not implemented.');
}
public addEscHandler(id: IFunctionIdentifier, handler: () => boolean): IDisposable {
public registerEscHandler(id: IFunctionIdentifier, handler: () => boolean | Promise<boolean>): IDisposable {
throw new Error('Method not implemented.');
}
public addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
public registerOscHandler(ident: number, callback: (data: string) => boolean | Promise<boolean>): IDisposable {
throw new Error('Method not implemented.');
}
public registerLinkMatcher(regex: RegExp, handler: (event: MouseEvent, uri: string) => boolean | void, options?: ILinkMatcherOptions): number {
Expand Down
8 changes: 4 additions & 4 deletions src/browser/Types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ export interface IPublicTerminal extends IDisposable {
resize(columns: number, rows: number): void;
open(parent: HTMLElement): void;
attachCustomKeyEventHandler(customKeyEventHandler: (event: KeyboardEvent) => boolean): void;
addCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean): IDisposable;
addDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: IParams) => boolean): IDisposable;
addEscHandler(id: IFunctionIdentifier, callback: () => boolean): IDisposable;
addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable;
registerCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean | Promise<boolean>): IDisposable;
registerDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: IParams) => boolean | Promise<boolean>): IDisposable;
registerEscHandler(id: IFunctionIdentifier, callback: () => boolean | Promise<boolean>): IDisposable;
registerOscHandler(ident: number, callback: (data: string) => boolean | Promise<boolean>): IDisposable;
registerLinkMatcher(regex: RegExp, handler: (event: MouseEvent, uri: string) => void, options?: ILinkMatcherOptions): number;
deregisterLinkMatcher(matcherId: number): void;
registerLinkProvider(linkProvider: ILinkProvider): IDisposable;
Expand Down
24 changes: 12 additions & 12 deletions src/browser/public/Terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,28 +292,28 @@ class BufferLineApiView implements IBufferLineApi {
class ParserApi implements IParser {
constructor(private _core: ITerminal) { }

public registerCsiHandler(id: IFunctionIdentifier, callback: (params: (number | number[])[]) => boolean): IDisposable {
return this._core.addCsiHandler(id, (params: IParams) => callback(params.toArray()));
public registerCsiHandler(id: IFunctionIdentifier, callback: (params: (number | number[])[]) => boolean | Promise<boolean>): IDisposable {
return this._core.registerCsiHandler(id, (params: IParams) => callback(params.toArray()));
}
public addCsiHandler(id: IFunctionIdentifier, callback: (params: (number | number[])[]) => boolean): IDisposable {
public addCsiHandler(id: IFunctionIdentifier, callback: (params: (number | number[])[]) => boolean | Promise<boolean>): IDisposable {
return this.registerCsiHandler(id, callback);
}
public registerDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: (number | number[])[]) => boolean): IDisposable {
return this._core.addDcsHandler(id, (data: string, params: IParams) => callback(data, params.toArray()));
public registerDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: (number | number[])[]) => boolean | Promise<boolean>): IDisposable {
return this._core.registerDcsHandler(id, (data: string, params: IParams) => callback(data, params.toArray()));
}
public addDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: (number | number[])[]) => boolean): IDisposable {
public addDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: (number | number[])[]) => boolean | Promise<boolean>): IDisposable {
return this.registerDcsHandler(id, callback);
}
public registerEscHandler(id: IFunctionIdentifier, handler: () => boolean): IDisposable {
return this._core.addEscHandler(id, handler);
public registerEscHandler(id: IFunctionIdentifier, handler: () => boolean | Promise<boolean>): IDisposable {
return this._core.registerEscHandler(id, handler);
}
public addEscHandler(id: IFunctionIdentifier, handler: () => boolean): IDisposable {
public addEscHandler(id: IFunctionIdentifier, handler: () => boolean | Promise<boolean>): IDisposable {
return this.registerEscHandler(id, handler);
}
public registerOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
return this._core.addOscHandler(ident, callback);
public registerOscHandler(ident: number, callback: (data: string) => boolean | Promise<boolean>): IDisposable {
return this._core.registerOscHandler(ident, callback);
}
public addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
public addOscHandler(ident: number, callback: (data: string) => boolean | Promise<boolean>): IDisposable {
return this.registerOscHandler(ident, callback);
}
}
Expand Down
30 changes: 20 additions & 10 deletions src/common/CoreTerminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
this.register(this.optionsService.onOptionChange(key => this._updateOptions(key)));

// Setup WriteBuffer
this._writeBuffer = new WriteBuffer(data => this._inputHandler.parse(data));
this._writeBuffer = new WriteBuffer((data, promiseResult) => this._inputHandler.parse(data, promiseResult));
}

public dispose(): void {
Expand All @@ -125,7 +125,17 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
this._writeBuffer.write(data, callback);
}

/**
* Write data to terminal synchonously.
*
* This method is unreliable with async parser handlers, thus should not
* be used anymore. If you need blocking semantics on data input consider
* `write` with a callback instead.
*
* @deprecated Unreliable, will be removed soon.
Copy link
Copy Markdown
Member

@Tyriar Tyriar Feb 26, 2021

Choose a reason for hiding this comment

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

FYI this will take a little while to adopt in vscode, it's used currently in the local echo and reconnect features. It's also used in a bunch of tests which I think should be harmless to stay.

Copy link
Copy Markdown
Member Author

@jerch jerch Feb 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, we also had many tests with writeSync, but those were easy replaced by a writeP implementation:

xterm.js/src/browser/TestUtils.test.ts

Lines 24 to 26 in 26bf1ab

public writeP(data: string | Uint8Array): Promise<void> {
return new Promise(r => this.write(data, r));
}

I used the same methodP idea for testing the methods along the async callstack:

xterm.js/src/common/InputHandler.test.ts

Lines 50 to 56 in 26bf1ab

public async parseP(data: string | Uint8Array): Promise<void> {
let result: Promise<boolean> | void;
let prev: boolean | undefined;
while (result = this.parse(data, prev)) {
prev = await result;
}
}

For the deeper calls it is a bit more cumbersome to get done right, thus my warnings all over the place not to call those methods directly anymore. Note that it always was dangerous to directly inject chunks at those lower levels while Terminal.write still holds data (will mess up parser states even in sync mode), but now with async it is even more dangerous as it might break the proper continuation. Lets hope that ppl dont dismantle the callstack and call the lower methods directly. Also I dont know of any TS way to prevent that, is there an access pattern like friend in C++?

Final note on writeSync - I did not remove it yet, because it will keep working as before as long as there are no async handlers hooked into the parser. It even keeps working with async handlers if pending data does not trigger any async action (thats what I meant with promise islands - you can still sail in sync water normally at high speed, but now there are promise shallow regions where the boat can crash if not used with writeP). Initially I thought about marking async handlers statically, so the parser would have a "map" of promise regions, but that turned out as not being helpful, as it cannot be decided prehand without actually parsing the data. Thus I went with the simpler handling of "whenever a promise is returned, it is an async handler" (effectively sailing on "runtime sight").

*/
public writeSync(data: string | Uint8Array): void {
console.error('writeSync is unreliable and will be removed soon.');
this._writeBuffer.writeSync(data);
}

Expand Down Expand Up @@ -268,23 +278,23 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
}

/** Add handler for ESC escape sequence. See xterm.d.ts for details. */
public addEscHandler(id: IFunctionIdentifier, callback: () => boolean): IDisposable {
return this._inputHandler.addEscHandler(id, callback);
public registerEscHandler(id: IFunctionIdentifier, callback: () => boolean | Promise<boolean>): IDisposable {
return this._inputHandler.registerEscHandler(id, callback);
}

/** Add handler for DCS escape sequence. See xterm.d.ts for details. */
public addDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: IParams) => boolean): IDisposable {
return this._inputHandler.addDcsHandler(id, callback);
public registerDcsHandler(id: IFunctionIdentifier, callback: (data: string, param: IParams) => boolean | Promise<boolean>): IDisposable {
return this._inputHandler.registerDcsHandler(id, callback);
}

/** Add handler for CSI escape sequence. See xterm.d.ts for details. */
public addCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean): IDisposable {
return this._inputHandler.addCsiHandler(id, callback);
public registerCsiHandler(id: IFunctionIdentifier, callback: (params: IParams) => boolean | Promise<boolean>): IDisposable {
return this._inputHandler.registerCsiHandler(id, callback);
}

/** Add handler for OSC escape sequence. See xterm.d.ts for details. */
public addOscHandler(ident: number, callback: (data: string) => boolean): IDisposable {
return this._inputHandler.addOscHandler(ident, callback);
public registerOscHandler(ident: number, callback: (data: string) => boolean | Promise<boolean>): IDisposable {
return this._inputHandler.registerOscHandler(ident, callback);
}

protected _setup(): void {
Expand Down Expand Up @@ -322,7 +332,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
if (!this._windowsMode) {
const disposables: IDisposable[] = [];
disposables.push(this.onLineFeed(updateWindowsModeWrappedState.bind(null, this._bufferService)));
disposables.push(this.addCsiHandler({ final: 'H' }, () => {
disposables.push(this.registerCsiHandler({ final: 'H' }, () => {
updateWindowsModeWrappedState(this._bufferService);
return false;
}));
Expand Down
Loading