Patchwork [5,of,6] pushbookmark: split an ultra long line in a saner version

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 18, 2014, 9:50 p.m.
Message ID <8db10be23d42b45f7f86.1408398603@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/5474/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 18, 2014, 9:50 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1408150974 25200
#      Fri Aug 15 18:02:54 2014 -0700
# Node ID 8db10be23d42b45f7f86cecad186858059ed91f6
# Parent  35c3e0ecdddf3d777f48f978788c5e418967f0f2
pushbookmark: split an ultra long line in a saner version

We make a temporary variable for the remote bookmark data and we do not expend
all elements from `bookmark.compare` since we are going to use only one.
Augie Fackler - Aug. 18, 2014, 10:48 p.m.
On Mon, Aug 18, 2014 at 02:50:03PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1408150974 25200
> #      Fri Aug 15 18:02:54 2014 -0700
> # Node ID 8db10be23d42b45f7f86cecad186858059ed91f6
> # Parent  35c3e0ecdddf3d777f48f978788c5e418967f0f2
> pushbookmark: split an ultra long line in a saner version

Queueing the others, but I think I prefer something that would at least check that the tuple length stayed constant. Thoughts?

(That is, queued 1-4, 6, but skipped 5).

>
> We make a temporary variable for the remote bookmark data and we do not expend
> all elements from `bookmark.compare` since we are going to use only one.
>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -619,15 +619,13 @@ def _pushbookmark(pushop):
>      ui.debug("checking for updated bookmarks\n")
>      ancestors = ()
>      if pushop.revs:
>          revnums = map(repo.changelog.rev, pushop.revs)
>          ancestors = repo.changelog.ancestors(revnums, inclusive=True)
> -    (addsrc, adddst, advsrc, advdst, diverge, differ, invalid
> -     ) = bookmarks.compare(repo, repo._bookmarks, remote.listkeys('bookmarks'),
> -                           srchex=hex)
> -
> -    for b, scid, dcid in advsrc:
> +    remotebookmark = remote.listkeys('bookmarks')
> +    comp = bookmarks.compare(repo, repo._bookmarks, remotebookmark, srchex=hex)
> +    for b, scid, dcid in comp[2]:  # advanced on local side
>          if ancestors and repo[scid].rev() not in ancestors:
>              continue
>          if remote.pushkey('bookmarks', b, dcid, scid):
>              ui.status(_("updating bookmark %s\n") % b)
>          else:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Aug. 18, 2014, 10:51 p.m.
On 08/18/2014 03:48 PM, Augie Fackler wrote:
> On Mon, Aug 18, 2014 at 02:50:03PM -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1408150974 25200
>> #      Fri Aug 15 18:02:54 2014 -0700
>> # Node ID 8db10be23d42b45f7f86cecad186858059ed91f6
>> # Parent  35c3e0ecdddf3d777f48f978788c5e418967f0f2
>> pushbookmark: split an ultra long line in a saner version
>
> Queueing the others, but I think I prefer something that would at least check that the tuple length stayed constant. Thoughts?

meh, not sure what this brings but complexity in the code.

If someone change the function signature in core it is his duty to 
update all call site. And hopefully anything too crazy wil be caught by 
the tests.

--
Pierre-Yves David

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -619,15 +619,13 @@  def _pushbookmark(pushop):
     ui.debug("checking for updated bookmarks\n")
     ancestors = ()
     if pushop.revs:
         revnums = map(repo.changelog.rev, pushop.revs)
         ancestors = repo.changelog.ancestors(revnums, inclusive=True)
-    (addsrc, adddst, advsrc, advdst, diverge, differ, invalid
-     ) = bookmarks.compare(repo, repo._bookmarks, remote.listkeys('bookmarks'),
-                           srchex=hex)
-
-    for b, scid, dcid in advsrc:
+    remotebookmark = remote.listkeys('bookmarks')
+    comp = bookmarks.compare(repo, repo._bookmarks, remotebookmark, srchex=hex)
+    for b, scid, dcid in comp[2]:  # advanced on local side
         if ancestors and repo[scid].rev() not in ancestors:
             continue
         if remote.pushkey('bookmarks', b, dcid, scid):
             ui.status(_("updating bookmark %s\n") % b)
         else: