added os.nativeToUnixPath, os.absolutePrefix

[timotheecour]

• nativeToUnixPath is occasionally useful (eg in unittests as well as for some PR's) and is the best-effort reverse of unixToNativePath
• absolutePrefix does the tricky OS-specific work of figuring out the 1st path component of absolute paths (and is used for example in nativeToUnixPath; isAbsolute could also be implemented on top of it but I didn't do it in this PR), eg:

doAssert absolutePrefix(r"C:\bar") == r"C:\"

note: another sensible choice would've been firstPathPart (analog to lastPathPart), instead of absolutePrefix, which would also return 1st path component for relative paths, eg doAssert firstPathPart("foo/bar") == "foo"

[timotheecour]

PTAL

[Varriount]

I don't see any tests for these new procedures, other than the examples.

I'm also unsure about the behavior of discarding the drive when converting from Windows to Posix. While this is the most popular interpretation, one could also keep the drive and use it as the first path segment (the way WSL and Cygwin translate paths).

Additionally, I don't really feel that these procedures have enough benefit to be included in the standard library. Hardly anyone uses classic MacOS anymore, and the Windows API accepts paths with both forward and backward slashes. In the case where a path variable differs between operating systems, I think I would prefer to just explicitly place the separate paths in when blocks.

[Varriount]

Why is the change log showing these changes?

[timotheecour]

it's a consequence of merging #14098 without #14025; my plan was to merge #14025 first, then update #14098 accordingly, and merge it next. That would've prevented this, but unfortunately #14025 got closed.

I've fixed the changelog here, but this could/will happen to other old PR's (pre-1.2) that rebase against devel. We should really re-open #14025 (and address whatever comments need to be addressed) or at least for time being git move changelog.md changelogs/changelog_1_4.md (but my PR was more careful about that and did other fixes)

[timotheecour]

I don't see any tests for these new procedures, other than the examples.

indeed, there were only runnableExamples; i will add to tos.nim; and update references to (1, 1)

I'm also unsure about the behavior of discarding the drive when converting from Windows to Posix. While this is the most popular interpretation, one could also keep the drive and use it as the first path segment (the way WSL and Cygwin translate paths).

i'll change to proc nativeToUnixPath*(path: string, keepDrive = false): string

Additionally, I don't really feel that these procedures have enough benefit to be included in the standard library.

stdlib still has lots of bugs related to path handling on windows, see #13986 and in particular #13986 (comment)

in fact this PR helps fixing #13986, as it allows extracting the drive and check whether the drive is the same when computing relativePath + friends

Windows API accepts paths with both forward and backward slashes.

not everything in windows supports /; and your point is more about the pre-existing unixToNativePath than this PR's nativeToUnixPath ; in any case testing requires making sure native paths are correctly handled on each platform, and this PR helps with that too.

[jibal]

I'm also unsure about the behavior of discarding the drive when converting from Windows to Posix.

I'm not unsure ... it's disastrous. The idea of "unixToNative" and "nativeToUnix" (which are obviously misnamed) for absolute paths is misconceived, unless you do like cygwin and have a well-defined round-trip mapping. You could actually get away with nativeToUnixPath mapping from X:\foo to /cygdrive/x/foo and unixToNativePath mapping from /cygdrive/x/foo to X:\foo ... it would provide a round-trip mapping, would make cygwin users happy, and wouldn't hurt other users. If you don't like a mapping that uses a convention from an environment Nim doesn't even support, you could make it /windowsDrive/X/foo and cygwin users could mount (which is just a text mapping) /windowsDrive onto /cygdrive and again would be happy. (Even more happiness would come from unixToNativePath recognizing /cygdrive, /windowsDrive, and any other convention used by POSIX-on-Windows systems). Simply throwing away the drive name would make them and all other Windows users very unhappy.

I don't really feel that these procedures have enough benefit to be included in the standard library.

absolutePrefix (not a good name; maybe getPathRoot) is useful and other path functions in the library can use it. Adding an API that maps Windows paths to "Unix" paths by throwing away the drive is a really bad idea.

stdlib still has lots of bugs related to path handling on windows

Throwing away the drive would add another. Speaking of such bugs, the library generally thinks that Windows doesn't support symlinks, which is false and then some. (Just as one example, for users who installed Nim via scoop, the libraries live in scoop\apps\Nim\current, which is a symlink (actually a junction point, but they do the same thing) to scoop\apps\Nim<version number>). At the very least, expandFilename (what an awful name ... the compiler at least calls it canonicalizePath; why not just realPath, like the POSIX function?) on Windows should call getFinalPathNameByHandle, which is comparable to the POSIX realpath, rather than getFullPathName, which isn't. I understand why it uses the latter: because the former didn't exist when the code was written. So I suppose it should be conditionalized on the Windows version.

not everything in windows supports /

The API does. Apps have random restrictions because people (including those working at MS) don't read the docs before coding. (And the fact that DOS commandline apps use / as the switch indicator strongly works against / in path names.) Nim does better than most, but that's a low bar.

[Varriount]

Just want to add a link to this comment, as it outlines the current uses of nativeToUnixPath, and why they can (mostly) be replaced with some rewriting to be more robust, and only use unixToNativePath.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.