Vscode: Allow extensions to contribute links to the terminal (aka link providers)

[Tyriar]

Currently the way terminal links work is by waiting until the viewport has stopped updating and then parsing the entire viewport. This has a few problems, most notable the performance cost of validating this every time (particularly bad on some environments, terminal.integrated.enableFileLinks was added to allow people to disable it) and this awfulness:

https://github.com/microsoft/vscode/blob/f6bd0bac9b1ed4d207772ae99ba72a1755beb4c6/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts#L40-L47

xterm.js just recently merged a new link provider API that allows moving away from this regex approach and instead resolving links when a hover happens. The PR to adopt this API is here https://github.com/microsoft/vscode/pull/90336.

@connor4312 and I just wrote up this proposal as a starting point for allowing extensions to leverage this API. This has some really cool possibilities:

• Language-specific handling, for example Node.js stack traces allowing to ctrl+click directly to node sources (eg. at Script.runInThisContext (vm.js:96:20))
• Compiler-specific line/col formatting instead of core having to add each one explicitly
• Auto-correcting CLIs based on suggestions
• Handling of file links not relative to the initial cwd (this will probably be in core eventually)
• Live share forwarding ports (instead of parsing all terminal output that it does now)

  interface LinkHoverContext {
    line: string;
    index: number;
    terminal: Terminal;
  }

  export interface TerminalLinkProvider {
    provideTerminalLink(context: LinkHoverContext): ProviderResult<TerminalLink>

    /**
     * Optionally provides custom handling logic for links returned from this
     * provider. This method can mutate the link `target`, or handle the
     * link internally. If a link is returned from this method, then VS
     * Code will try to open it using its default logic.
     */
    handleTerminalLink?(link: ResolvedTerminalLink): ProviderResult<ResolvedTerminalLink>;
  }

  interface TerminalLink {
    startIndex: number;
    length: number;

    /**
     * The uri this link points to. If set, and {@link TerminalLinkProvider.handlerTerminalLink}
     * is not implemented or returns false, then VS Code will try to open the Uri.
     */
    target?: Uri;

    /**
     * The tooltip text when you hover over this link.
     *
     * If a tooltip is provided, is will be displayed in a string that includes instructions on how to
     * trigger the link, such as `{0} (ctrl + click)`. The specific instructions vary depending on OS,
     * user settings, and localization.
     */
    tooltip?: string;
  }

  interface ResolvedTerminalLink {
    /**
     * The link text from the terminal, derived from the startIndex and length
     * returned in {@link TerminalLink}.
     */
    text: string;

    /**
     * The uri this link points to. If set in the link returned from {@link TerminalLinkProvider.handlerTerminalLink},
     * then VS Code will try to open it.
     */
    target?: Uri;

    /**
     * The tooltip text when you hover over this link.
     *
     * If a tooltip is provided, is will be displayed in a string that includes instructions on how to
     * trigger the link, such as `{0} (ctrl + click)`. The specific instructions vary depending on OS,
     * user settings, and localization.
     */
    tooltip?: string;
  }

Some notes/questions/unknowns:

• How to handle conflicting links?
  
  
  
  • It looks like DocumentLinkProvider just overwrites links based on the most recently registered provider
  
  
  • Advise that implementers try match links as precisely/narrowly as possible
  
  
• How would it work if Live Share used this API and js-debug did too?
  
  
  
  • Don't have an answer, the order being potentially inconsistent (based on activation time) is one problem. If it were consistent, js-debug could call into Live Share to get it to do the forwarding ahead of time, not ideal though.
  
  
• Should this be generic to documents as well?
  
  
  
  • Since DocumentLinkProvider gets the links for the entire file this is slow for really large files and it does not allow validating links. An example of this is the GitHub issue links where #999999 would get a link that fails to open.
  
  
  • Files deal with ranges, the terminal doesn't
  
  
  • The downside of doing this is that we cannot show underlines until after hovering. The terminal never did this anyway as it would conflict with terminal underline attributes.
  
  

[Tyriar]

@alexr00 this could be useful for GH:

I just clicked it instinctively even though I knew it was going to fail 😄

