Re: [PATCH v2 3/3] fetch: reduce duplicate in ref update status lines
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Jun 3, 2016, 23:49 UTC
- Message-ID
- <CACsJy8BiriuKyrP=0RcbFcSC9WSfcfDeKa-GsKosWxOQdpUZ4g@mail.gmail.com>
- In-Reply-To
- <xmqqlh2mp5qv.fsf@gitster.mtv.corp.google.com>
On Sat, Jun 4, 2016 at 12:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 32 quoted lines
>
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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 '/'?I wanted to check something else, then settled for slashes, but the two loops remained.
Show 14 quoted lines
> 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?Actually, Jeff's suggestion about common prefix makes me realize, why not reuse diff.c:pprint_rename()? Your {a -> b} idea came from that. We may need some tweaking there because right now it will not show { -> origin/}master.
Show 19 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 ;-)
Yeah... too lazy to check if } needs to be quoted or not :D
-- Duy