Fix path relative-to for case-insensitive filesystems#16310
Fix path relative-to for case-insensitive filesystems#16310WindSoilder merged 5 commits intonushell:mainfrom
Conversation
WindSoilder
left a comment
There was a problem hiding this comment.
Thank you for the fix! Just left some ideas on try_case_insensitive_strip_prefix
|
For reference what Microsoft does in .NET for getting relative paths: https://github.com/dotnet/runtime/blob/3f25a276772b8d0e54452321e7525b7a176aa4a7/src/libraries/System.Private.CoreLib/src/System/IO/Path.cs#L859 |
WindSoilder
left a comment
There was a problem hiding this comment.
Thanks, LGTM!
I found out MS handles something like C:\ and relative path(like ../) at the same time, I don't think we need such complexity for now
| } | ||
|
|
||
| /// Check if the current filesystem is typically case-insensitive | ||
| fn is_case_insensitive_filesystem() -> bool { |
There was a problem hiding this comment.
I'm not sure if hardcoding this based on the platform is a good idea. You can access NTFS or APFS from Linux, isn't there a better way to check the path belonging to a case insensitive fs?
There was a problem hiding this comment.
It might be better to make this configurable in some way. On Windows at least, case-sensitivity can be set per-directory. But I would not expect relative-to to need to stat each directory in the path (e.g. a path may not (yet) exist or it might be inaccessible).
There was a problem hiding this comment.
I did consider this issue earlier, but opted for this approach as it's the simplest and has minimal performance impact. If you have better ideas, feel free to refine it further
fixes #16205
Description
Before:
After:
For Windows and macOS:
Release notes summary
path relative-toworks better for case-insentitive filesystems, this works onWindowesormacOS:"/etc" | path relative-to "/Etc"