On 05/31/2015 11:18 PM, Junio C Hamano wrote:
Karthik Nayak <karthik....@gmail.com> writes:

  /*
- * A call-back given to for_each_ref().  Filter refs and keep them for
+ * A call-back given to for_each_ref(). Filter refs and keep them for
   * later object processing.
   */
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+int ref_filter_handler(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
  {

I am not sure if this is a good external interface, i.e. to expect
callers of ref-filter API to do this:

        prepare cbdata;
        for_each_ref(ref_filter_handler, &cbdata);

It might be better to expect callers to do this instead:

        prepare cbdata;
         filter_refs(for_each_ref, &cbdata);

i.e. introducing a new "filter_refs()" function as the entry point
to the ref-filter API.  The filter_refs() entry point may internally
use ref_filter_handler() that will be file-scope static to ref-filter.c
and at that point the overly generic "-handler" name would not bother
anybody ;-) but more importantly, then you can extend the function
signature of filter_refs() not to be so tied to for_each_ref() API.
It could be that the internals of cbdata may not be something the
callers of filter-refs API does not even have to know about, in
which case the call might even become something like:

        struct ref_array refs = REF_ARRAY_INIT;
         const char **ref_patterns = { "refs/heads/*", "refs/tags/*", NULL};

        filter_refs(&refs, for_each_rawref, ref_patterns);

         /* now "refs" has the result, the caller uses them */
        for (i = 0; i < refs.nr; i++)
                refs.item[i];

Just a thought.


Thats brilliant, How about I introduce something of this sort

int filter_refs(int (*for_each_ref_fn)(each_ref_fn, void *), ref_filter_cbdata *cbdata)
{
        return for_each_ref_fn(ref_filter_handler, cbdata);
}

where its the most basic form, and things like

>
>    struct ref_array refs = REF_ARRAY_INIT;
> const char **ref_patterns = { "refs/heads/*", "refs/tags/*", NULL};
>
>    filter_refs(&refs, for_each_rawref, ref_patterns);
>
>          /* now "refs" has the result, the caller uses them */
>    for (i = 0; i < refs.nr; i++)
>            refs.item[i];
>

Could be achieved using a simple wrapper around 'filter_refs()' something like this perhaps.

int filter_refs_with_pattern(struct ref_array *ref, int (*for_each_ref_fn)(each_ref_fn, void *), char **patterns)
{
        int i;
        struct ref_filter_cbdata data;
        data.filter.name_patterns = patterns;
        filter_refs(for_each_ref_fn, &data);
        refs->nr = data.array.nr;
        for(i = 0; i < refs->nr; i++) {
                /* copy over the refs */
        }
        return 0;
}

Is this on the lines of what you had in mind? If it is, than I could just create a new patch which would make ref_filter_handler() private and introduce filter_refs() as shown.

--
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

Reply via email to