filepath icon indicating copy to clipboard operation
filepath copied to clipboard

"one" </> "/two" == "/two" needs explanation

Open wereHamster opened this issue 9 years ago • 21 comments

Nothing in the documentation indicates that behavior: http://hackage.haskell.org/package/filepath-1.4.0.0/docs/System-FilePath-Posix.html#v:-60--47--62-

wereHamster avatar Jun 24 '15 08:06 wereHamster

So the docs for </> point at combine, but I agree a few more examples at </> are wise. Unfortunately the docs for combine have been misrendered, which makes it harder to see the examples as well. That's very unfortunately, I should fix both issues.

ndmitchell avatar Jun 24 '15 20:06 ndmitchell

It would be nice if the package exposed a function which combines the two paths as "one/two". There is such a function internally (combineAlways), but it is not exported. I use joinDrive which is a synonym for combineAlways. But that workaround makes my code more complicated to understand.

wereHamster avatar Jun 24 '15 21:06 wereHamster

What are you using that function for? Any examples? I'm not totally against it, but I'd like to understand in what circumstances it should be recommended.

ndmitchell avatar Jun 26 '15 12:06 ndmitchell

I have a function which may return an absolute URL path (eg. /assets/app-1234.js). I need to take that path and write the file contents to the filesystem under a base path (eg. dist/). This base path may or may not be absolute and may or may not have a trailing slash.

So, in Haskell language:

dump :: FilePath -> FilePath -> ByteString -> IO ()
dump basePath assetPath body = writeFile (basePath </> assetPath) body

main = do
    dump "./dist/" "/assets/app-1234.js" "..."

I would rather have two functions which are clearly documented than one which tries to guess what my intentions are.

wereHamster avatar Jun 26 '15 13:06 wereHamster

Without having examined all the examples for combine closely, I think it has a single clear purpose, though the documentation doesn't really state it. That purpose is:

If path1 names a directory B relative to a directory A, and path2 names an object (file or directory) C relative to directory B, then combine path1 path2 names the object C relative to directory A. Or in other words, to use a weird shell/Haskell hybrid, (cd path1 && cat path2) should cat the same file as cat $(combine path1 path2).

@wereHamster's operation I think is best viewed as splitting the absolute path /assets/app-1234.js into a relative path assets/app-1234.js on top of the absolute path /, then combining that relative path with the bast path ./dist/. In general this operation is useful even when splitting off an absolute path other than /; for example you might have your web server configured to serve /assets/ from a directory that is not named assets, and need to split off /assets/. In general you might want an equivalent to Data.List's stripPrefix for this purpose, though I guess makeRelative and isRelative handle this pretty well. In the case of / you can obviously write it in a number of simple ways depending on how you want to handle the input not actually being an absolute pathname.

rwbarton avatar Jun 26 '15 14:06 rwbarton

Maybe we should rewrite combineAlways a bit, and enforce the following rules:

combineAlways x y == x </> (dropDrive y)
Valid x y => isValid (combineAlways x y)

It would prevent current invalid results such as:

combineAlways "C:/" "D:/foo" == "C:/D:/foo"

@wereHamster: please note that joinDrive is only a synonym for combineAlways since filepath-1.4.0.

From the 1.4 changelog:

  • Semantic change: joinDrive /foo bar now returns /foo/bar, instead of /foobar

Edit: a consumer of this new version of combineAlways would be Cabal, in the file Cabal/Distribution/Simple/InstallDirs.hs, when installing a library with the --prefix configure option:

CopyTo destdir -> fmap ((destdir </>) . dropDrive)

thomie avatar Aug 18 '15 00:08 thomie

I've clarified a lot, added the intuition from @rwbarton (in Haskell form), put all the docs under </> and included the example that spawned this bug report. I would recommend that if you want to splice URL's in this way you do:

 basePath </> dropWhile (== '/') assetPath

I don't think exposing combineAlways is a good idea - it's very much an internal detail, and one that I'm not convinced will remain consistent forever. Alternatively, define your own:

x +/+ y = x ++ "/" ++ y

This is a lexical operation, rather than a filepath one.

ndmitchell avatar Dec 22 '15 10:12 ndmitchell

My inclination is to close this ticket now the docs are fixed - anyone still think it would be good to have "something else", and if so, what that something else should do (not in terms of code, more the idea behind it).

ndmitchell avatar Dec 22 '15 10:12 ndmitchell

I think the new comment might be a bit easier to read if you swap the two Haskell bits; then it reads more like a definition of </>, which is what you are documenting. And yes I see I wrote it the "wrong way" originally :)

