[darcs-users] darcs patch: Replace indexed lists by map in PatchCheck (and 1 more)

Eric Kow kowey at darcs.net
Thu Apr 23 17:04:13 UTC 2009


Hi,

On Thu, Apr 23, 2009 at 20:48:39 +0400, Dmitry Kurochkin wrote:
> Patches look good. The first patch makes code much cleaner and
> simpler. The second one is tricky but does the right thing.

Thanks for the review!

> I think they should be applied if Reinier tested them.

Having seen (at a distance) Reinier do lots of testing at the sprint,
I'm going to go ahead and apply these now, but it sounds like we're in
for a few tweaks?

> Few comments:
> 
> FileContent fc_maxline is updated only in delete_line. I understand
> that this is sufficient for code to work. But FileContent comment and
> field name suggest that it should be updated in insert_line as well. A
> bit misleading.
> 
> hunk ./src/Darcs/Patch/Check.hs 31
> >  import Data.List (isPrefixOf)
> >  import Control.Monad.State ( State, evalState, runState )
> >  import Control.Monad.State.Class ( get, put, modify )
> > +-- use Map, not IntMap, because Map has mapKeys and IntMap hasn't
> > +import Data.Map ( Map )
> > +import qualified Data.Map as M ( mapKeys, delete, insert, empty, lookup, null )
> >
> 
> Would this comment be more useful near the use of Map,
> i.e. FileContents definition?
> 
> hunk ./src/Darcs/Patch/Test.hs 49
> >  import Control.Monad ( liftM, liftM2, liftM3, liftM4, replicateM )
> >
> >  import Darcs.Patch.Info ( PatchInfo, patchinfo )
> > -import Darcs.Patch.Check ( PatchCheck, Possibly(..),
> > +import Darcs.Patch.Check ( PatchCheck,
> >                      check_move, remove_dir, create_dir,
> >                      is_valid, insert_line, file_empty, file_exists,
> >                      delete_line, modify_file, create_file, remove_file,
> hunk ./src/Darcs/Patch/Test.hs 53
> > -                    do_check, do_verbose_check,
> > +                    do_check, do_verbose_check, FileContents(..)
> >                    )
> >  import RegChars ( regChars )
> >  import ByteStringUtils ( linesPS )
> 
> This could be a nice one line change :)

-- 
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: 197 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090423/3caca616/attachment.pgp>


More information about the darcs-users mailing list