Skip to content

Commit 5d17203

Browse files
GabeDeBackerGabeDeBacker
committed
Code review updates
1 parent f8f8f51 commit 5d17203

13 files changed

Lines changed: 94 additions & 109 deletions

File tree

‎src/vs/vscode.proposed.d.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,7 +1184,7 @@ declare module 'vscode' {
11841184
/**
11851185
* Class used to execute an extension callback as a task.
11861186
*/
1187-
export class CustomTaskExecution {
1187+
export class CustomExecution {
11881188
/**
11891189
* @param callback The callback that will be called when the extension callback task is executed.
11901190
*/
@@ -1199,7 +1199,7 @@ declare module 'vscode' {
11991199
/**
12001200
* A task to execute
12011201
*/
1202-
export class TaskWithCustomTaskExecution extends Task {
1202+
export class Task2 extends Task {
12031203
/**
12041204
* Creates a new task.
12051205
*
@@ -1212,12 +1212,12 @@ declare module 'vscode' {
12121212
* or '$eslint'. Problem matchers can be contributed by an extension using
12131213
* the `problemMatchers` extension point.
12141214
*/
1215-
constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomTaskExecution, problemMatchers?: string | string[]);
1215+
constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution | CustomExecution, problemMatchers?: string | string[]);
12161216

12171217
/**
12181218
* The task's execution engine
12191219
*/
1220-
executionWithExtensionCallback?: ProcessExecution | ShellExecution | CustomTaskExecution;
1220+
execution2?: ProcessExecution | ShellExecution | CustomExecution;
12211221
}
12221222

12231223
//#region Tasks

‎src/vs/workbench/api/electron-browser/mainThreadTask.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { extHostNamedCustomer } from 'vs/workbench/api/electron-browser/extHostC
3030
import { ExtHostContext, MainThreadTaskShape, ExtHostTaskShape, MainContext, IExtHostContext } from 'vs/workbench/api/node/extHost.protocol';
3131
import {
3232
TaskDefinitionDTO, TaskExecutionDTO, ProcessExecutionOptionsDTO, TaskPresentationOptionsDTO,
33-
ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, CustomTaskExecutionDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO,
33+
ProcessExecutionDTO, ShellExecutionDTO, ShellExecutionOptionsDTO, CustomExecutionDTO, TaskDTO, TaskSourceDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO,
3434
RunOptionsDTO
3535
} from 'vs/workbench/api/shared/tasks';
3636
import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver';
@@ -138,7 +138,7 @@ namespace ProcessExecutionOptionsDTO {
138138
}
139139

140140
namespace ProcessExecutionDTO {
141-
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ProcessExecutionDTO {
141+
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ProcessExecutionDTO {
142142
let candidate = value as ProcessExecutionDTO;
143143
return candidate && !!candidate.process;
144144
}
@@ -206,7 +206,7 @@ namespace ShellExecutionOptionsDTO {
206206
}
207207

208208
namespace ShellExecutionDTO {
209-
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ShellExecutionDTO {
209+
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ShellExecutionDTO {
210210
let candidate = value as ShellExecutionDTO;
211211
return candidate && (!!candidate.commandLine || !!candidate.command);
212212
}
@@ -237,21 +237,21 @@ namespace ShellExecutionDTO {
237237
}
238238
}
239239

240-
namespace CustomTaskExecutionDTO {
241-
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is CustomTaskExecutionDTO {
242-
let candidate = value as CustomTaskExecutionDTO;
243-
return candidate && candidate.customTaskExecution === 'customTaskExecution';
240+
namespace CustomExecutionDTO {
241+
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is CustomExecutionDTO {
242+
let candidate = value as CustomExecutionDTO;
243+
return candidate && candidate.customExecution === 'customExecution';
244244
}
245245

246-
export function from(value: CommandConfiguration): CustomTaskExecutionDTO {
246+
export function from(value: CommandConfiguration): CustomExecutionDTO {
247247
return {
248-
customTaskExecution: 'customTaskExecution'
248+
customExecution: 'customExecution'
249249
};
250250
}
251251

252-
export function to(value: CustomTaskExecutionDTO): CommandConfiguration {
252+
export function to(value: CustomExecutionDTO): CommandConfiguration {
253253
return {
254-
runtime: RuntimeType.CustomTaskExecution,
254+
runtime: RuntimeType.CustomExecution,
255255
presentation: undefined
256256
};
257257
}
@@ -358,8 +358,8 @@ namespace TaskDTO {
358358
command = ShellExecutionDTO.to(task.execution);
359359
} else if (ProcessExecutionDTO.is(task.execution)) {
360360
command = ProcessExecutionDTO.to(task.execution);
361-
} else if (CustomTaskExecutionDTO.is(task.execution)) {
362-
command = CustomTaskExecutionDTO.to(task.execution);
361+
} else if (CustomExecutionDTO.is(task.execution)) {
362+
command = CustomExecutionDTO.to(task.execution);
363363
}
364364
}
365365

@@ -518,7 +518,7 @@ export class MainThreadTask implements MainThreadTaskShape {
518518
});
519519
}
520520

521-
public $customTaskExecutionComplete(id: string, result?: number): Promise<void> {
521+
public $customExecutionComplete(id: string, result?: number): Promise<void> {
522522
return new Promise<void>((resolve, reject) => {
523523
this._taskService.getActiveTasks().then((tasks) => {
524524
for (let task of tasks) {

‎src/vs/workbench/api/node/extHost.api.impl.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ export function createApiFactory(
783783
DocumentSymbol: extHostTypes.DocumentSymbol,
784784
EndOfLine: extHostTypes.EndOfLine,
785785
EventEmitter: Emitter,
786-
CustomTaskExecution: extHostTypes.CustomTaskExecution,
786+
CustomExecution: extHostTypes.CustomExecution,
787787
FileChangeType: extHostTypes.FileChangeType,
788788
FileSystemError: extHostTypes.FileSystemError,
789789
FileType: files.FileType,
@@ -819,7 +819,7 @@ export function createApiFactory(
819819
SymbolInformation: extHostTypes.SymbolInformation,
820820
SymbolKind: extHostTypes.SymbolKind,
821821
Task: extHostTypes.Task,
822-
TaskWithCustomTaskExecution: extHostTypes.Task,
822+
Task2: extHostTypes.Task,
823823
TaskGroup: extHostTypes.TaskGroup,
824824
TaskPanelKind: extHostTypes.TaskPanelKind,
825825
TaskRevealKind: extHostTypes.TaskRevealKind,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ export interface MainThreadTaskShape extends IDisposable {
556556
$executeTask(task: TaskHandleDTO | TaskDTO): Promise<TaskExecutionDTO>;
557557
$terminateTask(id: string): Promise<void>;
558558
$registerTaskSystem(scheme: string, info: TaskSystemInfoDTO): void;
559-
$customTaskExecutionComplete(id: string, result?: number): Promise<void>;
559+
$customExecutionComplete(id: string, result?: number): Promise<void>;
560560
}
561561

562562
export interface MainThreadExtensionServiceShape extends IDisposable {

‎src/vs/workbench/api/node/extHostTask.ts‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
TaskDefinitionDTO, TaskExecutionDTO, TaskPresentationOptionsDTO,
2323
ProcessExecutionOptionsDTO, ProcessExecutionDTO,
2424
ShellExecutionOptionsDTO, ShellExecutionDTO,
25-
CustomTaskExecutionDTO,
25+
CustomExecutionDTO,
2626
TaskDTO, TaskHandleDTO, TaskFilterDTO, TaskProcessStartedDTO, TaskProcessEndedDTO, TaskSystemInfoDTO, TaskSetDTO
2727
} from '../shared/tasks';
2828
import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService';
@@ -79,7 +79,7 @@ namespace ProcessExecutionOptionsDTO {
7979
}
8080

8181
namespace ProcessExecutionDTO {
82-
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ProcessExecutionDTO {
82+
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ProcessExecutionDTO {
8383
let candidate = value as ProcessExecutionDTO;
8484
return candidate && !!candidate.process;
8585
}
@@ -120,7 +120,7 @@ namespace ShellExecutionOptionsDTO {
120120
}
121121

122122
namespace ShellExecutionDTO {
123-
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is ShellExecutionDTO {
123+
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is ShellExecutionDTO {
124124
let candidate = value as ShellExecutionDTO;
125125
return candidate && (!!candidate.commandLine || !!candidate.command);
126126
}
@@ -153,15 +153,15 @@ namespace ShellExecutionDTO {
153153
}
154154
}
155155

156-
namespace CustomTaskExecutionDTO {
157-
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO): value is CustomTaskExecutionDTO {
158-
let candidate = value as CustomTaskExecutionDTO;
159-
return candidate && candidate.customTaskExecution === 'customTaskExecution';
156+
namespace CustomExecutionDTO {
157+
export function is(value: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO): value is CustomExecutionDTO {
158+
let candidate = value as CustomExecutionDTO;
159+
return candidate && candidate.customExecution === 'customExecution';
160160
}
161161

162-
export function from(value: vscode.CustomTaskExecution): CustomTaskExecutionDTO {
162+
export function from(value: vscode.CustomExecution): CustomExecutionDTO {
163163
return {
164-
customTaskExecution: 'customTaskExecution'
164+
customExecution: 'customExecution'
165165
};
166166
}
167167
}
@@ -199,13 +199,13 @@ namespace TaskDTO {
199199
if (value === undefined || value === null) {
200200
return undefined;
201201
}
202-
let execution: ShellExecutionDTO | ProcessExecutionDTO | CustomTaskExecutionDTO;
202+
let execution: ShellExecutionDTO | ProcessExecutionDTO | CustomExecutionDTO;
203203
if (value.execution instanceof types.ProcessExecution) {
204204
execution = ProcessExecutionDTO.from(value.execution);
205205
} else if (value.execution instanceof types.ShellExecution) {
206206
execution = ShellExecutionDTO.from(value.execution);
207-
} else if ((<vscode.TaskWithCustomTaskExecution>value).executionWithExtensionCallback && (<vscode.TaskWithCustomTaskExecution>value).executionWithExtensionCallback instanceof types.CustomTaskExecution) {
208-
execution = CustomTaskExecutionDTO.from(<types.CustomTaskExecution>(<vscode.TaskWithCustomTaskExecution>value).executionWithExtensionCallback);
207+
} else if ((<vscode.Task2>value).execution2 && (<vscode.Task2>value).execution2 instanceof types.CustomExecution) {
208+
execution = CustomExecutionDTO.from(<types.CustomExecution>(<vscode.Task2>value).execution2);
209209
}
210210

211211
let definition: TaskDefinitionDTO = TaskDefinitionDTO.from(value.definition);
@@ -336,24 +336,24 @@ interface HandlerData {
336336
extension: IExtensionDescription;
337337
}
338338

339-
class CustomTaskExecutionData implements IDisposable {
339+
class CustomExecutionData implements IDisposable {
340340
private _cancellationSource?: CancellationTokenSource;
341-
private readonly _onTaskExecutionComplete: Emitter<CustomTaskExecutionData> = new Emitter<CustomTaskExecutionData>();
341+
private readonly _onTaskExecutionComplete: Emitter<CustomExecutionData> = new Emitter<CustomExecutionData>();
342342
private readonly _disposables: IDisposable[] = [];
343343
private terminal?: vscode.Terminal;
344344
private terminalId?: number;
345345
public result: number | undefined;
346346

347347
constructor(
348-
private readonly callbackData: vscode.CustomTaskExecution,
348+
private readonly callbackData: vscode.CustomExecution,
349349
private readonly terminalService: ExtHostTerminalService) {
350350
}
351351

352352
public dispose(): void {
353353
dispose(this._disposables);
354354
}
355355

356-
public get onTaskExecutionComplete(): Event<CustomTaskExecutionData> {
356+
public get onTaskExecutionComplete(): Event<CustomExecutionData> {
357357
return this._onTaskExecutionComplete.event;
358358
}
359359

@@ -396,7 +396,7 @@ class CustomTaskExecutionData implements IDisposable {
396396
}
397397

398398
this.terminal = callbackTerminals[0];
399-
const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.createTerminalRendererForTerminal(this.terminal);
399+
const terminalRenderer: vscode.TerminalRenderer = await this.terminalService.resolveTerminalRenderer(terminalId);
400400

401401
this._cancellationSource = new CancellationTokenSource();
402402
this._disposables.push(this._cancellationSource);
@@ -424,8 +424,8 @@ export class ExtHostTask implements ExtHostTaskShape {
424424
private _handleCounter: number;
425425
private _handlers: Map<number, HandlerData>;
426426
private _taskExecutions: Map<string, TaskExecutionImpl>;
427-
private _providedCustomTaskExecutions: Map<string, CustomTaskExecutionData>;
428-
private _activeCustomTaskExecutions: Map<string, CustomTaskExecutionData>;
427+
private _providedCustomExecutions: Map<string, CustomExecutionData>;
428+
private _activeCustomExecutions: Map<string, CustomExecutionData>;
429429

430430
private readonly _onDidExecuteTask: Emitter<vscode.TaskStartEvent> = new Emitter<vscode.TaskStartEvent>();
431431
private readonly _onDidTerminateTask: Emitter<vscode.TaskEndEvent> = new Emitter<vscode.TaskEndEvent>();
@@ -447,8 +447,8 @@ export class ExtHostTask implements ExtHostTaskShape {
447447
this._handleCounter = 0;
448448
this._handlers = new Map<number, HandlerData>();
449449
this._taskExecutions = new Map<string, TaskExecutionImpl>();
450-
this._providedCustomTaskExecutions = new Map<string, CustomTaskExecutionData>();
451-
this._activeCustomTaskExecutions = new Map<string, CustomTaskExecutionData>();
450+
this._providedCustomExecutions = new Map<string, CustomExecutionData>();
451+
this._activeCustomExecutions = new Map<string, CustomExecutionData>();
452452
}
453453

454454
public registerTaskProvider(extension: IExtensionDescription, provider: vscode.TaskProvider): vscode.Disposable {
@@ -518,16 +518,16 @@ export class ExtHostTask implements ExtHostTaskShape {
518518
// Once a terminal is spun up for the custom execution task this event will be fired.
519519
// At that point, we need to actually start the callback, but
520520
// only if it hasn't already begun.
521-
const extensionCallback: CustomTaskExecutionData | undefined = this._providedCustomTaskExecutions.get(execution.id);
521+
const extensionCallback: CustomExecutionData | undefined = this._providedCustomExecutions.get(execution.id);
522522
if (extensionCallback) {
523-
if (this._activeCustomTaskExecutions.get(execution.id) !== undefined) {
523+
if (this._activeCustomExecutions.get(execution.id) !== undefined) {
524524
throw new Error('We should not be trying to start the same custom task executions twice.');
525525
}
526526

527-
this._activeCustomTaskExecutions.set(execution.id, extensionCallback);
527+
this._activeCustomExecutions.set(execution.id, extensionCallback);
528528

529529
const taskExecutionComplete: IDisposable = extensionCallback.onTaskExecutionComplete(() => {
530-
this.customTaskExecutionComplete(execution);
530+
this.customExecutionComplete(execution);
531531
taskExecutionComplete.dispose();
532532
});
533533

@@ -548,7 +548,7 @@ export class ExtHostTask implements ExtHostTaskShape {
548548
const workspaceProvider = await this._workspaceService.getWorkspaceProvider();
549549
const _execution = this.getTaskExecution(execution, workspaceProvider);
550550
this._taskExecutions.delete(execution.id);
551-
this.customTaskExecutionComplete(execution);
551+
this.customExecutionComplete(execution);
552552
this._onDidTerminateTask.fire({
553553
execution: _execution
554554
});
@@ -593,7 +593,7 @@ export class ExtHostTask implements ExtHostTaskShape {
593593
// For custom execution tasks, we need to store the execution objects locally
594594
// since we obviously cannot send callback functions through the proxy.
595595
// So, clear out any existing ones.
596-
this._providedCustomTaskExecutions.clear();
596+
this._providedCustomExecutions.clear();
597597

598598
// Set up a list of task ID promises that we can wait on
599599
// before returning the provided tasks. The ensures that
@@ -614,13 +614,13 @@ export class ExtHostTask implements ExtHostTaskShape {
614614
const taskDTO: TaskDTO = TaskDTO.from(task, handler.extension);
615615
taskDTOs.push(taskDTO);
616616

617-
if (CustomTaskExecutionDTO.is(taskDTO.execution)) {
617+
if (CustomExecutionDTO.is(taskDTO.execution)) {
618618
taskIdPromises.push(new Promise((resolve) => {
619619
// The ID is calculated on the main thread task side, so, let's call into it here.
620620
// We need the task id's pre-computed for custom task executions because when OnDidStartTask
621621
// is invoked, we have to be able to map it back to our data.
622622
this._proxy.$createTaskId(taskDTO).then((taskId) => {
623-
this._providedCustomTaskExecutions.set(taskId, new CustomTaskExecutionData(<vscode.CustomTaskExecution>(<vscode.TaskWithCustomTaskExecution>task).executionWithExtensionCallback, this._terminalService));
623+
this._providedCustomExecutions.set(taskId, new CustomExecutionData(<vscode.CustomExecution>(<vscode.Task2>task).execution2, this._terminalService));
624624
resolve();
625625
});
626626
}));
@@ -698,11 +698,11 @@ export class ExtHostTask implements ExtHostTaskShape {
698698
return result;
699699
}
700700

701-
private customTaskExecutionComplete(execution: TaskExecutionDTO): void {
702-
const extensionCallback: CustomTaskExecutionData | undefined = this._activeCustomTaskExecutions.get(execution.id);
701+
private customExecutionComplete(execution: TaskExecutionDTO): void {
702+
const extensionCallback: CustomExecutionData | undefined = this._activeCustomExecutions.get(execution.id);
703703
if (extensionCallback) {
704-
this._activeCustomTaskExecutions.delete(execution.id);
705-
this._proxy.$customTaskExecutionComplete(execution.id, extensionCallback.result);
704+
this._activeCustomExecutions.delete(execution.id);
705+
this._proxy.$customExecutionComplete(execution.id, extensionCallback.result);
706706
extensionCallback.dispose();
707707
}
708708
}

‎src/vs/workbench/api/node/extHostTerminalService.ts‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -332,18 +332,16 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
332332
return renderer;
333333
}
334334

335-
public async createTerminalRendererForTerminal(terminal: vscode.Terminal): Promise<vscode.TerminalRenderer> {
336-
if (!(terminal instanceof ExtHostTerminal)) {
337-
throw new Error('Only expected instance extension host terminal type');
338-
}
335+
public async resolveTerminalRenderer(id: number): Promise<vscode.TerminalRenderer> {
339336
// Check to see if the extension host already knows about this terminal.
340337
for (const terminalRenderer of this._terminalRenderers) {
341-
if (terminalRenderer._id === terminal._id) {
338+
if (terminalRenderer._id === id) {
342339
return terminalRenderer;
343340
}
344341
}
345342

346-
const dimensions = await this._proxy.$terminalGetDimensions(terminal._id);
343+
const dimensions = await this._proxy.$terminalGetDimensions(id);
344+
const terminal = this._getTerminalById(id);
347345
const renderer = new ExtHostTerminalRenderer(this._proxy, terminal.name, terminal, terminal._id, dimensions.cols, dimensions.rows);
348346
this._terminalRenderers.push(renderer);
349347

@@ -415,31 +413,18 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
415413
}
416414
}
417415

418-
public $acceptTerminalOpened(id: number, name: string, isRendererOnly: boolean, cols: number, rows: number): void {
419-
let index = this._getTerminalObjectIndexById(this._terminals, id);
416+
public $acceptTerminalOpened(id: number, name: string): void {
417+
const index = this._getTerminalObjectIndexById(this._terminals, id);
420418
if (index !== null) {
419+
// The terminal has already been created (via createTerminal*), only fire the event
421420
this._onDidOpenTerminal.fire(this.terminals[index]);
422-
}
423-
424-
let renderer = this._getTerminalRendererById(id);
425-
426-
// If this is a terminal created by one of the public createTerminal* APIs
427-
// then @acceptTerminalOpened was called from the main thread task
428-
// to indicate that the terminal is ready for use.
429-
// In those cases, we don't need to create the extension host objects
430-
// as they already exist, but, we do still need to fire the events
431-
// to our consumers.
432-
if ((renderer !== null) && (index !== null)) {
433421
return;
434422
}
435423

436-
// The extension host did not know about this terminal, so create extension host
437-
// objects to represent them.
438-
if (!index) {
439-
const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined);
440-
index = this._terminals.push(terminal);
441-
this._onDidOpenTerminal.fire(terminal);
442-
}
424+
const renderer = this._getTerminalRendererById(id);
425+
const terminal = new ExtHostTerminal(this._proxy, name, id, renderer ? RENDERER_NO_PROCESS_ID : undefined);
426+
this._terminals.push(terminal);
427+
this._onDidOpenTerminal.fire(terminal);
443428
}
444429

445430
public $acceptTerminalProcessId(id: number, processId: number): void {

0 commit comments

Comments
 (0)