align "configure task" and "task quick open" with vs code

- edit the right task.json when clicking "configure task" in multi-root
workspace (fixed #4919)

- in the current Theia, when users configure a detected task, the entire
task config is written into tasks.json, which introduces redundancy.
With this change, only properties that define the detected task, plus `problemMatcher`, are
written into tasks.json. (fixed #5679)

- allow users to override any task properties other than `type`, and those that are used to define the in its task definition.

- `TaskConfigurations.taskCustomizations` is a flat array, and the user can only customize one type of detected task in one way.
With this change Theia supports having different ways of task customization in different root folders.

- The detected tasks, once customized, should be displayed as configured tasks in the quick open. (fixed #5747)

- The same task shouldn’t have more than one customization. Otherwise it would cause ambiguities and duplication in tasks.json (fixed #5719)

Signed-off-by: Liang Huang <liang.huang@ericsson.com>

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Change Log
 
+## v0.10.0
+
+- [task] added support for VS Code task contribution points: `taskDefinitions`, `problemMatchers`, and `problemPatterns`
+- [task] added multi-root support to "configure task" and customizing tasks in `tasks.json`
+- [task] changed the way that "configure task" copies the entire task config, to only writting properties that define the detected task plus `problemMatcher`, into `tasks.json`
+- [task] fixed the problem where a detected task can be customized more than once
+- [task] displayed the customized tasks as "configured tasks" in the task quick open
+- [task] allowed users to override any task properties other than the ones used in the task definition
+
+Breaking changes:
+
+- [task] `TaskService.getConfiguredTasks()` returns `Promise<TaskConfiguration[]>` instead of `TaskConfiguration[]`.
+
 ## v0.9.0
 - [core] added `theia-widget-noInfo` css class to be used by widgets when displaying no information messages [#5717](https://github.com/theia-ide/theia/pull/5717)
 - [core] added additional options to the tree search input [#5566](https://github.com/theia-ide/theia/pull/5566)

packages/task/src/browser/provided-task-configurations.spec.ts

@@ -17,6 +17,7 @@
 import { assert } from 'chai';
 import { Container } from 'inversify';
 import { ProvidedTaskConfigurations } from './provided-task-configurations';
+import { TaskDefinitionRegistry } from './task-definition-registry';
 import { TaskProviderRegistry } from './task-contribution';
 import { TaskConfiguration } from '../common';
 
@@ -26,6 +27,7 @@ describe('provided-task-configurations', () => {
         container = new Container();
         container.bind(ProvidedTaskConfigurations).toSelf().inSingletonScope();
         container.bind(TaskProviderRegistry).toSelf().inSingletonScope();
+        container.bind(TaskDefinitionRegistry).toSelf().inSingletonScope();
     });
 
     it('provided-task-search', async () => {

packages/task/src/browser/provided-task-configurations.ts

@@ -15,8 +15,10 @@
  ********************************************************************************/
 
 import { inject, injectable } from 'inversify';
-import { TaskConfiguration } from '../common/task-protocol';
 import { TaskProviderRegistry } from './task-contribution';
+import { TaskDefinitionRegistry } from './task-definition-registry';
+import { TaskConfiguration, TaskCustomization } from '../common';
+import URI from '@theia/core/lib/common/uri';
 
 @injectable()
 export class ProvidedTaskConfigurations {
@@ -30,13 +32,14 @@ export class ProvidedTaskConfigurations {
     @inject(TaskProviderRegistry)
     protected readonly taskProviderRegistry: TaskProviderRegistry;
 
+    @inject(TaskDefinitionRegistry)
+    protected readonly taskDefinitionRegistry: TaskDefinitionRegistry;
+
     /** returns a list of provided tasks */
     async getTasks(): Promise<TaskConfiguration[]> {
-        const providedTasks: TaskConfiguration[] = [];
         const providers = this.taskProviderRegistry.getProviders();
-        for (const provider of providers) {
-            providedTasks.push(...await provider.provideTasks());
-        }
+        const providedTasks: TaskConfiguration[] = (await Promise.all(providers.map(p => p.provideTasks())))
+            .reduce((acc, taskArray) => acc.concat(taskArray), []);
         this.cacheTasks(providedTasks);
         return providedTasks;
     }
@@ -52,6 +55,51 @@ export class ProvidedTaskConfigurations {
         }
     }
 
+    /**
+     * Finds the detected task for the given task customization.
+     * The detected task is considered as a "match" to the task customization if it has all the `required` properties.
+     * In case that more than one customization is found, return the one that has the biggest number of matched properties.
+     *
+     * @param customization the task customization
+     * @return the detected task for the given task customization. If the task customization is not found, `undefined` is returned.
+     */
+    async getTaskToCustomize(customization: TaskCustomization, rootFolderPath: string): Promise<TaskConfiguration | undefined> {
+        const definition = this.taskDefinitionRegistry.getDefinition(customization);
+        if (!definition) {
+            return undefined;
+        }
+
+        const matchedTasks: TaskConfiguration[] = [];
+        let highest = -1;
+        const tasks = await this.getTasks();
+        for (const task of tasks) { // find detected tasks that match the `definition`
+            let score = 0;
+            if (!definition.properties.required.every(requiredProp => customization[requiredProp] !== undefined)) {
+                continue;
+            }
+            score += definition.properties.required.length; // number of required properties
+            const requiredProps = new Set(definition.properties.required);
+            // number of optional properties
+            score += definition.properties.all.filter(p => !requiredProps.has(p) && customization[p] !== undefined).length;
+            if (score >= highest) {
+                if (score > highest) {
+                    highest = score;
+                    matchedTasks.length = 0;
+                }
+                matchedTasks.push(task);
+            }
+        }
+
+        // find the task that matches the `customization`.
+        // The scenario where more than one match is found should not happen unless users manually enter multiple customizations for one type of task
+        // If this does happen, return the first match
+        const rootFolderUri = new URI(rootFolderPath).toString();
+        const matchedTask = matchedTasks.filter(t =>
+            rootFolderUri === t._scope && definition.properties.all.every(p => t[p] === customization[p])
+        )[0];
+        return matchedTask;
+    }
+
     protected getCachedTask(source: string, taskLabel: string): TaskConfiguration | undefined {
         const labelConfigMap = this.tasksMap.get(source);
         if (labelConfigMap) {

packages/task/src/browser/quick-open-task.ts

@@ -20,7 +20,7 @@ import {
     QuickOpenGroupItem, QuickOpenMode, QuickOpenHandler, QuickOpenOptions, QuickOpenActionProvider, QuickOpenGroupItemOptions
 } from '@theia/core/lib/browser/quick-open/';
 import { TaskService } from './task-service';
-import { TaskInfo, TaskConfiguration } from '../common/task-protocol';
+import { ContributedTaskConfiguration, TaskInfo, TaskConfiguration } from '../common/task-protocol';
 import { TaskConfigurations } from './task-configurations';
 import { TaskDefinitionRegistry } from './task-definition-registry';
 import URI from '@theia/core/lib/common/uri';
@@ -66,7 +66,7 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler {
     /** Initialize this quick open model with the tasks. */
     async init(): Promise<void> {
         const recentTasks = this.taskService.recentTasks;
-        const configuredTasks = this.taskService.getConfiguredTasks();
+        const configuredTasks = await this.taskService.getConfiguredTasks();
         const providedTasks = await this.taskService.getProvidedTasks();
 
         const { filteredRecentTasks, filteredConfiguredTasks, filteredProvidedTasks } = this.getFilteredTasks(recentTasks, configuredTasks, providedTasks);
@@ -213,7 +213,7 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler {
 
         const filteredProvidedTasks: TaskConfiguration[] = [];
         providedTasks.forEach(provided => {
-            const exist = [...filteredRecentTasks, ...configuredTasks].some(t => TaskConfiguration.equals(provided, t));
+            const exist = [...filteredRecentTasks, ...configuredTasks].some(t => ContributedTaskConfiguration.equals(provided, t));
             if (!exist) {
                 filteredProvidedTasks.push(provided);
             }

packages/task/src/browser/task-configurations.ts

@@ -15,8 +15,9 @@
  ********************************************************************************/
 
 import { inject, injectable } from 'inversify';
-import { TaskConfiguration, TaskCustomization, ContributedTaskConfiguration } from '../common';
+import { ContributedTaskConfiguration, TaskConfiguration, TaskCustomization, TaskDefinition } from '../common';
 import { TaskDefinitionRegistry } from './task-definition-registry';
+import { ProvidedTaskConfigurations } from './provided-task-configurations';
 import { Disposable, DisposableCollection, ResourceProvider } from '@theia/core/lib/common';
 import URI from '@theia/core/lib/common/uri';
 import { FileSystemWatcher, FileChangeEvent } from '@theia/filesystem/lib/browser/filesystem-watcher';
@@ -44,11 +45,14 @@ export class TaskConfigurations implements Disposable {
 
     protected readonly toDispose = new DisposableCollection();
     /**
-     * Map of source (path of root folder that the task config comes from) and task config map.
+     * Map of source (path of root folder that the task configs come from) and task config map.
      * For the inner map (i.e., task config map), the key is task label and value TaskConfiguration
      */
     protected tasksMap = new Map<string, Map<string, TaskConfiguration>>();
-    protected taskCustomizations: TaskCustomization[] = [];
+    /**
+     * Map of source (path of root folder that the task configs come from) and task customizations map.
+     */
+    protected taskCustomizationMap = new Map<string, TaskCustomization[]>();
 
     protected watchedConfigFileUris: string[] = [];
     protected watchersMap = new Map<string, Disposable>(); // map of watchers for task config files, where the key is folder uri
@@ -72,6 +76,9 @@ export class TaskConfigurations implements Disposable {
     @inject(TaskDefinitionRegistry)
     protected readonly taskDefinitionRegistry: TaskDefinitionRegistry;
 
+    @inject(ProvidedTaskConfigurations)
+    protected readonly providedTaskConfigurations: ProvidedTaskConfigurations;
+
     constructor(
         @inject(FileSystemWatcher) protected readonly watcherServer: FileSystemWatcher,
         @inject(FileSystem) protected readonly fileSystem: FileSystem
@@ -160,14 +167,28 @@ export class TaskConfigurations implements Disposable {
         return Array.from(this.tasksMap.values()).reduce((acc, labelConfigMap) => acc.concat(Array.from(labelConfigMap.keys())), [] as string[]);
     }
 
-    /** returns the list of known tasks */
-    getTasks(): TaskConfiguration[] {
-        return Array.from(this.tasksMap.values()).reduce((acc, labelConfigMap) => acc.concat(Array.from(labelConfigMap.values())), [] as TaskConfiguration[]);
+    /**
+     * returns the list of known tasks, which includes:
+     * - all the configured tasks in `tasks.json`, and
+     * - the customized detected tasks
+     */
+    async getTasks(): Promise<TaskConfiguration[]> {
+        const configuredTasks = Array.from(this.tasksMap.values()).reduce((acc, labelConfigMap) => acc.concat(Array.from(labelConfigMap.values())), [] as TaskConfiguration[]);
+        const detectedTasksAsConfigured: TaskConfiguration[] = [];
+        for (const [rootFolder, customizations] of Array.from(this.taskCustomizationMap.entries())) {
+            for (const cus of customizations) {
+                const detected = await this.providedTaskConfigurations.getTaskToCustomize(cus, rootFolder);
+                if (detected) {
+                    detectedTasksAsConfigured.push(detected);
+                }
+            }
+        }
+        return [...configuredTasks, ...detectedTasksAsConfigured];
     }
 
     /** returns the task configuration for a given label or undefined if none */
-    getTask(source: string, taskLabel: string): TaskConfiguration | undefined {
-        const labelConfigMap = this.tasksMap.get(source);
+    getTask(rootFolderPath: string, taskLabel: string): TaskConfiguration | undefined {
+        const labelConfigMap = this.tasksMap.get(rootFolderPath);
         if (labelConfigMap) {
             return labelConfigMap.get(taskLabel);
         }
@@ -179,8 +200,55 @@ export class TaskConfigurations implements Disposable {
         this.tasksMap.delete(source);
     }
 
-    getTaskCustomizations(type: string): TaskCustomization[] {
-        return this.taskCustomizations.filter(c => c.type === type);
+    /**
+     * Removes task customization objects found in the given task config file from the memory.
+     * Please note: this function does not modify the task config file.
+     */
+    removeTaskCustomizations(configFileUri: string) {
+        const source = this.getSourceFolderFromConfigUri(configFileUri);
+        this.taskCustomizationMap.delete(source);
+    }
+
+    /**
+     * Returns the task customizations by type from a given root folder in the workspace.
+     * @param type the type of task customizations
+     * @param rootFolder the root folder to find task customizations from. If `undefined`, this function returns an empty array.
+     */
+    getTaskCustomizations(type: string, rootFolder?: string): TaskCustomization[] {
+        if (!rootFolder) {
+            return [];
+        }
+
+        const customizationInRootFolder = this.taskCustomizationMap.get(new URI(rootFolder).path.toString());
+        if (customizationInRootFolder) {
+            return customizationInRootFolder.filter(c => c.type === type);
+        }
+        return [];
+    }
+
+    /**
+     * Returns the customization object in `tasks.json` for the given task. Please note, this function
+     * returns `undefined` if the given task is not a detected task, because configured tasks don't need
+     * customization objects - users can modify its config directly in `tasks.json`.
+     * @param taskConfig The task config, which could either be a configured task or a detected task.
+     */
+    getCustomizationForTask(taskConfig: TaskConfiguration): TaskCustomization | undefined {
+        if (!this.isDetectedTask(taskConfig)) {
+            return undefined;
+        }
+
+        const customizationByType = this.getTaskCustomizations(taskConfig.taskType || taskConfig.type, taskConfig._scope) || [];
+        const hasCustomization = customizationByType.length > 0;
+        if (hasCustomization) {
+            const taskDefinition = this.taskDefinitionRegistry.getDefinition(taskConfig);
+            if (taskDefinition) {
+                const cus = customizationByType.filter(customization =>
+                    taskDefinition.properties.required.every(rp => customization[rp] === taskConfig[rp])
+                )[0]; // Only support having one customization per task
+                return cus;
+            }
+        }
+        return undefined;
     }
 
     /** returns the string uri of where the config file would be, if it existed under a given root directory */
@@ -226,19 +294,19 @@ export class TaskConfigurations implements Disposable {
             // user is editing the file in the auto-save mode, having momentarily
             // non-parsing JSON.
             this.removeTasks(configFileUri);
+            this.removeTaskCustomizations(configFileUri);
+            const rootFolderUri = this.getSourceFolderFromConfigUri(configFileUri);
 
             if (configuredTasksArray.length > 0) {
                 const newTaskMap = new Map<string, TaskConfiguration>();
                 for (const task of configuredTasksArray) {
                     newTaskMap.set(task.label, task);
                 }
-                const source = this.getSourceFolderFromConfigUri(configFileUri);
-                this.tasksMap.set(source, newTaskMap);
+                this.tasksMap.set(rootFolderUri, newTaskMap);
             }
 
             if (customizations.length > 0) {
-                this.taskCustomizations.length = 0;
-                this.taskCustomizations = customizations;
+                this.taskCustomizationMap.set(rootFolderUri, customizations);
             }
         }
     }
@@ -275,8 +343,21 @@ export class TaskConfigurations implements Disposable {
             return;
         }
 
-        const configFileUri = this.getConfigFileUri(workspace.uri);
-        if (!this.getTasks().some(t => t.label === task.label)) {
+        const isDetectedTask = this.isDetectedTask(task);
+        let sourceFolderUri: string | undefined;
+        if (isDetectedTask) {
+            sourceFolderUri = task._scope;
+        } else {
+            sourceFolderUri = task._source;
+        }
+        if (!sourceFolderUri) {
+            console.error('Global task cannot be customized');
+            return;
+        }
+
+        const configFileUri = this.getConfigFileUri(sourceFolderUri);
+        const configuredAndCustomizedTasks = await this.getTasks();
+        if (!configuredAndCustomizedTasks.some(t => ContributedTaskConfiguration.equals(t, task))) {
             await this.saveTask(configFileUri, task);
         }
 
@@ -287,19 +368,38 @@ export class TaskConfigurations implements Disposable {
         }
     }
 
+    private getTaskCustomizationTemplate(task: TaskConfiguration): TaskCustomization | undefined {
+        const definition = this.getTaskDefinition(task);
+        if (!definition) {
+            console.error('Detected / Contributed tasks should have a task definition.');
+            return;
+        }
+        const customization: TaskCustomization = { type: task.taskType || task.type };
+        definition.properties.all.forEach(p => {
+            if (task[p] !== undefined) {
+                customization[p] = task[p];
+            }
+        });
+        return {
+            ...customization,
+            problemMatcher: []
+        };
+    }
+
     /** Writes the task to a config file. Creates a config file if this one does not exist */
     async saveTask(configFileUri: string, task: TaskConfiguration): Promise<void> {
         if (configFileUri && !await this.fileSystem.exists(configFileUri)) {
             await this.fileSystem.createFile(configFileUri);
         }
 
         const { _source, $ident, ...preparedTask } = task;
+        const customizedTaskTemplate = this.getTaskCustomizationTemplate(task) || preparedTask;
         try {
             const response = await this.fileSystem.resolveContent(configFileUri);
             const content = response.content;
 
             const formattingOptions = { tabSize: 4, insertSpaces: true, eol: '' };
-            const edits = jsoncparser.modify(content, ['tasks', -1], preparedTask, { formattingOptions });
+            const edits = jsoncparser.modify(content, ['tasks', -1], customizedTaskTemplate, { formattingOptions });
             const result = jsoncparser.applyEdits(content, edits);
 
             const resource = await this.resourceProvider(new URI(configFileUri));
@@ -330,8 +430,15 @@ export class TaskConfigurations implements Disposable {
 
     /** checks if the config is a detected / contributed task */
     private isDetectedTask(task: TaskConfiguration): task is ContributedTaskConfiguration {
-        const taskDefinition = this.taskDefinitionRegistry.getDefinition(task);
+        const taskDefinition = this.getTaskDefinition(task);
         // it is considered as a customization if the task definition registry finds a def for the task configuration
         return !!taskDefinition;
     }
+
+    private getTaskDefinition(task: TaskConfiguration): TaskDefinition | undefined {
+        return this.taskDefinitionRegistry.getDefinition({
+            ...task,
+            type: task.taskType || task.type
+        });
+    }
 }

packages/task/src/browser/task-definition-registry.ts

@@ -15,7 +15,7 @@
  ********************************************************************************/
 
 import { injectable } from 'inversify';
-import { TaskDefinition, TaskConfiguration } from '../common';
+import { TaskConfiguration, TaskCustomization, TaskDefinition } from '../common';
 
 @injectable()
 export class TaskDefinitionRegistry {
@@ -41,7 +41,7 @@ export class TaskDefinitionRegistry {
      * @param taskConfiguration the task configuration
      * @return the task definition for the task configuration. If the task definition is not found, `undefined` is returned.
      */
-    getDefinition(taskConfiguration: TaskConfiguration): TaskDefinition | undefined {
+    getDefinition(taskConfiguration: TaskConfiguration | TaskCustomization): TaskDefinition | undefined {
         const definitions = this.getDefinitions(taskConfiguration.taskType || taskConfiguration.type);
         let matchedDefinition: TaskDefinition | undefined;
         let highest = -1;