diff mbox series

[v4,5/9] parse-options: parse into strvec

Message ID 20200909004939.1942347-6-emilyshaffer@google.com
State New
Headers show
Series propose config-based hooks | expand

Commit Message

Emily Shaffer Sept. 9, 2020, 12:49 a.m. UTC
parse-options already knows how to read into a string_list, and it knows
how to read into an strvec as a passthrough (that is, including the
argument as well as its value). string_list and strvec 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
strvec 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 strvec 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(+)

Comments

Jonathan Nieder Oct. 5, 2020, 11:30 p.m. UTC | #1
Emily Shaffer wrote:

> 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 strvec would contain
> { "--quiet", "--format=pretty" }.

An alternative is to use OPT_STRING_LIST and then convert in the
caller.  One advantage of that is that it would guarantee the behavior
with --no-arg etc is going to match exactly.

I prefer this OPT_STRVEC approach nonetheless.  Can the
parse_opt_strvec and parse_opt_string_list functions get comments
pointing to each other as an alternative way to encourage that kind of
consistency?

[...]
> --- 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)`::

nit: this should be OPT_STRVEC

Thanks,
Jonathan
Junio C Hamano Oct. 6, 2020, 4:49 a.m. UTC | #2
Jonathan Nieder <jrnieder@gmail.com> writes:

> Emily Shaffer wrote:
>
>> 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 strvec would contain
>> { "--quiet", "--format=pretty" }.
>
> An alternative is to use OPT_STRING_LIST and then convert in the
> caller.  One advantage of that is that it would guarantee the behavior
> with --no-arg etc is going to match exactly.
>
> I prefer this OPT_STRVEC approach nonetheless.  Can the
> parse_opt_strvec and parse_opt_string_list functions get comments
> pointing to each other as an alternative way to encourage that kind of
> consistency?
>
> [...]
>> --- 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)`::
>
> nit: this should be OPT_STRVEC

Sigh.  I thought I caught all of these with a SQUASH fix-up patch
the last round.  Thanks for being extra careful.
diff mbox series

Patch

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 5a60bbfa7f..b4f1fc4a1a 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 d9d3b0819f..d2b8b7b98a 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_strvec(const struct option *opt, const char *arg, int unset)
+{
+	struct strvec *v = opt->value;
+
+	if (unset) {
+		strvec_clear(v);
+		return 0;
+	}
+
+	if (!arg)
+		return -1;
+
+	strvec_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..177259488b 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_STRVEC(s, l, v, a, h) \
+				    { OPTION_CALLBACK, (s), (l), (v), (a), \
+				      (h), 0, &parse_opt_strvec }
 #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_strvec(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 *,