Aside from that, I agree with closing this ticket.

rwbarton avatar Dec 22 '15 11:12 rwbarton

You mean make it:

The intention is that readFile (dir </> file) will access the same file as setCurrentDirectory dir; readFile file.

? If that seems clearer to you, I'll go with that.

ndmitchell avatar Dec 22 '15 12:12 ndmitchell

Yes, exactly.

rwbarton avatar Dec 22 '15 12:12 rwbarton

Done, thanks for the suggestion.

ndmitchell avatar Dec 22 '15 12:12 ndmitchell

The same problem applies to joinPath where joinPath ["/foo", "/"] == "/" which isn't documented either. I think that behavior is wrong altogether, but it should at least be consistently documented so haskell code doesn't accidentally try to remove your whole root file system, because somewhere down the call stack System.FilePath caused semi-defined behavior.

hasufell avatar Jan 20 '16 14:01 hasufell

Doesn't the new comment conflict with "C:\\home" </> "\\bob" == "\\bob"?

mat8913 avatar Dec 11 '17 23:12 mat8913

So this behavior seems already sufficiently documented.

I'm providing a PR for discussion, which exposes an alternative (<\>) that is essentially the already existing combineAlways: https://github.com/haskell/filepath/pull/97

hasufell avatar Dec 03 '21 17:12 hasufell

I also just discovered:

Prelude System.FilePath.Windows> "lol" </> "/bar"
"/bar"

This is incorrect on windows, since /bar on windows is a relative path:

Prelude System.FilePath.Windows> isRelative   "/bar"
True

But from what I understand it's relative to the current device?

hasufell avatar Dec 04 '21 23:12 hasufell

On Windows you have drive-relative paths and path-relative paths. /bar is drive relative, so if you are on C: it's C:/barand if you are onD:it'sD:/bar. That means its not fully-relative, but definitely not absolute, and in that case is right to return not . isAbsolute. Even though it's relative, if you were in the loldirectory and didcd /bar, you'd end up in /bar, so that's the right answer. If you were in C:/loland didcd /baryou'd get toC:/bar` , which is not what FilePath returns. But in the corner case of somewhat relative paths, I don't think I'd term any answer as unambiguously wrong, but more a case of interpretation.

ndmitchell avatar Dec 12 '21 20:12 ndmitchell

FWIW, I still think exposing combineAlways is a mistake. I don't think it does what anyone wants and seems like a foot gun. But I'm no longer maintainer of this library, so if you want to, it's your choice.

ndmitchell avatar Dec 12 '21 20:12 ndmitchell

On Windows you have drive-relative paths and path-relative paths. /bar is drive relative, so if you are on C: it's C:/barand if you are onD:it'sD:/bar. That means its not fully-relative, but definitely not absolute, and in that case is right to return not . isAbsolute. Even though it's relative, if you were in the loldirectory and didcd /bar, you'd end up in /bar, so that's the right answer. If you were in C:/loland didcd /baryou'd get toC:/bar` , which is not what FilePath returns. But in the corner case of somewhat relative paths, I don't think I'd term any answer as unambiguously wrong, but more a case of interpretation.

I think one solution could be to return a sum type of cardinality 3, then mark isAbsolute/isRelative as deprecated (for a couple years or indefinitely).

My opinion is that we should ultimately define platform specific functions that are unambiguous.

The platform agnostic module could then:

  1. Export only the strictly common functions and
  2. Define semantically less strict functions

hasufell avatar Dec 13 '21 09:12 hasufell

My opinion is that we should ultimately define platform specific functions that are unambiguous.

I disagree, since I think that's a good way to have none of the ecosystem work on Windows. In my view, working transparently across platforms is way more important than high fidelity. But its your choice now :)

ndmitchell avatar Dec 13 '21 10:12 ndmitchell

My opinion is that we should ultimately define platform specific functions that are unambiguous.

I disagree, since I think that's a good way to have none of the ecosystem work on Windows. In my view, working transparently across platforms is way more important than high fidelity. But its your choice now :)

We already expose platform specific functions via the qualified modules. If someone imports them without understanding the consequences, they already break cross platform support.

The idea is rather that those platform specific modules can have more specific functions (not existing in the "cross" module) that people can use when implementing their own platform specific logic. I don't believe we will ever be able to cover all of the use cases, as the relative path issues show.

Now, users who want more details about their windows paths, could easily get it without re-implementing stuff, which is rather error prone.

hasufell avatar Dec 13 '21 11:12 hasufell