Derrick Stolee wrote:

On 8/9/22 7:53 PM, Victoria Dye wrote:

quoted

Derrick Stolee wrote:

quoted

On 8/3/2022 9:45 PM, Victoria Dye via GitGitGadget wrote:

quoted

quoted

quoted

+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