Allow extension to provide callback functions as tasks

[GabeDeBacker]

Our build environment is quite complex and offers more that just console output. For example, information can be obtained by listening to event tracing providers.

As such, our VSCode extension communicates with an out of process server (using JSON-RPC) to perform the build actions in our environment. The extension exposes commands that users must specifically invoke to perform a build action.

The desire is to integrate with VSCode's task infrastructure and have the task infrastructure execute an extension callback rather than having to launch a process.

This way, users can use VSCode's user interface as it is currently designed rather than have to locate our build commands.

[dbaeumer]

@Tyriar, @alexr00 and I had a discussion about that yesterday and we agreed on the following design / API / implementation.

• we add a method resolveTerminalRenderer to ExtHostTerminalService which takes a terminal ID.
• when a custom task is executed the task system sends the terminal ID over to the extension host and that id is then used to resolve a terminal renderer via the new method resolveTerminalRenderer
• the task API will expose a TerminalRenderer interface. If extensions start to call API on the renderer which tasks wants to control (for example show / hide) we can proxy the renderer / terminal and provide our own implementation.

The final custom task API will then look like this:

	/**
	 * Class used to execute an extension callback as a task.
	 */
	export class CustomTaskExecution {
		/**
		 * @param callback The callback that will be called when the task taking this execution is executed.
		 */
		constructor(callback: (renderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable<number | undefined>);
		/**
		 * The callback used to execute the task.
		 */
		callback: (renderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable<number | undefined>;
	}

I also changes the return value to Thenable<number | undefined> to be able to signal an error code from a started process.

[dbaeumer]

@jrieken above a new proposal for the CustomTask API.

[Gama11]

I guess it should be possible to use this API to work around various limitations that problem matchers have, such as #449, #7401, #43664, #55253 and others?

As in, don't provide any problem matchers for the task and use the diagnostics API to report the errors instead. If so, very much looking forward to this. :)

[alexr00]

That's correct, this API will provide a way overcome the limitations of problem matchers. If a shell or process task + a problem matcher can work for you, awesome. They are easier. But, if that's not enough then the idea is that you can use a custom task instead.

[jrieken]

That's correct, this API will provide a way overcome the limitations of problem matchers.

It will overcome those limitations but will also make you implement everything else, e.g launching task processes, manage diagnostics collections, honour various diagnostic customisation like problemMatcher.applyTo etc. etc.

Given the current API, you cannot implement that correctly. Given that, it might be easier for extension authors and better for users to add another API that's for problem matching only something like

interface ProblemMatcher {
  // to be defined
  matchProblem(renderer: TerminalRenderer, diagnostics: DiagnosticsCollection)
}

registerProblemMatcher(taskType, problemMatcher: ProblemMatcher)

[GabeDeBacker]

One comment on the API, perhaps Thenable<number | void> would be better than "undefined"

	/**
	 * Class used to execute an extension callback as a task.
	 */
	export class CustomTaskExecution {
		/**
		 * @param callback The callback that will be called when the task taking this execution is executed.
		 */
		constructor(callback: (renderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable<number | void>);
		/**
		 * The callback used to execute the task.
		 */
		callback: (renderer: TerminalRenderer, cancellationToken: CancellationToken, thisArg?: any) => Thenable<number | void>;
	}

or perhaps use the already defined ProviderResult?

[Tyriar]

@GabeDeBacker I don't see any other instances of using void like that in the API, there are many in the form Thenable<T | undefined> though.

[GabeDeBacker]

I guess what I was thinking about is if there was any case where someone uses this API..

		/**
		 * Executes a task that is managed by VS Code. The returned
		 * task execution can be used to terminate the task.
		 *
		 * @param task the task to execute
		 */
		export function executeTask(task: Task): Thenable<TaskExecution>;

To start a task where the result of the execution is something more complex than a number.

But, from inspection of that API, there is no way for to "wait" for a task execution to complete in VSCode's current API as TaskExecution is returned when the task starts and it only has terminate.

do we forsee any examples where someone might want to retrieve a result from an executing task where it doesn't involve a process launch?

[dbaeumer]

@jrieken full agree. Such an API would be help full on a CustomTaskExecution as well. So may be we should change the callback to

(renderer: TerminalRenderer, context: ExecutionContext, cancellationToken: CancellationToken, thisArg?: any)

Then we can later on add a diagnostics: DiagnosticsCollection property to is like proposed by @jriekne for the ProblemMatcher interface.

[GabeDeBacker]

#66819 - Adding reference to PR

[jrieken]

Then we can later on add a diagnostics: DiagnosticsCollection property to is like proposed by @jriekne for the ProblemMatcher interface.

Adding an empty context and later adding useful properties is hard because you need to ping each and every extension that has already programmed against that API, e.g. in that case an extension author would likely be managing its own collection. Also, passing in the collection doesn't help with extensions being unable to implement some of the things that are defined in tasks.json, like problemMatcher.applyTo.

In yesterdays sync we have concluded that this proposal is only about task execution, not problem detection, presentation-, or run-options. All of that should work just like with other tasks (process/shell execution). @dbaeumer is that what you have in mind?

E.g. a task of type foo will use CustomTaskExecution but it will not be in charge of anything apart from driving the terminal renderer, so replacing a process but not the rest. For instance, given a config like this:

{
    "type": "foo",
    "fooOption2": "42",
    "label": "Hello Foo",
    "presentation": {
        "reveal": "never",
        "focus": false,
    },
    "problemMatcher": {
        "base": "$tsc-watch",
        "owner": "foo-issues",
        "applyTo": "allDocuments"
    },
    "runOptions": {
        "runOn": "folderOpen",
        "reevaluateOnRerun": false
    }
}

The runOptions will be interpreted by the task-framework and we don't ask extensions to *-activate and to start launching tasks, right? Same for presentation options and others. So, the CustomTaskExecution replaces something like tsc --watch, producing output, accepting input but not more.

Then, to enable programmatic problem matching (see #66818 (comment)) we would need another API, an API that is only for problem matching but one that works for any task execution kind.

[siegebell]

This will make launching remote tasks much easier :)

(I currently have ProcessExecution launch a local script that connects to my extension's websocket, which in turn pipes stdio and resize events to a remote process. It's kind of tedious...)

[dbaeumer]

I fully agree that @jrieken example

{
    "type": "foo",
    "fooOption2": "42",
    "label": "Hello Foo",
    "presentation": {
        "reveal": "never",
        "focus": false,
    },
    "problemMatcher": {
        "base": "$tsc-watch",
        "owner": "foo-issues",
        "applyTo": "allDocuments"
    },
    "runOptions": {
        "runOn": "folderOpen",
        "reevaluateOnRerun": false
    }
}

must work for custom tasks out of the box and I see no reason why it shouldn't.

I also agree that the problem matcher issue is an orthogonal issue. However I don't think that we should provide a problem matcher for tasks that are fully defined in the tasks.json. For example I think we don't need support for

{
    "type": "shell",
    "command": "gcc ...."
}

Since it will be hard to identify that task on the extension side.

I was imagining that a custom problem matcher can only be attached to a task created by an extension be it a custom execution, a process execution or a shell execution.

An better API than having the context would be it allow passing in a problem matcher callback with the task constructor. The current constructor looks like this:

constructor(taskDefinition: TaskDefinition, scope: WorkspaceFolder | TaskScope.Global | TaskScope.Workspace, name: string, source: string, execution?: ProcessExecution | ShellExecution, problemMatchers?: string | string[])

and we could change problemMatchers to string | string[] | ProblemMatcher were ProblemMatcher does the actual matching. A possible API would look like this:

class ProblemMatcher {
    constructor(applyTo: ApplyTo, match: (rawData: string, diagnostics: DiagnosticsCollection);
}

This is comparable to #66818 (comment). Difference is that is passes the raw terminal data (including VT sequences) to the matcher instead of the terminal itself.

[dbaeumer]

Using the above approach would even allow to register a problem matcher under a given name that can later on be reference in any task in the tasks.json. That would be in line with how regular expression based matchers are currently contributed via the package.json

[dbaeumer]

To give an example this would look like this:

tasks.registerProblemMatcher(name: string, matcher: ProblemMatcher): void

and the matcher would be referenced like a normal matcher using its name:

{
    "type": "shell",
    "command": "gcc ....",
    "problemMatcher": "$customMatcher"
}

The only thing that will not work is to use such a matcher as a base matcher and change attributes that a regexp specific.

[OmarTawfik]

@alexr00 can you please tell me if this will be part of March's release?

[alexr00]

This has been merged in to master! The API is available as a proposed API and should be part of the March release.

[Tyriar]

The API is available as a proposed API and should be part of the March release.

@OmarTawfik but will almost certainly change, see #67923 (comment)

[OmarTawfik]

@Tyriar my use cases will all either do UI-based operations, or invoke online APIs that has nothing to do with a terminal. So I just need a:

1. a callback API that returns a Promise<void>
2. When resolved/rejected, VSCode API can treat the result as a successful/failed task.
3. It will show up correctly in the tasks StatusBarItem, in the Show Running Tasks command, etc..
4. My extension would still be responsible for extra UI operations/notifying users with progress.

Does that still hold? I see nothing in your comment that changes that behavior.

[alexr00]

@OmarTawfik you should still be able to acomplish that with the callback task API. However, the API surface will change and anyone who uses the proposed API will need to adapt to the change.