Make os.remove behave more like Files.deleteIfExists, add checkExists flag to fall back to old behavior #89

Follow up from the discussion in #86. As discussed, we hope that it would be more intuitive to avoid failing loudly if a file we want to delete already doesn't exist. If the user cares, they can always check the returned `Boolean`, or pass `checkExists = true` if they want the exception

CC @iulian-db

Review by @lefou @sake92

Author: lihaoyi

os/src/FileOps.scala

@@ -280,8 +280,16 @@ object copy {
  * any files or folders in the target path, or
  * does nothing if there aren't any
  */
-object remove extends Function1[Path, Unit]{
-  def apply(target: Path): Unit = Files.delete(target.wrapped)
+object remove extends Function1[Path, Boolean]{
+  def apply(target: Path): Boolean = apply(target, false)
+  def apply(target: Path, checkExists: Boolean = false): Boolean = {
+    if (checkExists) {
+      Files.delete(target.wrapped)
+      true
+    }else{
+      Files.deleteIfExists(target.wrapped)
+    }
+  }
 
   object all extends Function1[Path, Unit]{
     def apply(target: Path) = {
@@ -290,7 +298,7 @@ object remove extends Function1[Path, Unit]{
       val nioTarget = target.wrapped
       if (Files.exists(nioTarget, LinkOption.NOFOLLOW_LINKS)) {
         if (Files.isDirectory(nioTarget, LinkOption.NOFOLLOW_LINKS)) {
-          walk.stream(target, preOrder = false).foreach(remove)
+          walk.stream(target, preOrder = false).foreach(remove(_))
         }
         Files.delete(nioTarget)
       }

os/test/src/OpTests.scala

@@ -27,9 +27,12 @@ object OpTests extends TestSuite{
     test("rm"){
       // shouldn't crash
       os.remove.all(os.pwd/"out"/"scratch"/"nonexistent")
+      // shouldn't crash
+      os.remove(os.pwd/"out"/"scratch"/"nonexistent") ==> false
+
       // should crash
       intercept[NoSuchFileException]{
-        os.remove(os.pwd/"out"/"scratch"/"nonexistent")
+        os.remove(os.pwd/"out"/"scratch"/"nonexistent", checkExists = true)
       }
     }
   }