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

Use IUriIdentityService#extUri #135935

Closed
jrieken opened this issue Oct 27, 2021 · 8 comments
Closed

Use IUriIdentityService#extUri #135935

jrieken opened this issue Oct 27, 2021 · 8 comments
Assignees
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Oct 27, 2021

In a few places I have seen code like this this.extUri = !!(this.capabilities & FileSystemProviderCapabilities.PathCaseSensitive) ? extUri : extUriIgnorePathCase;. This shouldn't be needed because there is IUriIdentityService#extUri which is the central place for this knowledge

@jrieken jrieken added the debt Code quality issues label Oct 27, 2021
@sandy081 sandy081 added this to the November 2021 milestone Oct 27, 2021
@sandy081
Copy link
Member

If you are talking about the following reference then, it was done because this class is created before all services are created.

this.extUri = !!(this.capabilities & FileSystemProviderCapabilities.PathCaseSensitive) ? extUri : extUriIgnorePathCase;

At that moment, IUriIdentityService was not available. I just checked and it seems IUriIdentityService is now available in startup. So will adopt it.

@sandy081
Copy link
Member

I think I can also replace following

function ignorePathCasing(uri: URI, extHostFileSystemInfo: IExtHostFileSystemInfo): boolean {
const capabilities = extHostFileSystemInfo.getCapabilities(uri.scheme);
return !(capabilities && (capabilities & FileSystemProviderCapabilities.PathCaseSensitive));

with

export interface IExtHostFileSystemInfo extends ExtHostFileSystemInfo {
readonly extUri: IExtUri;
}

@jrieken
Copy link
Member Author

jrieken commented Oct 27, 2021

This is what I have found on top (basically reference search on FileSystemProviderCapabilities.PathCaseSensitive)

@sandy081
Copy link
Member

All of the above are in platform namespace and run in shared process but IUriIdentityService is a workbench service and not available in platform. I would be happy to adopt it if we have this service available for shared process.

@jrieken
Copy link
Member Author

jrieken commented Oct 27, 2021

The only thing the UriIdentityService needs is the file service so it should be easy to add that to the shared process

@bpasero
Copy link
Member

bpasero commented Oct 27, 2021

👍 should be easy to add anywhere the file service is there, even main process.

@sandy081
Copy link
Member

Cool, then will add it in shared process.

@sandy081
Copy link
Member

sandy081 commented Nov 3, 2021

  • Moved IUriIdentityService to platform
  • Registered this in Shared process, CLI, remote
  • Used IUriIdentityService#extUri at above mentioned places except in fileUserDataProvider because the service is not available during then.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Development

No branches or pull requests

4 participants
@bpasero @jrieken @sandy081 and others