diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts index edb404b8653..1f7a9c93197 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts @@ -69,7 +69,64 @@ suite('workspace-namespace', () => { }, resolveTask(_task: Task): Task | undefined { try { - assert.fail('resolveTask should not trigger during the test'); + assert.fail('resolveTask should not trigger during the test'); + } catch (e) { + done(e); + } + return undefined; + } + })); + commands.executeCommand('workbench.action.tasks.runTask', `${taskType}: ${taskName}`); + }); + + test('sync CustomExecution task should flush all data on close', (done) => { + interface CustomTestingTaskDefinition extends TaskDefinition { + /** + * One of the task properties. This can be used to customize the task in the tasks.json + */ + customProp1: string; + } + const taskType: string = 'customTesting'; + const taskName = 'First custom task'; + disposables.push(window.onDidOpenTerminal(term => { + disposables.push(window.onDidWriteTerminalData(e => { + try { + assert.equal(e.data, 'exiting'); + } catch (e) { + done(e); + } + disposables.push(window.onDidCloseTerminal(() => done())); + term.dispose(); + })); + })); + disposables.push(tasks.registerTaskProvider(taskType, { + provideTasks: () => { + const result: Task[] = []; + const kind: CustomTestingTaskDefinition = { + type: taskType, + customProp1: 'testing task one' + }; + const writeEmitter = new EventEmitter(); + const closeEmitter = new EventEmitter(); + const execution = new CustomExecution((): Thenable => { + const pty: Pseudoterminal = { + onDidWrite: writeEmitter.event, + onDidClose: closeEmitter.event, + open: () => { + writeEmitter.fire('exiting'); + closeEmitter.fire(); + }, + close: () => {} + }; + return Promise.resolve(pty); + }); + const task = new Task2(kind, TaskScope.Workspace, taskName, taskType, execution); + result.push(task); + return result; + }, + resolveTask(_task: Task): Task | undefined { + try { + assert.fail('resolveTask should not trigger during the test'); } catch (e) { done(e); } diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index c3a20304172..9d7b6dff46a 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -343,7 +343,7 @@ class TerminalDataEventTracker extends Disposable { ) { super(); - this._register(this._bufferer = new TerminalDataBufferer()); + this._register(this._bufferer = new TerminalDataBufferer(this._callback)); this._terminalService.terminalInstances.forEach(instance => this._registerInstance(instance)); this._register(this._terminalService.onInstanceCreated(instance => this._registerInstance(instance))); @@ -352,6 +352,6 @@ class TerminalDataEventTracker extends Disposable { private _registerInstance(instance: ITerminalInstance): void { // Buffer data events to reduce the amount of messages going to the extension host - this._register(this._bufferer.startBuffering(instance.id, instance.onData, this._callback)); + this._register(this._bufferer.startBuffering(instance.id, instance.onData)); } } diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 50d9901b68d..e08d1f50b85 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -310,9 +310,8 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ constructor( @IExtHostRpcService extHostRpc: IExtHostRpcService ) { - this._bufferer = new TerminalDataBufferer(); - this._proxy = extHostRpc.getProxy(MainContext.MainThreadTerminalService); + this._bufferer = new TerminalDataBufferer(this._proxy.$sendProcessData); this._onDidWriteTerminalData = new Emitter({ onFirstListenerAdd: () => this._proxy.$startSendingDataEvents(), onLastListenerRemove: () => this._proxy.$stopSendingDataEvents() @@ -477,7 +476,7 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); // Buffer data events to reduce the amount of messages going to the renderer - this._bufferer.startBuffering(id, p.onProcessData, this._proxy.$sendProcessData); + this._bufferer.startBuffering(id, p.onProcessData); p.onProcessExit(exitCode => this._onProcessExit(id, exitCode)); if (p.onProcessOverrideDimensions) { diff --git a/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts b/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts index 8b2008bf29b..82f3fe6f1d0 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts @@ -14,13 +14,16 @@ interface TerminalDataBuffer extends IDisposable { export class TerminalDataBufferer implements IDisposable { private readonly _terminalBufferMap = new Map(); + constructor(private readonly _callback: (id: number, data: string) => void) { + } + dispose() { for (const buffer of this._terminalBufferMap.values()) { buffer.dispose(); } } - startBuffering(id: number, event: Event, callback: (id: number, data: string) => void, throttleBy: number = 5): IDisposable { + startBuffering(id: number, event: Event, throttleBy: number = 5): IDisposable { let disposable: IDisposable; disposable = event((e: string) => { let buffer = this._terminalBufferMap.get(id); @@ -30,16 +33,13 @@ export class TerminalDataBufferer implements IDisposable { return; } - const timeoutId = setTimeout(() => { - this._terminalBufferMap.delete(id); - callback(id, buffer!.data.join('')); - }, throttleBy); + const timeoutId = setTimeout(() => this._flushBuffer(id), throttleBy); buffer = { data: [e], timeoutId: timeoutId, dispose: () => { clearTimeout(timeoutId); - this._terminalBufferMap.delete(id); + this._flushBuffer(id); disposable.dispose(); } }; @@ -54,4 +54,12 @@ export class TerminalDataBufferer implements IDisposable { buffer.dispose(); } } + + private _flushBuffer(id: number): void { + const buffer = this._terminalBufferMap.get(id); + if (buffer) { + this._terminalBufferMap.delete(id); + this._callback(id, buffer.data.join('')); + } + } } diff --git a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts index 54a2e4be0ab..6a2e3b93b21 100644 --- a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts @@ -11,20 +11,28 @@ const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); suite('Workbench - TerminalDataBufferer', () => { let bufferer: TerminalDataBufferer; + let counter: { [id: number]: number }; + let data: { [id: number]: string }; setup(async () => { - bufferer = new TerminalDataBufferer(); + counter = {}; + data = {}; + bufferer = new TerminalDataBufferer((id, e) => { + if (!(id in counter)) { + counter[id] = 0; + } + counter[id]++; + if (!(id in data)) { + data[id] = ''; + } + data[id] = e; + }); }); test('start', async () => { - let terminalOnData = new Emitter(); - let counter = 0; - let data: string | undefined; + const terminalOnData = new Emitter(); - bufferer.startBuffering(1, terminalOnData.event, (id, e) => { - counter++; - data = e; - }, 0); + bufferer.startBuffering(1, terminalOnData.event, 0); terminalOnData.fire('1'); terminalOnData.fire('2'); @@ -34,33 +42,21 @@ suite('Workbench - TerminalDataBufferer', () => { terminalOnData.fire('4'); - assert.equal(counter, 1); - assert.equal(data, '123'); + assert.equal(counter[1], 1); + assert.equal(data[1], '123'); await wait(0); - assert.equal(counter, 2); - assert.equal(data, '4'); + assert.equal(counter[1], 2); + assert.equal(data[1], '4'); }); test('start 2', async () => { - let terminal1OnData = new Emitter(); - let terminal1Counter = 0; - let terminal1Data: string | undefined; + const terminal1OnData = new Emitter(); + const terminal2OnData = new Emitter(); - bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { - terminal1Counter++; - terminal1Data = e; - }, 0); - - let terminal2OnData = new Emitter(); - let terminal2Counter = 0; - let terminal2Data: string | undefined; - - bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { - terminal2Counter++; - terminal2Data = e; - }, 0); + bufferer.startBuffering(1, terminal1OnData.event, 0); + bufferer.startBuffering(2, terminal2OnData.event, 0); terminal1OnData.fire('1'); terminal2OnData.fire('4'); @@ -70,60 +66,41 @@ suite('Workbench - TerminalDataBufferer', () => { terminal2OnData.fire('6'); terminal2OnData.fire('7'); - assert.equal(terminal1Counter, 0); - assert.equal(terminal1Data, undefined); - assert.equal(terminal2Counter, 0); - assert.equal(terminal2Data, undefined); + assert.equal(counter[1], undefined); + assert.equal(data[1], undefined); + assert.equal(counter[2], undefined); + assert.equal(data[2], undefined); await wait(0); - assert.equal(terminal1Counter, 1); - assert.equal(terminal1Data, '123'); - assert.equal(terminal2Counter, 1); - assert.equal(terminal2Data, '4567'); + assert.equal(counter[1], 1); + assert.equal(data[1], '123'); + assert.equal(counter[2], 1); + assert.equal(data[2], '4567'); }); test('stop', async () => { let terminalOnData = new Emitter(); - let counter = 0; - let data: string | undefined; - bufferer.startBuffering(1, terminalOnData.event, (id, e) => { - counter++; - data = e; - }, 0); + bufferer.startBuffering(1, terminalOnData.event, 0); terminalOnData.fire('1'); terminalOnData.fire('2'); terminalOnData.fire('3'); bufferer.stopBuffering(1); - await wait(0); - assert.equal(counter, 0); - assert.equal(data, undefined); + assert.equal(counter[1], 1); + assert.equal(data[1], '123'); }); test('start 2 stop 1', async () => { - let terminal1OnData = new Emitter(); - let terminal1Counter = 0; - let terminal1Data: string | undefined; - - bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { - terminal1Counter++; - terminal1Data = e; - }, 0); - - let terminal2OnData = new Emitter(); - let terminal2Counter = 0; - let terminal2Data: string | undefined; - - bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { - terminal2Counter++; - terminal2Data = e; - }, 0); + const terminal1OnData = new Emitter(); + const terminal2OnData = new Emitter(); + bufferer.startBuffering(1, terminal1OnData.event, 0); + bufferer.startBuffering(2, terminal2OnData.event, 0); terminal1OnData.fire('1'); terminal2OnData.fire('4'); @@ -133,39 +110,26 @@ suite('Workbench - TerminalDataBufferer', () => { terminal2OnData.fire('6'); terminal2OnData.fire('7'); - assert.equal(terminal1Counter, 0); - assert.equal(terminal1Data, undefined); - assert.equal(terminal2Counter, 0); - assert.equal(terminal2Data, undefined); + assert.equal(counter[1], undefined); + assert.equal(data[1], undefined); + assert.equal(counter[2], undefined); + assert.equal(data[2], undefined); bufferer.stopBuffering(1); await wait(0); - assert.equal(terminal1Counter, 0); - assert.equal(terminal1Data, undefined); - assert.equal(terminal2Counter, 1); - assert.equal(terminal2Data, '4567'); + assert.equal(counter[1], 1); + assert.equal(data[1], '123'); + assert.equal(counter[2], 1); + assert.equal(data[2], '4567'); }); - test('dispose', async () => { - let terminal1OnData = new Emitter(); - let terminal1Counter = 0; - let terminal1Data: string | undefined; - - bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { - terminal1Counter++; - terminal1Data = e; - }, 0); - - let terminal2OnData = new Emitter(); - let terminal2Counter = 0; - let terminal2Data: string | undefined; - - bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { - terminal2Counter++; - terminal2Data = e; - }, 0); + test('dispose should flush remaining data events', async () => { + const terminal1OnData = new Emitter(); + const terminal2OnData = new Emitter(); + bufferer.startBuffering(1, terminal1OnData.event, 0); + bufferer.startBuffering(2, terminal2OnData.event, 0); terminal1OnData.fire('1'); terminal2OnData.fire('4'); @@ -175,17 +139,17 @@ suite('Workbench - TerminalDataBufferer', () => { terminal2OnData.fire('6'); terminal2OnData.fire('7'); - assert.equal(terminal1Counter, 0); - assert.equal(terminal1Data, undefined); - assert.equal(terminal2Counter, 0); - assert.equal(terminal2Data, undefined); + assert.equal(counter[1], undefined); + assert.equal(data[1], undefined); + assert.equal(counter[2], undefined); + assert.equal(data[2], undefined); bufferer.dispose(); await wait(0); - assert.equal(terminal1Counter, 0); - assert.equal(terminal1Data, undefined); - assert.equal(terminal2Counter, 0); - assert.equal(terminal2Data, undefined); + assert.equal(counter[1], 1); + assert.equal(data[1], '123'); + assert.equal(counter[2], 1); + assert.equal(data[2], '4567'); }); });