-
Notifications
You must be signed in to change notification settings - Fork 196
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 - file system path conversion fixes #447
Conversation
file_path = file_uri |> SourceFile.path_from_uri() |> Path.absname() | ||
|
||
not String.starts_with?(file_path, project_dir) or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The negation that was here seems wrong. @axelson do you have any idea why it was like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it does seem backwards. I'd have to do some investigation but I would guess that it would be related to the fact that previously it wasn't doing a Path.absname
before. But I'd have to do some more investigation to be more sure. If we do keep a check like it around, then I think we should refactor it to make it a bit more obvious exactly what it is checking (and add some relevant tests).
For reference this is the commit that introduced it: d143772
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, this module would use a refactor and better test coverage. Should I create a new issue? Edit: #459
tests and implementation basing on https://github.com/microsoft/vscode-uri adds support for UNC paths adds downcasing for Windows drive letters fixes character escaping for paths with URI control characters (?, #, : etc) don't translate non file: URIs to paths
file://project/file.ex is a path to /file.ex on project server valid UNIX path URI to /project/file.ex is file:///project/file.ex
e9c8b10
to
1a7b2e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good ❤️
This PR adds tests and some fixes to URI handling. The tests was based on test cases from https://github.com/microsoft/vscode-uri/blob/master/src/test/uri.test.ts - this is the library used by vscode
Improvements to URI handling include:
#
,?
, etc)file
schemeOther improvements
.formatter.exs
Fixes #444
Fixes #416