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

add api for virtual file system provider #124903

Closed
wants to merge 2 commits into from
Closed

add api for virtual file system provider #124903

wants to merge 2 commits into from

Conversation

sbatten
Copy link
Member

@sbatten sbatten commented May 29, 2021

This PR adds the same getCanonicalUri method from resolvers to file system providers. This is necessary to allow remapping arbitrary uris to those of the workspace when relevant.

e.g. gitlens allows opening the workspace at a given commit. we should trust this operation because the files are those in the workspace but the workspace uri scheme is not matching.

	/**
	 * Gets the canonical form of a resource if provided by a file system provider, otherwise, return unmodified.
	 */
	getCanonicalUri(resource: URI): Promise<URI>;

@sbatten sbatten self-assigned this May 29, 2021
@lszomoru lszomoru added the workspace-trust Trusted workspaces label May 31, 2021
@lszomoru
Copy link
Member

There are scenarios in which a file system provider cannot map a URI to a local path. In order to satisfy those cases, we should enable file system providers to specify whether the file system is trusted or not. I believe that this would resolve the LiveShare scenario (#120251 (comment)) to that they can transfer trust from the host to the guest.

@jrieken
Copy link
Member

jrieken commented Jun 17, 2021

This PR adds the same getCanonicalUri method from resolvers to file system providers.

Needing to duplicate an API in two places tells me that both places are wrong. Why isn’t there one central place where extensions can register such canonical uri logic?

Assuming, there is FileSystemProvider#getCanonicalUri would you suggest that VS Code calls this all the time for things like knowing what the true casing of a path is? Today, we assume that all paths that are returned from a FSP are canonical and that they don't go through further massage.

I believe that you don't want to get in the business of canonical uris because it will quickly turn into a bottom-less hole. Isn't your the problem more like "How do I know that the vls:///foo/bar-workspace is actually the file:///foo/bazz-workspace?" Do you maybe just need a ways of introducing "aliases" for workspaces?

@sbatten
Copy link
Member Author

sbatten commented Jun 17, 2021

Why isn’t there one central place where extensions can register such canonical uri logic?

This is itself interesting because one of the challenges we will face with virtual file systems is that they are registered "too late". At the present, we are able to calculate workspace trust before the extension host is brought online. With virtual file systems, we would not be able to do that unless we bring those up like resolvers.

would you suggest that VS Code calls this all the time for things like knowing what the true casing of a path is?

It would be great to get @eamodio 's thoughts on this since he has multiple use-cases of vfs and can share his expectations

Do you maybe just need a ways of introducing "aliases" for workspaces?

I'm happy to explore this. Do have thoughts on what that would look like? I assume you would expect it to be general as mentioned in the first point. It's not necessarily just workspace aliases thought. Arbitrary uris need to be transformed as well.

@eamodio
Copy link
Contributor

eamodio commented Jun 22, 2021

I question if what we want here is really a "canonical" uri. IMO, it is more specifically a "trust" uri. While they are could be similar, we want a specific behavior here. Given a virtual uri, we want to be given the "root" uri that we should trust. But if we call it getCanonicalUri it can be interpreted to be many things. getCanonicalRootUri might be better or even better is getCanonicalTrustUri is even better imo, as it clearly defines what we are looking for with this api.

@jrieken jrieken modified the milestones: July 2021, August 2021 Jul 13, 2021
@jrieken
Copy link
Member

jrieken commented Jul 13, 2021

@lszomoru fyi we moved this out to August

@sbatten sbatten removed this from the August 2021 milestone Aug 26, 2021
@Arthurm1
Copy link

I'm not sure exactly what's being proposed but I'd very much like it if the file system API offered mapping of URIs so that when any URI within my file system API scheme is opened/closed/focused then I'd get the canonical URI in my server instead of the current one I'm supplying.

The use case for this would be presenting a flat list of dependencies to the user even though those dependency files could be anywhere on the local file system.

So in Java - even though these jar files are in directories like...

/root/.cache/coursier/v1/https/maven.google.com/com/android/tools/build/aapt2-proto/4.0.2-6197926/aapt2-proto-4.0.2-6197926-sources.jar/android/aapt/pb/internal/ResourcesInternal.java

The filesystem api would present this file as
\metalsLibraries\source\aapt2-proto-4.0.2-6197926-sources.jar\android\aapt\pb\internal\ResourcesInternal.java

and so in the file explorer the user would see....
image

and for breadcrumbs would see...
image

So file navigation and breadcrumbs would be presented using the current FileSystem URI i.e.
\metalsLibraries\source\aapt2-proto-4.0.2-6197926-sources.jar\android\aapt\pb\internal\ResourcesInternal.java

But any LSP messages e.g. onDidOpenTextDocument would supply the canoncial URI i.e. /root/.cache/coursier/v1/https/maven.google.com/com/android/tools/build/aapt2-proto/4.0.2-6197926/aapt2-proto-4.0.2-6197926-sources.jar/android/aapt/pb/internal/ResourcesInternal.java

Is that what this PR is implementing? Trying not to hijack it but also this would make my life a lot easier 😄

@sbatten
Copy link
Member Author

sbatten commented Dec 20, 2023

closing in favor of new canonical uri provider

@sbatten sbatten closed this Dec 20, 2023
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants