Re: [PATCH 09/17] refs: generalize `refs_for_each_fullref_in_prefixes()`

> The function `refs_for_each_fullref_in_prefixes()` can be used to
> iterate over all references part of any of the user-provided prefixes.
> In contrast to the `prefix` parameter of `refs_for_each_ref_ext()` it
> knows to handle the case well where multiple of the passed-in prefixes
> start with a common prefix by computing longest common prefixes and then
> iterating over those.
>
> While we could move this logic into `refs_for_each_ref_ext()`, this one
> feels somewhat special as we perform multiple iterations. But what we
> _can_ do is to generalize how this function works: instead of accepting
> only a small handful of parameters, we can have it accept the full
> options structure.
>
> One obvious exception is that the caller must not provide a prefix via
> the options. But this case can be easily detected.
>
> Refactor the code accordingly.
>

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  ls-refs.c    | 11 +++++++----
>  ref-filter.c | 11 +++++++----
>  refs.c       | 39 +++++++++++++++------------------------
>  refs.h       | 16 +++++-----------
>  4 files changed, 34 insertions(+), 43 deletions(-)
>
> diff --git a/ls-refs.c b/ls-refs.c
> index 8641281b86..9759826ca7 100644
> --- a/ls-refs.c
> +++ b/ls-refs.c
> @@ -160,6 +160,7 @@ static int ls_refs_config(const char *var, const char *value,
>
>  int ls_refs(struct repository *r, struct packet_reader *request)
>  {
> +	struct refs_for_each_ref_options opts = { 0 };
>  	struct ls_refs_data data;
>
>  	memset(&data, 0, sizeof(data));
> @@ -201,10 +202,12 @@ int ls_refs(struct repository *r, struct packet_reader *request)
>  	send_possibly_unborn_head(&data);
>  	if (!data.prefixes.nr)
>  		strvec_push(&data.prefixes, "");
> -	refs_for_each_fullref_in_prefixes(get_main_ref_store(r),
> -					  get_git_namespace(), data.prefixes.v,
> -					  hidden_refs_to_excludes(&data.hidden_refs),
> -					  send_ref, &data);
> +
> +	opts.exclude_patterns = hidden_refs_to_excludes(&data.hidden_refs);
> +	opts.namespace = get_git_namespace();
> +
> +	refs_for_each_ref_in_prefixes(get_main_ref_store(r), data.prefixes.v,
> +				      &opts, send_ref, &data);
>  	packet_fflush(stdout);
>  	strvec_clear(&data.prefixes);
>  	strbuf_release(&data.buf);
> diff --git a/ref-filter.c b/ref-filter.c
> index 049e845a19..7c682e0a33 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -2807,6 +2807,10 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
>  				       refs_for_each_cb cb,
>  				       void *cb_data)
>  {
> +	struct refs_for_each_ref_options opts = {
> +		.exclude_patterns = filter->exclude.v,
> +	};
> +
>  	if (filter->kind & FILTER_REFS_ROOT_REFS) {
>  		/* In this case, we want to print all refs including root refs. */
>  		return for_each_fullref_with_seek(filter, cb, cb_data,
> @@ -2836,10 +2840,9 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
>  		return for_each_fullref_with_seek(filter, cb, cb_data, 0);
>  	}
>
> -	return refs_for_each_fullref_in_prefixes(get_main_ref_store(the_repository),
> -						 NULL, filter->name_patterns,
> -						 filter->exclude.v,
> -						 cb, cb_data);
> +	return refs_for_each_ref_in_prefixes(get_main_ref_store(the_repository),
> +					     filter->name_patterns, &opts,
> +					     cb, cb_data);
>  }
>
>  /*
> diff --git a/refs.c b/refs.c
> index 99994879d9..20d34faeb5 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2029,40 +2029,31 @@ static void find_longest_prefixes(struct string_list *out,
>  	strbuf_release(&prefix);
>  }
>
> -int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store,
> -				      const char *namespace,
> -				      const char **patterns,
> -				      const char **exclude_patterns,
> -				      refs_for_each_cb fn, void *cb_data)
> +int refs_for_each_ref_in_prefixes(struct ref_store *ref_store,
> +				  const char **prefixes,
> +				  const struct refs_for_each_ref_options *opts,
> +				  refs_for_each_cb cb, void *cb_data)
>  {
> -	struct strvec namespaced_exclude_patterns = STRVEC_INIT;
> -	struct string_list prefixes = STRING_LIST_INIT_DUP;
> +	struct string_list longest_prefixes = STRING_LIST_INIT_DUP;
>  	struct string_list_item *prefix;
> -	struct strbuf buf = STRBUF_INIT;
> -	int ret = 0, namespace_len;
> +	int ret = 0;
>
> -	find_longest_prefixes(&prefixes, patterns);
> +	if (opts->prefix)
> +		BUG("refs_for_each_ref_in_prefixes called with specific prefix");
>
> -	if (namespace)
> -		strbuf_addstr(&buf, namespace);
> -	namespace_len = buf.len;
> +	find_longest_prefixes(&longest_prefixes, prefixes);
>
> -	exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns,
> -							   namespace,
> -							   &namespaced_exclude_patterns);
> +	for_each_string_list_item(prefix, &longest_prefixes) {
> +		struct refs_for_each_ref_options prefix_opts = *opts;
> +		prefix_opts.prefix = prefix->string;
>
> -	for_each_string_list_item(prefix, &prefixes) {
> -		strbuf_addstr(&buf, prefix->string);
> -		ret = refs_for_each_fullref_in(ref_store, buf.buf,
> -					       exclude_patterns, fn, cb_data);
> +		ret = refs_for_each_ref_ext(ref_store, cb, cb_data,
> +					    &prefix_opts);