#47, isValid now detects invalid files like "nul .txt" on Windows
authorNeil Mitchell <ndmitchell@gmail.com>
Tue, 22 Dec 2015 11:43:16 +0000 (11:43 +0000)
committerNeil Mitchell <ndmitchell@gmail.com>
Tue, 22 Dec 2015 11:43:16 +0000 (11:43 +0000)
System/FilePath/Internal.hs
changelog.md
tests/TestGen.hs

index 30ee372..8f0acdd 100644 (file)
@@ -872,6 +872,8 @@ badElements =
 -- > Windows: isValid "\\\\\\foo" == False
 -- > Windows: isValid "\\\\?\\D:file" == False
 -- > Windows: isValid "foo\tbar" == False
+-- > Windows: isValid "nul .txt" == False
+-- > Windows: isValid " nul.txt" == True
 isValid :: FilePath -> Bool
 isValid "" = False
 isValid x | '\0' `elem` x = False
@@ -883,7 +885,7 @@ isValid path =
         not (isJust (readDriveUNC x1) && not (hasTrailingPathSeparator x1))
     where
         (x1,x2) = splitDrive path
-        f x = map toUpper (dropExtensions x) `elem` badElements
+        f x = map toUpper (dropWhileEnd (== ' ') $ dropExtensions x) `elem` badElements
 
 
 -- | Take a FilePath and make it valid; does not change already valid FilePaths.
@@ -901,6 +903,7 @@ isValid path =
 -- > Windows: makeValid "c:\\nul\\file" == "c:\\nul_\\file"
 -- > Windows: makeValid "\\\\\\foo" == "\\\\drive"
 -- > Windows: makeValid "\\\\?\\D:file" == "\\\\?\\D:\\file"
+-- > Windows: makeValid "nul .txt" == "nul _.txt"
 makeValid :: FilePath -> FilePath
 makeValid "" = "_"
 makeValid path
@@ -918,7 +921,7 @@ makeValid path
         validElements x = joinPath $ map g $ splitPath x
         g x = h a ++ b
             where (a,b) = break isPathSeparator x
-        h x = if map toUpper a `elem` badElements then a ++ "_" <.> b else x
+        h x = if map toUpper (dropWhileEnd (== ' ') a) `elem` badElements then a ++ "_" <.> b else x
             where (a,b) = splitExtensions x
 
 
index 4e753af..e749e3f 100644 (file)
@@ -4,6 +4,8 @@ _Note: below all `FilePath` values are unquoted, so `\\` really means two backsl
 
 ## 1.4.1.0  *Unreleased*
 
+ * Make `isValid` detect more invalid Windows paths, e.g. `nul .txt` and `foo\nbar`.
+
  * Improve the documentation.
 
  * Bug fix: `isValid "\0"` now returns `False`, instead of `True`
index c3b1acd..ead85d8 100755 (executable)
@@ -385,6 +385,8 @@ tests =
     ,("W.isValid \"\\\\\\\\\\\\foo\" == False", test $ W.isValid "\\\\\\foo" == False)
     ,("W.isValid \"\\\\\\\\?\\\\D:file\" == False", test $ W.isValid "\\\\?\\D:file" == False)
     ,("W.isValid \"foo\\tbar\" == False", test $ W.isValid "foo\tbar" == False)
+    ,("W.isValid \"nul .txt\" == False", test $ W.isValid "nul .txt" == False)
+    ,("W.isValid \" nul.txt\" == True", test $ W.isValid " nul.txt" == True)
     ,("P.isValid (P.makeValid x)", test $ \(QFilePath x) -> P.isValid (P.makeValid x))
     ,("W.isValid (W.makeValid x)", test $ \(QFilePath x) -> W.isValid (W.makeValid x))
     ,("P.isValid x ==> P.makeValid x == x", test $ \(QFilePath x) -> P.isValid x ==> P.makeValid x == x)
@@ -402,6 +404,7 @@ tests =
     ,("W.makeValid \"c:\\\\nul\\\\file\" == \"c:\\\\nul_\\\\file\"", test $ W.makeValid "c:\\nul\\file" == "c:\\nul_\\file")
     ,("W.makeValid \"\\\\\\\\\\\\foo\" == \"\\\\\\\\drive\"", test $ W.makeValid "\\\\\\foo" == "\\\\drive")
     ,("W.makeValid \"\\\\\\\\?\\\\D:file\" == \"\\\\\\\\?\\\\D:\\\\file\"", test $ W.makeValid "\\\\?\\D:file" == "\\\\?\\D:\\file")
+    ,("W.makeValid \"nul .txt\" == \"nul _.txt\"", test $ W.makeValid "nul .txt" == "nul _.txt")
     ,("W.isRelative \"path\\\\test\" == True", test $ W.isRelative "path\\test" == True)
     ,("W.isRelative \"c:\\\\test\" == False", test $ W.isRelative "c:\\test" == False)
     ,("W.isRelative \"c:test\" == True", test $ W.isRelative "c:test" == True)