[PATCH 9 of 9] scmutil: add bad character checking to checknewlabel

Wagner Bruna wagner.bruna+mercurial at gmail.com
Fri Oct 19 11:11:41 CDT 2012


Em 18-10-2012 00:31, Kevin Bullock escreveu:
> # HG changeset patch
> # User Kevin Bullock <kbullock at ringworld.org>
> # Date 1350528126 18000
> # Node ID 0f80eecd05b8271461e9da83f5dcc9dee6ef6089
> # Parent  5b18705d6e80c044c3a46a7a3bf728d58b2b276a
> 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.
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -794,11 +794,7 @@ def bookmark(ui, repo, mark=None, rev=No
>          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):
> @@ -5641,7 +5637,7 @@ def tag(ui, repo, name1, *names, **opts)
>          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'))
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -261,7 +261,7 @@ class dirstate(object):
>  
>      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:
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -385,17 +385,12 @@ class localrepository(object):
>      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:
> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -27,9 +27,13 @@ def nochangesfound(ui, repo, excluded=No
>      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)))

The 'kind' became untranslated here. Actually, this message can't be translate
correctly at all, since 'tag', 'bookmark' and 'branch' could have eg.
different grammatical genders in the target language.

Perhaps the parameter could hold a whole message defined by the caller instead?

Regards,
Wagner

>  def checkfilename(f):
>      '''Check that the filename f is an acceptable filename for a tracked file'''
> diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
> --- a/tests/test-bookmarks.t
> +++ b/tests/test-bookmarks.t
> @@ -302,7 +302,12 @@ bookmark name with whitespace only
>  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
> diff --git a/tests/test-branches.t b/tests/test-branches.t
> --- a/tests/test-branches.t
> +++ b/tests/test-branches.t
> @@ -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'
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel



More information about the Mercurial-devel mailing list