threads / discuss / 17855

Blamming a diff between two commits?

Subject: Blamming a diff between two commits?

## tl;dr

10 messages between Feb 17, 2009 and Feb 22, 2009.

replies: 9people: 5as markdown or json

Samuel Lucas Vaz de Mello· Feb 17, 2009, 13:16 UTC · lore
Hi,
Is there any way to git blame (or annotate) a diff between two commits?
I'm looking for something that produce something like this:
                              /**
85a02065 (samuel 2009-01-02) - * \brief      Define a participacao da porta estatica/dinamica
85a02065 (samuel 2009-01-02) - * \param      unit            indice da unidade
e106303a (arthur 2009-01-17) - * \param	     extraparam      extra parameter
85a02065 (samuel 2009-01-02) - * \param      port            indice da porta
50e22e7d (fabian 2009-01-09) - * \param      deleted         param to be deleted
85a02065 (samuel 2009-01-02) + * \brief      Sets port membership on a static / dynamic 
85a02065 (samuel 2009-01-02) + * \param      unit            unit index
85a02065 (samuel 2009-01-02) + * \param      port            port index
e106303a (arthur 2009-01-17) + * \param	     another         another index
                               * \return     0 if Ok; -1 in error
                               */
This would be useful for code reviews. I can use a diff containing all changes committed to a branch, for example, in the last 10 days to review. Doing this instead of reviewing individual commit patches save us from waste time analyzing code that has already been changed/fixed. 
Using a git-blame in the resulting file give me the commits for the lines added, but not for the deleted ones.
Any suggestion on how to do this?
Thanks,
 - Samuel
Johannes Schindelin· Feb 17, 2009, 13:53 UTC · re: Samuel Lucas Vaz de Mello · lore

Re: Blamming a diff between two commits?

Hi,
On Tue, 17 Feb 2009, Samuel Lucas Vaz de Mello wrote:
> Is there any way to git blame (or annotate) a diff between two commits?
If you do not mean the diff, but a commit range:
	$ git blame A..B -- file

"Unblameable" lines will be shown with a prefix ^A (not literal, of course, but the short commit name of A).

Hth, Dscho

Samuel Lucas Vaz de Mello· Feb 17, 2009, 14:09 UTC · re: Johannes Schindelin · lore

Re: Blamming a diff between two commits?

Johannes Schindelin wrote:
Show 10 quoted lines
> 
>> Is there any way to git blame (or annotate) a diff between two commits?
> 
> If you do not mean the diff, but a commit range:
> 
> 	$ git blame A..B -- file
> 
> "Unblameable" lines will be shown with a prefix ^A (not literal, of 
> course, but the short commit name of A).
> 
This work fine for lines that were added or changed, but not for deleted lines.
If a commit in the range just delete a couple of lines and adds nothing, the whole file is marked as "unblameable" as the deleted lines doesn't exist anymore.
 - Samuel
Johannes Schindelin· Feb 17, 2009, 14:27 UTC · re: Samuel Lucas Vaz de Mello · lore

Re: Blamming a diff between two commits?

Hi,
On Tue, 17 Feb 2009, Samuel Lucas Vaz de Mello wrote:
Show 18 quoted lines
> Johannes Schindelin wrote:
> > 
> >> Is there any way to git blame (or annotate) a diff between two commits?
> > 
> > If you do not mean the diff, but a commit range:
> > 
> > 	$ git blame A..B -- file
> > 
> > "Unblameable" lines will be shown with a prefix ^A (not literal, of 
> > course, but the short commit name of A).
> > 
> 
> This work fine for lines that were added or changed, but not for deleted 
> lines.
> 
> If a commit in the range just delete a couple of lines and adds nothing, 
> the whole file is marked as "unblameable" as the deleted lines doesn't 
> exist anymore.
You might want to add the --reverse option for that.

Ciao, Dscho

Matthieu Moy· Feb 17, 2009, 14:03 UTC · re: Samuel Lucas Vaz de Mello · lore

Re: Blamming a diff between two commits?

Samuel Lucas Vaz de Mello <samuellucas@datacom.ind.br> writes:
> Hi,
>
> Is there any way to git blame (or annotate) a diff between two commits?
I don't think this is implemented (but would be nice).
> Using a git-blame in the resulting file give me the commits for the
> lines added, but not for the deleted ones.

A preliminary implementation of "git blame --reverse" was proposed by Junio here:

  http://kerneltrap.org/mailarchive/git/2008/4/3/1338234
And an approximation of it is proposed in the thread:
  http://kerneltrap.org/mailarchive/git/2008/4/3/1343554
I don't know the status of this patch (dropped?).
-- 
Matthieu
Thomas Rast· Feb 17, 2009, 14:09 UTC · re: Samuel Lucas Vaz de Mello · lore

