Re: [PATCH 2/3] Move refspec parser from connect.c and cache.h to remote.{c,h}

From: Junio C Hamano <junkio@cox.net>
Date: May 12, 2007, 07:32 UTC
Message-ID: <7v7ire5wf4.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0705112238420.18541@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 17 quoted lines

> diff --git a/remote.c b/remote.c
> index 1dd2e77..2ac7bc8 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -181,6 +181,44 @@ static void read_config(void)
> + ...
> +static int match_explicit_refs(struct ref *src, struct ref *dst,
> +			       struct ref ***dst_tail, struct refspec *rs,
> +			       int rs_nr)
> +{
> +	int i, errs;
> +	for (i = errs = 0; i < rs_nr; i++) {
> +		struct ref *matched_src, *matched_dst;
> +
> +		const char *dest = rs[i].dest;
> +		if (dest == NULL)
> +			dest = rs[i].src;

I am not a big fan of gratuitous renaming of structure members and variables such as this s/dst/dest/, especially when the change breaks consistency (between src vs dst -- you left matched_dst intact).

Show 12 quoted lines

> ...
> +		switch (count_refspec_match(dest, dst, &matched_dst)) {
> ...
> +		case 0:
> +			if (!memcmp(dest, "refs/", 5)) {
> ...
> +			else {
> +				errs = 1;
> +				error("dst refspec %s does not match any "
> +				      "existing ref on the remote and does "
> +				      "not start with refs/.", rs[i].dest);
> +			}

I think you meant to say 'dest' instead of 'rs[i].dest' here.

> +...
> +	}
> +	return -errs;
> +}