Blamming a diff between two commits?

Blamming a diff between two commits?

[Samuel Lucas Vaz de Mello]

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

Re: Blamming a diff between two commits?

[Johannes Schindelin]

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

Re: Blamming a diff between two commits?

[Matthieu Moy]

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

Re: Blamming a diff between two commits?

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1908 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Blamming a diff between two commits?

[Samuel Lucas Vaz de Mello]

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.

 - Samuel

Re: Blamming a diff between two commits?

[Johannes Schindelin]

Hi,

On Tue, 17 Feb 2009, Samuel Lucas Vaz de Mello wrote:

> 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

Re: Blamming a diff between two commits?

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Blamming a diff between two commits?

[Jan Hudec]

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).

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

Re: Blamming a diff between two commits?

[Matthieu Moy]

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?
>> 
>> 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

Re: Blamming a diff between two commits?

[Jan Hudec]

[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]

On Sun, Feb 22, 2009 at 19:49:16 +0100, Matthieu Moy wrote:
> 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>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]