From 855576121b2710f40dd02a87c33e57e5e0fb6da2 Mon Sep 17 00:00:00 2001 From: Liang Huang Date: Tue, 20 Aug 2019 22:29:45 -0400 Subject: [PATCH] use task definition as a reference to compare tasks - Although task users have much flexibility to define tasks through contributing to the Task definitions, TaskConfiguration.equals() and ContributedTaskConfiguration.equals() compare certain properties of task configs, and thus are not reliable enough. With changes in this pull request, Theia uses task definitions as a reference to decide which properties in task configs should be considered in comparison. - fixed #5878 Signed-off-by: Liang Huang --- packages/cpp/src/browser/cpp-task-provider.ts | 1 + .../src/hosted/node/scanners/scanner-theia.ts | 5 +++-- .../browser/process/process-task-resolver.ts | 8 ++++++-- packages/task/src/browser/quick-open-task.ts | 18 +++++++++++------- .../task/src/browser/task-configurations.ts | 15 +++++++++++---- .../browser/task-definition-registry.spec.ts | 2 ++ .../src/browser/task-definition-registry.ts | 13 +++++++++++++ packages/task/src/browser/task-service.ts | 5 ++--- packages/task/src/common/task-protocol.ts | 12 +----------- 9 files changed, 50 insertions(+), 29 deletions(-) diff --git a/packages/cpp/src/browser/cpp-task-provider.ts b/packages/cpp/src/browser/cpp-task-provider.ts index 82751bfc38e74..2949c9b55e65f 100644 --- a/packages/cpp/src/browser/cpp-task-provider.ts +++ b/packages/cpp/src/browser/cpp-task-provider.ts @@ -145,6 +145,7 @@ export class CppTaskProvider implements TaskContribution, TaskProvider, TaskReso private registerTaskDefinition(): void { this.taskDefinitionRegistry.register({ taskType: CPP_BUILD_TASK_TYPE_KEY, + source: 'cpp', properties: { required: ['label'], all: ['label'] diff --git a/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts b/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts index 66764dc656bad..905eec79a1471 100644 --- a/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts +++ b/packages/plugin-ext/src/hosted/node/scanners/scanner-theia.ts @@ -189,7 +189,7 @@ export class TheiaPluginScanner implements PluginScanner { } if (rawPlugin.contributes!.taskDefinitions) { - contributions.taskDefinitions = rawPlugin.contributes!.taskDefinitions!.map(definitionContribution => this.readTaskDefinition(definitionContribution)); + contributions.taskDefinitions = rawPlugin.contributes!.taskDefinitions!.map(definitionContribution => this.readTaskDefinition(rawPlugin.name, definitionContribution)); } if (rawPlugin.contributes!.problemMatchers) { @@ -372,9 +372,10 @@ export class TheiaPluginScanner implements PluginScanner { return result; } - private readTaskDefinition(definitionContribution: PluginTaskDefinitionContribution): TaskDefinition { + private readTaskDefinition(pluginName: string, definitionContribution: PluginTaskDefinitionContribution): TaskDefinition { return { taskType: definitionContribution.type, + source: pluginName, properties: { required: definitionContribution.required, all: Object.keys(definitionContribution.properties) diff --git a/packages/task/src/browser/process/process-task-resolver.ts b/packages/task/src/browser/process/process-task-resolver.ts index 511406951c3c9..b226aea7aec13 100644 --- a/packages/task/src/browser/process/process-task-resolver.ts +++ b/packages/task/src/browser/process/process-task-resolver.ts @@ -19,6 +19,7 @@ import { VariableResolverService } from '@theia/variable-resolver/lib/browser'; import { TaskResolver } from '../task-contribution'; import { TaskConfiguration } from '../../common/task-protocol'; import { ProcessTaskConfiguration } from '../../common/process/task-protocol'; +import { TaskDefinitionRegistry } from '../task-definition-registry'; import URI from '@theia/core/lib/common/uri'; @injectable() @@ -27,6 +28,9 @@ export class ProcessTaskResolver implements TaskResolver { @inject(VariableResolverService) protected readonly variableResolverService: VariableResolverService; + @inject(TaskDefinitionRegistry) + protected readonly taskDefinitionRegistry: TaskDefinitionRegistry; + /** * Perform some adjustments to the task launch configuration, before sending * it to the backend to be executed. We can make sure that parameters that @@ -37,8 +41,8 @@ export class ProcessTaskResolver implements TaskResolver { if (taskConfig.type !== 'process' && taskConfig.type !== 'shell') { throw new Error('Unsupported task configuration type.'); } - - const variableResolverOptions = { context: new URI(taskConfig._source).withScheme('file') }; + const context = new URI(this.taskDefinitionRegistry.getDefinition(taskConfig) ? taskConfig.scope : taskConfig._source).withScheme('file'); + const variableResolverOptions = { context }; const processTaskConfig = taskConfig as ProcessTaskConfiguration; const result: ProcessTaskConfiguration = { ...processTaskConfig, diff --git a/packages/task/src/browser/quick-open-task.ts b/packages/task/src/browser/quick-open-task.ts index 96ebcf4ccfa9b..3f62fb8638551 100644 --- a/packages/task/src/browser/quick-open-task.ts +++ b/packages/task/src/browser/quick-open-task.ts @@ -16,7 +16,7 @@ import { inject, injectable } from 'inversify'; import { TaskService } from './task-service'; -import { ContributedTaskConfiguration, TaskInfo, TaskConfiguration } from '../common/task-protocol'; +import { TaskInfo, TaskConfiguration } from '../common/task-protocol'; import { TaskDefinitionRegistry } from './task-definition-registry'; import URI from '@theia/core/lib/common/uri'; import { TaskActionProvider } from './task-action-provider'; @@ -207,7 +207,7 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler { const filteredRecentTasks: TaskConfiguration[] = []; recentTasks.forEach(recent => { - const exist = [...configuredTasks, ...providedTasks].some(t => TaskConfiguration.equals(recent, t)); + const exist = [...configuredTasks, ...providedTasks].some(t => this.taskDefinitionRegistry.compareTasks(recent, t)); if (exist) { filteredRecentTasks.push(recent); } @@ -215,7 +215,7 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler { const filteredProvidedTasks: TaskConfiguration[] = []; providedTasks.forEach(provided => { - const exist = [...filteredRecentTasks, ...configuredTasks].some(t => ContributedTaskConfiguration.equals(provided, t)); + const exist = [...filteredRecentTasks, ...configuredTasks].some(t => this.taskDefinitionRegistry.compareTasks(provided, t)); if (!exist) { filteredProvidedTasks.push(provided); } @@ -223,7 +223,7 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler { const filteredConfiguredTasks: TaskConfiguration[] = []; configuredTasks.forEach(configured => { - const exist = filteredRecentTasks.some(t => TaskConfiguration.equals(configured, t)); + const exist = filteredRecentTasks.some(t => this.taskDefinitionRegistry.compareTasks(configured, t)); if (!exist) { filteredConfiguredTasks.push(configured); } @@ -254,7 +254,7 @@ export class TaskRunQuickOpenItem extends QuickOpenGroupItem { getLabel(): string { if (this.taskDefinitionRegistry && !!this.taskDefinitionRegistry.getDefinition(this.task)) { - return `${this.task._source}: ${this.task.label}`; + return `${this.task.source}: ${this.task.label}`; } return `${this.task.type}: ${this.task.label}`; } @@ -283,7 +283,11 @@ export class TaskRunQuickOpenItem extends QuickOpenGroupItem { return false; } - this.taskService.run(this.task._source, this.task.label); + if (this.taskDefinitionRegistry && !!this.taskDefinitionRegistry.getDefinition(this.task)) { + this.taskService.run(this.task.source, this.task.label); + } else { + this.taskService.run(this.task._source, this.task.label); + } return true; } } @@ -330,7 +334,7 @@ export class TaskConfigureQuickOpenItem extends QuickOpenGroupItem { getLabel(): string { if (this.taskDefinitionRegistry && !!this.taskDefinitionRegistry.getDefinition(this.task)) { - return `${this.task._source}: ${this.task.label}`; + return `${this.task.source}: ${this.task.label}`; } return `${this.task.type}: ${this.task.label}`; } diff --git a/packages/task/src/browser/task-configurations.ts b/packages/task/src/browser/task-configurations.ts index 485f17dce2fb1..43a68daef175a 100644 --- a/packages/task/src/browser/task-configurations.ts +++ b/packages/task/src/browser/task-configurations.ts @@ -330,9 +330,16 @@ export class TaskConfigurations implements Disposable { this.rawTaskConfigurations.delete(rootFolderUri); } if (rawTasks && rawTasks['tasks']) { - const tasks = rawTasks['tasks'].map((t: TaskCustomization | TaskConfiguration) => - Object.assign(t, { _source: t.source || this.getSourceFolderFromConfigUri(uri) }) - ); + const tasks = rawTasks['tasks'].map((t: TaskCustomization | TaskConfiguration) => { + if (this.isDetectedTask(t)) { + const def = this.getTaskDefinition(t); + return Object.assign(t, { + _source: def!.source, + _scope: this.getSourceFolderFromConfigUri(uri) + }); + } + return Object.assign(t, { _source: this.getSourceFolderFromConfigUri(uri) }); + }); this.rawTaskConfigurations.set(rootFolderUri, tasks); return tasks; } else { @@ -366,7 +373,7 @@ export class TaskConfigurations implements Disposable { const configFileUri = this.getConfigFileUri(sourceFolderUri); const configuredAndCustomizedTasks = await this.getTasks(); - if (!configuredAndCustomizedTasks.some(t => ContributedTaskConfiguration.equals(t, task))) { + if (!configuredAndCustomizedTasks.some(t => this.taskDefinitionRegistry.compareTasks(t, task))) { await this.saveTask(configFileUri, task); } diff --git a/packages/task/src/browser/task-definition-registry.spec.ts b/packages/task/src/browser/task-definition-registry.spec.ts index 4acc918c5b23c..de00d2021e3eb 100644 --- a/packages/task/src/browser/task-definition-registry.spec.ts +++ b/packages/task/src/browser/task-definition-registry.spec.ts @@ -22,6 +22,7 @@ describe('TaskDefinitionRegistry', () => { let registry: TaskDefinitionRegistry; const definitonContributionA = { taskType: 'extA', + source: 'extA', required: ['extensionType'], properties: { required: ['extensionType'], @@ -30,6 +31,7 @@ describe('TaskDefinitionRegistry', () => { }; const definitonContributionB = { taskType: 'extA', + source: 'extA', properties: { required: ['extensionType', 'taskLabel', 'taskDetailedLabel'], all: ['extensionType', 'taskLabel', 'taskDetailedLabel'] diff --git a/packages/task/src/browser/task-definition-registry.ts b/packages/task/src/browser/task-definition-registry.ts index 40cb1233f5cdf..90afbb48aa1dc 100644 --- a/packages/task/src/browser/task-definition-registry.ts +++ b/packages/task/src/browser/task-definition-registry.ts @@ -78,4 +78,17 @@ export class TaskDefinitionRegistry { this.definitions.set(taskType, [...this.getDefinitions(taskType), definition]); this.onDidRegisterTaskDefinitionEmitter.fire(undefined); } + + compareTasks(one: TaskConfiguration, other: TaskConfiguration): boolean { + const oneType = one.taskType || one.type; + const otherType = other.taskType || other.type; + if (oneType !== otherType) { + return false; + } + const def = this.getDefinition(one); + if (def) { + return def.properties.all.every(p => p === 'type' || one[p] === other[p]) && one._scope === other._scope; + } + return one.label === other.label && one._source === other._source; + } } diff --git a/packages/task/src/browser/task-service.ts b/packages/task/src/browser/task-service.ts index 21e79ba1c64a9..fffc40da75c5b 100644 --- a/packages/task/src/browser/task-service.ts +++ b/packages/task/src/browser/task-service.ts @@ -27,7 +27,6 @@ import { ProblemManager } from '@theia/markers/lib/browser/problem/problem-manag import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service'; import { VariableResolverService } from '@theia/variable-resolver/lib/browser'; import { - ContributedTaskConfiguration, ProblemMatcher, ProblemMatchData, TaskCustomization, @@ -206,7 +205,7 @@ export class TaskService implements TaskConfigurationClient { const configuredTasks = await this.getConfiguredTasks(); const providedTasks = await this.getProvidedTasks(); const notCustomizedProvidedTasks = providedTasks.filter(provided => - !configuredTasks.some(configured => ContributedTaskConfiguration.equals(configured, provided)) + !configuredTasks.some(configured => this.taskDefinitionRegistry.compareTasks(configured, provided)) ); return [...configuredTasks, ...notCustomizedProvidedTasks]; } @@ -225,7 +224,7 @@ export class TaskService implements TaskConfigurationClient { if (Array.isArray(tasks)) { tasks.forEach(task => this.addRecentTasks(task)); } else { - const ind = this.cachedRecentTasks.findIndex(recent => TaskConfiguration.equals(recent, tasks)); + const ind = this.cachedRecentTasks.findIndex(recent => this.taskDefinitionRegistry.compareTasks(recent, tasks)); if (ind >= 0) { this.cachedRecentTasks.splice(ind, 1); } diff --git a/packages/task/src/common/task-protocol.ts b/packages/task/src/common/task-protocol.ts index 294bee2332fdd..6b066496ff607 100644 --- a/packages/task/src/common/task-protocol.ts +++ b/packages/task/src/common/task-protocol.ts @@ -33,12 +33,6 @@ export interface TaskConfiguration extends TaskCustomization { /** A label that uniquely identifies a task configuration per source */ readonly label: string; } -export namespace TaskConfiguration { - export function equals(one: TaskConfiguration, other: TaskConfiguration): boolean { - return (one.taskType || one.type) === (other.taskType || other.type) && - one.label === other.label && one._source === other._source; - } -} export interface ContributedTaskConfiguration extends TaskConfiguration { /** @@ -53,11 +47,6 @@ export interface ContributedTaskConfiguration extends TaskConfiguration { */ readonly _scope: string | undefined; } -export namespace ContributedTaskConfiguration { - export function equals(one: TaskConfiguration, other: TaskConfiguration): boolean { - return TaskConfiguration.equals(one, other) && one._scope === other._scope; - } -} /** Runtime information about Task. */ export interface TaskInfo { @@ -139,6 +128,7 @@ export interface TaskClient { export interface TaskDefinition { taskType: string; + source: string; properties: { required: string[]; all: string[];