Re: Blamming a diff between two commits?

Samuel Lucas Vaz de Mello wrote:
> Is there any way to git blame (or annotate) a diff between two
> commits?

Piecing it together from existing tools isn't really hard, and made for a nice distraction.

Call it as './git-blame-diff.perl HEAD^ HEAD' or so.

This lacks proper argument checking and a chdir to the repository top level. Maybe you could fill in the gaps and shape it as a contrib patch? For bonus points, change it so that the workdir version can be used as the new side of the diff, by omitting the second argument.

--- 8< --- #!/usr/bin/perl -w

sub parse_hunk_header {
	my ($line) = @_;
	my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =
	    $line =~ /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/;
	$o_cnt = 1 unless defined $o_cnt;
	$n_cnt = 1 unless defined $n_cnt;
	return ($o_ofs, $o_cnt, $n_ofs, $n_cnt);
}
sub get_blame_prefix {
	my ($line) = @_;
	$line =~ /^([0-9a-f]+\s+\([^\)]+\))/ or die "bad blame output: $line";
	return $1;
}

my ($oldrev, $newrev) = @ARGV; open($diff, '-|', 'git', '--no-pager', 'diff', $oldrev, $newrev) or die;

my ($pre, $post);
my $filename;
while (<$diff>) {
	if (m{^diff --git ./(.*) ./\1$}) {
		$filename = $1;
	} elsif (m{^(\+\+\+|---) ./$filename$}) {
		# ignore
	} elsif (m{^@@ }) {
		my ($o_ofs, $o_cnt, $n_ofs, $n_cnt)
			= parse_hunk_header($_);
		my $o_end = $o_ofs + $o_cnt;
		my $n_end = $n_ofs + $n_cnt;
		open($pre, '-|', 'git', 'blame', "-L$o_ofs,$o_end",
		     $oldrev, '--', $filename) or die;
		open($post, '-|', 'git', 'blame', "-L$n_ofs,$n_end",
		     $newrev, '--', $filename) or die;
	} elsif (m{^ }) {
		print get_blame_prefix(scalar <$pre>), "\t", $_;
		scalar <$post>; # discard
	} elsif (m{^\-}) {
		print get_blame_prefix(scalar <$pre>), "\t", $_;
	} elsif (m{^\+}) {
		print get_blame_prefix(scalar <$post>), "\t", $_;
	} 
}
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Thomas Rast· Feb 20, 2009, 20:04 UTC · re: Thomas Rast · lore

Re: Blamming a diff between two commits?

Thomas Rast wrote:
> Samuel Lucas Vaz de Mello wrote:
> > Is there any way to git blame (or annotate) a diff between two
> > commits?
[...]
> Call it as './git-blame-diff.perl HEAD^ HEAD' or so.

Any reports from the field? Does it do what you wanted? Does it have other shortcomings than the ones I mentioned:

> This lacks proper argument checking and a chdir to the repository top
> level.  Maybe you could fill in the gaps and shape it as a contrib
> patch?  For bonus points, change it so that the workdir version can be
> used as the new side of the diff, by omitting the second argument.
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Jan Hudec· Feb 22, 2009, 12:32 UTC · re: Samuel Lucas Vaz de Mello · lore

Re: Blamming a diff between two commits?

On Tue, Feb 17, 2009 at 10:16:17 -0300, Samuel Lucas Vaz de Mello wrote:
Show 24 quoted lines
> Hi,
> 
> Is there any way to git blame (or annotate) a diff between two commits?
> 
> I'm looking for something that produce something like this:
> 
>                               /**
> 85a02065 (samuel 2009-01-02) - * \brief      Define a participacao da porta estatica/dinamica
> 85a02065 (samuel 2009-01-02) - * \param      unit            indice da unidade
> e106303a (arthur 2009-01-17) - * \param	     extraparam      extra parameter
> 85a02065 (samuel 2009-01-02) - * \param      port            indice da porta
> 50e22e7d (fabian 2009-01-09) - * \param      deleted         param to be deleted
> 85a02065 (samuel 2009-01-02) + * \brief      Sets port membership on a static / dynamic 
> 85a02065 (samuel 2009-01-02) + * \param      unit            unit index
> 85a02065 (samuel 2009-01-02) + * \param      port            port index
> e106303a (arthur 2009-01-17) + * \param	     another         another index
>                                * \return     0 if Ok; -1 in error
>                                */
> 
> This would be useful for code reviews. I can use a diff containing all changes committed to a branch, for example, in the last 10 days to review. Doing this instead of reviewing individual commit patches save us from waste time analyzing code that has already been changed/fixed. 
> 
> Using a git-blame in the resulting file give me the commits for the lines added, but not for the deleted ones.
> 
> Any suggestion on how to do this?

