[0/3] blame: fix bug in coalescing non-adjacent "-L" chunks

Message

Jeff King Aug. 13, 2020, 5:20 a.m. UTC

On Fri, Aug 07, 2020 at 06:35:22PM -0400, Jeff King wrote:

> > > I think the only conceivable case is that in the middle of a single
> > > block of text in an ancient version, another block of lines gets
> > > inserted during the evolution of the file, but in the end these
> > > intermediate edits all go away and the same original text remains.
> > > 
> > > In such a case, without coalescing, we would not treat the original
> > > single block of text as a single unit.
> > 
> > Yeah, that makes sense, and it should be possible to construct a case
> > based on that.

I started to add a test for this, and it turns out we already had one!
It just wasn't checking the output as carefully as it could. :)

So here's a series which actually checks that blame_coalesce() is doing
something useful, and then fixes Nuthan's bug on top (with a test case,
but I also confirmed it makes the original tomcat issue go away).

  [1/3]: t8003: check output of coalesced blame
  [2/3]: t8003: factor setup out of coalesce test
  [3/3]: blame: only coalesce lines that are adjacent in result

 blame.c                       |  1 +
 t/t8003-blame-corner-cases.sh | 28 +++++++++++++++++++---------
 2 files changed, 20 insertions(+), 9 deletions(-)

-Peff

Comments

Barret Rhoden Aug. 13, 2020, 12:25 p.m. UTC | #1

On 8/13/20 1:20 AM, Jeff King wrote:
> On Fri, Aug 07, 2020 at 06:35:22PM -0400, Jeff King wrote:
> 
>>>> I think the only conceivable case is that in the middle of a single
>>>> block of text in an ancient version, another block of lines gets
>>>> inserted during the evolution of the file, but in the end these
>>>> intermediate edits all go away and the same original text remains.
>>>>
>>>> In such a case, without coalescing, we would not treat the original
>>>> single block of text as a single unit.
>>>
>>> Yeah, that makes sense, and it should be possible to construct a case
>>> based on that.
> 
> I started to add a test for this, and it turns out we already had one!
> It just wasn't checking the output as carefully as it could. :)
> 
> So here's a series which actually checks that blame_coalesce() is doing
> something useful, and then fixes Nuthan's bug on top (with a test case,
> but I also confirmed it makes the original tomcat issue go away).
> 
>    [1/3]: t8003: check output of coalesced blame
>    [2/3]: t8003: factor setup out of coalesce test
>    [3/3]: blame: only coalesce lines that are adjacent in result
> 

thanks for the fixes, looks good to me.