Re: [WIP/PATCH v4 6/8] for-each-ref: rename some functions and make them public

From: Junio C Hamano <gitster@pobox.com>
Date: May 31, 2015, 17:48 UTC
Message-ID: <xmqqmw0kab94.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1433008411-8550-6-git-send-email-karthik.188@gmail.com>

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

Show 8 quoted lines

>  /*
> - * 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.