Description
Migrated from https://ghc.haskell.org/trac/ghc/ticket/8752:
@joeyh said:
"Combine two paths, if the second path isAbsolute, then it returns the second."
But, the implementation of combine checks if the first character of a path is a path separator, which on Windows is not the same as checking if isAbsolute.
This can have counterintuitive results. For example:
import System.FilePath.Windows
prop_windows_is_sane :: Bool
prop_windows_is_sane = isAbsolute p || ("C:\\STUFF" </> p /= p)
where p = "\\foo\\bar"
@thomie said:
Are you proposing a change to the behavior of combine, or can we just change its docstring to:
"Combine two paths, if the second path 'hasDrive' or starts with a path separator, then it returns the second."
@joeyh said:
Changing the doc string in only System.FilePath.Windows.combine would be confusing because System.FilePath.Posix.combine has the same doc string. And I assume most users just use System.FilePath and assume it works for any OS, so probably won't notice if the windows version has a caveat added.
Would changing the implementation to really check isAbsolute be likely to break things?
@thomie said:
Can you explain why the docstring I posed is confusing? It is valid for both Posix as Windows, so it's not really a caveat.
Correct me if I'm wrong, but the way I understand it is that on Windows, if a filepath starts with a single slash, it is considered to be relative to the root of the current drive. The system keeps track of the current drive along with the current directory of that drive.
Currently, if the second argument starts with a slash, combine returns it as is. The implementation is essentially:
combine :: FilePath -> FilePath -> FilePath
combine a b | hasDrive b || hasLeadingPathSeparator b = b
| otherwise = combineAlways a b
I am guessing you are looking for the following behavior:
combine "C:\\" "\\foo\\bar" == "C:\\foo\\bar"
combine "C:\\STUFF" "\\foo\\bar" == "C:\\foo\\bar"
combine "\\\\shared" "\\foo\\bar" == "\\\\shared\\foo\\bar"
combine "\\\\shared\\stuff" "\\foo\\bar" == "\\\\shared\\foo\\bar"
But, since anything else doesn't make any sense:
combine "home" "\\bob" == "\\bob"
So just changing the check to isAbsolute (or hasDrive, since combining with "C:foo", which isRelative, is not implemented at the moment, but that is another issue) doesn't work.
An implementation that has this behavior would be:
combine :: FilePath -> FilePath -> FilePath
combine a b | hasDrive b || hasLeadingPathSeparator b && not (hasDrive a) = b
| hasLeadingPathSeparator b = combineAlways (takeDrive a) (dropLeadingPathSeparator b)
| otherwise = combineAlways a b
Or, an option with the same behavior as above, except:
combine "C:\\STUFF" "\\foo\\bar" == "\\foo\\bar"
combine ""\\\\shared\\stuff" "\\foo\\bar" == "\\foo\\bar"
, would be:
combine :: FilePath -> FilePath -> FilePath
combine a b | hasDrive b || hasLeadingPathSeparator b && not (isDrive a) = b
| hasLeadingPathSeparator b = combineAlways a (dropLeadingPathSeparator b)
| otherwise = combineAlways a b
I'm not sure which of the three is better. We have to change the docstring either way.