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

loaded scripts explorer: show '~' for paths outside of workspace #31002

Closed
weinand opened this issue Jul 19, 2017 · 10 comments
Closed

loaded scripts explorer: show '~' for paths outside of workspace #31002

weinand opened this issue Jul 19, 2017 · 10 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Jul 19, 2017

show '~' for paths outside of workspace (identical to workbench behavior)

@vscodebot vscodebot bot added the workbench label Jul 19, 2017
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues and removed workbench labels Jul 19, 2017
@weinand weinand self-assigned this Jul 19, 2017
@weinand weinand added this to the July 2017 milestone Jul 19, 2017
@bpasero
Copy link
Member

bpasero commented Jul 19, 2017

@weinand
Copy link
Contributor Author

weinand commented Jul 19, 2017

@bpasero great, thx!

@weinand weinand added the feature-request Request for new features or functionality label Jul 19, 2017
@weinand
Copy link
Contributor Author

weinand commented Jul 19, 2017

@bpasero it would make sense to offer getPathLabel() from https://github.com/Microsoft/vscode/blob/master/src/vs/base/common/labels.ts#L31 in vscode.d.ts
This could ensure that paths in the UI are rendered in a consistent way.

@bpasero
Copy link
Member

bpasero commented Jul 20, 2017

@weinand I would actually argue that we should use getPathLabel() by default when you show an item in the custom views and give it a resource but no label. Maybe only if that resource uses the file:// scheme.

/cc @sandy081

@sandy081
Copy link
Member

This would need to expose a resource property in TreeItem object. This will also helps in resolving the icon.

@bpasero
Copy link
Member

bpasero commented Jul 20, 2017

Yeah that's what I thought 👍

@weinand
Copy link
Contributor Author

weinand commented Jul 20, 2017

In my case I would use "getPathLabel()" to convert a debug resource into a "standard" path, but I need my own extension code to break that path into a hierarchy for the TreeDataProvider. I'm not sure that this could/should be done in a generic way.

@bpasero
Copy link
Member

bpasero commented Jul 20, 2017

@jrieken @egamma (since you did something similar via f911acf) after talking to @weinand we wonder if we should not have a API method similar to asRelativePath that allows to put in a resource URI with a path (not necessarily a file URI) and that returns:

  • the value of asRelativePath if the resource is inside the workspace
  • the result of todays getPathLabel() otherwise

@jrieken
Copy link
Member

jrieken commented Jul 20, 2017

the result of todays getPathLabel() otherwise

That is just a *nix-special-case to replace my home-directory with ~, right?

@bpasero
Copy link
Member

bpasero commented Jul 20, 2017

@jrieken yeah this method actually already does what asRelativePath does but on top of that will shorten long paths by adding a "~" prefix on macOS and Linux in case the path is within user.home and removing the user.home portion.

That said I wonder if we need to be careful about changing asRelativePath when you are in multi-root (by adding the root folder as prefix). That seems like a somewhat breaking change for extensions that expect a certain format here. We could leave asRelativePath as is and only support the first folder when converting to a relative path (to be in sync with workspace.rootPath).

I would deprecate asRelativePath in favour of a new method getPathLabel that supports multi root and leave asRelativePath as it was.

It is easy to find out how extensions use asRelativePath: https://github.com/search?l=TypeScript&q=asRelativePath&type=Code&utf8=%E2%9C%93

I think as long as an extension is using this method purely as a way to get a label, it is fine. But having this method be more explicit that the use case is for labels only makes sense to me.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

4 participants