Patchwork [2,of,9,bm-refactor] commands: move checkconflict to bookmarks module

login
register
mail settings
Submitter Sean Farley
Date June 21, 2017, 12:29 a.m.
Message ID <92a9b407c3b96df20e12.1498004965@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/21562/
State Accepted
Headers show

Comments

Sean Farley - June 21, 2017, 12:29 a.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1497163358 25200
#      Sat Jun 10 23:42:38 2017 -0700
# Branch bm-refactor
# Node ID 92a9b407c3b96df20e12a2076578bdc664600e09
# Parent  03591d8a6d8247d35bbdc93cebce1757755ca463
commands: move checkconflict to bookmarks module

Again, commands.bookmark is getting too large. checkconflict already has
a lot of state and putting it in the bmstore makes more sense than
having it as a closure. This also allows extensions a place to override
this behavior.

While we're here, add a documentation string because, well, we should be
documenting more of our methods.

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
index 948bfbe..451c557 100644
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -11,10 +11,11 @@  import errno
 
 from .i18n import _
 from .node import (
     bin,
     hex,
+    short,
 )
 from . import (
     encoding,
     error,
     lock as lockmod,
@@ -154,10 +155,65 @@  class bmstore(dict):
                 return self.active
             else:
                 raise error.Abort(_("no active bookmark"))
         return bname
 
+    def checkconflict(self, mark, force=False, target=None):
+        """check repo for a potential clash of mark with an existing bookmark,
+        branch, or hash
+
+        If target is supplied, then check that we are moving the bookmark
+        forward.
+
+        If force is supplied, then forcibly move the bookmark to a new commit
+        regardless if it is a move forward.
+        """
+        cur = self._repo.changectx('.').node()
+        if mark in self and not force:
+            if target:
+                if self[mark] == target and target == cur:
+                    # re-activating a bookmark
+                    return
+                rev = self._repo[target].rev()
+                anc = self._repo.changelog.ancestors([rev])
+                bmctx = self._repo[self[mark]]
+                divs = [self._repo[b].node() for b in self
+                        if b.split('@', 1)[0] == mark.split('@', 1)[0]]
+
+                # allow resolving a single divergent bookmark even if moving
+                # the bookmark across branches when a revision is specified
+                # that contains a divergent bookmark
+                if bmctx.rev() not in anc and target in divs:
+                    deletedivergent(self._repo, [target], mark)
+                    return
+
+                deletefrom = [b for b in divs
+                              if self._repo[b].rev() in anc or b == target]
+                deletedivergent(self._repo, deletefrom, mark)
+                if validdest(self._repo, bmctx, self._repo[target]):
+                    self._repo.ui.status(
+                        _("moving bookmark '%s' forward from %s\n") %
+                        (mark, short(bmctx.node())))
+                    return
+            raise error.Abort(_("bookmark '%s' already exists "
+                                "(use -f to force)") % mark)
+        if ((mark in self._repo.branchmap() or
+             mark == self._repo.dirstate.branch()) and not force):
+            raise error.Abort(
+                _("a bookmark cannot have the name of an existing branch"))
+        if len(mark) > 3 and not force:
+            try:
+                shadowhash = (mark in self._repo)
+            except error.LookupError:  # ambiguous identifier
+                shadowhash = False
+            if shadowhash:
+                self._repo.ui.warn(
+                    _("bookmark %s matches a changeset hash\n"
+                      "(did you leave a -r out of an 'hg bookmark' "
+                      "command?)\n")
+                    % mark)
+
 def _readactive(repo, marks):
     """
     Get the active bookmark. We can have an active bookmark that updates
     itself as we commit. This function returns the name of that bookmark.
     It is stored in .hg/bookmarks.current
diff --git a/mercurial/commands.py b/mercurial/commands.py
index abb42f7..4dd2521 100644
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -955,52 +955,10 @@  def bookmark(ui, repo, *names, **opts):
     rev = opts.get('rev')
     delete = opts.get('delete')
     rename = opts.get('rename')
     inactive = opts.get('inactive')
 
-    def checkconflict(repo, mark, cur, force=False, target=None):
-        if mark in marks and not force:
-            if target:
-                if marks[mark] == target and target == cur:
-                    # re-activating a bookmark
-                    return
-                anc = repo.changelog.ancestors([repo[target].rev()])
-                bmctx = repo[marks[mark]]
-                divs = [repo[b].node() for b in marks
-                        if b.split('@', 1)[0] == mark.split('@', 1)[0]]
-
-                # allow resolving a single divergent bookmark even if moving
-                # the bookmark across branches when a revision is specified
-                # that contains a divergent bookmark
-                if bmctx.rev() not in anc and target in divs:
-                    bookmarks.deletedivergent(repo, [target], mark)
-                    return
-
-                deletefrom = [b for b in divs
-                              if repo[b].rev() in anc or b == target]
-                bookmarks.deletedivergent(repo, deletefrom, mark)
-                if bookmarks.validdest(repo, bmctx, repo[target]):
-                    ui.status(_("moving bookmark '%s' forward from %s\n") %
-                              (mark, short(bmctx.node())))
-                    return
-            raise error.Abort(_("bookmark '%s' already exists "
-                               "(use -f to force)") % mark)
-        if ((mark in repo.branchmap() or mark == repo.dirstate.branch())
-            and not force):
-            raise error.Abort(
-                _("a bookmark cannot have the name of an existing branch"))
-        if len(mark) > 3 and not force:
-            try:
-                shadowhash = (mark in repo)
-            except error.LookupError: # ambiguous identifier
-                shadowhash = False
-            if shadowhash:
-                repo.ui.warn(
-                    _("bookmark %s matches a changeset hash\n"
-                      "(did you leave a -r out of an 'hg bookmark' command?)\n")
-                    % mark)
-
     if delete and rename:
         raise error.Abort(_("--delete and --rename are incompatible"))
     if delete and rev:
         raise error.Abort(_("--rev is incompatible with --delete"))
     if rename and rev:
@@ -1033,11 +991,11 @@  def bookmark(ui, repo, *names, **opts):
                     raise error.Abort(_("only one new bookmark name allowed"))
                 mark = bookmarks.checkformat(repo, names[0])
                 if rename not in marks:
                     raise error.Abort(_("bookmark '%s' does not exist")
                                       % rename)
-                checkconflict(repo, mark, cur, force)
+                marks.checkconflict(mark, force)
                 marks[mark] = marks[rename]
                 if repo._activebookmark == rename and not inactive:
                     bookmarks.activate(repo, mark)
                 del marks[rename]
             elif names:
@@ -1051,11 +1009,11 @@  def bookmark(ui, repo, *names, **opts):
                         bookmarks.deactivate(repo)
                         return
                     tgt = cur
                     if rev:
                         tgt = scmutil.revsingle(repo, rev).node()
-                    checkconflict(repo, mark, cur, force, tgt)
+                    marks.checkconflict(mark, force, tgt)
                     marks[mark] = tgt
                 if not inactive and cur == marks[newact] and not rev:
                     bookmarks.activate(repo, newact)
                 elif cur != tgt and newact == repo._activebookmark:
                     bookmarks.deactivate(repo)