Terminal Providers

[IlyaBiryukov]

Issue Type: Feature Request

Enable extensions provide custom terminals to other extensions and tasks

Today extensions and tasks use vscode's API vscode.window.createTerminal(terminalOptions) to create a new terminal window, and some other APIs to render task output as terminal output.
Our extension can change the way a terminal is started, and we want user to be able to apply this change to other terminals and tasks vscode may be running without changing their code.

The idea is to introduce a concept of Terminal Provider, an object that will be chained when something starts a new terminal:

interface TerminalProvider {
    createTerminal(options: TerminalOptions, provider: TerminalProvider): Promise<Terminal>;
}

// A way to register a terminal provider. This function returns a disposable
// which, when disposed, unregisters the provider.
window.registerTerminalProvider(provider: TerminalProvider): Disposable;

When a provider is registered, it will be called when anyone calls window.createTerminal(...). The provider argument in createTerminal is the previous chained provider, or internal vscode implementation that actually creates a terminal.

Registering or un-registering a provider should not affect existing terminals.

[dbaeumer]

As @Tyriar pointed out in mail this is something we discussed this week as well, especially in the use case of tasks. I would like to suggest a different form of API instead of a terminal provider mainly because it will be unclear if two extensions register such a terminal provider. (side note: I looked into a task execution provider for a comparable use case this milestone and couldn't find a nice working solution there either).

I would like to see three different sets of API around terminals that extensions then can use to control how a terminal is functioning:

• events about the existence of a terminal process (e.g. the node-pty process in VS Code). The events would provide access to stdout and stdin of that process either via streams or Event.
• API to create a terminal renderer without spawning a underlying terminal process (eg. node-pty processs). The returned renderer would offer methods to write and read from the terminal and events when it gets resized. Not sure if we need special handling for Ctrl+C or other Control characters.
• API to create a terminal process (e.g. the node-pty process) and access to its stdin and stdout stream. Here I am not sure if this is really necessary or if we ask developers to simply use child_process functions. Providing API ensure that for node-pty we can reuse our tested version of node-pty.

Combining these three API would allow to implement a TerminalProvider concept.

[Tyriar]

Not sure if we need special handling for Ctrl+C or other Control characters.

These would be translated by automatically and passed through the read event in this case.

API to create a terminal process

child_process is insufficient as it doesn't have dimensions.

[IlyaBiryukov]

@dbaeumer The main reason for this feature request for us is to be able to "hijack" or "substitute" a terminal that other extensions would start without changing their code. They use vscode.window.createTerminal. The terminal provider registration should be able to affect how vscode.window.createTerminal works.

[dbaeumer]

@IlyaBiryukov but your provider is a one of solution. If a second extensions wants to do this it either can't or replaces your hijack terminal in a way that your provider is lost. The proposed API would allow for equal participation on the output / input stream. This allows for hijacking the input and output only but to my understanding this should be ok in the use cases I am aware of.

[IlyaBiryukov]

@dbaeumer My idea was to form a chain of providers, when they fire one-by-one, and and each pass vscode.TerminalOptions to the next in line. So registering a new provider won't replace or hide previous ones, it'll just add another step in the processing.
That being said, I'm not opposed to having more granular control on terminals and the ability to hijack input and output streams.

[dbaeumer]

@IlyaBiryukov make sense.

[Tyriar]

@IlyaBiryukov this is what we've come up with so far, I'm interested in your thoughts:

export namespace window {
    // A Terminal owns a process and a renderer
    export interface Terminal {
        // This is unchanged
        readonly name: string;

        // processId not needs to also return undefined when there is no backing
        // pid (TerminalRenderer)
        readonly processId: Thenable<number | undefined>;

        // Setting to undefined will reset to use the maximum available
        dimensions: TerminalDimensions;

        // Send text to process
        sendText(data: string): void; // in

        // Data from the process
        onData: Event<string>; // out

        // Fires when the panel area is resized, this DOES NOT fire when `dimensions` is set
        onDidChangeDimensions: Event<TerminalDimensions>;
    }

    // Creates and returns an empty TerminalRenderer
    export function createTerminalRenderer(name: string): TerminalRenderer;
    
    // A TerminalRender does not own a process, it's similar to an output window
    // but it understands ANSI
    export interface TerminalRenderer {
        // Extensions can set the name (what appears in the dropdown)
        name: string;

        // Setting to undefined will reset to use the maximum available
        dimensions: TerminalDimensions;

        // Write to xterm.js
        write(data: string): void; // out

        // key press, or sendText was triggered by an extension on the terminal
        onData: Event<string>; // in

        // Fires when the panel area is resized, this DOES NOT fire when `dimensions` is set
        onDidChangeDimensions: Event<TerminalDimensions>;
    }

    export interface TerminalDimensions {
        cols: number;
        rows: number;
    }

    // Allows listening to terminal creation events, when a TerminalRenderer is
    // created in this manner it uses Terminal as a facade
    export function onDidOpenTerminal: Event<Terminal>;

    export let terminals: Terminal[];

    // Do not allow interception/cancellation of terminals, we do not want to
    // encourage extensions to do this.
}

An example extension: When a terminal is created it creates a another terminal which mirrors it and accepts input:

let wasMirrored = false;
const linkedTerminals: {[terminal: Terminal]: TerminalRenderer} = {};

onDidCreateTerminal(terminal => {
    if (wasMirrored) {
        // Ignore events that came from mirrored terminals
        wasMirrored = false;
    } else {
        wasMirrored = true;
        linkedTerminals[terminal] = window.createTerminalRenderer();
        setupListeners(terminal, linkedTerminals[terminal]);
    }
});

function setupListeners(master: Terminal, slave: TerminalRenderer) {
    // Send print instructions to slave
    master.onData(data => slave.write(data));
    //  Redirect slave input to master
    slave.onData(data => master.sendText(data));
}

[Tyriar]

I updated inline with feedback from @jrieken:

• onResize, onData is an event
• Size functions now work on TerminalDimensions
• name is a mandatory createTerminalRenderer parameter

[Tyriar]

Added terminals: Terminal[] for completeness/consistency with rest of API (from #13267).

[IlyaBiryukov]

@Tyriar, here is my feedback on the proposed API:

1. How one can figure out the actual size allowed by the panel (the maximum for the terminal dimensions allowed by setSize)? Can this maximum sizes change over time, e.g. when user resizes the window?
2. If something wants to create a terminal, the way to intercept it is to listen to onDidCreateTerminal and then create terminal renderer? How will VSCode know that it doesn't need to create its own renderer?
3. About this:
   // Do not allow interception/cancellation of terminal
   Isn't onDidCreateTerminal a way to intercept creation of a terminal?
4. About createTerminalRenderer, what about specifying the dimensions, and whether the renderer has fixed dimensions or fit the window?
5. About TerminalRenderer interface, how one can close it? Say that underlying terminal has closed, how the renderer would know that and close itself?
6. The example extension, will it only work for the first terminal, and then will stop working because wasMirrored will become true?
7. Are Terminal and TerminalRenderer persistent objects, can === be used on them?
8. Why processId is needed on Terminal?
9. For Terminal object, is there a way to query its current dimensions?
10. Can 'onData' event handler block the terminal from producing more data? The scenario is that the consumers can be slow to render the data, and the terminal may be producing lots of data fast. Can onData block the terminal from producing more data? Considering that there may be more than one handler there (e.g. a natural handler, and the one that broadcasts the data to other participants).
11. For 'TerminalRenderer', and 'Terminal' we will need onDidClose event to know when they're gone.

[Tyriar]

@IlyaBiryukov

1. onResize will fire any time the window/panel is resized such that the terminal cols/rows change. So you can do this by tracking these onResize events.

2. How will VSCode know that it doesn't need to create its own renderer?

   Don't totally understand this, a single terminal in the dropdown can either be backed by a single Terminal (which has a process) or a single TerminalRenderer (which does not), not both.

3. The comment is a little ambiguous, you can listen to terminal creation but you cannot stop it as the team doesn't want to encourage extensions to do this.

4. You could do this with:

   const term = createTerminalRenderer('me');
   term.setSize(x, y);

5. Good question, it should probably also have dispose.

6. wasMirrored was there so the creation events would be ignored and stop a stack overflow.

7. No you cannot compare them and there is no reliable way to do this.

8. Terminal.processId already exists, the change is to add undefined when the Terminal is backed only by a TerminalRenderer.

9. No, see 1

10. I think this is fine from your perspective, we can go into more detail in email.

11. This already exists, but I'm beginning to feel we need some way of comparing Terminal and TerminalRenderer though.

[DanTup]

Could this help with #46696? Would it allow us to manipulate the PATH being set for a terminal from an extension?

[dbaeumer]

@weinand FYI.

[Tyriar]

@DanTup no you cannot manipulate a terminal's environment that was launched (onDidCreateTerminal happened after it was already created).

[DanTup]

Understood. Though maybe this pattern would work there - if this was extended to have an onWillCreateTerminal that could modify the environment, it might fix that issue? (I don't want to derail this though, so if it sounds reasonable I could add comments to the other issue suggesting it?)

[alpaix]

@Tyriar Thanks for pushing this change. I tested it by running the terminal-sample extension with the latest VSCode insiders. It works well for interactive terminals. Unfortunately, it seems to fail to capture output from a task terminal. Is there any limitation that prevents certain v2.0 tasks to be handled correctly.

[Tyriar]

@alpaix the extension command works for me on VS Code's build task:

[alpaix]

I tried running it on my Windows desktop. I have a project with the following task:

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "build",
            "type": "shell",
            "command": "npm",
            "args": [
                "run",
                "compile"
            ],
            "problemMatcher": "$tsc-watch",
            "group": "build",
            "presentation": {
                "echo": true,
                "reveal": "always",
                "focus": false,
                "panel": "shared"
            }
        }
    ]
}

