-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Context
The linkify addon has been broken for some time with little attention to it, yet it remains quite a popular request to have be have active links for VS Code (microsoft/vscode#7321). I'm proposing we pull the linkify logic into the core and expose it as an option.
Why?
One of the reasons linkify is currently broken with no way forward as it is implemented as an addon outside of xterm.js, dealing with things that are very much internal. In addition, with all the performance changes on their way in (particularly this one #450), it is not safe to modify .xterm-rows. This is also very performance critical so it should not be messed with at all my addons.
How it will work
When linkify gets triggered
In order to prevent this from causing a big performance hit I propose we only check for links a certain period of time (100ms, 200ms?) after there has been no changes to the viewport's buffer. This is in contrast to the current implementation which I believe requires an external call.
Web vs local
The current implementation only linkifies hyperlinks as it was designed just for the browser. One of the most compelling use cases however is to linkify local files so that they can be opened in the editor.
Obviously regular <a> tags won't cut it for non-hyperlinks as they won't know how to handle them, we will need something like an attachLinkifyHandler function (even better deprecate and merge attachCustomKeydownHandler into some generic attachHandler/registerHandler function).
It's also important to note that in Electron it's probably not desirable to jump use <a> tags but handle them custom as well. We do however want to keep <a> tags for web files to get the nice middle click/ctrl+click behavior. So the links will be:
- web:
<a>with href set - local:
<a>with no href provided a linkify handler has been attached, otherwise don't check for local links
Better viewport row reuse
How Terminal.refresh works currently, all links would disappear when a single line is added to bottom of the buffer. To better support linkify and also save some CPU cycles while we're at it, we should look into reusing rows by informing Terminal.rowContainers that we simply shifted some elements whenever possible and only run a targeted refresh on a specific row.
Make the span object pool aware of the links
The DomElementObjectPool logic within Terminal.refresh which keeps <span>s around so not as many DOM nodes are constructed will need to be made aware of the potential for <a> tags wrapping <span> and even splitting <span>s in some cases.