On 3/28/2022 11:43 AM, Ævar Arnfjörð Bjarmason wrote:
> An earlier version of this commit[3] went behind
> opt_parse_list_objects_filter()'s back by faking up a "struct option"
> before calling it. Let's avoid that and instead create a blessed API
> for this pattern.
...
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index f02d8df1422..4b25287886d 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -285,6 +285,10 @@ int opt_parse_list_objects_filter(const struct option *opt,
> const char *arg, int unset)
> {
> struct list_objects_filter_options *filter_options = opt->value;
> + opt_lof_init init = (opt_lof_init)opt->defval;
> +
> + if (init)
> + filter_options = init(opt->value);
>
> if (unset || !arg)
> list_objects_filter_set_no_filter(filter_options);
> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
> index 90e4bc96252..ffc02d77e76 100644
> --- a/list-objects-filter-options.h
> +++ b/list-objects-filter-options.h
> @@ -104,13 +104,31 @@ void parse_list_objects_filter(
> struct list_objects_filter_options *filter_options,
> const char *arg);
>
> +/**
> + * The opt->value to opt_parse_list_objects_filter() is either a
> + * "struct list_objects_filter_option *" when using
> + * OPT_PARSE_LIST_OBJECTS_FILTER().
> + *
> + * Or, if using no "struct option" field is used by the callback,
> + * except the "defval" which is expected to be an "opt_lof_init"
> + * function, which is called with the "opt->value" and must return a
> + * pointer to the ""struct list_objects_filter_option *" to be used.
> + *
> + * The OPT_PARSE_LIST_OBJECTS_FILTER_INIT() can be used e.g. the
> + * "struct list_objects_filter_option" is embedded in a "struct
> + * rev_info", which the "defval" could be tasked with lazily
> + * initializing. See cmd_pack_objects() for an example.
> + */
> int opt_parse_list_objects_filter(const struct option *opt,
> const char *arg, int unset);
> +typedef struct list_objects_filter_options *(*opt_lof_init)(void *);
> +#define OPT_PARSE_LIST_OBJECTS_FILTER_INIT(fo, init) \
> + { OPTION_CALLBACK, 0, "filter", (fo), N_("args"), \
> + N_("object filtering"), 0, opt_parse_list_objects_filter, \
> + (intptr_t)(init) }
>
> #define OPT_PARSE_LIST_OBJECTS_FILTER(fo) \
> - OPT_CALLBACK(0, "filter", fo, N_("args"), \
> - N_("object filtering"), \
> - opt_parse_list_objects_filter)
> + OPT_PARSE_LIST_OBJECTS_FILTER_INIT((fo), NULL)
I like this way of combining the macros into one implementation
(guarded with a NULL check).
This version looks good to me.
Thanks,
-Stolee
