scmutil: add bad character checking to checknewlabel
authorKevin Bullock <kbullock@ringworld.org>
Wed, 17 Oct 2012 21:42:06 -0500
changeset 17821 361ab1e2086f
parent 17820 c547e1acc37c
child 17822 c284085d17a8
scmutil: add bad character checking to checknewlabel This factors out the checks from tags and bookmarks, and newly applies the same prohibitions to branches. checknewlabel takes a new parameter, kind, indicating the kind of label being checked. Test coverage is added for all three types of labels.
mercurial/commands.py
mercurial/dirstate.py
mercurial/localrepo.py
mercurial/scmutil.py
tests/test-bookmarks.t
tests/test-branches.t
--- a/mercurial/commands.py	Wed Oct 17 21:39:07 2012 -0500
+++ b/mercurial/commands.py	Wed Oct 17 21:42:06 2012 -0500
@@ -794,11 +794,7 @@
         if not mark:
             raise util.Abort(_("bookmark names cannot consist entirely of "
                                "whitespace"))
-        for c in (':', '\0', '\n', '\r'):
-            if c in mark:
-                raise util.Abort(_("bookmark '%s' contains illegal "
-                    "character" % mark))
-        scmutil.checknewlabel(repo, mark)
+        scmutil.checknewlabel(repo, mark, 'bookmark')
         return mark
 
     def checkconflict(repo, mark, force=False):
@@ -5645,7 +5641,7 @@
         if len(names) != len(set(names)):
             raise util.Abort(_('tag names must be unique'))
         for n in names:
-            scmutil.checknewlabel(repo, n)
+            scmutil.checknewlabel(repo, n, 'tag')
             if not n:
                 raise util.Abort(_('tag names cannot consist entirely of '
                                    'whitespace'))
--- a/mercurial/dirstate.py	Wed Oct 17 21:39:07 2012 -0500
+++ b/mercurial/dirstate.py	Wed Oct 17 21:42:06 2012 -0500
@@ -261,7 +261,7 @@
 
     def setbranch(self, branch):
         # no repo object here, just check for reserved names
-        scmutil.checknewlabel(None, branch)
+        scmutil.checknewlabel(None, branch, 'branch')
         self._branch = encoding.fromlocal(branch)
         f = self._opener('branch', 'w', atomictemp=True)
         try:
--- a/mercurial/localrepo.py	Wed Oct 17 21:39:07 2012 -0500
+++ b/mercurial/localrepo.py	Wed Oct 17 21:42:06 2012 -0500
@@ -385,17 +385,9 @@
     def hook(self, name, throw=False, **args):
         return hook.hook(self.ui, self, name, throw, **args)
 
-    tag_disallowed = ':\0\r\n'
-
     def _tag(self, names, node, message, local, user, date, extra={}):
         if isinstance(names, str):
-            allchars = names
             names = (names,)
-        else:
-            allchars = ''.join(names)
-        for c in self.tag_disallowed:
-            if c in allchars:
-                raise util.Abort(_('%r cannot be used in a tag name') % c)
 
         branches = self.branchmap()
         for name in names:
--- a/mercurial/scmutil.py	Wed Oct 17 21:39:07 2012 -0500
+++ b/mercurial/scmutil.py	Wed Oct 17 21:42:06 2012 -0500
@@ -27,9 +27,13 @@
     else:
         ui.status(_("no changes found\n"))
 
-def checknewlabel(repo, lbl):
+def checknewlabel(repo, lbl, kind):
     if lbl in ['tip', '.', 'null']:
         raise util.Abort(_("the name '%s' is reserved") % lbl)
+    for c in (':', '\0', '\n', '\r'):
+        if c in lbl:
+            raise util.Abort(_("%r cannot be used in a %s name") %
+                               (c, kind))
 
 def checkfilename(f):
     '''Check that the filename f is an acceptable filename for a tracked file'''
--- a/tests/test-bookmarks.t	Wed Oct 17 21:39:07 2012 -0500
+++ b/tests/test-bookmarks.t	Wed Oct 17 21:42:06 2012 -0500
@@ -302,7 +302,12 @@
 invalid bookmark
 
   $ hg bookmark 'foo:bar'
-  abort: bookmark 'foo:bar' contains illegal character
+  abort: ':' cannot be used in a bookmark name
+  [255]
+
+  $ hg bookmark 'foo
+  > bar'
+  abort: '\n' cannot be used in a bookmark name
   [255]
 
 the bookmark extension should be ignored now that it is part of core
--- a/tests/test-branches.t	Wed Oct 17 21:39:07 2012 -0500
+++ b/tests/test-branches.t	Wed Oct 17 21:42:06 2012 -0500
@@ -45,6 +45,8 @@
   (branches are permanent and global, did you want a bookmark?)
   $ hg commit -d '5 0' -m "Adding c branch"
 
+reserved names
+
   $ hg branch tip
   abort: the name 'tip' is reserved
   [255]
@@ -55,6 +57,17 @@
   abort: the name '.' is reserved
   [255]
 
+invalid characters
+
+  $ hg branch 'foo:bar'
+  abort: ':' cannot be used in a branch name
+  [255]
+
+  $ hg branch 'foo
+  > bar'
+  abort: '\n' cannot be used in a branch name
+  [255]
+
   $ echo 'd' >d
   $ hg add d
   $ hg branch 'a branch name much longer than the default justification used by branches'