Skip to content

Commit

Permalink
define Task CommandOptions and move "cwd" into it
Browse files Browse the repository at this point in the history
- "cwd" (i.e., the current working directory) is currently defined directly under Task, which is inconsistent from the VS Code Task schema. In this pull request "cwd" is moved into the "options" as part of the CommandOptions.
- part of eclipse-theia#5516

Signed-off-by: elaihau <[email protected]>
  • Loading branch information
elaihau committed Jun 20, 2019
1 parent 1295130 commit 957d8f9
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Breaking changes:
- [debug] align commands with VS Code [#5102](https://github.com/theia-ide/theia/issues/5102)
- `debug.restart` renamed to `workbench.action.debug.restart`
- [preferences] renamed overridenPreferenceName to overriddenPreferenceName
- [task] `cwd`, which used to be defined directly under `Task`, is moved into `Task.options` object

## v0.7.0

Expand Down
16 changes: 10 additions & 6 deletions packages/task/src/browser/process/process-task-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,22 @@ export class ProcessTaskResolver implements TaskResolver {
throw new Error('Unsupported task configuration type.');
}

const options = { context: new URI(taskConfig._source).withScheme('file') };
const variableResolverOptions = { context: new URI(taskConfig._source).withScheme('file') };
const processTaskConfig = taskConfig as ProcessTaskConfiguration;
const result: ProcessTaskConfiguration = {
...processTaskConfig,
command: await this.variableResolverService.resolve(processTaskConfig.command, options),
args: processTaskConfig.args ? await this.variableResolverService.resolveArray(processTaskConfig.args, options) : undefined,
command: await this.variableResolverService.resolve(processTaskConfig.command, variableResolverOptions),
args: processTaskConfig.args ? await this.variableResolverService.resolveArray(processTaskConfig.args, variableResolverOptions) : undefined,
windows: processTaskConfig.windows ? {
command: await this.variableResolverService.resolve(processTaskConfig.windows.command, options),
args: processTaskConfig.windows.args ? await this.variableResolverService.resolveArray(processTaskConfig.windows.args, options) : undefined,
command: await this.variableResolverService.resolve(processTaskConfig.windows.command, variableResolverOptions),
args: processTaskConfig.windows.args ? await this.variableResolverService.resolveArray(processTaskConfig.windows.args, variableResolverOptions) : undefined,
options: processTaskConfig.windows.options
} : undefined,
cwd: await this.variableResolverService.resolve(processTaskConfig.cwd || '${workspaceFolder}', options)
options: {
cwd: await this.variableResolverService.resolve(processTaskConfig.options && processTaskConfig.options.cwd || '${workspaceFolder}', variableResolverOptions),
env: processTaskConfig.options && processTaskConfig.options.env,
shell: processTaskConfig.options && processTaskConfig.options.shell
}
};
return result;
}
Expand Down
43 changes: 33 additions & 10 deletions packages/task/src/common/process/task-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,42 @@ import { ApplicationError } from '@theia/core/lib/common/application-error';

export type ProcessType = 'shell' | 'process';

export interface CommandOptions {
/**
* The 'current working directory' the task will run in. Can be a uri-as-string
* or plain string path. If the cwd is meant to be somewhere under the workspace,
* one can use the variable `${workspaceFolder}`, which will be replaced by its path,
* at runtime. If not specified, defaults to the workspace root.
* ex: cwd: '${workspaceFolder}/foo'
*/
cwd?: string;

/**
* The environment of the executed program or shell. If omitted the parent process' environment is used.
*/
env?: { [key: string]: string | undefined; };

/**
* Configuration of the shell when task type is `shell`
*/
shell?: {
/**
* The shell to use.
*/
executable: string;

/**
* The arguments to be passed to the shell executable to run in command mode
* (e.g ['-c'] for bash or ['/S', '/C'] for cmd.exe).
*/
args?: string[];
};
}

export interface CommandProperties<T = string> {
readonly command: string;
readonly args?: T[];
readonly options?: object;
readonly options?: CommandOptions;
}

/** Configuration of a Task that may be run as a process or a command inside a shell. */
Expand All @@ -33,15 +65,6 @@ export interface ProcessTaskConfiguration<T = string> extends TaskConfiguration,
* Windows version of CommandProperties. Used in preference on Windows, if defined.
*/
readonly windows?: CommandProperties<T>;

/**
* The 'current working directory' the task will run in. Can be a uri-as-string
* or plain string path. If the cwd is meant to be somewhere under the workspace,
* one can use the variable `${workspaceFolder}`, which will be replaced by its path,
* at runtime. If not specified, defaults to the workspace root.
* ex: cwd: '${workspaceFolder}/foo'
*/
readonly cwd?: string;
}