What about doing a diff of the blames? It should do the same thing (except the +/- would be at the begining of the lines). Well, not exactly, because if there was a change, that was reverted again, blames would change, so it would appear here, but that's probably rare enough to ignore (if you don't even want to see it rather than not).

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
Matthieu Moy· Feb 22, 2009, 18:49 UTC · re: Jan Hudec · lore

Re: Blamming a diff between two commits?

Jan Hudec <bulb@ucw.cz> writes:
Show 31 quoted lines
> On Tue, Feb 17, 2009 at 10:16:17 -0300, Samuel Lucas Vaz de Mello wrote:
>> Hi,
>> 
>> Is there any way to git blame (or annotate) a diff between two commits?
>> 
>> I'm looking for something that produce something like this:
>> 
>>                               /**
>> 85a02065 (samuel 2009-01-02) - * \brief      Define a participacao da porta estatica/dinamica
>> 85a02065 (samuel 2009-01-02) - * \param      unit            indice da unidade
>> e106303a (arthur 2009-01-17) - * \param	     extraparam      extra parameter
>> 85a02065 (samuel 2009-01-02) - * \param      port            indice da porta
>> 50e22e7d (fabian 2009-01-09) - * \param      deleted         param to be deleted
>> 85a02065 (samuel 2009-01-02) + * \brief      Sets port membership on a static / dynamic 
>> 85a02065 (samuel 2009-01-02) + * \param      unit            unit index
>> 85a02065 (samuel 2009-01-02) + * \param      port            port index
>> e106303a (arthur 2009-01-17) + * \param	     another         another index
>>                                * \return     0 if Ok; -1 in error
>>                                */
>> 
>> This would be useful for code reviews. I can use a diff containing all changes committed to a branch, for example, in the last 10 days to review. Doing this instead of reviewing individual commit patches save us from waste time analyzing code that has already been changed/fixed. 
>> 
>> Using a git-blame in the resulting file give me the commits for the lines added, but not for the deleted ones.
>> 
>> Any suggestion on how to do this?
>
> What about doing a diff of the blames? It should do the same thing (except
> the +/- would be at the begining of the lines). Well, not exactly, because if
> there was a change, that was reverted again, blames would change, so it would
> appear here, but that's probably rare enough to ignore (if you don't even
> want to see it rather than not).
I don't think that would do it.
Suppose I have the following history (in a one-line file) :

revision: R1 -> R2 -> R3 -> R4 -> R5 -> R6 content: A -> B -> B -> C -> D -> E

If I do a blame at revision R3, I'll get
R2 B
Then, at revision R6, I'd get
R6 E
so the diff will be
- R2 B
+ R6 E
while the original poster actually wanted
- R3 B
+ R6 E

In the first case, the annotation for - lines tell where the removed line had been introduced before, while the second case tells up to which revision the line has been existing (or, it could show the revision which removed it, R4 here).

-- 
Matthieu
Jan Hudec· Feb 22, 2009, 19:11 UTC · re: Matthieu Moy · lore

Re: Blamming a diff between two commits?

On Sun, Feb 22, 2009 at 19:49:16 +0100, Matthieu Moy wrote:
Show 42 quoted lines
> Jan Hudec <bulb@ucw.cz> writes:
> > On Tue, Feb 17, 2009 at 10:16:17 -0300, Samuel Lucas Vaz de Mello wrote:
> >> Hi,
> >> 
> >> Is there any way to git blame (or annotate) a diff between two commits?
> >> [...]
> >
> > What about doing a diff of the blames? It should do the same thing (except
> > the +/- would be at the begining of the lines). Well, not exactly, because if
> > there was a change, that was reverted again, blames would change, so it would
> > appear here, but that's probably rare enough to ignore (if you don't even
> > want to see it rather than not).
> 
> I don't think that would do it.
> 
> Suppose I have the following history (in a one-line file) :
> 
> revision: R1 -> R2 -> R3 -> R4 -> R5 -> R6
> content:  A  -> B  -> B  -> C  -> D  -> E 
> 
> If I do a blame at revision R3, I'll get
> 
> R2 B
> 
> Then, at revision R6, I'd get
> 
> R6 E
> 
> so the diff will be
> 
> - R2 B
> + R6 E
> 
> while the original poster actually wanted
> 
> - R3 B
> + R6 E
> 
> In the first case, the annotation for - lines tell where the removed
> line had been introduced before, while the second case tells up to
> which revision the line has been existing (or, it could show the
> revision which removed it, R4 here).
You are right, it is something different.

Both are probably useful, though -- knowing where the deleted line was introduced would be useful so you can look up rationale for the old code in the respective commit message and check whether the new version does not miss any points mentioned there. Provided you have good descriptions in the comments, of course.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

← back to recent threads