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

Support links that point outside of the workspace (output pane) #23917

Closed
frankcai opened this issue Apr 5, 2017 · 11 comments
Closed

Support links that point outside of the workspace (output pane) #23917

frankcai opened this issue Apr 5, 2017 · 11 comments
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities output Output channel system issues
Milestone

Comments

@frankcai
Copy link

frankcai commented Apr 5, 2017

Text inside the output pane is only recognized as a link if it points to somewhere within the VSCode workspace. I have an extension that writes files to a temporary path and would like links to these to be clickable in the output pane.

@Tyriar Tyriar assigned jrieken and unassigned Tyriar Apr 5, 2017
@jrieken jrieken assigned isidorn and unassigned jrieken Apr 10, 2017
@michelkaporin
Copy link
Contributor

@isidorn sorry, not my area of expertise. This it in the output pane, where editor's link detection is used. Reassign me if you'd like me to look into it after I am done with link detection in debug console/exception widget.

@michelkaporin michelkaporin removed their assignment Apr 10, 2017
@isidorn isidorn added the feature-request Request for new features or functionality label Apr 10, 2017
@isidorn
Copy link
Contributor

isidorn commented Apr 10, 2017

@isidorn isidorn added the help wanted Issues identified as good community contribution opportunities label Apr 10, 2017
@isidorn isidorn added this to the Backlog milestone Apr 10, 2017
@isidorn isidorn removed their assignment Apr 10, 2017
@bpasero
Copy link
Member

bpasero commented Jun 21, 2017

@Lixire @michelkaporin @isidorn can we revisit this? I see that there is a link detector pattern for the debug repl. Can we share the link detection from repl with the output link detector by moving it into a reusable helper?

There is a couple of reasons:

  • I do not like the fact that the output editor only detects links for files inside the workspace
  • we have many places of link detection that all seem related

Also, I see a questionable use of contextService.toResource that will no longer work when in a multi root environment where you will need to know for which folder resource the link belongs to. I filed #29190

@bpasero bpasero modified the milestones: On Deck, Backlog Jun 21, 2017
@isidorn
Copy link
Contributor

isidorn commented Jun 21, 2017

I am all up for revisiting this.
If interns are not interested for this please let me know and I will investigate.

@michelkaporin
Copy link
Contributor

@bpasero I would love if we would use same utility for link detection. Currently we have own detector in every area (editor, repl & exception widget, terminal). Although I am not sure if I have enough time for this @isidorn.

We can try merging link detection between editor and repl with exception widget, but terminal will still handle links separately (see #24489).

@bpasero
Copy link
Member

bpasero commented Jun 21, 2017

As far as I am concerned about the code we have in Output editor I think we should drop it in favour of what you have for the repl and exception widget. The output one is really bad because it only supports links for files that are inside the workspace. This probably dates back from the time we ran in the browser and did not want to show any links for any file outside of the workspace.

@Lixire
Copy link
Contributor

Lixire commented Jun 23, 2017

I think I'll be kept busy with my task project for a while so if you want to investigate it right now then go for it. Otherwise, I can do it for the next iteration.

@michelkaporin
Copy link
Contributor

@bpasero solution in repl and exception widget for link detection is also not the perfect one. We do not support http links, for instance, and it is quite complicated to introduce this support with the current regex. I'd prefer to redesign the solution, rather then reusing.

@Tyriar
Copy link
Member

Tyriar commented Jun 24, 2017

@michelkaporin maybe we should look into having a shared component between xterm.js and vscode? xterm.js doesn't have any non-dev dependencies right now but it would be good if all effort could be spent on the same solution. The terminal does support http links for example with a follow up item to use a proper parser for even better link detection xtermjs/xterm.js#583 (eg. "(http://some.com/link(2))" should exclude the outer brackets). /cc @parisk

In the end there is the common logic of finding the best substrings to turn into link(s) within a string, the actual application of those links may or may not be shareable (it's a relatively complex task for the terminal).

@parisk
Copy link

parisk commented Jun 26, 2017

I am quite about introducing non-dev dependencies in xterm.js, because it forces us to keep it simple, lightweight and portable, which is important for browser consumers.

This is something that can always be discussed and reconsidered though 😄 . In any case, even if we decide to move with a non-dev dependency, this should not happen before 3.0.

@bpasero bpasero assigned isidorn and unassigned michelkaporin Nov 17, 2017
@isidorn
Copy link
Contributor

isidorn commented Nov 17, 2017

Duplicate of #34026

@isidorn isidorn marked this as a duplicate of #34026 Nov 17, 2017
@isidorn isidorn closed this as completed Nov 17, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 1, 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 help wanted Issues identified as good community contribution opportunities output Output channel system issues
Projects
None yet
Development

No branches or pull requests

8 participants