From c73695ab2dd2a819b61df7f9bb8008941c1b7447 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 16 Jan 2019 07:22:11 -0800 Subject: [PATCH 01/36] Implement new extension callback execution for a task and create a terminal for it --- build/builtInExtensions.json | 15 ++++++ src/vs/vscode.d.ts | 36 ++++++++++++-- .../api/electron-browser/mainThreadTask.ts | 29 +++++++++-- src/vs/workbench/api/node/extHost.api.impl.ts | 1 + src/vs/workbench/api/node/extHostTask.ts | 29 +++++++++-- src/vs/workbench/api/node/extHostTypes.ts | 41 ++++++++++++++-- src/vs/workbench/api/shared/tasks.ts | 6 ++- src/vs/workbench/parts/tasks/common/tasks.ts | 5 +- .../electron-browser/terminalTaskSystem.ts | 48 +++++++++++++++---- .../electron-browser/terminalInstance.ts | 2 + 10 files changed, 185 insertions(+), 27 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index b4b5bcdfb87..93ac8571a26 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,5 +43,20 @@ }, "publisherDisplayName": "Microsoft" } + }, + { + "name": "ms-vscode.cpptools", + "version": "0.20.1", + "repo": "https://github.com/Microsoft/vscode-cpptools", + "metadata": { + "id": "36d19e17-7569-4841-a001-947eb18602b2", + "publisherId": { + "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", + "publisherName": "ms-vscode", + "displayName": "Microsoft", + "flags": "verified" + }, + "publisherDisplayName": "Microsoft" + } } ] diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 42d9d87e05e..819041cf9a5 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5074,6 +5074,36 @@ declare module 'vscode' { options?: ShellExecutionOptions; } + /** + * Arguments passed to the callback provided in [ExtensionCallbackExecution](#ExtensionCallbackExecution) class. + */ + export interface ExtensionCallbackExecutionArgs { + /** + * @param text Text to output from the task. + **/ + sendText(text: string, addNewLine?: boolean): void; + + /** + * An event fired when the task is to accept input data. + */ + onTextInput: Event; + } + + /** + * Class used to execute an extension callback as a task. + */ + export class ExtensionCallbackExecution { + /** + * @param callback The callback that will be called when the extension callback task is executed. + */ + constructor(callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken) => Thenable); + + /** + * The callback used to execute the task. + */ + callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken) => Thenable; + } + /** * The scope of a task. */ @@ -5116,7 +5146,7 @@ declare module 'vscode' { * or '$eslint'. Problem matchers can be contributed by an extension using * the `problemMatchers` extension point. */ - constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); + constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); /** * ~~Creates a new task.~~ @@ -5131,7 +5161,7 @@ declare module 'vscode' { * or '$eslint'. Problem matchers can be contributed by an extension using * the `problemMatchers` extension point. */ - constructor(taskDefinition: TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); + constructor(taskDefinition: TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); /** * The task's definition. @@ -5151,7 +5181,7 @@ declare module 'vscode' { /** * The task's execution engine */ - execution?: ProcessExecution | ShellExecution; + execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution; /** * Whether the task is a background task or not. diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index e4d3db05dfe..ac233ceeb65 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -31,7 +31,8 @@ import { ExtHostContext, MainThreadTaskShape, ExtHostTaskShape, MainContext, IEx import { TaskDefinitionDTO, TaskExecutionDTO, ProcessExecutionOptionsDTO, TaskPresentationOptionsDTO, ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, - RunOptionsDTO + RunOptionsDTO, + CallbackExecutionDTO } from 'vs/workbench/api/shared/tasks'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; @@ -138,7 +139,7 @@ namespace ProcessExecutionOptionsDTO { } namespace ProcessExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO): value is ProcessExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ProcessExecutionDTO { let candidate = value as ProcessExecutionDTO; return candidate && !!candidate.process; } @@ -206,7 +207,7 @@ namespace ShellExecutionOptionsDTO { } namespace ShellExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO): value is ShellExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ShellExecutionDTO { let candidate = value as ShellExecutionDTO; return candidate && (!!candidate.commandLine || !!candidate.command); } @@ -237,6 +238,26 @@ namespace ShellExecutionDTO { } } +namespace CallbackExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is CallbackExecutionDTO { + let candidate = value as CallbackExecutionDTO; + return candidate && candidate.extensionCallback === 'extensionCallback'; + } + + export function from(value: CommandConfiguration): CallbackExecutionDTO { + return { + extensionCallback: 'extensionCallback' + }; + } + + export function to(value: CallbackExecutionDTO): CommandConfiguration { + return { + runtime: RuntimeType.ExtensionCallback, + presentation: undefined + }; + } +} + namespace TaskSourceDTO { export function from(value: TaskSource): TaskSourceDTO { let result: TaskSourceDTO = { @@ -336,6 +357,8 @@ namespace TaskDTO { command = ShellExecutionDTO.to(task.execution); } else if (ProcessExecutionDTO.is(task.execution)) { command = ProcessExecutionDTO.to(task.execution); + } else if (CallbackExecutionDTO.is(task.execution)) { + command = CallbackExecutionDTO.to(task.execution); } if (!command) { return undefined; diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index f044aa65682..89700ce33fb 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -765,6 +765,7 @@ export function createApiFactory( DocumentSymbol: extHostTypes.DocumentSymbol, EndOfLine: extHostTypes.EndOfLine, EventEmitter: Emitter, + ExtensionCallbackExecution: extHostTypes.ExtensionCallbackExecution, FileChangeType: extHostTypes.FileChangeType, FileSystemError: extHostTypes.FileSystemError, FileType: files.FileType, diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 6bbb0f26c42..1e536286bba 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -19,8 +19,11 @@ import * as types from 'vs/workbench/api/node/extHostTypes'; import { ExtHostWorkspace } from 'vs/workbench/api/node/extHostWorkspace'; import * as vscode from 'vscode'; import { - TaskDefinitionDTO, TaskExecutionDTO, TaskPresentationOptionsDTO, ProcessExecutionOptionsDTO, ProcessExecutionDTO, - ShellExecutionOptionsDTO, ShellExecutionDTO, TaskDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, TaskSetDTO + TaskDefinitionDTO, TaskExecutionDTO, TaskPresentationOptionsDTO, + ProcessExecutionOptionsDTO, ProcessExecutionDTO, + ShellExecutionOptionsDTO, ShellExecutionDTO, + CallbackExecutionDTO, + TaskDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, TaskSetDTO } from '../shared/tasks'; import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/node/extHostDocumentsAndEditors'; @@ -74,7 +77,7 @@ namespace ProcessExecutionOptionsDTO { } namespace ProcessExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO): value is ProcessExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ProcessExecutionDTO { let candidate = value as ProcessExecutionDTO; return candidate && !!candidate.process; } @@ -115,7 +118,7 @@ namespace ShellExecutionOptionsDTO { } namespace ShellExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO): value is ShellExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ShellExecutionDTO { let candidate = value as ShellExecutionDTO; return candidate && (!!candidate.commandLine || !!candidate.command); } @@ -148,6 +151,19 @@ namespace ShellExecutionDTO { } } +namespace CallbackExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is CallbackExecutionDTO { + let candidate = value as CallbackExecutionDTO; + return candidate && candidate.extensionCallback === 'extensionCallback'; + } + + export function from(value: vscode.ExtensionCallbackExecution): CallbackExecutionDTO { + return { + extensionCallback: 'extensionCallback' + }; + } +} + namespace TaskHandleDTO { export function from(value: types.Task): TaskHandleDTO { let folder: UriComponents; @@ -181,12 +197,15 @@ namespace TaskDTO { if (value === undefined || value === null) { return undefined; } - let execution: ShellExecutionDTO | ProcessExecutionDTO; + let execution: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO; if (value.execution instanceof types.ProcessExecution) { execution = ProcessExecutionDTO.from(value.execution); } else if (value.execution instanceof types.ShellExecution) { execution = ShellExecutionDTO.from(value.execution); + } else if (value.execution instanceof types.ExtensionCallbackExecution) { + execution = CallbackExecutionDTO.from(value.execution); } + let definition: TaskDefinitionDTO = TaskDefinitionDTO.from(value.definition); let scope: number | UriComponents; if (value.scope) { diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 90c05fd6e11..54a5bf89c50 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1621,8 +1621,34 @@ export enum TaskScope { Workspace = 2 } +export class ExtensionCallbackExecution implements vscode.ExtensionCallbackExecution { + private _callback: (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable; + + constructor(callback: (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable) { + this._callback = callback; + } + + public computeId(): string { + const hash = crypto.createHash('md5'); + hash.update('extensionCallback'); + // TODO: How is this ID used. Not sure how to create a valid ID from a function. + // TODO: Also not clear on how the ID is used + hash.update(generateUuid()); + return hash.digest('hex'); + } + + public set callback(value: (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable) { + this._callback = value; + } + + public get callback(): (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable { + return this._callback; + } +} + export class Task implements vscode.Task { + private static ExtensionCallbackType: string = 'extensionCallback'; private static ProcessType: string = 'process'; private static ShellType: string = 'shell'; private static EmptyType: string = '$empty'; @@ -1632,7 +1658,7 @@ export class Task implements vscode.Task { private _definition: vscode.TaskDefinition; private _scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder | undefined; private _name: string; - private _execution: ProcessExecution | ShellExecution | undefined; + private _execution: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined; private _problemMatchers: string[]; private _hasDefinedMatchers: boolean; private _isBackground: boolean; @@ -1641,8 +1667,8 @@ export class Task implements vscode.Task { private _presentationOptions: vscode.TaskPresentationOptions; private _runOptions: vscode.RunOptions; - constructor(definition: vscode.TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); - constructor(definition: vscode.TaskDefinition, scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); + constructor(definition: vscode.TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); + constructor(definition: vscode.TaskDefinition, scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); constructor(definition: vscode.TaskDefinition, arg2: string | (vscode.TaskScope.Global | vscode.TaskScope.Workspace) | vscode.WorkspaceFolder, arg3: any, arg4?: any, arg5?: any, arg6?: any) { this.definition = definition; let problemMatchers: string | string[]; @@ -1707,6 +1733,11 @@ export class Task implements vscode.Task { type: Task.ShellType, id: this._execution.computeId() }; + } else if (this._execution instanceof ExtensionScriptApis) { + this._definition = { + type: Task.ExtensionCallbackType, + id: this._execution.computeId() + }; } else { this._definition = { type: Task.EmptyType, @@ -1748,11 +1779,11 @@ export class Task implements vscode.Task { this._name = value; } - get execution(): ProcessExecution | ShellExecution | undefined { + get execution(): ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined { return this._execution; } - set execution(value: ProcessExecution | ShellExecution | undefined) { + set execution(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { if (value === null) { value = undefined; } diff --git a/src/vs/workbench/api/shared/tasks.ts b/src/vs/workbench/api/shared/tasks.ts index 06dc343e75b..ae9b2d3a548 100644 --- a/src/vs/workbench/api/shared/tasks.ts +++ b/src/vs/workbench/api/shared/tasks.ts @@ -65,6 +65,10 @@ export interface ShellExecutionDTO { options?: ShellExecutionOptionsDTO; } +export interface CallbackExecutionDTO { + extensionCallback: 'extensionCallback'; +} + export interface TaskSourceDTO { label: string; extensionId?: string; @@ -79,7 +83,7 @@ export interface TaskHandleDTO { export interface TaskDTO { _id: string; name: string; - execution: ProcessExecutionDTO | ShellExecutionDTO; + execution: ProcessExecutionDTO | ShellExecutionDTO | CallbackExecutionDTO; definition: TaskDefinitionDTO; isBackground: boolean; source: TaskSourceDTO; diff --git a/src/vs/workbench/parts/tasks/common/tasks.ts b/src/vs/workbench/parts/tasks/common/tasks.ts index fce06577c70..8e4ad35743a 100644 --- a/src/vs/workbench/parts/tasks/common/tasks.ts +++ b/src/vs/workbench/parts/tasks/common/tasks.ts @@ -224,7 +224,8 @@ export namespace PresentationOptions { export enum RuntimeType { Shell = 1, - Process = 2 + Process = 2, + ExtensionCallback = 3 } export namespace RuntimeType { @@ -234,6 +235,8 @@ export namespace RuntimeType { return RuntimeType.Shell; case 'process': return RuntimeType.Process; + case 'extensionCallback': + return RuntimeType.ExtensionCallback; default: return RuntimeType.Process; } diff --git a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts index 65fcf039535..a46305d01ef 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts @@ -689,11 +689,16 @@ export class TerminalTaskSystem implements ITaskSystem { }); } + private createTerminalName(task: CustomTask | ContributedTask): string { + const needsFolderQualification = this.currentTask.workspaceFolder && this.contextService.getWorkbenchState() === WorkbenchState.WORKSPACE; + return nls.localize('TerminalTaskSystem.terminalName', 'Task - {0}', needsFolderQualification ? task.getQualifiedLabel() : task.configurationProperties.name); + } + private createShellLaunchConfig(task: CustomTask | ContributedTask, variableResolver: VariableResolver, platform: Platform.Platform, options: CommandOptions, command: CommandString, args: CommandString[], waitOnExit: boolean | string): IShellLaunchConfig | undefined { let shellLaunchConfig: IShellLaunchConfig; let isShellCommand = task.command.runtime === RuntimeType.Shell; let needsFolderQualification = this.currentTask.workspaceFolder && this.contextService.getWorkbenchState() === WorkbenchState.WORKSPACE; - let terminalName = nls.localize('TerminalTaskSystem.terminalName', 'Task - {0}', needsFolderQualification ? task.getQualifiedLabel() : task.configurationProperties.name); + let terminalName = this.createTerminalName(task); let originalCommand = task.command.name; if (isShellCommand) { shellLaunchConfig = { name: terminalName, executable: undefined, args: undefined, waitOnExit }; @@ -775,7 +780,7 @@ export class TerminalTaskSystem implements ITaskSystem { } } } else { - let commandExecutable = CommandString.value(command); + let commandExecutable = task.command.runtime !== RuntimeType.ExtensionCallback ? CommandString.value(command) : undefined; let executable = !isShellCommand ? this.resolveVariable(variableResolver, '${' + TerminalTaskSystem.ProcessVarName + '}') : commandExecutable; @@ -834,9 +839,8 @@ export class TerminalTaskSystem implements ITaskSystem { private createTerminal(task: CustomTask | ContributedTask, resolver: VariableResolver): [ITerminalInstance | undefined, string | undefined, TaskError | undefined] { let platform = resolver.taskSystemInfo ? resolver.taskSystemInfo.platform : Platform.platform; let options = this.resolveOptions(resolver, task.command.options); + let waitOnExit: boolean | string = false; - let { command, args } = this.resolveCommandAndArgs(resolver, task.command); - let commandExecutable = CommandString.value(command); const presentationOptions = task.command.presentation; if (!presentationOptions) { throw new Error('Task presentation options should not be undefined here.'); @@ -851,10 +855,23 @@ export class TerminalTaskSystem implements ITaskSystem { waitOnExit = true; } } - this.currentTask.shellLaunchConfig = this.isRerun ? this.lastTask.getVerifiedTask().shellLaunchConfig : this.createShellLaunchConfig(task, resolver, platform, options, command, args, waitOnExit); - if (this.currentTask.shellLaunchConfig === undefined) { - return [undefined, undefined, new TaskError(Severity.Error, nls.localize('TerminalTaskSystem', 'Can\'t execute a shell command on an UNC drive using cmd.exe.'), TaskErrors.UnknownError)]; + + let commandExecutable: string | undefined; + let command: CommandString | undefined; + let args: CommandString[] | undefined; + + if (task.command.runtime !== RuntimeType.ExtensionCallback) { + let resolvedResult: { command: CommandString, args: CommandString[] } = this.resolveCommandAndArgs(resolver, task.command); + command = resolvedResult.command; + args = resolvedResult.args; + commandExecutable = CommandString.value(command); + + this.currentTask.shellLaunchConfig = this.isRerun ? this.lastTask.getVerifiedTask().shellLaunchConfig : this.createShellLaunchConfig(task, resolver, platform, options, command, args, waitOnExit); + if (this.currentTask.shellLaunchConfig === undefined) { + return [undefined, undefined, new TaskError(Severity.Error, nls.localize('TerminalTaskSystem', 'Can\'t execute a shell command on an UNC drive using cmd.exe.'), TaskErrors.UnknownError)]; + } } + let prefersSameTerminal = presentationOptions.panel === PanelKind.Dedicated; let allowsSharedTerminal = presentationOptions.panel === PanelKind.Shared; @@ -873,14 +890,21 @@ export class TerminalTaskSystem implements ITaskSystem { } } if (terminalToReuse) { - terminalToReuse.terminal.reuseTerminal(this.currentTask.shellLaunchConfig); + if (task.command.runtime !== RuntimeType.ExtensionCallback) { + if (!this.currentTask.shellLaunchConfig) { + throw new Error('Task shell launch configuration should not be undefined here.'); + } + + terminalToReuse.terminal.reuseTerminal(this.currentTask.shellLaunchConfig); + } + if (task.command.presentation && task.command.presentation.clear) { terminalToReuse.terminal.clear(); } return [terminalToReuse.terminal, commandExecutable, undefined]; } - const result = this.terminalService.createTerminal(this.currentTask.shellLaunchConfig); + const result = task.command.runtime !== RuntimeType.ExtensionCallback ? this.terminalService.createTerminal(this.currentTask.shellLaunchConfig) : this.terminalService.createTerminalRenderer(this.createTerminalName(task)); const terminalKey = result.id.toString(); result.onDisposed((terminal) => { let terminalData = this.terminals[terminalKey]; @@ -1016,6 +1040,12 @@ export class TerminalTaskSystem implements ITaskSystem { } private collectCommandVariables(variables: Set, command: CommandConfiguration, task: CustomTask | ContributedTask): void { + // An extension callback should have everything it needs already as it provided + // the callback. + if (command.runtime === RuntimeType.ExtensionCallback) { + return; + } + if (command.name === undefined) { throw new Error('Command name should never be undefined here.'); } diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts index 598227eb4fc..57ea6e88a55 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts @@ -260,6 +260,8 @@ export class TerminalInstance implements ITerminalInstance { this._logService.trace(`terminalInstance#ctor (id: ${this.id})`, this._shellLaunchConfig); this._initDimensions(); + // There may not be an executable name if a terminal is being used + // for rendering as well as input\output if (!this.shellLaunchConfig.isRendererOnly) { this._createProcess(); } else { From 8fe69fe73f4caa0659709f97277525cb984ea886 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 16 Jan 2019 21:59:26 -0800 Subject: [PATCH 02/36] Start to hook up the terminal and execution --- src/vs/vscode.d.ts | 4 +- .../api/electron-browser/mainThreadTask.ts | 9 +- src/vs/workbench/api/node/extHost.api.impl.ts | 2 +- src/vs/workbench/api/node/extHost.protocol.ts | 3 +- src/vs/workbench/api/node/extHostTask.ts | 126 ++++++++++++++++-- src/vs/workbench/parts/tasks/common/tasks.ts | 17 ++- .../electron-browser/terminalTaskSystem.ts | 4 +- 7 files changed, 142 insertions(+), 23 deletions(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 819041cf9a5..7156214fe6f 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5096,12 +5096,12 @@ declare module 'vscode' { /** * @param callback The callback that will be called when the extension callback task is executed. */ - constructor(callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken) => Thenable); + constructor(callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken, thisArg?: any) => Thenable); /** * The callback used to execute the task. */ - callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken) => Thenable; + callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken, thisArg?: any) => Thenable; } /** diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index ac233ceeb65..bb5b2f761fb 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -417,7 +417,7 @@ export class MainThreadTask implements MainThreadTaskShape { this._taskService.onDidStateChange((event: TaskEvent) => { let task = event.__task; if (event.kind === TaskEventKind.Start) { - this._proxy.$onDidStartTask(TaskExecutionDTO.from(task.getTaskExecution())); + this._proxy.$onDidStartTask(TaskExecutionDTO.from(task.getTaskExecution()), event.terminalId); } else if (event.kind === TaskEventKind.ProcessStarted) { this._proxy.$onDidStartTaskProcess(TaskProcessStartedDTO.from(task.getTaskExecution(), event.processId)); } else if (event.kind === TaskEventKind.ProcessEnded) { @@ -435,6 +435,13 @@ export class MainThreadTask implements MainThreadTaskShape { this._providers.clear(); } + $createTaskId(taskDTO: TaskDTO): Promise { + return new Promise((resolve) => { + let task = TaskDTO.to(taskDTO, this._workspaceContextServer, true); + resolve(task._id); + }); + } + public $registerTaskProvider(handle: number): Promise { let provider: ITaskProvider = { provideTasks: (validTypes: IStringDictionary) => { diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index 89700ce33fb..3eb2fe278a2 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -116,7 +116,7 @@ export function createApiFactory( const extHostDebugService = rpcProtocol.set(ExtHostContext.ExtHostDebugService, new ExtHostDebugService(rpcProtocol, extHostWorkspace, extensionService, extHostDocumentsAndEditors, extHostConfiguration, extHostTerminalService, extHostCommands)); const extHostSCM = rpcProtocol.set(ExtHostContext.ExtHostSCM, new ExtHostSCM(rpcProtocol, extHostCommands, extHostLogService)); const extHostSearch = rpcProtocol.set(ExtHostContext.ExtHostSearch, new ExtHostSearch(rpcProtocol, schemeTransformer, extHostLogService, extHostConfiguration)); - const extHostTask = rpcProtocol.set(ExtHostContext.ExtHostTask, new ExtHostTask(rpcProtocol, extHostWorkspace, extHostDocumentsAndEditors, extHostConfiguration)); + const extHostTask = rpcProtocol.set(ExtHostContext.ExtHostTask, new ExtHostTask(rpcProtocol, extHostWorkspace, extHostDocumentsAndEditors, extHostConfiguration, extHostTerminalService)); const extHostWindow = rpcProtocol.set(ExtHostContext.ExtHostWindow, new ExtHostWindow(rpcProtocol)); rpcProtocol.set(ExtHostContext.ExtHostExtensionService, extensionService); const extHostProgress = rpcProtocol.set(ExtHostContext.ExtHostProgress, new ExtHostProgress(rpcProtocol.getProxy(MainContext.MainThreadProgress))); diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index d6341733d16..4c15cc806d2 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -527,6 +527,7 @@ export interface MainThreadSearchShape extends IDisposable { } export interface MainThreadTaskShape extends IDisposable { + $createTaskId(task: TaskDTO): Promise; $registerTaskProvider(handle: number): Promise; $unregisterTaskProvider(handle: number): Promise; $fetchTasks(filter?: TaskFilterDTO): Promise; @@ -938,7 +939,7 @@ export interface ExtHostSCMShape { export interface ExtHostTaskShape { $provideTasks(handle: number, validTypes: { [key: string]: boolean; }): Thenable; - $onDidStartTask(execution: TaskExecutionDTO): void; + $onDidStartTask(execution: TaskExecutionDTO, terminalId: number): void; $onDidStartTaskProcess(value: TaskProcessStartedDTO): void; $onDidEndTaskProcess(value: TaskProcessEndedDTO): void; $OnDidEndTask(execution: TaskExecutionDTO): void; diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 1e536286bba..3783d359f67 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -28,8 +28,9 @@ import { import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/node/extHostDocumentsAndEditors'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; +import { ExtHostTerminalService, ExtHostTerminal } from 'vs/workbench/api/node/extHostTerminalService'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; -import { CancellationToken } from 'vs/base/common/cancellation'; +import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; namespace TaskDefinitionDTO { export function from(value: vscode.TaskDefinition): TaskDefinitionDTO { @@ -331,15 +332,55 @@ interface HandlerData { extension: IExtensionDescription; } +class ExtensionCallbackExecutionData { + private _cancellationSource?: CancellationTokenSource; + constructor(private readonly callbackData: vscode.ExtensionCallbackExecution, private readonly terminalService: ExtHostTerminalService) { + } + + public terminateCallback(): void { + if (this._cancellationSource) { + return undefined; + } + + this._cancellationSource.cancel(); + this._cancellationSource.dispose(); + this._cancellationSource = undefined; + } + + public startCallback(terminalId: number): Thenable { + if (this._cancellationSource) { + return undefined; + } + + let foundTerminal: ExtHostTerminal | undefined; + for (let terminal of this.terminalService.terminals) { + if (terminal._id === terminalId) { + foundTerminal = terminal; + break; + } + } + + if (!foundTerminal) { + throw new Error('Should have a terminal by this point.'); + } + + this._cancellationSource = new CancellationTokenSource(); + return this.callbackData.callback(undefined, this._cancellationSource.token); + } +} + export class ExtHostTask implements ExtHostTaskShape { private _proxy: MainThreadTaskShape; private _workspaceService: ExtHostWorkspace; private _editorService: ExtHostDocumentsAndEditors; private _configurationService: ExtHostConfiguration; + private _terminalService: ExtHostTerminalService; private _handleCounter: number; private _handlers: Map; private _taskExecutions: Map; + private _providedExtensionCallbacks: Map; + private _activeExtensionCallbacks: Map; private readonly _onDidExecuteTask: Emitter = new Emitter(); private readonly _onDidTerminateTask: Emitter = new Emitter(); @@ -347,14 +388,22 @@ export class ExtHostTask implements ExtHostTaskShape { private readonly _onDidTaskProcessStarted: Emitter = new Emitter(); private readonly _onDidTaskProcessEnded: Emitter = new Emitter(); - constructor(mainContext: IMainContext, workspaceService: ExtHostWorkspace, editorService: ExtHostDocumentsAndEditors, configurationService: ExtHostConfiguration) { + constructor( + mainContext: IMainContext, + workspaceService: ExtHostWorkspace, + editorService: ExtHostDocumentsAndEditors, + configurationService: ExtHostConfiguration, + extHostTerminalService: ExtHostTerminalService) { this._proxy = mainContext.getProxy(MainContext.MainThreadTask); this._workspaceService = workspaceService; this._editorService = editorService; this._configurationService = configurationService; + this._terminalService = extHostTerminalService; this._handleCounter = 0; this._handlers = new Map(); this._taskExecutions = new Map(); + this._providedExtensionCallbacks = new Map(); + this._activeExtensionCallbacks = new Map(); } public get extHostWorkspace(): ExtHostWorkspace { @@ -422,7 +471,21 @@ export class ExtHostTask implements ExtHostTaskShape { return this._onDidExecuteTask.event; } - public $onDidStartTask(execution: TaskExecutionDTO): void { + public $onDidStartTask(execution: TaskExecutionDTO, terminalId: number): void { + // Once a terminal is spun up for the extension callback task execution + // this event will be fired. + // At that point, we need to actually start the callback, but + // only if it hasn't already begun. + const extensionCallback: ExtensionCallbackExecutionData | undefined = this._providedExtensionCallbacks.get(execution.id); + if (extensionCallback) { + // TODO: Verify whether this can ever happen??? + if (this._activeExtensionCallbacks.get(execution.id) === undefined) { + this._activeExtensionCallbacks.set(execution.id, extensionCallback); + } + + extensionCallback.startCallback(terminalId); + } + this._onDidExecuteTask.fire({ execution: this.getTaskExecution(execution) }); @@ -435,6 +498,7 @@ export class ExtHostTask implements ExtHostTaskShape { public $OnDidEndTask(execution: TaskExecutionDTO): void { const _execution = this.getTaskExecution(execution); this._taskExecutions.delete(execution.id); + this.terminateExtensionCallbackExecution(execution); this._onDidTerminateTask.fire({ execution: _execution }); @@ -473,21 +537,55 @@ export class ExtHostTask implements ExtHostTaskShape { if (!handler) { return Promise.reject(new Error('no handler found')); } - return asPromise(() => handler.provider.provideTasks(CancellationToken.None)).then(value => { - let sanitized: vscode.Task[] = []; + + // For extension callback tasks, we need to store the execution objects locally + // since we obviously cannot send callback functions through the proxy. + // So, clear out any existing ones. + this._providedExtensionCallbacks.clear(); + + // Set up a list of task ID promises that we can wait on + // before returning the provided tasks. The ensures that + // our task IDs are calculated for any extension callback tasks. + // Knowing this ID ahead of time is needed because when a task + // start event is fired this is when the extension callback is called. + // The task start event is also the first time we see the ID from the main + // thread, which is too late for us because we need to save an map + // from an ID to an extension callback function. (Kind of a cart before the horse problem). + let taskIdPromises: Promise[] = []; + let fetchPromise = asPromise(() => handler.provider.provideTasks(CancellationToken.None)).then(value => { + const taskDTOs: TaskDTO[] = []; for (let task of value) { - if (task.definition && validTypes[task.definition.type] === true) { - sanitized.push(task); - } else { - sanitized.push(task); + if (!task.definition || !validTypes[task.definition.type]) { console.warn(`The task [${task.source}, ${task.name}] uses an undefined task type. The task will be ignored in the future.`); } + + const taskDTO: TaskDTO = TaskDTO.from(task, handler.extension); + taskDTOs.push(taskDTO); + + if (CallbackExecutionDTO.is(taskDTO.execution)) { + taskIdPromises.push(new Promise((resolve) => { + // The ID is calculated on the main thread task side, so, let's call into it here. + this._proxy.$createTaskId(taskDTO).then((taskId) => { + this._providedExtensionCallbacks.set(taskId, new ExtensionCallbackExecutionData(task.execution, this._terminalService)); + resolve(); + }); + })); + } } + return { - tasks: TaskDTO.fromMany(sanitized, handler.extension), + tasks: taskDTOs, extension: handler.extension }; }); + + return new Promise((resolve) => { + fetchPromise.then((result) => { + Promise.all(taskIdPromises).then(() => { + resolve(result); + }); + }); + }); } public async $resolveVariables(uriComponents: UriComponents, toResolve: { process?: { name: string; cwd?: string; path?: string }, variables: string[] }): Promise<{ process?: string, variables: { [key: string]: string; } }> { @@ -544,4 +642,12 @@ export class ExtHostTask implements ExtHostTaskShape { this._taskExecutions.set(execution.id, result); return result; } + + private terminateExtensionCallbackExecution(execution: TaskExecutionDTO): void { + const extensionCallback: ExtensionCallbackExecutionData | undefined = this._activeExtensionCallbacks.get(execution.id); + if (extensionCallback) { + extensionCallback.terminateCallback(); + this._activeExtensionCallbacks.delete(execution.id); + } + } } diff --git a/src/vs/workbench/parts/tasks/common/tasks.ts b/src/vs/workbench/parts/tasks/common/tasks.ts index 8e4ad35743a..0343ca42e7c 100644 --- a/src/vs/workbench/parts/tasks/common/tasks.ts +++ b/src/vs/workbench/parts/tasks/common/tasks.ts @@ -858,6 +858,7 @@ export interface TaskEvent { group?: string; processId?: number; exitCode?: number; + terminalId?: number; __task?: Task; } @@ -869,11 +870,12 @@ export const enum TaskRunSource { export namespace TaskEvent { export function create(kind: TaskEventKind.ProcessStarted | TaskEventKind.ProcessEnded, task: Task, processIdOrExitCode: number): TaskEvent; - export function create(kind: TaskEventKind.Start | TaskEventKind.Active | TaskEventKind.Inactive | TaskEventKind.Terminated | TaskEventKind.End, task: Task): TaskEvent; + export function create(kind: TaskEventKind.Start, task: Task, terminalId?: number): TaskEvent; + export function create(kind: TaskEventKind.Active | TaskEventKind.Inactive | TaskEventKind.Terminated | TaskEventKind.End, task: Task): TaskEvent; export function create(kind: TaskEventKind.Changed): TaskEvent; - export function create(kind: TaskEventKind, task?: Task, processIdOrExitCode?: number): TaskEvent { + export function create(kind: TaskEventKind, task?: Task, processIdOrExitCodeOrTerminalId?: number): TaskEvent { if (task) { - let result = { + let result: TaskEvent = { kind: kind, taskId: task._id, taskName: task.configurationProperties.name, @@ -881,12 +883,15 @@ export namespace TaskEvent { group: task.configurationProperties.group, processId: undefined as number | undefined, exitCode: undefined as number | undefined, + terminalId: undefined as number | undefined, __task: task, }; - if (kind === TaskEventKind.ProcessStarted) { - result.processId = processIdOrExitCode; + if (kind === TaskEventKind.Start) { + result.terminalId = processIdOrExitCodeOrTerminalId; + } else if (kind === TaskEventKind.ProcessStarted) { + result.processId = processIdOrExitCodeOrTerminalId; } else if (kind === TaskEventKind.ProcessEnded) { - result.exitCode = processIdOrExitCode; + result.exitCode = processIdOrExitCodeOrTerminalId; } return Object.freeze(result); } else { diff --git a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts index a46305d01ef..a71996a03dc 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts @@ -520,7 +520,7 @@ export class TerminalTaskSystem implements ITaskSystem { }, (_error) => { // The process never got ready. Need to think how to handle this. }); - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Start, task)); + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Start, task, terminal.id)); const registeredLinkMatchers = this.registerLinkMatchers(terminal, problemMatchers); const onData = terminal.onLineData((line) => { watchingProblemMatcher.processLine(line); @@ -587,7 +587,7 @@ export class TerminalTaskSystem implements ITaskSystem { }, (_error) => { // The process never got ready. Need to think how to handle this. }); - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Start, task)); + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Start, task, terminal.id)); this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Active, task)); let problemMatchers = this.resolveMatchers(resolver, task.configurationProperties.problemMatchers); let startStopProblemMatcher = new StartStopProblemCollector(problemMatchers, this.markerService, this.modelService); From 54b9bba47e786f9ac8b7ce76a24f7fda6b523757 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Thu, 17 Jan 2019 09:29:47 -0800 Subject: [PATCH 03/36] Callbacks now function --- src/vs/vscode.d.ts | 19 +---- .../mainThreadTerminalService.ts | 4 +- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostTask.ts | 75 ++++++++++++++----- .../api/node/extHostTerminalService.ts | 46 +++++++++--- src/vs/workbench/api/node/extHostTypes.ts | 8 +- 6 files changed, 102 insertions(+), 52 deletions(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 7156214fe6f..effc4fe6315 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5074,21 +5074,6 @@ declare module 'vscode' { options?: ShellExecutionOptions; } - /** - * Arguments passed to the callback provided in [ExtensionCallbackExecution](#ExtensionCallbackExecution) class. - */ - export interface ExtensionCallbackExecutionArgs { - /** - * @param text Text to output from the task. - **/ - sendText(text: string, addNewLine?: boolean): void; - - /** - * An event fired when the task is to accept input data. - */ - onTextInput: Event; - } - /** * Class used to execute an extension callback as a task. */ @@ -5096,12 +5081,12 @@ declare module 'vscode' { /** * @param callback The callback that will be called when the extension callback task is executed. */ - constructor(callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken, thisArg?: any) => Thenable); + constructor(callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); /** * The callback used to execute the task. */ - callback: (args: ExtensionCallbackExecutionArgs, cancellationToken: CancellationToken, thisArg?: any) => Thenable; + callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; } /** diff --git a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts index 49bee8d7d2d..52145033512 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts @@ -183,10 +183,10 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape private _onTerminalOpened(terminalInstance: ITerminalInstance): void { if (terminalInstance.title) { - this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title); + this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title, terminalInstance.shellLaunchConfig.isRendererOnly); } else { terminalInstance.waitForTitle().then(title => { - this._proxy.$acceptTerminalOpened(terminalInstance.id, title); + this._proxy.$acceptTerminalOpened(terminalInstance.id, title, terminalInstance.shellLaunchConfig.isRendererOnly); }); } } diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 4c15cc806d2..26dec76d559 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -916,7 +916,7 @@ export interface ShellLaunchConfigDto { export interface ExtHostTerminalServiceShape { $acceptTerminalClosed(id: number): void; - $acceptTerminalOpened(id: number, name: string): void; + $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean): void; $acceptActiveTerminalChanged(id: number | null): void; $acceptTerminalProcessId(id: number, processId: number): void; $acceptTerminalProcessData(id: number, data: string): void; diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 3783d359f67..c1e73c6e467 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -28,9 +28,10 @@ import { import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/node/extHostDocumentsAndEditors'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; -import { ExtHostTerminalService, ExtHostTerminal } from 'vs/workbench/api/node/extHostTerminalService'; +import { ExtHostTerminalService, ExtHostTerminalRenderer } from 'vs/workbench/api/node/extHostTerminalService'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; +import { IDisposable } from 'vs/base/common/lifecycle'; namespace TaskDefinitionDTO { export function from(value: vscode.TaskDefinition): TaskDefinitionDTO { @@ -332,40 +333,76 @@ interface HandlerData { extension: IExtensionDescription; } -class ExtensionCallbackExecutionData { +class ExtensionCallbackExecutionData implements IDisposable { private _cancellationSource?: CancellationTokenSource; + private _onDidOpenRendererTerminal?: IDisposable; + private _terminalId?: number; + constructor(private readonly callbackData: vscode.ExtensionCallbackExecution, private readonly terminalService: ExtHostTerminalService) { } - public terminateCallback(): void { + public dispose(): void { if (this._cancellationSource) { - return undefined; + this._cancellationSource.cancel(); + this._cancellationSource.dispose(); } - this._cancellationSource.cancel(); - this._cancellationSource.dispose(); - this._cancellationSource = undefined; + if (this._onDidOpenRendererTerminal) { + this._onDidOpenRendererTerminal.dispose(); + } } - public startCallback(terminalId: number): Thenable { + private onDidOpenRenderTerminal(terminal: vscode.TerminalRenderer): Thenable { + // If we have already started the extension task callback, then + // do not start it again. + // It is completely valid for multiple terminals to be opened + // before the one for our task. if (this._cancellationSource) { return undefined; } - let foundTerminal: ExtHostTerminal | undefined; - for (let terminal of this.terminalService.terminals) { + if (terminal instanceof ExtHostTerminalRenderer && terminal._id === this._terminalId) { + // Stop listening (if we are) for more terminals + // to be created. + if (this._onDidOpenRendererTerminal) { + this._onDidOpenRendererTerminal.dispose(); + this._onDidOpenRendererTerminal = undefined; + } + + if (!(terminal instanceof ExtHostTerminalRenderer)) { + throw new Error('Expected a terminal renderer'); + } + + this._cancellationSource = new CancellationTokenSource(); + return this.callbackData.callback(terminal, this._cancellationSource.token).then(() => { + }); + } + + return undefined; + } + + public startCallback(terminalId: number): void { + this._terminalId = terminalId; + + // In order to start the task, we need to wait for the extension host + // to know about the new terminal. + // The order in which the events make it to the extension host (currently) + // is "task created" followed by "terminal opened". + // So, we need to wait for that event. + // However, this loop below ensures that if the order of those events + // ever changes, this code continues to function. + + // Check to see if the extension host already knows about this terminal. + for (let terminal of this.terminalService.rendererTerminals) { if (terminal._id === terminalId) { - foundTerminal = terminal; - break; + this.onDidOpenRenderTerminal(terminal); + return; } } - if (!foundTerminal) { - throw new Error('Should have a terminal by this point.'); - } - - this._cancellationSource = new CancellationTokenSource(); - return this.callbackData.callback(undefined, this._cancellationSource.token); + // If we get here, then the terminal is unknown to the extension host. Let's wait + // for it to be created and the start our extension callback. + this._onDidOpenRendererTerminal = this.terminalService.onDidOpenRendererTerminal(this.onDidOpenRenderTerminal.bind(this)); } } @@ -646,7 +683,7 @@ export class ExtHostTask implements ExtHostTaskShape { private terminateExtensionCallbackExecution(execution: TaskExecutionDTO): void { const extensionCallback: ExtensionCallbackExecutionData | undefined = this._activeExtensionCallbacks.get(execution.id); if (extensionCallback) { - extensionCallback.terminateCallback(); + extensionCallback.dispose(); this._activeExtensionCallbacks.delete(execution.id); } } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 95ec3d5874a..7648b7b98b2 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -208,13 +208,16 @@ export class ExtHostTerminalRenderer extends BaseExtHostTerminal implements vsco constructor( proxy: MainThreadTerminalServiceShape, private _name: string, - private _terminal: ExtHostTerminal + private _terminal: ExtHostTerminal, + id?: number ) { - super(proxy); - this._proxy.$createTerminalRenderer(this._name).then(id => { - this._runQueuedRequests(id); - (this._terminal)._runQueuedRequests(id); - }); + super(proxy, id); + if (!id) { + this._proxy.$createTerminalRenderer(this._name).then(id => { + this._runQueuedRequests(id); + (this._terminal)._runQueuedRequests(id); + }); + } } public write(data: string): void { @@ -245,11 +248,14 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { public get activeTerminal(): ExtHostTerminal { return this._activeTerminal; } public get terminals(): ExtHostTerminal[] { return this._terminals; } + public get rendererTerminals(): ExtHostTerminalRenderer[] { return this._terminalRenderers; } private readonly _onDidCloseTerminal: Emitter = new Emitter(); public get onDidCloseTerminal(): Event { return this._onDidCloseTerminal && this._onDidCloseTerminal.event; } private readonly _onDidOpenTerminal: Emitter = new Emitter(); public get onDidOpenTerminal(): Event { return this._onDidOpenTerminal && this._onDidOpenTerminal.event; } + private readonly _onDidOpenRendererTerminal: Emitter = new Emitter(); + public get onDidOpenRendererTerminal(): Event { return this._onDidOpenRendererTerminal && this._onDidOpenRendererTerminal.event; } private readonly _onDidChangeActiveTerminal: Emitter = new Emitter(); public get onDidChangeActiveTerminal(): Event { return this._onDidChangeActiveTerminal && this._onDidChangeActiveTerminal.event; } @@ -342,17 +348,39 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { this._onDidCloseTerminal.fire(terminal); } - public $acceptTerminalOpened(id: number, name: string): void { + public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean): void { + // If this is a terminal created by one of the public createrTerminal* APIs + // then @acceptTerminalOpened was called from the main thread task + // to indicate that the terminal is ready for use. + // In those cases, we don't need to create the extension host objects + // as they alrady exist, but, we do still need to fire the events + // to our consumers. const index = this._getTerminalObjectIndexById(this._terminals, id); if (index !== null) { - // The terminal has already been created (via createTerminal*), only fire the event this._onDidOpenTerminal.fire(this.terminals[index]); + } + + let renderer = this._getTerminalRendererById(id); + if (renderer) { + // The terminal has already been created (via createTerminal*), only fire the event + this._onDidOpenRendererTerminal.fire(renderer); + } + + if (renderer || index) { return; } - const renderer = this._getTerminalRendererById(id); + + // The extension host did not know about this terminal, so create extension host + // objects to represent them. const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined); this._terminals.push(terminal); this._onDidOpenTerminal.fire(terminal); + + if (isRendererOnly) { + renderer = new ExtHostTerminalRenderer(this._proxy, name, terminal, id); + this.rendererTerminals.push(renderer); + this._onDidOpenRendererTerminal.fire(renderer); + } } public $acceptTerminalProcessId(id: number, processId: number): void { diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 54a5bf89c50..a255e5945cf 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1622,9 +1622,9 @@ export enum TaskScope { } export class ExtensionCallbackExecution implements vscode.ExtensionCallbackExecution { - private _callback: (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable; + private _callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable; - constructor(callback: (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable) { + constructor(callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { this._callback = callback; } @@ -1637,11 +1637,11 @@ export class ExtensionCallbackExecution implements vscode.ExtensionCallbackExecu return hash.digest('hex'); } - public set callback(value: (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable) { + public set callback(value: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { this._callback = value; } - public get callback(): (args: vscode.ExtensionCallbackExecutionArgs, cancellationToken: vscode.CancellationToken) => Thenable { + public get callback(): (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable { return this._callback; } } From cb3f8a16a3cacdc4d96189083fdde6426f65a323 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Thu, 17 Jan 2019 11:33:17 -0800 Subject: [PATCH 04/36] The beginning of cancellation --- src/vs/workbench/api/node/extHostTask.ts | 47 ++++++++++++++++-------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index c1e73c6e467..8a75e7717bd 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -28,10 +28,10 @@ import { import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/node/extHostDocumentsAndEditors'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; -import { ExtHostTerminalService, ExtHostTerminalRenderer } from 'vs/workbench/api/node/extHostTerminalService'; +import { ExtHostTerminalService, ExtHostTerminal, ExtHostTerminalRenderer } from 'vs/workbench/api/node/extHostTerminalService'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; -import { IDisposable } from 'vs/base/common/lifecycle'; +import { IDisposable, dispose } from 'vs/base/common/lifecycle'; namespace TaskDefinitionDTO { export function from(value: vscode.TaskDefinition): TaskDefinitionDTO { @@ -337,22 +337,29 @@ class ExtensionCallbackExecutionData implements IDisposable { private _cancellationSource?: CancellationTokenSource; private _onDidOpenRendererTerminal?: IDisposable; private _terminalId?: number; + private readonly _onTaskExecutionComplete: Emitter = new Emitter(); + private readonly _disposables: IDisposable[] = []; - constructor(private readonly callbackData: vscode.ExtensionCallbackExecution, private readonly terminalService: ExtHostTerminalService) { + constructor( + private readonly callbackData: vscode.ExtensionCallbackExecution, + private readonly terminalService: ExtHostTerminalService) { } public dispose(): void { - if (this._cancellationSource) { - this._cancellationSource.cancel(); - this._cancellationSource.dispose(); - } + dispose(this._disposables); + } - if (this._onDidOpenRendererTerminal) { - this._onDidOpenRendererTerminal.dispose(); + public get onTaskExecutionComplete(): Event { + return this._onTaskExecutionComplete.event; + } + + private onDidCloseTerminal(terminal: vscode.Terminal): void { + if (terminal instanceof ExtHostTerminal && terminal._id === this._terminalId) { + this._cancellationSource.cancel(); } } - private onDidOpenRenderTerminal(terminal: vscode.TerminalRenderer): Thenable { + private onDidOpenRenderTerminal(terminalRenderer: vscode.TerminalRenderer): Thenable { // If we have already started the extension task callback, then // do not start it again. // It is completely valid for multiple terminals to be opened @@ -361,7 +368,7 @@ class ExtensionCallbackExecutionData implements IDisposable { return undefined; } - if (terminal instanceof ExtHostTerminalRenderer && terminal._id === this._terminalId) { + if (terminalRenderer instanceof ExtHostTerminalRenderer && terminalRenderer._id === this._terminalId) { // Stop listening (if we are) for more terminals // to be created. if (this._onDidOpenRendererTerminal) { @@ -369,13 +376,19 @@ class ExtensionCallbackExecutionData implements IDisposable { this._onDidOpenRendererTerminal = undefined; } - if (!(terminal instanceof ExtHostTerminalRenderer)) { + if (!(terminalRenderer instanceof ExtHostTerminalRenderer)) { throw new Error('Expected a terminal renderer'); } this._cancellationSource = new CancellationTokenSource(); - return this.callbackData.callback(terminal, this._cancellationSource.token).then(() => { - }); + this._disposables.push(this._cancellationSource); + + this._disposables.push(this.terminalService.onDidCloseTerminal(this.onDidCloseTerminal.bind(this))); + + // Regardless of how the task completes, we are done with this extension callback task execution. + return this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( + () => this._onTaskExecutionComplete.fire(this), + () => this._onTaskExecutionComplete.fire(this)); } return undefined; @@ -402,7 +415,7 @@ class ExtensionCallbackExecutionData implements IDisposable { // If we get here, then the terminal is unknown to the extension host. Let's wait // for it to be created and the start our extension callback. - this._onDidOpenRendererTerminal = this.terminalService.onDidOpenRendererTerminal(this.onDidOpenRenderTerminal.bind(this)); + this._disposables.push(this.terminalService.onDidOpenRendererTerminal(this.onDidOpenRenderTerminal.bind(this))); } } @@ -520,6 +533,10 @@ export class ExtHostTask implements ExtHostTaskShape { this._activeExtensionCallbacks.set(execution.id, extensionCallback); } + extensionCallback.onTaskExecutionComplete(() => { + this.terminateExtensionCallbackExecution(execution); + }); + extensionCallback.startCallback(terminalId); } From 1faf0981a03b8c711161b2cccb403ec7378a0f26 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Fri, 18 Jan 2019 07:35:22 -0800 Subject: [PATCH 05/36] Make sure to dispose callback registration --- src/vs/workbench/api/node/extHostTask.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 8a75e7717bd..6597e4ba0c1 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -533,8 +533,9 @@ export class ExtHostTask implements ExtHostTaskShape { this._activeExtensionCallbacks.set(execution.id, extensionCallback); } - extensionCallback.onTaskExecutionComplete(() => { + const taskExecutionComplete: IDisposable = extensionCallback.onTaskExecutionComplete(() => { this.terminateExtensionCallbackExecution(execution); + taskExecutionComplete.dispose(); }); extensionCallback.startCallback(terminalId); From 50d49c8c36f78ab4424eac90173e669a64ec3e3c Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Fri, 18 Jan 2019 13:16:55 -0800 Subject: [PATCH 06/36] Move to proposed API and rename things for consistency --- src/vs/vscode.d.ts | 22 ++--------- src/vs/vscode.proposed.d.ts | 39 +++++++++++++++++++ .../api/electron-browser/mainThreadTask.ts | 14 +++---- src/vs/workbench/api/node/extHost.api.impl.ts | 2 +- src/vs/workbench/api/node/extHostTask.ts | 33 +++++++++------- src/vs/workbench/api/node/extHostTypes.ts | 26 ++++++++++--- src/vs/workbench/api/shared/tasks.ts | 4 +- .../electron-browser/terminalTaskSystem.ts | 8 +++- 8 files changed, 98 insertions(+), 50 deletions(-) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index effc4fe6315..18c5dbf463b 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5074,21 +5074,6 @@ declare module 'vscode' { options?: ShellExecutionOptions; } - /** - * Class used to execute an extension callback as a task. - */ - export class ExtensionCallbackExecution { - /** - * @param callback The callback that will be called when the extension callback task is executed. - */ - constructor(callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); - - /** - * The callback used to execute the task. - */ - callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; - } - /** * The scope of a task. */ @@ -5118,7 +5103,6 @@ declare module 'vscode' { * A task to execute */ export class Task { - /** * Creates a new task. * @@ -5131,7 +5115,7 @@ declare module 'vscode' { * or '$eslint'. Problem matchers can be contributed by an extension using * the `problemMatchers` extension point. */ - constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); + constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); /** * ~~Creates a new task.~~ @@ -5146,7 +5130,7 @@ declare module 'vscode' { * or '$eslint'. Problem matchers can be contributed by an extension using * the `problemMatchers` extension point. */ - constructor(taskDefinition: TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); + constructor(taskDefinition: TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[]); /** * The task's definition. @@ -5166,7 +5150,7 @@ declare module 'vscode' { /** * The task's execution engine */ - execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution; + execution?: ProcessExecution | ShellExecution; /** * Whether the task is a background task or not. diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index edbf4b240fa..6db56b3942e 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1098,4 +1098,43 @@ declare module 'vscode' { readonly activeSignatureHelp?: SignatureHelp; } //#endregion + + /** + * Class used to execute an extension callback as a task. + */ + export class ExtensionCallbackExecution { + /** + * @param callback The callback that will be called when the extension callback task is executed. + */ + constructor(callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); + + /** + * The callback used to execute the task. + */ + callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; + } + + /** + * A task to execute + */ + export class TaskWithExtensionCallback extends Task { + /** + * Creates a new task. + * + * @param definition The task definition as defined in the taskDefinitions extension point. + * @param scope Specifies the task's scope. It is either a global or a workspace task or a task for a specific workspace folder. + * @param name The task's name. Is presented in the user interface. + * @param source The task's source (e.g. 'gulp', 'npm', ...). Is presented in the user interface. + * @param execution The process or shell execution. + * @param problemMatchers the names of problem matchers to use, like '$tsc' + * or '$eslint'. Problem matchers can be contributed by an extension using + * the `problemMatchers` extension point. + */ + constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | ExtensionCallbackExecution, problemMatchers?: string | string[]); + + /** + * The task's execution engine + */ + executionWithExtensionCallback?: ProcessExecution | ShellExecution | ExtensionCallbackExecution; + } } diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index bb5b2f761fb..724cceea194 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -32,7 +32,7 @@ import { TaskDefinitionDTO, TaskExecutionDTO, ProcessExecutionOptionsDTO, TaskPresentationOptionsDTO, ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, RunOptionsDTO, - CallbackExecutionDTO + ExtensionCallbackExecutionDTO } from 'vs/workbench/api/shared/tasks'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; @@ -139,7 +139,7 @@ namespace ProcessExecutionOptionsDTO { } namespace ProcessExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ProcessExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO): value is ProcessExecutionDTO { let candidate = value as ProcessExecutionDTO; return candidate && !!candidate.process; } @@ -207,7 +207,7 @@ namespace ShellExecutionOptionsDTO { } namespace ShellExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ShellExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO): value is ShellExecutionDTO { let candidate = value as ShellExecutionDTO; return candidate && (!!candidate.commandLine || !!candidate.command); } @@ -239,18 +239,18 @@ namespace ShellExecutionDTO { } namespace CallbackExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is CallbackExecutionDTO { - let candidate = value as CallbackExecutionDTO; + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO): value is ExtensionCallbackExecutionDTO { + let candidate = value as ExtensionCallbackExecutionDTO; return candidate && candidate.extensionCallback === 'extensionCallback'; } - export function from(value: CommandConfiguration): CallbackExecutionDTO { + export function from(value: CommandConfiguration): ExtensionCallbackExecutionDTO { return { extensionCallback: 'extensionCallback' }; } - export function to(value: CallbackExecutionDTO): CommandConfiguration { + export function to(value: ExtensionCallbackExecutionDTO): CommandConfiguration { return { runtime: RuntimeType.ExtensionCallback, presentation: undefined diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index 3eb2fe278a2..7179a57a5ee 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -800,7 +800,7 @@ export function createApiFactory( SymbolInformation: extHostTypes.SymbolInformation, SymbolKind: extHostTypes.SymbolKind, Task: extHostTypes.Task, - Task2: extHostTypes.Task, + TaskWithExtensionCallback: extHostTypes.Task, TaskGroup: extHostTypes.TaskGroup, TaskPanelKind: extHostTypes.TaskPanelKind, TaskRevealKind: extHostTypes.TaskRevealKind, diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 6597e4ba0c1..def67669a93 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -22,7 +22,7 @@ import { TaskDefinitionDTO, TaskExecutionDTO, TaskPresentationOptionsDTO, ProcessExecutionOptionsDTO, ProcessExecutionDTO, ShellExecutionOptionsDTO, ShellExecutionDTO, - CallbackExecutionDTO, + ExtensionCallbackExecutionDTO, TaskDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, TaskSetDTO } from '../shared/tasks'; import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; @@ -79,7 +79,7 @@ namespace ProcessExecutionOptionsDTO { } namespace ProcessExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ProcessExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO): value is ProcessExecutionDTO { let candidate = value as ProcessExecutionDTO; return candidate && !!candidate.process; } @@ -120,7 +120,7 @@ namespace ShellExecutionOptionsDTO { } namespace ShellExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is ShellExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO): value is ShellExecutionDTO { let candidate = value as ShellExecutionDTO; return candidate && (!!candidate.commandLine || !!candidate.command); } @@ -153,13 +153,13 @@ namespace ShellExecutionDTO { } } -namespace CallbackExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO): value is CallbackExecutionDTO { - let candidate = value as CallbackExecutionDTO; +namespace ExtensionCallbackExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO): value is ExtensionCallbackExecutionDTO { + let candidate = value as ExtensionCallbackExecutionDTO; return candidate && candidate.extensionCallback === 'extensionCallback'; } - export function from(value: vscode.ExtensionCallbackExecution): CallbackExecutionDTO { + export function from(value: vscode.ExtensionCallbackExecution): ExtensionCallbackExecutionDTO { return { extensionCallback: 'extensionCallback' }; @@ -199,13 +199,13 @@ namespace TaskDTO { if (value === undefined || value === null) { return undefined; } - let execution: ShellExecutionDTO | ProcessExecutionDTO | CallbackExecutionDTO; + let execution: ShellExecutionDTO | ProcessExecutionDTO | ExtensionCallbackExecutionDTO; if (value.execution instanceof types.ProcessExecution) { execution = ProcessExecutionDTO.from(value.execution); } else if (value.execution instanceof types.ShellExecution) { execution = ShellExecutionDTO.from(value.execution); - } else if (value.execution instanceof types.ExtensionCallbackExecution) { - execution = CallbackExecutionDTO.from(value.execution); + } else if ((value).executionWithExtensionCallback && (value).executionWithExtensionCallback instanceof types.ExtensionCallbackExecution) { + execution = ExtensionCallbackExecutionDTO.from((value).executionWithExtensionCallback); } let definition: TaskDefinitionDTO = TaskDefinitionDTO.from(value.definition); @@ -387,8 +387,13 @@ class ExtensionCallbackExecutionData implements IDisposable { // Regardless of how the task completes, we are done with this extension callback task execution. return this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( - () => this._onTaskExecutionComplete.fire(this), - () => this._onTaskExecutionComplete.fire(this)); + (success) => { + this._onTaskExecutionComplete.fire(this); + terminalRenderer.terminal.dispose(); + }, (rejected) => { + this._onTaskExecutionComplete.fire(this); + terminalRenderer.terminal.dispose(); + }); } return undefined; @@ -617,11 +622,11 @@ export class ExtHostTask implements ExtHostTaskShape { const taskDTO: TaskDTO = TaskDTO.from(task, handler.extension); taskDTOs.push(taskDTO); - if (CallbackExecutionDTO.is(taskDTO.execution)) { + if (ExtensionCallbackExecutionDTO.is(taskDTO.execution)) { taskIdPromises.push(new Promise((resolve) => { // The ID is calculated on the main thread task side, so, let's call into it here. this._proxy.$createTaskId(taskDTO).then((taskId) => { - this._providedExtensionCallbacks.set(taskId, new ExtensionCallbackExecutionData(task.execution, this._terminalService)); + this._providedExtensionCallbacks.set(taskId, new ExtensionCallbackExecutionData((task).executionWithExtensionCallback, this._terminalService)); resolve(); }); })); diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index a255e5945cf..912dac95366 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1646,7 +1646,7 @@ export class ExtensionCallbackExecution implements vscode.ExtensionCallbackExecu } } -export class Task implements vscode.Task { +export class Task implements vscode.TaskWithExtensionCallback { private static ExtensionCallbackType: string = 'extensionCallback'; private static ProcessType: string = 'process'; @@ -1733,7 +1733,7 @@ export class Task implements vscode.Task { type: Task.ShellType, id: this._execution.computeId() }; - } else if (this._execution instanceof ExtensionScriptApis) { + } else if (this._execution instanceof ExtensionCallbackExecution) { this._definition = { type: Task.ExtensionCallbackType, id: this._execution.computeId() @@ -1779,18 +1779,34 @@ export class Task implements vscode.Task { this._name = value; } - get execution(): ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined { + get execution(): ProcessExecution | ShellExecution | undefined { + return (this._execution instanceof ExtensionCallbackExecution) ? undefined : this._execution; + } + + set execution(value: ProcessExecution | ShellExecution | undefined) { + this.executionInternal = value; + } + + get executionWithExtensionCallback(): ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined { + return this.executionInternal; + } + + set executionWithExtensionCallback(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { + this.executionInternal = value; + } + + private get executionInternal(): ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined { return this._execution; } - set execution(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { + private set executionInternal(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { if (value === null) { value = undefined; } this.clear(); this._execution = value; let type = this._definition.type; - if (Task.EmptyType === type || Task.ProcessType === type || Task.ShellType === type) { + if (Task.EmptyType === type || Task.ProcessType === type || Task.ShellType === type || Task.ExtensionCallbackType === type) { this.computeDefinitionBasedOnExecution(); } } diff --git a/src/vs/workbench/api/shared/tasks.ts b/src/vs/workbench/api/shared/tasks.ts index ae9b2d3a548..e7a392ac0c3 100644 --- a/src/vs/workbench/api/shared/tasks.ts +++ b/src/vs/workbench/api/shared/tasks.ts @@ -65,7 +65,7 @@ export interface ShellExecutionDTO { options?: ShellExecutionOptionsDTO; } -export interface CallbackExecutionDTO { +export interface ExtensionCallbackExecutionDTO { extensionCallback: 'extensionCallback'; } @@ -83,7 +83,7 @@ export interface TaskHandleDTO { export interface TaskDTO { _id: string; name: string; - execution: ProcessExecutionDTO | ShellExecutionDTO | CallbackExecutionDTO; + execution: ProcessExecutionDTO | ShellExecutionDTO | ExtensionCallbackExecutionDTO; definition: TaskDefinitionDTO; isBackground: boolean; source: TaskSourceDTO; diff --git a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts index a71996a03dc..57d765804d6 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts @@ -514,7 +514,9 @@ export class TerminalTaskSystem implements ITaskSystem { let processStartedSignaled = false; terminal.processReady.then(() => { if (!processStartedSignaled) { - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); + if (task.command.runtime !== RuntimeType.ExtensionCallback) { + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); + } processStartedSignaled = true; } }, (_error) => { @@ -581,7 +583,9 @@ export class TerminalTaskSystem implements ITaskSystem { let processStartedSignaled = false; terminal.processReady.then(() => { if (!processStartedSignaled) { - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); + if (task.command.runtime !== RuntimeType.ExtensionCallback) { + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); + } processStartedSignaled = true; } }, (_error) => { From 75f82ce09ac82dbd1f9212cb1751d45b96553de3 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sat, 19 Jan 2019 08:00:36 -0800 Subject: [PATCH 07/36] Get terminal re-use and task restart working --- .../api/electron-browser/mainThreadTask.ts | 18 +++++ src/vs/workbench/api/node/extHost.protocol.ts | 1 + src/vs/workbench/api/node/extHostTask.ts | 21 +++--- .../api/node/extHostTerminalService.ts | 26 ++++--- .../parts/tasks/common/taskService.ts | 2 + .../parts/tasks/common/taskSystem.ts | 1 + src/vs/workbench/parts/tasks/common/tasks.ts | 18 ++++- .../electron-browser/task.contribution.ts | 7 ++ .../electron-browser/terminalTaskSystem.ts | 41 ++++++++++- .../parts/tasks/node/processTaskSystem.ts | 4 ++ .../parts/terminal/common/terminal.ts | 9 ++- .../parts/terminal/common/terminalService.ts | 2 +- .../electron-browser/terminalInstance.ts | 70 ++++++++++++++++--- .../electron-browser/terminalService.ts | 4 +- 14 files changed, 189 insertions(+), 35 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index 724cceea194..f946be24d18 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -515,6 +515,24 @@ export class MainThreadTask implements MainThreadTaskShape { }); } + public $extensionCallbackTaskComplete(id: string): Promise { + return new Promise((resolve, reject) => { + this._taskService.getActiveTasks().then((tasks) => { + for (let task of tasks) { + if (id === task._id) { + this._taskService.extensionCallbackTaskComplete(task).then((value) => { + resolve(undefined); + }, (error) => { + reject(undefined); + }); + return; + } + } + reject(new Error('Task to mark as complete not found')); + }); + }); + } + public $terminateTask(id: string): Promise { return new Promise((resolve, reject) => { this._taskService.getActiveTasks().then((tasks) => { diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 26dec76d559..ceebe5dbe85 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -534,6 +534,7 @@ export interface MainThreadTaskShape extends IDisposable { $executeTask(task: TaskHandleDTO | TaskDTO): Promise; $terminateTask(id: string): Promise; $registerTaskSystem(scheme: string, info: TaskSystemInfoDTO): void; + $extensionCallbackTaskComplete(id: string): Promise; } export interface MainThreadExtensionServiceShape extends IDisposable { diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index def67669a93..0b221fc7a18 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -28,7 +28,7 @@ import { import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/node/extHostDocumentsAndEditors'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; -import { ExtHostTerminalService, ExtHostTerminal, ExtHostTerminalRenderer } from 'vs/workbench/api/node/extHostTerminalService'; +import { ExtHostTerminalService, ExtHostTerminalRenderer } from 'vs/workbench/api/node/extHostTerminalService'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; @@ -353,8 +353,8 @@ class ExtensionCallbackExecutionData implements IDisposable { return this._onTaskExecutionComplete.event; } - private onDidCloseTerminal(terminal: vscode.Terminal): void { - if (terminal instanceof ExtHostTerminal && terminal._id === this._terminalId) { + private onDidCloseTerminalRenderer(terminalRenderer: vscode.TerminalRenderer): void { + if (terminalRenderer instanceof ExtHostTerminalRenderer && terminalRenderer._id === this._terminalId) { this._cancellationSource.cancel(); } } @@ -383,16 +383,14 @@ class ExtensionCallbackExecutionData implements IDisposable { this._cancellationSource = new CancellationTokenSource(); this._disposables.push(this._cancellationSource); - this._disposables.push(this.terminalService.onDidCloseTerminal(this.onDidCloseTerminal.bind(this))); + this._disposables.push(this.terminalService.onDidCloseTerminalRenderer(this.onDidCloseTerminalRenderer.bind(this))); // Regardless of how the task completes, we are done with this extension callback task execution. return this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( (success) => { this._onTaskExecutionComplete.fire(this); - terminalRenderer.terminal.dispose(); }, (rejected) => { this._onTaskExecutionComplete.fire(this); - terminalRenderer.terminal.dispose(); }); } @@ -411,7 +409,7 @@ class ExtensionCallbackExecutionData implements IDisposable { // ever changes, this code continues to function. // Check to see if the extension host already knows about this terminal. - for (let terminal of this.terminalService.rendererTerminals) { + for (let terminal of this.terminalService.terminalRenderers) { if (terminal._id === terminalId) { this.onDidOpenRenderTerminal(terminal); return; @@ -420,7 +418,7 @@ class ExtensionCallbackExecutionData implements IDisposable { // If we get here, then the terminal is unknown to the extension host. Let's wait // for it to be created and the start our extension callback. - this._disposables.push(this.terminalService.onDidOpenRendererTerminal(this.onDidOpenRenderTerminal.bind(this))); + this._disposables.push(this.terminalService.onDidOpenTerminalRenderer(this.onDidOpenRenderTerminal.bind(this))); } } @@ -539,7 +537,7 @@ export class ExtHostTask implements ExtHostTaskShape { } const taskExecutionComplete: IDisposable = extensionCallback.onTaskExecutionComplete(() => { - this.terminateExtensionCallbackExecution(execution); + this.extensionCallbackTaskComplete(execution); taskExecutionComplete.dispose(); }); @@ -558,7 +556,7 @@ export class ExtHostTask implements ExtHostTaskShape { public $OnDidEndTask(execution: TaskExecutionDTO): void { const _execution = this.getTaskExecution(execution); this._taskExecutions.delete(execution.id); - this.terminateExtensionCallbackExecution(execution); + this.extensionCallbackTaskComplete(execution); this._onDidTerminateTask.fire({ execution: _execution }); @@ -703,11 +701,12 @@ export class ExtHostTask implements ExtHostTaskShape { return result; } - private terminateExtensionCallbackExecution(execution: TaskExecutionDTO): void { + private extensionCallbackTaskComplete(execution: TaskExecutionDTO): void { const extensionCallback: ExtensionCallbackExecutionData | undefined = this._activeExtensionCallbacks.get(execution.id); if (extensionCallback) { extensionCallback.dispose(); this._activeExtensionCallbacks.delete(execution.id); + this._proxy.$extensionCallbackTaskComplete(execution.id); } } } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 7648b7b98b2..bf8e4582500 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -248,14 +248,16 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { public get activeTerminal(): ExtHostTerminal { return this._activeTerminal; } public get terminals(): ExtHostTerminal[] { return this._terminals; } - public get rendererTerminals(): ExtHostTerminalRenderer[] { return this._terminalRenderers; } + public get terminalRenderers(): ExtHostTerminalRenderer[] { return this._terminalRenderers; } private readonly _onDidCloseTerminal: Emitter = new Emitter(); public get onDidCloseTerminal(): Event { return this._onDidCloseTerminal && this._onDidCloseTerminal.event; } + private readonly _onDidCloseTerminalRenderer: Emitter = new Emitter(); + public get onDidCloseTerminalRenderer(): Event { return this._onDidCloseTerminalRenderer && this._onDidCloseTerminalRenderer.event; } private readonly _onDidOpenTerminal: Emitter = new Emitter(); public get onDidOpenTerminal(): Event { return this._onDidOpenTerminal && this._onDidOpenTerminal.event; } - private readonly _onDidOpenRendererTerminal: Emitter = new Emitter(); - public get onDidOpenRendererTerminal(): Event { return this._onDidOpenRendererTerminal && this._onDidOpenRendererTerminal.event; } + private readonly _onDidOpenTerminalRenderer: Emitter = new Emitter(); + public get onDidOpenTerminalRenderer(): Event { return this._onDidOpenTerminalRenderer && this._onDidOpenTerminalRenderer.event; } private readonly _onDidChangeActiveTerminal: Emitter = new Emitter(); public get onDidChangeActiveTerminal(): Event { return this._onDidChangeActiveTerminal && this._onDidChangeActiveTerminal.event; } @@ -341,11 +343,15 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { public $acceptTerminalClosed(id: number): void { const index = this._getTerminalObjectIndexById(this.terminals, id); - if (index === null) { - return; + if (index !== null) { + const terminal = this._terminals.splice(index, 1)[0]; + this._onDidCloseTerminal.fire(terminal); + } + + const renderer = this._getTerminalRendererById(id); + if (renderer) { + this._onDidCloseTerminalRenderer.fire(renderer); } - const terminal = this._terminals.splice(index, 1)[0]; - this._onDidCloseTerminal.fire(terminal); } public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean): void { @@ -363,7 +369,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { let renderer = this._getTerminalRendererById(id); if (renderer) { // The terminal has already been created (via createTerminal*), only fire the event - this._onDidOpenRendererTerminal.fire(renderer); + this._onDidOpenTerminalRenderer.fire(renderer); } if (renderer || index) { @@ -378,8 +384,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { if (isRendererOnly) { renderer = new ExtHostTerminalRenderer(this._proxy, name, terminal, id); - this.rendererTerminals.push(renderer); - this._onDidOpenRendererTerminal.fire(renderer); + this.terminalRenderers.push(renderer); + this._onDidOpenTerminalRenderer.fire(renderer); } } diff --git a/src/vs/workbench/parts/tasks/common/taskService.ts b/src/vs/workbench/parts/tasks/common/taskService.ts index 827cb4c1cfa..a8f1bb09a04 100644 --- a/src/vs/workbench/parts/tasks/common/taskService.ts +++ b/src/vs/workbench/parts/tasks/common/taskService.ts @@ -82,4 +82,6 @@ export interface ITaskService { registerTaskProvider(taskProvider: ITaskProvider): IDisposable; registerTaskSystem(scheme: string, taskSystemInfo: TaskSystemInfo): void; + + extensionCallbackTaskComplete(task: Task): Promise; } diff --git a/src/vs/workbench/parts/tasks/common/taskSystem.ts b/src/vs/workbench/parts/tasks/common/taskSystem.ts index 722fcffdc93..d9de66ce6d4 100644 --- a/src/vs/workbench/parts/tasks/common/taskSystem.ts +++ b/src/vs/workbench/parts/tasks/common/taskSystem.ts @@ -136,4 +136,5 @@ export interface ITaskSystem { terminate(task: Task): Promise; terminateAll(): Promise; revealTask(task: Task): boolean; + extensionCallbackTaskComplete(task: Task): Promise; } \ No newline at end of file diff --git a/src/vs/workbench/parts/tasks/common/tasks.ts b/src/vs/workbench/parts/tasks/common/tasks.ts index 0343ca42e7c..957987964dd 100644 --- a/src/vs/workbench/parts/tasks/common/tasks.ts +++ b/src/vs/workbench/parts/tasks/common/tasks.ts @@ -593,7 +593,23 @@ export class CustomTask extends CommonTask { } else { let type: string; if (this.command !== undefined) { - type = this.command.runtime === RuntimeType.Shell ? 'shell' : 'process'; + switch (this.command.runtime) { + case RuntimeType.Shell: + type = 'shell'; + break; + + case RuntimeType.Process: + type = 'process'; + break; + + case RuntimeType.ExtensionCallback: + type = 'extensionCallback'; + break; + + default: + throw new Error('Unexpected task runtime'); + break; + } } else { type = '$composite'; } diff --git a/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts index 3d57be482f0..69888c34ea4 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/task.contribution.ts @@ -706,6 +706,13 @@ class TaskService extends Disposable implements ITaskService { this._taskSystemInfos.set(key, info); } + public extensionCallbackTaskComplete(task: Task): Promise { + if (!this._taskSystem) { + return Promise.resolve(); + } + return this._taskSystem.extensionCallbackTaskComplete(task); + } + public getTask(folder: IWorkspaceFolder | string, identifier: string | TaskIdentifier, compareId: boolean = false): Promise { let name = Types.isString(folder) ? folder : folder.name; if (this.ignoredWorkspaceFolders.some(ignored => ignored.name === name)) { diff --git a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts index 57d765804d6..fccf47b33b4 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts @@ -265,6 +265,23 @@ export class TerminalTaskSystem implements ITaskSystem { return Object.keys(this.activeTasks).map(key => this.activeTasks[key].task); } + public extensionCallbackTaskComplete(task: Task): Promise { + const definition = task.getDefinition(); + if (definition === undefined || definition.type !== 'extensionCallback') { + return Promise.resolve(); + } + + let activeTerminal = this.activeTasks[task.getMapKey()]; + if (!activeTerminal) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + activeTerminal.terminal.finishedWithRenderer(); + resolve(); + }); + } + public terminate(task: Task): Promise { let activeTerminal = this.activeTasks[task.getMapKey()]; if (!activeTerminal) { @@ -272,6 +289,7 @@ export class TerminalTaskSystem implements ITaskSystem { } return new Promise((resolve, reject) => { let terminal = activeTerminal.terminal; + const onExit = terminal.onExit(() => { let task = activeTerminal.task; try { @@ -286,6 +304,27 @@ export class TerminalTaskSystem implements ITaskSystem { }); } + public extensionCallbackTaskEnded(task: Task): Promise { + let activeTerminal = this.activeTasks[task.getMapKey()]; + if (!activeTerminal) { + return Promise.resolve({ success: false, task: undefined }); + } + return new Promise((resolve, reject) => { + let terminal = activeTerminal.terminal; + const onDisposed = terminal.onDisposed(() => { + let task = activeTerminal.task; + try { + onDisposed.dispose(); + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.End, task)); + } catch (error) { + // Do nothing. + } + resolve({ success: true, task: task }); + }); + terminal.dispose(); + }); + } + public terminateAll(): Promise { let promises: Promise[] = []; Object.keys(this.activeTasks).forEach((key) => { @@ -908,7 +947,7 @@ export class TerminalTaskSystem implements ITaskSystem { return [terminalToReuse.terminal, commandExecutable, undefined]; } - const result = task.command.runtime !== RuntimeType.ExtensionCallback ? this.terminalService.createTerminal(this.currentTask.shellLaunchConfig) : this.terminalService.createTerminalRenderer(this.createTerminalName(task)); + const result = task.command.runtime !== RuntimeType.ExtensionCallback ? this.terminalService.createTerminal(this.currentTask.shellLaunchConfig) : this.terminalService.createTerminalRenderer(this.createTerminalName(task), waitOnExit); const terminalKey = result.id.toString(); result.onDisposed((terminal) => { let terminalData = this.terminals[terminalKey]; diff --git a/src/vs/workbench/parts/tasks/node/processTaskSystem.ts b/src/vs/workbench/parts/tasks/node/processTaskSystem.ts index 1a5f7caec5c..3d794d1d144 100644 --- a/src/vs/workbench/parts/tasks/node/processTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/node/processTaskSystem.ts @@ -105,6 +105,10 @@ export class ProcessTaskSystem implements ITaskSystem { return true; } + public extensionCallbackTaskComplete(task: Task): Promise { + return Promise.resolve(); + } + public hasErrors(value: boolean): void { this.errorsShown = !value; } diff --git a/src/vs/workbench/parts/terminal/common/terminal.ts b/src/vs/workbench/parts/terminal/common/terminal.ts index 28ff29643b2..c2cf93a3c59 100644 --- a/src/vs/workbench/parts/terminal/common/terminal.ts +++ b/src/vs/workbench/parts/terminal/common/terminal.ts @@ -221,8 +221,10 @@ export interface ITerminalService { /** * Creates a terminal renderer. * @param name The name of the terminal. + * @param waitOnExit Indicates whether to wait on exit when a consumer is finished with the terminal. */ - createTerminalRenderer(name: string): ITerminalInstance; + createTerminalRenderer(name: string, waitOnExit?: string | boolean): ITerminalInstance; + /** * Creates a raw terminal instance, this should not be used outside of the terminal part. */ @@ -432,6 +434,11 @@ export interface ITerminalInstance { */ dispose(immediate?: boolean): void; + /** + * Indicates that a consumer of a renderer only terminal is finished with it. + */ + finishedWithRenderer(): void; + /** * Forces the terminal to redraw its viewport. */ diff --git a/src/vs/workbench/parts/terminal/common/terminalService.ts b/src/vs/workbench/parts/terminal/common/terminalService.ts index 447418f27f5..43dc15a6138 100644 --- a/src/vs/workbench/parts/terminal/common/terminalService.ts +++ b/src/vs/workbench/parts/terminal/common/terminalService.ts @@ -89,7 +89,7 @@ export abstract class TerminalService implements ITerminalService { protected abstract _showTerminalCloseConfirmation(): Promise; protected abstract _showNotEnoughSpaceToast(): void; public abstract createTerminal(shell?: IShellLaunchConfig, wasNewTerminalAction?: boolean): ITerminalInstance; - public abstract createTerminalRenderer(name: string): ITerminalInstance; + public abstract createTerminalRenderer(name: string, waitOnExit?: string | boolean): ITerminalInstance; public abstract createInstance(terminalFocusContextKey: IContextKey, configHelper: ITerminalConfigHelper, container: HTMLElement, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance; public abstract getActiveOrCreateInstance(wasNewTerminalAction?: boolean): ITerminalInstance; public abstract selectDefaultWindowsShell(): Promise; diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts index 57ea6e88a55..b032a5c88de 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts @@ -260,8 +260,6 @@ export class TerminalInstance implements ITerminalInstance { this._logService.trace(`terminalInstance#ctor (id: ${this.id})`, this._shellLaunchConfig); this._initDimensions(); - // There may not be an executable name if a terminal is being used - // for rendering as well as input\output if (!this.shellLaunchConfig.isRendererOnly) { this._createProcess(); } else { @@ -723,7 +721,13 @@ export class TerminalInstance implements ITerminalInstance { } if (this._processManager) { this._processManager.dispose(immediate); + } else { + // In cases where there is no associated process (for example executing an exetnsion callback task) + // consumers still expect on onExit event to be fired. An example of this is terminating the extnesion callback + // task. + this._onExit.fire(this._id); } + if (!this._isDisposed) { this._isDisposed = true; this._onDisposed.fire(this); @@ -731,6 +735,45 @@ export class TerminalInstance implements ITerminalInstance { this._disposables = lifecycle.dispose(this._disposables); } + public finishedWithRenderer(): void { + //NOTE: This looks a lot like _onProcessExit, and it should. + // The use of this API is for cases where there is no backing process + // behind a terminal instance (such as when executiong an extension callback task). + // There is no associated string, error text, etc, as the consumer of the renderer + // can simply outout the text to the renderer themselves. + // All this code does is handle the "wait on exit" condition. + if (!this.shellLaunchConfig.isRendererOnly) { + return; + } + + // Prevent dispose functions being triggered multiple times + if (this._isExiting) { + return; + } + + this._isExiting = true; + + // Only trigger wait on exit when the exit was *not* triggered by the + // user (via the `workbench.action.terminal.kill` command). + if (this._shellLaunchConfig.waitOnExit) { + if (typeof this._shellLaunchConfig.waitOnExit === 'string') { + let message = this._shellLaunchConfig.waitOnExit; + // Bold the message and add an extra new line to make it stand out from the rest of the output + message = `\n\x1b[1m${message}\x1b[0m`; + this._xterm.writeln(message); + } + // Disable all input if the terminal is exiting and listen for next keypress + this._xterm.setOption('disableStdin', true); + if (this._xterm.textarea) { + this._attachPressAnyKeyToCloseListener(); + } + + this._onExit.fire(this._id); + } else { + this.dispose(); + } + } + public forceRedraw(): void { this._xterm.refresh(0, this._xterm.rows - 1); } @@ -1020,15 +1063,19 @@ export class TerminalInstance implements ITerminalInstance { } private _attachPressAnyKeyToCloseListener() { - this._processManager!.addDisposable(dom.addDisposableListener(this._xterm.textarea, 'keypress', (event: KeyboardEvent) => { + const keyPressListener = dom.addDisposableListener(this._xterm.textarea, 'keypress', (event: KeyboardEvent) => { + keyPressListener.dispose(); this.dispose(); event.preventDefault(); - })); + }); } public reuseTerminal(shell: IShellLaunchConfig): void { // Kill and clear up the process, making the process manager ready for a new process - this._processManager!.dispose(); + if (this._processManager) { + this._processManager.dispose(); + this._processManager = undefined; + } // Ensure new processes' output starts at start of new line this._xterm.write('\n\x1b[G'); @@ -1047,12 +1094,19 @@ export class TerminalInstance implements ITerminalInstance { // Set the new shell launch config this._shellLaunchConfig = shell; // Must be done before calling _createProcess() - // Initialize new process - this._createProcess(); + + // Initialize new process if we have one. + if (this._shellLaunchConfig.executable) { + this._createProcess(); + } + if (oldTitle !== this._title) { this.setTitle(this._title, true); } - this._processManager!.onProcessData(data => this._onProcessData(data)); + + if (this._processManager) { + this._processManager.onProcessData(data => this._onProcessData(data)); + } } private _sendRendererInput(input: string): void { diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts index 53a7eaecb0a..0abf47b7810 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts @@ -98,8 +98,8 @@ export class TerminalService extends AbstractTerminalService implements ITermina return instance; } - public createTerminalRenderer(name: string): ITerminalInstance { - return this.createTerminal({ name, isRendererOnly: true }); + public createTerminalRenderer(name: string, waitOnExit?: string | boolean): ITerminalInstance { + return this.createTerminal({ name, isRendererOnly: true, waitOnExit }); } public createInstance(terminalFocusContextKey: IContextKey, configHelper: ITerminalConfigHelper, container: HTMLElement | undefined, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance { From 9953659e5dc99487d8a687b4deee281dcd57a064 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sun, 20 Jan 2019 07:51:36 -0800 Subject: [PATCH 08/36] Fix issues with terminal re-use --- .../electron-browser/terminalTaskSystem.ts | 20 +++++++++++-------- .../parts/terminal/common/terminal.ts | 3 +-- .../parts/terminal/common/terminalService.ts | 2 +- .../electron-browser/terminalService.ts | 4 ++-- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts index fccf47b33b4..bd2a28a8855 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts @@ -903,7 +903,13 @@ export class TerminalTaskSystem implements ITaskSystem { let command: CommandString | undefined; let args: CommandString[] | undefined; - if (task.command.runtime !== RuntimeType.ExtensionCallback) { + if (task.command.runtime === RuntimeType.ExtensionCallback) { + this.currentTask.shellLaunchConfig = { + isRendererOnly: true, + waitOnExit, + name: this.createTerminalName(task) + }; + } else { let resolvedResult: { command: CommandString, args: CommandString[] } = this.resolveCommandAndArgs(resolver, task.command); command = resolvedResult.command; args = resolvedResult.args; @@ -933,21 +939,19 @@ export class TerminalTaskSystem implements ITaskSystem { } } if (terminalToReuse) { - if (task.command.runtime !== RuntimeType.ExtensionCallback) { - if (!this.currentTask.shellLaunchConfig) { - throw new Error('Task shell launch configuration should not be undefined here.'); - } - - terminalToReuse.terminal.reuseTerminal(this.currentTask.shellLaunchConfig); + if (!this.currentTask.shellLaunchConfig) { + throw new Error('Task shell launch configuration should not be undefined here.'); } + terminalToReuse.terminal.reuseTerminal(this.currentTask.shellLaunchConfig); + if (task.command.presentation && task.command.presentation.clear) { terminalToReuse.terminal.clear(); } return [terminalToReuse.terminal, commandExecutable, undefined]; } - const result = task.command.runtime !== RuntimeType.ExtensionCallback ? this.terminalService.createTerminal(this.currentTask.shellLaunchConfig) : this.terminalService.createTerminalRenderer(this.createTerminalName(task), waitOnExit); + const result = this.terminalService.createTerminal(this.currentTask.shellLaunchConfig); const terminalKey = result.id.toString(); result.onDisposed((terminal) => { let terminalData = this.terminals[terminalKey]; diff --git a/src/vs/workbench/parts/terminal/common/terminal.ts b/src/vs/workbench/parts/terminal/common/terminal.ts index c2cf93a3c59..a9254da93dd 100644 --- a/src/vs/workbench/parts/terminal/common/terminal.ts +++ b/src/vs/workbench/parts/terminal/common/terminal.ts @@ -221,9 +221,8 @@ export interface ITerminalService { /** * Creates a terminal renderer. * @param name The name of the terminal. - * @param waitOnExit Indicates whether to wait on exit when a consumer is finished with the terminal. */ - createTerminalRenderer(name: string, waitOnExit?: string | boolean): ITerminalInstance; + createTerminalRenderer(name: string): ITerminalInstance; /** * Creates a raw terminal instance, this should not be used outside of the terminal part. diff --git a/src/vs/workbench/parts/terminal/common/terminalService.ts b/src/vs/workbench/parts/terminal/common/terminalService.ts index 43dc15a6138..447418f27f5 100644 --- a/src/vs/workbench/parts/terminal/common/terminalService.ts +++ b/src/vs/workbench/parts/terminal/common/terminalService.ts @@ -89,7 +89,7 @@ export abstract class TerminalService implements ITerminalService { protected abstract _showTerminalCloseConfirmation(): Promise; protected abstract _showNotEnoughSpaceToast(): void; public abstract createTerminal(shell?: IShellLaunchConfig, wasNewTerminalAction?: boolean): ITerminalInstance; - public abstract createTerminalRenderer(name: string, waitOnExit?: string | boolean): ITerminalInstance; + public abstract createTerminalRenderer(name: string): ITerminalInstance; public abstract createInstance(terminalFocusContextKey: IContextKey, configHelper: ITerminalConfigHelper, container: HTMLElement, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance; public abstract getActiveOrCreateInstance(wasNewTerminalAction?: boolean): ITerminalInstance; public abstract selectDefaultWindowsShell(): Promise; diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts index 0abf47b7810..53a7eaecb0a 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts @@ -98,8 +98,8 @@ export class TerminalService extends AbstractTerminalService implements ITermina return instance; } - public createTerminalRenderer(name: string, waitOnExit?: string | boolean): ITerminalInstance { - return this.createTerminal({ name, isRendererOnly: true, waitOnExit }); + public createTerminalRenderer(name: string): ITerminalInstance { + return this.createTerminal({ name, isRendererOnly: true }); } public createInstance(terminalFocusContextKey: IContextKey, configHelper: ITerminalConfigHelper, container: HTMLElement | undefined, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance { From 68e477d95e20ec9502ef3614dec4997bd717e782 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sun, 20 Jan 2019 07:55:48 -0800 Subject: [PATCH 09/36] Make sure vscode.d.ts is unmodified --- src/vs/vscode.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index 18c5dbf463b..42d9d87e05e 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -5103,6 +5103,7 @@ declare module 'vscode' { * A task to execute */ export class Task { + /** * Creates a new task. * From adff5cabd2df3b7f9ce3ff100c01043ce2fb704b Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sun, 20 Jan 2019 09:43:25 -0800 Subject: [PATCH 10/36] Adress strict null check issue --- .../parts/terminal/electron-browser/terminalInstance.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts index 9df7ba9c07a..03fde800a62 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts @@ -1105,7 +1105,12 @@ export class TerminalInstance implements ITerminalInstance { } if (this._processManager) { - this._processManager.onProcessData(data => this._onProcessData(data)); + // TODO: Why is this a string-null check failure without the "!"? + // The process manager can indeed be undefined when using an extension callback + // as a task, and the if check is correct. + // The "force assume to be not-null !" operator was there before the addition + // of extension callback as task functionality. + this._processManager!.onProcessData(data => this._onProcessData(data)); } } From c62dd7463f5418b7b6a2fba73badf340c1c3e5a2 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sun, 20 Jan 2019 09:46:47 -0800 Subject: [PATCH 11/36] Remove c/c++ tools from built-in-extensions JSON --- build/builtInExtensions.json | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index c12d922c3de..b730189c0f1 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,20 +43,5 @@ }, "publisherDisplayName": "Microsoft" } - }, - { - "name": "ms-vscode.cpptools", - "version": "0.20.1", - "repo": "https://github.com/Microsoft/vscode-cpptools", - "metadata": { - "id": "36d19e17-7569-4841-a001-947eb18602b2", - "publisherId": { - "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", - "publisherName": "ms-vscode", - "displayName": "Microsoft", - "flags": "verified" - }, - "publisherDisplayName": "Microsoft" - } } ] From 971331334e6bd243619b66ffe795d42e1f9d0223 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 23 Jan 2019 09:01:10 -0800 Subject: [PATCH 12/36] Updates based on API change request --- src/vs/vscode.proposed.d.ts | 49 ++++++++++++++++--- .../api/node/extHostTerminalService.ts | 6 +-- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index bd857731fce..38f45c1af80 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -924,7 +924,7 @@ declare module 'vscode' { /** * Represents a terminal without a process where all interaction and output in the terminal is * controlled by an extension. This is similar to an output window but has the same VT sequence - * compatility as the regular terminal. + * compatibility as the regular terminal. * * Note that an instance of [Terminal](#Terminal) will be created when a TerminalRenderer is * created with all its APIs available for use by extensions. When using the Terminal object @@ -972,7 +972,7 @@ declare module 'vscode' { readonly maximumDimensions: TerminalDimensions | undefined; /** - * The corressponding [Terminal](#Terminal) for this TerminalRenderer. + * The corresponding [Terminal](#Terminal) for this TerminalRenderer. */ readonly terminal: Terminal; @@ -1012,7 +1012,7 @@ declare module 'vscode' { readonly onDidAcceptInput: Event; /** - * An event which fires when the [maximum dimensions](#TerminalRenderer.maimumDimensions) of + * An event which fires when the [maximum dimensions](#TerminalRenderer.maximumDimensions) of * the terminal renderer change. */ readonly onDidChangeMaximumDimensions: Event; @@ -1107,7 +1107,7 @@ declare module 'vscode' { } //#endregion - //#region SignatureHelpContext active paramters - mjbvz + //#region SignatureHelpContext active parameters - mjbvz export interface SignatureHelpContext { /** * The currently active [`SignatureHelp`](#SignatureHelp). @@ -1118,6 +1118,43 @@ declare module 'vscode' { } //#endregion + /** + * Interface used to render ANSI output (typically to a [terminal](TerminalRenderer). + */ + export interface AnsiRenderer { + /** + * The dimensions of the renderer, the rows and columns of the renderer can only be set to + * a value smaller than the maximum value, if this is undefined the renderer will auto fit + * to the maximum value [maximumDimensions](AnsiRenderer.maximumDimensions). + */ + dimensions: TerminalDimensions | undefined; + + /** + * The maximum dimensions of the renderer, this will be undefined immediately after a + * renderer is created and also until the renderer becomes visible in the UI. + * Listen to [onDidChangeMaximumDimensions](AnsiRenderer.onDidChangeMaximumDimensions) + * to get notified when this value changes. + */ + readonly maximumDimensions: TerminalDimensions | undefined; + + /** + * Write text to the terminal. + * @param text The text to write. + */ + write(text: string): void; + + /** + * An event which fires on keystrokes entered in the renderer. + */ + readonly onDidAcceptInput: Event; + + /** + * An event which fires when the [maximum dimensions](#AnsiRenderer.maximumDimensions) of + * the renderer change. + */ + readonly onDidChangeMaximumDimensions: Event; + } + /** * Class used to execute an extension callback as a task. */ @@ -1125,12 +1162,12 @@ declare module 'vscode' { /** * @param callback The callback that will be called when the extension callback task is executed. */ - constructor(callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); + constructor(callback: (ansiRenderer: AnsiRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); /** * The callback used to execute the task. */ - callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; + callback: (ansiRenderer: AnsiRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; } /** diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index b2c8adfb3f7..faf59841f87 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -164,7 +164,7 @@ export class ExtHostTerminal extends BaseExtHostTerminal implements vscode.Termi } } -export class ExtHostTerminalRenderer extends BaseExtHostTerminal implements vscode.TerminalRenderer { +export class ExtHostTerminalRenderer extends BaseExtHostTerminal implements vscode.TerminalRenderer, vscode.AnsiRenderer { public get name(): string { return this._name; } public set name(newName: string) { this._name = newName; @@ -361,11 +361,11 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean): void { - // If this is a terminal created by one of the public createrTerminal* APIs + // If this is a terminal created by one of the public createTerminal* APIs // then @acceptTerminalOpened was called from the main thread task // to indicate that the terminal is ready for use. // In those cases, we don't need to create the extension host objects - // as they alrady exist, but, we do still need to fire the events + // as they already exist, but, we do still need to fire the events // to our consumers. const index = this._getTerminalObjectIndexById(this._terminals, id); if (index !== null) { From cb511b2d91e51fc98831971165def040ddac8198 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Fri, 25 Jan 2019 10:18:47 -0800 Subject: [PATCH 13/36] Fix some bugs related to initial terminal dimensions --- .../mainThreadTerminalService.ts | 4 +-- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- .../api/node/extHostTerminalService.ts | 33 +++++++++++++------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts index 52145033512..167e484b36f 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts @@ -183,10 +183,10 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape private _onTerminalOpened(terminalInstance: ITerminalInstance): void { if (terminalInstance.title) { - this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title, terminalInstance.shellLaunchConfig.isRendererOnly); + this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title, terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); } else { terminalInstance.waitForTitle().then(title => { - this._proxy.$acceptTerminalOpened(terminalInstance.id, title, terminalInstance.shellLaunchConfig.isRendererOnly); + this._proxy.$acceptTerminalOpened(terminalInstance.id, title, terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); }); } } diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index ce1717797a0..fcc259dded9 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -928,7 +928,7 @@ export interface ShellLaunchConfigDto { export interface ExtHostTerminalServiceShape { $acceptTerminalClosed(id: number): void; - $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean): void; + $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void; $acceptActiveTerminalChanged(id: number | null): void; $acceptTerminalProcessId(id: number, processId: number): void; $acceptTerminalProcessData(id: number, data: string): void; diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index faf59841f87..a2428efa60a 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -213,9 +213,20 @@ export class ExtHostTerminalRenderer extends BaseExtHostTerminal implements vsco proxy: MainThreadTerminalServiceShape, private _name: string, private _terminal: ExtHostTerminal, - id?: number + id?: number, + cols?: number, + rows?: number ) { super(proxy, id); + + // TODO: Should we set maximum dimensions to these as well? + if (cols !== null && rows !== null) { + this._dimensions = { + columns: cols, + rows: rows + }; + } + if (!id) { this._proxy.$createTerminalRenderer(this._name).then(id => { this._runQueuedRequests(id); @@ -360,7 +371,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } } - public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean): void { + public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void { // If this is a terminal created by one of the public createTerminal* APIs // then @acceptTerminalOpened was called from the main thread task // to indicate that the terminal is ready for use. @@ -378,20 +389,22 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { this._onDidOpenTerminalRenderer.fire(renderer); } - if (renderer || index) { + if ((renderer !== null) && (index !== null)) { return; } // The extension host did not know about this terminal, so create extension host // objects to represent them. - const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined); - this._terminals.push(terminal); - this._onDidOpenTerminal.fire(terminal); + if (!index) { + const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined); + this._terminals.push(terminal); + this._onDidOpenTerminal.fire(terminal); - if (isRendererOnly) { - renderer = new ExtHostTerminalRenderer(this._proxy, name, terminal, id); - this.terminalRenderers.push(renderer); - this._onDidOpenTerminalRenderer.fire(renderer); + if (!renderer && isRendererOnly) { + renderer = new ExtHostTerminalRenderer(this._proxy, name, terminal, id, cols, rows); + this.terminalRenderers.push(renderer); + this._onDidOpenTerminalRenderer.fire(renderer); + } } } From 3f66432eb2a752b1607d7963c0fcb3681c6968f7 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sat, 26 Jan 2019 11:55:40 -0800 Subject: [PATCH 14/36] Code review feedback --- build/builtInExtensions.json | 15 +++++++++++++++ .../api/electron-browser/mainThreadTask.ts | 2 +- .../workbench/api/node/extHostTerminalService.ts | 12 ++++++------ src/vs/workbench/api/node/extHostTypes.ts | 2 -- .../tasks/electron-browser/terminalTaskSystem.ts | 7 +------ 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index 5bc85092c84..fc36319c02e 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,5 +43,20 @@ }, "publisherDisplayName": "Microsoft" } + }, + { + "name": "ms-vscode.cpptools", + "version": "0.20.1", + "repo": "https://github.com/Microsoft/vscode-cpptools", + "metadata": { + "id": "36d19e17-7569-4841-a001-947eb18602b2", + "publisherId": { + "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", + "publisherName": "ms-vscode", + "displayName": "Microsoft", + "flags": "verified" + }, + "publisherDisplayName": "Microsoft" + } } ] diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index f946be24d18..ffb6dbaf806 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -523,7 +523,7 @@ export class MainThreadTask implements MainThreadTaskShape { this._taskService.extensionCallbackTaskComplete(task).then((value) => { resolve(undefined); }, (error) => { - reject(undefined); + reject(error); }); return; } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index a2428efa60a..496cfcf4ee7 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -372,12 +372,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void { - // If this is a terminal created by one of the public createTerminal* APIs - // then @acceptTerminalOpened was called from the main thread task - // to indicate that the terminal is ready for use. - // In those cases, we don't need to create the extension host objects - // as they already exist, but, we do still need to fire the events - // to our consumers. const index = this._getTerminalObjectIndexById(this._terminals, id); if (index !== null) { this._onDidOpenTerminal.fire(this.terminals[index]); @@ -389,6 +383,12 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { this._onDidOpenTerminalRenderer.fire(renderer); } + // If this is a terminal created by one of the public createTerminal* APIs + // then @acceptTerminalOpened was called from the main thread task + // to indicate that the terminal is ready for use. + // In those cases, we don't need to create the extension host objects + // as they already exist, but, we do still need to fire the events + // to our consumers. if ((renderer !== null) && (index !== null)) { return; } diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 10331a230f4..9776609dce2 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1636,8 +1636,6 @@ export class ExtensionCallbackExecution implements vscode.ExtensionCallbackExecu public computeId(): string { const hash = crypto.createHash('md5'); hash.update('extensionCallback'); - // TODO: How is this ID used. Not sure how to create a valid ID from a function. - // TODO: Also not clear on how the ID is used hash.update(generateUuid()); return hash.digest('hex'); } diff --git a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts index bd2a28a8855..df1507cefdd 100644 --- a/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts @@ -266,14 +266,9 @@ export class TerminalTaskSystem implements ITaskSystem { } public extensionCallbackTaskComplete(task: Task): Promise { - const definition = task.getDefinition(); - if (definition === undefined || definition.type !== 'extensionCallback') { - return Promise.resolve(); - } - let activeTerminal = this.activeTasks[task.getMapKey()]; if (!activeTerminal) { - return Promise.resolve(); + return Promise.reject(new Error('Expected to have a terminal for an extension callback task')); } return new Promise((resolve) => { From 5404905f0b07b5f17d8254c1ed5a3d41d91421e5 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sat, 26 Jan 2019 13:00:38 -0800 Subject: [PATCH 15/36] Re-use code in the terminal instance for handling extension task callback shutdown --- src/vs/workbench/api/node/extHostTypes.ts | 12 +--- .../parts/tasks/node/processTaskSystem.ts | 2 +- .../electron-browser/terminalInstance.ts | 58 ++++++------------- 3 files changed, 22 insertions(+), 50 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 9776609dce2..1c792906765 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1787,22 +1787,14 @@ export class Task implements vscode.TaskWithExtensionCallback { } set execution(value: ProcessExecution | ShellExecution | undefined) { - this.executionInternal = value; + this.executionWithExtensionCallback = value; } get executionWithExtensionCallback(): ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined { - return this.executionInternal; - } - - set executionWithExtensionCallback(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { - this.executionInternal = value; - } - - private get executionInternal(): ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined { return this._execution; } - private set executionInternal(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { + set executionWithExtensionCallback(value: ProcessExecution | ShellExecution | ExtensionCallbackExecution | undefined) { if (value === null) { value = undefined; } diff --git a/src/vs/workbench/parts/tasks/node/processTaskSystem.ts b/src/vs/workbench/parts/tasks/node/processTaskSystem.ts index 3d794d1d144..61e388b01d4 100644 --- a/src/vs/workbench/parts/tasks/node/processTaskSystem.ts +++ b/src/vs/workbench/parts/tasks/node/processTaskSystem.ts @@ -106,7 +106,7 @@ export class ProcessTaskSystem implements ITaskSystem { } public extensionCallbackTaskComplete(task: Task): Promise { - return Promise.resolve(); + throw new TaskError(Severity.Error, 'Extension callback task completion is never expected in the process task system.', TaskErrors.UnknownError); } public hasErrors(value: boolean): void { diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts index ed0fc95713a..0949fabe74a 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts @@ -736,42 +736,16 @@ export class TerminalInstance implements ITerminalInstance { } public finishedWithRenderer(): void { - //NOTE: This looks a lot like _onProcessExit, and it should. // The use of this API is for cases where there is no backing process - // behind a terminal instance (such as when executiong an extension callback task). + // behind a terminal instance (such as when executing an extension callback task). // There is no associated string, error text, etc, as the consumer of the renderer - // can simply outout the text to the renderer themselves. + // can simply output the text to the renderer themselves. // All this code does is handle the "wait on exit" condition. if (!this.shellLaunchConfig.isRendererOnly) { return; } - // Prevent dispose functions being triggered multiple times - if (this._isExiting) { - return; - } - - this._isExiting = true; - - // Only trigger wait on exit when the exit was *not* triggered by the - // user (via the `workbench.action.terminal.kill` command). - if (this._shellLaunchConfig.waitOnExit) { - if (typeof this._shellLaunchConfig.waitOnExit === 'string') { - let message = this._shellLaunchConfig.waitOnExit; - // Bold the message and add an extra new line to make it stand out from the rest of the output - message = `\n\x1b[1m${message}\x1b[0m`; - this._xterm.writeln(message); - } - // Disable all input if the terminal is exiting and listen for next keypress - this._xterm.setOption('disableStdin', true); - if (this._xterm.textarea) { - this._attachPressAnyKeyToCloseListener(); - } - - this._onExit.fire(this._id); - } else { - this.dispose(); - } + return this._onProcessOrExtensionCallbackExit(); } public forceRedraw(): void { @@ -958,7 +932,7 @@ export class TerminalInstance implements ITerminalInstance { protected _createProcess(): void { this._processManager = this._instantiationService.createInstance(TerminalProcessManager, this._id, this._configHelper); this._processManager.onProcessReady(() => this._onProcessIdReady.fire(this)); - this._processManager.onProcessExit(exitCode => this._onProcessExit(exitCode)); + this._processManager.onProcessExit(exitCode => this._onProcessOrExtensionCallbackExit(exitCode)); this._processManager.onProcessData(data => this._onData.fire(data)); if (this._shellLaunchConfig.name) { @@ -995,8 +969,12 @@ export class TerminalInstance implements ITerminalInstance { } } - private _onProcessExit(exitCode: number): void { - this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); + private _onProcessOrExtensionCallbackExit(exitCode?: number): void { + // Use 'typeof exitCode' because simply doing if (exitCode) would return false for both "not undefined" and a value of 0 + // which is not the intention. + if (typeof exitCode === `number`) { + this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); + } // Prevent dispose functions being triggered multiple times if (this._isExiting) { @@ -1006,16 +984,18 @@ export class TerminalInstance implements ITerminalInstance { this._isExiting = true; let exitCodeMessage: string; - if (exitCode) { + if (typeof exitCode === `number`) { exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); } - this._logService.debug(`Terminal process exit (id: ${this.id}) state ${this._processManager!.processState}`); + if (this._processManager) { + this._logService.debug(`Terminal process exit (id: ${this.id}) state ${this._processManager!.processState}`); + } // Only trigger wait on exit when the exit was *not* triggered by the // user (via the `workbench.action.terminal.kill` command). - if (this._shellLaunchConfig.waitOnExit && this._processManager!.processState !== ProcessState.KILLED_BY_USER) { - if (exitCode) { + if (this._shellLaunchConfig.waitOnExit && (!this._processManager || this._processManager.processState !== ProcessState.KILLED_BY_USER)) { + if (typeof exitCode === `number`) { this._xterm.writeln(exitCodeMessage!); } if (typeof this._shellLaunchConfig.waitOnExit === 'string') { @@ -1031,8 +1011,8 @@ export class TerminalInstance implements ITerminalInstance { } } else { this.dispose(); - if (exitCode) { - if (this._processManager!.processState === ProcessState.KILLED_DURING_LAUNCH) { + if (typeof exitCode === `number`) { + if (this._processManager && this._processManager!.processState === ProcessState.KILLED_DURING_LAUNCH) { let args = ''; if (typeof this._shellLaunchConfig.args === 'string') { args = this._shellLaunchConfig.args; @@ -1059,7 +1039,7 @@ export class TerminalInstance implements ITerminalInstance { } } - this._onExit.fire(exitCode); + this._onExit.fire((typeof exitCode === `number`) ? exitCode : 0); } private _attachPressAnyKeyToCloseListener() { From 9a6dff1dc2a64c84cf638edc3941bdaec3abc8d2 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sat, 26 Jan 2019 13:59:39 -0800 Subject: [PATCH 16/36] Fix up built-in-extensions.josn --- build/builtInExtensions.json | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index fc36319c02e..5bc85092c84 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,20 +43,5 @@ }, "publisherDisplayName": "Microsoft" } - }, - { - "name": "ms-vscode.cpptools", - "version": "0.20.1", - "repo": "https://github.com/Microsoft/vscode-cpptools", - "metadata": { - "id": "36d19e17-7569-4841-a001-947eb18602b2", - "publisherId": { - "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", - "publisherName": "ms-vscode", - "displayName": "Microsoft", - "flags": "verified" - }, - "publisherDisplayName": "Microsoft" - } } ] From 69db919cf5edb36afefd78f583cb97a5aff813a1 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Mon, 4 Feb 2019 17:52:04 -0800 Subject: [PATCH 17/36] Clean this up a bit and create extension host renderes on demand --- build/builtInExtensions.json | 17 +++- .../mainThreadTerminalService.ts | 9 ++ src/vs/workbench/api/node/extHost.protocol.ts | 1 + src/vs/workbench/api/node/extHostTask.ts | 91 ++++++++----------- .../api/node/extHostTerminalService.ts | 41 ++++----- 5 files changed, 85 insertions(+), 74 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index d0d0107677b..ab33d38451b 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,5 +43,20 @@ }, "publisherDisplayName": "Microsoft" } + }, + { + "name": "ms-vscode.cpptools", + "version": "0.20.1", + "repo": "https://github.com/Microsoft/vscode-cpptools", + "metadata": { + "id": "36d19e17-7569-4841-a001-947eb18602b2", + "publisherId": { + "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", + "publisherName": "ms-vscode", + "displayName": "Microsoft", + "flags": "verified" + }, + "publisherDisplayName": "Microsoft" + } } -] +] \ No newline at end of file diff --git a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts index 223def00fc1..035b0904b16 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts @@ -113,6 +113,15 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape } } + public $terminalGetDimensions(terminalId: number): Promise { + const terminalInstance = this.terminalService.getInstanceFromId(terminalId); + if (terminalInstance && terminalInstance.shellLaunchConfig.isRendererOnly) { + return Promise.resolve({ cols: terminalInstance.cols, rows: terminalInstance.rows }); + } + + return Promise.reject(new Error('Dimensions cannot be retrieved')); + } + public $terminalRendererSetDimensions(terminalId: number, dimensions: ITerminalDimensions): void { const terminalInstance = this.terminalService.getInstanceFromId(terminalId); if (terminalInstance && terminalInstance.shellLaunchConfig.isRendererOnly) { diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 3c55d1f6c67..31de7edb493 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -369,6 +369,7 @@ export interface MainThreadTerminalServiceShape extends IDisposable { // Renderer $terminalRendererSetName(terminalId: number, name: string): void; $terminalRendererSetDimensions(terminalId: number, dimensions: ITerminalDimensions): void; + $terminalGetDimensions(terminalId: number): Promise; $terminalRendererWrite(terminalId: number, text: string): void; $terminalRendererRegisterOnInputListener(terminalId: number): void; } diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 0b221fc7a18..a96ab0933fc 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -28,7 +28,7 @@ import { import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/node/extHostDocumentsAndEditors'; import { ExtHostConfiguration } from 'vs/workbench/api/node/extHostConfiguration'; -import { ExtHostTerminalService, ExtHostTerminalRenderer } from 'vs/workbench/api/node/extHostTerminalService'; +import { ExtHostTerminalService, ExtHostTerminal } from 'vs/workbench/api/node/extHostTerminalService'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; @@ -335,10 +335,10 @@ interface HandlerData { class ExtensionCallbackExecutionData implements IDisposable { private _cancellationSource?: CancellationTokenSource; - private _onDidOpenRendererTerminal?: IDisposable; - private _terminalId?: number; private readonly _onTaskExecutionComplete: Emitter = new Emitter(); private readonly _disposables: IDisposable[] = []; + private terminal?: vscode.Terminal; + private terminalId?: number; constructor( private readonly callbackData: vscode.ExtensionCallbackExecution, @@ -353,13 +353,25 @@ class ExtensionCallbackExecutionData implements IDisposable { return this._onTaskExecutionComplete.event; } - private onDidCloseTerminalRenderer(terminalRenderer: vscode.TerminalRenderer): void { - if (terminalRenderer instanceof ExtHostTerminalRenderer && terminalRenderer._id === this._terminalId) { + private onDidCloseTerminal(terminal: vscode.Terminal): void { + if (this.terminal === terminal) { this._cancellationSource.cancel(); } } - private onDidOpenRenderTerminal(terminalRenderer: vscode.TerminalRenderer): Thenable { + private onDidOpenTerminal(terminal: vscode.Terminal): void { + if (!(terminal instanceof ExtHostTerminal)) { + throw new Error('How could this not be a extension host terminal?'); + } + + if (this.terminalId && terminal._id === this.terminalId) { + this.startCallback(this.terminalId); + } + } + + public async startCallback(terminalId: number): Promise { + this.terminalId = terminalId; + // If we have already started the extension task callback, then // do not start it again. // It is completely valid for multiple terminals to be opened @@ -368,57 +380,32 @@ class ExtensionCallbackExecutionData implements IDisposable { return undefined; } - if (terminalRenderer instanceof ExtHostTerminalRenderer && terminalRenderer._id === this._terminalId) { - // Stop listening (if we are) for more terminals - // to be created. - if (this._onDidOpenRendererTerminal) { - this._onDidOpenRendererTerminal.dispose(); - this._onDidOpenRendererTerminal = undefined; - } + const callbackTerminals: vscode.Terminal[] = this.terminalService.terminals.filter((terminal) => terminal._id === terminalId); - if (!(terminalRenderer instanceof ExtHostTerminalRenderer)) { - throw new Error('Expected a terminal renderer'); - } - - this._cancellationSource = new CancellationTokenSource(); - this._disposables.push(this._cancellationSource); - - this._disposables.push(this.terminalService.onDidCloseTerminalRenderer(this.onDidCloseTerminalRenderer.bind(this))); - - // Regardless of how the task completes, we are done with this extension callback task execution. - return this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( - (success) => { - this._onTaskExecutionComplete.fire(this); - }, (rejected) => { - this._onTaskExecutionComplete.fire(this); - }); + if (!callbackTerminals || callbackTerminals.length === 0) { + this._disposables.push(this.terminalService.onDidOpenTerminal(this.onDidOpenTerminal.bind(this))); + return; } - return undefined; - } - - public startCallback(terminalId: number): void { - this._terminalId = terminalId; - - // In order to start the task, we need to wait for the extension host - // to know about the new terminal. - // The order in which the events make it to the extension host (currently) - // is "task created" followed by "terminal opened". - // So, we need to wait for that event. - // However, this loop below ensures that if the order of those events - // ever changes, this code continues to function. - - // Check to see if the extension host already knows about this terminal. - for (let terminal of this.terminalService.terminalRenderers) { - if (terminal._id === terminalId) { - this.onDidOpenRenderTerminal(terminal); - return; - } + if (callbackTerminals.length !== 1) { + throw new Error(`Expected to only have one terminal at this point`); } - // If we get here, then the terminal is unknown to the extension host. Let's wait - // for it to be created and the start our extension callback. - this._disposables.push(this.terminalService.onDidOpenTerminalRenderer(this.onDidOpenRenderTerminal.bind(this))); + this.terminal = callbackTerminals[0]; + const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.createTerminalRendererForTerminal(this.terminal); + + this._cancellationSource = new CancellationTokenSource(); + this._disposables.push(this._cancellationSource); + + this._disposables.push(this.terminalService.onDidCloseTerminal(this.onDidCloseTerminal.bind(this))); + + // Regardless of how the task completes, we are done with this extension callback task execution. + this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( + (success) => { + this._onTaskExecutionComplete.fire(this); + }, (rejected) => { + this._onTaskExecutionComplete.fire(this); + }); } } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 9faeb66533d..e129b6a62bd 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -269,12 +269,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { private readonly _onDidCloseTerminal: Emitter = new Emitter(); public get onDidCloseTerminal(): Event { return this._onDidCloseTerminal && this._onDidCloseTerminal.event; } - private readonly _onDidCloseTerminalRenderer: Emitter = new Emitter(); - public get onDidCloseTerminalRenderer(): Event { return this._onDidCloseTerminalRenderer && this._onDidCloseTerminalRenderer.event; } private readonly _onDidOpenTerminal: Emitter = new Emitter(); public get onDidOpenTerminal(): Event { return this._onDidOpenTerminal && this._onDidOpenTerminal.event; } - private readonly _onDidOpenTerminalRenderer: Emitter = new Emitter(); - public get onDidOpenTerminalRenderer(): Event { return this._onDidOpenTerminalRenderer && this._onDidOpenTerminalRenderer.event; } private readonly _onDidChangeActiveTerminal: Emitter = new Emitter(); public get onDidChangeActiveTerminal(): Event { return this._onDidChangeActiveTerminal && this._onDidChangeActiveTerminal.event; } @@ -312,6 +308,24 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return renderer; } + public async createTerminalRendererForTerminal(terminal: vscode.Terminal): Promise { + if (!(terminal instanceof ExtHostTerminal)) { + throw new Error('Only expected instance extension host terminal type'); + } + // Check to see if the extension host already knows about this terminal. + for (const terminalRenderer of this.terminalRenderers) { + if (terminalRenderer._id === terminal._id) { + return terminalRenderer; + } + } + + const dimensions = await this._proxy.$terminalGetDimensions(terminal._id); + const renderer = new ExtHostTerminalRenderer(this._proxy, terminal.name, terminal, terminal._id, dimensions.cols, dimensions.rows); + this._terminalRenderers.push(renderer); + + return renderer; + } + public $acceptActiveTerminalChanged(id: number | null): void { const original = this._activeTerminal; if (id === null) { @@ -365,24 +379,15 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { const terminal = this._terminals.splice(index, 1)[0]; this._onDidCloseTerminal.fire(terminal); } - - const renderer = this._getTerminalRendererById(id); - if (renderer) { - this._onDidCloseTerminalRenderer.fire(renderer); - } } public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void { - const index = this._getTerminalObjectIndexById(this._terminals, id); + let index = this._getTerminalObjectIndexById(this._terminals, id); if (index !== null) { this._onDidOpenTerminal.fire(this.terminals[index]); } let renderer = this._getTerminalRendererById(id); - if (renderer) { - // The terminal has already been created (via createTerminal*), only fire the event - this._onDidOpenTerminalRenderer.fire(renderer); - } // If this is a terminal created by one of the public createTerminal* APIs // then @acceptTerminalOpened was called from the main thread task @@ -398,14 +403,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { // objects to represent them. if (!index) { const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined); - this._terminals.push(terminal); + index = this._terminals.push(terminal); this._onDidOpenTerminal.fire(terminal); - - if (!renderer && isRendererOnly) { - renderer = new ExtHostTerminalRenderer(this._proxy, name, terminal, id, cols, rows); - this.terminalRenderers.push(renderer); - this._onDidOpenTerminalRenderer.fire(renderer); - } } } From d2e8c8465e641262221b603aa1c081660599de9e Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Tue, 12 Feb 2019 10:48:40 -0800 Subject: [PATCH 18/36] Cleanup round one --- .../api/electron-browser/mainThreadTask.ts | 11 +++-- .../mainThreadTerminalService.ts | 4 +- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostTask.ts | 43 +++++++++++-------- .../contrib/tasks/common/taskService.ts | 2 +- .../contrib/tasks/common/taskSystem.ts | 2 +- .../workbench/contrib/tasks/common/tasks.ts | 6 +-- .../electron-browser/task.contribution.ts | 4 +- .../electron-browser/terminalTaskSystem.ts | 14 +++--- .../contrib/tasks/node/processTaskSystem.ts | 2 +- .../contrib/terminal/common/terminal.ts | 2 +- .../electron-browser/terminalInstance.ts | 4 +- 12 files changed, 50 insertions(+), 46 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index 2b8f2332153..f32f986225b 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -30,9 +30,8 @@ import { extHostNamedCustomer } from 'vs/workbench/api/electron-browser/extHostC import { ExtHostContext, MainThreadTaskShape, ExtHostTaskShape, MainContext, IExtHostContext } from 'vs/workbench/api/node/extHost.protocol'; import { TaskDefinitionDTO, TaskExecutionDTO, ProcessExecutionOptionsDTO, TaskPresentationOptionsDTO, - ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, - RunOptionsDTO, - CustomTaskExecutionDTO + ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, CustomTaskExecutionDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, + RunOptionsDTO } from 'vs/workbench/api/shared/tasks'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; @@ -252,7 +251,7 @@ namespace CustomTaskExecutionDTO { export function to(value: CustomTaskExecutionDTO): CommandConfiguration { return { - runtime: RuntimeType.ExtensionCallback, + runtime: RuntimeType.CustomTaskExecution, presentation: undefined }; } @@ -519,12 +518,12 @@ export class MainThreadTask implements MainThreadTaskShape { }); } - public $extensionCallbackTaskComplete(id: string): Promise { + public $customTaskExecutionComplete(id: string, result: number | undefined): Promise { return new Promise((resolve, reject) => { this._taskService.getActiveTasks().then((tasks) => { for (let task of tasks) { if (id === task._id) { - this._taskService.extensionCallbackTaskComplete(task).then((value) => { + this._taskService.extensionCallbackTaskComplete(task, result).then((value) => { resolve(undefined); }, (error) => { reject(error); diff --git a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts index efe7ac0adf7..3250ee93d64 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts @@ -196,10 +196,10 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape private _onTerminalOpened(terminalInstance: ITerminalInstance): void { if (terminalInstance.title) { - this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title, terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); + this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title, !!terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); } else { terminalInstance.waitForTitle().then(title => { - this._proxy.$acceptTerminalOpened(terminalInstance.id, title, terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); + this._proxy.$acceptTerminalOpened(terminalInstance.id, title, !!terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); }); } } diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 9b5701a051a..4e317caea49 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -548,7 +548,7 @@ export interface MainThreadTaskShape extends IDisposable { $executeTask(task: TaskHandleDTO | TaskDTO): Promise; $terminateTask(id: string): Promise; $registerTaskSystem(scheme: string, info: TaskSystemInfoDTO): void; - $extensionCallbackTaskComplete(id: string): Promise; + $customTaskExecutionComplete(id: string, result: number | undefined): Promise; } export interface MainThreadExtensionServiceShape extends IDisposable { diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 2b7284634b8..4c575fecba2 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -336,12 +336,13 @@ interface HandlerData { extension: IExtensionDescription; } -class ExtensionCallbackExecutionData implements IDisposable { +class CustomTaskExecutionData implements IDisposable { private _cancellationSource?: CancellationTokenSource; - private readonly _onTaskExecutionComplete: Emitter = new Emitter(); + private readonly _onTaskExecutionComplete: Emitter = new Emitter(); private readonly _disposables: IDisposable[] = []; private terminal?: vscode.Terminal; private terminalId?: number; + public result: number | undefined; constructor( private readonly callbackData: vscode.CustomTaskExecution, @@ -352,7 +353,7 @@ class ExtensionCallbackExecutionData implements IDisposable { dispose(this._disposables); } - public get onTaskExecutionComplete(): Event { + public get onTaskExecutionComplete(): Event { return this._onTaskExecutionComplete.event; } @@ -405,6 +406,7 @@ class ExtensionCallbackExecutionData implements IDisposable { // Regardless of how the task completes, we are done with this extension callback task execution. this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( (success) => { + this.result = success; this._onTaskExecutionComplete.fire(this); }, (rejected) => { this._onTaskExecutionComplete.fire(this); @@ -422,8 +424,8 @@ export class ExtHostTask implements ExtHostTaskShape { private _handleCounter: number; private _handlers: Map; private _taskExecutions: Map; - private _providedExtensionCallbacks: Map; - private _activeExtensionCallbacks: Map; + private _providedCustomTaskExecutions: Map; + private _activeCustomTaskExecutions: Map; private readonly _onDidExecuteTask: Emitter = new Emitter(); private readonly _onDidTerminateTask: Emitter = new Emitter(); @@ -445,8 +447,8 @@ export class ExtHostTask implements ExtHostTaskShape { this._handleCounter = 0; this._handlers = new Map(); this._taskExecutions = new Map(); - this._providedExtensionCallbacks = new Map(); - this._activeExtensionCallbacks = new Map(); + this._providedCustomTaskExecutions = new Map(); + this._activeCustomTaskExecutions = new Map(); } public registerTaskProvider(extension: IExtensionDescription, provider: vscode.TaskProvider): vscode.Disposable { @@ -517,15 +519,16 @@ export class ExtHostTask implements ExtHostTaskShape { // this event will be fired. // At that point, we need to actually start the callback, but // only if it hasn't already begun. - const extensionCallback: ExtensionCallbackExecutionData | undefined = this._providedExtensionCallbacks.get(execution.id); + const extensionCallback: CustomTaskExecutionData | undefined = this._providedCustomTaskExecutions.get(execution.id); if (extensionCallback) { - // TODO: Verify whether this can ever happen??? - if (this._activeExtensionCallbacks.get(execution.id) === undefined) { - this._activeExtensionCallbacks.set(execution.id, extensionCallback); + if (this._activeCustomTaskExecutions.get(execution.id) !== undefined) { + throw new Error('We should not be trying to start the same custom task executions twice.'); } + this._activeCustomTaskExecutions.set(execution.id, extensionCallback); + const taskExecutionComplete: IDisposable = extensionCallback.onTaskExecutionComplete(() => { - this.extensionCallbackTaskComplete(execution); + this.customTaskExecutionComplete(execution); taskExecutionComplete.dispose(); }); @@ -546,7 +549,7 @@ export class ExtHostTask implements ExtHostTaskShape { const workspaceProvider = await this._workspaceService.getWorkspaceProvider(); const _execution = this.getTaskExecution(execution, workspaceProvider); this._taskExecutions.delete(execution.id); - this.extensionCallbackTaskComplete(execution); + this.customTaskExecutionComplete(execution); this._onDidTerminateTask.fire({ execution: _execution }); @@ -591,7 +594,7 @@ export class ExtHostTask implements ExtHostTaskShape { // For extension callback tasks, we need to store the execution objects locally // since we obviously cannot send callback functions through the proxy. // So, clear out any existing ones. - this._providedExtensionCallbacks.clear(); + this._providedCustomTaskExecutions.clear(); // Set up a list of task ID promises that we can wait on // before returning the provided tasks. The ensures that @@ -615,8 +618,10 @@ export class ExtHostTask implements ExtHostTaskShape { if (CustomTaskExecutionDTO.is(taskDTO.execution)) { taskIdPromises.push(new Promise((resolve) => { // The ID is calculated on the main thread task side, so, let's call into it here. + // We need the task id's pre-computed for custom task executions because when OnDidStartTask + // is invoked, we have to be able to map it back to our data. this._proxy.$createTaskId(taskDTO).then((taskId) => { - this._providedExtensionCallbacks.set(taskId, new ExtensionCallbackExecutionData((task).executionWithExtensionCallback, this._terminalService)); + this._providedCustomTaskExecutions.set(taskId, new CustomTaskExecutionData((task).executionWithExtensionCallback, this._terminalService)); resolve(); }); })); @@ -694,12 +699,12 @@ export class ExtHostTask implements ExtHostTaskShape { return result; } - private extensionCallbackTaskComplete(execution: TaskExecutionDTO): void { - const extensionCallback: ExtensionCallbackExecutionData | undefined = this._activeExtensionCallbacks.get(execution.id); + private customTaskExecutionComplete(execution: TaskExecutionDTO): void { + const extensionCallback: CustomTaskExecutionData | undefined = this._activeCustomTaskExecutions.get(execution.id); if (extensionCallback) { + this._activeCustomTaskExecutions.delete(execution.id); + this._proxy.$customTaskExecutionComplete(execution.id, extensionCallback.result); extensionCallback.dispose(); - this._activeExtensionCallbacks.delete(execution.id); - this._proxy.$extensionCallbackTaskComplete(execution.id); } } } diff --git a/src/vs/workbench/contrib/tasks/common/taskService.ts b/src/vs/workbench/contrib/tasks/common/taskService.ts index 559f6bf3318..1e07f82b769 100644 --- a/src/vs/workbench/contrib/tasks/common/taskService.ts +++ b/src/vs/workbench/contrib/tasks/common/taskService.ts @@ -83,5 +83,5 @@ export interface ITaskService { registerTaskSystem(scheme: string, taskSystemInfo: TaskSystemInfo): void; - extensionCallbackTaskComplete(task: Task): Promise; + extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise; } diff --git a/src/vs/workbench/contrib/tasks/common/taskSystem.ts b/src/vs/workbench/contrib/tasks/common/taskSystem.ts index d9de66ce6d4..de574c37413 100644 --- a/src/vs/workbench/contrib/tasks/common/taskSystem.ts +++ b/src/vs/workbench/contrib/tasks/common/taskSystem.ts @@ -136,5 +136,5 @@ export interface ITaskSystem { terminate(task: Task): Promise; terminateAll(): Promise; revealTask(task: Task): boolean; - extensionCallbackTaskComplete(task: Task): Promise; + extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise; } \ No newline at end of file diff --git a/src/vs/workbench/contrib/tasks/common/tasks.ts b/src/vs/workbench/contrib/tasks/common/tasks.ts index 3fa163d169a..e3c408f8a7a 100644 --- a/src/vs/workbench/contrib/tasks/common/tasks.ts +++ b/src/vs/workbench/contrib/tasks/common/tasks.ts @@ -230,7 +230,7 @@ export namespace PresentationOptions { export enum RuntimeType { Shell = 1, Process = 2, - ExtensionCallback = 3 + CustomTaskExecution = 3 } export namespace RuntimeType { @@ -241,7 +241,7 @@ export namespace RuntimeType { case 'process': return RuntimeType.Process; case 'customTaskExecution': - return RuntimeType.ExtensionCallback; + return RuntimeType.CustomTaskExecution; default: return RuntimeType.Process; } @@ -607,7 +607,7 @@ export class CustomTask extends CommonTask { type = 'process'; break; - case RuntimeType.ExtensionCallback: + case RuntimeType.CustomTaskExecution: type = 'customTaskExecution'; break; diff --git a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts index 49db090d136..a21382a9f86 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts @@ -706,11 +706,11 @@ class TaskService extends Disposable implements ITaskService { this._taskSystemInfos.set(key, info); } - public extensionCallbackTaskComplete(task: Task): Promise { + public extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise { if (!this._taskSystem) { return Promise.resolve(); } - return this._taskSystem.extensionCallbackTaskComplete(task); + return this._taskSystem.extensionCallbackTaskComplete(task, result); } public getTask(folder: IWorkspaceFolder | string, identifier: string | TaskIdentifier, compareId: boolean = false): Promise { diff --git a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts index ad3d26739ec..10bb828e198 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts @@ -266,14 +266,14 @@ export class TerminalTaskSystem implements ITaskSystem { return Object.keys(this.activeTasks).map(key => this.activeTasks[key].task); } - public extensionCallbackTaskComplete(task: Task): Promise { + public extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise { let activeTerminal = this.activeTasks[task.getMapKey()]; if (!activeTerminal) { return Promise.reject(new Error('Expected to have a terminal for an extension callback task')); } return new Promise((resolve) => { - activeTerminal.terminal.finishedWithRenderer(); + activeTerminal.terminal.finishedWithRenderer(result); resolve(); }); } @@ -550,7 +550,7 @@ export class TerminalTaskSystem implements ITaskSystem { let processStartedSignaled = false; terminal.processReady.then(() => { if (!processStartedSignaled) { - if (task.command.runtime !== RuntimeType.ExtensionCallback) { + if (task.command.runtime !== RuntimeType.CustomTaskExecution) { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); } processStartedSignaled = true; @@ -622,7 +622,7 @@ export class TerminalTaskSystem implements ITaskSystem { let processStartedSignaled = false; terminal.processReady.then(() => { if (!processStartedSignaled) { - if (task.command.runtime !== RuntimeType.ExtensionCallback) { + if (task.command.runtime !== RuntimeType.CustomTaskExecution) { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); } processStartedSignaled = true; @@ -826,7 +826,7 @@ export class TerminalTaskSystem implements ITaskSystem { } } } else { - let commandExecutable = task.command.runtime !== RuntimeType.ExtensionCallback ? CommandString.value(command) : undefined; + let commandExecutable = task.command.runtime !== RuntimeType.CustomTaskExecution ? CommandString.value(command) : undefined; let executable = !isShellCommand ? this.resolveVariable(variableResolver, '${' + TerminalTaskSystem.ProcessVarName + '}') : commandExecutable; @@ -906,7 +906,7 @@ export class TerminalTaskSystem implements ITaskSystem { let command: CommandString | undefined; let args: CommandString[] | undefined; - if (task.command.runtime === RuntimeType.ExtensionCallback) { + if (task.command.runtime === RuntimeType.CustomTaskExecution) { this.currentTask.shellLaunchConfig = { isRendererOnly: true, waitOnExit, @@ -1126,7 +1126,7 @@ export class TerminalTaskSystem implements ITaskSystem { private collectCommandVariables(variables: Set, command: CommandConfiguration, task: CustomTask | ContributedTask): void { // An extension callback should have everything it needs already as it provided // the callback. - if (command.runtime === RuntimeType.ExtensionCallback) { + if (command.runtime === RuntimeType.CustomTaskExecution) { return; } diff --git a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts index 532c3adeed1..62850a8600b 100644 --- a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts @@ -105,7 +105,7 @@ export class ProcessTaskSystem implements ITaskSystem { return true; } - public extensionCallbackTaskComplete(task: Task): Promise { + public extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise { throw new TaskError(Severity.Error, 'Extension callback task completion is never expected in the process task system.', TaskErrors.UnknownError); } diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 6dd06a68ffa..283a3bb27e7 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -432,7 +432,7 @@ export interface ITerminalInstance { /** * Indicates that a consumer of a renderer only terminal is finished with it. */ - finishedWithRenderer(): void; + finishedWithRenderer(result: number | undefined): void; /** * Forces the terminal to redraw its viewport. diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index d1bf39ae507..7212ec53b1a 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -754,7 +754,7 @@ export class TerminalInstance implements ITerminalInstance { this._disposables = lifecycle.dispose(this._disposables); } - public finishedWithRenderer(): void { + public finishedWithRenderer(result: number | undefined): void { // The use of this API is for cases where there is no backing process // behind a terminal instance (such as when executing an extension callback task). // There is no associated string, error text, etc, as the consumer of the renderer @@ -764,7 +764,7 @@ export class TerminalInstance implements ITerminalInstance { return; } - return this._onProcessOrExtensionCallbackExit(); + return this._onProcessOrExtensionCallbackExit(result); } public forceRedraw(): void { From 27ff8463c03bf753eae2f615a95f62091d19dd66 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sun, 17 Feb 2019 08:46:30 -0800 Subject: [PATCH 19/36] Remove C/CPP tools again --- build/builtInExtensions.json | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index f88b607bfbc..ecc0b8c72ae 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,20 +43,5 @@ }, "publisherDisplayName": "Microsoft" } - }, - { - "name": "ms-vscode.cpptools", - "version": "0.20.1", - "repo": "https://github.com/Microsoft/vscode-cpptools", - "metadata": { - "id": "36d19e17-7569-4841-a001-947eb18602b2", - "publisherId": { - "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", - "publisherName": "ms-vscode", - "displayName": "Microsoft", - "flags": "verified" - }, - "publisherDisplayName": "Microsoft" - } } ] \ No newline at end of file From a7565721ba7ec0710b0a1bbe6345be51afef61b1 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Sun, 17 Feb 2019 09:18:41 -0800 Subject: [PATCH 20/36] Clean up comments, fix some code review issues, and make the exit code optional throughout to make it clear that it isn't always a number --- .../api/electron-browser/mainThreadTask.ts | 2 +- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostTask.ts | 13 +++++----- .../api/node/extHostTerminalService.ts | 3 +-- .../contrib/tasks/common/taskSystem.ts | 2 +- .../workbench/contrib/tasks/common/tasks.ts | 2 +- .../electron-browser/task.contribution.ts | 2 +- .../electron-browser/terminalTaskSystem.ts | 6 ++--- .../contrib/tasks/node/processTaskSystem.ts | 4 +-- .../contrib/terminal/common/terminal.ts | 6 +++-- .../electron-browser/terminalInstance.ts | 25 ++++++++++--------- 11 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index f32f986225b..344f03f96d6 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -518,7 +518,7 @@ export class MainThreadTask implements MainThreadTaskShape { }); } - public $customTaskExecutionComplete(id: string, result: number | undefined): Promise { + public $customTaskExecutionComplete(id: string, result?: number): Promise { return new Promise((resolve, reject) => { this._taskService.getActiveTasks().then((tasks) => { for (let task of tasks) { diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 60175a1a0fa..806dd35f10b 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -554,7 +554,7 @@ export interface MainThreadTaskShape extends IDisposable { $executeTask(task: TaskHandleDTO | TaskDTO): Promise; $terminateTask(id: string): Promise; $registerTaskSystem(scheme: string, info: TaskSystemInfoDTO): void; - $customTaskExecutionComplete(id: string, result: number | undefined): Promise; + $customTaskExecutionComplete(id: string, result?: number): Promise; } export interface MainThreadExtensionServiceShape extends IDisposable { diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index eb5c05321fa..6713812a53a 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -403,7 +403,7 @@ class CustomTaskExecutionData implements IDisposable { this._disposables.push(this.terminalService.onDidCloseTerminal(this.onDidCloseTerminal.bind(this))); - // Regardless of how the task completes, we are done with this extension callback task execution. + // Regardless of how the task completes, we are done with this custom execution task. this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( (success) => { this.result = success; @@ -515,8 +515,7 @@ export class ExtHostTask implements ExtHostTaskShape { } public async $onDidStartTask(execution: TaskExecutionDTO, terminalId: number): Promise { - // Once a terminal is spun up for the extension callback task execution - // this event will be fired. + // Once a terminal is spun up for the custom execution task this event will be fired. // At that point, we need to actually start the callback, but // only if it hasn't already begun. const extensionCallback: CustomTaskExecutionData | undefined = this._providedCustomTaskExecutions.get(execution.id); @@ -591,19 +590,19 @@ export class ExtHostTask implements ExtHostTaskShape { return Promise.reject(new Error('no handler found')); } - // For extension callback tasks, we need to store the execution objects locally + // For custom execution tasks, we need to store the execution objects locally // since we obviously cannot send callback functions through the proxy. // So, clear out any existing ones. this._providedCustomTaskExecutions.clear(); // Set up a list of task ID promises that we can wait on // before returning the provided tasks. The ensures that - // our task IDs are calculated for any extension callback tasks. + // our task IDs are calculated for any custom execution tasks. // Knowing this ID ahead of time is needed because when a task - // start event is fired this is when the extension callback is called. + // start event is fired this is when the custom execution is called. // The task start event is also the first time we see the ID from the main // thread, which is too late for us because we need to save an map - // from an ID to an extension callback function. (Kind of a cart before the horse problem). + // from an ID to the custom execution function. (Kind of a cart before the horse problem). let taskIdPromises: Promise[] = []; let fetchPromise = asPromise(() => handler.provider.provideTasks(CancellationToken.None)).then(value => { const taskDTOs: TaskDTO[] = []; diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 9c5703d459a..fe3457e5f8b 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -288,7 +288,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { public get activeTerminal(): ExtHostTerminal { return this._activeTerminal; } public get terminals(): ExtHostTerminal[] { return this._terminals; } - public get terminalRenderers(): ExtHostTerminalRenderer[] { return this._terminalRenderers; } private readonly _onDidCloseTerminal: Emitter = new Emitter(); public get onDidCloseTerminal(): Event { return this._onDidCloseTerminal && this._onDidCloseTerminal.event; } @@ -338,7 +337,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { throw new Error('Only expected instance extension host terminal type'); } // Check to see if the extension host already knows about this terminal. - for (const terminalRenderer of this.terminalRenderers) { + for (const terminalRenderer of this._terminalRenderers) { if (terminalRenderer._id === terminal._id) { return terminalRenderer; } diff --git a/src/vs/workbench/contrib/tasks/common/taskSystem.ts b/src/vs/workbench/contrib/tasks/common/taskSystem.ts index de574c37413..d2f6f1de695 100644 --- a/src/vs/workbench/contrib/tasks/common/taskSystem.ts +++ b/src/vs/workbench/contrib/tasks/common/taskSystem.ts @@ -136,5 +136,5 @@ export interface ITaskSystem { terminate(task: Task): Promise; terminateAll(): Promise; revealTask(task: Task): boolean; - extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise; + customTaskExecutionComplete(task: Task, result: number | undefined): Promise; } \ No newline at end of file diff --git a/src/vs/workbench/contrib/tasks/common/tasks.ts b/src/vs/workbench/contrib/tasks/common/tasks.ts index e3c408f8a7a..d5937a9f1f9 100644 --- a/src/vs/workbench/contrib/tasks/common/tasks.ts +++ b/src/vs/workbench/contrib/tasks/common/tasks.ts @@ -891,7 +891,7 @@ export const enum TaskRunSource { } export namespace TaskEvent { - export function create(kind: TaskEventKind.ProcessStarted | TaskEventKind.ProcessEnded, task: Task, processIdOrExitCode: number): TaskEvent; + export function create(kind: TaskEventKind.ProcessStarted | TaskEventKind.ProcessEnded, task: Task, processIdOrExitCode?: number): TaskEvent; export function create(kind: TaskEventKind.Start, task: Task, terminalId?: number): TaskEvent; export function create(kind: TaskEventKind.DependsOnStarted | TaskEventKind.Start | TaskEventKind.Active | TaskEventKind.Inactive | TaskEventKind.Terminated | TaskEventKind.End, task: Task): TaskEvent; export function create(kind: TaskEventKind.Changed): TaskEvent; diff --git a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts index 27045da5cea..6f53b37fa6b 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts @@ -722,7 +722,7 @@ class TaskService extends Disposable implements ITaskService { if (!this._taskSystem) { return Promise.resolve(); } - return this._taskSystem.extensionCallbackTaskComplete(task, result); + return this._taskSystem.customTaskExecutionComplete(task, result); } public getTask(folder: IWorkspaceFolder | string, identifier: string | TaskIdentifier, compareId: boolean = false): Promise { diff --git a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts index 5a09245b2aa..531d01d5fd0 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts @@ -265,10 +265,10 @@ export class TerminalTaskSystem implements ITaskSystem { return Object.keys(this.activeTasks).map(key => this.activeTasks[key].task); } - public extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise { + public customTaskExecutionComplete(task: Task, result?: number): Promise { let activeTerminal = this.activeTasks[task.getMapKey()]; if (!activeTerminal) { - return Promise.reject(new Error('Expected to have a terminal for an extension callback task')); + return Promise.reject(new Error('Expected to have a terminal for an custom execution task')); } return new Promise((resolve) => { @@ -1114,7 +1114,7 @@ export class TerminalTaskSystem implements ITaskSystem { } private collectCommandVariables(variables: Set, command: CommandConfiguration, task: CustomTask | ContributedTask): void { - // An extension callback should have everything it needs already as it provided + // The custom execution should have everything it needs already as it provided // the callback. if (command.runtime === RuntimeType.CustomTaskExecution) { return; diff --git a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts index 62850a8600b..e12684b46fb 100644 --- a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts @@ -105,8 +105,8 @@ export class ProcessTaskSystem implements ITaskSystem { return true; } - public extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise { - throw new TaskError(Severity.Error, 'Extension callback task completion is never expected in the process task system.', TaskErrors.UnknownError); + public customTaskExecutionComplete(task: Task, result?: number): Promise { + throw new TaskError(Severity.Error, 'Custom execution task completion is never expected in the process task system.', TaskErrors.UnknownError); } public hasErrors(value: boolean): void { diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 283a3bb27e7..6c41bca0732 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -380,7 +380,7 @@ export interface ITerminalInstance { * is the processes' exit code, an exit code of null means the process was killed as a result of * the ITerminalInstance being disposed. */ - onExit: Event; + onExit: Event; processReady: Promise; @@ -431,8 +431,10 @@ export interface ITerminalInstance { /** * Indicates that a consumer of a renderer only terminal is finished with it. + * + * @param result - Optional result code from a from a task process or custom execution. */ - finishedWithRenderer(result: number | undefined): void; + finishedWithRenderer(result?: number): void; /** * Forces the terminal to redraw its viewport. diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index e179f4204bd..779f58e5932 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -212,8 +212,8 @@ export class TerminalInstance implements ITerminalInstance { public get shellLaunchConfig(): IShellLaunchConfig { return this._shellLaunchConfig; } public get commandTracker(): TerminalCommandTracker { return this._commandTracker; } - private readonly _onExit = new Emitter(); - public get onExit(): Event { return this._onExit.event; } + private readonly _onExit = new Emitter(); + public get onExit(): Event { return this._onExit.event; } private readonly _onDisposed = new Emitter(); public get onDisposed(): Event { return this._onDisposed.event; } private readonly _onFocused = new Emitter(); @@ -753,9 +753,9 @@ export class TerminalInstance implements ITerminalInstance { this._disposables = lifecycle.dispose(this._disposables); } - public finishedWithRenderer(result: number | undefined): void { + public finishedWithRenderer(result?: number): void { // The use of this API is for cases where there is no backing process - // behind a terminal instance (such as when executing an extension callback task). + // behind a terminal instance (such as when executing an custom execution task). // There is no associated string, error text, etc, as the consumer of the renderer // can simply output the text to the renderer themselves. // All this code does is handle the "wait on exit" condition. @@ -990,7 +990,8 @@ export class TerminalInstance implements ITerminalInstance { private _onProcessOrExtensionCallbackExit(exitCode?: number): void { // Use 'typeof exitCode' because simply doing if (exitCode) would return false for both "not undefined" and a value of 0 // which is not the intention. - if (typeof exitCode === `number`) { + const isExitCodeSpecified: boolean = (typeof exitCode === `number`); + if (isExitCodeSpecified) { this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); } @@ -1002,7 +1003,7 @@ export class TerminalInstance implements ITerminalInstance { this._isExiting = true; let exitCodeMessage: string; - if (typeof exitCode === `number`) { + if (isExitCodeSpecified) { exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); } @@ -1013,7 +1014,7 @@ export class TerminalInstance implements ITerminalInstance { // Only trigger wait on exit when the exit was *not* triggered by the // user (via the `workbench.action.terminal.kill` command). if (this._shellLaunchConfig.waitOnExit && (!this._processManager || this._processManager.processState !== ProcessState.KILLED_BY_USER)) { - if (typeof exitCode === `number`) { + if (isExitCodeSpecified) { this._xterm.writeln(exitCodeMessage!); } if (typeof this._shellLaunchConfig.waitOnExit === 'string') { @@ -1029,8 +1030,8 @@ export class TerminalInstance implements ITerminalInstance { } } else { this.dispose(); - if (typeof exitCode === `number`) { - if (this._processManager && this._processManager!.processState === ProcessState.KILLED_DURING_LAUNCH) { + if (isExitCodeSpecified) { + if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { let args = ''; if (typeof this._shellLaunchConfig.args === 'string') { args = this._shellLaunchConfig.args; @@ -1057,7 +1058,7 @@ export class TerminalInstance implements ITerminalInstance { } } - this._onExit.fire((typeof exitCode === `number`) ? exitCode : 0); + this._onExit.fire(exitCode); } private _attachPressAnyKeyToCloseListener() { @@ -1104,10 +1105,10 @@ export class TerminalInstance implements ITerminalInstance { if (this._processManager) { // TODO: Why is this a string-null check failure without the "!"? - // The process manager can indeed be undefined when using an extension callback + // The process manager can indeed be undefined when using a custom execution // as a task, and the if check is correct. // The "force assume to be not-null !" operator was there before the addition - // of extension callback as task functionality. + // of custom execution as task functionality. this._processManager!.onProcessData(data => this._onProcessData(data)); } } From 5d1720321dc6f4ac6c2bb4117e563c021bd23a41 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Tue, 19 Feb 2019 14:33:25 -0800 Subject: [PATCH 21/36] Code review updates --- src/vs/vscode.proposed.d.ts | 8 +-- .../api/electron-browser/mainThreadTask.ts | 28 ++++---- src/vs/workbench/api/node/extHost.api.impl.ts | 4 +- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostTask.ts | 66 +++++++++---------- .../api/node/extHostTerminalService.ts | 37 ++++------- src/vs/workbench/api/node/extHostTypes.ts | 24 +++---- src/vs/workbench/api/shared/tasks.ts | 6 +- .../contrib/tasks/common/taskSystem.ts | 2 +- .../workbench/contrib/tasks/common/tasks.ts | 10 +-- .../electron-browser/task.contribution.ts | 2 +- .../electron-browser/terminalTaskSystem.ts | 12 ++-- .../contrib/tasks/node/processTaskSystem.ts | 2 +- 13 files changed, 94 insertions(+), 109 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index a7d71d01d99..c11f5987e2c 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1184,7 +1184,7 @@ declare module 'vscode' { /** * Class used to execute an extension callback as a task. */ - export class CustomTaskExecution { + export class CustomExecution { /** * @param callback The callback that will be called when the extension callback task is executed. */ @@ -1199,7 +1199,7 @@ declare module 'vscode' { /** * A task to execute */ - export class TaskWithCustomTaskExecution extends Task { + export class Task2 extends Task { /** * Creates a new task. * @@ -1212,12 +1212,12 @@ declare module 'vscode' { * or '$eslint'. Problem matchers can be contributed by an extension using * the `problemMatchers` extension point. */ - constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomTaskExecution, problemMatchers?: string | string[]); + constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomExecution, problemMatchers?: string | string[]); /** * The task's execution engine */ - executionWithExtensionCallback?: ProcessExecution | ShellExecution | CustomTaskExecution; + execution2?: ProcessExecution | ShellExecution | CustomExecution; } //#region Tasks diff --git a/src/vs/workbench/api/electron-browser/mainThreadTask.ts b/src/vs/workbench/api/electron-browser/mainThreadTask.ts index 344f03f96d6..893d864fe10 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTask.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTask.ts @@ -30,7 +30,7 @@ import { extHostNamedCustomer } from 'vs/workbench/api/electron-browser/extHostC import { ExtHostContext, MainThreadTaskShape, ExtHostTaskShape, MainContext, IExtHostContext } from 'vs/workbench/api/node/extHost.protocol'; import { TaskDefinitionDTO, TaskExecutionDTO, ProcessExecutionOptionsDTO, TaskPresentationOptionsDTO, - ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, CustomTaskExecutionDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, + ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, CustomExecutionDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, RunOptionsDTO } from 'vs/workbench/api/shared/tasks'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; @@ -138,7 +138,7 @@ namespace ProcessExecutionOptionsDTO { } namespace ProcessExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ProcessExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ProcessExecutionDTO { let candidate = value as ProcessExecutionDTO; return candidate && !!candidate.process; } @@ -206,7 +206,7 @@ namespace ShellExecutionOptionsDTO { } namespace ShellExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ShellExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ShellExecutionDTO { let candidate = value as ShellExecutionDTO; return candidate && (!!candidate.commandLine || !!candidate.command); } @@ -237,21 +237,21 @@ namespace ShellExecutionDTO { } } -namespace CustomTaskExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is CustomTaskExecutionDTO { - let candidate = value as CustomTaskExecutionDTO; - return candidate && candidate.customTaskExecution === 'customTaskExecution'; +namespace CustomExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is CustomExecutionDTO { + let candidate = value as CustomExecutionDTO; + return candidate && candidate.customExecution === 'customExecution'; } - export function from(value: CommandConfiguration): CustomTaskExecutionDTO { + export function from(value: CommandConfiguration): CustomExecutionDTO { return { - customTaskExecution: 'customTaskExecution' + customExecution: 'customExecution' }; } - export function to(value: CustomTaskExecutionDTO): CommandConfiguration { + export function to(value: CustomExecutionDTO): CommandConfiguration { return { - runtime: RuntimeType.CustomTaskExecution, + runtime: RuntimeType.CustomExecution, presentation: undefined }; } @@ -358,8 +358,8 @@ namespace TaskDTO { command = ShellExecutionDTO.to(task.execution); } else if (ProcessExecutionDTO.is(task.execution)) { command = ProcessExecutionDTO.to(task.execution); - } else if (CustomTaskExecutionDTO.is(task.execution)) { - command = CustomTaskExecutionDTO.to(task.execution); + } else if (CustomExecutionDTO.is(task.execution)) { + command = CustomExecutionDTO.to(task.execution); } } @@ -518,7 +518,7 @@ export class MainThreadTask implements MainThreadTaskShape { }); } - public $customTaskExecutionComplete(id: string, result?: number): Promise { + public $customExecutionComplete(id: string, result?: number): Promise { return new Promise((resolve, reject) => { this._taskService.getActiveTasks().then((tasks) => { for (let task of tasks) { diff --git a/src/vs/workbench/api/node/extHost.api.impl.ts b/src/vs/workbench/api/node/extHost.api.impl.ts index ae5711c453c..eec38cbcfa9 100644 --- a/src/vs/workbench/api/node/extHost.api.impl.ts +++ b/src/vs/workbench/api/node/extHost.api.impl.ts @@ -783,7 +783,7 @@ export function createApiFactory( DocumentSymbol: extHostTypes.DocumentSymbol, EndOfLine: extHostTypes.EndOfLine, EventEmitter: Emitter, - CustomTaskExecution: extHostTypes.CustomTaskExecution, + CustomExecution: extHostTypes.CustomExecution, FileChangeType: extHostTypes.FileChangeType, FileSystemError: extHostTypes.FileSystemError, FileType: files.FileType, @@ -819,7 +819,7 @@ export function createApiFactory( SymbolInformation: extHostTypes.SymbolInformation, SymbolKind: extHostTypes.SymbolKind, Task: extHostTypes.Task, - TaskWithCustomTaskExecution: extHostTypes.Task, + Task2: extHostTypes.Task, TaskGroup: extHostTypes.TaskGroup, TaskPanelKind: extHostTypes.TaskPanelKind, TaskRevealKind: extHostTypes.TaskRevealKind, diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 28e71142d2e..6b6d00970a0 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -556,7 +556,7 @@ export interface MainThreadTaskShape extends IDisposable { $executeTask(task: TaskHandleDTO | TaskDTO): Promise; $terminateTask(id: string): Promise; $registerTaskSystem(scheme: string, info: TaskSystemInfoDTO): void; - $customTaskExecutionComplete(id: string, result?: number): Promise; + $customExecutionComplete(id: string, result?: number): Promise; } export interface MainThreadExtensionServiceShape extends IDisposable { diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 6713812a53a..1e7656efab5 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -22,7 +22,7 @@ import { TaskDefinitionDTO, TaskExecutionDTO, TaskPresentationOptionsDTO, ProcessExecutionOptionsDTO, ProcessExecutionDTO, ShellExecutionOptionsDTO, ShellExecutionDTO, - CustomTaskExecutionDTO, + CustomExecutionDTO, TaskDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, TaskSetDTO } from '../shared/tasks'; import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService'; @@ -79,7 +79,7 @@ namespace ProcessExecutionOptionsDTO { } namespace ProcessExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ProcessExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ProcessExecutionDTO { let candidate = value as ProcessExecutionDTO; return candidate && !!candidate.process; } @@ -120,7 +120,7 @@ namespace ShellExecutionOptionsDTO { } namespace ShellExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ShellExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ShellExecutionDTO { let candidate = value as ShellExecutionDTO; return candidate && (!!candidate.commandLine || !!candidate.command); } @@ -153,15 +153,15 @@ namespace ShellExecutionDTO { } } -namespace CustomTaskExecutionDTO { - export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is CustomTaskExecutionDTO { - let candidate = value as CustomTaskExecutionDTO; - return candidate && candidate.customTaskExecution === 'customTaskExecution'; +namespace CustomExecutionDTO { + export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is CustomExecutionDTO { + let candidate = value as CustomExecutionDTO; + return candidate && candidate.customExecution === 'customExecution'; } - export function from(value: vscode.CustomTaskExecution): CustomTaskExecutionDTO { + export function from(value: vscode.CustomExecution): CustomExecutionDTO { return { - customTaskExecution: 'customTaskExecution' + customExecution: 'customExecution' }; } } @@ -199,13 +199,13 @@ namespace TaskDTO { if (value === undefined || value === null) { return undefined; } - let execution: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO; + let execution: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO; if (value.execution instanceof types.ProcessExecution) { execution = ProcessExecutionDTO.from(value.execution); } else if (value.execution instanceof types.ShellExecution) { execution = ShellExecutionDTO.from(value.execution); - } else if ((value).executionWithExtensionCallback && (value).executionWithExtensionCallback instanceof types.CustomTaskExecution) { - execution = CustomTaskExecutionDTO.from((value).executionWithExtensionCallback); + } else if ((value).execution2 && (value).execution2 instanceof types.CustomExecution) { + execution = CustomExecutionDTO.from((value).execution2); } let definition: TaskDefinitionDTO = TaskDefinitionDTO.from(value.definition); @@ -336,16 +336,16 @@ interface HandlerData { extension: IExtensionDescription; } -class CustomTaskExecutionData implements IDisposable { +class CustomExecutionData implements IDisposable { private _cancellationSource?: CancellationTokenSource; - private readonly _onTaskExecutionComplete: Emitter = new Emitter(); + private readonly _onTaskExecutionComplete: Emitter = new Emitter(); private readonly _disposables: IDisposable[] = []; private terminal?: vscode.Terminal; private terminalId?: number; public result: number | undefined; constructor( - private readonly callbackData: vscode.CustomTaskExecution, + private readonly callbackData: vscode.CustomExecution, private readonly terminalService: ExtHostTerminalService) { } @@ -353,7 +353,7 @@ class CustomTaskExecutionData implements IDisposable { dispose(this._disposables); } - public get onTaskExecutionComplete(): Event { + public get onTaskExecutionComplete(): Event { return this._onTaskExecutionComplete.event; } @@ -396,7 +396,7 @@ class CustomTaskExecutionData implements IDisposable { } this.terminal = callbackTerminals[0]; - const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.createTerminalRendererForTerminal(this.terminal); + const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.resolveTerminalRenderer(terminalId); this._cancellationSource = new CancellationTokenSource(); this._disposables.push(this._cancellationSource); @@ -424,8 +424,8 @@ export class ExtHostTask implements ExtHostTaskShape { private _handleCounter: number; private _handlers: Map; private _taskExecutions: Map; - private _providedCustomTaskExecutions: Map; - private _activeCustomTaskExecutions: Map; + private _providedCustomExecutions: Map; + private _activeCustomExecutions: Map; private readonly _onDidExecuteTask: Emitter = new Emitter(); private readonly _onDidTerminateTask: Emitter = new Emitter(); @@ -447,8 +447,8 @@ export class ExtHostTask implements ExtHostTaskShape { this._handleCounter = 0; this._handlers = new Map(); this._taskExecutions = new Map(); - this._providedCustomTaskExecutions = new Map(); - this._activeCustomTaskExecutions = new Map(); + this._providedCustomExecutions = new Map(); + this._activeCustomExecutions = new Map(); } public registerTaskProvider(extension: IExtensionDescription, provider: vscode.TaskProvider): vscode.Disposable { @@ -518,16 +518,16 @@ export class ExtHostTask implements ExtHostTaskShape { // Once a terminal is spun up for the custom execution task this event will be fired. // At that point, we need to actually start the callback, but // only if it hasn't already begun. - const extensionCallback: CustomTaskExecutionData | undefined = this._providedCustomTaskExecutions.get(execution.id); + const extensionCallback: CustomExecutionData | undefined = this._providedCustomExecutions.get(execution.id); if (extensionCallback) { - if (this._activeCustomTaskExecutions.get(execution.id) !== undefined) { + if (this._activeCustomExecutions.get(execution.id) !== undefined) { throw new Error('We should not be trying to start the same custom task executions twice.'); } - this._activeCustomTaskExecutions.set(execution.id, extensionCallback); + this._activeCustomExecutions.set(execution.id, extensionCallback); const taskExecutionComplete: IDisposable = extensionCallback.onTaskExecutionComplete(() => { - this.customTaskExecutionComplete(execution); + this.customExecutionComplete(execution); taskExecutionComplete.dispose(); }); @@ -548,7 +548,7 @@ export class ExtHostTask implements ExtHostTaskShape { const workspaceProvider = await this._workspaceService.getWorkspaceProvider(); const _execution = this.getTaskExecution(execution, workspaceProvider); this._taskExecutions.delete(execution.id); - this.customTaskExecutionComplete(execution); + this.customExecutionComplete(execution); this._onDidTerminateTask.fire({ execution: _execution }); @@ -593,7 +593,7 @@ export class ExtHostTask implements ExtHostTaskShape { // For custom execution tasks, we need to store the execution objects locally // since we obviously cannot send callback functions through the proxy. // So, clear out any existing ones. - this._providedCustomTaskExecutions.clear(); + this._providedCustomExecutions.clear(); // Set up a list of task ID promises that we can wait on // before returning the provided tasks. The ensures that @@ -614,13 +614,13 @@ export class ExtHostTask implements ExtHostTaskShape { const taskDTO: TaskDTO = TaskDTO.from(task, handler.extension); taskDTOs.push(taskDTO); - if (CustomTaskExecutionDTO.is(taskDTO.execution)) { + if (CustomExecutionDTO.is(taskDTO.execution)) { taskIdPromises.push(new Promise((resolve) => { // The ID is calculated on the main thread task side, so, let's call into it here. // We need the task id's pre-computed for custom task executions because when OnDidStartTask // is invoked, we have to be able to map it back to our data. this._proxy.$createTaskId(taskDTO).then((taskId) => { - this._providedCustomTaskExecutions.set(taskId, new CustomTaskExecutionData((task).executionWithExtensionCallback, this._terminalService)); + this._providedCustomExecutions.set(taskId, new CustomExecutionData((task).execution2, this._terminalService)); resolve(); }); })); @@ -698,11 +698,11 @@ export class ExtHostTask implements ExtHostTaskShape { return result; } - private customTaskExecutionComplete(execution: TaskExecutionDTO): void { - const extensionCallback: CustomTaskExecutionData | undefined = this._activeCustomTaskExecutions.get(execution.id); + private customExecutionComplete(execution: TaskExecutionDTO): void { + const extensionCallback: CustomExecutionData | undefined = this._activeCustomExecutions.get(execution.id); if (extensionCallback) { - this._activeCustomTaskExecutions.delete(execution.id); - this._proxy.$customTaskExecutionComplete(execution.id, extensionCallback.result); + this._activeCustomExecutions.delete(execution.id); + this._proxy.$customExecutionComplete(execution.id, extensionCallback.result); extensionCallback.dispose(); } } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index fe3457e5f8b..18783abf84b 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -332,18 +332,16 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return renderer; } - public async createTerminalRendererForTerminal(terminal: vscode.Terminal): Promise { - if (!(terminal instanceof ExtHostTerminal)) { - throw new Error('Only expected instance extension host terminal type'); - } + public async resolveTerminalRenderer(id: number): Promise { // Check to see if the extension host already knows about this terminal. for (const terminalRenderer of this._terminalRenderers) { - if (terminalRenderer._id === terminal._id) { + if (terminalRenderer._id === id) { return terminalRenderer; } } - const dimensions = await this._proxy.$terminalGetDimensions(terminal._id); + const dimensions = await this._proxy.$terminalGetDimensions(id); + const terminal = this._getTerminalById(id); const renderer = new ExtHostTerminalRenderer(this._proxy, terminal.name, terminal, terminal._id, dimensions.cols, dimensions.rows); this._terminalRenderers.push(renderer); @@ -415,31 +413,18 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } } - public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void { - let index = this._getTerminalObjectIndexById(this._terminals, id); + public $acceptTerminalOpened(id: number, name: string): void { + const index = this._getTerminalObjectIndexById(this._terminals, id); if (index !== null) { + // The terminal has already been created (via createTerminal*), only fire the event this._onDidOpenTerminal.fire(this.terminals[index]); - } - - let renderer = this._getTerminalRendererById(id); - - // If this is a terminal created by one of the public createTerminal* APIs - // then @acceptTerminalOpened was called from the main thread task - // to indicate that the terminal is ready for use. - // In those cases, we don't need to create the extension host objects - // as they already exist, but, we do still need to fire the events - // to our consumers. - if ((renderer !== null) && (index !== null)) { return; } - // The extension host did not know about this terminal, so create extension host - // objects to represent them. - if (!index) { - const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined); - index = this._terminals.push(terminal); - this._onDidOpenTerminal.fire(terminal); - } + const renderer = this._getTerminalRendererById(id); + const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined); + this._terminals.push(terminal); + this._onDidOpenTerminal.fire(terminal); } public $acceptTerminalProcessId(id: number, processId: number): void { diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 6645557f4d0..50b5cd8b7cf 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1708,7 +1708,7 @@ export enum TaskScope { Workspace = 2 } -export class CustomTaskExecution implements vscode.CustomTaskExecution { +export class CustomExecution implements vscode.CustomExecution { private _callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable; constructor(callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { @@ -1717,7 +1717,7 @@ export class CustomTaskExecution implements vscode.CustomTaskExecution { public computeId(): string { const hash = crypto.createHash('md5'); - hash.update('customTaskExecution'); + hash.update('customExecution'); hash.update(generateUuid()); return hash.digest('hex'); } @@ -1732,9 +1732,9 @@ export class CustomTaskExecution implements vscode.CustomTaskExecution { } @es5ClassCompat -export class Task implements vscode.TaskWithCustomTaskExecution { +export class Task implements vscode.Task2 { - private static ExtensionCallbackType: string = 'customTaskExecution'; + private static ExtensionCallbackType: string = 'customExecution'; private static ProcessType: string = 'process'; private static ShellType: string = 'shell'; private static EmptyType: string = '$empty'; @@ -1744,7 +1744,7 @@ export class Task implements vscode.TaskWithCustomTaskExecution { private _definition: vscode.TaskDefinition; private _scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder | undefined; private _name: string; - private _execution: ProcessExecution | ShellExecution | CustomTaskExecution | undefined; + private _execution: ProcessExecution | ShellExecution | CustomExecution | undefined; private _problemMatchers: string[]; private _hasDefinedMatchers: boolean; private _isBackground: boolean; @@ -1753,8 +1753,8 @@ export class Task implements vscode.TaskWithCustomTaskExecution { private _presentationOptions: vscode.TaskPresentationOptions; private _runOptions: vscode.RunOptions; - constructor(definition: vscode.TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomTaskExecution, problemMatchers?: string | string[]); - constructor(definition: vscode.TaskDefinition, scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomTaskExecution, problemMatchers?: string | string[]); + constructor(definition: vscode.TaskDefinition, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomExecution, problemMatchers?: string | string[]); + constructor(definition: vscode.TaskDefinition, scope: vscode.TaskScope.Global | vscode.TaskScope.Workspace | vscode.WorkspaceFolder, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomExecution, problemMatchers?: string | string[]); constructor(definition: vscode.TaskDefinition, arg2: string | (vscode.TaskScope.Global | vscode.TaskScope.Workspace) | vscode.WorkspaceFolder, arg3: any, arg4?: any, arg5?: any, arg6?: any) { this.definition = definition; let problemMatchers: string | string[]; @@ -1819,7 +1819,7 @@ export class Task implements vscode.TaskWithCustomTaskExecution { type: Task.ShellType, id: this._execution.computeId() }; - } else if (this._execution instanceof CustomTaskExecution) { + } else if (this._execution instanceof CustomExecution) { this._definition = { type: Task.ExtensionCallbackType, id: this._execution.computeId() @@ -1866,18 +1866,18 @@ export class Task implements vscode.TaskWithCustomTaskExecution { } get execution(): ProcessExecution | ShellExecution | undefined { - return (this._execution instanceof CustomTaskExecution) ? undefined : this._execution; + return (this._execution instanceof CustomExecution) ? undefined : this._execution; } set execution(value: ProcessExecution | ShellExecution | undefined) { - this.executionWithExtensionCallback = value; + this.execution2 = value; } - get executionWithExtensionCallback(): ProcessExecution | ShellExecution | CustomTaskExecution | undefined { + get execution2(): ProcessExecution | ShellExecution | CustomExecution | undefined { return this._execution; } - set executionWithExtensionCallback(value: ProcessExecution | ShellExecution | CustomTaskExecution | undefined) { + set execution2(value: ProcessExecution | ShellExecution | CustomExecution | undefined) { if (value === null) { value = undefined; } diff --git a/src/vs/workbench/api/shared/tasks.ts b/src/vs/workbench/api/shared/tasks.ts index 2ee360a1cdd..0abf582d133 100644 --- a/src/vs/workbench/api/shared/tasks.ts +++ b/src/vs/workbench/api/shared/tasks.ts @@ -66,8 +66,8 @@ export interface ShellExecutionDTO { options?: ShellExecutionOptionsDTO; } -export interface CustomTaskExecutionDTO { - customTaskExecution: 'customTaskExecution'; +export interface CustomExecutionDTO { + customExecution: 'customExecution'; } export interface TaskSourceDTO { @@ -84,7 +84,7 @@ export interface TaskHandleDTO { export interface TaskDTO { _id: string; name?: string; - execution: ProcessExecutionDTO | ShellExecutionDTO | CustomTaskExecutionDTO; + execution: ProcessExecutionDTO | ShellExecutionDTO | CustomExecutionDTO; definition: TaskDefinitionDTO; isBackground?: boolean; source: TaskSourceDTO; diff --git a/src/vs/workbench/contrib/tasks/common/taskSystem.ts b/src/vs/workbench/contrib/tasks/common/taskSystem.ts index d2f6f1de695..56e9f05b226 100644 --- a/src/vs/workbench/contrib/tasks/common/taskSystem.ts +++ b/src/vs/workbench/contrib/tasks/common/taskSystem.ts @@ -136,5 +136,5 @@ export interface ITaskSystem { terminate(task: Task): Promise; terminateAll(): Promise; revealTask(task: Task): boolean; - customTaskExecutionComplete(task: Task, result: number | undefined): Promise; + customExecutionComplete(task: Task, result: number | undefined): Promise; } \ No newline at end of file diff --git a/src/vs/workbench/contrib/tasks/common/tasks.ts b/src/vs/workbench/contrib/tasks/common/tasks.ts index d5937a9f1f9..e1391a4a6b4 100644 --- a/src/vs/workbench/contrib/tasks/common/tasks.ts +++ b/src/vs/workbench/contrib/tasks/common/tasks.ts @@ -230,7 +230,7 @@ export namespace PresentationOptions { export enum RuntimeType { Shell = 1, Process = 2, - CustomTaskExecution = 3 + CustomExecution = 3 } export namespace RuntimeType { @@ -240,8 +240,8 @@ export namespace RuntimeType { return RuntimeType.Shell; case 'process': return RuntimeType.Process; - case 'customTaskExecution': - return RuntimeType.CustomTaskExecution; + case 'customExecution': + return RuntimeType.CustomExecution; default: return RuntimeType.Process; } @@ -607,8 +607,8 @@ export class CustomTask extends CommonTask { type = 'process'; break; - case RuntimeType.CustomTaskExecution: - type = 'customTaskExecution'; + case RuntimeType.CustomExecution: + type = 'customExecution'; break; default: diff --git a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts index 6f53b37fa6b..c9ecfd45b90 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts @@ -722,7 +722,7 @@ class TaskService extends Disposable implements ITaskService { if (!this._taskSystem) { return Promise.resolve(); } - return this._taskSystem.customTaskExecutionComplete(task, result); + return this._taskSystem.customExecutionComplete(task, result); } public getTask(folder: IWorkspaceFolder | string, identifier: string | TaskIdentifier, compareId: boolean = false): Promise { diff --git a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts index 531d01d5fd0..e67d7e57eb8 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts @@ -265,7 +265,7 @@ export class TerminalTaskSystem implements ITaskSystem { return Object.keys(this.activeTasks).map(key => this.activeTasks[key].task); } - public customTaskExecutionComplete(task: Task, result?: number): Promise { + public customExecutionComplete(task: Task, result?: number): Promise { let activeTerminal = this.activeTasks[task.getMapKey()]; if (!activeTerminal) { return Promise.reject(new Error('Expected to have a terminal for an custom execution task')); @@ -549,7 +549,7 @@ export class TerminalTaskSystem implements ITaskSystem { let processStartedSignaled = false; terminal.processReady.then(() => { if (!processStartedSignaled) { - if (task.command.runtime !== RuntimeType.CustomTaskExecution) { + if (task.command.runtime !== RuntimeType.CustomExecution) { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); } processStartedSignaled = true; @@ -621,7 +621,7 @@ export class TerminalTaskSystem implements ITaskSystem { let processStartedSignaled = false; terminal.processReady.then(() => { if (!processStartedSignaled) { - if (task.command.runtime !== RuntimeType.CustomTaskExecution) { + if (task.command.runtime !== RuntimeType.CustomExecution) { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); } processStartedSignaled = true; @@ -825,7 +825,7 @@ export class TerminalTaskSystem implements ITaskSystem { } } } else { - let commandExecutable = task.command.runtime !== RuntimeType.CustomTaskExecution ? CommandString.value(command) : undefined; + let commandExecutable = task.command.runtime !== RuntimeType.CustomExecution ? CommandString.value(command) : undefined; let executable = !isShellCommand ? this.resolveVariable(variableResolver, '${' + TerminalTaskSystem.ProcessVarName + '}') : commandExecutable; @@ -896,7 +896,7 @@ export class TerminalTaskSystem implements ITaskSystem { let command: CommandString | undefined; let args: CommandString[] | undefined; - if (task.command.runtime === RuntimeType.CustomTaskExecution) { + if (task.command.runtime === RuntimeType.CustomExecution) { this.currentTask.shellLaunchConfig = { isRendererOnly: true, waitOnExit, @@ -1116,7 +1116,7 @@ export class TerminalTaskSystem implements ITaskSystem { private collectCommandVariables(variables: Set, command: CommandConfiguration, task: CustomTask | ContributedTask): void { // The custom execution should have everything it needs already as it provided // the callback. - if (command.runtime === RuntimeType.CustomTaskExecution) { + if (command.runtime === RuntimeType.CustomExecution) { return; } diff --git a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts index e12684b46fb..6117f7ff292 100644 --- a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts @@ -105,7 +105,7 @@ export class ProcessTaskSystem implements ITaskSystem { return true; } - public customTaskExecutionComplete(task: Task, result?: number): Promise { + public customExecutionComplete(task: Task, result?: number): Promise { throw new TaskError(Severity.Error, 'Custom execution task completion is never expected in the process task system.', TaskErrors.UnknownError); } From 5a79a1eddc681d2836be0a4602a8e6afc2636797 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 20 Feb 2019 13:27:00 -0800 Subject: [PATCH 22/36] Updates based on feedback --- .../electron-browser/terminalTaskSystem.ts | 31 +++++-------------- .../contrib/tasks/node/processTaskSystem.ts | 4 +-- .../electron-browser/terminalInstance.ts | 12 +++---- 3 files changed, 16 insertions(+), 31 deletions(-) diff --git a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts index e67d7e57eb8..4cf7b777fbd 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts @@ -299,27 +299,6 @@ export class TerminalTaskSystem implements ITaskSystem { }); } - public extensionCallbackTaskEnded(task: Task): Promise { - let activeTerminal = this.activeTasks[task.getMapKey()]; - if (!activeTerminal) { - return Promise.resolve({ success: false, task: undefined }); - } - return new Promise((resolve, reject) => { - let terminal = activeTerminal.terminal; - const onDisposed = terminal.onDisposed(() => { - let task = activeTerminal.task; - try { - onDisposed.dispose(); - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.End, task)); - } catch (error) { - // Do nothing. - } - resolve({ success: true, task: task }); - }); - terminal.dispose(); - }); - } - public terminateAll(): Promise { let promises: Promise[] = []; Object.keys(this.activeTasks).forEach((key) => { @@ -599,7 +578,11 @@ export class TerminalTaskSystem implements ITaskSystem { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!)); processStartedSignaled = true; } - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessEnded, task, exitCode)); + + if (task.command.runtime !== RuntimeType.CustomExecution) { + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessEnded, task, exitCode)); + } + for (let i = 0; i < eventCounter; i++) { let event = TaskEvent.create(TaskEventKind.Inactive, task); this._onDidStateChange.fire(event); @@ -671,7 +654,9 @@ export class TerminalTaskSystem implements ITaskSystem { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal.processId!)); processStartedSignaled = true; } - this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessEnded, task, exitCode)); + if (task.command.runtime !== RuntimeType.CustomExecution) { + this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessEnded, task, exitCode)); + } this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.Inactive, task)); this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.End, task)); resolve({ exitCode }); diff --git a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts index 6117f7ff292..11d8e8c3b9f 100644 --- a/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/node/processTaskSystem.ts @@ -283,7 +283,7 @@ export class ProcessTaskSystem implements ITaskSystem { this.childProcessEnded(); watchingProblemMatcher.done(); watchingProblemMatcher.dispose(); - if (processStartedSignaled) { + if (processStartedSignaled && task.command.runtime !== RuntimeType.CustomExecution) { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessEnded, task, success.cmdCode!)); } toDispose = dispose(toDispose!); @@ -339,7 +339,7 @@ export class ProcessTaskSystem implements ITaskSystem { startStopProblemMatcher.done(); startStopProblemMatcher.dispose(); this.checkTerminated(task, success); - if (processStartedSignaled) { + if (processStartedSignaled && task.command.runtime !== RuntimeType.CustomExecution) { this._onDidStateChange.fire(TaskEvent.create(TaskEventKind.ProcessEnded, task, success.cmdCode!)); } this._onDidStateChange.fire(inactiveEvent); diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index 779f58e5932..c7ded8b9b93 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -740,10 +740,10 @@ export class TerminalInstance implements ITerminalInstance { if (this._processManager) { this._processManager.dispose(immediate); } else { - // In cases where there is no associated process (for example executing an exetnsion callback task) - // consumers still expect on onExit event to be fired. An example of this is terminating the extnesion callback - // task. - this._onExit.fire(this._id); + // In cases where there is no associated process (for example executing an extension callback task) + // consumers still expect on onExit event to be fired. An example of this is terminating the extension callback + // task. There is no exit code at this point, so firing undefined is appropriate. + this._onExit.fire(undefined); } if (!this._isDisposed) { @@ -1094,8 +1094,8 @@ export class TerminalInstance implements ITerminalInstance { // Set the new shell launch config this._shellLaunchConfig = shell; // Must be done before calling _createProcess() - // Initialize new process if we have one. - if (this._shellLaunchConfig.executable) { + // Launch the process unless this is only a renderer. + if (!this._shellLaunchConfig.isRendererOnly) { this._createProcess(); } From 55fb00e167f9692bc0df93d6f7e5189824243a92 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Thu, 21 Feb 2019 07:50:00 -0800 Subject: [PATCH 23/36] Remove comment and '!' about strict null check --- .../contrib/terminal/electron-browser/terminalInstance.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index c7ded8b9b93..e8f3c6a2c9c 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -1104,12 +1104,7 @@ export class TerminalInstance implements ITerminalInstance { } if (this._processManager) { - // TODO: Why is this a string-null check failure without the "!"? - // The process manager can indeed be undefined when using a custom execution - // as a task, and the if check is correct. - // The "force assume to be not-null !" operator was there before the addition - // of custom execution as task functionality. - this._processManager!.onProcessData(data => this._onProcessData(data)); + this._processManager.onProcessData(data => this._onProcessData(data)); } } From 19dc5cdf5add811f80d1038651ebf8b61b46a94e Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Mon, 4 Mar 2019 05:22:29 -0800 Subject: [PATCH 24/36] rename finishedWithRenderer to rendererExit --- .../contrib/tasks/electron-browser/terminalTaskSystem.ts | 2 +- src/vs/workbench/contrib/terminal/common/terminal.ts | 2 +- .../contrib/terminal/electron-browser/terminalInstance.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts index 5f6a909b63f..ca84f2a7dde 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts @@ -278,7 +278,7 @@ export class TerminalTaskSystem implements ITaskSystem { } return new Promise((resolve) => { - activeTerminal.terminal.finishedWithRenderer(result); + activeTerminal.terminal.rendererExit(result); resolve(); }); } diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 6c41bca0732..1c10767a87e 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -434,7 +434,7 @@ export interface ITerminalInstance { * * @param result - Optional result code from a from a task process or custom execution. */ - finishedWithRenderer(result?: number): void; + rendererExit(result?: number): void; /** * Forces the terminal to redraw its viewport. diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index a143478e1be..5b89d409beb 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -754,7 +754,7 @@ export class TerminalInstance implements ITerminalInstance { this._disposables = lifecycle.dispose(this._disposables); } - public finishedWithRenderer(result?: number): void { + public rendererExit(result?: number): void { // The use of this API is for cases where there is no backing process // behind a terminal instance (such as when executing an custom execution task). // There is no associated string, error text, etc, as the consumer of the renderer From f2a58a6b850d9bb79efea93fab6f8649f784aa82 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Mon, 4 Mar 2019 05:32:28 -0800 Subject: [PATCH 25/36] Fix press any key event handler leak --- .../electron-browser/terminalInstance.ts | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index 5b89d409beb..fb957ff91eb 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -36,7 +36,7 @@ import { TerminalProcessManager } from 'vs/workbench/contrib/terminal/electron-b import { TerminalCommandTracker } from 'vs/workbench/contrib/terminal/node/terminalCommandTracker'; import { WindowsShellHelper } from 'vs/workbench/contrib/terminal/node/windowsShellHelper'; import { IPanelService } from 'vs/workbench/services/panel/common/panelService'; -import { ISearchOptions, Terminal as XTermTerminal } from 'vscode-xterm'; +import { ISearchOptions, Terminal as XTermTerminal, IDisposable } from 'vscode-xterm'; import { IAccessibilityService, AccessibilitySupport } from 'vs/platform/accessibility/common/accessibility'; // How long in milliseconds should an average frame take to render for a notification to appear @@ -160,6 +160,7 @@ export class TerminalInstance implements ITerminalInstance { private static _idCounter = 1; private _processManager: ITerminalProcessManager | undefined; + private _keyPressListener: IDisposable | undefined; private _id: number; private _isExiting: boolean; @@ -1063,14 +1064,25 @@ export class TerminalInstance implements ITerminalInstance { } private _attachPressAnyKeyToCloseListener() { - const keyPressListener = dom.addDisposableListener(this._xterm.textarea, 'keypress', (event: KeyboardEvent) => { - keyPressListener.dispose(); - this.dispose(); - event.preventDefault(); - }); + if (!this._keyPressListener) { + this._keyPressListener = dom.addDisposableListener(this._xterm.textarea, 'keypress', (event: KeyboardEvent) => { + if (this._keyPressListener) { + this._keyPressListener.dispose(); + this._keyPressListener = undefined; + this.dispose(); + event.preventDefault(); + } + }); + } } public reuseTerminal(shell: IShellLaunchConfig): void { + // Unsubscribe any key listener we may have. + if (!this._keyPressListener) { + this._keyPressListener.dispose(); + this._keyPressListener = undefined; + } + // Kill and clear up the process, making the process manager ready for a new process if (this._processManager) { this._processManager.dispose(); From 5da21fb665ad970aee41e48db632ad3af5e44dbf Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Mon, 4 Mar 2019 05:40:02 -0800 Subject: [PATCH 26/36] Allow undefined exit codes to be logged --- .../contrib/terminal/electron-browser/terminalInstance.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index fb957ff91eb..ee285366f92 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -993,7 +993,7 @@ export class TerminalInstance implements ITerminalInstance { // Use 'typeof exitCode' because simply doing if (exitCode) would return false for both "not undefined" and a value of 0 // which is not the intention. const isExitCodeSpecified: boolean = (typeof exitCode === `number`); - if (isExitCodeSpecified) { + if (exitCode) { this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); } From 97b4dc8b00981b8c56352c1956c709a934b37c36 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Mon, 4 Mar 2019 05:42:31 -0800 Subject: [PATCH 27/36] Throw an error when renderer exit is called on a non-renderer only terminal --- .../contrib/terminal/electron-browser/terminalInstance.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index ee285366f92..bfbaf2afa1f 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -762,7 +762,7 @@ export class TerminalInstance implements ITerminalInstance { // can simply output the text to the renderer themselves. // All this code does is handle the "wait on exit" condition. if (!this.shellLaunchConfig.isRendererOnly) { - return; + throw new Error('renderExit is only expected to be called on a renderer only terminal'); } return this._onProcessOrExtensionCallbackExit(result); From 0ba7481840281aea9d387cdee0467f0a2abefba7 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Mon, 4 Mar 2019 07:37:39 -0800 Subject: [PATCH 28/36] Add JSDoc for process or custom execution completion --- .../contrib/terminal/electron-browser/terminalInstance.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts index bfbaf2afa1f..3cc3a104545 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstance.ts @@ -989,6 +989,10 @@ export class TerminalInstance implements ITerminalInstance { } } + /** + * Called when either a process tied to a terminal has exited or when a custom execution has completed. + * @param exitCode The exit code can be undefined if the terminal was exited through user action or if a custom execution callback did not provide a exit code when it finished. + */ private _onProcessOrExtensionCallbackExit(exitCode?: number): void { // Use 'typeof exitCode' because simply doing if (exitCode) would return false for both "not undefined" and a value of 0 // which is not the intention. From 41d06331968e6ba7099436b284e3359669dd6154 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Tue, 5 Mar 2019 08:12:29 -0800 Subject: [PATCH 29/36] Address code review feedback --- src/vs/vscode.proposed.d.ts | 7 +- src/vs/workbench/api/node/extHostTypes.ts | 8 +- .../contrib/tasks/common/taskSystem.ts | 2 +- .../electron-browser/task.contribution.ts | 2 +- .../electron-browser/terminalTaskSystem.ts | 2 +- .../terminal/browser/terminalInstance.ts | 77 ++++++++----------- .../contrib/terminal/common/terminal.ts | 6 +- 7 files changed, 49 insertions(+), 55 deletions(-) diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 99cc8aa3311..2d317183ab6 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1254,12 +1254,15 @@ declare module 'vscode' { /** * @param callback The callback that will be called when the extension callback task is executed. */ - constructor(callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); + constructor(callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable); /** * The callback used to execute the task. + * @param terminalRenderer Used by the task to render output and receive input. + * @param cancellationToken Cancellation used to signal a cancel request to the executing task. + * @returns The callback should return '0' for success and a non-zero value for failure. */ - callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; + callback: (terminalRenderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable; } /** diff --git a/src/vs/workbench/api/node/extHostTypes.ts b/src/vs/workbench/api/node/extHostTypes.ts index 50b5cd8b7cf..7f87ee4cf13 100644 --- a/src/vs/workbench/api/node/extHostTypes.ts +++ b/src/vs/workbench/api/node/extHostTypes.ts @@ -1709,9 +1709,9 @@ export enum TaskScope { } export class CustomExecution implements vscode.CustomExecution { - private _callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable; + private _callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable; - constructor(callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { + constructor(callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { this._callback = callback; } @@ -1722,11 +1722,11 @@ export class CustomExecution implements vscode.CustomExecution { return hash.digest('hex'); } - public set callback(value: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { + public set callback(value: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable) { this._callback = value; } - public get callback(): (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable { + public get callback(): (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable { return this._callback; } } diff --git a/src/vs/workbench/contrib/tasks/common/taskSystem.ts b/src/vs/workbench/contrib/tasks/common/taskSystem.ts index 3ea4e9feb3d..3c462491828 100644 --- a/src/vs/workbench/contrib/tasks/common/taskSystem.ts +++ b/src/vs/workbench/contrib/tasks/common/taskSystem.ts @@ -136,5 +136,5 @@ export interface ITaskSystem { terminate(task: Task): Promise; terminateAll(): Promise; revealTask(task: Task): boolean; - customExecutionComplete(task: Task, result: number | undefined): Promise; + customExecutionComplete(task: Task, result: number): Promise; } \ No newline at end of file diff --git a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts index 3e38b08ccb6..ed9b3395159 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/task.contribution.ts @@ -718,7 +718,7 @@ class TaskService extends Disposable implements ITaskService { this._taskSystemInfos.set(key, info); } - public extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise { + public extensionCallbackTaskComplete(task: Task, result: number): Promise { if (!this._taskSystem) { return Promise.resolve(); } diff --git a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts index 33740907b16..5ded7e4a924 100644 --- a/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/electron-browser/terminalTaskSystem.ts @@ -269,7 +269,7 @@ export class TerminalTaskSystem implements ITaskSystem { return Object.keys(this.activeTasks).map(key => this.activeTasks[key].task); } - public customExecutionComplete(task: Task, result?: number): Promise { + public customExecutionComplete(task: Task, result: number): Promise { let activeTerminal = this.activeTasks[task.getMapKey()]; if (!activeTerminal) { return Promise.reject(new Error('Expected to have a terminal for an custom execution task')); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 1d5afea55e3..a7e529b0b7b 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -208,8 +208,8 @@ export class TerminalInstance implements ITerminalInstance { public get shellLaunchConfig(): IShellLaunchConfig { return this._shellLaunchConfig; } public get commandTracker(): TerminalCommandTracker { return this._commandTracker; } - private readonly _onExit = new Emitter(); - public get onExit(): Event { return this._onExit.event; } + private readonly _onExit = new Emitter(); + public get onExit(): Event { return this._onExit.event; } private readonly _onDisposed = new Emitter(); public get onDisposed(): Event { return this._onDisposed.event; } private readonly _onFocused = new Emitter(); @@ -731,8 +731,8 @@ export class TerminalInstance implements ITerminalInstance { } else { // In cases where there is no associated process (for example executing an extension callback task) // consumers still expect on onExit event to be fired. An example of this is terminating the extension callback - // task. There is no exit code at this point, so firing undefined is appropriate. - this._onExit.fire(undefined); + // task. + this._onExit.fire(0); } if (!this._isDisposed) { @@ -742,7 +742,7 @@ export class TerminalInstance implements ITerminalInstance { this._disposables = lifecycle.dispose(this._disposables); } - public rendererExit(result?: number): void { + public rendererExit(result: number): void { // The use of this API is for cases where there is no backing process // behind a terminal instance (such as when executing an custom execution task). // There is no associated string, error text, etc, as the consumer of the renderer @@ -918,13 +918,8 @@ export class TerminalInstance implements ITerminalInstance { * Called when either a process tied to a terminal has exited or when a custom execution has completed. * @param exitCode The exit code can be undefined if the terminal was exited through user action or if a custom execution callback did not provide a exit code when it finished. */ - private _onProcessOrExtensionCallbackExit(exitCode?: number): void { - // Use 'typeof exitCode' because simply doing if (exitCode) would return false for both "not undefined" and a value of 0 - // which is not the intention. - const isExitCodeSpecified: boolean = (typeof exitCode === `number`); - if (exitCode) { - this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); - } + private _onProcessOrExtensionCallbackExit(exitCode: number): void { + this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); // Prevent dispose functions being triggered multiple times if (this._isExiting) { @@ -932,11 +927,8 @@ export class TerminalInstance implements ITerminalInstance { } this._isExiting = true; - let exitCodeMessage: string; - if (isExitCodeSpecified) { - exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); - } + const exitCodeMessage: string = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); if (this._processManager) { this._logService.debug(`Terminal process exit (id: ${this.id}) state ${this._processManager!.processState}`); @@ -945,9 +937,7 @@ export class TerminalInstance implements ITerminalInstance { // Only trigger wait on exit when the exit was *not* triggered by the // user (via the `workbench.action.terminal.kill` command). if (this._shellLaunchConfig.waitOnExit && (!this._processManager || this._processManager.processState !== ProcessState.KILLED_BY_USER)) { - if (isExitCodeSpecified) { - this._xterm.writeln(exitCodeMessage!); - } + this._xterm.writeln(exitCodeMessage!); if (typeof this._shellLaunchConfig.waitOnExit === 'string') { let message = this._shellLaunchConfig.waitOnExit; // Bold the message and add an extra new line to make it stand out from the rest of the output @@ -961,30 +951,28 @@ export class TerminalInstance implements ITerminalInstance { } } else { this.dispose(); - if (isExitCodeSpecified) { - if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { - let args = ''; - if (typeof this._shellLaunchConfig.args === 'string') { - args = this._shellLaunchConfig.args; - } else if (this._shellLaunchConfig.args && this._shellLaunchConfig.args.length) { - args = ' ' + this._shellLaunchConfig.args.map(a => { - if (typeof a === 'string' && a.indexOf(' ') !== -1) { - return `'${a}'`; - } - return a; - }).join(' '); - } - if (this._shellLaunchConfig.executable) { - this._notificationService.error(nls.localize('terminal.integrated.launchFailed', 'The terminal process command \'{0}{1}\' failed to launch (exit code: {2})', this._shellLaunchConfig.executable, args, exitCode)); - } else { - this._notificationService.error(nls.localize('terminal.integrated.launchFailedExtHost', 'The terminal process failed to launch (exit code: {0})', exitCode)); - } + if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { + let args = ''; + if (typeof this._shellLaunchConfig.args === 'string') { + args = this._shellLaunchConfig.args; + } else if (this._shellLaunchConfig.args && this._shellLaunchConfig.args.length) { + args = ' ' + this._shellLaunchConfig.args.map(a => { + if (typeof a === 'string' && a.indexOf(' ') !== -1) { + return `'${a}'`; + } + return a; + }).join(' '); + } + if (this._shellLaunchConfig.executable) { + this._notificationService.error(nls.localize('terminal.integrated.launchFailed', 'The terminal process command \'{0}{1}\' failed to launch (exit code: {2})', this._shellLaunchConfig.executable, args, exitCode)); } else { - if (this._configHelper.config.showExitAlert) { - this._notificationService.error(exitCodeMessage!); - } else { - console.warn(exitCodeMessage!); - } + this._notificationService.error(nls.localize('terminal.integrated.launchFailedExtHost', 'The terminal process failed to launch (exit code: {0})', exitCode)); + } + } else { + if (this._configHelper.config.showExitAlert) { + this._notificationService.error(exitCodeMessage!); + } else { + console.warn(exitCodeMessage!); } } } @@ -1007,7 +995,7 @@ export class TerminalInstance implements ITerminalInstance { public reuseTerminal(shell: IShellLaunchConfig): void { // Unsubscribe any key listener we may have. - if (!this._keyPressListener) { + if (this._keyPressListener) { this._keyPressListener.dispose(); this._keyPressListener = undefined; } @@ -1037,8 +1025,11 @@ export class TerminalInstance implements ITerminalInstance { this._shellLaunchConfig = shell; // Must be done before calling _createProcess() // Launch the process unless this is only a renderer. + // In the renderer only cases, we still need to set the title correctly. if (!this._shellLaunchConfig.isRendererOnly) { this._createProcess(); + } else if (this._shellLaunchConfig.name) { + this.setTitle(this._shellLaunchConfig.name, false); } if (oldTitle !== this._title) { diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 44d5f37e52a..d7cb9d9ee5a 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -390,7 +390,7 @@ export interface ITerminalInstance { * is the processes' exit code, an exit code of null means the process was killed as a result of * the ITerminalInstance being disposed. */ - onExit: Event; + onExit: Event; processReady: Promise; @@ -442,9 +442,9 @@ export interface ITerminalInstance { /** * Indicates that a consumer of a renderer only terminal is finished with it. * - * @param result - Optional result code from a from a task process or custom execution. + * @param result - Result code from a from a task process or custom execution. Zero indicates success, non-zero indicates failure. */ - rendererExit(result?: number): void; + rendererExit(result: number): void; /** * Forces the terminal to redraw its viewport. From 991d836c11d3b1709453af0aff31b67c96c5b676 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 6 Mar 2019 08:55:30 -0800 Subject: [PATCH 30/36] Comment a struct null check false positive --- .../workbench/contrib/terminal/browser/terminalInstance.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index a7e529b0b7b..2aebb53e32d 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -1037,7 +1037,12 @@ export class TerminalInstance implements ITerminalInstance { } if (this._processManager) { - this._processManager.onProcessData(data => this._onProcessData(data)); + // NOTE: The "!" operator should not be required here. But, the strict-null check + // tasks is giving a false positive of. + // src/vs/workbench/contrib/terminal/browser/terminalInstance.ts(1040,25): error TS2339: Property 'onProcessData' does not exist on type 'never'. + // This is because the strict-null check isn't good enough to know that the "this._createProcess" call above will + // reset this._processManager. + this._processManager!.onProcessData(data => this._onProcessData(data)); } } From d5e4009ae9866c5c102b52bc9f36a6f53d2b8679 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 6 Mar 2019 11:47:57 -0800 Subject: [PATCH 31/36] Remove need for acquiring dimensions for terminal from main thread --- build/builtInExtensions.json | 15 ++++++++++++++ .../mainThreadTerminalService.ts | 13 ++---------- src/vs/workbench/api/node/extHost.protocol.ts | 3 +-- src/vs/workbench/api/node/extHostTask.ts | 20 +++++++++++++++++++ .../api/node/extHostTerminalService.ts | 15 ++------------ 5 files changed, 40 insertions(+), 26 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index 4dc8c0619af..c3b1e0cf8ff 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,5 +43,20 @@ }, "publisherDisplayName": "Microsoft" } + }, + { + "name": "ms-vscode.cpptools", + "version": "0.20.1", + "repo": "https://github.com/Microsoft/vscode-cpptools", + "metadata": { + "id": "36d19e17-7569-4841-a001-947eb18602b2", + "publisherId": { + "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", + "publisherName": "ms-vscode", + "displayName": "Microsoft", + "flags": "verified" + }, + "publisherDisplayName": "Microsoft" + } } ] \ No newline at end of file diff --git a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts index 34d53be480b..8a1fc7d216d 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts @@ -118,15 +118,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape } } - public $terminalGetDimensions(terminalId: number): Promise { - const terminalInstance = this.terminalService.getInstanceFromId(terminalId); - if (terminalInstance && terminalInstance.shellLaunchConfig.isRendererOnly) { - return Promise.resolve({ cols: terminalInstance.cols, rows: terminalInstance.rows }); - } - - return Promise.reject(new Error('Dimensions cannot be retrieved')); - } - public $terminalRendererSetDimensions(terminalId: number, dimensions: ITerminalDimensions): void { const terminalInstance = this.terminalService.getInstanceFromId(terminalId); if (terminalInstance && terminalInstance.shellLaunchConfig.isRendererOnly) { @@ -197,10 +188,10 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape private _onTerminalOpened(terminalInstance: ITerminalInstance): void { if (terminalInstance.title) { - this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title, !!terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); + this._proxy.$acceptTerminalOpened(terminalInstance.id, terminalInstance.title); } else { terminalInstance.waitForTitle().then(title => { - this._proxy.$acceptTerminalOpened(terminalInstance.id, title, !!terminalInstance.shellLaunchConfig.isRendererOnly, terminalInstance.cols, terminalInstance.rows); + this._proxy.$acceptTerminalOpened(terminalInstance.id, title); }); } } diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index e4e061ff53f..6300d1062d1 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -387,7 +387,6 @@ export interface MainThreadTerminalServiceShape extends IDisposable { // Renderer $terminalRendererSetName(terminalId: number, name: string): void; $terminalRendererSetDimensions(terminalId: number, dimensions: ITerminalDimensions): void; - $terminalGetDimensions(terminalId: number): Promise; $terminalRendererWrite(terminalId: number, text: string): void; $terminalRendererRegisterOnInputListener(terminalId: number): void; } @@ -974,7 +973,7 @@ export interface ShellLaunchConfigDto { export interface ExtHostTerminalServiceShape { $acceptTerminalClosed(id: number): void; - $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void; + $acceptTerminalOpened(id: number, name: string): void; $acceptActiveTerminalChanged(id: number | null): void; $acceptTerminalProcessId(id: number, processId: number): void; $acceptTerminalProcessData(id: number, data: string): void; diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 04cc8b9f85f..122fff4582b 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -336,6 +336,7 @@ interface HandlerData { } class CustomExecutionData implements IDisposable { + private static waitForDimensionsTimeoutInMs: number = 5000; private _cancellationSource?: CancellationTokenSource; private readonly _onTaskExecutionComplete: Emitter = new Emitter(); private readonly _disposables: IDisposable[] = []; @@ -397,6 +398,25 @@ class CustomExecutionData implements IDisposable { this.terminal = callbackTerminals[0]; const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.resolveTerminalRenderer(terminalId); + + const dimensionTimeout: Promise = new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, CustomExecutionData.waitForDimensionsTimeoutInMs); + }); + + let dimensionsRegistration: IDisposable | undefined; + const dimensionsPromise: Promise = new Promise((resolve) => { + dimensionsRegistration = terminalRenderer.onDidChangeMaximumDimensions((newDimensions) => { + resolve(); + }); + }); + + await Promise.race([dimensionTimeout, dimensionsPromise]); + if (dimensionsRegistration) { + dimensionsRegistration.dispose(); + } + this._cancellationSource = new CancellationTokenSource(); this._disposables.push(this._cancellationSource); diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index f420a4aff54..115b263be66 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -234,20 +234,10 @@ export class ExtHostTerminalRenderer extends BaseExtHostTerminal implements vsco proxy: MainThreadTerminalServiceShape, private _name: string, private _terminal: ExtHostTerminal, - id?: number, - cols?: number, - rows?: number + id?: number ) { super(proxy, id); - // TODO: Should we set maximum dimensions to these as well? - if (cols !== null && rows !== null) { - this._dimensions = { - columns: cols, - rows: rows - }; - } - if (!id) { this._proxy.$createTerminalRenderer(this._name).then(id => { this._runQueuedRequests(id); @@ -335,9 +325,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } } - const dimensions = await this._proxy.$terminalGetDimensions(id); const terminal = this._getTerminalById(id); - const renderer = new ExtHostTerminalRenderer(this._proxy, terminal.name, terminal, terminal._id, dimensions.cols, dimensions.rows); + const renderer = new ExtHostTerminalRenderer(this._proxy, terminal.name, terminal, terminal._id); this._terminalRenderers.push(renderer); return renderer; From 6d665482c81823f5456562fc46757d85fdbfa6d3 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 6 Mar 2019 14:03:16 -0800 Subject: [PATCH 32/36] Put back more of the terminal changes --- src/vs/workbench/api/node/extHostTask.ts | 39 +++++---- .../terminal/browser/terminalInstance.ts | 79 ++++++++++--------- 2 files changed, 65 insertions(+), 53 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTask.ts b/src/vs/workbench/api/node/extHostTask.ts index 122fff4582b..f0184406cab 100644 --- a/src/vs/workbench/api/node/extHostTask.ts +++ b/src/vs/workbench/api/node/extHostTask.ts @@ -345,7 +345,7 @@ class CustomExecutionData implements IDisposable { public result: number | undefined; constructor( - private readonly callbackData: vscode.CustomExecution, + private readonly customExecution: vscode.CustomExecution, private readonly terminalService: ExtHostTerminalService) { } @@ -398,23 +398,28 @@ class CustomExecutionData implements IDisposable { this.terminal = callbackTerminals[0]; const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.resolveTerminalRenderer(terminalId); - - const dimensionTimeout: Promise = new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, CustomExecutionData.waitForDimensionsTimeoutInMs); - }); - - let dimensionsRegistration: IDisposable | undefined; - const dimensionsPromise: Promise = new Promise((resolve) => { - dimensionsRegistration = terminalRenderer.onDidChangeMaximumDimensions((newDimensions) => { - resolve(); + // If we don't have the maximum dimensions yet, then we need to wait for them (but not indefinitely). + // Custom executions will expect the dimensions to be set properly before they are launched. + // BUT, due to the API contract VSCode has for terminals and dimensions, they are still responsible for + // handling cases where they are not set. + if (!terminalRenderer.maximumDimensions) { + const dimensionTimeout: Promise = new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, CustomExecutionData.waitForDimensionsTimeoutInMs); }); - }); - await Promise.race([dimensionTimeout, dimensionsPromise]); - if (dimensionsRegistration) { - dimensionsRegistration.dispose(); + let dimensionsRegistration: IDisposable | undefined; + const dimensionsPromise: Promise = new Promise((resolve) => { + dimensionsRegistration = terminalRenderer.onDidChangeMaximumDimensions((newDimensions) => { + resolve(); + }); + }); + + await Promise.race([dimensionTimeout, dimensionsPromise]); + if (dimensionsRegistration) { + dimensionsRegistration.dispose(); + } } this._cancellationSource = new CancellationTokenSource(); @@ -423,7 +428,7 @@ class CustomExecutionData implements IDisposable { this._disposables.push(this.terminalService.onDidCloseTerminal(this.onDidCloseTerminal.bind(this))); // Regardless of how the task completes, we are done with this custom execution task. - this.callbackData.callback(terminalRenderer, this._cancellationSource.token).then( + this.customExecution.callback(terminalRenderer, this._cancellationSource.token).then( (success) => { this.result = success; this._onTaskExecutionComplete.fire(this); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 2aebb53e32d..945dba47a8e 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -155,7 +155,7 @@ export class TerminalInstance implements ITerminalInstance { private static _idCounter = 1; private _processManager: ITerminalProcessManager | undefined; - private _keyPressListener: IDisposable | undefined; + private _pressAnyKeyToCloseListener: IDisposable | undefined; private _id: number; private _isExiting: boolean; @@ -749,7 +749,7 @@ export class TerminalInstance implements ITerminalInstance { // can simply output the text to the renderer themselves. // All this code does is handle the "wait on exit" condition. if (!this.shellLaunchConfig.isRendererOnly) { - throw new Error('renderExit is only expected to be called on a renderer only terminal'); + throw new Error('rendererExit is only expected to be called on a renderer only terminal'); } return this._onProcessOrExtensionCallbackExit(result); @@ -918,7 +918,7 @@ export class TerminalInstance implements ITerminalInstance { * Called when either a process tied to a terminal has exited or when a custom execution has completed. * @param exitCode The exit code can be undefined if the terminal was exited through user action or if a custom execution callback did not provide a exit code when it finished. */ - private _onProcessOrExtensionCallbackExit(exitCode: number): void { + private _onProcessOrExtensionCallbackExit(exitCode?: number): void { this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); // Prevent dispose functions being triggered multiple times @@ -927,17 +927,22 @@ export class TerminalInstance implements ITerminalInstance { } this._isExiting = true; + let exitCodeMessage: string; - const exitCodeMessage: string = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); + if (exitCode) { + exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); + } if (this._processManager) { - this._logService.debug(`Terminal process exit (id: ${this.id}) state ${this._processManager!.processState}`); + this._logService.debug(`Terminal process exit (id: ${this.id}) state ${this._processManager.processState}`); } // Only trigger wait on exit when the exit was *not* triggered by the // user (via the `workbench.action.terminal.kill` command). if (this._shellLaunchConfig.waitOnExit && (!this._processManager || this._processManager.processState !== ProcessState.KILLED_BY_USER)) { - this._xterm.writeln(exitCodeMessage!); + if (exitCode) { + this._xterm.writeln(exitCodeMessage!); + } if (typeof this._shellLaunchConfig.waitOnExit === 'string') { let message = this._shellLaunchConfig.waitOnExit; // Bold the message and add an extra new line to make it stand out from the rest of the output @@ -951,41 +956,43 @@ export class TerminalInstance implements ITerminalInstance { } } else { this.dispose(); - if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { - let args = ''; - if (typeof this._shellLaunchConfig.args === 'string') { - args = this._shellLaunchConfig.args; - } else if (this._shellLaunchConfig.args && this._shellLaunchConfig.args.length) { - args = ' ' + this._shellLaunchConfig.args.map(a => { - if (typeof a === 'string' && a.indexOf(' ') !== -1) { - return `'${a}'`; - } - return a; - }).join(' '); - } - if (this._shellLaunchConfig.executable) { - this._notificationService.error(nls.localize('terminal.integrated.launchFailed', 'The terminal process command \'{0}{1}\' failed to launch (exit code: {2})', this._shellLaunchConfig.executable, args, exitCode)); + if (exitCode) { + if (this._processManager && this._processManager.processState === ProcessState.KILLED_DURING_LAUNCH) { + let args = ''; + if (typeof this._shellLaunchConfig.args === 'string') { + args = this._shellLaunchConfig.args; + } else if (this._shellLaunchConfig.args && this._shellLaunchConfig.args.length) { + args = ' ' + this._shellLaunchConfig.args.map(a => { + if (typeof a === 'string' && a.indexOf(' ') !== -1) { + return `'${a}'`; + } + return a; + }).join(' '); + } + if (this._shellLaunchConfig.executable) { + this._notificationService.error(nls.localize('terminal.integrated.launchFailed', 'The terminal process command \'{0}{1}\' failed to launch (exit code: {2})', this._shellLaunchConfig.executable, args, exitCode)); + } else { + this._notificationService.error(nls.localize('terminal.integrated.launchFailedExtHost', 'The terminal process failed to launch (exit code: {0})', exitCode)); + } } else { - this._notificationService.error(nls.localize('terminal.integrated.launchFailedExtHost', 'The terminal process failed to launch (exit code: {0})', exitCode)); - } - } else { - if (this._configHelper.config.showExitAlert) { - this._notificationService.error(exitCodeMessage!); - } else { - console.warn(exitCodeMessage!); + if (this._configHelper.config.showExitAlert) { + this._notificationService.error(exitCodeMessage!); + } else { + console.warn(exitCodeMessage!); + } } } } - this._onExit.fire(exitCode); + this._onExit.fire(exitCode || 0); } private _attachPressAnyKeyToCloseListener() { - if (!this._keyPressListener) { - this._keyPressListener = dom.addDisposableListener(this._xterm.textarea, 'keypress', (event: KeyboardEvent) => { - if (this._keyPressListener) { - this._keyPressListener.dispose(); - this._keyPressListener = undefined; + if (!this._pressAnyKeyToCloseListener) { + this._pressAnyKeyToCloseListener = dom.addDisposableListener(this._xterm.textarea, 'keypress', (event: KeyboardEvent) => { + if (this._pressAnyKeyToCloseListener) { + this._pressAnyKeyToCloseListener.dispose(); + this._pressAnyKeyToCloseListener = undefined; this.dispose(); event.preventDefault(); } @@ -995,9 +1002,9 @@ export class TerminalInstance implements ITerminalInstance { public reuseTerminal(shell: IShellLaunchConfig): void { // Unsubscribe any key listener we may have. - if (this._keyPressListener) { - this._keyPressListener.dispose(); - this._keyPressListener = undefined; + if (this._pressAnyKeyToCloseListener) { + this._pressAnyKeyToCloseListener.dispose(); + this._pressAnyKeyToCloseListener = undefined; } // Kill and clear up the process, making the process manager ready for a new process From ac9c1d2615ab9ae2227857eaee786feaa16f96dc Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 6 Mar 2019 14:03:51 -0800 Subject: [PATCH 33/36] Remove CPP tools from built-in json --- build/builtInExtensions.json | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index c3b1e0cf8ff..4dc8c0619af 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -43,20 +43,5 @@ }, "publisherDisplayName": "Microsoft" } - }, - { - "name": "ms-vscode.cpptools", - "version": "0.20.1", - "repo": "https://github.com/Microsoft/vscode-cpptools", - "metadata": { - "id": "36d19e17-7569-4841-a001-947eb18602b2", - "publisherId": { - "publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee", - "publisherName": "ms-vscode", - "displayName": "Microsoft", - "flags": "verified" - }, - "publisherDisplayName": "Microsoft" - } } ] \ No newline at end of file From 9cdff80e19b0742b4a294bbf36c334c6e1762837 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 6 Mar 2019 14:11:09 -0800 Subject: [PATCH 34/36] Make sure to dispose key press event listener in dispose as well --- .../workbench/contrib/terminal/browser/terminalInstance.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 945dba47a8e..553ec0b253f 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -726,6 +726,12 @@ export class TerminalInstance implements ITerminalInstance { this._sendLineData(buffer, buffer.ybase + buffer.y); this._xterm.dispose(); } + + if (this._pressAnyKeyToCloseListener) { + this._pressAnyKeyToCloseListener.dispose(); + this._pressAnyKeyToCloseListener = undefined; + } + if (this._processManager) { this._processManager.dispose(immediate); } else { From a224fa02edc598a59e47abeb448b2065dfc21eb0 Mon Sep 17 00:00:00 2001 From: Gabriel DeBacker Date: Wed, 6 Mar 2019 19:19:48 -0800 Subject: [PATCH 35/36] Use IDisposable from lifecycle --- src/vs/workbench/contrib/terminal/browser/terminalInstance.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 553ec0b253f..3a33cd560ef 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -32,7 +32,7 @@ import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/term import { TerminalLinkHandler } from 'vs/workbench/contrib/terminal/browser/terminalLinkHandler'; import { TerminalCommandTracker } from 'vs/workbench/contrib/terminal/browser/terminalCommandTracker'; import { IPanelService } from 'vs/workbench/services/panel/common/panelService'; -import { ISearchOptions, Terminal as XTermTerminal, IDisposable } from 'vscode-xterm'; +import { ISearchOptions, Terminal as XTermTerminal } from 'vscode-xterm'; import { IAccessibilityService, AccessibilitySupport } from 'vs/platform/accessibility/common/accessibility'; import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; @@ -155,7 +155,7 @@ export class TerminalInstance implements ITerminalInstance { private static _idCounter = 1; private _processManager: ITerminalProcessManager | undefined; - private _pressAnyKeyToCloseListener: IDisposable | undefined; + private _pressAnyKeyToCloseListener: lifecycle.IDisposable | undefined; private _id: number; private _isExiting: boolean; From 286a61bd0560d8740aec9281ce80b496654d715b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 7 Mar 2019 09:13:40 -0800 Subject: [PATCH 36/36] Clean up some comments/names --- build/builtInExtensions.json | 2 +- .../terminal/browser/terminalInstance.ts | 32 ++++++++----------- .../contrib/terminal/common/terminal.ts | 5 +-- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/build/builtInExtensions.json b/build/builtInExtensions.json index 4dc8c0619af..e0794621304 100644 --- a/build/builtInExtensions.json +++ b/build/builtInExtensions.json @@ -44,4 +44,4 @@ "publisherDisplayName": "Microsoft" } } -] \ No newline at end of file +] diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 3a33cd560ef..b8ef5a55e61 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -748,17 +748,14 @@ export class TerminalInstance implements ITerminalInstance { this._disposables = lifecycle.dispose(this._disposables); } - public rendererExit(result: number): void { - // The use of this API is for cases where there is no backing process - // behind a terminal instance (such as when executing an custom execution task). - // There is no associated string, error text, etc, as the consumer of the renderer - // can simply output the text to the renderer themselves. - // All this code does is handle the "wait on exit" condition. + public rendererExit(exitCode: number): void { + // The use of this API is for cases where there is no backing process behind a terminal + // instance (eg. a custom execution task). if (!this.shellLaunchConfig.isRendererOnly) { throw new Error('rendererExit is only expected to be called on a renderer only terminal'); } - return this._onProcessOrExtensionCallbackExit(result); + return this._onProcessExit(exitCode); } public forceRedraw(): void { @@ -883,7 +880,7 @@ export class TerminalInstance implements ITerminalInstance { protected _createProcess(): void { this._processManager = this._terminalInstanceService.createTerminalProcessManager(this._id, this._configHelper); this._processManager.onProcessReady(() => this._onProcessIdReady.fire(this)); - this._processManager.onProcessExit(exitCode => this._onProcessOrExtensionCallbackExit(exitCode)); + this._processManager.onProcessExit(exitCode => this._onProcessExit(exitCode)); this._processManager.onProcessData(data => this._onData.fire(data)); if (this._shellLaunchConfig.name) { @@ -921,10 +918,12 @@ export class TerminalInstance implements ITerminalInstance { } /** - * Called when either a process tied to a terminal has exited or when a custom execution has completed. - * @param exitCode The exit code can be undefined if the terminal was exited through user action or if a custom execution callback did not provide a exit code when it finished. + * Called when either a process tied to a terminal has exited or when a terminal renderer + * simulates a process exiting (eg. custom execution task). + * @param exitCode The exit code of the process, this is undefined when the terminal was exited + * through user action. */ - private _onProcessOrExtensionCallbackExit(exitCode?: number): void { + private _onProcessExit(exitCode?: number): void { this._logService.debug(`Terminal process exit (id: ${this.id}) with code ${exitCode}`); // Prevent dispose functions being triggered multiple times @@ -939,9 +938,7 @@ export class TerminalInstance implements ITerminalInstance { exitCodeMessage = nls.localize('terminal.integrated.exitedWithCode', 'The terminal process terminated with exit code: {0}', exitCode); } - if (this._processManager) { - this._logService.debug(`Terminal process exit (id: ${this.id}) state ${this._processManager.processState}`); - } + this._logService.debug(`Terminal process exit (id: ${this.id})${this._processManager ? ' state ' + this._processManager.processState : ''}`); // Only trigger wait on exit when the exit was *not* triggered by the // user (via the `workbench.action.terminal.kill` command). @@ -1050,11 +1047,8 @@ export class TerminalInstance implements ITerminalInstance { } if (this._processManager) { - // NOTE: The "!" operator should not be required here. But, the strict-null check - // tasks is giving a false positive of. - // src/vs/workbench/contrib/terminal/browser/terminalInstance.ts(1040,25): error TS2339: Property 'onProcessData' does not exist on type 'never'. - // This is because the strict-null check isn't good enough to know that the "this._createProcess" call above will - // reset this._processManager. + // The "!" operator is required here because _processManager is set to undefiend earlier + // and TS does not know that createProcess sets it. this._processManager!.onProcessData(data => this._onProcessData(data)); } } diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index d7cb9d9ee5a..b718dba571e 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -442,9 +442,10 @@ export interface ITerminalInstance { /** * Indicates that a consumer of a renderer only terminal is finished with it. * - * @param result - Result code from a from a task process or custom execution. Zero indicates success, non-zero indicates failure. + * @param exitCode The exit code of the terminal. Zero indicates success, non-zero indicates + * failure. */ - rendererExit(result: number): void; + rendererExit(exitCode: number): void; /** * Forces the terminal to redraw its viewport.