-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Fix rooted path without volume label regression on Windows #96
Conversation
Thanks for fixing this. I was just looking at APIs available in .NET. Perhaps what we want is I also found this helpful: There's also an ability to skip normalization with GetFullPath. It'll have to be tested to see if it can work for your ~ paths: Not sure if any of these are preferable to your implementation. Just throwing ideas out there. |
That looks cleaner, I'll give it a try tomorrow. Thanks!
That was an option I thought about for the original fix, prefixing with |
Ah it's a little less clean because What do you think @xoofx, do an |
Yes, it's fine. I will drop support for NET Framework at some point. I don't use it, and I don't want to continue maintaining it. That was ok a few years ago when I started this project, but now... |
Is the PR good to go as it is? |
Also explain why we're not just always calling Path.GetFullPath xoofx#92
033055d
to
86b2af6
Compare
I've cleaned up the history a bit, it's ready if you're happy. |
Thank you! |
On Windows, PhysicalFileSystem.ConvertPathFromInternalImpl requires a volume label after resolving an absolute path. My fix for #91 meant that rooted paths without a volume label, like
/example
, wouldn't be resolved to include such a label.This PR fixes that by explicitly checking for a volume label before deciding to use Path.GetFullPath, and adds a test demonstrating the fix.
Reported here: #92 (comment)