Patchwork [6,of,8,v2] fancyopts: rearrange takes-value if statement

mail settings
Submitter Augie Fackler
Date Aug. 30, 2016, 8:16 p.m.
Message ID <>
Download mbox | patch
Permalink /patch/16492/
State Changes Requested
Headers show


Augie Fackler - Aug. 30, 2016, 8:16 p.m.
# HG changeset patch
# User Augie Fackler <>
# Date 1472583543 14400
#      Tue Aug 30 14:59:03 2016 -0400
# Node ID 6127fee6ac8208a62f718d53aca6cb814f43b742
# Parent  120b94374a1226a152e46f42df00bcc477f38240
fancyopts: rearrange takes-value if statement

Now that we're doing something on both sides of the if statement, we
may as well drop the negation and clarify the comments a little bit.

It's tempting to discard the 'is False/True' parts of this if
statement, but they remain important, since early option parsing means
that "defaults" of True or False can propagate through here as a
result of global option parsing.


diff --git a/mercurial/ b/mercurial/
--- a/mercurial/
+++ b/mercurial/
@@ -104,20 +104,21 @@  def fancyopts(args, options, state, gnu=
             state[name] = default
-        # does it take a parameter?
-        if not (default is None or default is True or default is False):
+        if default is None or default is False or default is True:
+            # flag is a boolean flag, so insert a --no-$option variant
+            # of the flag
             if oname.startswith('no-'):
             # Note slight weirdness: this means flags like --no-backup
             # for revert can be negated with --no-no-backup. This
             # seems fine.
             namelist.append('no-' + oname)
+        else:
+            # flag is a non-boolean flag, and so takes a parameter
             if short:
                 short += ':'
             if oname:
                 oname += '='
-        else:
-            namelist.append('no-' + oname)
         if short:
             shortlist += short
         if name: