Re: [PATCH 1/7] add special "matching refs" refspec

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 30, 2008, 09:23 UTC
Message-ID: <7vfxt31ys6.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <af57d23aca6137c1ae7702027ce3742433840872.1209391614.git.bonzini@gnu.org>

Paolo Bonzini <bonzini@gnu.org> writes:

Show 23 quoted lines

> @@ -937,13 +945,23 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
>  						 const struct ref *src)
>  {
>  	int i;
> +	int matching_refs = -1;
>  	for (i = 0; i < rs_nr; i++) {
> +		if (rs[i].matching &&
> +		    (matching_refs == -1 || rs[i].force)) {
> +			matching_refs = i;
> +			continue;
> +		}
> +
>  		if (rs[i].pattern &&
>  		    !prefixcmp(src->name, rs[i].src) &&
>  		    src->name[strlen(rs[i].src)] == '/')
>  			return rs + i;
>  	}
> -	return NULL;
> +	if (matching_refs != -1)
> +		return rs + matching_refs;
> +	else
> +		return NULL;
>  }

It is probably better to document that you would force if you have both "+:" and ":" for the same remote, even though I am not sure if allowing that (instead of diagnosing it as an error) is the right thing to do.

Is it an error to have both ":" and "some:other" refspecs for the same remote? If so who makes the check?

Otherwise this patch seems to be very cleanly done. Especially I like how the updated match_refs() looks.