Subject: Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines

From: Junio C Hamano <[email-masked]>
Date: Fri, 03 Jun 2016 10:00:24 -0700

Nguyễn Thái Ngọc Duy  <[email-masked]> writes:

> +static int common_suffix_length(const char *a, const char *b)
> +{
> +	const char *pa = a + strlen(a);
> +	const char *pb = b + strlen(b);
> +	int count = 0;
> +
> +	while (pa > a && pb > b && pa[-1] == pb[-1]) {
> +		pa--;
> +		pb--;
> +		count++;
> +	}
> +
> +	/* stick to '/' boundary, do not break in the middle of a word */
> +	while (count) {
> +		if (*pa == '/' ||
> +		    (pa == a && pb > b && pb[-1] == '/') ||
> +		    (pb == b && pa > a && pa[-1] == '/'))
> +			break;
> +		pa++;
> +		pb++;
> +		count--;
> +	}
> +
> +	return count;
> +}
> +

Why do you need two loops, one going backward from the tail and then
going forward toward '/'?  Wouldn't it be sufficient to keep track
of the last slash you saw in a while scanning backwards?  I.e
something along the lines of:

	tail_a = a + strlen(a);
	for (pa = tail_a, pb = b + strlen(b), slash_in_a = NULL;
             a < pa && b < pb && pa[-1] == pb[-1];
	     pa--, pb--) {
		if (*pa == '/')
			slash_in_a = pa;
	}
	count = a + strlen(a) - slash_in_a;
		
perhaps?

> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 454d896..9a7649c 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -222,11 +222,11 @@ test_expect_success 'fetch uses remote ref names to describe new refs' '
>  	(
>  		cd descriptive &&
>  		git fetch o 2>actual &&
> -		grep " -> refs/crazyheads/descriptive-branch$" actual |
> +		grep " -> refs/crazyheads/.descriptive-branch$" actual |
>  		test_i18ngrep "new branch" &&
>  		grep " -> descriptive-tag$" actual |
>  		test_i18ngrep "new tag" &&
> -		grep " -> crazy$" actual |
> +		grep " -> .crazy$" actual |
>  		test_i18ngrep "new ref"
>  	) &&

These are somewhat cryptic ;-)

Other than that, the patch looks OK.

Thanks.