[PATCH 01 of 12 V2] bookmarks: add function to centralize the logic to compare bookmarks

[FUJIWARA Katsunori]

At Tue, 24 Sep 2013 22:46:41 -0500,
Kevin Bullock wrote:
> 
> On 22 Sep 2013, at 9:05 AM, FUJIWARA Katsunori wrote:
> 
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1379858556 -32400
> > #      Sun Sep 22 23:02:36 2013 +0900
> > # Node ID 2bb09ac15facc79ae3e926317d250d4f2bf50bfc
> > # Parent  ea35caf324bb04cbc9ab5e2328367bc50f558cfb
> > bookmarks: add function to centralize the logic to compare bookmarks
> > 
> > This patch adds "compare()" function to centralize the logic to
> > compare bookmarks between two repositories.
> > 
> > This patch also adds utility function "_execactions()" to execute
> > action corresponded to comparison result for each bookmarks.
> > 
> > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> > --- a/mercurial/bookmarks.py
> > +++ b/mercurial/bookmarks.py
> > @@ -239,6 +239,84 @@
> >     finally:
> >         w.release()
> > 
> > +def compare(localrepo, srcmarks, dstmarks,
> > +            srchex=None, dsthex=None, targets=None):
> > +    '''Compare bookmarks between srcmarks and dstmarks, and create result list
> > +
> > +    Each elements in result list are tuple "(kind, bookmark name,
> > +    changeset ID on source side, changeset ID on destination
> > +    side)". "kind" is one of "addsrc", "adddst", "advsrc", "advdst",
> > +    "diverge", "differ" or "invalid". Each changeset IDs are 40
> > +    hexadecimal digit string or None.
> > +
> > +    Changeset IDs in result tuples may be unknown for localrepo, if
> > +    "kind" is one of "addsrc", "adddst", "differ" or "invalid".
> 
> Seems like it would be better to return a tuple of sets, like we do
> from localrepo.status(). I like the clarity of the dispatch-table
> ("actionmap") you have _execactions() take, but it would be just as
> clear (and _execactions would be unnecessary) if we had a call
> pattern like:
> 
>     addsrc, adddst, advsrc, advdst, diverge, differ, invalid =
>         bookmarks.compare(...)
> 
>     for mark, srcnode, dstnode in addsrc | advsrc | advdst | diverge | differ:
>         ...
>     for mark, srcnode, dstnode in adddst:
>         ...
>     etc.
> 
> We would also avoid making callers know the "magic strings" that are
> returned in these tuples, and callers could only look at the return
> sets they're interested in (cf. patch 4, where updateremote() is
> only interested in the advsrc tuples).


Thank you for your suggestion.

I just followed "merge.manifestmerge()" style, but
"localrepo.status()" style looks better for purpose of this series, as
you described.

I'll post revised ones.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp