Thread (70 messages) 70 messages, 4 authors, 2022-08-18

Re: [PATCH v2 08/10] builtin/bugreport.c: create '--diagnose' option

From: Derrick Stolee <hidden>
Date: 2022-08-10 12:52:35

On 8/9/22 7:53 PM, Victoria Dye wrote:
Derrick Stolee wrote:
quoted
On 8/3/2022 9:45 PM, Victoria Dye via GitGitGadget wrote:
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).

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).

Thanks,
-Stolee
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help