Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 3, 2016, 17:00 UTC
- Message-ID
- <xmqqlh2mp5qv.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160603110843.15434-4-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 26 quoted lines
> +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?Show 17 quoted lines
> 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.