But it also contains struct ref_filter_item **items, which as I
understand it contains a list of refs (with name, sha1 & such).
That's the part I do not find natural: the same structure contains both
the list of refs and the way it should be filtered.
Re-reading the patch, I seem to understand that you're putting both on
the same struct because of the API of for_each_ref() which takes one
'data' pointer to be casted, so you want both the input (filter
description) and the output (list of refs after filtering) to be
contained in the same struct.
Was kinda confused, This clears out things, Thanks.
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;
const char **name_patterns;
};
struct ref_filter {
const char **name_patterns;
/* There will be more here later */
};
This seems cleaner, agreed.
>
> I agree that it might be clearer to separate both. In this case
> instead of "ref_list" the struct might be called "ref_filter_array" as
> we already have "argv_array" in argv-array.h and "sha1_array" in
> "sha1-array.h".
>
Somehow ref_list seems more real to me, list of refs.
>
> 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?).
>
That's also a good point to consider, I shall rename and restructure the
code as discussed here, thanks.
--
Regards,
Karthik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