[Tyriar]

The new link system now lets you set hover labels, this will very likely come to the link provider API. eg. "Open on GitHub", "Open commit with in GitLens", "Launch debug browser".

[Tyriar]

Unknown: How do link providers conflicts resolve? If you have GitLens and GitHub PR installed and click on a commit, who wins? Should we show both in the hover in this case and ctrl+click lets you select the default/priority?

[jmbockhorst]

Yeah, I think it should be left up to the user. Showing both providers on the hover popup could work well. Ctrl+click without a default set would force the user to use the popup and choose one. Maybe also a dialog when they click a default for the first time, something like: "Do you want to make this the default link handler for this type of link?". And also there should be a way to change/reset the defaults. This might be able to be directly on the hover popup (For Example: Highlight the default provider with an "x" to reset it), as long it isn't too cluttered.

[Tyriar]

This might be able to be directly on the hover popup

I like this idea:

Open on GitHub (make default)
Open commit (ctrl + click)

That way you can action it both ways, change the click behavior right in the hover. I guess behind the scenes we'd just track the decision that extension A beats out extension B.

[Tyriar]

If link providers has some additional context of the buffer they could support links like this.

Git diff:

eslint error

The tricky thing is how much context is it, and how does the extension access it?

[Tyriar]

Current xterm API:

https://github.com/xtermjs/xterm.js/blob/1168c6c52f49c7155094fe173ac037ea0e9ce8ac/typings/xterm.d.ts#L752-L758

This will change to this soon to avoid unconsistent links based on what is hovered first:

  interface ILinkProvider {
    provideLinks(position: IBufferCellPosition, callback: (links: ILink[] | undefined) => void): void;
  }

https://github.com/xtermjs/xterm.js/blob/1168c6c52f49c7155094fe173ac037ea0e9ce8ac/typings/xterm.d.ts#L1097-L1168

[Tyriar]