Then I invoked Terminal API: Attach data listener command and executed the task. No data have been passed to the listener.

At the same time, it worked perfectly well when I attached a listener to a regular Powershell terminal.

[Tyriar]

@alpaix you need to use onData on the specific Terminal, which can only be done after you have started the task.

[alpaix]

The Attach command asks for a concrete terminal instance to get attached to. So it does find the task terminal instance and adds a listener to the right terminal.

I tweaked the sample extension to attach the listener to every new terminal:

// vscode.window.onDidOpenTerminal
	if ('onDidOpenTerminal' in vscode.window) {
		(<any>vscode.window).onDidOpenTerminal((terminal: vscode.Terminal) => {
			vscode.window.showInformationMessage(`onDidOpenTerminal, name: ${terminal.name}`);
			(<any>terminal).onData((data: string) => {
				console.log('onData: ' + data);
			});
		});
	}

After this change, it started getting the data.

Does the implementation expect listeners to get attached within a short period of time after the terminal instance is created? Based on my observation, a listener attached later simply won't get any data,

[Tyriar]

@alpaix it should work later as well. When onData is called the extension host tells the renderer process to start sending data events:

https://github.com/Microsoft/vscode/blob/e5f33576004c4c2bf07af3a5661d47df446468c0/src/vs/workbench/api/electron-browser/mainThreadTerminalService.ts#L91-L97

