| 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
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('-', '_')