Patchwork D3087: bookmarks: calculateupdate() returns a bookmark, not a rev

login
register
mail settings
Submitter phabricator
Date April 4, 2018, 10:53 p.m.
Message ID <differential-rev-PHID-DREV-xwk54sprcu7vjg6jnm5w-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30302/
State Superseded
Headers show

Comments

phabricator - April 4, 2018, 10:53 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This changes the inaccurate/unclear documentation and also changes the
  code so "node" now contains a binary nodeid.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3087

AFFECTED FILES
  mercurial/bookmarks.py
  mercurial/destutil.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - April 5, 2018, 1:23 p.m.
yuja accepted this revision.
yuja added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> bookmarks.py:353
> +    '''Return a tuple (activemark, movemarkfrom) indicating the active bookmark
> +    and where to move the active bookmark from, if needed.'''
>      movemarkfrom = None

That's true only if `checkout` is None.

Perhaps we should remove the checkout argument. Can you send
a follow up?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3087

To: martinvonz, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/destutil.py b/mercurial/destutil.py
--- a/mercurial/destutil.py
+++ b/mercurial/destutil.py
@@ -55,10 +55,10 @@ 
 def _destupdatebook(repo, clean):
     """decide on an update destination from active bookmark"""
     # we also move the active bookmark, if any
-    activemark = None
-    node, movemark = bookmarks.calculateupdate(repo.ui, repo, None)
-    if node is not None:
-        activemark = node
+    node = None
+    activemark, movemark = bookmarks.calculateupdate(repo.ui, repo, None)
+    if activemark is not None:
+        node = repo.lookup(activemark)
     return node, movemark, activemark
 
 def _destupdatebranch(repo, clean):
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -349,8 +349,8 @@ 
     return heads
 
 def calculateupdate(ui, repo, checkout):
-    '''Return a tuple (targetrev, movemarkfrom) indicating the rev to
-    check out and where to move the active bookmark from, if needed.'''
+    '''Return a tuple (activemark, movemarkfrom) indicating the active bookmark
+    and where to move the active bookmark from, if needed.'''
     movemarkfrom = None
     if checkout is None:
         activemark = repo._activebookmark