Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 11, 2015, 17:00 UTC
Message-ID: <vpq1thixjs3.fsf@anie.imag.fr>
In-Reply-To: <1434039003-10928-11-git-send-email-karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

> +	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);

I think it is more common to have options at the end, so I'd write it as

filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);

(changing the declaration too, obviously)

I really like the way cmd_for_each_ref looks like now.

Show 25 quoted lines

> @@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
>  	array->nr = array->alloc = 0;
>  }
>  
> +/*
> + * API for filtering a set of refs. Based on the type of refs the user
> + * has requested, we iterate through those refs and apply filters
> + * as per the given ref_filter structure and finally store the
> + * filtered refs in the ref_array structure.
> + */
> +int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter)
> +{
> +	struct ref_filter_cbdata ref_cbdata;
> +
> +	ref_cbdata.array = array;
> +	ref_cbdata.filter = filter;
> +
> +	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
> +		return for_each_rawref(ref_filter_handler, &ref_cbdata);
> +	else if (type & FILTER_REFS_ALL)
> +		return for_each_ref(ref_filter_handler, &ref_cbdata);
> +	else
> +		die("filter_refs: invalid type");
> +	return 0;
> +}

I thought you would make a helper function that would return a pointer to either for_each_rawref or for_each_ref (or another later), but that would probably be overkill.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/