Derrick Stolee wrote:
> On 8/9/22 7:53 PM, Victoria Dye wrote:
>> Derrick Stolee wrote:
>>> On 8/3/2022 9:45 PM, Victoria Dye via GitGitGadget wrote:
>
>>>> +static int option_parse_diagnose(const struct option *opt,
>>>> + const char *arg, int unset)
>>>> +{
>>>> + enum diagnose_mode *diagnose = opt->value;
>>>> +
>>>> + BUG_ON_OPT_NEG(unset);
>>>> +
>>>> + if (!arg || !strcmp(arg, "basic"))
>>>> + *diagnose = DIAGNOSE_BASIC;
>>>> + else if (!strcmp(arg, "all"))
>>>> + *diagnose = DIAGNOSE_ALL;
>>>
>>> Should we allow "none" to reset the value to DIAGNOSE_NONE?
>>
>> As far as I can tell, while some builtins have options that match the
>> default behavior of the command (e.g., '--no-autosquash' in 'git rebase'),
>> those options typically exist to override a config setting (e.g.,
>> 'rebase.autosquash'). No config exists for 'bugreport --diagnose' (and I
>> don't think it would make sense to add one), so '--diagnose=none' would only
>> be used to override another '--diagnose' specification in the same
>> command/alias (e.g., 'git bugreport --diagnose=basic --diagnose=none').
>
> Ah, so --diagnose=none isn't valuable because --no-diagnose would be
> the better way to write the same thing. You would need to remove the
> PARSE_OPT_NONEG from your OPT_CALLBACK_F() to allow that (and then do
> the appropriate logic with the "unset" parameter).
I'm not sure I follow. I wasn't suggesting a difference in value between
'--no-diagnose' and '--diagnose=none'. My point was that, when there's an
option variant that "resets" the value to the default (like
'--no-autosquash', '--no-recurse-submodules', etc.), it usually *also*
corresponds to an overridable config setting ('rebase.autosquash',
'push.recurseSubmodules'). No such 'bugreport.diagnose' config exists (or,
IMO, should exist), so the need for a "reset to default" option seemed
weaker.
I used boolean options as my examples, but they aren't intended to imply a
meaningful difference between '--no-diagnose' amd '--diagnose=none'.
>
> The reason to have these things is basically so users can create
> aliases (say 'git br' expands to 'git bugreport --diagnose=all', but
> they want to run 'git br --no-diagnose' to clear that --diagnose=all).
I considered that usage ("'--diagnose=none' would only be used to override
another '--diagnose' specification in the same command/alias"), but wasn't
sure how common it would be for this particular option. It sounds like you
can see it being useful, so I'll include '--diagnose=none' in the next
version.
>
> Thanks,
> -Stolee
