Re: [WIP/PATCH v4 1/8] for-each-ref: extract helper functions out of grab_single_ref()

[Junio C Hamano <gitster@pobox.com>]

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

>  /*
> + * Given a refname, return 1 if the refname matches with one of the patterns
> + * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
> + * and so on, else return 0. Supports use of wild characters.
> + */
> +static int match_name_as_path(const char **pattern, const char *refname)
> +{

I wonder why this is not "match_refname", in other words, what the
significance of "as path" part of the name?  If you later are going
to introuce match_name_as_something_else() and that name may not be
a refname, then this naming is perfectly sane.  If such a function
you will later introduce will still deal with names that are only
refnames, then match_refname_as_path() would be sensible.  Otherwise
this name seems overly long (i.e. "as_path" may not be adding value)
while not being desctiptive enough (i.e. is meant to be limited to
refnames but just says "name").

Just being curious.

> @@ -851,40 +888,16 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
>  		  return 0;
>  	}
>  
> -	if (*cb->grab_pattern) {
> -...
> -	}
> +	if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
> +		return 0;
>  
> -	/*
> -	 * We do not open the object yet; sort may only need refname
> -	 * to do its job and the resulting list may yet to be pruned
> -	 * by maxcount logic.
> -	 */
> -	ref = xcalloc(1, sizeof(*ref));

The comment still is a good reminder for those who will later touch
this grab_single_ref() function to make them think twice.

Thanks.