changeset 17789:4cfd02c2df9a

bookmarks: check bookmark format during rename (issue3662)

--- a/mercurial/commands.py	Tue Oct 16 13:35:58 2012 -0500
+++ b/mercurial/commands.py	Wed Oct 17 08:44:49 2012 +0200
@@ -789,6 +789,24 @@
     marks = repo._bookmarks
     cur   = repo.changectx('.').node()
 
+    def checkformat(mark):
+        if "\n" in mark:
+            raise util.Abort(_("bookmark name cannot contain newlines"))
+        mark = mark.strip()
+        if not mark:
+            raise util.Abort(_("bookmark names cannot consist entirely of "
+                               "whitespace"))
+        return mark
+
+    def checkconflict(repo, mark, force=False):
+        if mark in marks and not force:
+            raise util.Abort(_("bookmark '%s' already exists "
+                               "(use -f to force)") % mark)
+        if ((mark in repo.branchmap() or mark == repo.dirstate.branch())
+            and not force):
+            raise util.Abort(
+                _("a bookmark cannot have the name of an existing branch"))
+
     if delete:
         if mark is None:
             raise util.Abort(_("bookmark name required"))
@@ -801,13 +819,12 @@
         return
 
     if rename:
+        if mark is None:
+            raise util.Abort(_("new bookmark name required"))
+        mark = checkformat(mark)
         if rename not in marks:
             raise util.Abort(_("bookmark '%s' does not exist") % rename)
-        if mark in marks and not force:
-            raise util.Abort(_("bookmark '%s' already exists "
-                               "(use -f to force)") % mark)
-        if mark is None:
-            raise util.Abort(_("new bookmark name required"))
+        checkconflict(repo, mark, force)
         marks[mark] = marks[rename]
         if repo._bookmarkcurrent == rename and not inactive:
             bookmarks.setcurrent(repo, mark)
@@ -816,22 +833,11 @@
         return
 
     if mark is not None:
-        if "\n" in mark:
-            raise util.Abort(_("bookmark name cannot contain newlines"))
-        mark = mark.strip()
-        if not mark:
-            raise util.Abort(_("bookmark names cannot consist entirely of "
-                               "whitespace"))
+        mark = checkformat(mark)
         if inactive and mark == repo._bookmarkcurrent:
             bookmarks.setcurrent(repo, None)
             return
-        if mark in marks and not force:
-            raise util.Abort(_("bookmark '%s' already exists "
-                               "(use -f to force)") % mark)
-        if ((mark in repo.branchmap() or mark == repo.dirstate.branch())
-            and not force):
-            raise util.Abort(
-                _("a bookmark cannot have the name of an existing branch"))
+        checkconflict(repo, mark, force)
         if rev:
             marks[mark] = scmutil.revsingle(repo, rev).node()
         else:

--- a/tests/test-bookmarks.t	Tue Oct 16 13:35:58 2012 -0500
+++ b/tests/test-bookmarks.t	Wed Oct 17 08:44:49 2012 +0200
@@ -217,12 +217,31 @@
   abort: bookmark name cannot contain newlines
   [255]
 
+  $ hg bookmark -m Z '
+  > '
+  abort: bookmark name cannot contain newlines
+  [255]
+
 bookmark with existing name
 
   $ hg bookmark Z
   abort: bookmark 'Z' already exists (use -f to force)
   [255]
 
+  $ hg bookmark -m Y Z
+  abort: bookmark 'Z' already exists (use -f to force)
+  [255]
+
+bookmark with name of branch
+
+  $ hg bookmark default
+  abort: a bookmark cannot have the name of an existing branch
+  [255]
+
+  $ hg bookmark -m Y default
+  abort: a bookmark cannot have the name of an existing branch
+  [255]
+
 force bookmark with existing name
 
   $ hg bookmark -f Z
@@ -247,6 +266,10 @@
   abort: bookmark names cannot consist entirely of whitespace
   [255]
 
+  $ hg bookmark -m Y ' '
+  abort: bookmark names cannot consist entirely of whitespace
+  [255]
+
 invalid bookmark
 
   $ hg bookmark 'foo:bar'