[PATCH 3 of 5] revset: drop redundant check for unknown alias arguments
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Tue Mar 29 20:36:37 EDT 2016
On 03/29/2016 08:27 AM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1455449228 -32400
> # Sun Feb 14 20:27:08 2016 +0900
> # Node ID e89acb348d532903e002bf834609472f9a50bdfb
> # Parent fa7880cac50272f805ae1a233a477b0a9f56a585
> revset: drop redundant check for unknown alias arguments
>
> Since _parsealiasdefn() rejects unknown alias arguments, _checkaliasarg() is
> unnecessary. New test is added to make sure unknown '$n' symbols are rejected.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -2267,17 +2267,6 @@ def _getaliasarg(tree):
> return tree[1]
> return None
>
> -def _checkaliasarg(tree, known=None):
> - """Check tree contains no _aliasarg construct or only ones which
> - value is in known. Used to avoid alias placeholders injection.
> - """
> - if isinstance(tree, tuple):
> - arg = _getaliasarg(tree)
> - if arg is not None and (not known or arg not in known):
> - raise error.UnknownIdentifier('_aliasarg', [])
> - for t in tree:
> - _checkaliasarg(t, known)
> -
> # the set of valid characters for the initial letter of symbols in
> # alias declarations and definitions
> _aliassyminitletters = set(c for c in [chr(i) for i in xrange(256)]
> @@ -2443,8 +2432,6 @@ class revsetalias(object):
>
> try:
> self.replacement = _parsealiasdefn(value, self.args)
> - # Check for placeholder injection
> - _checkaliasarg(self.replacement, self.args)
> except error.ParseError as inst:
> self.error = _('failed to parse the definition of revset alias'
> ' "%s": %s') % (self.name, parseerrordetail(inst))
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -1820,6 +1820,15 @@ but 'all()' should never be substituded
> <spanset+ 0:9>>
> 0
>
> +test unknown reference:
> +
> + $ try "unknownref(0)" --config 'revsetalias.unknownref($1)=$1:$2'
> + (func
> + ('symbol', 'unknownref')
> + ('symbol', '0'))
> + abort: failed to parse the definition of revset alias "unknownref": '$' not for alias arguments
> + [255]
This abort is complaining about something without naming it. Could we
get updated to: '$' not for alias arguments "$2"
This patches seems to just show the issue (by adding a test, great!),
not actually introducing it so I've taken the series, can you follow up
on this ?
Cheers,
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list