Skip to content

hyperlinks addon#2904

Closed
jerch wants to merge 4 commits intoxtermjs:masterfrom
jerch:addon_hyperlinks
Closed

hyperlinks addon#2904
jerch wants to merge 4 commits intoxtermjs:masterfrom
jerch:addon_hyperlinks

Conversation

@jerch
Copy link
Copy Markdown
Member

@jerch jerch commented May 4, 2020

Early WIP.

* Copyright (c) 2019 The xterm.js authors. All rights reserved.
* @license MIT
*
* UnicodeVersionProvider for V11.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Victim of c&p 😄

/**
* TODO:
* Need the following changes in xterm.js:
* - allow LinkProvider callback to contain multiple ranges (currently sloppy hacked into Linkifier2)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be working on #2848 soon which will change the API to return all links for the current line.

Copy link
Copy Markdown
Member Author

@jerch jerch May 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well for the purpose of this addon all it needs are multiple buffer ranges for an URL to still recognize http://example.com as one link anchor text here:

╔═ file1 ════╗
║          ╔═ file2 ═══╗
║http://exa║Lorem ipsum║
║le.com    ║ dolor sit ║
║          ║amet, conse║
╚══════════║ctetur adip║
           ╚═══════════╝

* TODO:
* Need the following changes in xterm.js:
* - allow LinkProvider callback to contain multiple ranges (currently sloppy hacked into Linkifier2)
* - make extended attributes contributable by outer code
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about an ICellMarker API instead? Or tweaking the current one to optionally track a cell in a wrapped line?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs some more design work, yes I think this can be turned into something like cell markers (that stick to a cell, thus would move with it).

Comment on lines +140 to +145
[...this._urlMap.keys()]
.filter(key => key < this._lowestId)
.forEach(key => this._urlMap.delete(key));
[...this._idMap.entries()]
.filter(([unused, value]) => value < this._lowestId)
.forEach(([key, unused]) => this._idMap.delete(key));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably know but [...arr].filter() is probably a little slow to put into a parser hook?

Would LRUMap be useful here? We could pull into the addon?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, well it gets only executed once in a while (with default limits every 1000 urls). For some reason this declarative way was the only one I could find with proper type inference. Every loop attempt (which tend to be faster in general) was countered with type issues by TS lol.

LRUMap:
I think a linked list is not needed here, since the identifiers are just auto increasing integers (always to be cut from left side). Gonna rework that to a loop with early exit condition.

Comment on lines +296 to +299
activate: this._urlMap.get(urlId)!.schemeHandler.opener,
hover: (event: MouseEvent, text: string) => {
console.log('tooltip to show:', text);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

activate, hover and leave need to be settable when creating the addon.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As talked about earlier, this prolly will end up with the LinkProvider being exposed on addon API level for individual customization.

@jerch jerch mentioned this pull request May 6, 2020
@jerch
Copy link
Copy Markdown
Member Author

jerch commented May 7, 2020

Playing around with this addon and a custom xtermjs: scheme:

recorded

The openBox function replaces buffer parts with the box. A closeBox function triggered on x restores the old state. This would also work for lines in the scrollbuffer. Hurray, terminal getting into browser realms now. Who said a LinkProvider should only handle http urls? 😸

@Tyriar
Copy link
Copy Markdown
Member

Tyriar commented May 8, 2020

That gif 🤯

@jerch
Copy link
Copy Markdown
Member Author

jerch commented Oct 4, 2021

@Tyriar This playground PR was mainly stalled due to the new linkprovider API back then. Do you think that API is stable enough to continue working on this PR?

(Sidenote: I'd go for #3489 first, as it is needed here as well. Oh well and the cell markers, will see about that.)

@Tyriar
Copy link
Copy Markdown
Member

Tyriar commented Oct 4, 2021

@jerch link providers have been working great in vscode, 👍 for removing experimental from it

@Tyriar
Copy link
Copy Markdown
Member

Tyriar commented Oct 4, 2021

#3493

@jerch
Copy link
Copy Markdown
Member Author

jerch commented Nov 16, 2021

This is far off, thus closing it.

@jerch jerch closed this Nov 16, 2021
@jerch jerch mentioned this pull request Jul 18, 2022
7 tasks
@Tyriar Tyriar added the reference A closed issue/pr that is expected to be useful later as a reference label Jul 25, 2022
@Tyriar Tyriar mentioned this pull request Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reference A closed issue/pr that is expected to be useful later as a reference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants