Code review updates

This commit is contained in:
Gabriel DeBacker
2019-02-19 14:33:25 -08:00
parent f8f8f519d9
commit 5d1720321d
13 changed files with 94 additions and 109 deletions

View File

@@ -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,

View File

@@ -556,7 +556,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): Promise<void>;
$customExecutionComplete(id: string, result?: number): Promise<void>;
}
export interface MainThreadExtensionServiceShape extends IDisposable {

View File

@@ -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 ((<vscode.TaskWithCustomTaskExecution>value).executionWithExtensionCallback && (<vscode.TaskWithCustomTaskExecution>value).executionWithExtensionCallback instanceof types.CustomTaskExecution) {
execution = CustomTaskExecutionDTO.from(<types.CustomTaskExecution>(<vscode.TaskWithCustomTaskExecution>value).executionWithExtensionCallback);
} else if ((<vscode.Task2>value).execution2 && (<vscode.Task2>value).execution2 instanceof types.CustomExecution) {
execution = CustomExecutionDTO.from(<types.CustomExecution>(<vscode.Task2>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<CustomTaskExecutionData> = new Emitter<CustomTaskExecutionData>();
private readonly _onTaskExecutionComplete: Emitter<CustomExecutionData> = new Emitter<CustomExecutionData>();
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<CustomTaskExecutionData> {
public get onTaskExecutionComplete(): Event<CustomExecutionData> {
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<number, HandlerData>;
private _taskExecutions: Map<string, TaskExecutionImpl>;
private _providedCustomTaskExecutions: Map<string, CustomTaskExecutionData>;
private _activeCustomTaskExecutions: Map<string, CustomTaskExecutionData>;
private _providedCustomExecutions: Map<string, CustomExecutionData>;
private _activeCustomExecutions: Map<string, CustomExecutionData>;
private readonly _onDidExecuteTask: Emitter<vscode.TaskStartEvent> = new Emitter<vscode.TaskStartEvent>();
private readonly _onDidTerminateTask: Emitter<vscode.TaskEndEvent> = new Emitter<vscode.TaskEndEvent>();
@@ -447,8 +447,8 @@ export class ExtHostTask implements ExtHostTaskShape {
this._handleCounter = 0;
this._handlers = new Map<number, HandlerData>();
this._taskExecutions = new Map<string, TaskExecutionImpl>();
this._providedCustomTaskExecutions = new Map<string, CustomTaskExecutionData>();
this._activeCustomTaskExecutions = new Map<string, CustomTaskExecutionData>();
this._providedCustomExecutions = new Map<string, CustomExecutionData>();
this._activeCustomExecutions = new Map<string, CustomExecutionData>();
}
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(<vscode.CustomTaskExecution>(<vscode.TaskWithCustomTaskExecution>task).executionWithExtensionCallback, this._terminalService));
this._providedCustomExecutions.set(taskId, new CustomExecutionData(<vscode.CustomExecution>(<vscode.Task2>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();
}
}

View File

@@ -332,18 +332,16 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
return renderer;
}
public async createTerminalRendererForTerminal(terminal: vscode.Terminal): Promise<vscode.TerminalRenderer> {
if (!(terminal instanceof ExtHostTerminal)) {
throw new Error('Only expected instance extension host terminal type');
}
public async resolveTerminalRenderer(id: number): Promise<vscode.TerminalRenderer> {
// 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 {

View File

@@ -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<number | undefined>;
constructor(callback: (args: vscode.TerminalRenderer, cancellationToken: vscode.CancellationToken) => Thenable<number | undefined>) {
@@ -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;
}