On Thu, Mar 03 2022, Junio C Hamano wrote:
> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> [...]
>> +static int parse_opt_whitespace(const struct option *opt, const char *arg, int unset)
>> +{
>> + struct apply_state dummy = { };
>
> It is rare to see a completely empty initializer in this codebase, I
> think.
>
>> + if (parse_whitespace_option(&dummy, arg))
>> + return -1;
>> +
>> + return parse_opt_passthru_argv(opt, arg, unset);
>> +}
>
> Looks good.
>
>> static int git_am_config(const char *k, const char *v, void *cb)
>> {
>> int status;
>> @@ -2355,9 +2365,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>> OPT_CALLBACK_F(0, "quoted-cr", &state.quoted_cr, N_("action"),
>> N_("pass it through git-mailinfo"),
>> PARSE_OPT_NONEG, am_option_parse_quoted_cr),
>> - OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, N_("action"),
>> + OPT_CALLBACK(0, "whitespace", &state.git_apply_opts, N_("action"),
>> N_("pass it through git-apply"),
>> - 0),
>> + parse_opt_whitespace),
>> OPT_PASSTHRU_ARGV(0, "ignore-space-change", &state.git_apply_opts, NULL,
>> N_("pass it through git-apply"),
>> PARSE_OPT_NOARG),
Perfect shouldn't be the enemy of the good & all that, and this is an
improvement.
But having looked at this the general problem is that any
OPT_PASSTHRU_ARGV without a PARSE_OPT_NOARG has the same potential issue
in the case of "am", and I don't see how we can resolve the ambiguity
without calling e.g. parse_whitespace_option(), i.e. we need to call
whatever the validation function is for each one.
This change leaves the same problem in place for --exclude, --include,
and also -C, -p (but one is less likely to do -Cname).
A more general solution would be some continuation of this, i.e. we can
use the "defval" in "struct option" as a pointer to a validation
function for any arguments.
diff --git a/builtin/am.c b/builtin/am.c
index 0f4111bafa0..fa922fda200 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2355,9 +2355,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
OPT_CALLBACK_F(0, "quoted-cr", &state.quoted_cr, N_("action"),
N_("pass it through git-mailinfo"),
PARSE_OPT_NONEG, am_option_parse_quoted_cr),
- OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, N_("action"),
+ OPT_PASSTHRU_ARGV_CHECK(0, "whitespace", &state.git_apply_opts, N_("action"),
N_("pass it through git-apply"),
- 0),
+ 0, parse_whitespace_option),
OPT_PASSTHRU_ARGV(0, "ignore-space-change", &state.git_apply_opts, NULL,
N_("pass it through git-apply"),
PARSE_OPT_NOARG),
diff --git a/parse-options.h b/parse-options.h
index 685fccac137..8348343bf59 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -356,6 +356,8 @@ int parse_opt_tracking_mode(const struct option *, const char *, int);
{ OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), parse_opt_passthru }
#define OPT_PASSTHRU_ARGV(s, l, v, a, h, f) \
{ OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), parse_opt_passthru_argv }
+#define OPT_PASSTHRU_ARGV_CHECK(s, l, v, a, h, f, c) \
+ { OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), parse_opt_passthru_argv, (c) }
#define _OPT_CONTAINS_OR_WITH(name, variable, help, flag) \
{ OPTION_CALLBACK, 0, name, (variable), N_("commit"), (help), \
PARSE_OPT_LASTARG_DEFAULT | flag, \
