add task extension #690

marcdumais-work · 2017-10-20T11:17:42Z

Fixes #686

Signed-off-by: Marc Dumais [email protected]

hexa00

Looks very nice in general!!

I did a first pass but I think we need to design this a bit more.

I'll do a second pass later.

hexa00 · 2017-10-24T14:14:35Z

packages/process/src/node/terminal-process.ts


 const pty = require("node-pty");

 export const TerminalProcessOptions = Symbol("TerminalProcessOptions");
 export interface TerminalProcessOptions {
    command: string,
    args?: string[],
-    options?: object
+    options?: child.SpawnOptions


This is not correct as these are the node-pty options rather than the child_process options.

hexa00 · 2017-10-24T14:25:30Z

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

+        this.taskWatcher.onTaskOutputEntryFound((event: ITaskOutputEntryFoundEvent) => {
+            if (ProblemMarker.is(event.entry)) {
+                // problem marker
+                this.messageService.info(`Task ${event.taskId} has sent a problem marker`);


This should be removed.

hexa00 · 2017-10-24T14:26:19Z

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

+        @inject(WorkspaceService) protected readonly workspaceService: WorkspaceService
+    ) {
+        this.taskWatcher.onTaskOutputEntryFound((event: ITaskOutputEntryFoundEvent) => {
+            if (ProblemMarker.is(event.entry)) {


Should we iterate on the marker times and choose a proper handler?

I have removed this, since we do not yet generate markers in the backend. Will be back as part of the patch that will add parsing/handling of tasks output.

hexa00 · 2017-10-24T14:29:36Z

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

+        let tasks: string[];
+
+        tasks = await this.taskServer.getTasks();
+        return tasks;


you can just return await ...

hexa00 · 2017-10-24T14:32:52Z

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

+    }
+
+    protected attachTerminal(terminalId: number, taskId: number): void {
+        const newTerminal = this.terminalWidgetFactory({


We should use the WidgetManager here

I wanted to avoid having the task terminal(s) be managed by the widget manager. e.g. I do not want those terminals to be saved/restored by the frontend (make no sense, I think for tasks).

I think we should expand the widget manager to allow these features and still use it otherwise we're duplicating code.

now using widget manager to create the terminal widget for tasks

Opened issue #776, asking about widget restoration be optional

hexa00 · 2017-10-24T17:08:14Z

packages/task/src/node/task-server.ts

+        } else {
+            // look for command relative to cwd
+            const resolved_command = FileUri.fsPath(new URI(cwd).resolve(command));
+            if (fs.existsSync(resolved_command)) {


hexa00 · 2017-10-24T17:10:01Z

packages/task/src/node/task-server.ts

+    // Check if task command can be found, if so return its resolved
+    // absolute path, else returns undefined
+    private findCommand(command: string, cwd: string): string | undefined {
+        const systemPath = process.env.PATH;


not sure this is windows friendly?

It depends what you mean. I think it does work to get the system path, to try to find an executable in it. However, in many cases on windows, the command, that here we confirm exists, will just be "cmd.exe" and the real execution target will be passed as a parameter. ex: cmd.exe /c test.bat . In that sense, this is not so windows-friendly.

I just mean I'm not sure PATH is the env var windows uses.

So you mean if need to test cmd.exe and the destination ? I think we can assume cmd.exe exists no ?

Maybe we can assume that cmd.exe exists. The user could in theory point towards an alternate interpreter. The destination can be a script/executable but can also be an interpreter built-in, like "DIR". So we may not find it as a file in that case, even if we look.

indeed that's just the situation I wanted to avoid.

What if we were to try to run it, and after that if it fails try to narrow down the reason using findCommand?

That way these cases would be avoided most likely.

Interesting idea; I like it. This way we at least try to execute the command, even if we can't figure-out exactly what it's calling. If it succeeds, everything is good. If it fails, we dig down to try and see if we can figure-out why it failed, looking into locating the command, maybe in some cases its file parameter(s).

I do not think we avoid the corner cases entirely, however. There will still be cases like : "cmd.exe /c DIZ" that will be hard to pin down: did the user want the "DIR" built-in of cmd.exe and made a typo? Or was he trying to execute a DIZ.bat batch file in cwd?

hexa00 · 2017-10-24T17:11:04Z

packages/task/src/node/task-server.ts

+
+            const separator = isWindows ? ";" : ":";
+
+            // just a command without path?


I think it would just be simpler to try it out.

True. The reason for findCommand() is try to figure-out if the command/script, we're about to try executing, can be found. We could just skip ahead and try to execute it; if it's then found and executed successfully, then no problem. But if it's not found or not successful, it can be difficult to distinguish what happened:

the command was found and run, but exited with an error status (rc !=0)

the command was not found (rc != 0)

So, in findCommand() we try to determine if the command/script seems to exist, so that we can more correctly assume that any error is a result of its execution, not of it not having been found.

OK I wish RawProcess/TerminalProcess would take care of that, I know I can know it for RawProcess but I don't think it's possible now for the Terminal.

So OK for now and we'll try to improve that upstream

hexa00 · 2017-10-24T17:12:25Z

packages/task/src/node/task.ts

+export class Task {
+    protected taskId: number;
+    protected taskProcess: Process;
+    // linter: string | undefined;


hexa00 · 2017-10-24T17:20:54Z

packages/task/src/node/task.ts

+        this.taskId = this.taskManager.register(this);
+        this.taskProcess = process;
+
+        this.taskProcess.onExit(event => {


You also need to add ProcessManager.delete(this.taskProcess)
And cleanup the onExit listener.

Maybe this could be doner better in general we need to think about this.

svenefftinge · 2017-10-30T13:14:59Z

Could we reuse the format from vs code? (https://code.visualstudio.com/docs/editor/tasks-appendix#_schema-for-tasksjson)
Those files are checked into repositories and there are already quite a good number of those in e.g. github repos: https://github.com/search?utf8=%E2%9C%93&q=problemMatcher+extension%3Ajson&type=Code

It would make it easier for people to switch between vs code and cloud based theia workspaces.

marcdumais-work · 2017-10-30T19:01:54Z

@svenefftinge Possibly. We do not have, ATM as rich a feature-set as vs code does, w-r to tasks, matchers, etc. So just re-using their format "as-is" might give a false sense of hope that the features will work, and work the same way. But we could try to be more similar, when it makes sense. e.g. ErrorMatcher vs ProblemMatcher.

WDYT?

svenefftinge · 2017-10-31T07:25:25Z

As close as possible, doesn't really help with the reuse. But can't the problemMatcher stuff be considered a nice-to-have optional feature anyways? So I would say we could log out a warning about features that are not yet supported, as long as we can start the process.

svenefftinge · 2017-11-05T14:21:05Z

The current design is based on the assumptions of a single workspace and a fixed storage for user settings. As discussed in other contexts, the TaskConfiguration should be in the frontend based on user-storage service (user level) and file system watch (workspace level).
The TaskServer would only have run and kill and always gets the TaskOptions as parameter to run.

marcdumais-work · 2017-12-01T02:24:46Z

In this new version I moved the handling of task configuration files to the frontend.

The backend server is now a singleton, and it supports multiple clients. All clients that connect are remembered and task-related events are broadcasted to all clients, and includes a context field that is given by the client, when it calls run() . Currently the client uses its current WS root URI string as context. It filters received events and only show them to the user when the event's context matches their own.

This way multiple clients, using a same WS simultaneously, can each show their respective users, e.g. that a specific task has just been started. The user(s) that did not start the task in their client will then have the option of "attaching" to the task, opening a terminal to see the task's output so far, going back to the beginning, if it stays within reasonable limits, and see new outputs as they happen.

I have not yet added the user-level configuration; that can be done in a separate issue.

TODO now:

Investigate/ fix the 3 disabled (skipped) unit tests (on Windows only).
replace the JSON parser used in the FE to parse tasks.json, to put back the one from VS Code, that has better diagnostics. Patrick has made it work recently in the FE, so I can do similar

TODO later:

hexa00 · 2017-12-04T13:55:19Z

packages/task/src/browser/index.ts

+ * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ */
+
+export * from './task-frontend-module';


I don't think this should be exported. is it needed anywhere?

not needed - removed.

hexa00 · 2017-12-04T13:56:26Z

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

+
+import { inject, injectable } from "inversify";
+import { TaskService } from './task-service';
+import { ITaskInfo } from '../common/task-protocol';


The I prefix is now against the code guidelines... in favor of noprefix and Impl suffix.

Correct - Renamed all I*

hexa00 · 2017-12-04T13:58:06Z

packages/task/package-lock.json

@@ -0,0 +1,17 @@
+{


This should not happen... unless there's 2 conflicting versions.
I deleted that file, redid a yarn and only got a modified yarn.lock
So this file should be removed from git.

Correct - the dependency in there is not longer used. File removed.

hexa00 · 2017-12-04T14:01:05Z

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

+        if (this.task.processId) {
+            // reset focus on the previously active element.
+            this.activeElement.focus();
+            // this.taskService.run(this.taskLabel);


left over ?

removed left-over line

hexa00 · 2017-12-04T14:07:35Z

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

+        }));
+    }
+
+    init(client: TaskConfigurationClient) {


Should be setClient ? like the others

renamed method

hexa00 · 2017-12-04T19:17:29Z

packages/task/src/node/task-server.ts

+                    resolve(command);
+                    return;
+                } else {
+                    const uri = new URI(cwd).resolve(command);


Not sure I get this it's absolute, and it doesn't exist but we will check for cwd/absolutepath ?

Indeed, this looks like dead code; if a command with an absolute path is not found, there's not much more we can do. Removed.

hexa00 · 2017-12-04T19:19:42Z

packages/task/src/node/task-server.ts

+                }
+            } else {
+                // look for command relative to cwd
+                const resolved_command = FileUri.fsPath(new URI(cwd).resolve(command));


About CWD should it default to workspaceRoot somewhere? I don't think it makes sense to use the process.cwd

With the refactorings to support multiple clients, the Task backend server has become stateless, with regards to the workspace being used (it used to be that each client would get it's own instance of the Task server, that would know what the WS Root was). The backend doesn't know which workspace(s) are being used.

Instead, each task configuration defines what "CWD" should be used by that task. The F-E will replace the symbolic value "$workspace" by its value, if found in the task configuration's cwd. In the end, that same CWD value is used to create the process, used by the task.

update: now cwd defaults to the WS Root, unless otherwise defined in the task configuration (set by frontend).

hexa00 · 2017-12-04T19:25:13Z

packages/task/src/node/task-server.ts

+                    return;
+                } else {
+                    // should cover Unix and Windows cases
+                    const separator = /;|:/;


: is valid as a linux file we should check for isWindows and select 1 separator only.

You can use: https://nodejs.org/api/path.html#path_path_delimiter

➜ ~ touch ab:cd ➜ ~ ls -la ./ab:cd -rw-r--r-- 1 eantotr eusers 0 Dec 4 14:22 ./ab:cd ➜ ~ touch abc;cd ➜ ~ ls -l ab* -rw-r--r-- 1 eantotr eusers 0 Dec 4 14:23 abc -rw-r--r-- 1 eantotr eusers 0 Dec 4 14:22 ab:cd

Cool, I didn't know about path.delimiter. Since we get that value from node, I think it already takes into account which platform we are on.

Done (except handling the case where there is a ":" used within a directory in the system path - Sorry, if you do that, you deserve for your executable not to be found ;)

except handling the case where there is a ":" used within a directory in the system path - Sorry, if you do that, you deserve for your executable not to be found

That won't happen since I guess on windows you can't create a file or directory with : like you can't do a file /dir with ; in linux ?

hexa00 · 2017-12-04T19:30:07Z

packages/task/src/node/test-resources/task-long-running.bat

+   echo tasking... %%x
+   @REM sleep for ~1s
+   @REM  see: https://stackoverflow.com/questions/735285/how-to-wait-in-a-batch-script
+   ping 192.0.2.2 -n 1 -w 1000> nul


weird IP why not 127.0.0.1 ?

The answer is in the link above: basically this is garanteed, by definition, to be an address that can't be assigned. So we are certain that no one will answer and that we will spend the "1000" ms waiting. So basically a poor man's "sleep". If we used 127.0.0.1, it would answer immediately and not wait for 1000ms.

hexa00 · 2017-12-04T19:30:34Z

packages/task/todo.txt

@@ -0,0 +1,22 @@
+


Remove leave in github

epatpol · 2017-12-05T15:28:45Z

packages/task/package.json

+    "@theia/process": "^0.2.3",
+    "@theia/terminal": "^0.2.3",
+    "@theia/workspace": "^0.2.3",
+    "require-strip-json-comments": "^1.0.2"


I thought jsonc-parser ended up being used by modifying the webpack config file? If that is also used it's fine though.

Also if you really use jsonc-parser, shouldn't that dependency be specifically mentionned in the task package.json?

@epatpol I think you were looking at the previous version? This is no longer in there.

Also if you really use jsonc-parser, shouldn't that dependency be specifically mentionned in the task package.json?

It's added in the latest version

epatpol · 2017-12-05T15:42:57Z

.theia/tasks.json

@@ -0,0 +1,73 @@
+{
+    // Some sample Theia development tasks
+    "tasks": [


I like having a sample tasks.json but maybe the sample config should be in the package README? Or the supplied sample tasks should be commented out? I'm not sure master should have default tasks when starting theia. Maybe having a package README which would describe a sample task and go in details about every attributes? Or simply a link to vscode's page on the task.json file attributes if they're very similar.

Good suggestions. I removed the example tasks.json and added a README, in which I list the example tasks.

svenefftinge · 2017-11-05T14:02:52Z

.theia/tasks.json

+    // comment
+    "tasks": [
+        {
+            "label": "workspace: failing task",


Are these test tasks? Shouldn't they go into some test-resource forlder?

Yes, they are example / manual test tasks. At Patrick's suggestion, I deleted that file and instead added the task configurations as examples, to the README.md for the extension.

svenefftinge · 2017-12-06T06:58:35Z

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

+        protected taskService: TaskService
+    ) {
+        super();
+        this.activeElement = window.document.activeElement as HTMLElement;


This is already done generally in https://github.com/theia-ide/theia/blob/87825f441613cbd5caf145a6b80d8a358d3995da/packages/monaco/src/browser/monaco-quick-open-service.ts#L45
Didn't it work for you?

It doesn't seem to make much difference, either way, in this case. If we do proceed to create or attach to a task, we may not want to restore the previous focus anyway, and focus on the new terminal. Removed.

svenefftinge · 2017-12-06T06:58:51Z

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

+            return false;
+        }
+        // reset focus on the previously active element.
+        this.activeElement.focus();


see https://github.com/theia-ide/theia/blob/87825f441613cbd5caf145a6b80d8a358d3995da/packages/monaco/src/browser/monaco-quick-open-service.ts#L59

svenefftinge · 2017-12-06T07:11:59Z

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

+        protected taskService: TaskService
+    ) {
+        super();
+        this.activeElement = window.document.activeElement as HTMLElement;


removed too

svenefftinge · 2017-12-06T07:12:06Z

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

+        }
+        if (this.task.processId) {
+            // reset focus on the previously active element.
+            this.activeElement.focus();


removed too

svenefftinge · 2017-12-06T07:52:18Z

packages/task/src/node/task-server.ts

+    }
+
+    run(task: TaskOptions, ctx?: string): Promise<TaskInfo> {
+        return this.doRun(task, ctx);


why delegate to doRun?

No good reason any more -> removed run(), renamed doRun() to run().

svenefftinge · 2017-12-06T07:52:57Z

packages/task/src/node/task-server.ts

+    }
+
+    protected doRun(options: TaskOptions, ctx?: string): Promise<TaskInfo> {
+        return new Promise<TaskInfo>(async (resolve, reject) => {


unnecessary outer promise, async/await?

refactored to use async/await

svenefftinge · 2017-12-06T07:56:44Z

packages/task/src/node/task-server.ts

+            // because the executable or script was not found, or if it was found, ran, and exited
+            // unsuccessfully. So here we look to see if it seems we can find a file of that name that
+            // is likely to be the one we want, before attemting to execute it.
+            this.findCommand(command, cwd)


why do we do findCommand? Aren't the error message from the process factories good?

no, not so much. It's difficult to distinguish between the cases where a script or binary is found and executed, with an abnormal exit, versus the script/binary not being found at all (I think because of limitations in node-pty). Maybe we should enhance node-pty?

svenefftinge · 2017-12-06T07:57:55Z

packages/task/src/node/task-server.ts

+                                context: ctx
+                            });
+
+                        const toDispose = new DisposableCollection();


seems like there is always only one disposable in the collection so we could remove it.

svenefftinge · 2017-12-06T07:59:55Z

packages/task/src/node/task-server.ts

+                            processId: (options.processType === 'raw') ? proc.id : undefined,
+                            command: cmd,
+                            label: options.label,
+                            ctx: ctx === undefined ? '' : ctx


Could we extract the translation to a method and use it also from here https://github.com/theia-ide/theia/pull/690/files#diff-fccfd5a001011db19d656ecd3d66d03dR67

good idea - done

hexa00 · 2017-12-06T15:39:44Z

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

+        });
+    }
+
+    protected hasWatchedFileChanged(changes: FileChange[]): boolean {


Just a marker for @akosyakov #942

You should not use the watcher server in the frontend, it is the impl details of FileSystemWatcher, the watcher takes care of servers and reconnection to them if a connection is down as well as preserving watching.

I am working on PR which will add an optional callback to FileSystemWatcher.watch(uri): Promise<Disposable> that you can pass it to avoid dispatching changes to other clients.

@akosyakov ok, I will update this code accordingly, when your PR is ready.

hexa00

Just nits, OK with the changes

hexa00 · 2017-12-13T19:51:29Z

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

+export class TaskConfigurations implements Disposable {
+
+    protected readonly toDispose = new DisposableCollection();
+


hexa00 · 2017-12-13T19:51:35Z

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

+    protected readonly toDispose = new DisposableCollection();
+
+    protected tasksMap = new Map<string, TaskOptions>();
+


hexa00 · 2017-12-13T19:58:21Z

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

+                }
+            }
+        } else {
+            this.logger.info(`Config file tasks.json does not exist under ${rootUri}`);


change to warn ?

It can be normal that the user doesn't have one...

True, tasks.json presence is optional. done

hexa00 · 2017-12-13T20:00:49Z

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

+
+        if (await this.fileSystem.exists(cfgFile)) {
+            return cfgFile;
+        }


Missing else { return Promise.resolve(undefined) }

Following previous changes, this is no longer applicable (method no longer async now that it only computes the potential task config file URI)

hexa00 · 2017-12-13T20:03:57Z

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

+        }
+    }
+
+    protected async getConfigFile(rootDir: string): Promise<string | undefined> {


I would separate the existance from getConfigFile like:

getConfigFileURI

then in code inline this.filesystem.exits(getConfigFileURI(rootDir))

but just a nit.. do as you want

why not - done.

hexa00 · 2017-12-13T20:38:26Z

packages/task/src/node/task-server.ts

+     * @param command command name to look-for
+     * @param cwd current working directory
+     */
+    private async findCommand(command: string, cwd: string): Promise<string | undefined> {


protected ?

hexa00 · 2017-12-13T20:41:42Z

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

+@injectable()
+export class QuickOpenTask implements QuickOpenModel {
+
+    private items: QuickOpenItem[];


hexa00 · 2017-12-13T20:41:49Z

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

+        }
+    }
+
+    private filterDuplicates(tasks: TaskOptions[]): TaskOptions[] {


hexa00 · 2017-12-13T20:41:55Z

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

+        widget.start(terminalId);
+    }
+
+    private isEventForThisClient(context: string | undefined): boolean {


done. BTW, I noticed in general that we tend not to explicitly mark methods as "public". I was doing it in this class, so I removed these.

OK yes that's still a gray zone to me.

hexa00 · 2017-12-13T20:42:06Z

packages/task/src/common/task-watcher.ts

+        };
+    }
+
+    private onTaskCreatedEmitter = new Emitter<TaskInfo>();


marcdumais-work · 2017-12-14T13:44:19Z

@hexa00 Thanks for the second review pass. Let's see if the unit tests still pass!

marcdumais-work · 2017-12-14T13:54:07Z

Git issue during Windows tests again - will slightly modify PR and push again, to trigger new tests

marcdumais-work · 2017-12-14T14:13:47Z

almost this time - electron test failure

marcdumais-work · 2017-12-14T19:38:03Z

ok, the Windows tests finally went went through.

@svenefftinge I plan to rebase this on the latest master and merge probably on Monday morning, Montreal time. Please let me know if you have objections.

This extension provides the means to execute scripts or executables, from a frontend client, in the backend. Tasks life cycle events are send from the backend to the frontend clients, whenever a task is created or terminates. A task client can ask the backend for the list of tasks that are currently running. We support having multiple frontend task clients that are using the same backend task server. Each client uses its workspace root as context parameter, in all its communications with the backend. Whenever the backend sends notifications to its clients, the original client context is sent along. It's then up-to the clients to determine if they are interested in the event, given its context, and show it to the user or not. This mechanism makes it possible for multiple clients, opened on the same workspace, to all get the notifications related to all tasks that execute from that workspace. At the same time, clients can ignore tasks that are run from another workspace. Fixes #686 Signed-off-by: Marc Dumais <[email protected]>

akosyakov · 2017-12-19T08:38:28Z

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

+    ) {
+        this.toDispose.push(watcherServer);
+
+        watcherServer.setClient({


@marcdumais it is bogus, there is only one FileSystemWatcherServer which is used in FileSystemWatcher. With this change, either FileSystemWatcher or TaskConfiguration won't receive changes.

marcdumais-work requested a review from hexa00 October 20, 2017 11:17

marcdumais-work force-pushed the GH-686 branch 3 times, most recently from 331f04f to 165ae98 Compare October 20, 2017 17:52

marcdumais-work mentioned this pull request Oct 20, 2017

Task extension #686

Closed

hexa00 suggested changes Oct 24, 2017

View reviewed changes

marcdumais-work force-pushed the GH-686 branch 4 times, most recently from b2efb1d to fa59126 Compare November 10, 2017 19:32

marcdumais-work force-pushed the GH-686 branch 4 times, most recently from 3811f8a to 560aa68 Compare November 30, 2017 18:21

marcdumais-work force-pushed the GH-686 branch from 560aa68 to a402b12 Compare December 1, 2017 11:53

hexa00 reviewed Dec 4, 2017

View reviewed changes

epatpol reviewed Dec 5, 2017

View reviewed changes

marcdumais-work force-pushed the GH-686 branch 3 times, most recently from 008cdc3 to 923bfba Compare December 6, 2017 01:49

svenefftinge reviewed Dec 6, 2017

View reviewed changes

hexa00 reviewed Dec 6, 2017

View reviewed changes

marcdumais-work force-pushed the GH-686 branch 2 times, most recently from 9208917 to 3946915 Compare December 13, 2017 13:22

marcdumais-work force-pushed the GH-686 branch 7 times, most recently from 7096689 to 87592a3 Compare December 13, 2017 17:47

hexa00 approved these changes Dec 13, 2017

View reviewed changes

marcdumais-work force-pushed the GH-686 branch from 87592a3 to 22972ca Compare December 14, 2017 13:43

marcdumais-work force-pushed the GH-686 branch from 22972ca to cf3e405 Compare December 14, 2017 13:55

marcdumais-work force-pushed the GH-686 branch 2 times, most recently from f9268df to f0aaa2f Compare December 14, 2017 14:35

marcdumais-work force-pushed the GH-686 branch from f0aaa2f to c2143cd Compare December 18, 2017 11:40

marcdumais-work force-pushed the GH-686 branch from c2143cd to d986b8c Compare December 18, 2017 15:38

marcdumais-work merged commit 812e2dd into master Dec 18, 2017

marcdumais-work deleted the GH-686 branch December 18, 2017 19:28

akosyakov reviewed Dec 19, 2017

View reviewed changes

marcdumais-work mentioned this pull request Jan 18, 2018

[Tasks] potential improvement ideas #1080

Closed


		const separator = isWindows ? ";" : ":";

		// just a command without path?

		export class TaskConfigurations implements Disposable {

		protected readonly toDispose = new DisposableCollection();

		protected readonly toDispose = new DisposableCollection();

		protected tasksMap = new Map<string, TaskOptions>();

add task extension #690

add task extension #690

Conversation

marcdumais-work commented Oct 20, 2017

hexa00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexa00 Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenefftinge commented Oct 30, 2017

marcdumais-work commented Oct 30, 2017

svenefftinge commented Oct 31, 2017

svenefftinge commented Nov 5, 2017

marcdumais-work commented Dec 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcdumais-work Dec 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexa00 Oct 27, 2017 •

edited

Loading

marcdumais-work commented Dec 1, 2017 •

edited

Loading

marcdumais-work Dec 5, 2017 •

edited

Loading