It should fire the data event until the terminal instance is disposed/exited.

[dbaeumer]

I think @alpaix is asking what happpens to data that is already present in the terminal. Will it be sent as well or will it be lost in terms of events.

[Tyriar]

Does the implementation expect listeners to get attached within a short period of time after the terminal instance is created? Based on my observation, a listener attached later simply won't get any data,

@alpaix yes this is the case.

[Tyriar]

@IlyaBiryukov so this is done, check the proposed API file for the latest:

https://github.com/Microsoft/vscode/blob/442e5e202a89c1f8efe642c89c0554cab1f15a9e/src/vs/vscode.proposed.d.ts#L362-L383

I made a few tweaks on the initial design, namely:

• TerminalRenderer.dimensions is only the "overridden" dimensions
• TerminalRenderer.maximumDimensions always represents the maximum dimensions
• TerminalRenderer.onDidChangeMaximumDimensions fires whenever maximumDimensions is updated
• TerminalRenderer.terminal is a convenience property to fetch the underlying Terminal, I felt this was needed as it was super inconvenient to call Terminal.show for example and didn't want to duplicate things across the 2 interfaces. Note that this is a Thenable<Terminal>
• TerminalRenderer.onData was renamed to onInput to make it more obvious that it's different to Terminal.onData
• Dimensions are not exposed on Terminal as I only want the creator of the TerminalRenderer to change this

Any feedback on this would be great 😃. I've added pretty extensive doc comments so they're worth a read before trying it out.

[IlyaBiryukov]

@Tyriar Excellent news! I like the new API very much and look forward to playing with it. Will it be shipping in the next insiders update?

[Tyriar]

@IlyaBiryukov I think it's already in the latest, if not it will be in tomorrow's.

[Tyriar]

@IlyaBiryukov also the samples I was working against while developing are in this commit: microsoft/vscode-extension-samples@5f23d63

[IlyaBiryukov]

@Tyriar is the TerminalRenderer API stable in insiders build 1.25.0, or there will be more changes?
I adopted the terminal renderer APIs from that build and everything works like a charm.