All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Nuthan Munaiah <nm6061@rit.edu>, git@vger.kernel.org
Subject: Re: `git blame` Line Number Off-by-one
Date: Fri, 7 Aug 2020 18:35:22 -0400	[thread overview]
Message-ID: <20200807223522.GB3750245@coredump.intra.peff.net> (raw)
In-Reply-To: <20200807222630.GA3750245@coredump.intra.peff.net>

On Fri, Aug 07, 2020 at 06:26:30PM -0400, Jeff King wrote:

> On Fri, Aug 07, 2020 at 03:05:27PM -0700, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > Dropping it entirely, as below, doesn't break any tests. Junio, do you
> > > know of a case this is meant to improve?
> > 
> > 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.

It was even easier than I expected. :)

Try this:

  git init repo
  cd repo
  
  seq 20 >file
  git add file
  git commit -m base
  
  { seq 10; echo foo; seq 11 20; } >file
  git commit -am cruft
  
  seq 20 >file
  git commit -am revert

  git blame --root --porcelain file

With the coalescing, we get a single 20-line block:

  25d93ad4c4b27570ccd4b6412ccaa549ae6f9ff7 1 1 20
  author Jeff King
  ...
          1
  ...
  25d93ad4c4b27570ccd4b6412ccaa549ae6f9ff7 11 11
          11

and without it, we get two 10-line blocks:

  25d93ad4c4b27570ccd4b6412ccaa549ae6f9ff7 1 1 10
  author Jeff King
  ...
          1
  ...
  25d93ad4c4b27570ccd4b6412ccaa549ae6f9ff7 11 11 10
          11

I doubt it really matters all that much in practice, but it's possible
some consumer of --porcelain relies on us giving minimal blocks.

So that argues for fixing blame_coalesce() to make sure that we coalesce
only when the blocks are adjacent in both the source and the result.
Otherwise we're losing information.

-Peff

  reply	other threads:[~2020-08-07 22:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-07  1:05 `git blame` Line Number Off-by-one Nuthan Munaiah
2020-08-07 21:21 ` Jeff King
2020-08-07 21:33   ` Jeff King
2020-08-07 21:45     ` Jeff King
2020-08-07 22:05     ` Junio C Hamano
2020-08-07 22:26       ` Jeff King
2020-08-07 22:35         ` Jeff King [this message]
2020-08-13  5:20           ` [PATCH 0/3] blame: fix bug in coalescing non-adjacent "-L" chunks Jeff King
2020-08-13  5:23             ` [PATCH 1/3] t8003: check output of coalesced blame Jeff King
2020-08-13 17:04               ` Junio C Hamano
2020-08-13 17:22                 ` Jeff King
2020-08-13  5:23             ` [PATCH 2/3] t8003: factor setup out of coalesce test Jeff King
2020-08-13  5:25             ` [PATCH 3/3] blame: only coalesce lines that are adjacent in result Jeff King
2020-08-13 16:59               ` Junio C Hamano
2020-08-13 18:41               ` Junio C Hamano
2020-08-13 12:25             ` [PATCH 0/3] blame: fix bug in coalescing non-adjacent "-L" chunks Barret Rhoden

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200807223522.GB3750245@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=nm6061@rit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.