[PATCH 4 of 5 STABLE RFC] dispatch: ignore --early-bool-option that might not be a flag (BC)

[Augie Fackler]

On Wed, Nov 15, 2017 at 09:54:23PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1510321154 -32400
> #      Fri Nov 10 22:39:14 2017 +0900
> # Branch stable
> # Node ID c2c34cf080aefbd983320bdaca111ebbd0826ff3
> # Parent  5838a6130d07b588e4640542865e3c175a124bff
> dispatch: ignore --early-bool-option that might not be a flag (BC)

This one worries me less. Maybe we can land it?

>
> The basic idea is if the preceding arg doesn't look like a flag taking a
> value, the current arg can be parsed as a flag.
>
>   hg ci -m --foo --config      # bad: --foo may be a flag
>   hg ci -m --verbose --config  # good: --verbose known to take no value
>                                # whether it is a value for -m or a flag
>
> --debugger/--profile/--traceback are all for developers. They don't need to
> be any fancy. So let's turn it to safer side.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -106,6 +106,22 @@ globalopts = [
>  # TODO: perhaps --debugger should be included
>  earlyoptflags = ("--cwd", "-R", "--repository", "--repo", "--config")
>
> +# core options known to be always boolean type (i.e. takes no value)
> +corebooloptflags = {
> +    '-y', '--noninteractive',
> +    '-q', '--quiet',
> +    '-v', '--verbose',
> +    '--debug',
> +    '--debugger',
> +    '--traceback',
> +    '--time',
> +    '--profile',
> +    '--version',
> +    '-h', '--help',
> +    '--hidden',
> +    '--mq',  # mostly global as mqopt is inserted into all repo commands
> +}
> +
>  dryrunopts = cmdutil.dryrunopts
>  remoteopts = cmdutil.remoteopts
>  walkopts = cmdutil.walkopts
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -644,6 +644,13 @@ def _parseconfig(ui, config):
>
>      return configs
>
> +def _argmaytakevalue(arg):
> +    """True if the given arg looks like an option flag taking the next item
> +    as its value"""
> +    return (arg.startswith('-')
> +            and not (arg.startswith('--') and '=' in arg)  # --opt=val
> +            and arg not in commands.corebooloptflags)
> +
>  def _earlygetopt(aliases, args, strip=True):
>      """Return list of values for an option (or aliases).
>
> @@ -751,17 +758,48 @@ def _earlyreqoptbool(req, name, aliases)
>
>      >>> req = request([b'x', b'--', b'--debugger'])
>      >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +
> +    Options can't appear immediately after any option-like flag:
> +
> +    >>> req = request([b'x', b'-z', b'--debugger'])
> +    >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +
> +    >>> req = request([b'x', b'-z', b'--zzz', b'--debugger'])
> +    >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +
> +    unless the preceding flag is known to be a boolean:
> +
> +    >>> req = request([b'x', b'-v', b'--debugger'])
> +    >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +    True
> +
> +    >>> req = request([b'x', b'--debug', b'--debugger'])
> +    >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +    True
> +
> +    >>> req = request([b'x', b'-z', b'--debugger', b'--debugger'])
> +    >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +    True
> +
> +    or it takes an immediate value:
> +
> +    >>> req = request([b'x', b'--foo=bar', b'--debugger'])
> +    >>> _earlyreqoptbool(req, b'debugger', [b'--debugger'])
> +    True
>      """
>      try:
>          argcount = req.args.index("--")
>      except ValueError:
>          argcount = len(req.args)
>      value = None
> +    inopt = False
>      pos = 0
>      while pos < argcount:
>          arg = req.args[pos]
> -        if arg in aliases:
> +        if arg in aliases and not inopt:
>              value = True
> +        else:
> +            inopt = _argmaytakevalue(arg)
>          pos += 1
>      req.earlyoptions[name] = value
>      return value
> @@ -897,7 +935,8 @@ def _dispatch(req):
>                  "option -R has to be separated from other options (e.g. not "
>                  "-qR) and --repository may only be abbreviated as --repo!"))
>          if options["debugger"] != req.earlyoptions["debugger"]:
> -            raise error.Abort(_("option --debugger may not be abbreviated!"))
> +            raise error.Abort(_("option --debugger may not be abbreviated "
> +                                "and should come very first!"))
>          # don't validate --profile/--traceback, which can be enabled from now
>
>          if options["encoding"]:
> diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
> --- a/tests/test-dispatch.t
> +++ b/tests/test-dispatch.t
> @@ -54,7 +54,12 @@ Parsing of early options should stop at
>  Unparsable form of early options:
>
>    $ hg cat --debugg
> -  abort: option --debugger may not be abbreviated!
> +  abort: option --debugger may not be abbreviated and should come very first!
> +  [255]
> +  $ hg log -T --debugger
> +  --debugger (no-eol)
> +  $ hg log -T -T --debugger
> +  abort: option --debugger may not be abbreviated and should come very first!
>    [255]
>
>  Parsing failure of early options should be detected before executing the
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel