Patchwork [2,of,2,V2] update: update to current bookmark if it moved out from under us (issue3682)

login
register
mail settings
Submitter Kevin Bullock
Date Jan. 21, 2013, 8:15 p.m.
Message ID <669c7ba6f12dde14cfed.1358799304@slower-than-infinity.lan>
Download mbox | patch
Permalink /patch/703/
State Accepted
Commit 2096e025a728792c93e2f2cad005d43342a548f1
Headers show

Comments

Kevin Bullock - Jan. 21, 2013, 8:15 p.m.
# HG changeset patch
# User Kevin Bullock <kbullock@ringworld.org>
# Date 1358797630 21600
# Branch stable
# Node ID 669c7ba6f12dde14cfededf3f2c9a2fa9be8690f
# Parent  911229b0565cb0c031baaae36f21a5829f78e3b1
update: update to current bookmark if it moved out from under us (issue3682)

If the current bookmark (the one listed in .hg/bookmarks.current)
doesn't point to a parent of the working directory, e.g. if it was moved
by a pull, use that as the update target instead of the tipmost
descendent.

A small predicate is (finally) added to the bookmarks module to check
whether the current bookmark is also active.
Matt Mackall - Jan. 21, 2013, 11:29 p.m.
On Mon, 2013-01-21 at 14:15 -0600, Kevin Bullock wrote:
> # HG changeset patch
> # User Kevin Bullock <kbullock@ringworld.org>
> # Date 1358797630 21600
> # Branch stable
> # Node ID 669c7ba6f12dde14cfededf3f2c9a2fa9be8690f
> # Parent  911229b0565cb0c031baaae36f21a5829f78e3b1
> update: update to current bookmark if it moved out from under us (issue3682)
> 
> If the current bookmark (the one listed in .hg/bookmarks.current)
> doesn't point to a parent of the working directory, e.g. if it was moved
> by a pull, use that as the update target instead of the tipmost
> descendent.
> 
> A small predicate is (finally) added to the bookmarks module to check
> whether the current bookmark is also active.

Confused. I thought we were going to erase the distinction between
current and active.

> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -134,6 +134,19 @@ def unsetcurrent(repo):
>      finally:
>          wlock.release()
>  
> +def iscurrent(repo, mark=None, parents=None):
> +    '''Tell whether the current bookmark is also active
> +
> +    I.e., the bookmark listed in .hg/bookmarks.current also points to a
> +    parent of the working directory.
> +    '''
> +    if not mark:
> +        mark = repo._bookmarkcurrent
> +    if not parents:
> +        parents = [p.node() for p in repo[None].parents()]
> +    marks = repo._bookmarks
> +    return (mark in marks and marks[mark] in parents)
> +
>  def updatecurrentbookmark(repo, oldnode, curbranch):
>      try:
>          return update(repo, oldnode, repo.branchtip(curbranch))
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -5961,7 +5961,12 @@ def update(ui, repo, node=None, rev=None
>      # with no argument, we also move the current bookmark, if any
>      movemarkfrom = None
>      if rev is None:
> -        movemarkfrom = repo['.'].node()
> +        curmark = repo._bookmarkcurrent
> +        if bookmarks.iscurrent(repo):
> +            movemarkfrom = repo['.'].node()
> +        elif curmark:
> +            ui.status(_("updating to active bookmark %s\n") % curmark)
> +            rev = curmark
>  
>      # if we defined a bookmark, we have to remember the original bookmark name
>      brev = rev
> diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
> --- a/tests/test-bookmarks.t
> +++ b/tests/test-bookmarks.t
> @@ -458,7 +458,11 @@ create bundle with two heads
>    adding file changes
>    added 2 changesets with 2 changes to 2 files (+1 heads)
>    (run 'hg heads' to see heads, 'hg merge' to merge)
> +
> +update to current bookmark if it's not the parent
> +
>    $ hg update
> +  updating to active bookmark Z
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ hg bookmarks
>       X2                        1:925d80f479bb
Matt Mackall - Jan. 23, 2013, 12:46 a.m.
On Tue, 2013-01-22 at 10:32 -0600, Kevin Bullock wrote:
> On Jan 22, 2013, at 9:25 AM, Kevin Bullock wrote:
> 
> > On 21 Jan 2013, at 5:29 PM, Matt Mackall wrote:
> > 
> >> On Mon, 2013-01-21 at 14:15 -0600, Kevin Bullock wrote:
> >>> # HG changeset patch
> >>> # User Kevin Bullock <kbullock@ringworld.org>
> >>> # Date 1358797630 21600
> >>> # Branch stable
> >>> # Node ID 669c7ba6f12dde14cfededf3f2c9a2fa9be8690f
> >>> # Parent  911229b0565cb0c031baaae36f21a5829f78e3b1
> >>> update: update to current bookmark if it moved out from under us (issue3682)
> >>> 
> >>> If the current bookmark (the one listed in .hg/bookmarks.current)
> >>> doesn't point to a parent of the working directory, e.g. if it was moved
> >>> by a pull, use that as the update target instead of the tipmost
> >>> descendent.
> >>> 
> >>> A small predicate is (finally) added to the bookmarks module to check
> >>> whether the current bookmark is also active.
> >> 
> >> Confused. I thought we were going to erase the distinction between
> >> current and active.
> > 
> > Yes, that's the idea of this patch, although it's phrased differently. I thought this was the behavior you were suggesting?
> > 
> > Going thru the code to unify the usage of "current" and "active" terms feels like too much churn for stable.
> 
> To elaborate on this, if we erase the distinction between current and
> active, we introduce a new distinction: between active and
> active-but-not-at-the-working-dir. The new predicate distinguishes
> between these two states.

Alright. We'll have to discuss later how to bring some API and possibly
UI clarity to that distinction. I've queued both patches for stable,
thanks.

> I have another patch pending that changes `hg bookmarks` to show the latter state.

By that, I assume you mean "put a * next to the active bookmark even if
it's not .".

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -134,6 +134,19 @@  def unsetcurrent(repo):
     finally:
         wlock.release()
 
+def iscurrent(repo, mark=None, parents=None):
+    '''Tell whether the current bookmark is also active
+
+    I.e., the bookmark listed in .hg/bookmarks.current also points to a
+    parent of the working directory.
+    '''
+    if not mark:
+        mark = repo._bookmarkcurrent
+    if not parents:
+        parents = [p.node() for p in repo[None].parents()]
+    marks = repo._bookmarks
+    return (mark in marks and marks[mark] in parents)
+
 def updatecurrentbookmark(repo, oldnode, curbranch):
     try:
         return update(repo, oldnode, repo.branchtip(curbranch))
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5961,7 +5961,12 @@  def update(ui, repo, node=None, rev=None
     # with no argument, we also move the current bookmark, if any
     movemarkfrom = None
     if rev is None:
-        movemarkfrom = repo['.'].node()
+        curmark = repo._bookmarkcurrent
+        if bookmarks.iscurrent(repo):
+            movemarkfrom = repo['.'].node()
+        elif curmark:
+            ui.status(_("updating to active bookmark %s\n") % curmark)
+            rev = curmark
 
     # if we defined a bookmark, we have to remember the original bookmark name
     brev = rev
diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -458,7 +458,11 @@  create bundle with two heads
   adding file changes
   added 2 changesets with 2 changes to 2 files (+1 heads)
   (run 'hg heads' to see heads, 'hg merge' to merge)
+
+update to current bookmark if it's not the parent
+
   $ hg update
+  updating to active bookmark Z
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg bookmarks
      X2                        1:925d80f479bb