Fixed Inconsistencies between Drive Paths on Windows (C:\ vs c:\)#245
Fixed Inconsistencies between Drive Paths on Windows (C:\ vs c:\)#245felixfbecker merged 10 commits intoxdebug:masterfrom
Conversation
…le.php and c:\file.php
|
Build is failing |
|
Referencing #239 |
Codecov Report
@@ Coverage Diff @@
## master #245 +/- ##
=========================================
+ Coverage 69.48% 69.6% +0.12%
=========================================
Files 5 5
Lines 983 987 +4
Branches 158 160 +2
=========================================
+ Hits 683 687 +4
Misses 300 300
Continue to review full report at Codecov.
|
|
Builds fixed, ready for review |
| let localSourceRoot: string | undefined | ||
| let serverSourceRoot: string | undefined | ||
| let localFileUri = fileUrl(localPath, { resolve: false }) | ||
| let localFileUriLower = fileUrl(localPath.replace(/[a-zA-Z]:\\/, (match: any) => match.toLowerCase()), { |
There was a problem hiding this comment.
why is match any? Shouldn't this be inferred as string?
| // resolve from the server source root | ||
| serverFileUri = url.resolve(serverSourceRootUrl, urlRelativeToSourceRoot) | ||
|
|
||
| // we lowercased the Url, maybe we should undo that |
There was a problem hiding this comment.
This comment confuses me, what do you mean with maybe?
| resolve: false, | ||
| }) | ||
| let serverFileUri: string | ||
| let uriWasLowerCased = false |
There was a problem hiding this comment.
Frankly I am confused why this fix requires this variable - shouldn't it be possible to just normalise everything to a canonical representation?
There was a problem hiding this comment.
E.g. before the path mapping feature I had this PR: #198
Can't the same be done, but for all paths in the map?
There was a problem hiding this comment.
This is the purpose of the "maybe" in the comment above. I wanted to hand back the server URI with the casing that it was given, for consistency. But I wasn't sure it mattered either way (is there a case where a drive letter could be case sensitive? I don't know)
If you are saying there is no purpose for that, I'll remove it and we can be on our merry way. This issue is simply making using VSCode unbearable so getting it fixed is my top priority.
2 participants
It seems like VS Code or another dependency has begun using lowercase drive names in some places, but not in
${workspaceRoot}This will make it match up.