Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic link provider #2893

Closed
rodolf0 opened this issue May 2, 2020 · 14 comments
Closed

Generic link provider #2893

rodolf0 opened this issue May 2, 2020 · 14 comments
Labels
area/api area/links help wanted type/enhancement Features or improvements to existing features

Comments

@rodolf0
Copy link

rodolf0 commented May 2, 2020

The web link add-on allows matching a restricted set of text (eg. Http links) and lifting that into an object you can interact with.

Would it be possible to create a slightly more general version of it? It would takes 2 construction parameters:

  • match: a string defining a Regex Wich allows matching specific text snippets. This Regex should allow capture groups.
  • strategy: a string used as a template for rendering a rewrite of the matched snippet optionally using the capture groups from the match definition.
@Tyriar
Copy link
Member

Tyriar commented May 2, 2020

That's essentially what the old API can accomplish:

xterm.js/typings/xterm.d.ts

Lines 730 to 750 in 939de5d

/**
* (EXPERIMENTAL) Registers a link matcher, allowing custom link patterns to
* be matched and handled.
* @deprecated The link matcher API is now deprecated in favor of the link
* provider API, see `registerLinkProvider`.
* @param regex The regular expression to search for, specifically this
* searches the textContent of the rows. You will want to use \s to match a
* space ' ' character for example.
* @param handler The callback when the link is called.
* @param options Options for the link matcher.
* @return The ID of the new matcher, this can be used to deregister.
*/
registerLinkMatcher(regex: RegExp, handler: (event: MouseEvent, uri: string) => void, options?: ILinkMatcherOptions): number;
/**
* (EXPERIMENTAL) Deregisters a link matcher if it has been registered.
* @deprecated The link matcher API is now deprecated in favor of the link
* provider API, see `registerLinkProvider`.
* @param matcherId The link matcher's ID (returned after register)
*/
deregisterLinkMatcher(matcherId: number): void;

But this one's on the way out, there's been a bunch of work done recently on the "link provider" API that gets asks for lines at a particular cell:

xterm.js/typings/xterm.d.ts

Lines 752 to 758 in 939de5d

/**
* (EXPERIMENTAL) Registers a link provider, allowing a custom parser to
* be used to match and handle links. Multiple link providers can be used,
* they will be asked in the order in which they are registered.
* @param linkProvider The link provider to use to detect links.
*/
registerLinkProvider(linkProvider: ILinkProvider): IDisposable;

The link provider API will probably soon ask for all links on a particular line, see #2848

@Tyriar Tyriar closed this as completed May 2, 2020
@rodolf0
Copy link
Author

rodolf0 commented May 4, 2020

Are there other link provider addons available beyond the web links one? Main intention behind this request was to make it easier to craft new link providers with just {match, replacement template} instead of having to build a full addon for each snippet you want to match.

@Tyriar
Copy link
Member

Tyriar commented May 5, 2020

Only the web one is in this repo, there are more examples in the vscode repo though:

https://github.com/microsoft/vscode/tree/master/src/vs/workbench/contrib/terminal/browser/links

Isn't the link matcher API basically what you're after? I'm not sure exactly what you had in mind with strategy, but that API lets you set a regex, matchIndex to use and then a handler (which is needed for most cases so it's mandatory). Perhaps we should tweak that into a registerRegexLinkProvider so you can have the best of both worlds? This would go a long way towards solving #2850 as the complexity around wide and emoji character would be hidden for that particular API.

@Tyriar Tyriar reopened this May 5, 2020
@Tyriar Tyriar added the type/proposal A proposal that needs some discussion before proceeding label May 5, 2020
@rodolf0
Copy link
Author

rodolf0 commented May 5, 2020

Isn't the link matcher API basically what you're after? I'm not sure exactly what you had in mind with strategy, but that API lets you set a regex, matchIndex to use and then a handler (which is needed for most cases so it's mandatory). Perhaps we should tweak that into a registerRegexLinkProvider so you can have the best of both worlds? This would go a long way towards solving #2850 as the complexity around wide and emoji character would be hidden for that particular API.

I do see that this is what the original API being deprecated allows.

