[darcs-users] darcs patch: code clarification (and 1 more)

[Eric Kow]

Just some style comments again, with not thinking about the actual
functionality

code clarification
------------------
> -whatsnew_cmd opts args =  withRepository opts $- \repository -> do
> +whatsnew_cmd opts args
> +  | otherwise =
> +    withRepository opts $- \repository -> do

Just making the default case explicit, probably a good idea.

resolve issue1117: warn if file does not exist in whatsnew
----------------------------------------------------------
> -    when (areFileArgs files) $
> -         putStrLn $ "What's new in "++unwords (map show files)++":\n"
> +    when (areFileArgs files)
> +            (do warn_if_bogus ([slurp_recorded repository] ++ [slurp_pending repository]) files


Did you mean [slurp_recorded_repository, slurp_pending_repository]?
For that matter, do you just want a pair of slurpies instead of a list.

> +warn_if_bogus :: [IO Slurpy] -> [SubPath] -> IO()
> +warn_if_bogus _ [] = return ()
> +warn_if_bogus slurp f = 

It's customary to name plural things with an s at the end, like "slurps"
and "fs".  Not a hard rule again, just a nice trick

>
> +    do exist <- doesFileExist file
> +       if (not exist) then do putStrLn $ "WARNING: File '"++file++"' does not exist!"

The parentheses here are superflous (it took me a while to get the hang
of these)

also, why not just invert the then/else to avoid the not?
 
> +                      else do s <- head slurp
> +                              when (not $ slurp_has fp s)
> +                                (putStrLn $ "WARNING: File '"++file++"' not in repository!")
> +       warn_if_bogus (tail slurp) (tail f)
> +    where fp =  toFilePath (head f)
> +          file = encode_white fp

in general, I would try to use head/tail sparingly... maybe you could
have a helper function that focuses on just one file and/or one slurpy?
 

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20081026/9f99c95c/attachment.pgp