[PATCH 2 of 7 v3 flags] fancyopts: disallow true as a boolean flag default (API)

[Pierre-Yves David]

On 09/16/2016 04:16 PM, Yuya Nishihara wrote:
> On Fri, 16 Sep 2016 15:48:32 +0200, Pierre-Yves David wrote:
>> On 09/14/2016 06:37 PM, Yuya Nishihara wrote:
>>> On Wed, 14 Sep 2016 11:52:59 -0400, Augie Fackler wrote:
>>>>> On Sep 14, 2016, at 11:30, Yuya Nishihara <yuya at tcha.org> wrote:
>>>> Is the idea something like:
>>>>
>>>> fancyopts.py:
>>>>
>>>> unsetbool = object()
>>>>
>>>> in commands.py:
>>>>
>>>>  diffopts = [
>>>> -     ('a', 'text', None, _('treat all files as text')),
>>>> +     ('a', 'text', fancyopts.unsetbool, _('treat all files as text')),
>>>> -     ('g', 'git', None, _('use git extended diff format')),
>>>> +     ('g', 'git', fancyopts.unsetbool, _('use git extended diff format')),
>>>> -     ('', 'nodates', None, _('omit dates from diff headers'))
>>>> +     ('', 'nodates', fancyopts.unsetbool, _('omit dates from diff headers'))
>>>>  ]
>>>
>>> Yes.
>>>
>>>> And then we could synthesize negated versions only for booleans that have
>>>> fancyopts.unsetbool as the default?
>>>
>>> No. My idea is to not set 'git' at all.
>>>
>>>   default:  {}  # not {'git': None}
>>>   --git:    {'git': True}
>>>   --no-git: {'git': False}
>>>
>>> and falls back to ui.configbool if 'git' not in opts.
>>>
>>> My point is diffopts are special in that their defaults are configurable. Many
>>> other boolean options have no such feature, so they can simply be default=None
>>> (or False.)
>>
>> So far, the option parsing logic have ensure options always exist and
>> have some value in the 'opts' dictionary. Changing this would be a
>> massive API breakage. I really don't think we should go this route.
>
> That's why 'unsetbool' will be opt-in. We sometimes do opts.get() to support
> internal function calls not through dispatch(), so I think it's okay to omit
> explicitly-defined keys.
>
> That said, I'm not opposed to the special defaulttrue/false objects.
>
> Another crazy idea is to add 'bool_to_be_read_from_ui(section, name)' and pass
> ui to fancyopts.
>
>   ('g', 'git', bool_to_be_read_from_ui('diff', 'git'), ...)
>
> Good thing is "help diff" can show --no-git if diff.git is set, but this would
> be overly complicated.

marrying the command and config default option is definitly tempting, 
but I think we should keep this out of this first step for the sake of 
simplicity. In particular, we should probably have some centralised 
registry of config option in the process of making the advanced version 
happen.

Cheers.

-- 
Pierre-Yves David