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

Have Click to Follow Link tooltip label contain the URL #50029

Closed
TomasHubelbauer opened this issue May 17, 2018 · 7 comments
Closed

Have Click to Follow Link tooltip label contain the URL #50029

TomasHubelbauer opened this issue May 17, 2018 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality

Comments

@TomasHubelbauer
Copy link
Contributor

I'd like to suggest an improvement where Click to Follow Link tooltip label would also show the URL the link leads to. There is basic linkification in VS Code for textual URLs which themselves show you the target, but we also have DocumentLinkProviders which can underline arbitrary ranges. I think it would be useful to show the URL these lead to.

These are the files that I think need to be updated to make this possible:

src/vs/editor/contrib/links/links.ts:29

Also line 41. I am not sure how the dynamic URL would be appended to the string. The only use of HOVER_MESSAGE_GENERAL_META seems to be at line 57 and AFAICT there is no context from which the URL could be pulled.

src/vs/workbench/parts/debug/browser/linkDetector.ts:37

This file may also need to be updated, but I think this only handles converting textual URLs to clickable URLs, not contributed link ranges provided by DocumentLinkProviders from extensions, which is my main interest.

src/vs/workbench/parts/terminal/electron-browser/terminalLinkHandler.ts:186

This is for the context of the integrated terminal (AFAICT). I don't think DocumentLinkProviders contribute to the Terminal, so this should be just plan file/http/https links being converted to clickable ones. It's actually probably not too useful to show the URL in the tooltip here as you can see it, maybe only if it normalized and expanded file URLs so the user doesn't have to infer the path.

I'd like to PR this if this is deemed a worthwhile inclusion, but I'd like to get some pointers on how I could get the actual URL for the ranges highlighted by DocumentLinkProviders. Also would like to hear others opinion on whether to do just there ranges or all URLs, even linkified plain text links where it seems a little redundant because you can already see (some form) of the URL.

@vscodebot vscodebot bot added the terminal General terminal issues that don't fall under another label label May 17, 2018
@Tyriar
Copy link
Member

Tyriar commented May 17, 2018

Yes this probably doesn't apply to the terminal currently but eventually the terminal may support HTML <a>-like links which hide the URL in which case it would be more important. xtermjs/xterm.js#1134

@Tyriar Tyriar removed the terminal General terminal issues that don't fall under another label label May 17, 2018
@isidorn
Copy link
Contributor

isidorn commented May 18, 2018

Out link handling strategy needs to be revisited and shared across all those sections optimally.
I have a debt item trying to capture that #34026

You are correct about the debug/linkDetector it converts textual URLs to clickable URLs and with the current structure might not need changing.
As for the DocumentLinkProvider I leave the up for @alexandrudima

@isidorn isidorn removed their assignment May 18, 2018
@alexdima alexdima added the feature-request Request for new features or functionality label May 25, 2018
@alexdima
Copy link
Member

I'd like to suggest an improvement where Click to Follow Link tooltip label would also show the URL the link leads to.

I am not convinced this is something useful. i.e. I am personally never confused on what ctrl + click will open. Have you run into the situation where you don't know what ctrl + click will open? What was the case? (i.e. it sounds like a bug, and I'd like to avoid us adding more to the UI if it is not needed)

@TomasHubelbauer
Copy link
Contributor Author

@alexandrudima How do you know what DocumentLinkProvider-ed ranges will open? Like I said above, this is not useful for linkized URLs in text or file system paths (although there is argument to be made for displaying the extended, normalized path in a tooltip), but I think this is useful for ranges linkized by DocumentLinkProvider.

@TomasHubelbauer
Copy link
Contributor Author

Maybe it would also make sense to not show this in a tool tip but rather in the status bar like browsers do.
I think there is value in being able to see targets of links without clicking them, but I am not partial to it being through the tool tips.

@alexdima
Copy link
Member

@TomasHubelbauer

If I install an extension and the extension adds random links via DocumentLinkProvider that do not make any sense to me, I will (depending on annoyance level):

  1. create an issue against the extension
  2. disable the extension
  3. uninstall the extension

My point is: we are not a generic webpage browser. We are a code editor. There should not be non-obvious links on source code. i.e. All links in my code should make sense. If there are non-sensical links in my source code, then there is a bug with the extension which creates them.

@TomasHubelbauer
Copy link
Contributor Author

I see your point, initially I didn't consider this to be a big change, but I see how there is a risk of a death by a thousand cuts scenario if the bar at accepting UI changes is too low. Back to your original question, this isn't needed for anything, so I'll close this feature request for now.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants