Patchwork bookmarks: calculate visibility exceptions only once

login
register
mail settings
Submitter Pulkit Goyal
Date Jan. 14, 2018, 7:09 p.m.
Message ID <a1551e6da839be8d51dc.1515956977@workspace>
Download mbox | patch
Permalink /patch/26739/
State Accepted
Headers show

Comments

Pulkit Goyal - Jan. 14, 2018, 7:09 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1515955571 -19800
#      Mon Jan 15 00:16:11 2018 +0530
# Node ID a1551e6da839be8d51dc2371520de836ba0f0dba
# Parent  390f860228ba909499093e0e8861c908fe15a2d0
bookmarks: calculate visibility exceptions only once

In the loop "for mark in names", the rev is same in each iteration, so it does
not makes sense to call unhidehashlikerevs multiple times. Thanks to Yuya for
spotting this.
Yuya Nishihara - Jan. 15, 2018, 12:49 p.m.
On Mon, 15 Jan 2018 00:39:37 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1515955571 -19800
> #      Mon Jan 15 00:16:11 2018 +0530
> # Node ID a1551e6da839be8d51dc2371520de836ba0f0dba
> # Parent  390f860228ba909499093e0e8861c908fe15a2d0
> bookmarks: calculate visibility exceptions only once

Queued, thanks.

> +    # unhide revs if any
> +    if rev:
> +        repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
> +
>      for mark in names:
>          mark = checkformat(repo, mark)
>          if newact is None:
> @@ -840,17 +845,16 @@
>              return
>          tgt = cur
>          if rev:
> -            repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
>              ctx = scmutil.revsingle(repo, rev)

Maybe revsingle() can also be moved out of the loop?

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -830,7 +830,12 @@ 
     cur = repo.changectx('.').node()
     newact = None
     changes = []
-    hiddenrevs = set()
+    hiddenrev = None
+
+    # unhide revs if any
+    if rev:
+        repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
+
     for mark in names:
         mark = checkformat(repo, mark)
         if newact is None:
@@ -840,17 +845,16 @@ 
             return
         tgt = cur
         if rev:
-            repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
             ctx = scmutil.revsingle(repo, rev)
             if ctx.hidden():
-                hiddenrevs.add(ctx.hex()[:12])
+                hiddenrev = ctx.hex()[:12]
             tgt = ctx.node()
         for bm in marks.checkconflict(mark, force, tgt):
             changes.append((bm, None))
         changes.append((mark, tgt))
-    if hiddenrevs:
-        repo.ui.warn(_("bookmarking hidden changeset %s\n") %
-                     ', '.join(hiddenrevs))
+
+    if hiddenrev:
+        repo.ui.warn(_("bookmarking hidden changeset %s\n") % hiddenrev)
     marks.applychanges(repo, tr, changes)
     if not inactive and cur == marks[newact] and not rev:
         activate(repo, newact)