Patchwork [2,of,8,v2] fancyopts: disallow true as a boolean flag default (API)

login
register
mail settings
Submitter Augie Fackler
Date Aug. 30, 2016, 8:16 p.m.
Message ID <e88fe4eb4e8cc3c956ed.1472588175@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/16488/
State Changes Requested
Headers show

Comments

Augie Fackler - Aug. 30, 2016, 8:16 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1472584421 14400
#      Tue Aug 30 15:13:41 2016 -0400
# Node ID e88fe4eb4e8cc3c956ed50b35744a585d51a0e18
# Parent  4f76e8b136581d750cf98395ea8034dbb9666b96
fancyopts: disallow true as a boolean flag default (API)

This was nonsense, as there's not been a way for the user to
un-specify the flag. Restricting this behavior will open the door to
some nice fit-and-finish functionality in a later patch, and shouldn't
break any third party extensions. This is marked as (API) since it
might break a third party extension, but given the fact that it was
silly before that's mostly a formality.

Due to how early parsing of global options works, we have to add some
extra coupling between fancyopts and dispatch so that we can provide a
"default" of True for already-parsed boolean flags when doing
command-level flag parsing.

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -555,7 +555,10 @@  def _parse(ui, args):
 
     # combine global options into local
     for o in commands.globalopts:
-        c.append((o[0], o[1], options[o[1]], o[3]))
+        # Passing a tuple of length six through into the option parser
+        # allows otherwise-illegal defaults to survive, which is how
+        # we handle global options like --quiet.
+        c.append((o[0], o[1], options[o[1]], o[3], '', True))
 
     try:
         args = fancyopts.fancyopts(args, c, cmdoptions, gnu=True)
diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py
--- a/mercurial/fancyopts.py
+++ b/mercurial/fancyopts.py
@@ -67,10 +67,22 @@  def fancyopts(args, options, state, gnu=
     notnegated = set()
 
     for option in options:
-        if len(option) == 5:
+        boolok = False
+        if len(option) == 6:
+            # If the tuple is of length 6, then it's a global option
+            # that was already parsed, so we're really just passing
+            # its "default" through the second phase of flags parsing
+            # (for command-level flags). As a result, we have to allow
+            # defaults of True and not rewrite defaults of False.
+            short, name, default, comment, dummy, dummy = option
+            boolok = True
+        elif len(option) == 5:
             short, name, default, comment, dummy = option
         else:
             short, name, default, comment = option
+        if default is True and not boolok:
+            raise ValueError('fancyopts does not support default-true '
+                             'boolean flags: %r' % name)
         # convert opts to getopt format
         oname = name
         name = name.replace('-', '_')