[darcs-users] darcs patch: Resolve issue1279: fix spurious does-not... (and 1 more)

[Petr Rockai]

Eric Kow <kowey at darcs.net> writes:
> Petr: I think this might be worth considering for 2.2 (despite it being very
> last minute).  This should fix Wolfgang's problems.
Ok, looks fine to me. Please consider the inlined comments below. Comments or
not, please hurry to apply this to mainline (I mean, http://darcs.net) and I'll
pull it into release branch. A second review would be good, too, if there are
takers.

Resolve issue1279: fix spurious does-not-exist warning on directories
---------------------------------------------------------------------

[snip hunk]
> hunk ./src/Darcs/Commands/WhatsNew.lhs 159
>  warn_if_bogus :: (Slurpy,Slurpy) -> [SubPath] -> IO()
>  warn_if_bogus _ [] = return ()
>  warn_if_bogus (rec, pend) (f:fs) =
> -    do exist <- doesFileExist file
> +    do exist1 <- doesFileExist file
> +       exist2 <- doesDirectoryExist file
> +       let exist = exist1 || exist2
>         if exist then when (not (slurp_has fp rec) || (slurp_has fp pend))$
>                         putStrLn $ "WARNING: File '"
>                           ++file++"' not in repository!"
exist1 and exist2 may not be the best variable names ever... :) I agree with
the minimal change for this, but it might be worth recording a patch on top
that factors out the doesFileExist || doesDirectoryExist idiom.

Fix spurious file not in repository warning
-------------------------------------------

> hunk ./src/Darcs/Commands/WhatsNew.lhs 162
>      do exist1 <- doesFileExist file
>         exist2 <- doesDirectoryExist file
>         let exist = exist1 || exist2
> -       if exist then when (not (slurp_has fp rec) || (slurp_has fp pend))$
> +       if exist then when (not (slurp_has fp rec || slurp_has fp pend)) $
>                         putStrLn $ "WARNING: File '"
>                           ++file++"' not in repository!"
>                  else putStrLn $ "WARNING: File '"++file++"' does not exist!"
This change is a little confusing to me. The original meaning is (due to tight
binding of not) "the file is not recorded or it is pending" whereas the new is
"the file is not recorded nor pending". Indeed, the file is likely in a
repository if it's either recorded or pending. Again, further refactoring might
make sense -- is this the only place where we check whether a file is in
repository? What about Changes.lhs (I haven't checked... now I see that darcs
changes foobarh won't issue a warning... maybe we should add one...)?

Yours,
   Petr.

-- 
Peter Rockai | me()mornfall!net | prockai()redhat!com
 http://blog.mornfall.net | http://web.mornfall.net

"In My Egotistical Opinion, most people's C programs should be
 indented six feet downward and covered with dirt."
     -- Blair P. Houghton on the subject of C program indentation