[darcs-users] darcs patch: add substitution mechanism for PatchChoices (and 4 more)

Eric Kow kowey at darcs.net
Sat Sep 19 20:27:40 UTC 2009


On Sat, Sep 19, 2009 at 11:54:08 +0100, Ganesh Sittampalam wrote:
> Mon Aug  3 06:57:23 BST 2009  Ganesh Sittampalam <ganesh at earth.li>
>   * add substitution mechanism for PatchChoices
>
> Sat Sep 19 01:40:11 BST 2009  Ganesh Sittampalam <ganesh at earth.li>
>   * add canonization function for FL Prim
>
> Sat Sep 19 11:51:12 BST 2009  Ganesh Sittampalam <ganesh at earth.li>
>   * camelCase recently added functions

I'll apply these three patches, which I think I understand now

> Mon Aug 31 00:46:47 BST 2009  Ganesh Sittampalam <ganesh at earth.li>
>   * add some QC properties that demonstrate the problem with canonizeFL
>
> Sat Sep 19 01:40:53 BST 2009  Ganesh Sittampalam <ganesh at earth.li>
>   * Resolve issue291: add (basic) interactive patch splitting

And work on these later...

add substitution mechanism for PatchChoices
-------------------------------------------
> hunk ./src/Darcs/Patch/Choices.hs 70
> -newtype Tag = TG Integer deriving ( Eq, Ord )
> +data Tag = TG (Maybe Tag) Integer deriving ( Eq, Ord )

A tag is just an arbitrary label that we apply to patches so that we can
track them during patch selection.

Here, Ganesh extends tags with a back-pointer which is useful for the
case where you want to replace a patch with some whole new sequence of
patch(es).  We don't know how for the original sequence goes, so
rather than trying to jiggle things around or keep track of the names
we've used we just sub-address things.

So given:
  x1 x2                      x3, if you replace x2 with some of ys, you get
  x1 y2.1          y2.2 y2.3 x3 and if you replace y2.1, you get
  x1 z2.1.1 z2.1.2 y2.2 y2.3 x3

I'll submit some haddock saying something to this effect.
  
>  negTag :: Tag -> Tag
> -negTag (TG n) = TG (-n)
> +negTag (TG k n) = TG k (-n)

I have no idea what *negating* a tag is for, but I guess it's not too
important that I understand it for now.

> +-- This is dangerous if two patches from different tagged series are compared
> +-- ideally Tag (and hence TaggedPatch/PatchChoices) would have a witness type
> +-- to represent the originally tagged sequence.
> +compareTags :: TaggedPatch p C(a b) -> TaggedPatch p C(c d) -> EqCheck C((a, b) (c, d))
> +compareTags (TP t1 _) (TP t2 _) = if t1 == t2 then unsafeCoerceP IsEq else NotEq

Handy for 'substitute' below.  As Ganesh says, only to be used for the
case where you want to look dig up a given tag from the sequence it
belongs to

>  instance Invert p => Invert (TaggedPatch p) where
>      invert = liftTP invert
> -    identity = TP (TG (-1)) identity
> +    identity = TP (TG Nothing (-1)) identity

Nothing to see here.

> +-- |Tag a sequence of patches as subpatches of an existing tag. This is intended for
> +-- use when substituting a patch for an equivalent patch or patches.
> +patch_choices_tps_sub :: Patchy p
> +                      => Maybe Tag -> FL p C(x y)
> +                      -> (PatchChoices p C(x y), FL (TaggedPatch p) C(x y))
> +patch_choices_tps_sub tg ps = let tps = zipWithFL TP (map (TG tg) [1..]) ps
> +                              in (PCs $ zipWithFL (flip PC) (repeat InMiddle) tps, tps)

This is a refactor of the patch_choices_tps_sub.
Assigning tags is just zipping over [1..], but for the case where you're
replacing a patch with a sequence (eg. x2 with y2.1 y2.2 y2.3) you want
to generate identifiers for the new patches.

> -patch_choices_tps ps = let tps = zipWithFL TP (map TG [1..]) ps
> -                       in (PCs $ zipWithFL (flip PC) (repeat InMiddle) tps, tps)
> +patch_choices_tps = patch_choices_tps_sub Nothing

> +substitute :: forall p C(x y)
> +            . Patchy p
> +           => Sealed2 (TaggedPatch p :||: FL (TaggedPatch p))
> +           -> PatchChoices p C(x y)
> +           -> PatchChoices p C(x y)
> +substitute (Sealed2 (tp :||: new_tps)) (PCs pcs) = PCs (concatFL (mapFL_FL translate pcs))
> +   where translate :: PatchChoice p C(a b) -> FL (PatchChoice p) C(a b)
> +         translate (PC tp' c)
> +             | IsEq <- compareTags tp tp' = mapFL_FL (flip PC c) new_tps
> +             | otherwise = PC tp' c :>: NilFL

Given a patch and an equivalent sequence of patches (the result of using
the hunk-edit feature 'e'), this new function just plugs the sequence in
place of the original patch preserving the patch selection choice.

add canonization function for FL Prim
-------------------------------------
amend-record:
> -    in n2pia $ infodepspatch new_pinf pdeps $ fromPrims
> -             $ concatFL $ mapFL_FL canonize $ sort_coalesceFL $ concatFL $ mapFL_FL canonize
>               $ oldchs +>+ chs
> +    in n2pia $ infodepspatch new_pinf pdeps $ fromPrims
> +             $ canonizeFL
> +             $ oldchs +>+ chs

Darcs.Patch.Prim
> +canonizeFL :: FL Prim C(x y) -> FL Prim C(x y)
> +canonizeFL = concatFL . mapFL_FL canonize . sort_coalesceFL .
> +             concatFL . mapFL_FL canonize

Just a straightforward refactor which you can see in my whitespace
juggling above.

> +-- | 'canonizeFL' @ps@ puts a sequence of primitive patches into
> +-- canonical form. Even if the patches are just hunk patches,
> +-- this is not necessarily the same set of results as you would get
> +-- if you applied the sequence to a specific tree and recalculated
> +-- a diff.
> +--
> +-- Note that this process does not preserve the commutation behaviour
> +-- of the patches and is therefore not appropriate for use when
> +-- working with already recorded patches (unless doing amend-record
> +-- or the like).

Pointing out gotchas in haddocks.  I like that.

> +-- Running canonize twice is apparently necessary to fix issue525;
> +-- would be nice to understand why.

I'm not sure if this helps any, but in a response to my 'applied, but
huh?' review of issue525, David remarked that 'canonize' is really
a misnomer for what is really a patch simplification routine.

http://lists.osuosl.org/pipermail/darcs-users/2008-November/015902.html

camelCase recently added functions
----------------------------------
> Ganesh Sittampalam <ganesh at earth.li>**20090919105112
>  Ignore-this: b2eeda02acabdcaed819c19f18bbddc3
> ] replace ./src/Darcs/Lock.hs [A-Za-z_0-9] edit_text editText
> replace ./src/Darcs/Lock.hs [A-Za-z_0-9] run_edit runEditor
> replace ./src/Darcs/SelectChanges.hs [A-Za-z_0-9] edit_text editText
> replace ./src/Darcs/Utils.hs [A-Za-z_0-9] run_edit runEditor

Thanks

-- 
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/20090919/c20bd42e/attachment-0001.pgp>


More information about the darcs-users mailing list