On Mon, Sep 10, 2007 at 02:56:24AM +0200, Eric Y. Kow wrote: > Sorry, I should have given a heads-up. Patch review is going to be > delayed this week, perhaps until next weekend. Here's what I have so > far.
No problem! I'm seriously in refactoring mode, and am not sure quite when
I'll be moving onto conflicts again (although they're certainlyq the
driving force behind these refactorings). I think we can live with a bit
of delayed review.
I'm almost done splitting off Named patches as a distinct type. This is
the first benefit of all this refactoring into classes, as we can use the
same code for either NamedPatches or Patches.
> > replace ./src/Darcs/Patch/Commute.lhs [A-Za-z_0-9] commute commutex
>
> ok for me... nowawadays we write commutex (y :< x), and now we will be
> able to write commute (x :> y), which is the same
Right.
> Minor note: some documentation and error messages are hit by this
> commute to commutex change, may be worth looking into when the dusts
> settles
Yeah, there's going to have to be a complete docs rewrite. :(
> > + , _cancelled :: ![Int]
> > + , _conflicted :: ![Int]
>
> guess these should not have snuck in here. too bad
Yeah.
> > +invertFL :: Invert p => FL p C(x y) -> RL p C(y x)
> > +invertRL :: Invert p => RL p C(x y) -> FL p C(y x)
>
> check... note inversion taken care of by type switch
Well, the inversion is made more efficient by the type switch. Which is
also why these two in particular are defined: when possible, they're
faster to use, and the type checker ensures that we don't misuse the result
(as could happen if these were just "map invert").
> > [Move ReadPatch stuff into yet another class.
> > David Roundy <[EMAIL PROTECTED]>**20070904210525]
>
> > +instance ReadPatch p => ReadPatch (FL p) where
> > + readPatch' want_eof = do "{" <- sal_to_string `liftM` work my_lex
> > + Just `liftM` read_patches
> > + where read_patches :: (Stringalike s, ParserM m, Monad (m s))
> > + => m s (Sealed (FL p C(x )))
>
> This is just read_patches from Darcs.Patch.Read with one less parameter.
> The old read_patches is kept for parsing split patches. Is that going
> away sometime soon?
Hopefully.
> =======================================================================
> make merger code use RL/FL.
> =======================================================================
>
>
> and now refuses to deal with any merger patches prior to the 0.0 version
> (e.g. 0.9)... I guess this is the end of the phase-out part
Darcs has refused to commute merger patches other than 0.0 for some time,
so yea, this is the end of phasing those patches out.
> i didn't understand all of this, but I got the basic gist that we are
> using the new RL/FL types, and are using Merger/Regrem instead of
> Merger True and False.
Right.
> i have not attempted to verify if you are using the lists in the right
> directions :-)
That's tricky (took me a while to figure out), and basically is only
verifiable (so far as I can tell) by checking that we're using the code
right, which I believe I've done.
> Concern: should unwind accept Regrem patches? I ask because the old
> code does not specifically ask for Merger True
I think so, and think it's fixed in a later patch.
> > -commute_recursive_merger (Merger True _ _ _ _ _ :< _) = impossible
>
> cases like these were for non 0.0 mergers
Right.
> > +filenames (Merger p ps p1 p2) = filenames $ Regrem p ps p1 p2
> > +filenames (Regrem p ps p1 p2) = filenames p ++ filenames p1 ++
> > + filenames p2 ++
> > + concat (mapFL filenames $ reverseRL ps)
>
> should i worry about filenames coming out in the wrong order?
No, and in a later patch I'll replace filenames with a
list_touched_files for simplicity (and ease of moving it into a class).
Filenames is only used to check for possibly malicious patches.
--
David Roundy
Department of Physics
Oregon State University
signature.asc
Description: Digital signature
_______________________________________________ darcs-devel mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-devel
