Fix volume letter parsing in Windows Paths that contain forward slashes. #885
Conversation
This PR removes the requirement for a backslash to be present for volume letters to be parsed. Note that this doesn't fix the case of UNC paths that use forward slashes. A test like the following still fails: ```kotlin @test fun windowsUncAbsolutePathForwardSlashes() { val path = "//server/project/notes.txt".toPath() assertEquals("//server/project/notes.txt", path.toString()) assertEquals("//server/project".toPath(), path.parent) assertNull(path.volumeLetter) assertEquals("notes.txt", path.name) assertTrue(path.isAbsolute) assertFalse(path.isRoot) } ``` Since `//foo` is valid as both a UNC path and a POSIX path, I don't believe paths like that can be parsed unambiguously without the notion of a system file separator or similar. [1]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#parameters [2]: https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats#canonicalize-separators
| @Test | ||
| fun windowsAbsolutePathWithVolumeLetterMixedSlashesSlashFirst() { | ||
| val path = "C:/Windows\\notepad.exe".toPath() | ||
| assertEquals("C:/Windows/notepad.exe", path.toString()) |
There was a problem hiding this comment.
Paths separators are currently normalized based on the first slash in the path. This isn't necessarily wrong, but it is different from how JVM and most other systems behave.
|
It was my (misguided?) intention to use the path character to imply which rules the caller wanted to follow. Consider: On a UNIX system, I was thinking we’d look at the slash character to determine whether |
|
If the library uses the presence of a forward slash to dictate parsing behavior, I think that would make some common use cases more difficult. For example, if I wanted to take a path from user input and call |
|
Good point! I guess it comes to this decision:
|
|
Option 2 (behavior based on the current OS) is the choice taken by all the APIs I can think of off the top of my head. I expect most developers would be surprised by option 1. Option 1 would also make it difficult to write e.g. a cross-platform CLI app for the reasons I mentioned above. My personal vote would be to base the behavior on the current OS, and possibly allow forcing a certain OS style when configuring the FileSystem. |
|
We discussed offline! Thank you! Decisions:
|
#882 introduced a regression in Windows Path parsing. Windows allows forward slashes in path names, replacing them with backslashes as part of normalization (Here it's mentioned in the Win32 docs for the
lpFileNameparameter, and here it's mentioned for .NET).C:/fooandC:/foo\barare both valid Windows paths.This PR removes the requirement for a backslash to be present for volume letters to be parsed.
Note that this doesn't fix the case of UNC paths that use forward slashes. A test like the following still fails:
Since
//foois valid as both a UNC path and a POSIX path, I don't believe paths like that can be parsed unambiguously without the notion of a system file separator or similar.