Clean up comments, fix some code review issues, and make the exit code...

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

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<void> {
+	public $customTaskExecutionComplete(id: string, result?: number): Promise<void> {
 		return new Promise<void>((resolve, reject) => {
 			this._taskService.getActiveTasks().then((tasks) => {
 				for (let task of tasks) {

src/vs/workbench/api/node/extHost.protocol.ts

@@ -554,7 +554,7 @@ export interface MainThreadTaskShape extends IDisposable {
 	$executeTask(task: TaskHandleDTO | TaskDTO): Promise<TaskExecutionDTO>;
 	$terminateTask(id: string): Promise<void>;
 	$registerTaskSystem(scheme: string, info: TaskSystemInfoDTO): void;
-	$customTaskExecutionComplete(id: string, result: number | undefined): Promise<void>;
+	$customTaskExecutionComplete(id: string, result?: number): Promise<void>;
 }
 
 export interface MainThreadExtensionServiceShape extends IDisposable {

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<void> {
-		// 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<void>[] = [];
 		let fetchPromise = asPromise(() => handler.provider.provideTasks(CancellationToken.None)).then(value => {
 			const taskDTOs: TaskDTO[] = [];

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<vscode.Terminal> = new Emitter<vscode.Terminal>();
 	public get onDidCloseTerminal(): Event<vscode.Terminal> { 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;
 			}

src/vs/workbench/contrib/tasks/common/taskSystem.ts

@@ -136,5 +136,5 @@ export interface ITaskSystem {
 	terminate(task: Task): Promise<TaskTerminateResponse>;
 	terminateAll(): Promise<TaskTerminateResponse[]>;
 	revealTask(task: Task): boolean;
-	extensionCallbackTaskComplete(task: Task, result: number | undefined): Promise<void>;
+	customTaskExecutionComplete(task: Task, result: number | undefined): Promise<void>;
 }
\ No newline at end of file

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;

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<Task | undefined> {

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<void> {
+	public customTaskExecutionComplete(task: Task, result?: number): Promise<void> {
 		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<void>((resolve) => {
@@ -1114,7 +1114,7 @@ export class TerminalTaskSystem implements ITaskSystem {
 	}
 
 	private collectCommandVariables(variables: Set<string>, 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;

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<void> {
-		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<void> {
+		throw new TaskError(Severity.Error, 'Custom execution task completion is never expected in the process task system.', TaskErrors.UnknownError);
 	}
 
 	public hasErrors(value: boolean): void {

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<number>;
+	onExit: Event<number | undefined>;
 
 	processReady: Promise<void>;
 
@@ -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.

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<number>();
-	public get onExit(): Event<number> { return this._onExit.event; }
+	private readonly _onExit = new Emitter<number | undefined>();
+	public get onExit(): Event<number | undefined> { return this._onExit.event; }
 	private readonly _onDisposed = new Emitter<ITerminalInstance>();
 	public get onDisposed(): Event<ITerminalInstance> { return this._onDisposed.event; }
 	private readonly _onFocused = new Emitter<ITerminalInstance>();
@@ -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));
 		}
 	}