export interface ProcessTaskInfo extends TaskInfo {
Expand Down
19 changes: 10 additions & 9 deletions packages/task/src/node/process/process-task-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { TaskFactory } from './process-task';
import { TaskRunner } from '../task-runner';
import { Task } from '../task';
import { TaskConfiguration } from '../../common/task-protocol';
import { ProcessTaskError } from '../../common/process/task-protocol';
import { ProcessTaskError, CommandOptions } from '../../common/process/task-protocol';
import * as fs from 'fs';

/**
Expand Down Expand Up @@ -61,9 +61,9 @@ export class ProcessTaskRunner implements TaskRunner {

let command: string | undefined;
let args: Array<string | QuotedString> | undefined;
let options: any; // tslint:disable-line:no-any
let options: CommandOptions;

// on windows, prefer windows-specific options, if available
// on windows, windows-specific options, if available, takes precedence
if (isWindows && taskConfig.windows !== undefined) {
command = taskConfig.windows.command;
args = taskConfig.windows.args;
Expand All @@ -76,17 +76,18 @@ export class ProcessTaskRunner implements TaskRunner {

// sanity checks:
// - we expect the cwd to be set by the client.
if (!taskConfig.cwd) {
if (!options || !options.cwd) {
throw new Error("Can't run a task when 'cwd' is not provided by the client");
}

const cwd = this.asFsPath(taskConfig.cwd);
// Use task's cwd with spawned process and pass node env object to
// new process, so e.g. we can re-use the system path
options = {
cwd: cwd,
env: process.env
};
if (options) {
options.env = {
...process.env,
...(options.env || {})
};
}

try {
const processType = taskConfig.type === 'process' ? 'process' : 'shell';
Expand Down
10 changes: 5 additions & 5 deletions packages/task/src/node/task-server.slow-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ function createTaskConfig(taskType: string, command: string, args: string[]): Ta
_scope: '/source/folder',
command,
args,
cwd: wsRoot,
options: { cwd: wsRoot }
};
return options;
}
Expand All @@ -376,7 +376,7 @@ function createProcessTaskConfig(processType: ProcessType, command: string, args
_scope: '/source/folder',
command,
args,
cwd,
options: { cwd: wsRoot },
};
}

Expand All @@ -386,7 +386,7 @@ function createProcessTaskConfig2(processType: ProcessType, command: string, arg
type: processType,
command,
args,
cwd: wsRoot,
options: { cwd: wsRoot },
};
}

Expand All @@ -396,7 +396,7 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur
type: processType,
_source: '/source/folder',
_scope: '/source/folder',
cwd: wsRoot,
options: { cwd: wsRoot },
command: commandLongRunning,
args: [],
windows: {
Expand All @@ -406,7 +406,7 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur
};
}

function checkSuccessfullProcessExit(taskInfo: TaskInfo, taskWatcher: TaskWatcher): Promise<object> {
function checkSuccessfullProcessExit(taskInfo: TaskInfo, taskWatcher: TaskWatcher): Promise<unknown> {
const p = new Promise((resolve, reject) => {
const toDispose = taskWatcher.onTaskExit((event: TaskExitedEvent) => {
if (event.taskId === taskInfo.taskId && event.code === 0) {
Expand Down
16 changes: 10 additions & 6 deletions packages/task/test-resources/.theia/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
{
"label": "test task",
"type": "shell",
"cwd": "${workspaceFolder}",
"command": "./task",
"args": [
"test"
],
"options": {},
"options": {
"cwd": "${workspaceFolder}"
},
"windows": {
"command": "cmd.exe",
"args": [
Expand All @@ -22,10 +23,11 @@
{
"label": "long running test task",
"type": "shell",
"cwd": "${workspaceFolder}",
"command": "./task-long-running",
"args": [],
"options": {},
"options": {
"cwd": "${workspaceFolder}"
},
"windows": {
"command": "cmd.exe",
"args": [
Expand All @@ -38,12 +40,14 @@
{
"label": "list all files",
"type": "shell",
"cwd": "${workspaceFolder}",
"command": "ls",
"args": [
"-alR",
"/"
],
"options": {
"cwd": "${workspaceFolder}"
},
"windows": {
"command": "cmd.exe",
"args": [
Expand All @@ -53,4 +57,4 @@
}
}
]
}
}

0 comments on commit 957d8f9

Please sign in to comment.