Re: [PATCH] Re-re-re-fix common tail optimization

From: Jeff King <peff@peff.net>
Date: Dec 16, 2007, 22:15 UTC
Message-ID: <20071216221545.GA32596@coredump.intra.peff.net>
In-Reply-To: <7v3au2joo2.fsf_-_@gitster.siamese.dyndns.org>

On Sun, Dec 16, 2007 at 01:49:17PM -0800, Junio C Hamano wrote:

> Kind'a embarrassing that both of us cannot get this right without so
> many rounds, isn't it?

Good thing we can just delete the emails and nobody will be the wiser...

> +echo >expect <<\EOF

This would probably work better as 'cat'.

> +test_expect_success 'diff -U0' '
> +
> +	git diff -U0 | sed -e "/^index/d" -e "s/$z2047/Z/g" >actual &&
> +	diff -u expect actual

Aren't we using "git diff" for the second diff there nowadays?

Show 5 quoted lines

> -	while (recovered < trimmed && ctx)
> +	while (recovered < trimmed && 0 <= ctx)
>  		if (ap[recovered++] == '\n')
>  			ctx--;
>  	a->size -= (trimmed - recovered);

Oops (I think maybe I misunderstood what you were asking in the last email). This fix is correct, though the code is now kind of subtle. I think it would be more obvious as:

  /* finish off any changed line we are in */
  while (recovered < trimmed && ap[recovered++] != '\n')
    /* nothing */;
  /* recover context lines */
  while (recovered < trimmed && ctx)
    if (ap[recovered++] == '\n')
      ctx--;

Your loop does both actions in the same loop, which is correct, but took me 10 minutes of thinking and staring to realize what was going on.

-Peff