Re: [PATCH 2/4] ref-filter: add ref-filter API

From: Junio C Hamano <gitster@pobox.com>
Date: May 23, 2015, 17:18 UTC
Message-ID: <xmqqoalbciwj.fsf@gitster.dls.corp.google.com>
In-Reply-To: <vpq8ucffj8h.fsf@anie.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Show 12 quoted lines

> But I think this could be clearer in the code (and/or comment + commit
> message). Perhaps stg like:
>
> struct ref_filter_data /* Probably not the best name */ {
>         struct ref_list list;
>         struct ref_filter filter;
> };
>
> struct ref_list {
>  	int count, alloc;
>  	struct ref_filter_item **items;
> };

If you plan to use ALLOC_GROW() API, name the bookkeeping variables "alloc" & "nr", unless you have a compelling reason to deviate from the prevailing practice.

> struct ref_filter {
> 	const char **name_patterns;
> 	/* There will be more here later */
> };

Very good suggestion.

Whatever the final name would be, it is a good idea to separate the "list of things that are operated on" and the "set of operations to be applied". That makes things conceptually cleaner; you can have multiple of the former operated on with a singleton of the latter and then their results merged, etc. etc.

And I do not think an array of things that are operated on should not be named "ref_filter_item".

Surely, the latter "set of operations to be applied" may currently be only filtering, but who says it has to stay that way? "I have a set of refs that represent my local branches I am interested in. Please map them to their corresponding @{upstream}" is a reasonable request once you have an infrastructure to represent "set of refs to be worked on" and "set of operations to apply", and at that point, the items are no longer filter-items (map-items?).