I think a RegexLinkProvider addon would be pretty great! I think the word "strategy" may be confusing maybe we should call these "match" and "template" (instead of strategy).

Let me give some examples of what I'm after.

This addon allows matching different captures of the regex. Maybe the core idea is uri = "some string snippet in the terminal".replace(Regex(match), template); callback(uri)

@Tyriar
Copy link
Member

Tyriar commented May 5, 2020

Oh ok that makes sense. This new API would also need to surface these APIs so embedders could use them to give underline feedback, modifier clicks and tooltips in a consistent way:

xterm.js/typings/xterm.d.ts

Lines 1124 to 1152 in 1168c6c

/**
* What link decorations to show when hovering the link, this property is tracked and changes
* made after the link is provided will trigger changes. If not set, all decroations will be
* enabled.
*/
decorations?: ILinkDecorations;
/**
* Calls when the link is activated.
* @param event The mouse event triggering the callback.
* @param text The text of the link.
*/
activate(event: MouseEvent, text: string): void;
/**
* Called when the mouse hovers the link. To use this to create a DOM-based hover tooltip,
* create the hover element within `Terminal.element` and add the `xterm-hover` class to it,
* that will cause mouse events to not fall through and activate other links.
* @param event The mouse event triggering the callback.
* @param text The text of the link.
*/
hover?(event: MouseEvent, text: string): void;
/**
* Called when the mouse leaves the link.
* @param event The mouse event triggering the callback.
* @param text The text of the link.
*/
leave?(event: MouseEvent, text: string): void;

@Tyriar Tyriar added area/api area/links help wanted type/enhancement Features or improvements to existing features and removed type/proposal A proposal that needs some discussion before proceeding labels May 5, 2020
@Tyriar
Copy link
Member

Tyriar commented May 5, 2020

Maybe there should be a new class in the API instead:

class RegexLinkProvider implements ILinkProvider {
...
}

That way there's no need to worry about exposing/duplicating ILinkProvider members

@jerch
Copy link
Member

jerch commented May 5, 2020

+1 for the regexp link provider idea.

@Tyriar If we spawn things directly from LinkProvider - do we get into "first come/matched - first served" issues during the linkifier provider eval? (Sounds like a classical router problem to me, where registering order gets important...)

@Tyriar
Copy link
Member

Tyriar commented May 5, 2020

@jerch yes order is important, but the order is defined by the embedder so all good.

@rodolf0
Copy link
Author

rodolf0 commented May 5, 2020

The constructor for RegexLinkProvier could take an additional priority: number to resolve ambiguities?

@Tyriar
Copy link
Member

Tyriar commented May 5, 2020

@rodolf0 the link matcher system used a priority, link providers priority is defined by the order they are registered, so it's in complete control of the embedding application. That's called out here:

xterm.js/typings/xterm.d.ts

Lines 754 to 755 in 1168c6c

* be used to match and handle links. Multiple link providers can be used,
* they will be asked in the order in which they are registered.

@LabhanshAgrawal
Copy link
Contributor

LabhanshAgrawal commented Feb 25, 2021

@rodolf0
maybe you can take a look at LabhanshAgrawal/xterm-link-provider

@rodolf0
Copy link
Author

rodolf0 commented Feb 25, 2021

@rodolf0
maybe you can take a look at LabhanshAgrawal/xterm-link-provider

Hey @LabhanshAgrawal this is super cool! Per my understanding the callback is invoked with a range and the text matched correct?

Is there a chance to also invoke the callback with the returned match object from regexp.exec? This allows the callback to do fancier replacements. For example, you can use the match groups to extract bits of the text and write an action based on that.

@LabhanshAgrawal
Copy link
Contributor

I'm not sure if I understand your example fully
But that callback is provided by xterm, it's not the handler that you provide during creating the class. Your handler will be executed when the link is clicked and the args are provided by xterm i.e. a mouse event and the text from the link.

@Tyriar
Copy link
Member

Tyriar commented Oct 19, 2021

I think this ask is covered by https://github.com/LabhanshAgrawal/xterm-link-provider

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/links help wanted type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

4 participants