Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add '_scope' to task configuration #4452

Merged
merged 1 commit into from
Mar 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/cpp/src/browser/cpp-task-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export class CppTaskProvider implements TaskContribution, TaskProvider, TaskReso
return {
type: CPP_BUILD_TASK_TYPE_KEY,
_source: CPP_BUILD_TASK_SOURCE,
_scope: config.directory,
label: `C/C++ Build - ${config.name}`,
config
};
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-ext/src/api/plugin-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,7 @@ export interface TaskDto {
type: string;
label: string;
source?: string;
scope?: string;
// tslint:disable-next-line:no-any
[key: string]: any;
}
Expand Down
18 changes: 14 additions & 4 deletions packages/plugin-ext/src/main/browser/tasks-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,26 @@ export class TasksMainImpl implements TasksMain {
protected createTaskProvider(handle: number): TaskProvider {
return {
provideTasks: () =>
this.proxy.$provideTasks(handle).then(v => v!.map(taskDto =>
Object.assign(taskDto, { _source: taskDto.source || 'plugin' })
)),
this.proxy.$provideTasks(handle).then(v =>
v!.map(taskDto =>
Object.assign(taskDto, {
_source: taskDto.source || 'plugin',
_scope: taskDto.scope
})
)
)
};
}

protected createTaskResolver(handle: number): TaskResolver {
return {
resolveTask: taskConfig =>
this.proxy.$resolveTask(handle, taskConfig).then(v => Object.assign(v!, { _source: v!.source || 'plugin' })),
this.proxy.$resolveTask(handle, taskConfig).then(v =>
Object.assign(v!, {
_source: v!.source || 'plugin',
_scope: v!.scope
})
)
};
}
}
2 changes: 2 additions & 0 deletions packages/plugin-ext/src/plugin/type-converters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ describe('Type converters:', () => {
type,
label,
source,
scope: undefined,
command,
args,
cwd,
Expand Down Expand Up @@ -208,6 +209,7 @@ describe('Type converters:', () => {
type,
label,
source,
scope: undefined,
command,
args,
cwd,
Expand Down
15 changes: 12 additions & 3 deletions packages/plugin-ext/src/plugin/type-converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ export function fromTask(task: theia.Task): TaskDto | undefined {
const taskDto = {} as TaskDto;
taskDto.label = task.name;
taskDto.source = task.source;
taskDto.scope = typeof task.scope === 'object' ? task.scope.uri.toString() : undefined;

const taskDefinition = task.definition;
if (!taskDefinition) {
Expand All @@ -598,11 +599,11 @@ export function fromTask(task: theia.Task): TaskDto | undefined {
}

const processTaskDto = taskDto as ProcessTaskDto;
if (taskDefinition.type === 'shell') {
if (taskDefinition.type === 'shell' || types.ShellExecution.is(execution)) {
return fromShellExecution(execution, processTaskDto);
}

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

Expand All @@ -614,10 +615,18 @@ export function toTask(taskDto: TaskDto): theia.Task {
throw new Error('Task should be provided for converting');
}

const { type, label, source, command, args, options, windows, cwd, ...properties } = taskDto;
const { type, label, source, scope, command, args, options, windows, cwd, ...properties } = taskDto;
const result = {} as theia.Task;
result.name = label;
result.source = source;
if (scope) {
const uri = URI.parse(scope);
result.scope = {
uri,
name: uri.toString(),
index: 0
};
}

const taskType = type;
const taskDefinition: theia.TaskDefinition = {
Expand Down
10 changes: 10 additions & 0 deletions packages/plugin-ext/src/plugin/types-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,11 @@ export class ProcessExecution {
}
return hash.digest('hex');
}

public static is(value: theia.ShellExecution | theia.ProcessExecution): boolean {
const candidate = value as ProcessExecution;
return candidate && !!candidate.process;
}
}

export enum ShellQuoting {
Expand Down Expand Up @@ -1445,6 +1450,11 @@ export class ShellExecution {
}
return hash.digest('hex');
}

public static is(value: theia.ShellExecution | theia.ProcessExecution): boolean {
const candidate = value as ShellExecution;
return candidate && (!!candidate.commandLine || !!candidate.command);
}
}

export class TaskGroup {
Expand Down
1 change: 1 addition & 0 deletions packages/task/src/browser/process/process-task-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export class ProcessTaskResolver implements TaskResolver {
const result: ProcessTaskConfiguration = {
type: processTaskConfig.type,
_source: processTaskConfig._source,
_scope: processTaskConfig._scope,
label: processTaskConfig.label,
command: await this.variableResolverService.resolve(processTaskConfig.command, options),
args: processTaskConfig.args ? await this.variableResolverService.resolveArray(processTaskConfig.args, options) : undefined,
Expand Down
65 changes: 65 additions & 0 deletions packages/task/src/browser/provided-task-configurations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/********************************************************************************
* Copyright (C) 2019 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { inject, injectable } from 'inversify';
import { TaskConfiguration } from '../common/task-protocol';
import { TaskProviderRegistry } from './task-contribution';

@injectable()
export class ProvidedTaskConfigurations {

/**
* Map of source (name of extension, or path of root folder that the task config comes 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>>();

@inject(TaskProviderRegistry)
protected readonly taskProviderRegistry: TaskProviderRegistry;

/** 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());
}
this.cacheTasks(providedTasks);
return providedTasks;
}

/** returns the task configuration for a given source and label or undefined if none */
getTask(source: string, taskLabel: string): TaskConfiguration | undefined {
const labelConfigMap = this.tasksMap.get(source);
if (labelConfigMap) {
return labelConfigMap.get(taskLabel);
}
}

protected cacheTasks(tasks: TaskConfiguration[]): void {
for (const task of tasks) {
const label = task.label;
const source = task._source;
if (this.tasksMap.has(source)) {
this.tasksMap.get(source)!.set(label, task);
} else {
const labelTaskMap = new Map<string, TaskConfiguration>();
labelTaskMap.set(label, task);
this.tasksMap.set(source, labelTaskMap);
}
}
}
}
21 changes: 15 additions & 6 deletions packages/task/src/browser/quick-open-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import { inject, injectable } from 'inversify';
import { QuickOpenService, QuickOpenModel, QuickOpenItem, QuickOpenGroupItem, QuickOpenMode, QuickOpenHandler, QuickOpenOptions } from '@theia/core/lib/browser/quick-open/';
import { TaskService } from './task-service';
import { TaskConfigurations } from './task-configurations';
import { TaskInfo, TaskConfiguration } from '../common/task-protocol';
import { TaskConfigurations } from './task-configurations';
import URI from '@theia/core/lib/common/uri';

@injectable()
Expand All @@ -33,15 +33,18 @@ export class QuickOpenTask implements QuickOpenModel, QuickOpenHandler {
@inject(TaskService)
protected readonly taskService: TaskService;

@inject(TaskConfigurations)
protected readonly taskConfigurations: TaskConfigurations;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's 0.5.0

i'm fine with that, please be conscious when you break

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right. i will put it back. Thank you !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 more breaking changes in this PR.

I'm fine with 0.5.0 for next. There are PRs, like refactoring git to scm extension, for which it is not avoidable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akosyakov just thinking about API breaking going under the radar, would there be a strategy to adopt in order to catch any API breakage?

I guess this could be done with our test suite, but despite all the tests we have, it seems to not be enough. Is there something that should be done?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of any tools for TS allowing to check it. Even if there are I am not sure that we want to use them, since we are fine with breaking and updating the major version. It would cause a lot of noise.

Copy link
Contributor Author

@elaihau elaihau Mar 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put taskConfigurations back and commented To be removed in 0.5.0. We could leave it here for the release where other breaking changes are made. What do you think ?


@inject(QuickOpenService)
protected readonly quickOpenService: QuickOpenService;

/**
* @deprecated To be removed in 0.5.0
*/
@inject(TaskConfigurations)
protected readonly taskConfigurations: TaskConfigurations;

/** Initialize this quick open model with the tasks. */
async init(): Promise<void> {
const configuredTasks = this.taskConfigurations.getTasks();
const configuredTasks = this.taskService.getConfiguredTasks();
const providedTasks = await this.taskService.getProvidedTasks();

this.items = [];
Expand Down Expand Up @@ -128,7 +131,10 @@ export class TaskRunQuickOpenItem extends QuickOpenGroupItem {
}

getLabel(): string {
return `${this.task.type}: ${this.task.label}`;
if (this.isConfigured) {
return `${this.task.type}: ${this.task.label}`;
}
return `${this.task._source}: ${this.task.label}`;
}

getGroupLabel(): string {
Expand All @@ -139,6 +145,9 @@ export class TaskRunQuickOpenItem extends QuickOpenGroupItem {
if (this.isConfigured) {
return new URI(this.task._source).displayName;
}
if (this.task._scope) {
return new URI(this.task._scope).path.toString();
}
return this.task._source;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/task/src/browser/task-frontend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { QuickOpenTask } from './quick-open-task';
import { TaskContribution, TaskProviderRegistry, TaskResolverRegistry } from './task-contribution';
import { TaskService } from './task-service';
import { TaskConfigurations } from './task-configurations';
import { ProvidedTaskConfigurations } from './provided-task-configurations';
import { TaskFrontendContribution } from './task-frontend-contribution';
import { createCommonBindings } from '../common/task-common-module';
import { TaskServer, taskPath } from '../common/task-protocol';
Expand All @@ -38,6 +39,7 @@ export default new ContainerModule(bind => {

bind(QuickOpenTask).toSelf().inSingletonScope();
bind(TaskConfigurations).toSelf().inSingletonScope();
bind(ProvidedTaskConfigurations).toSelf().inSingletonScope();

bind(TaskServer).toDynamicValue(ctx => {
const connection = ctx.container.get(WebSocketConnectionProvider);
Expand Down
34 changes: 18 additions & 16 deletions packages/task/src/browser/task-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service
import { VariableResolverService } from '@theia/variable-resolver/lib/browser';
import { TaskWatcher } from '../common/task-watcher';
import { TaskConfigurationClient, TaskConfigurations } from './task-configurations';
import { ProvidedTaskConfigurations } from './provided-task-configurations';
import URI from '@theia/core/lib/common/uri';

@injectable()
Expand Down Expand Up @@ -64,12 +65,18 @@ export class TaskService implements TaskConfigurationClient {
@inject(TaskConfigurations)
protected readonly taskConfigurations: TaskConfigurations;

@inject(ProvidedTaskConfigurations)
protected readonly providedTaskConfigurations: ProvidedTaskConfigurations;

@inject(VariableResolverService)
protected readonly variableResolverService: VariableResolverService;

@inject(TaskResolverRegistry)
protected readonly taskResolverRegistry: TaskResolverRegistry;

/**
* @deprecated To be removed in 0.5.0
*/
@inject(TaskProviderRegistry)
protected readonly taskProviderRegistry: TaskProviderRegistry;

Expand Down Expand Up @@ -117,32 +124,27 @@ export class TaskService implements TaskConfigurationClient {

/** Returns an array of the task configurations configured in tasks.json and provided by the extensions. */
async getTasks(): Promise<TaskConfiguration[]> {
const configuredTasks = this.taskConfigurations.getTasks();
const configuredTasks = this.getConfiguredTasks();
const providedTasks = await this.getProvidedTasks();
return [...configuredTasks, ...providedTasks];
}

/** Returns an array of the task configurations which are configured in tasks.json files */
getConfiguredTasks(): TaskConfiguration[] {
return this.taskConfigurations.getTasks();
}

/** Returns an array of the task configurations which are provided by the extensions. */
async getProvidedTasks(): Promise<TaskConfiguration[]> {
const providedTasks: TaskConfiguration[] = [];
const providers = this.taskProviderRegistry.getProviders();
for (const provider of providers) {
providedTasks.push(...await provider.provideTasks());
}
return providedTasks;
getProvidedTasks(): Promise<TaskConfiguration[]> {
return this.providedTaskConfigurations.getTasks();
}

/**
* Returns a task configuration provided by an extension by task source and label.
* If there are no task configuration, returns undefined.
*/
async getProvidedTask(source: string, label: string): Promise<TaskConfiguration | undefined> {
const provider = this.taskProviderRegistry.getProvider(source);
if (provider) {
const tasks = await provider.provideTasks();
return tasks.find(t => t.label === label);
}
return undefined;
getProvidedTask(source: string, label: string): TaskConfiguration | undefined {
return this.providedTaskConfigurations.getTask(source, label);
}

/** Returns an array of running tasks 'TaskInfo' objects */
Expand All @@ -168,7 +170,7 @@ export class TaskService implements TaskConfigurationClient {
* It looks for configured and provided tasks.
*/
async run(source: string, taskLabel: string): Promise<void> {
let task = await this.getProvidedTask(source, taskLabel);
let task = this.getProvidedTask(source, taskLabel);
if (!task) {
task = this.taskConfigurations.getTask(source, taskLabel);
if (!task) {
Expand Down
5 changes: 5 additions & 0 deletions packages/task/src/common/task-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export interface TaskConfiguration {
* This field is not supposed to be used in `tasks.json`
*/
readonly _source: string;
/**
* For a provided task, it is the string representation of the URI where the task is supposed to run from. It is `undefined` for global tasks.
* This field is not supposed to be used in `tasks.json`
*/
readonly _scope: string | undefined;
/** A label that uniquely identifies a task configuration per source */
readonly label: string;
readonly type: string;
Expand Down
3 changes: 3 additions & 0 deletions packages/task/src/node/task-server.slow-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ function createTaskConfig(taskType: string, command: string, args: string[]): Ta
label: 'test task',
type: taskType,
_source: '/source/folder',
_scope: '/source/folder',
command: command,
args: args,
windows: {
Expand All @@ -359,6 +360,7 @@ function createProcessTaskConfig(processType: ProcessType, command: string, args
label: 'test task',
type: processType,
_source: '/source/folder',
_scope: '/source/folder',
command: command,
args: args,
windows: {
Expand Down Expand Up @@ -390,6 +392,7 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur
label: '[Task] long running test task (~300s)',
type: processType,
_source: '/source/folder',
_scope: '/source/folder',
cwd: wsRoot,
command: commandLongRunning,
args: [],
Expand Down