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

[Yuya Nishihara]

# 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)

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