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

FileSystemProvider and uri labels #51446

Closed
isidorn opened this issue Jun 8, 2018 · 6 comments
Closed

FileSystemProvider and uri labels #51446

isidorn opened this issue Jun 8, 2018 · 6 comments
Assignees
Labels
feature-request Request for new features or functionality file-explorer Explorer widget issues on-testplan
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2018

Currently for non file uris we always show the full uri.toString() as a label.
Code pointer

However this should be contributed by the fileSystem provider such that for each uri a label can be provided.
Doing this logic on the vscode side alone seems tricky since for some uris it is useful to show the authority (like ftp for example) and for some it is just noise.

Let me know what you think @jrieken @bpasero

@isidorn isidorn added the file-explorer Explorer widget issues label Jun 8, 2018
@isidorn isidorn added this to the June 2018 milestone Jun 8, 2018
@isidorn isidorn self-assigned this Jun 8, 2018
@jrieken
Copy link
Member

jrieken commented Jun 8, 2018

fileSystem provider such that for each uri a label can be provided.

Not sure that's a good idea. I think we should use the workspace space information. Often you have a config like this

{
	"uri": "remotehub://github.com/rogpeppe/godef",
	"name": "godef"
},

and when that's the case we should replace remotehub://github.com/rogpeppe/godef with godef, similar us replacing \Users\jrieken\ with ~

@bpasero
Copy link
Member

bpasero commented Jun 8, 2018

I would not tie this into the workspace file because we know that we at one point probably will support to open a remote folder without having a workspace file. So we will always have a situation where a remote URI does not have a meaningful name (same applies when actually no name is provided from the workspace file in the first place).

Nevertheless, if that name property is defined we can think about adding it to the label (as a hint), but I would say we could do that also if the workspace folder is a local one.

Can we come up with some heuristic to convert a remote URI to a meaningful label without having to go to each file system provider? We already expect the remote URIs to be in a folder-like hierarchy, so we could just show that part + maybe some indicator that the resource is not local but remote?

@jrieken
Copy link
Member

jrieken commented Jun 8, 2018

Can we come up with some heuristic to convert a remote URI to a meaningful label without having to go to each file system provider?

I think using a workspace folder name is the most simple heuristic that there is. Sure, there must not always be one but its a straight forward thing to add, easy to implement, and doesn't put us in a bad spot. I think pinging the extension (host) for every URI that we want to render is slight overkill and I wouldn't know what recommendation to give to extension authors.

We already expect the remote URIs to be in a folder-like hierarchy, so we could just show that part + maybe some indicator that the resource is not local but remote?

That would only work if things are unique, right? I can easily have ftp://1jsdodiu039dkh.some.thing.com/Users/jrieken/code and file:///Users/jrieken/code which puts us into a situation in which we must render the host. Then, shortening hostnames isn't as simple as shorting/unique-ifying paths because there is no common separator like the slash.

I would not tie this into the workspace file because

When you say "tie" you make it sound like a dependency which I think its not. It about pragmatically using that information if its around, e.g use it in the getPathLabel-function

@isidorn
Copy link
Contributor Author

isidorn commented Jun 13, 2018

We are now showing a name of the workspace as a prefix to the relative path (only when in a multi root workspace). If the workspace name is not specified in the workspace configuration file we use the basename.
Also for non file uris we are always showing the relative path to the workspace, not the full uri as before.
This all together should provide much nicer labels - example can be seen below.

Currently we distuingish the root name from the rest of the label with a centered dot •
Alternatives include to render the root name in a different color / style and also to shot the root name at the end.

@bolinfest @hansonw @siegebell try it out from tomorrow's insiders and let us know how the labels feel

screen shot 2018-06-13 at 15 49 31

@bpasero bpasero added the feature-request Request for new features or functionality label Jun 13, 2018
@isidorn
Copy link
Contributor Author

isidorn commented Jun 18, 2018

also fyi @eamodio

@isidorn
Copy link
Contributor Author

isidorn commented Jun 19, 2018

Closing as I did not recieve any negative feedback. Feel free to create follow up items if something can be improved more here. Thanks

@isidorn isidorn closed this as completed Jun 19, 2018
@isidorn isidorn mentioned this issue Jun 19, 2018
4 tasks
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 3, 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 file-explorer Explorer widget issues on-testplan
Projects
None yet
Development

No branches or pull requests

3 participants