Terminal quick fix API

[Tyriar]

Forking this off from https://github.com/microsoft/vscode/issues/145234 as that one is more focused on listening for and reacting to commands generally, not quick fixes.

[Tyriar]

@meganrogge responding to https://github.com/microsoft/vscode/issues/145234#issuecomment-1269100829

registerQuickFixProvider(...options: ITerminalQuickFixOptions[]): void;

Should be: registerTerminalQuickFixProvider and return a Disposable

Also the pattern in the API is to just accept a single object (see registerTerminalLinkProvider, registerTerminalProfileProvider)

export type TerminalQuickFixAction = IAction | ITerminalQuickFixCommandAction | ITerminalQuickFixOpenerAction;

IAction, ITerminalCommand, etc. isn't available in the extension API.

/**
 * The number of rows to match against, this should be as small as possible for performance
 * reasons.
 */
length: number;

We'll need to be more restrictive here, beyond just recommending a low number

addNewLine: boolean;

Should we always execute unless alt is held?

TerminalQuickFixAction[] | TerminalQuickFixAction | undefined

ProviderResult instead of T[] | T | undefined

registerQuickFixProvider(...options: ITerminalQuickFixOptions[]): void;

We will want to register a provider interface, not just options, see TerminalLinkProvider for inspiration:

https://github.com/microsoft/vscode/blob/ae4926234561a8619c5359d9574b3dd9e6153f53/src/vscode-dts/vscode.d.ts#L6757-L6776

Also consider activation events and whether we should be declaring the matchers in the package.json to trigger activation?

[Tyriar]

Reopening as it's just a proposal atm

[zardoy]

@Tyriar just curios. There is API in title, but it seems to me that pr implements contribution point instead. Will API like https://github.com/microsoft/vscode/commit/2fdae24fde11c0cb74c683c04a9ea8a28a4e27db happen in future? (In my case I can provide quickfixes only generating them dynamically)

[meganrogge]

We talked about this in the API sync and think the provider approach would be better because we can:

1. Cancel if it's taking too long to resolve the quick fixes from the provider
2. Since the quick fixes will be handled in the extension host, it shouldn't impact the renderer performance
3. The contribution point is too constrictive - for example, the git similar command cannot be supported using this model

[Tyriar]

@zardoy what is your use case where you can only generate them dynamically?

[Tyriar]

Since the quick fixes will be handled in the extension host, it shouldn't impact the renderer performance

It would be evaluated in the renderer though. The initial regex evaluation will be on the renderer so we can avoid sending all terminal data to the ext host.

The contribution point is too constrictive - for example, the git similar command cannot be supported using this model

I think git similar should work fine.

[zardoy]

@Tyriar thank you so much for the question! I must admit git similar quick fixes can be done via static contributions as Git already suggests similar command and this is not fair for some other CLIs that don't do the same thing. In my case I think I can provide quickfixes for a lot of clis using fig autocomplete knowledge. We already have extension that can successfully warn you in case of unknown cli option via diagnostic. Also it does suggest a similar valid option (since we know all valid options for supported clis). We can do the same analysis when supported cli hits an output like error: unknown option '--option' (which is common for commander). Example:

> vsce publish --no-depednencies
error: unknown option '--no-depednencies'

Here we could provide a quickfix to use replace this option with --no-dependencies instead.

Also, as I understand even with current static contribution model its not possible to implement Git Two Slash fix (as it programmatically replaces content in command to preserve other options)

Please, ping me if some parts were unclear to you!

[Tyriar]

@zardoy thanks for the context, neat idea 🙂

Also, as I understand even with current static contribution model its not possible to implement Git Two Slash fix (as it programmatically replaces content in command to preserve other options)

You're right, that one won't be possible.

[meganrogge]

this is still a proposal, so reopening

[Tyriar]

Current proposal:

https://github.com/microsoft/vscode/blob/ea490e5545df658e05d0332bfb2a04435a0ef7f0/src/vscode-dts/vscode.proposed.terminalQuickFixProvider.d.ts#L8-L83

For copilot I've found we need to also be able to run VS Code commands in order to evaluate what to actually run lazily, and potentially gate it behind a confirmation. So a fix with copilot would open chat or an inline view with a suggestion. Something like this will be needed to do that:

    export interface TerminalQuickFixProvider {
        /**
         * Provides terminal quick fixes
         * @param commandMatchResult The command match result for which to provide quick fixes
         * @param token A cancellation token indicating the result is no longer needed
         * @return Terminal quick fix(es) if any
         */
        provideTerminalQuickFixes(commandMatchResult: TerminalCommandMatchResult, token: CancellationToken): 
            ProviderResult<
                (TerminalQuickFixCommand | TerminalQuickFixOpener | TerminalQuickFixVscodeCommand)[]
                |
                TerminalQuickFixCommand | TerminalQuickFixOpener | TerminalQuickFixVscodeCommand
            >;
    }

    export class TerminalQuickFixVscodeCommand {
        /**
         * The (vscode) command to run
         */
        command: string;
        constructor(command: string);
    }

    enum TerminalQuickFixType {
        Command = 0,
        Opener = 1,
        VscodeCommand = 2
    }

