Skip to content

Commit

Permalink
Ensure we keep task type and properly support custom execution (#12770)
Browse files Browse the repository at this point in the history
- Make TaskProviderAdapter more resilient against undefined
- Ensure task type is not lost during conversion (used for custom exec)
- Ensure task type is considered during comparison

Fixes #12721
  • Loading branch information
martin-fleck-at authored Aug 10, 2023
1 parent f00450f commit f75c9a2
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 31 deletions.
4 changes: 2 additions & 2 deletions packages/plugin-ext/src/common/plugin-api-rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2175,8 +2175,8 @@ export const MAIN_RPC_CONTEXT = {

export interface TasksExt {
$initLoadedTasks(executions: TaskExecutionDto[]): Promise<void>;
$provideTasks(handle: number): Promise<TaskDto[] | undefined>;
$resolveTask(handle: number, task: TaskDto, token?: CancellationToken): Promise<TaskDto | undefined>;
$provideTasks(handle: number): Promise<TaskDto[]>;
$resolveTask(handle: number, task: TaskDto, token?: CancellationToken): Promise<TaskDto>;
$onDidStartTask(execution: TaskExecutionDto, terminalId: number): void;
$onDidEndTask(id: number): void;
$onDidStartTaskProcess(processId: number | undefined, execution: TaskExecutionDto): void;
Expand Down
8 changes: 4 additions & 4 deletions packages/plugin-ext/src/main/browser/tasks-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ export class TasksMainImpl implements TasksMain, Disposable {
protected createTaskProvider(handle: number): TaskProvider {
return {
provideTasks: () =>
this.proxy.$provideTasks(handle).then(v =>
v!.map(taskDto =>
this.proxy.$provideTasks(handle).then(tasks =>
tasks.map(taskDto =>
this.toTaskConfiguration(taskDto)
)
)
Expand All @@ -195,8 +195,8 @@ export class TasksMainImpl implements TasksMain, Disposable {
protected createTaskResolver(handle: number): TaskResolver {
return {
resolveTask: taskConfig =>
this.proxy.$resolveTask(handle, this.fromTaskConfiguration(taskConfig)).then(v =>
this.toTaskConfiguration(v!)
this.proxy.$resolveTask(handle, this.fromTaskConfiguration(taskConfig)).then(task =>
this.toTaskConfiguration(task)
)
};
}
Expand Down
21 changes: 9 additions & 12 deletions packages/plugin-ext/src/plugin/tasks/task-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
// *****************************************************************************

import * as theia from '@theia/plugin';
import * as Converter from '../type-converters';
import { TaskDto } from '../../common';
import * as Converter from '../type-converters';

export class TaskProviderAdapter {

constructor(private readonly provider: theia.TaskProvider) { }

provideTasks(token: theia.CancellationToken): Promise<TaskDto[] | undefined> {
provideTasks(token: theia.CancellationToken): Promise<TaskDto[]> {
return Promise.resolve(this.provider.provideTasks(token)).then(tasks => {
if (!Array.isArray(tasks)) {
return undefined;
return [];
}
const result: TaskDto[] = [];
for (const task of tasks) {
Expand All @@ -40,21 +40,18 @@ export class TaskProviderAdapter {
});
}

resolveTask(task: TaskDto, token: theia.CancellationToken): Promise<TaskDto | undefined> {
async resolveTask(task: TaskDto, token: theia.CancellationToken): Promise<TaskDto> {
if (typeof this.provider.resolveTask !== 'function') {
return Promise.resolve(undefined);
return task;
}

const item = Converter.toTask(task);
if (!item) {
return Promise.resolve(undefined);
return task;
}

return Promise.resolve(this.provider.resolveTask(item, token)).then(value => {
if (value) {
return Converter.fromTask(value);
}
return undefined;
});
const resolved = await this.provider.resolveTask(item, token);
const converted = resolved ? Converter.fromTask(resolved) : Converter.fromTask(item);
return converted ?? task;
}
}
31 changes: 18 additions & 13 deletions packages/plugin-ext/src/plugin/tasks/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,37 +158,42 @@ export class TasksExtImpl implements TasksExt {
throw new Error('Task was not successfully transformed into a task config');
}

$provideTasks(handle: number): Promise<TaskDto[] | undefined> {
async $provideTasks(handle: number): Promise<TaskDto[]> {
const adapter = this.adaptersMap.get(handle);
if (adapter) {
return adapter.provideTasks(CancellationToken.None).then(tasks => {
if (tasks) {
for (const task of tasks) {
if (task.taskType === 'customExecution') {
task.executionId = this.addCustomExecution(task.callback);
task.callback = undefined;
}
for (const task of tasks) {
if (task.taskType === 'customExecution') {
this.applyCustomExecution(task);
}
}
return tasks;
});
} else {
return Promise.reject(new Error('No adapter found to provide tasks'));
throw new Error('No adapter found to provide tasks');
}
}

$resolveTask(handle: number, task: TaskDto, token: theia.CancellationToken): Promise<TaskDto | undefined> {
async $resolveTask(handle: number, task: TaskDto, token: theia.CancellationToken): Promise<TaskDto> {
const adapter = this.adaptersMap.get(handle);
if (adapter) {
return adapter.resolveTask(task, token).then(resolvedTask => {
if (resolvedTask && resolvedTask.taskType === 'customExecution') {
resolvedTask.executionId = this.addCustomExecution(resolvedTask.callback);
resolvedTask.callback = undefined;
// ensure we do not lose task type and execution id during resolution as we need it for custom execution
resolvedTask.taskType = resolvedTask.taskType ?? task.taskType;
if (resolvedTask.taskType === 'customExecution') {
this.applyCustomExecution(resolvedTask);
}
return resolvedTask;
});
} else {
return Promise.reject(new Error('No adapter found to resolve task'));
throw new Error('No adapter found to resolve task');
}
}

private applyCustomExecution(task: TaskDto): void {
if (task.callback) {
task.executionId = this.addCustomExecution(task.callback);
task.callback = undefined;
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/task/src/browser/task-definition-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ export class TaskDefinitionRegistry {
if (oneType !== otherType) {
return false;
}
if (one['taskType'] !== other['taskType']) {
return false;
}
const def = this.getDefinition(one);
if (def) {
// scope is either a string or an enum value. Anyway...they must exactly match
Expand Down

0 comments on commit f75c9a2

Please sign in to comment.