Idea: a contextLine which is a regex that will be looked at above the line to provide context. Example for the git and eslint examples about are /^+++/ and /^(?! )/ (might be wrong syntax, first line that doesn't start with 2 spaces).

[Tyriar]

New proposal with a search context feature:

export interface TerminalLinkSearchContext {
    /**
     * A regular expression that will be searched upwards to give additional context of the
     * link. This is useful for example when a linter/compiler prints a file name that contains
     * errors and then all the errors on lines below it.
     */
    searchContextPattern?: RegExp;

    /**
     * The number of lines to search upwards for the regexp pattern. This defaults to 5 and will
     * never search beyond 100 lines.
     */
    searchContextLimit?: number;
}

export interface TerminalLinkProviderOptions {
    searchContext?: TerminalLinkSearchContext;
}

export namespace window {
    export function registerTerminalLinkProvider(provider: TerminalLinkProvider, options?: TerminalLinkProviderOptions): Disposable;
}

export interface TerminalLinkContext {
    /**
     * This is the text from the unwrapped line in the terminal.
     */
    line: string;

    /**
     * The terminal the link belongs to.
     */
    terminal: Terminal;

    /**
     * An additional context line if a [TerminalLinkSearchContext](#TerminalLinkSearchContext)
     * was used when registering the link provider.
     */
    searchContextLine?: string;
}

export interface TerminalLinkProvider<T = Uri> {
    provideTerminalLinks(context: TerminalLinkContext): ProviderResult<TerminalLink<T>[]>

    /**
     * Handle an activated terminal link.
     * 
     * @returns an optional Uri, if provided VS Code will attempt to open it.
     */
    handleTerminalLink(link: ResolvedTerminalLink<T>): ProviderResult<Uri>;
}

export interface TerminalLink<T = Uri> {
    /**
     * The start index of the link on [TerminalLinkContext.line](#TerminalLinkContext.line].
     */
    startIndex: number;

    /**
     * The length of the link on [TerminalLinkContext.line](#TerminalLinkContext.line]
     */
    length: number;

    /**
     * The uri this link points to. If set, and {@link TerminalLinkProvider.handlerTerminalLink}
     * is not implemented or returns false, then VS Code will try to open the Uri.
     */
    target?: T;

    /**
     * The tooltip text when you hover over this link.
     *
     * If a tooltip is provided, is will be displayed in a string that includes instructions on how to
     * trigger the link, such as `{0} (ctrl + click)`. The specific instructions vary depending on OS,
     * user settings, and localization.
     */
    tooltip?: string;
}

export interface ResolvedTerminalLink<T = Uri> {
    /**
     * The link text from the terminal, derived from the startIndex and length returned in
     * [TerminalLink](#TerminalLink).
     */
    text: string;

    /**
     * The uri this link points to. If set in the link returned from
     * [handlerTerminalLink](#TerminalLinkProvider.handlerTerminalLink), then VS Code will try to open it.
     */
    target?: T;

    /**
     * The tooltip text when you hover over this link.
     *
     * If a tooltip is provided, is will be displayed in a string that includes instructions on how to
     * trigger the link, such as `{0} (ctrl + click)`. The specific instructions vary depending on OS,
     * user settings, and localization.
     */
    tooltip?: string;
}

Examples below, they may not be exactly right but they demonstrate the idea:

// Simple link provider that turns the whole line into a Uri link
const provider1: TerminalLinkProvider = {
    provideTerminalLinks(context) {
        const links = [{
            startIndex: 0,
            length: context.line.length,
            target: Uri.file(context.line)
        }];
        return links;
    },
    handleTerminalLink(link) {
        return link.target;
    }
};
window.registerTerminalLinkProvider(provider1);

// Match github issue references
const provider2: TerminalLinkProvider = {
    provideTerminalLinks(context) {
        const links = [];
        const matches = context.line.match(/#\d+/g);
        matches.forEach(m => {
            // Outside helper that could check for example that the number is < max issue/PR number
            const issueNumber = m.substr(1);
            if (isValidIssueReference(issueNumber)) {
                links.push({
                    startIndex = ...;
                    length = ...;
                    target = Uri.from(`https://github.com/${user}/${repo}/issues/${issueNumber}`);
                });
            }
        });
        return links;
    },
    handleTerminalLink(link) {
        return link.target;
    }
}
window.registerTerminalLinkProvider(provider2);

// Match git diff links, for example:
// ```
// diff --git a/src/vs/editor/contrib/anchorSelect/anchorSelect.ts b/src/vs/editor/contrib/anchorSelect/anchorSelect.ts
// index 8a422cddbb..19fc827161 100644
// --- a/src/vs/editor/contrib/anchorSelect/anchorSelect.ts
// +++ b/src/vs/editor/contrib/anchorSelect/anchorSelect.ts
// @@ -15,6 +15,7 @@
// ```
const provider3: TerminalLinkProvider<Uri & { line: number }> = {
    provideTerminalLinks(context) {
        const match = context.line.match(/^@@ -([0-9]+),.* @@/);
        if (match !== null) {
            const fileName = context.searchContextLine.replace(/^\+\+\+/, '');
            const link = {
                startIndex: 0,
                length: match.input.length,
                target: Uri.file(fileName)
            };
            link.target.line = match[1] + 3 /* git provides 3 lines of context */;
            return [link];
        }
    },
    handleTerminalLink(link) {
        workspace.openTextDocument(link.target).then((textDocument) => {
            const range = new Range(link.target.line, 0, link.target.line, 0);
            window.activeTextEditor?.revealRange(range, TextEditorRevealType.InCenter)
        );
    }
};
window.registerTerminalLinkProvider(provider3, {
    searchContext: {
        searchContextPattern: /^\+\+\+/,
        searchContextLimit: 1
    }
});

[Tyriar]

Some comments on the API:

• The search context idea would enable some links we couldn't do before like git diff/eslint. We could build those in core but it would be great if this sort of thing was done by extensions as anything we add to core adds burden for all terminals.
• The generics idea was to give additional context for the link beyond text and the Uri, not sure how else to achieve transferring line/column information over.
• Two of the examples above have handleTerminalLink just return the target URI. The problem with making that optional is it could let links slip through the craps if target is not provided. One way to workaround this is to make 2 link provider interfaces, one where target is mandatory and one where it's optional.

[jrieken]

Not sure where the ResolvedTerminalLink type comes from. Usually, providers resolve a type they have previously returned. Similar <T=Uri> is funky because there isn't much to extend from uri. To align with other providers, maybe this

export interface TerminalLinkProvider<T = TerminalLink > {
    provideTerminalLinks(context: TerminalLinkContext): ProviderResult<T[]>

    /**
     * Handle an activated terminal link.
     * 
     * @returns success state
     */
    handleTerminalLink(link: T, more): ProviderResult<boolean>;
}

[Tyriar]

Not sure where the ResolvedTerminalLink type comes from.

@jrieken that is to convert startIndex and length (which the terminal needs to convert into buffer coordinates and extract the text from) into text (which the link handler may need).

T = TerminalLink looks much nicer if we can figure out what to do above.

[Tyriar]

One way to do this could be to just remove the text property and let extensions authors fill it using the generic type if they want to.

[Tyriar]

Proposal after @jrieken's comments are resolved:

export namespace window {
    export function registerTerminalLinkProvider(provider: TerminalLinkProvider): Disposable;
}

export interface TerminalLinkContext {
    /**
     * This is the text from the unwrapped line in the terminal.
     */
    line: string;

    /**
     * The terminal the link belongs to.
     */
    terminal: Terminal;
}

export interface TerminalLinkProvider<T = TerminalLink> {
    provideTerminalLinks(context: TerminalLinkContext): ProviderResult<T[]>

    /**
     * Handle an activated terminal link.
     * 
     * @returns Whether the link was handled, if not VS Code will attempt to open it.
     */
    handleTerminalLink(link: T): ProviderResult<boolean>;
}

export interface TerminalLink {
    /**
     * The start index of the link on [TerminalLinkContext.line](#TerminalLinkContext.line].
     */
    startIndex: number;

    /**
     * The length of the link on [TerminalLinkContext.line](#TerminalLinkContext.line]
     */
    length: number;

    /**
     * The uri this link points to. If set, and {@link TerminalLinkProvider.handlerTerminalLink}
     * is not implemented or returns false, then VS Code will try to open the Uri.
     */
    target?: Uri;

    /**
     * The tooltip text when you hover over this link.
     *
     * If a tooltip is provided, is will be displayed in a string that includes instructions on how to
     * trigger the link, such as `{0} (ctrl + click)`. The specific instructions vary depending on OS,
     * user settings, and localization.
     */
    tooltip?: string;
}

Note that the search context is missing as this can easily be separated out into a separate feature request: https://github.com/microsoft/vscode/issues/100293

[Tyriar]

@connor4312 ready to try out tomorrow. Note that TerminalLink.target currently does nothing and handleTerminalLink is mandatory and it's return value doesn't matter, I'll be discussing whether we should implement this part in the upcoming API call as all it ends up being is a convenience to call window.openUri(link.target) which an extension could easily do.

[connor4312]

export interface TerminalLinkProvider<T = TerminalLink> {

Should that be T extends TerminalLink = TerminalLink?

[Tyriar]

@connor4312 not sure, there are cases of both in the API. What the intent is, is being able to create your own object that implements the TerminalLink interface which I think works fine as is.

[connor4312]

In that case I think constraining the generic would be appropriate, right? As-is I could make a TerminalLinkProvider<number> which would not make sense. If we add the extends constraint it would enforce that returned objects have at least a length and startIndex.

Looks like we unreleased the Electron 8 insiders, I have code working that I'll try out tomorrow if it's fixed or on OSS.

[Tyriar]

Topics for API sync:

• Should we keep TerminalLink.target and make handleTerminalLink optional?
• Use T extends TerminalLink = TerminalLink instead?
• Should tooltip be named label or something? It's not the full tooltip string, it's the command's label that will be included in the tooltip.
• Any more feedback?

[Tyriar]

Feedback/actions:

• Remove .target
• Use T extends TerminalLink = TerminalLink
• Change length to endIndex

[Tyriar]

Things to discuss in API call:

• Consider adding a command object to TerminalLink rather than the handleTerminalLink method https://github.com/microsoft/vscode/issues/101457
• Consider making handleTerminalLink return a Promise https://github.com/microsoft/vscode/issues/101391
• TerminalLink endIndex should be inclusive (and it should say that in the documentation) https://github.com/microsoft/vscode/issues/101394
• Consider changing TerminalLink endIndex to length https://github.com/microsoft/vscode/issues/101455

[DanTup]

Consider making handleTerminalLink return a Promise #101391

Isn't this already handled? In the definition above, it's returning a ProviderResult<boolean> and ProviderResult includes Thenable?

The API looks like it should support what I want (https://github.com/Dart-Code/Dart-Code/issues/2581#issuecomment-649818833), though if there may be further changes (eg. returning a command - which if I understand correctly sounds like a good idea - allowing us to do things other than just open a file from clicks) I'll hold off until they're done before trying to implement against it in Insiders.

[connor4312]

• The new API could provide the ability to cleanly have multiple actions for a single link. For example, being able to debug a URL via ctrl+click is a nice default, but I could provide a second link for the same text range to open a URL without debugging.
• :+1: on returning a promise from handleTerminalLink
• 👎 on making endIndex inclusive. String and array slicing in JS and (most?) other languages is non-inclusive at the end, I think it will be surprising to most implementors if the end index is inclusive.

Because of the possible changes to endIndex/length being inclusive (breaking) I won't push this to js-debug yet, but you can see my implementation here: https://github.com/microsoft/vscode-js-debug/compare/update-terminal-link-api

[Tyriar]

@DanTup the old proposal comment isn't up to date: https://github.com/microsoft/vscode/blob/d95b65443d9785a3f7a119a1d6fae340885a0a81/src/vs/vscode.proposed.d.ts#L1067-L1070

[Tyriar]

Feedback from call:

• Keep as is, don't return command
• Use ProviderResult<void> as return, we may want to do something while waiting for resolve in the future (like show UI, block multiple calls, etc.)
• Go back to length. While there is an existing [number, number] in the API for start/end(exclusive) in the API, it's a different form and considering the fact we would need to call out that it's exclusive anyway to differentiate it from the document link provider, let's just use length here.

[Tyriar]

Looks like this will stay proposed this iteration and finalize next, please test it out and give feedback now before it's too late 😃

[testforstephen]

@Tyriar I tried the proposed API on the terminal output from Java Debugger, it works well.

Here is a demo to detect the Java exception stacktrace in the terminal.

Only one tiny feedback is to allow the provider show inline message when no result after clicking the link.

User Scenario:

In Java Debugger, it launches the Java application in the terminal by default. And many users expect the exception stacktrace printed to the terminal to be clickable and able to jump to the source file. This proposed API satisfies the feature pretty well. Thanks for this awesome API.

Regarding the implementation, we just use regular expression in the provideTerminalLinks API to check whether the current line looks like a stacktrace. This will keep the provideTerminalLinks fast and lightweight. And only when the user clicks the link, then searching the source mapping at the backend and jump to the file if there is result. Meanwhile, if no source found on clicking, we want to show some inline message to feedback user.

Expected:

Expect to display inline message inside the terminal, just like what the "go to definition" action did, where it displays an inline message inside the editor if no definition found.

[Tyriar]

@testforstephen great feedback thanks!

Only one tiny feedback is to allow the provider show inline message when no result after clicking the link.

The intent for the API is that if you provide a link, you must do something when it's clicked. I suggest reverting to the default behavior and trigger a quick open search for something relevant, in your case either a symbol search for doCreateBean, or a file search for AbstractAutowireCapableBeanFactory. This is even better than a no impl found message imo as it provides more information and potentially finds what the user was looking for anyway.

I'm not sure how you do this via the API exactly I'm sure it's possible though. Maybe running the command workbench.action.quickOpen or workbench.action.showAllSymbols with args if there is not an API in vscode.d.ts?

[Tyriar]

Moved the API to stable and added a sample: https://github.com/microsoft/vscode-extension-samples/commit/83261e3f32421da84513c5a76e8b6a08fff3c332

[Tyriar]

This was tested last iteration, to verify run the new sample and make sure it works as expected.