I'm not sure right now what the best way to clearly differentiate a terminal command and VS Code command is.

[Tyriar]

Another piece of feedback from myself was it felt quite awkward to implement a match all failing commands matcher as outputMatcher must be specified. This was sort of intentional as we don't generally want users to do this, but since you can do it anyway it's just getting in the way?

[Tyriar]

Feedback from API sync:

• Use "TerminalCommand" everywhere that talks about terminal commands (not vscode commands)
• Use Command for a VS Code command, this could be returned directly from provideTerminalQuickFixes - this also solves the issue of include title and args

[Tyriar]

Incorporating changes above, that would make it:

    export interface TerminalQuickFixProvider {
        /**
         * Provides terminal quick fixes
         * @param commandMatchResult The command match result for which to provide quick fixes
         * @param token A cancellation token indicating the result is no longer needed
         * @return Terminal quick fix(es) if any
         */
        provideTerminalQuickFixes(commandMatchResult: TerminalCommandMatchResult, token: CancellationToken): 
            ProviderResult<
                (TerminalQuickFixTerminalCommand | TerminalQuickFixOpener | Command)[]
                |
                TerminalQuickFixTerminalCommand | TerminalQuickFixOpener | Command
            >;
    }

Where TerminalQuickFixCommand has been renamed to TerminalQuickFixTerminalCommand

[Tyriar]

Topics for API sync:

• New return shape with Command and TerminalQuickFixExecuteTerminalCommand
• Make outputMatch contribution optional, even though it could lead to accidents?

[Tyriar]

Feedback:

• Move kind to quick fix object, wrap Command so this metadata fits
• Rename "explain" to "chat" for now

[BrunoRDS]

Hello,

Looking to use this API in an internal-use extension (not published to market). It's reliably working for us in our use-cases in testing.

Needing to have all interested parties daily drive vscode insiders is a blocker for us in deploying this extension.

What is the current plan for finalizing this proposed API?

Thanks @Tyriar & all involved.

[Tyriar]

@BrunoRDS surprisingly to me, we haven't had any feedback yet so there's been no pressure to stabilize. Additionally, there were some recently tweaks to the API for the copilot extension.

[JustinGrote]

@Tyriar this is great, I only found out about this today. For the PowerShell extension, there is a new Feedback Provider mechanism in PowerShell 7.4 for suggestions to be surfaced for errors, command not found, or other scenarios. I would love to be able to do the following (a combination of the other API and this one):

1. Contribute to this status page, such as showing execution or timing/profiling information, as well as info from Feedback Providers. Being able to contribute should also allow the status icon to be influenced (for instance, add a warning or error marker rather than just what the shell integration detects)
2. Contributing Quick Fixes to rewrite a command based on PSScriptAnalyzer Rules. In PowerShell's case we control our own integrated terminal most of the time but obviously this would ideally clear the buffer and rewrite based on the output.

Thanks for the work!

[Tyriar]

Contribute to this status page, such as showing execution or timing/profiling information, as well as info from Feedback Providers.

By status page you mean the blue dot? I want to add a timestamp/duration at some point as it's trivial for us to get, created https://github.com/microsoft/vscode/issues/199170 to track that. I'm not sure if it's worth it to expose an API to allow extensions to add info there given that?

Being able to contribute should also allow the status icon to be influenced (for instance, add a warning or error marker rather than just what the shell integration detects)

To clarify, there are two things here; the "shell integration decoration" (blue or red circle) and the quick fix lightbulb/sparkle:

So you want to react to a command finishing and show a warning icon where the blue dot is to mean successful with warnings? I don't think we've considered a way to do something like this yet where an extension can change something about the command result (quick fixes provide actions to act upon a result, not change it).

Contributing Quick Fixes to rewrite a command based on PSScriptAnalyzer Rules

We have a couple of built-in quick fix providers that deal with the feedback providers:

https://github.com/microsoft/vscode/blob/0e16324eff7e45a43223f9c11dafa1b3768d3174/src/vs/workbench/contrib/terminalContrib/quickFix/browser/terminalQuickFixBuiltinActions.ts#L233-L348

Unfortunately there is not a standard format, at least when I was working on it, so we can't have a general catch all atm.

For this do you mean running the command through PSScriptAnalyzer for the project and presenting a quick fix if it differs? Is this just a lint rule that isn't that important, or do you think it justifies a "high confidence" quick fix?

[connor4312]

I think it'd be useful to provide the terminal on the TerminalCommandMatchResult. For a quick fix provider I'm working on, I would like to get the working directory of the terminal, which is accessible via the shellIntergation property. At the moment I just assume that the active terminal is the one the quick fix is being provided for.

[Tyriar]

@connor4312 sounds a lot more useful now that shellIntegration is there 👍