[darcs-devel] darcs patch: Add test case for issue154 (rmdir on non... (and 2 more)

[Juliusz Chroboczek]

> As suggested by Zachary Landau, here is a resubmission of the patches
> with the [DarcsFlags] first.

Forgot to mention it -- but I agree with Zachary.

> Thu Apr  6 00:52:01 CEST 2006  Eric Kow <eric.kow at gmail.com>
>   * Add a 'is working dir' flag to PatchApply.apply.

Looks good to me.  Don't you wish Haskell had optional arguments?
(Unfortunately, they doen't fit well with currying, especially if you
want your evaluation strategy to be type-free.)

>   * Allow for non-empty subdir when applying rmdir patch to working dir.

>   Do not delete the directory; just print a warning. Fix for issue154.

Certainly the right thing to do.

I'm going to apply this, so if you choose to take the comments below
into account, please send a new patch rather than amending this one.

> +apply :: WriteableDirectory m
> +      => [DarcsFlag]
> +      -> Bool -- ^ working directory - if True, this means some inconsitencies such
> +              --   as removing a non-empty directory are treated as warnings, not
> +              --   errors.  When in doubt, set to False.
> +      -> Patch -> m ()

I prefer comments to be separated from the type, so that the type
remains compact and easily readable.  But that's just me.

+    mRemoveDirectoryIfPossible :: FileName -> m ()
+    mRemoveDirectoryIfPossible d =
+      (mRemoveDirectory d)
+      `catch` (\e -> if "(Directory not empty)" `isSuffixOf` (show e)
+                     then putStrLn $ "Warning: Not deleting " ++ (fn2fp d) ++ " because it is non-empty."
+                     else ioError e)

Hmm, the putStrLn is going to cause problems with the ``Quiet'' flag.
I'd much prefer something like

+    mRemoveDirectoryIfPossible :: FileName -> m (Maybe String)

with the warning, if desired, being emitted by the caller.

                                        Juliusz