Patchwork [5,of,8,v2] fancyopts: notice default of false and rewrite it to None (API)

login
register
mail settings
Submitter Augie Fackler
Date Aug. 30, 2016, 8:16 p.m.
Message ID <120b94374a1226a152e4.1472588178@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/16491/
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 1472583470 14400
#      Tue Aug 30 14:57:50 2016 -0400
# Node ID 120b94374a1226a152e46f42df00bcc477f38240
# Parent  c65c0181a9885d3c95e808272c2f609c3a9c8749
fancyopts: notice default of false and rewrite it to None (API)

This opens the door to noticing the difference between "flag not
specified false" and "explicitly false" in an upcoming patch. Marked
as an internal API change since I did have to tweak a couple of spots
in our code that were making assumptions about the nature of falseness.
Pierre-Yves David - Sept. 6, 2016, 3:59 p.m.
On 08/30/2016 10:16 PM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1472583470 14400
> #      Tue Aug 30 14:57:50 2016 -0400
> # Node ID 120b94374a1226a152e46f42df00bcc477f38240
> # Parent  c65c0181a9885d3c95e808272c2f609c3a9c8749
> fancyopts: notice default of false and rewrite it to None (API)
>
> This opens the door to noticing the difference between "flag not
> specified false" and "explicitly false" in an upcoming patch. Marked
> as an internal API change since I did have to tweak a couple of spots
> in our code that were making assumptions about the nature of falseness.
>
> diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py
> --- a/mercurial/fancyopts.py
> +++ b/mercurial/fancyopts.py
> @@ -80,9 +80,15 @@ def fancyopts(args, options, state, gnu=
>              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)
> +        if not boolok:
> +            if default is True:

I'm strongly opposed to identity testing to True and False. We should 
identity test to None whenever possible to True and False are a bit too 
magical and may be validly replaced by many thing. I really don't thing 
we should identity test them.

Cheers,

Patch

diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py
--- a/mercurial/fancyopts.py
+++ b/mercurial/fancyopts.py
@@ -80,9 +80,15 @@  def fancyopts(args, options, state, gnu=
             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)
+        if not boolok:
+            if default is True:
+                raise ValueError('fancyopts does not support default-true '
+                                 'boolean flags: %r' % name)
+            if default is False:
+                # So higher layers of hg can identify the difference
+                # between unspecified and explicitly-false, set this to
+                # None here.
+                default = None
         # convert opts to getopt format
         oname = name
         name = name.replace('-', '_')