| Message ID | 20200728222455.3023400-6-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | propose config-based hooks | expand |
Emily Shaffer <emilyshaffer@google.com> writes: > parse-options already knows how to read into a string_list, and it knows > how to read into an argv_array as a passthrough (that is, including the > argument as well as its value). string_list and argv_array serve similar > purposes but are somewhat painful to convert between; so, let's teach > parse-options to read values of string arguments directly into an > argv_array without preserving the argument name. > > This is useful if collecting generic arguments to pass through to > another command, for example, 'git hook run --arg "--quiet" --arg > "--format=pretty" some-hook'. The resulting argv_array would contain > { "--quiet", "--format=pretty" }. > > The implementation is based on that of OPT_STRING_LIST. Be it argv_array or strvec, I think this is a useful thing to do. I grepped for the users of OPT_STRING_LIST() to see if some of them are better served by this, but none of them stood out as candidates that are particularly good match. > +int parse_opt_argv_array(const struct option *opt, const char *arg, int unset) > +{ > + struct argv_array *v = opt->value; > + > + if (unset) { > + argv_array_clear(v); > + return 0; > + } > + > + if (!arg) > + return -1; I think the calling parse_options() loop would catch this negative return and raise an error, but is it better for this code to stay silent or would it be better to say that opt->long_name/short_name is not a boolean? > + argv_array_push(v, arg); > + return 0; > +}
Junio C Hamano <gitster@pobox.com> writes: > Be it argv_array or strvec, I think this is a useful thing to do. > > I grepped for the users of OPT_STRING_LIST() to see if some of them > are better served by this, but none of them stood out as candidates > that are particularly good match. > >> +int parse_opt_argv_array(const struct option *opt, const char *arg, int unset) >> +{ >> + struct argv_array *v = opt->value; >> + >> + if (unset) { >> + argv_array_clear(v); >> + return 0; >> + } >> + >> + if (!arg) >> + return -1; > > I think the calling parse_options() loop would catch this negative > return and raise an error, but is it better for this code to stay > silent or would it be better to say that opt->long_name/short_name > is not a boolean? I am still waiting for this to be answered, but I queued the whole topic, these last two steps included, just to see how bad adjusting to the strvec API migration would be. It wasn't too bad. I would not recommend you, or other contributors who use argv-array API in their topics, to build on top of jk/strvec, not just yet, as I expect it to go through at least one more reroll to update the details. Thanks.
diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 2e2e7c10c6..1e97343338 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -173,6 +173,11 @@ There are some macros to easily define options: The string argument is stored as an element in `string_list`. Use of `--no-option` will clear the list of preceding values. +`OPT_ARGV_ARRAY(short, long, &struct argv_array, arg_str, description)`:: + Introduce an option with a string argument. + The string argument is stored as an element in `argv_array`. + Use of `--no-option` will clear the list of preceding values. + `OPT_INTEGER(short, long, &int_var, description)`:: Introduce an option with integer argument. The integer is put into `int_var`. diff --git a/parse-options-cb.c b/parse-options-cb.c index 86cd393013..94c2dd397a 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -205,6 +205,22 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_argv_array(const struct option *opt, const char *arg, int unset) +{ + struct argv_array *v = opt->value; + + if (unset) { + argv_array_clear(v); + return 0; + } + + if (!arg) + return -1; + + argv_array_push(v, arg); + return 0; +} + int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) { return 0; diff --git a/parse-options.h b/parse-options.h index 46af942093..e2e2de75c8 100644 --- a/parse-options.h +++ b/parse-options.h @@ -177,6 +177,9 @@ struct option { #define OPT_STRING_LIST(s, l, v, a, h) \ { OPTION_CALLBACK, (s), (l), (v), (a), \ (h), 0, &parse_opt_string_list } +#define OPT_ARGV_ARRAY(s, l, v, a, h) \ + { OPTION_CALLBACK, (s), (l), (v), (a), \ + (h), 0, &parse_opt_argv_array } #define OPT_UYN(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG, &parse_opt_tertiary } #define OPT_EXPIRY_DATE(s, l, v, h) \ @@ -296,6 +299,7 @@ int parse_opt_commits(const struct option *, const char *, int); int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); +int parse_opt_argv_array(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *,
parse-options already knows how to read into a string_list, and it knows how to read into an argv_array as a passthrough (that is, including the argument as well as its value). string_list and argv_array serve similar purposes but are somewhat painful to convert between; so, let's teach parse-options to read values of string arguments directly into an argv_array without preserving the argument name. This is useful if collecting generic arguments to pass through to another command, for example, 'git hook run --arg "--quiet" --arg "--format=pretty" some-hook'. The resulting argv_array would contain { "--quiet", "--format=pretty" }. The implementation is based on that of OPT_STRING_LIST. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- Documentation/technical/api-parse-options.txt | 5 +++++ parse-options-cb.c | 16 ++++++++++++++++ parse-options.h | 4 ++++ 3 files changed, 25 insertions(+)