fix volume case issue in windows platform #199
Conversation
There was a problem hiding this comment.
Good catch @njuCZ !
The code looks pretty good, may I just ask for a test, to make sure that we don't reintroduce the same bug in the future as part of some refactoring?
A simple unit test testing pathEquals() with different combinations of lowercase/uppercase should suffice.
And one nitpick - I'd try to avoid calling files or packages utils generally, it makes it tempting to later add some more random unrelated logic and harder to find where things are. How about calling it path.go?
njuCZ
force-pushed
the
fix_windows_root_module
branch
from
16f7c6b
to
d776a7e
Compare
|
@radeksimko thanks for your suggestions, I have added the test case, please have a look again |
| func pathEquals(path1, path2 string) bool { | ||
| volume1 := filepath.VolumeName(path1) | ||
| volume2 := filepath.VolumeName(path2) | ||
| return strings.EqualFold(volume1, volume2) && path1[len(volume1):] == path2[len(volume2):] |
There was a problem hiding this comment.
I don't know enough about filesystems on Windows myself to decide, but I suspect we may need to compare of the other part of the path in case-insensitive way too at some point. It can be addressed as and when needed though.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
ghost
locked and limited conversation to collaborators
In windows platform, when initializing, the vscode sends root uri in this format
file:///c%3A/Users/user/workspace, the volumncis in lowercase. The volume in all root module path are in lowercase.But when there is any change for
pluginlockfileormodulemanifestfile, the file path starts with upper caseC, which will cause path not match using==, the language server could not get the latest schema or modules info.this is a similiar issue golang/go@935faf3. It seems windows path are case insentitive and linux is case sensitive
I think for now it's ok to only igore the volume case, there seems no need to ignore case for the whole path.