[PATCH] tag: allow multiple tags to be added or removed

[Thomas Arendsen Hein]

* John Coomes <John.Coomes at sun.com> [20080223 09:33]:
> Patch to allow multiple tags to be added/removed in a single invocation
> of hg tag.  Basic example of the new usage is
> 
> 	hg tag -r 42 build-25 beta-1
> 
> which adds tags 'build-25' and 'beta-1' for rev 42.

Generally I want this, but some comments on the implementation.

> -def tag(ui, repo, name, rev_=None, **opts):
> -    """add a tag for the current or given revision
> +def tag(ui, repo, name1, *othernames, **opts):
> +    """add one or more tags for the current or given revision

Just "name, names" or "name1, names"? We don't have the other...
notation in other parts of the code.

> -    very useful to compare different revision, to go back to significant
> +    very useful to compare different revisions, to go back to significant

Good catch, I committed this change to crew separately.

> +    def abort_nonempty(names, singular_msg, plural_msg):
> +        '''if names is a non-empty list, abort with a message listing the
> +        offending name or names'''
> +        if not names:
> +            return
> +        if len(names) == 1:
> +            raise util.Abort(singular_msg % ("'" + names[0] + "'"))
> +        raise util.Abort(plural_msg % ("'" + "', '".join(names) + "'"))

Generally we try to avoid singular vs. plural messages to make
parsing such messages (automatically and by brain) easier.

> +    abort_nonempty([n for n in names if n in ['tip', '.', 'null']],
> +                   _('the name %s is reserved'),
> +                   _('the names %s are reserved'))

Maybe just loop over reserved names with singular message here.

> +    tag_word = (len(names) > 1 and _('tags')) or _('tag')

Some languages use singular for 21, 31, ... so this will get you (or
others) in translation hell :)

> -         _('hg tag [-l] [-m TEXT] [-d DATE] [-u USER] [-r REV] NAME')),
> +         _('hg tag [-l] [-m TEXT] [-d DATE] [-u USER] [-r REV] NAME ...')),

Just NAME... here.

> -    def tag(self, name, node, message, local, user, date):
> -        '''tag a revision with a symbolic name.
> +    def tag(self, names, node, message, local, user, date):
> +        '''tag a revision with one or more symbolic names.

localrepo.tag() is part of the API, so it should still accept a
single tag as string, too.

Thomas

-- 
thomas at intevation.de - http://intevation.de/~thomas/ - OpenPGP key: 0x5816791A
Intevation GmbH, Osnabrueck - Register: Amtsgericht Osnabrueck, HR B 18998
Geschaeftsfuehrer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner