From 9b26b190f934162369dede5c9f89998f400a5670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 20:15:04 +0200 Subject: [PATCH 01/21] flow control handling --- src/interfaces.ts | 3 +++ src/terminal.test.ts | 48 ++++++++++++++++++++++++++++++++++ src/terminal.ts | 61 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/src/interfaces.ts b/src/interfaces.ts index c86215b60..ead819836 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -115,6 +115,9 @@ export interface IPtyForkOptions { uid?: number; gid?: number; encoding?: string; + handleFlowControl?: boolean; + flowPause?: string; + flowResume?: string; } export interface IPtyOpenOptions { diff --git a/src/terminal.test.ts b/src/terminal.test.ts index e3aa46da4..83df06079 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -5,6 +5,14 @@ import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; import { UnixTerminal } from './unixTerminal'; +import { IPtyForkOptions } from './interfaces'; +import { ArgvOrCommandLine } from './types'; + +interface ITerminalCtor { + new(file?: string, args?: ArgvOrCommandLine, opt?: IPtyForkOptions): WindowsTerminal | UnixTerminal; +} + +const terminalConstructor = (process.platform === 'win32') ? WindowsTerminal : UnixTerminal; let terminalCtor: WindowsTerminal | UnixTerminal; if (process.platform === 'win32') { @@ -13,6 +21,7 @@ if (process.platform === 'win32') { terminalCtor = require('./unixTerminal'); } + describe('Terminal', () => { describe('constructor', () => { it('should do basic type checks', () => { @@ -22,4 +31,43 @@ describe('Terminal', () => { ); }); }); + + // FIXME: adopt for windows + describe('automatic flow control', () => { + it('should respect ctor flow control options', () => { + const pty = new terminalConstructor('/bin/bash', [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); + // write got replaced + assert.equal((pty as any)._realWrite !== null, true); + // correct values of flowPause and flowResume + assert.equal((pty as any)._flowPause, 'abc'); + assert.equal((pty as any)._flowResume, '123'); + }); + it('should do flow control automatically', (done) => { + const pty = new terminalConstructor('/bin/bash', [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); + const read: string[] = []; + pty.on('data', data => read.push(data)); + pty.on('pause', () => read.push('paused')); + pty.on('resume', () => read.push('resumed')); + setTimeout(() => pty.write('1'), 100); + setTimeout(() => pty.write('PAUSE'), 200); + setTimeout(() => pty.write('2'), 300); + setTimeout(() => pty.write('RESUME'), 400); + setTimeout(() => pty.write('3'), 500); + setTimeout(() => { + // read should contain ['resumed', '', '1', 'paused', 'resumed', '2', '3'] + // import here: no data should be delivered between 'paused' and 'resumed' + assert.deepEqual(read.slice(2), ['1', 'paused', 'resumed', '2', '3']); + done(); + }, 1000); + }); + it('should enable/disable automatic flow control', () => { + const pty = new terminalConstructor('/bin/bash', []); + // write got not yet replaced + assert.equal((pty as any)._realWrite, null); + pty.enableFlowHandling(); + assert.equal((pty as any)._realWrite !== null, true); + pty.disableFlowHandling(); + assert.equal((pty as any)._realWrite, null); + }); + }); }); diff --git a/src/terminal.ts b/src/terminal.ts index 7de63e197..83ee1da76 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -11,6 +11,14 @@ import { ITerminal, IPtyForkOptions } from './interfaces'; export const DEFAULT_COLS: number = 80; export const DEFAULT_ROWS: number = 24; +/** + * Default messages to indicate PAUSE/RESUME for automatic flow control. + * To avoid conflicts with rebound XON/XOFF control codes, + * the sequences can be customized in `IPtyForkOptions`. + */ +const FLOW_PAUSE = '\x13'; // defaults to XOFF +const FLOW_RESUME = '\x11'; // defaults to XON + export abstract class Terminal implements ITerminal { protected _socket: Socket; protected _pid: number; @@ -26,6 +34,9 @@ export abstract class Terminal implements ITerminal { protected _writable: boolean; protected _internalee: EventEmitter; + private _realWrite: (data: string) => void = null; + private _flowPause: string; + private _flowResume: string; public get pid(): number { return this._pid; } @@ -47,6 +58,56 @@ export abstract class Terminal implements ITerminal { this._checkType('uid', opt.uid ? opt.uid : null, 'number'); this._checkType('gid', opt.gid ? opt.gid : null, 'number'); this._checkType('encoding', opt.encoding ? opt.encoding : null, 'string'); + + // setup flow control handling + if (opt.handleFlowControl) { + this._flowPause = opt.flowPause || FLOW_PAUSE; + this._flowResume = opt.flowResume || FLOW_RESUME; + this.enableFlowHandling(); + } + } + + /** + * Enable automatic flow control handling. + * Instead of relying on XON/XOFF directly we pause/resume + * the underlying socket. Main advantage is that flow control will + * still work even when XON/XOFF is not available. As a downside it + * does not act immediately, instead the OS pty buffer and node's socket buffer + * fill up first before the slave program gets paused. + * Note that `flowPause` and `flowResume` must be in line with the + * messages sent by the terminal emulator to indicate PAUSE/RESUME. + * @param flowPause String to pause the pty. Defaults to XOFF (x13). + * @param flowResume String to resume the pty. Defaults to XON (x11). + */ + public enableFlowHandling(flowPause: string = this._flowPause, flowResume: string = this._flowResume): void { + if (this._realWrite) { + return; + } + this._realWrite = this.write; + this.write = (data: string) => { + // PAUSE/RESUME messages are not forwarded to the pty + if (data === flowPause) { + this.pause(); + return; + } + if (data === flowResume) { + this.resume(); + return; + } + // normal pty.write + this._realWrite(data); + }; + } + + /** + * Disable automatic flow control handling. + */ + public disableFlowHandling(): void { + if (!this._realWrite) { + return; + } + this.write = this._realWrite; + this._realWrite = null; } private _checkType(name: string, value: any, type: string): void { From 7093ba292cdfd4951d4afecfb3f2bf26ab87e1be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 20:22:16 +0200 Subject: [PATCH 02/21] remove ctor interface --- src/terminal.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 83df06079..47de31083 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -5,12 +5,6 @@ import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; import { UnixTerminal } from './unixTerminal'; -import { IPtyForkOptions } from './interfaces'; -import { ArgvOrCommandLine } from './types'; - -interface ITerminalCtor { - new(file?: string, args?: ArgvOrCommandLine, opt?: IPtyForkOptions): WindowsTerminal | UnixTerminal; -} const terminalConstructor = (process.platform === 'win32') ? WindowsTerminal : UnixTerminal; From f93f8c7dd3037a5a0101fc7366693b4dbc955c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 20:32:46 +0200 Subject: [PATCH 03/21] fix typo --- src/terminal.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 47de31083..97250d750 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -49,7 +49,7 @@ describe('Terminal', () => { setTimeout(() => pty.write('3'), 500); setTimeout(() => { // read should contain ['resumed', '', '1', 'paused', 'resumed', '2', '3'] - // import here: no data should be delivered between 'paused' and 'resumed' + // important here: no data should be delivered between 'paused' and 'resumed' assert.deepEqual(read.slice(2), ['1', 'paused', 'resumed', '2', '3']); done(); }, 1000); From 1492a811385d7a54a81504e1f64ebdb8d6e3c2b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 20:37:42 +0200 Subject: [PATCH 04/21] fix windows call --- src/terminal.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 97250d750..24f26393f 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -7,6 +7,7 @@ import { WindowsTerminal } from './windowsTerminal'; import { UnixTerminal } from './unixTerminal'; const terminalConstructor = (process.platform === 'win32') ? WindowsTerminal : UnixTerminal; +const IS_WIN = process.platform === 'win32'; let terminalCtor: WindowsTerminal | UnixTerminal; if (process.platform === 'win32') { @@ -29,7 +30,7 @@ describe('Terminal', () => { // FIXME: adopt for windows describe('automatic flow control', () => { it('should respect ctor flow control options', () => { - const pty = new terminalConstructor('/bin/bash', [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); + const pty = new terminalConstructor(IS_WIN ? 'cmd.exe' : '/bin/bash', [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); // write got replaced assert.equal((pty as any)._realWrite !== null, true); // correct values of flowPause and flowResume @@ -37,7 +38,7 @@ describe('Terminal', () => { assert.equal((pty as any)._flowResume, '123'); }); it('should do flow control automatically', (done) => { - const pty = new terminalConstructor('/bin/bash', [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); + const pty = new terminalConstructor(IS_WIN ? 'cmd.exe' : '/bin/bash', [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); const read: string[] = []; pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); @@ -55,7 +56,7 @@ describe('Terminal', () => { }, 1000); }); it('should enable/disable automatic flow control', () => { - const pty = new terminalConstructor('/bin/bash', []); + const pty = new terminalConstructor(IS_WIN ? 'cmd.exe' : '/bin/bash', []); // write got not yet replaced assert.equal((pty as any)._realWrite, null); pty.enableFlowHandling(); From d17e038548e0c3d553082badc19b7cfd2af13aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 20:40:10 +0200 Subject: [PATCH 05/21] cleanup tests --- src/terminal.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 24f26393f..8caa8dbc0 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -7,7 +7,7 @@ import { WindowsTerminal } from './windowsTerminal'; import { UnixTerminal } from './unixTerminal'; const terminalConstructor = (process.platform === 'win32') ? WindowsTerminal : UnixTerminal; -const IS_WIN = process.platform === 'win32'; +const SHELL = (process.platform === 'win32') ? 'cmd.exe' : '/bin/bash'; let terminalCtor: WindowsTerminal | UnixTerminal; if (process.platform === 'win32') { @@ -27,10 +27,9 @@ describe('Terminal', () => { }); }); - // FIXME: adopt for windows describe('automatic flow control', () => { it('should respect ctor flow control options', () => { - const pty = new terminalConstructor(IS_WIN ? 'cmd.exe' : '/bin/bash', [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); + const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); // write got replaced assert.equal((pty as any)._realWrite !== null, true); // correct values of flowPause and flowResume @@ -38,7 +37,7 @@ describe('Terminal', () => { assert.equal((pty as any)._flowResume, '123'); }); it('should do flow control automatically', (done) => { - const pty = new terminalConstructor(IS_WIN ? 'cmd.exe' : '/bin/bash', [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); + const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); const read: string[] = []; pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); @@ -56,7 +55,7 @@ describe('Terminal', () => { }, 1000); }); it('should enable/disable automatic flow control', () => { - const pty = new terminalConstructor(IS_WIN ? 'cmd.exe' : '/bin/bash', []); + const pty = new terminalConstructor(SHELL, []); // write got not yet replaced assert.equal((pty as any)._realWrite, null); pty.enableFlowHandling(); From 67d2128f5e73884456b1e8a974935cb8e22decfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 21:07:44 +0200 Subject: [PATCH 06/21] changes to write: - default write is provided by Terminal with flowcontrol - derived classes attach their write implementation to _writeMethod --- src/terminal.test.ts | 8 +++--- src/terminal.ts | 58 +++++++++++++++++++++--------------------- src/unixTerminal.ts | 7 +++-- src/windowsTerminal.ts | 3 +++ 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 8caa8dbc0..6df60e18e 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -30,9 +30,7 @@ describe('Terminal', () => { describe('automatic flow control', () => { it('should respect ctor flow control options', () => { const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); - // write got replaced - assert.equal((pty as any)._realWrite !== null, true); - // correct values of flowPause and flowResume + assert.equal((pty as any)._handleFlowControl, true); assert.equal((pty as any)._flowPause, 'abc'); assert.equal((pty as any)._flowResume, '123'); }); @@ -58,9 +56,9 @@ describe('Terminal', () => { const pty = new terminalConstructor(SHELL, []); // write got not yet replaced assert.equal((pty as any)._realWrite, null); - pty.enableFlowHandling(); + pty.enableFlowControl(); assert.equal((pty as any)._realWrite !== null, true); - pty.disableFlowHandling(); + pty.disableFlowControl(); assert.equal((pty as any)._realWrite, null); }); }); diff --git a/src/terminal.ts b/src/terminal.ts index 83ee1da76..03995cd64 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -34,9 +34,10 @@ export abstract class Terminal implements ITerminal { protected _writable: boolean; protected _internalee: EventEmitter; - private _realWrite: (data: string) => void = null; + protected _writeMethod: (data: string) => void = () => {}; private _flowPause: string; private _flowResume: string; + private _handleFlowControl: boolean; public get pid(): number { return this._pid; } @@ -60,11 +61,25 @@ export abstract class Terminal implements ITerminal { this._checkType('encoding', opt.encoding ? opt.encoding : null, 'string'); // setup flow control handling - if (opt.handleFlowControl) { - this._flowPause = opt.flowPause || FLOW_PAUSE; - this._flowResume = opt.flowResume || FLOW_RESUME; - this.enableFlowHandling(); + this._handleFlowControl = !!(opt.handleFlowControl); + this._flowPause = opt.flowPause || FLOW_PAUSE; + this._flowResume = opt.flowResume || FLOW_RESUME; + } + + public write(data: string): void { + if (this._handleFlowControl) { + // PAUSE/RESUME messages are not forwarded to the pty + if (data === this._flowPause) { + this.pause(); + return; + } + if (data === this._flowResume) { + this.resume(); + return; + } } + // everything else goes to the real pty + this._writeMethod(data); } /** @@ -79,35 +94,21 @@ export abstract class Terminal implements ITerminal { * @param flowPause String to pause the pty. Defaults to XOFF (x13). * @param flowResume String to resume the pty. Defaults to XON (x11). */ - public enableFlowHandling(flowPause: string = this._flowPause, flowResume: string = this._flowResume): void { - if (this._realWrite) { - return; + public enableFlowControl(flowPause?: string, flowResume?: string): void { + if (flowPause) { + this._flowPause = flowPause; } - this._realWrite = this.write; - this.write = (data: string) => { - // PAUSE/RESUME messages are not forwarded to the pty - if (data === flowPause) { - this.pause(); - return; - } - if (data === flowResume) { - this.resume(); - return; - } - // normal pty.write - this._realWrite(data); - }; + if (flowResume) { + this._flowResume = flowResume; + } + this._handleFlowControl = true; } /** * Disable automatic flow control handling. */ - public disableFlowHandling(): void { - if (!this._realWrite) { - return; - } - this.write = this._realWrite; - this._realWrite = null; + public disableFlowControl(): void { + this._handleFlowControl = false; } private _checkType(name: string, value: any, type: string): void { @@ -178,7 +179,6 @@ export abstract class Terminal implements ITerminal { this._socket.once(eventName, listener); } - public abstract write(data: string): void; public abstract resize(cols: number, rows: number): void; public abstract destroy(): void; public abstract kill(signal?: string): void; diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 99c308df4..a71d5802c 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -157,6 +157,9 @@ export class UnixTerminal extends Terminal { this._close(); this.emit('close'); }); + + // attach write method + this._writeMethod = (data: string) => this._socket.write(data); } /** @@ -214,10 +217,6 @@ export class UnixTerminal extends Terminal { return self; } - public write(data: string): void { - this._socket.write(data); - } - public destroy(): void { this._close(); diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index d31bfcccf..97340e1e4 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -118,6 +118,9 @@ export class WindowsTerminal extends Terminal { this._readable = true; this._writable = true; + + // attach write method + this._writeMethod = (data: string) => this._defer(() => this._agent.inSocket.write(data)); } /** From 550f57b21038cf61d6204150da6fd1321e283338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 22:18:13 +0200 Subject: [PATCH 07/21] readme section for flow control --- README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/README.md b/README.md index d07b07882..fcf65c74c 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,32 @@ This happens when PowerShell is launched with no `SystemRoot` environment variab This error can occur due to anti-virus software intercepting winpty from creating a pty. To workaround this you can exclude this file from your anti-virus scanning `node-pty\build\Release\winpty-agent.exe` +### Flow Control + +Automatic flow control can be enabled by either providing `handleFlowControl = true` in the constructor options or calling +`enableFlowControl` later on: + +```js +const PAUSE = '\x13'; // XOFF +const RESUME = '\x11'; // XON + +const ptyProcess = pty.spawn(shell, [], {handleFlowControl: true}); + +// flow control in action +ptyProcess.write(PAUSE); // pty will block and pause the slave program +... +ptyProcess.write(RESUME); // pty will enter flow mode and resume the slave program + +// temporarily disable/re-enable flow control +pty.disableFlowControl(); +... +pty.enableFlowControl(); +``` + +By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that +use these control codes for different purposes the messages can be customized as `flowPause: string` and +`flowResume: string` in the constructor options or arguments to `enableFlowControl`. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. + ## pty.js This project is forked from [chjj/pty.js](https://github.com/chjj/pty.js) with the primary goals being to provide better support for later Node.JS versions and Windows. From e7ea7c4bceaea49ea473895b6b6ae18ea7ae2dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 22:21:02 +0200 Subject: [PATCH 08/21] fix headline size --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fcf65c74c..5686c49fe 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ This happens when PowerShell is launched with no `SystemRoot` environment variab This error can occur due to anti-virus software intercepting winpty from creating a pty. To workaround this you can exclude this file from your anti-virus scanning `node-pty\build\Release\winpty-agent.exe` -### Flow Control +## Flow Control Automatic flow control can be enabled by either providing `handleFlowControl = true` in the constructor options or calling `enableFlowControl` later on: From c8a1259c475a8d481602008d36ff84202ddaa8e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 22:22:35 +0200 Subject: [PATCH 09/21] fix naming --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5686c49fe..288d1dcfd 100644 --- a/README.md +++ b/README.md @@ -129,9 +129,9 @@ ptyProcess.write(PAUSE); // pty will block and pause the slave program ptyProcess.write(RESUME); // pty will enter flow mode and resume the slave program // temporarily disable/re-enable flow control -pty.disableFlowControl(); +ptyProcess.disableFlowControl(); ... -pty.enableFlowControl(); +ptyProcess.enableFlowControl(); ``` By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that From d561b8bcb45de4c47cfeb2547b27db47acb5d9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 22:45:13 +0200 Subject: [PATCH 10/21] alter test for windows --- src/terminal.test.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 9e47ccd94..f51c4b8a0 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -41,15 +41,19 @@ describe('Terminal', () => { pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); pty.on('resume', () => read.push('resumed')); - setTimeout(() => pty.write('1'), 100); - setTimeout(() => pty.write('PAUSE'), 200); - setTimeout(() => pty.write('2'), 300); - setTimeout(() => pty.write('RESUME'), 400); - setTimeout(() => pty.write('3'), 500); + setTimeout(() => pty.write('1'), 500); + setTimeout(() => pty.write('PAUSE'), 600); + setTimeout(() => pty.write('2'), 700); + setTimeout(() => pty.write('RESUME'), 800); + setTimeout(() => pty.write('3'), 900); setTimeout(() => { - // read should contain ['resumed', '', '1', 'paused', 'resumed', '2', '3'] // important here: no data should be delivered between 'paused' and 'resumed' - assert.deepEqual(read.slice(2), ['1', 'paused', 'resumed', '2', '3']); + if (process.platform === 'win32') { + // cmd.exe always clears to the end of line? + assert.deepEqual(read.slice(2), ['1\u001b[0K', 'paused', 'resumed', '2\u001b[0K', '3\u001b[0K']); + } else { + assert.deepEqual(read.slice(2), ['1', 'paused', 'resumed', '2', '3']); + } done(); }, 1000); }); From 198f19434b5e60ebf95efa1a79b140f8eb29857b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 22:47:27 +0200 Subject: [PATCH 11/21] slice test from end --- src/terminal.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index f51c4b8a0..971b09925 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -50,9 +50,9 @@ describe('Terminal', () => { // important here: no data should be delivered between 'paused' and 'resumed' if (process.platform === 'win32') { // cmd.exe always clears to the end of line? - assert.deepEqual(read.slice(2), ['1\u001b[0K', 'paused', 'resumed', '2\u001b[0K', '3\u001b[0K']); + assert.deepEqual(read.slice(-5), ['1\u001b[0K', 'paused', 'resumed', '2\u001b[0K', '3\u001b[0K']); } else { - assert.deepEqual(read.slice(2), ['1', 'paused', 'resumed', '2', '3']); + assert.deepEqual(read.slice(-5), ['1', 'paused', 'resumed', '2', '3']); } done(); }, 1000); From 11c10caabb7e7f7f4522d56c1171b0f17b304a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 23:04:23 +0200 Subject: [PATCH 12/21] raise timeouts for MacOS build --- src/terminal.test.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 971b09925..16cb4c7b3 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -35,17 +35,18 @@ describe('Terminal', () => { assert.equal((pty as any)._flowPause, 'abc'); assert.equal((pty as any)._flowResume, '123'); }); - it('should do flow control automatically', (done) => { + it('should do flow control automatically', function(done) { + this.timeout(3000); const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); const read: string[] = []; pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); pty.on('resume', () => read.push('resumed')); - setTimeout(() => pty.write('1'), 500); - setTimeout(() => pty.write('PAUSE'), 600); - setTimeout(() => pty.write('2'), 700); - setTimeout(() => pty.write('RESUME'), 800); - setTimeout(() => pty.write('3'), 900); + setTimeout(() => pty.write('1'), 1000); + setTimeout(() => pty.write('PAUSE'), 1200); + setTimeout(() => pty.write('2'), 1400); + setTimeout(() => pty.write('RESUME'), 1600); + setTimeout(() => pty.write('3'), 1800); setTimeout(() => { // important here: no data should be delivered between 'paused' and 'resumed' if (process.platform === 'win32') { @@ -55,7 +56,7 @@ describe('Terminal', () => { assert.deepEqual(read.slice(-5), ['1', 'paused', 'resumed', '2', '3']); } done(); - }, 1000); + }, 2000); }); it('should enable/disable automatic flow control', () => { const pty = new terminalConstructor(SHELL, []); From f1cb1c4aa3bfb6d4069904640b5ff3397927cbdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 23:09:34 +0200 Subject: [PATCH 13/21] fixing missing typedefs --- src/terminal.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 16cb4c7b3..5074e66ea 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -35,18 +35,18 @@ describe('Terminal', () => { assert.equal((pty as any)._flowPause, 'abc'); assert.equal((pty as any)._flowResume, '123'); }); - it('should do flow control automatically', function(done) { - this.timeout(3000); + it('should do flow control automatically', function(done: Function): void { + this.timeout(3500); const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); const read: string[] = []; pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); pty.on('resume', () => read.push('resumed')); - setTimeout(() => pty.write('1'), 1000); - setTimeout(() => pty.write('PAUSE'), 1200); - setTimeout(() => pty.write('2'), 1400); - setTimeout(() => pty.write('RESUME'), 1600); - setTimeout(() => pty.write('3'), 1800); + setTimeout(() => pty.write('1'), 2000); + setTimeout(() => pty.write('PAUSE'), 2200); + setTimeout(() => pty.write('2'), 2400); + setTimeout(() => pty.write('RESUME'), 2600); + setTimeout(() => pty.write('3'), 2800); setTimeout(() => { // important here: no data should be delivered between 'paused' and 'resumed' if (process.platform === 'win32') { @@ -56,7 +56,7 @@ describe('Terminal', () => { assert.deepEqual(read.slice(-5), ['1', 'paused', 'resumed', '2', '3']); } done(); - }, 2000); + }, 3200); }); it('should enable/disable automatic flow control', () => { const pty = new terminalConstructor(SHELL, []); From d52b97f44991d662adfbfff8678acc4e94499a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Wed, 22 May 2019 23:19:54 +0200 Subject: [PATCH 14/21] longer timeout in test --- src/terminal.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 5074e66ea..cd1b81155 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -36,17 +36,17 @@ describe('Terminal', () => { assert.equal((pty as any)._flowResume, '123'); }); it('should do flow control automatically', function(done: Function): void { - this.timeout(3500); + this.timeout(10000); const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); const read: string[] = []; pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); pty.on('resume', () => read.push('resumed')); - setTimeout(() => pty.write('1'), 2000); - setTimeout(() => pty.write('PAUSE'), 2200); - setTimeout(() => pty.write('2'), 2400); - setTimeout(() => pty.write('RESUME'), 2600); - setTimeout(() => pty.write('3'), 2800); + setTimeout(() => pty.write('1'), 7000); + setTimeout(() => pty.write('PAUSE'), 7200); + setTimeout(() => pty.write('2'), 7400); + setTimeout(() => pty.write('RESUME'), 7600); + setTimeout(() => pty.write('3'), 7800); setTimeout(() => { // important here: no data should be delivered between 'paused' and 'resumed' if (process.platform === 'win32') { @@ -56,7 +56,7 @@ describe('Terminal', () => { assert.deepEqual(read.slice(-5), ['1', 'paused', 'resumed', '2', '3']); } done(); - }, 3200); + }, 9500); }); it('should enable/disable automatic flow control', () => { const pty = new terminalConstructor(SHELL, []); From 179cd54d41da333edfe814ab6d6dc17457be7aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 23 May 2019 00:18:08 +0200 Subject: [PATCH 15/21] API defs, cleanup --- README.md | 10 +++++----- src/terminal.test.ts | 11 +---------- src/terminal.ts | 35 +++-------------------------------- typings/node-pty.d.ts | 23 +++++++++++++++++++++++ 4 files changed, 32 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 288d1dcfd..d4b0a9511 100644 --- a/README.md +++ b/README.md @@ -114,8 +114,8 @@ This error can occur due to anti-virus software intercepting winpty from creatin ## Flow Control -Automatic flow control can be enabled by either providing `handleFlowControl = true` in the constructor options or calling -`enableFlowControl` later on: +Automatic flow control can be enabled by either providing `handleFlowControl = true` in the constructor options or setting +it later on: ```js const PAUSE = '\x13'; // XOFF @@ -129,14 +129,14 @@ ptyProcess.write(PAUSE); // pty will block and pause the slave program ptyProcess.write(RESUME); // pty will enter flow mode and resume the slave program // temporarily disable/re-enable flow control -ptyProcess.disableFlowControl(); +ptyProcess.handleFlowControl = false; ... -ptyProcess.enableFlowControl(); +ptyProcess.handleFlowControl = true; ``` By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that use these control codes for different purposes the messages can be customized as `flowPause: string` and -`flowResume: string` in the constructor options or arguments to `enableFlowControl`. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. +`flowResume: string` in the constructor options. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. ## pty.js diff --git a/src/terminal.test.ts b/src/terminal.test.ts index cd1b81155..5bc1566e5 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -31,7 +31,7 @@ describe('Terminal', () => { describe('automatic flow control', () => { it('should respect ctor flow control options', () => { const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); - assert.equal((pty as any)._handleFlowControl, true); + assert.equal(pty.handleFlowControl, true); assert.equal((pty as any)._flowPause, 'abc'); assert.equal((pty as any)._flowResume, '123'); }); @@ -58,14 +58,5 @@ describe('Terminal', () => { done(); }, 9500); }); - it('should enable/disable automatic flow control', () => { - const pty = new terminalConstructor(SHELL, []); - // write got not yet replaced - assert.equal((pty as any)._realWrite, null); - pty.enableFlowControl(); - assert.equal((pty as any)._realWrite !== null, true); - pty.disableFlowControl(); - assert.equal((pty as any)._realWrite, null); - }); }); }); diff --git a/src/terminal.ts b/src/terminal.ts index debce647f..b502003ce 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -39,7 +39,7 @@ export abstract class Terminal implements ITerminal { protected _writeMethod: (data: string) => void = () => {}; private _flowPause: string; private _flowResume: string; - private _handleFlowControl: boolean; + public handleFlowControl: boolean; private _onData = new EventEmitter2(); public get onData(): IEvent { return this._onData.event; } @@ -70,13 +70,13 @@ export abstract class Terminal implements ITerminal { this._checkType('encoding', opt.encoding ? opt.encoding : null, 'string'); // setup flow control handling - this._handleFlowControl = !!(opt.handleFlowControl); + this.handleFlowControl = !!(opt.handleFlowControl); this._flowPause = opt.flowPause || FLOW_PAUSE; this._flowResume = opt.flowResume || FLOW_RESUME; } public write(data: string): void { - if (this._handleFlowControl) { + if (this.handleFlowControl) { // PAUSE/RESUME messages are not forwarded to the pty if (data === this._flowPause) { this.pause(); @@ -91,35 +91,6 @@ export abstract class Terminal implements ITerminal { this._writeMethod(data); } - /** - * Enable automatic flow control handling. - * Instead of relying on XON/XOFF directly we pause/resume - * the underlying socket. Main advantage is that flow control will - * still work even when XON/XOFF is not available. As a downside it - * does not act immediately, instead the OS pty buffer and node's socket buffer - * fill up first before the slave program gets paused. - * Note that `flowPause` and `flowResume` must be in line with the - * messages sent by the terminal emulator to indicate PAUSE/RESUME. - * @param flowPause String to pause the pty. Defaults to XOFF (x13). - * @param flowResume String to resume the pty. Defaults to XON (x11). - */ - public enableFlowControl(flowPause?: string, flowResume?: string): void { - if (flowPause) { - this._flowPause = flowPause; - } - if (flowResume) { - this._flowResume = flowResume; - } - this._handleFlowControl = true; - } - - /** - * Disable automatic flow control handling. - */ - public disableFlowControl(): void { - this._handleFlowControl = false; - } - protected _forwardEvents(): void { this.on('data', e => this._onData.fire(e)); this.on('exit', (exitCode, signal) => this._onExit.fire({ exitCode, signal })); diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index be87c2269..592de5363 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -34,6 +34,23 @@ declare module 'node-pty' { * This setting does nothing on non-Windows. */ experimentalUseConpty?: boolean; + /** + * Whether to enable flow control handling (false by default). If enabled a message of `flowPause` + * will pause the socket and thus blocking the slave program execution due to buffer back pressure. + * A message of `flowResume` will resume the socket into flow mode. + * For performance reasons only a single message as a whole will match (no message part matching). + * If flow control is enabled the `flowPause` and `flowResume` messages are not forwarded to + * the underlying pseudoterminal. + */ + handleFlowControl?: boolean; + /** + * String upon flow control shall pause the pty. Default is XOFF ('\x13'). + */ + flowPause: string; + /** + * String upon flow control shall resume the pty. Default is XON ('\x11'). + */ + flowResume: string; } /** @@ -110,6 +127,12 @@ declare module 'node-pty' { * @throws Will throw when signal is used on Windows. */ kill(signal?: string): void; + + /** + * Whether to handle flow control. Useful to disable/re-enable flow control during runtime. + * Use this for binary data that is likely to contain the `flowPause` string by accident. + */ + handleFlowControl: boolean; } /** From 4e70a7ee2d6124d8d10efc0743d99256199ef688 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 May 2019 15:21:30 -0700 Subject: [PATCH 16/21] Move flow control above troubleshooting --- README.md | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index d4b0a9511..ba8eb7d25 100644 --- a/README.md +++ b/README.md @@ -100,22 +100,9 @@ All processes launched from node-pty will launch at the same permission level of Note that node-pty is not thread safe so running it across multiple worker threads in node.js could cause issues. -## Troubleshooting - -### Powershell gives error 8009001d - -> Internal Windows PowerShell error. Loading managed Windows PowerShell failed with error 8009001d. - -This happens when PowerShell is launched with no `SystemRoot` environment variable present. - -### ConnectNamedPipe failed: Windows error 232 - -This error can occur due to anti-virus software intercepting winpty from creating a pty. To workaround this you can exclude this file from your anti-virus scanning `node-pty\build\Release\winpty-agent.exe` - ## Flow Control -Automatic flow control can be enabled by either providing `handleFlowControl = true` in the constructor options or setting -it later on: +Automatic flow control can be enabled by either providing `handleFlowControl = true` in the constructor options or setting it later on: ```js const PAUSE = '\x13'; // XOFF @@ -134,9 +121,19 @@ ptyProcess.handleFlowControl = false; ptyProcess.handleFlowControl = true; ``` -By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that -use these control codes for different purposes the messages can be customized as `flowPause: string` and -`flowResume: string` in the constructor options. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. +By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that use these control codes for different purposes the messages can be customized as `flowPause: string` and `flowResume: string` in the constructor options. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. + +## Troubleshooting + +### Powershell gives error 8009001d + +> Internal Windows PowerShell error. Loading managed Windows PowerShell failed with error 8009001d. + +This happens when PowerShell is launched with no `SystemRoot` environment variable present. + +### ConnectNamedPipe failed: Windows error 232 + +This error can occur due to anti-virus software intercepting winpty from creating a pty. To workaround this you can exclude this file from your anti-virus scanning `node-pty\build\Release\winpty-agent.exe` ## pty.js From 9dd3e801a12d4c9826ef36bf8f674485b9eefdb8 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 May 2019 15:26:18 -0700 Subject: [PATCH 17/21] Improve wording --- typings/node-pty.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 592de5363..25e98626a 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -44,11 +44,11 @@ declare module 'node-pty' { */ handleFlowControl?: boolean; /** - * String upon flow control shall pause the pty. Default is XOFF ('\x13'). + * The string that should pause the pty when `handleFlowControl` is true. Default is XOFF ('\x13'). */ flowPause: string; /** - * String upon flow control shall resume the pty. Default is XON ('\x11'). + * The string that should resume the pty when `handleFlowControl` is true. Default is XON ('\x11'). */ flowResume: string; } From b1a93e5c6fee8fc2600044b1a23197b534789de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 23 May 2019 00:37:21 +0200 Subject: [PATCH 18/21] cleanup --- README.md | 2 +- src/interfaces.ts | 4 ++-- src/terminal.test.ts | 8 ++++---- src/terminal.ts | 18 +++++++++--------- typings/node-pty.d.ts | 22 +++++++++++----------- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index ba8eb7d25..a36bba38d 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ ptyProcess.handleFlowControl = false; ptyProcess.handleFlowControl = true; ``` -By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that use these control codes for different purposes the messages can be customized as `flowPause: string` and `flowResume: string` in the constructor options. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. +By default `PAUSE` and `RESUME` are XON/XOFF control codes (as shown above). To avoid conflicts in environments that use these control codes for different purposes the messages can be customized as `flowControlPause: string` and `flowControlResume: string` in the constructor options. `PAUSE` and `RESUME` are not passed to the underlying pseudoterminal if flow control is enabled. ## Troubleshooting diff --git a/src/interfaces.ts b/src/interfaces.ts index b39aa0501..144f7393d 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -118,8 +118,8 @@ export interface IPtyForkOptions { encoding?: string; experimentalUseConpty?: boolean | undefined; handleFlowControl?: boolean; - flowPause?: string; - flowResume?: string; + flowControlPause?: string; + flowControlResume?: string; } export interface IPtyOpenOptions { diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 5bc1566e5..dd591e8e8 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -30,14 +30,14 @@ describe('Terminal', () => { describe('automatic flow control', () => { it('should respect ctor flow control options', () => { - const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'abc', flowResume: '123'}); + const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowControlPause: 'abc', flowControlResume: '123'}); assert.equal(pty.handleFlowControl, true); - assert.equal((pty as any)._flowPause, 'abc'); - assert.equal((pty as any)._flowResume, '123'); + assert.equal((pty as any)._flowControlPause, 'abc'); + assert.equal((pty as any)._flowControlResume, '123'); }); it('should do flow control automatically', function(done: Function): void { this.timeout(10000); - const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowPause: 'PAUSE', flowResume: 'RESUME'}); + const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowControlPause: 'PAUSE', flowControlResume: 'RESUME'}); const read: string[] = []; pty.on('data', data => read.push(data)); pty.on('pause', () => read.push('paused')); diff --git a/src/terminal.ts b/src/terminal.ts index b502003ce..ee8ec8670 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -15,11 +15,11 @@ export const DEFAULT_ROWS: number = 24; /** * Default messages to indicate PAUSE/RESUME for automatic flow control. - * To avoid conflicts with rebound XON/XOFF control codes, + * To avoid conflicts with rebound XON/XOFF control codes (such as on-my-zsh), * the sequences can be customized in `IPtyForkOptions`. */ -const FLOW_PAUSE = '\x13'; // defaults to XOFF -const FLOW_RESUME = '\x11'; // defaults to XON +const FLOW_CONTROL_PAUSE = '\x13'; // defaults to XOFF +const FLOW_CONTROL_RESUME = '\x11'; // defaults to XON export abstract class Terminal implements ITerminal { protected _socket: Socket; @@ -37,8 +37,8 @@ export abstract class Terminal implements ITerminal { protected _internalee: EventEmitter; protected _writeMethod: (data: string) => void = () => {}; - private _flowPause: string; - private _flowResume: string; + private _flowControlPause: string; + private _flowControlResume: string; public handleFlowControl: boolean; private _onData = new EventEmitter2(); @@ -71,18 +71,18 @@ export abstract class Terminal implements ITerminal { // setup flow control handling this.handleFlowControl = !!(opt.handleFlowControl); - this._flowPause = opt.flowPause || FLOW_PAUSE; - this._flowResume = opt.flowResume || FLOW_RESUME; + this._flowControlPause = opt.flowControlPause || FLOW_CONTROL_PAUSE; + this._flowControlResume = opt.flowControlResume || FLOW_CONTROL_RESUME; } public write(data: string): void { if (this.handleFlowControl) { // PAUSE/RESUME messages are not forwarded to the pty - if (data === this._flowPause) { + if (data === this._flowControlPause) { this.pause(); return; } - if (data === this._flowResume) { + if (data === this._flowControlResume) { this.resume(); return; } diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index 592de5363..24938f240 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -35,22 +35,22 @@ declare module 'node-pty' { */ experimentalUseConpty?: boolean; /** - * Whether to enable flow control handling (false by default). If enabled a message of `flowPause` + * Whether to enable flow control handling (false by default). If enabled a message of `flowControlPause` * will pause the socket and thus blocking the slave program execution due to buffer back pressure. - * A message of `flowResume` will resume the socket into flow mode. + * A message of `flowControlResume` will resume the socket into flow mode. * For performance reasons only a single message as a whole will match (no message part matching). - * If flow control is enabled the `flowPause` and `flowResume` messages are not forwarded to + * If flow control is enabled the `flowControlPause` and `flowControlResume` messages are not forwarded to * the underlying pseudoterminal. */ handleFlowControl?: boolean; /** * String upon flow control shall pause the pty. Default is XOFF ('\x13'). */ - flowPause: string; + flowControlPause?: string; /** * String upon flow control shall resume the pty. Default is XON ('\x11'). */ - flowResume: string; + flowControlResume?: string; } /** @@ -77,6 +77,12 @@ declare module 'node-pty' { */ process: string; + /** + * Whether to handle flow control. Useful to disable/re-enable flow control during runtime. + * Use this for binary data that is likely to contain the `flowControlPause` string by accident. + */ + handleFlowControl: boolean; + /** * Adds an event listener for when a data event fires. This happens when data is returned from * the pty. @@ -127,12 +133,6 @@ declare module 'node-pty' { * @throws Will throw when signal is used on Windows. */ kill(signal?: string): void; - - /** - * Whether to handle flow control. Useful to disable/re-enable flow control during runtime. - * Use this for binary data that is likely to contain the `flowPause` string by accident. - */ - handleFlowControl: boolean; } /** From 1802efe43aedb2784ceedf7dd9408e88a3128f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Breitbart?= Date: Thu, 23 May 2019 00:57:53 +0200 Subject: [PATCH 19/21] dont provide default _writeMethod --- src/terminal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal.ts b/src/terminal.ts index ee8ec8670..88961779d 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -36,7 +36,7 @@ export abstract class Terminal implements ITerminal { protected _writable: boolean; protected _internalee: EventEmitter; - protected _writeMethod: (data: string) => void = () => {}; + protected _writeMethod: (data: string) => void; private _flowControlPause: string; private _flowControlResume: string; public handleFlowControl: boolean; From 6c711349bc599292184d39eb9de5efc0a684868f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 May 2019 16:00:33 -0700 Subject: [PATCH 20/21] Make flow control test magically fast --- src/terminal.test.ts | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index dd591e8e8..24b81d7ea 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -6,6 +6,7 @@ import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; import { UnixTerminal } from './unixTerminal'; +import pollUntil = require('pollUntil'); const terminalConstructor = (process.platform === 'win32') ? WindowsTerminal : UnixTerminal; const SHELL = (process.platform === 'win32') ? 'cmd.exe' : '/bin/bash'; @@ -35,28 +36,26 @@ describe('Terminal', () => { assert.equal((pty as any)._flowControlPause, 'abc'); assert.equal((pty as any)._flowControlResume, '123'); }); - it('should do flow control automatically', function(done: Function): void { + it('should do flow control automatically', async function(): Promise { this.timeout(10000); const pty = new terminalConstructor(SHELL, [], {handleFlowControl: true, flowControlPause: 'PAUSE', flowControlResume: 'RESUME'}); - const read: string[] = []; - pty.on('data', data => read.push(data)); - pty.on('pause', () => read.push('paused')); - pty.on('resume', () => read.push('resumed')); - setTimeout(() => pty.write('1'), 7000); - setTimeout(() => pty.write('PAUSE'), 7200); - setTimeout(() => pty.write('2'), 7400); - setTimeout(() => pty.write('RESUME'), 7600); - setTimeout(() => pty.write('3'), 7800); - setTimeout(() => { + let read: string = ''; + pty.on('data', data => read += data); + pty.on('pause', () => read += 'paused'); + pty.on('resume', () => read += 'resumed'); + pty.write('1'); + pty.write('PAUSE'); + pty.write('2'); + pty.write('RESUME'); + pty.write('3'); + await (pollUntil)(() => { // important here: no data should be delivered between 'paused' and 'resumed' if (process.platform === 'win32') { - // cmd.exe always clears to the end of line? - assert.deepEqual(read.slice(-5), ['1\u001b[0K', 'paused', 'resumed', '2\u001b[0K', '3\u001b[0K']); + read.endsWith('1\u001b[0Kpausedresumed2\u001b[0K3\u001b[0K'); } else { - assert.deepEqual(read.slice(-5), ['1', 'paused', 'resumed', '2', '3']); + read.endsWith('1pausedresumed23'); } - done(); - }, 9500); + }, [], 200, 10); }); }); }); From c32663d6e9bda3a111abf6b4337dbef55e5b7ed8 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 22 May 2019 16:03:20 -0700 Subject: [PATCH 21/21] Make test even faster --- src/terminal.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal.test.ts b/src/terminal.test.ts index 24b81d7ea..242390c71 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -55,7 +55,7 @@ describe('Terminal', () => { } else { read.endsWith('1pausedresumed23'); } - }, [], 200, 10); + }, [], 20, 10); }); }); });