-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
path.relative parse wrong in windows #5485
Comments
cc: @mscdex |
> path.win32.relative('E:/name.d/', 'E:/name.dir/file.txt')
'ir\\file.txt' |
Affects both posix and win32. Only occurs at the device root EDIT: win32 occurs outside of the device root in > path.win32.relative('E:/name.d', 'E:/name.dir/file.txt')
'ir\\file.txt'
> path.win32.relative('E:/foo/name.d', 'E:/foo/name.dir/file.txt')
'..\\name.dir\\file.txt'
> path.posix.relative('/name.d', '/name.dir/file.txt')
'ir/file.txt'
> path.posix.relative('/foo/name.d', '/foo/name.dir/file.txt')
'../name.dir/file.txt' |
So, pretty sure this is #5447, but we didn't resolve it at the device root. The other direction of "prefixness" problems also exist at the device root which we didn't catch either. Clearly need to add more test cases off of the root, which has ended up being a bit of an edge case: > path.win32.relative('C:/baz-quux', 'C:/baz')
'..baz'
> path.win32.relative('C:/baz', 'C:/baz-quux')
'-quux'
> path.posix.relative('/baz-quux', '/baz')
'..'
> path.posix.relative('/baz', '/baz-quux')
'-quux' |
I have a fix for win32, looking at posix |
It seems the issue is observed only in root for posix, and everywhere for win32: > path.posix.relative('/name.d', '/name.dir/file.txt')
'ir/file.txt'
> path.posix.relative('/a/name.d', '/a/name.dir/file.txt')
'../name.dir/file.txt' // correct > path.win32.relative('E:/name.d', 'E:/name.dir/file.txt')
'ir\\file.txt'
> path.win32.relative('E:/a/name.d', 'E:/a/name.dir/file.txt')
'ir\\file.txt' |
Yep, that second win32 case should be working in |
Fixes #5485 PR-URL: #5490 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Brian White <[email protected]>
Node Version: 5.7.0
result:
The text was updated successfully, but these errors were encountered: