Terminal quick fix API

[Tyriar]

Forking this off from #145234 as that one is more focused on listening for and reacting to commands generally, not quick fixes.

[Tyriar]

@meganrogge responding to #145234 (comment)

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:

vscode/src/vscode-dts/vscode.d.ts

Lines 6757 to 6776 in ae49262

	/**
	* A provider that enables detection and handling of links within terminals.
	*/
	export interface TerminalLinkProvider<T extends TerminalLink = TerminalLink> {
	/**
	* Provide terminal links for the given context. Note that this can be called multiple times
	* even before previous calls resolve, make sure to not share global objects (eg. `RegExp`)
	* that could have problems when asynchronous usage may overlap.
	* @param context Information about what links are being provided for.
	* @param token A cancellation token.
	* @return A list of terminal links for the given line.
	*/
	provideTerminalLinks(context: TerminalLinkContext, token: CancellationToken): ProviderResult<T[]>;
	/**
	* Handle an activated terminal link.
	* @param link The link to handle.
	*/
	handleTerminalLink(link: T): ProviderResult<void>;
	}

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 2fdae24 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.

[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 #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:

vscode/src/vs/workbench/contrib/terminalContrib/quickFix/browser/terminalQuickFixBuiltinActions.ts

Lines 233 to 348 in 0e16324

	export function pwshGeneralError(): ITerminalQuickFixInternalOptions {
	return {
	id: 'Pwsh General Error',
	type: 'internal',
	commandLineMatcher: /.+/,
	outputMatcher: {
	lineMatcher: PwshGeneralErrorOutputRegex,
	anchor: 'bottom',
	offset: 0,
	length: 10
	},
	commandExitResult: 'error',
	getQuickFixes: (matchResult: ITerminalCommandMatchResult) => {
	const lines = matchResult.outputMatch?.regexMatch.input?.split('\n');
	if (!lines) {
	return;
	}
	// Find the start
	let i = 0;
	let inFeedbackProvider = false;
	for (; i < lines.length; i++) {
	if (lines[i].match(PwshGeneralErrorOutputRegex)) {
	inFeedbackProvider = true;
	break;
	}
	}
	if (!inFeedbackProvider) {
	return;
	}
	const suggestions = lines[i + 1].match(/The most similar commands are: (?<values>.+)./)?.groups?.values?.split(', ');
	if (!suggestions) {
	return;
	}
	const result: ITerminalQuickFixTerminalCommandAction[] = [];
	for (const suggestion of suggestions) {
	result.push({
	id: 'Pwsh General Error',
	type: TerminalQuickFixType.TerminalCommand,
	terminalCommand: suggestion,
	source: QuickFixSource.Builtin
	});
	}
	return result;
	}
	};
	}
	export function pwshUnixCommandNotFoundError(): ITerminalQuickFixInternalOptions {
	return {
	id: 'Unix Command Not Found',
	type: 'internal',
	commandLineMatcher: /.+/,
	outputMatcher: {
	lineMatcher: PwshUnixCommandNotFoundErrorOutputRegex,
	anchor: 'bottom',
	offset: 0,
	length: 10
	},
	commandExitResult: 'error',
	getQuickFixes: (matchResult: ITerminalCommandMatchResult) => {
	const lines = matchResult.outputMatch?.regexMatch.input?.split('\n');
	if (!lines) {
	return;
	}
	// Find the start
	let i = 0;
	let inFeedbackProvider = false;
	for (; i < lines.length; i++) {
	if (lines[i].match(PwshUnixCommandNotFoundErrorOutputRegex)) {
	inFeedbackProvider = true;
	break;
	}
	}
	if (!inFeedbackProvider) {
	return;
	}
	// Always remove the first element as it's the "Suggestion [cmd-not-found]"" line
	const result: ITerminalQuickFixTerminalCommandAction[] = [];
	let inSuggestions = false;
	for (; i < lines.length; i++) {
	const line = lines[i].trim();
	if (line.length === 0) {
	break;
	}
	const installCommand = line.match(/You also have .+ installed, you can run '(?<command>.+)' instead./)?.groups?.command;
	if (installCommand) {
	result.push({
	id: 'Pwsh Unix Command Not Found Error',
	type: TerminalQuickFixType.TerminalCommand,
	terminalCommand: installCommand,
	source: QuickFixSource.Builtin
	});
	inSuggestions = false;
	continue;
	}
	if (line.match(/Command '.+' not found, but can be installed with:/)) {
	inSuggestions = true;
	continue;
	}
	if (inSuggestions) {
	result.push({
	id: 'Pwsh Unix Command Not Found Error',
	type: TerminalQuickFixType.TerminalCommand,
	terminalCommand: line.trim(),
	source: QuickFixSource.Builtin
	});
	}
	}
	return result;
	}
	};
	}

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 👍