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

uri module lowercases drive letters on Windows #40057

Closed
janicduplessis opened this issue Dec 11, 2017 · 3 comments
Closed

uri module lowercases drive letters on Windows #40057

janicduplessis opened this issue Dec 11, 2017 · 3 comments
Assignees
Labels
*as-designed Described behavior is as designed info-needed Issue requires more information from poster uri

Comments

@janicduplessis
Copy link

janicduplessis commented Dec 11, 2017

Hello, we are using vscode-uri (https://github.com/Microsoft/vscode/blob/master/src/vs/base/common/uri.ts in this repo) to deal with URIs in https://github.com/flowtype/flow-language-server (a Language Server Protocol implementation for flow) and noticed an issue when converting fs paths to uri then back to fs path we lose drive letter casing, which then causes diagnostic not to show properly in atom.

Here's what happens:
C:\my\path -> file:///c/my/path -> c:\my\path

Looking at the code, this lowercasing is definitely intentional and was wondering why it was done, and if it would be possible not to do it since it would fix my use case. If there is a valid reason why it is done and could cause issues I can just fork it but wanted to ask here first :).

PS: The irony of posting a flow/atom issue in a ts/vscode repo 😆

@vscodebot vscodebot bot added the api label Dec 11, 2017
@jrieken jrieken added uri and removed api labels Dec 12, 2017
@jrieken
Copy link
Member

jrieken commented Dec 12, 2017

If there is a valid reason why it is done and could cause issues

We wanted consistency and picked on of two options ;-). I wonder why that's an issue for you?

@jrieken jrieken added *as-designed Described behavior is as designed info-needed Issue requires more information from poster labels Dec 12, 2017
@janicduplessis
Copy link
Author

janicduplessis commented Dec 12, 2017

@jrieken My guess is that atom uses strict comparison for file paths so C:\my\path !== c:\my\path when displaying diagnostics. I just debugged the inputs / outputs and noticed the paths were different when converting a fs path to uri then back to fs path, I don't really know more than that.

Technically it should work on Windows since paths are not case sensitive but in practice this is not always the case. I feel like fixing it here would probably be easier than fixing all paths comparisons and make sure they are not case sensitive on Windows (Are you doing this in vscode?).

@jrieken
Copy link
Member

jrieken commented Dec 13, 2017

I feel like fixing it here would probably be easier than fixing all paths comparisons and make sure they are not case sensitive on Windows

Yeah, we kind of do that. When creating paths/uris we know how we create them, when accepting uris from 3rd parties, e.g. extensions & user content, we normalise them.

@jrieken jrieken closed this as completed Dec 15, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed info-needed Issue requires more information from poster uri
Projects
None yet
Development

No branches or pull requests

2 participants