Skip to content

Commit

Permalink
Clean up handling of 'taskType' field
Browse files Browse the repository at this point in the history 
Signed-off-by: Thomas Mäder <tmader@redhat.com>
  • Loading branch information
@tsmaeder
tsmaeder committed Apr 29, 2021
1 parent ce8b208 commit 6439a6d
Show file tree
Hide file tree
Showing 12 changed files with 155 additions and 211 deletions.
2 changes: 2 additions & 0 deletions packages/plugin-ext/src/common/plugin-api-rpc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,8 @@ export interface CommandProperties {

export interface TaskDto {
type: string;
taskType?: 'shell' | 'process' | 'customExecution'; // the task execution type
executionId?: string,
label: string;
source?: string;
scope: string | number;
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/main/browser/tasks-main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class TasksMainImpl implements TasksMain, Disposable {

const toDispose = new DisposableCollection(
this.taskProviderRegistry.register(type, taskProvider, handle),
this.taskResolverRegistry.register(type, taskResolver),
this.taskResolverRegistry.registerTaskResolver(type, taskResolver),
Disposable.create(() => this.taskProviders.delete(handle))
);
this.taskProviders.set(handle, toDispose);
Expand Down
75 changes: 28 additions & 47 deletions packages/plugin-ext/src/plugin/tasks/tasks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,19 @@ import { RPCProtocol, ConnectionClosedError } from '../../common/rpc-protocol';
import { TaskProviderAdapter } from './task-provider';
import { Emitter, Event } from '@theia/core/lib/common/event';
import { TerminalServiceExtImpl } from '../terminal-ext';
import { UUID } from '@theia/core/shared/@phosphor/coreutils';

type ExecutionCallback = (resolvedDefintion: theia.TaskDefinition) => Thenable<theia.Pseudoterminal>;
export class TasksExtImpl implements TasksExt {
private proxy: TasksMain;

private callId = 0;
private adaptersMap = new Map<number, TaskProviderAdapter>();
private executions = new Map<number, theia.TaskExecution>();
protected providedCustomExecutions: Map<number, CustomExecution>;
protected notProvidedCustomExecutions: Set<number>;
protected activeCustomExecutions: Map<number, CustomExecution>;
protected callbackIdBase: string = UUID.uuid4();
protected callbackId: number;
protected customExecutionIds: Map<ExecutionCallback, string> = new Map();
protected customExecutionFunctions: Map<string, ExecutionCallback> = new Map();
protected lastStartedTask: number | undefined;

private readonly onDidExecuteTask: Emitter<theia.TaskStartEvent> = new Emitter<theia.TaskStartEvent>();
Expand All @@ -49,9 +52,6 @@ export class TasksExtImpl implements TasksExt {

constructor(rpc: RPCProtocol, readonly terminalExt: TerminalServiceExtImpl) {
this.proxy = rpc.getProxy(PLUGIN_RPC_CONTEXT.TASKS_MAIN);
this.providedCustomExecutions = new Map<number, CustomExecution>();
this.notProvidedCustomExecutions = new Set<number>();
this.activeCustomExecutions = new Map<number, CustomExecution>();
this.fetchTaskExecutions();
}

Expand All @@ -68,16 +68,10 @@ export class TasksExtImpl implements TasksExt {
}

async $onDidStartTask(execution: TaskExecutionDto, terminalId: number): Promise<void> {
const customExecution: CustomExecution | undefined = this.providedCustomExecutions.get(execution.task.id);
const customExecution = this.customExecutionFunctions.get(execution.task.executionId || '');
if (customExecution) {
if (this.activeCustomExecutions.get(execution.id) !== undefined) {
throw new Error('We should not be trying to start the same custom task executions twice.');
}

// Clone the custom execution to keep the original untouched. This is important for multiple runs of the same task.
this.activeCustomExecutions.set(execution.id, customExecution);
const taskDefinition = converter.toTask(execution.task).definition;
const pty = await customExecution.callback(taskDefinition);
const pty = await customExecution(taskDefinition);
this.terminalExt.attachPtyToTerminal(terminalId, pty);
if (pty.onDidClose) {
const disposable = pty.onDidClose((e: number | void = undefined) => {
Expand Down Expand Up @@ -105,7 +99,6 @@ export class TasksExtImpl implements TasksExt {
}

this.executions.delete(id);
this.customExecutionComplete(id);

this.onDidTerminateTask.fire({
execution: taskExecution
Expand Down Expand Up @@ -159,7 +152,7 @@ export class TasksExtImpl implements TasksExt {
// in the provided custom execution map that is cleaned up after the
// task is executed.
if (CustomExecution.is(task.execution!)) {
this.addCustomExecution(taskDto, false);
taskDto.executionId = this.addCustomExecution(task.execution!.callback);
}
const executionDto = await this.proxy.$executeTask(taskDto);
if (executionDto) {
Expand All @@ -177,8 +170,9 @@ export class TasksExtImpl implements TasksExt {
return adapter.provideTasks(token).then(tasks => {
if (tasks) {
for (const task of tasks) {
if (task.type === 'customExecution' || task.taskType === 'customExecution') {
this.addCustomExecution(task, true);
if (task.taskType === 'customExecution') {
task.executionId = this.addCustomExecution(task.callback);
task.callback = undefined;
}
}
}
Expand All @@ -192,7 +186,13 @@ export class TasksExtImpl implements TasksExt {
$resolveTask(handle: number, task: TaskDto, token: theia.CancellationToken): Promise<TaskDto | undefined> {
const adapter = this.adaptersMap.get(handle);
if (adapter) {
return adapter.resolveTask(task, token);
return adapter.resolveTask(task, token).then(resolvedTask => {
if (resolvedTask && resolvedTask.taskType === 'customExecution') {
resolvedTask.executionId = this.addCustomExecution(resolvedTask.callback);
resolvedTask.callback = undefined;
}
return resolvedTask;
});
} else {
return Promise.reject(new Error('No adapter found to resolve task'));
}
Expand Down Expand Up @@ -244,36 +244,17 @@ export class TasksExtImpl implements TasksExt {
return result;
}

private addCustomExecution(taskDto: TaskDto, isProvided: boolean): void {
const taskId = taskDto.id;
if (!isProvided && !this.providedCustomExecutions.has(taskId)) {
this.notProvidedCustomExecutions.add(taskId);
private addCustomExecution(callback: ExecutionCallback): string {
let id = this.customExecutionIds.get(callback);
if (!id) {
id = this.nextCallbackId();
this.customExecutionIds.set(callback, id);
this.customExecutionFunctions.set(id, callback);
}
this.providedCustomExecutions.set(taskDto.id, new CustomExecution(taskDto.callback));
return id;
}

private customExecutionComplete(id: number): void {
const extensionCallback2: CustomExecution | undefined = this.activeCustomExecutions.get(id);
if (extensionCallback2) {
this.activeCustomExecutions.delete(id);
}

// Technically we don't really need to do this, however, if an extension
// is executing a task through "executeTask" over and over again
// with different properties in the task definition, then the map of executions
// could grow indefinitely, something we don't want.
if (this.notProvidedCustomExecutions.has(id) && (this.lastStartedTask !== id)) {
this.providedCustomExecutions.delete(id);
this.notProvidedCustomExecutions.delete(id);
}
const iterator = this.notProvidedCustomExecutions.values();
let iteratorResult = iterator.next();
while (!iteratorResult.done) {
if (!this.activeCustomExecutions.has(iteratorResult.value) && (this.lastStartedTask !== iteratorResult.value)) {
this.providedCustomExecutions.delete(iteratorResult.value);
this.notProvidedCustomExecutions.delete(iteratorResult.value);
}
iteratorResult = iterator.next();
}
private nextCallbackId(): string {
return this.callbackIdBase + this.callbackId++;
}
}
2 changes: 2 additions & 0 deletions packages/plugin-ext/src/plugin/type-converters.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ describe('Type converters:', () => {

const shellTaskDto: TaskDto = {
type: shellType,
taskType: shellType,
label,
source,
scope: 1,
Expand All @@ -203,6 +204,7 @@ describe('Type converters:', () => {

const shellTaskDtoWithCommandLine: TaskDto = {
type: shellType,
taskType: shellType,
label,
source,
scope: 2,
Expand Down
15 changes: 9 additions & 6 deletions packages/plugin-ext/src/plugin/type-converters.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -750,16 +750,16 @@ export function fromTask(task: theia.Task): TaskDto | undefined {
return taskDto;
}

if (taskDefinition.taskType === 'shell' || types.ShellExecution.is(execution)) {
return fromShellExecution(<theia.ShellExecution>execution, taskDto);
if (types.ShellExecution.is(execution)) {
return fromShellExecution(execution, taskDto);
}

if (taskDefinition.taskType === 'process' || types.ProcessExecution.is(execution)) {
return fromProcessExecution(<theia.ProcessExecution>execution, taskDto);
if (types.ProcessExecution.is(execution)) {
return fromProcessExecution(execution, taskDto);
}

if (taskDefinition.taskType === 'customExecution' || types.CustomExecution.is(execution)) {
return fromCustomExecution(<theia.CustomExecution>execution, taskDto);
if (types.CustomExecution.is(execution)) {
return fromCustomExecution(execution, taskDto);
}

return taskDto;
Expand Down Expand Up @@ -839,6 +839,7 @@ export function toTask(taskDto: TaskDto): theia.Task {
}

export function fromProcessExecution(execution: theia.ProcessExecution, taskDto: TaskDto): TaskDto {
taskDto.taskType = 'process';
taskDto.command = execution.process;
taskDto.args = execution.args;

Expand All @@ -850,6 +851,7 @@ export function fromProcessExecution(execution: theia.ProcessExecution, taskDto:
}

export function fromShellExecution(execution: theia.ShellExecution, taskDto: TaskDto): TaskDto {
taskDto.taskType = 'shell';
const options = execution.options;
if (options) {
taskDto.options = getShellExecutionOptions(options);
Expand All @@ -872,6 +874,7 @@ export function fromShellExecution(execution: theia.ShellExecution, taskDto: Tas
}

export function fromCustomExecution(execution: theia.CustomExecution, taskDto: TaskDto): TaskDto {
taskDto.taskType = 'customExecution';
const callback = execution.callback;
if (callback) {
taskDto.callback = callback;
Expand Down
74 changes: 3 additions & 71 deletions packages/plugin-ext/src/plugin/types-impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1484,14 +1484,6 @@ export enum ProgressLocation {
Notification = 15
}

function computeTaskExecutionId(values: string[]): string {
let id: string = '';
for (let i = 0; i < values.length; i++) {
id += values[i].replace(/,/g, ',,') + ',';
}
return id;
}

export class ProcessExecution {
private executionProcess: string;
private arguments: string[];
Expand Down Expand Up @@ -1547,21 +1539,7 @@ export class ProcessExecution {
this.executionOptions = value;
}

public computeId(): string {
const props: string[] = [];
props.push('process');
if (this.executionProcess !== undefined) {
props.push(this.executionProcess);
}
if (this.arguments && this.arguments.length > 0) {
for (const arg of this.arguments) {
props.push(arg);
}
}
return computeTaskExecutionId(props);
}

public static is(value: theia.ShellExecution | theia.ProcessExecution | theia.CustomExecution): boolean {
public static is(value: theia.ShellExecution | theia.ProcessExecution | theia.CustomExecution): value is ProcessExecution {
const candidate = value as ProcessExecution;
return candidate && !!candidate.process;
}
Expand Down Expand Up @@ -1652,24 +1630,7 @@ export class ShellExecution {
this.shellOptions = value;
}

public computeId(): string {
const props: string[] = [];
props.push('shell');
if (this.shellCommandLine !== undefined) {
props.push(this.shellCommandLine);
}
if (this.shellCommand !== undefined) {
props.push(typeof this.shellCommand === 'string' ? this.shellCommand : this.shellCommand.value);
}
if (this.arguments && this.arguments.length > 0) {
for (const arg of this.arguments) {
props.push(typeof arg === 'string' ? arg : arg.value);
}
}
return computeTaskExecutionId(props);
}

public static is(value: theia.ShellExecution | theia.ProcessExecution | theia.CustomExecution): boolean {
public static is(value: theia.ShellExecution | theia.ProcessExecution | theia.CustomExecution): value is ShellExecution {
const candidate = value as ShellExecution;
return candidate && (!!candidate.commandLine || !!candidate.command);
}
Expand All @@ -1680,9 +1641,6 @@ export class CustomExecution {
constructor(callback: (resolvedDefintion: theia.TaskDefinition) => Thenable<theia.Pseudoterminal>) {
this._callback = callback;
}
public computeId(): string {
return 'customExecution' + UUID.uuid4();
}

public set callback(value: (resolvedDefintion: theia.TaskDefinition) => Thenable<theia.Pseudoterminal>) {
this._callback = value;
Expand All @@ -1692,7 +1650,7 @@ export class CustomExecution {
return this._callback;
}

public static is(value: theia.ShellExecution | theia.ProcessExecution | theia.CustomExecution): boolean {
public static is(value: theia.ShellExecution | theia.ProcessExecution | theia.CustomExecution): value is CustomExecution {
const candidate = value as CustomExecution;
return candidate && (!!candidate._callback);
}
Expand Down Expand Up @@ -1860,7 +1818,6 @@ export class Task {
value = undefined;
}
this.taskExecution = value;
this.updateDefinitionBasedOnExecution();
}

get problemMatchers(): string[] {
Expand Down Expand Up @@ -1925,31 +1882,6 @@ export class Task {
}
this.taskPresentationOptions = value;
}

private updateDefinitionBasedOnExecution(): void {
if (this.taskExecution instanceof ProcessExecution) {
Object.assign(this.taskDefinition, {
id: this.taskExecution.computeId(),
taskType: 'process'
});
} else if (this.taskExecution instanceof ShellExecution) {
Object.assign(this.taskDefinition, {
id: this.taskExecution.computeId(),
taskType: 'shell'
});
} else if (this.taskExecution instanceof CustomExecution) {
Object.assign(this.taskDefinition, {
id: this.taskDefinition.id ? this.taskDefinition.id : this.taskExecution.computeId(),
taskType: 'customExecution'
});
} else {
Object.assign(this.taskDefinition, {
type: '$empty',
id: UUID.uuid4(),
taskType: this.taskDefinition!.type
});
}
}
}

export class Task2 extends Task {
Expand Down
4 changes: 2 additions & 2 deletions packages/task/src/browser/process/process-task-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class ProcessTaskContribution implements TaskContribution {
protected readonly processTaskResolver: ProcessTaskResolver;

registerResolvers(resolvers: TaskResolverRegistry): void {
resolvers.register('process', this.processTaskResolver);
resolvers.register('shell', this.processTaskResolver);
resolvers.registerExecutionResolver('process', this.processTaskResolver);
resolvers.registerExecutionResolver('shell', this.processTaskResolver);
}
}
11 changes: 4 additions & 7 deletions packages/task/src/browser/task-configurations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export class TaskConfigurations implements Disposable {
return undefined;
}

const customizationByType = this.getTaskCustomizations(taskConfig.taskType || taskConfig.type, taskConfig._scope) || [];
const customizationByType = this.getTaskCustomizations(taskConfig.type, taskConfig._scope) || [];
const hasCustomization = customizationByType.length > 0;
if (hasCustomization) {
const taskDefinition = this.taskDefinitionRegistry.getDefinition(taskConfig);
Expand Down Expand Up @@ -329,7 +329,7 @@ export class TaskConfigurations implements Disposable {
console.error('Detected / Contributed tasks should have a task definition.');
return;
}
const customization: TaskCustomization = { type: task.taskType || task.type };
const customization: TaskCustomization = { type: task.type };
definition.properties.all.forEach(p => {
if (task[p] !== undefined) {
customization[p] = task[p];
Expand Down Expand Up @@ -457,7 +457,7 @@ export class TaskConfigurations implements Disposable {
const jsonTasks = this.taskConfigurationManager.getTasks(scope);
if (jsonTasks) {
const ind = jsonTasks.findIndex((t: TaskCustomization | TaskConfiguration) => {
if (t.type !== (task.taskType || task.type)) {
if (t.type !== (task.type)) {
return false;
}
const def = this.taskDefinitionRegistry.getDefinition(t);
Expand Down Expand Up @@ -503,9 +503,6 @@ export class TaskConfigurations implements Disposable {
}

private getTaskDefinition(task: TaskCustomization): TaskDefinition | undefined {
return this.taskDefinitionRegistry.getDefinition({
...task,
type: typeof task.taskType === 'string' ? task.taskType : task.type
});
return this.taskDefinitionRegistry.getDefinition(task);
}
}
Loading

0 comments on commit 6439a6d

Please sign in to comment.