Thanks! Amending and applying. On Thu, Nov 13, 2008 at 14:40:15 +0300, Dmitry Kurochkin wrote: > All looks good. The only thing I would change is using takeFilename > instead of takeBaseName in Darcs.Commands.Dist. See below for more > (mostly useless) comments.
I appreciate the summary and the breaking up of the patches :-) > > setCurrentDirectory (toFilePath tempdir) > > - exec "tar" ["-cf", "-", safename $ basename $ toFilePath ddir] > > + exec "tar" ["-cf", "-", safename $ takeBaseName $ toFilePath ddir] > > (Null, File tarfile, AsIs) > > when verb $ withTemp $ \tar_listing -> do > > exec "tar" ["-tf", "-"] > > hunk ./src/Darcs/Commands/Dist.lhs 125 > > (File tarfile, File resultfile, AsIs) > > putStrLn $ "Created dist as "++resultfile > > where > > - basename = fn2fp . own_name . fp2fn > > safename n@(c:_) | isAlphaNum c = n > > safename n = "./" ++ n > Why do you use takeBaseName here and not takeFileName? They have the > same effect here since we apply them to directories. But I think > takeFileName would make it a little easier to read. Mistake on my part. Good catch! I'll apply an amended version. Hooray for patch review! > Replace (///) with </>. Looks good. But it took me a second to > understand why you import (///) from System.FilePath.Posix and not </> > :) Yeah, we (I) need to learn how to get a better feel for the balance between using darcs replace patches to avoid making a mess, and using hunk patches when it's clearer to do so. -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
pgpbOhcRxBWpZ.pgp
Description: PGP signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
