[PATCH 0/3] gitweb: 'blame' view improvements

[PATCH 0/3] gitweb: 'blame' view improvements

[Jakub Narebski]

This patch series was inspired by the study before sending proposal
for git-blame improvements; I have noticed that "previous"/"parent"
blame header was already implemented, just not documented:
  Subject: git-blame: Documenting "previous" header
  Message-ID: <200907071423.40938.jnareb@gmail.com>
  http://thread.gmane.org/gmane.comp.version-control.git/122837

Therefore proposal got cut down to tree blame:
  Subject: [RFC] Tree blame (git blame <directory>)
  Message-ID: <200907071058.39390.jnareb@gmail.com>
  http://thread.gmane.org/gmane.comp.version-control.git/122830


Using "previous" header should improve gitweb performance and (as
I have noticed during implementing it) allow to follow 'linenr'
links correctly through rename in a blamed commit.

Marking "boundary" somewhat was required to make it possible to
distinguish which 'linenr' links would lead to parent of a blame
commit (to previous version of a file), and which would elad to blamed
commit.  Current styling is up to debate.

After implementing using "previous" header in 'blame' view (which
would allow to do the same for proposed 'blame_incremental' view)
I remembered that last version of implementing 'blame_incremental'
added author initials a'la "git gui blame".  So I did the same for
non-incremental 'blame' view.  Exact formatting is up to debate.

Table of contents:
==================
 [PATCH 1/3] gitweb: Mark boundary commits in 'blame' view
 [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view
 [PATCH 3/3] gitweb: Add author initials in 'blame' view, a la "git gui blame"

Jakub Narebski (3):
  gitweb: Mark boundary commits in 'blame' view
  gitweb: Use "previous" header of git-blame -p in 'blame' view
  gitweb: Add author initials in 'blame' view, a la "git gui blame"


 gitweb/gitweb.css  |    4 ++++
 gitweb/gitweb.perl |   51 ++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 40 insertions(+), 15 deletions(-)

-- 
Jakub Narebski
Poland

[PATCH 1/3] gitweb: Mark boundary commits in 'blame' view

[Jakub Narebski]

Use "boundary" class to mark boundary commits, which currently results
in using bold weight font for SHA-1 of a commit (to be more exact for
all text in the cell that contains SHA-1 of a commit).

Detecting boundary commits is done by watching for "boundary" header
in "git blame -p" output.  Because this header doesn't carry
additional data the regular expression for blame header fields had to
be adjusted.

With current gitweb API only root (parentless) commits can be boundary
commits.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Formatting (styling) of boundary commits is currently very minimal.
I'd like to see what other possible solution would you like to have.

Bikeshedding open!

 gitweb/gitweb.css  |    4 ++++
 gitweb/gitweb.perl |    6 ++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index d05bc37..5e2f629 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -242,6 +242,10 @@ tr.dark:hover {
 	background-color: #edece6;
 }
 
+tr.boundary td.sha1 {
+	font-weight: bold;
+}
+
 td {
 	padding: 2px 5px;
 	font-size: 100%;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6a1b5b5..fe73c2c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4826,7 +4826,7 @@ HTML
 		while ($data = <$fd>) {
 			chomp $data;
 			last if ($data =~ s/^\t//); # contents of line
-			if ($data =~ /^(\S+) (.*)$/) {
+			if ($data =~ /^(\S+)(?: (.*))?$/) {
 				$meta->{$1} = $2;
 			}
 		}
@@ -4838,7 +4838,9 @@ HTML
 		if ($group_size) {
 			$current_color = ($current_color + 1) % $num_colors;
 		}
-		print "<tr id=\"l$lineno\" class=\"$rev_color[$current_color]\">\n";
+		my $tr_class = $rev_color[$current_color];
+		$tr_class .= ' boundary' if (exists $meta->{'boundary'});
+		print "<tr id=\"l$lineno\" class=\"$tr_class\">\n";
 		if ($group_size) {
 			print "<td class=\"sha1\"";
 			print " title=\"". esc_html($author) . ", $date\"";
-- 
1.6.3.3

[PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view

[Jakub Narebski]

Luben Tuikov changed 'lineno' link (line number link) from pointing to
'blame' view at given line at blamed commit, to the one at parent of
blamed commit in
  244a70e (Blame "linenr" link jumps to previous state at
           "orig_lineno", 2007-01-04).
This made it possible to do data mining using 'blame' view, by going
through history of a line using mentioned line number link.

Original implementation called "git rev-parse <commit>^" to find SHA-1
of a parent of a given commit once per each blamed line.  In
  39c19ce (gitweb: cache $parent_commit info in git_blame(),
           2008-12-11)
this was improved so rev-parse was called once per each unique commit
in git-blame output.  Alternate solution would be to relax validation
for 'hb' parameter by allowing extended SHA-1 syntax of the form
<rev>^ (perhaps redirecting to gitweb URL with <rev>^ resolved, in
practice moving call to rev-parse to 'the other side of link').

This solution had a bug that it didn't work for boundary commits,
which did not have parents, so "git rev-parse <commit>^" returned
literal "<commit>^" (which didn't exists), which gitweb passed
as 'hb' parameter in 'linenr' link... following which gave
  400 - Invalid hash base parameter
error.  This bug could have been fixed by checking if commit is
boundary commit, or check if rev-parse result is unchanged (still
ends in '^' prefix).

The solution employing rev-parse to find parent of commit had inherent
problem if blamed commit renamed file; then name of file would be
different in its parent.  Solving this outside git-blame would be
difficult and costly (at least cost of additional fork for extra git
command).


Currently gitweb uses information in "previous" header, which was
introduced by Junio C Hamano in
  96e1170 (blame: show "previous" information in
           --porcelain/--incremental format, 2008-06-04)
This (currently undocumented) header has the following format:
  "previous <sha1 of parent commit> <filename at parent>"
Using "previous" header solves both problem of performance and the
problem that blamed commit could have renaming blamed file.

Because "previous" header can be repeated for the same commit when
blamed commit is merge (has more than one parent), and we are
interested usually in _first_ parent, currently we store only first
value if blame header repeats.  Using first parent (first "previous"
line) was what gitweb did before; without this change gitweb would use
last parent instead.

While at it introduce helper subroutine unquote_maybe(), which
unquotes filename if it is needed, which is marked by filename being
surrounded in doublequotes (which are not part of name, and which are
stripped by unquote_maybe() - which makes this function idempotent).
Currently unquote_maybe() us used only in git_blame.

If there is no previous commit 'linenr' link points to blamed commit
and blamed filename, making it work correctly for boundary commits.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

IMHO more important is that result is MORE CORRECT, not the better
performance.

In the table below you can see simple benchmark comparing gitweb
performance before and after this patch.  Operating system used was
Linux 2.6.14, on 1 GHz AMD Athlon processor (2002.43 BogoMIPS).
As there is one "git rev-parse <rev>^" per each individual commit
in blame output, it would be much worse 'before' for operating
systems with costly fork.

File                  |  C[1] || Time0[2] | Before[3] | After[3]
=================================================================
revision.c            |   121 ||   2.820s |    5.548s |   5.172s
gitweb/gitweb.perl[4] |   428 ||  11.749s |   19.797s |  17.293s

Table footnotes:
~~~~~~~~~~~~~~~~
[1] Individual commits in blame output:
    $ git blame -p <file> | grep author-time | wc -l
[2] Time for running "git blame -p" (user time, single run):
    $ time git blame -p <file> >/dev/null
[3] Time to run gitweb as Perl script from command line:
    $ time gitweb-run.sh "p=.git;a=blame;f=<file>" >/dev/null 2>&1
[4] Starting at 'origin', which is v1.6.3.3-412-gf581de1
    > git blame -p origin -- gitweb/gitweb.perl
    > "p=.git;a=blame;hb=origin;f=gitweb/gitweb.perl"


For comparison there is similar table for my 39c19ce (gitweb: cache
$parent_commit info in git_blame(), 2008-12-11):

File               | L[1] | C[2] || Time0[3] | Before[4] | After[4]
====================================================================
blob.h             |   18 |    4 || 0m1.727s |  0m2.545s |  0m2.474s
GIT-VERSION-GEN    |   42 |   13 || 0m2.165s |  0m2.448s |  0m2.071s
README             |   46 |    6 || 0m1.593s |  0m2.727s |  0m2.242s
revision.c         | 1923 |  121 || 0m2.357s | 0m30.365s |  0m7.028s
gitweb/gitweb.perl | 6291 |  428 || 0m8.080s | 1m37.244s | 0m20.627s

File               | L/C  | Before/After
=========================================
blob.h             |  4.5 |         1.03
GIT-VERSION-GEN    |  3.2 |         1.18
README             |  7.7 |         1.22
revision.c         | 15.9 |         4.32
gitweb/gitweb.perl | 14.7 |         4.71

As you can see the greater ratio of lines in file to unique commits
in blame output, the greater gain from the new implementation.

  Legend:

  [1] Number of lines:
      $ wc -l <file>
  [2] Number of unique commits in the blame output:
      $ git blame -p <file> | grep author-time | wc -l
  [3] Time for running "git blame -p" (user time, single run):
      $ time git blame -p <file> >/dev/null
  [4] Time to run gitweb as Perl script from command line:
      $ gitweb-run.sh "p=.git;a=blame;f=<file>" > /dev/null 2>&1


 gitweb/gitweb.perl |   37 ++++++++++++++++++++++++-------------
 1 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fe73c2c..36b1ce5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1187,6 +1187,16 @@ sub unquote {
 	return $str;
 }
 
+# if filename is surrounded in double quotes, it need to be unquoted
+sub unquote_maybe {
+	my $str = shift;
+
+	if ($str =~ /^"(.*)"$/) {
+		return unquote($1);
+	}
+	return $str;
+}
+
 # escape tabs (convert tabs to spaces)
 sub untabify {
 	my $line = shift;
@@ -4827,7 +4837,7 @@ HTML
 			chomp $data;
 			last if ($data =~ s/^\t//); # contents of line
 			if ($data =~ /^(\S+)(?: (.*))?$/) {
-				$meta->{$1} = $2;
+				$meta->{$1} = $2 unless exists $meta->{$1};
 			}
 		}
 		my $short_rev = substr($full_rev, 0, 8);
@@ -4852,20 +4862,21 @@ HTML
 			              esc_html($short_rev));
 			print "</td>\n";
 		}
-		my $parent_commit;
-		if (!exists $meta->{'parent'}) {
-			open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
-				or die_error(500, "Open git-rev-parse failed");
-			$parent_commit = <$dd>;
-			close $dd;
-			chomp($parent_commit);
-			$meta->{'parent'} = $parent_commit;
-		} else {
-			$parent_commit = $meta->{'parent'};
-		}
+		# 'previous' <sha1 of parent commit> <filename at commit>
+		if (exists $meta->{'previous'} &&
+		    $meta->{'previous'} =~ /^([a-fA-F0-9]{40}) (.*)$/) {
+			$meta->{'parent'} = $1;
+			$meta->{'file_parent'} = unquote_maybe($2);
+		}
+		my $linenr_commit =
+			exists($meta->{'parent'}) ?
+			$meta->{'parent'} : $full_rev;
+		my $linenr_filename =
+			exists($meta->{'file_parent'}) ?
+			$meta->{'file_parent'} : unquote_maybe($meta->{'filename'});
 		my $blamed = href(action => 'blame',
-		                  file_name => $meta->{'filename'},
-		                  hash_base => $parent_commit);
+		                  file_name => $linenr_filename,
+		                  hash_base => $linenr_commit);
 		print "<td class=\"linenr\">";
 		print $cgi->a({ -href => "$blamed#l$orig_lineno",
 		                -class => "linenr" },
-- 
1.6.3.3

[PATCH 3/3] gitweb: Add author initials in 'blame' view, a la "git gui blame"

[Jakub Narebski]

For example for "Junio C Hamano" initials would be "JH".  Of course
initials are added (below shortened SHA-1 of blamed commit) only if
group of lines has 2 lines or more in it.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Let the bikeshedding begin!

Should it be "JH" or perhaps "J.H." for "Junio C Hamano"?  Or perhaps
username part of author email would be better solution than initials?
Should we use different style for those initials?

This patch was inspired by me adding the same feature in similar 
'blame_incremental' view in
  http://thread.gmane.org/gmane.comp.version-control.git/102657/focus=102712

 gitweb/gitweb.perl |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 36b1ce5..5336c92 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4860,6 +4860,14 @@ HTML
 			                             hash=>$full_rev,
 			                             file_name=>$file_name)},
 			              esc_html($short_rev));
+			if ($group_size >= 2) {
+				my @author_initials = ($author =~ /\b([[:upper:]])\B/g);
+				if (@author_initials) {
+					print "<br />" .
+					      esc_html(join('', @author_initials));
+					#           or join('.', ...)
+				}
+			}
 			print "</td>\n";
 		}
 		# 'previous' <sha1 of parent commit> <filename at commit>
-- 
1.6.3.3

Re: [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Luben Tuikov changed 'lineno' link (line number link) from pointing to
> 'blame' view at given line at blamed commit, to the one at parent of
> blamed commit in
>   244a70e (Blame "linenr" link jumps to previous state at
>            "orig_lineno", 2007-01-04).
> This made it possible to do data mining using 'blame' view, by going
> through history of a line using mentioned line number link.

I was playing with this feature the other day (and I think you can guess
what I was writing when I was doing so as preparation).  I was mildly
annoyed that these links on the commit object names go to the commit view.

I think going to commitdiff view would make it far more useful while
digging.

Suppose if you were somehow interested in the recent commit by Peff,
"Makefile: install 'git' in execdir".  You go to:

    http://repo.or.cz/w/alt-git.git

and look at commitdiff of the commit from the shortlog part.

You read the diff, understand what the changed Makefile does, but you get
curious to see the blame.  Nicely, the commitdiff view has a list of the
files changed, and each entry in the list has "blame" link.

Clicking it would give you the blame on each line from the Makefile.

So far, very smooth experience.  Then you scroll to an area of the file
you are interested in, and click on one of the commits.

Oops.

It does not show the change of the commit made by this one, even though it
does list Makefile in the list of files changed, and it has a blame link,
the commit view without diff disrupts the thought process I had in the
previous blame page, and I have to go to commitdiff to reorient myself.

Re: [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view

[Jakub Narebski]

On Sat, 11 July 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Luben Tuikov changed 'lineno' link (line number link) from pointing to
> > 'blame' view at given line at blamed commit, to the one at parent of
> > blamed commit in
> >   244a70e (Blame "linenr" link jumps to previous state at
> >            "orig_lineno", 2007-01-04).
> > This made it possible to do data mining using 'blame' view, by going
> > through history of a line using mentioned line number link.
> 
> I was playing with this feature the other day (and I think you can guess
> what I was writing when I was doing so as preparation).  I was mildly
> annoyed that these links on the commit object names go to the commit view.
> 
> I think going to commitdiff view would make it far more useful while
> digging.
[...]

Also with a slight extension of diff part of 'commitdiff', by adding
line numbers for preimage in chunk and line numbers for postimage in
chunk, like e.g. in 'commit' view on GitHub[1] or on Gitorious[2] or
in SVN::Web[3] or in Atlassian FishEye, and adding anchors for those
line numbers we could go to given line in dif/patch output and examine
how it looked like before.

 [1] http://github.com/jnareb/softsnow-xchat2-filter/commit/7b68fcd777f94534f0b794c5dc2e109c49938395
 [2] http://gitorious.org/softsnow-xchat2-filter/mainline/commit/7b68fcd777f94534f0b794c5dc2e109c49938395
 [3] http://jc.ngo.org.uk/svnweb/jc/diff/nik/CPAN/SVN-Web/trunk/README?revs=1283&revs=981

(This would probably require adding 'fmt' / 'format' parameter, where
one could choose between possible ways to view diff, or possible ways
to view log or log-like view.)


P.S. BTW. 'blame' view passes _original_ filename (not filename at
blamed commit) to 'commit' view link... but it doesn't matter because
'commit' view does not use 'f'/$file_name parameter.
-- 
Jakub Narebski
Poland

Re: [PATCH 0/3] gitweb: 'blame' view improvements

[Jakub Narebski]

On Fri, 10 July 2009, Jakub Narebski wrote:

> Table of contents:
> ==================
>  [PATCH 1/3] gitweb: Mark boundary commits in 'blame' view
>  [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view
>  [PATCH 3/3] gitweb: Add author initials in 'blame' view, a la "git gui blame"
>

By the way I plan on submitting 'blame_incremental' view similar to
the one in "[RFC/PATCH 4/3] gitweb: Incremental blame (proof of concept)"

  http://thread.gmane.org/gmane.comp.version-control.git/102657/focus=102712

This time having the same (or nearly the same) features as ordinary
'blame' view.

-- 
Jakub Narebski
Poland

Re: [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view

[Luben Tuikov]

Acked-by: Luben Tuikov <ltuikov@yahoo.com>

Junio, can you also verify that this patch doesn't alter
behaviour in code data mining, before accepting it.

Thanks,
     Luben

________________________________
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Luben Tuikov <ltuikov@yahoo.com>; Junio C Hamano <gitster@pobox.com>
Sent: Friday, July 10, 2009 2:57:42 PM
Subject: [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view

Luben Tuikov changed 'lineno' link (line number link) from pointing to
'blame' view at given line at blamed commit, to the one at parent of
blamed commit in
  244a70e (Blame "linenr" link jumps to previous state at
           "orig_lineno", 2007-01-04).
This made it possible to do data mining using 'blame' view, by going
through history of a line using mentioned line number link.

Original implementation called "git rev-parse <commit>^" to find SHA-1
of a parent of a given commit once per each blamed line.  In
  39c19ce (gitweb: cache $parent_commit info in git_blame(),
           2008-12-11)
this was improved so rev-parse was called once per each unique commit
in git-blame output.  Alternate solution would be to relax validation
for 'hb' parameter by allowing extended SHA-1 syntax of the form
<rev>^ (perhaps redirecting to gitweb URL with <rev>^ resolved, in
practice moving call to rev-parse to 'the other side of link').

This solution had a bug that it didn't work for boundary commits,
which did not have parents, so "git rev-parse <commit>^" returned
literal "<commit>^" (which didn't exists), which gitweb passed
as 'hb' parameter in 'linenr' link... following which gave
  400 - Invalid hash base parameter
error.  This bug could have been fixed by checking if commit is
boundary commit, or check if rev-parse result is unchanged (still
ends in '^' prefix).

The solution employing rev-parse to find parent of commit had inherent
problem if blamed commit renamed file; then name of file would be
different in its parent.  Solving this outside git-blame would be
difficult and costly (at least cost of additional fork for extra git
command).


Currently gitweb uses information in "previous" header, which was
introduced by Junio C Hamano in
  96e1170 (blame: show "previous" information in
           --porcelain/--incremental format, 2008-06-04)
This (currently undocumented) header has the following format:
  "previous <sha1 of parent commit> <filename at parent>"
Using "previous" header solves both problem of performance and the
problem that blamed commit could have renaming blamed file.

Because "previous" header can be repeated for the same commit when
blamed commit is merge (has more than one parent), and we are
interested usually in _first_ parent, currently we store only first
value if blame header repeats.  Using first parent (first "previous"
line) was what gitweb did before; without this change gitweb would use
last parent instead.

While at it introduce helper subroutine unquote_maybe(), which
unquotes filename if it is needed, which is marked by filename being
surrounded in doublequotes (which are not part of name, and which are
stripped by unquote_maybe() - which makes this function idempotent).
Currently unquote_maybe() us used only in git_blame.

If there is no previous commit 'linenr' link points to blamed commit
and blamed filename, making it work correctly for boundary commits.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

IMHO more important is that result is MORE CORRECT, not the better
performance.

In the table below you can see simple benchmark comparing gitweb
performance before and after this patch.  Operating system used was
Linux 2.6.14, on 1 GHz AMD Athlon processor (2002.43 BogoMIPS).
As there is one "git rev-parse <rev>^" per each individual commit
in blame output, it would be much worse 'before' for operating
systems with costly fork.

File                  |  C[1] || Time0[2] | Before[3] | After[3]
=================================================================
revision.c            |   121 ||   2.820s |    5.548s |   5.172s
gitweb/gitweb.perl[4] |   428 ||  11.749s |   19.797s |  17.293s

Table footnotes:
~~~~~~~~~~~~~~~~
[1] Individual commits in blame output:
    $ git blame -p <file> | grep author-time | wc -l
[2] Time for running "git blame -p" (user time, single run):
    $ time git blame -p <file> >/dev/null
[3] Time to run gitweb as Perl script from command line:
    $ time gitweb-run.sh "p=.git;a=blame;f=<file>" >/dev/null 2>&1
[4] Starting at 'origin', which is v1.6.3.3-412-gf581de1
    > git blame -p origin -- gitweb/gitweb.perl
    > "p=.git;a=blame;hb=origin;f=gitweb/gitweb.perl"


For comparison there is similar table for my 39c19ce (gitweb: cache
$parent_commit info in git_blame(), 2008-12-11):

File               | L[1] | C[2] || Time0[3] | Before[4] | After[4]
====================================================================
blob.h             |   18 |    4 || 0m1.727s |  0m2.545s |  0m2.474s
GIT-VERSION-GEN    |   42 |   13 || 0m2.165s |  0m2.448s |  0m2.071s
README             |   46 |    6 || 0m1.593s |  0m2.727s |  0m2.242s
revision.c         | 1923 |  121 || 0m2.357s | 0m30.365s |  0m7.028s
gitweb/gitweb.perl | 6291 |  428 || 0m8.080s | 1m37.244s | 0m20.627s

File               | L/C  | Before/After
=========================================
blob.h             |  4.5 |         1.03
GIT-VERSION-GEN    |  3.2 |         1.18
README             |  7.7 |         1.22
revision.c         | 15.9 |         4.32
gitweb/gitweb.perl | 14.7 |         4.71

As you can see the greater ratio of lines in file to unique commits
in blame output, the greater gain from the new implementation.

  Legend:

  [1] Number of lines:
      $ wc -l <file>
  [2] Number of unique commits in the blame output:
      $ git blame -p <file> | grep author-time | wc -l
  [3] Time for running "git blame -p" (user time, single run):
      $ time git blame -p <file> >/dev/null
  [4] Time to run gitweb as Perl script from command line:
      $ gitweb-run.sh "p=.git;a=blame;f=<file>" > /dev/null 2>&1


gitweb/gitweb.perl |   37 ++++++++++++++++++++++++-------------
1 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fe73c2c..36b1ce5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1187,6 +1187,16 @@ sub unquote {
    return $str;
}

+# if filename is surrounded in double quotes, it need to be unquoted
+sub unquote_maybe {
+    my $str = shift;
+
+    if ($str =~ /^"(.*)"$/) {
+        return unquote($1);
+    }
+    return $str;
+}
+
# escape tabs (convert tabs to spaces)
sub untabify {
    my $line = shift;
@@ -4827,7 +4837,7 @@ HTML
            chomp $data;
            last if ($data =~ s/^\t//); # contents of line
            if ($data =~ /^(\S+)(?: (.*))?$/) {
-                $meta->{$1} = $2;
+                $meta->{$1} = $2 unless exists $meta->{$1};
            }
        }
        my $short_rev = substr($full_rev, 0, 8);
@@ -4852,20 +4862,21 @@ HTML
                          esc_html($short_rev));
            print "</td>\n";
        }
-        my $parent_commit;
-        if (!exists $meta->{'parent'}) {
-            open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
-                or die_error(500, "Open git-rev-parse failed");
-            $parent_commit = <$dd>;
-            close $dd;
-            chomp($parent_commit);
-            $meta->{'parent'} = $parent_commit;
-        } else {
-            $parent_commit = $meta->{'parent'};
-        }
+        # 'previous' <sha1 of parent commit> <filename at commit>
+        if (exists $meta->{'previous'} &&
+            $meta->{'previous'} =~ /^([a-fA-F0-9]{40}) (.*)$/) {
+            $meta->{'parent'} = $1;
+            $meta->{'file_parent'} = unquote_maybe($2);
+        }
+        my $linenr_commit =
+            exists($meta->{'parent'}) ?
+            $meta->{'parent'} : $full_rev;
+        my $linenr_filename =
+            exists($meta->{'file_parent'}) ?
+            $meta->{'file_parent'} : unquote_maybe($meta->{'filename'});
        my $blamed = href(action => 'blame',
-                          file_name => $meta->{'filename'},
-                          hash_base => $parent_commit);
+                          file_name => $linenr_filename,
+                          hash_base => $linenr_commit);
        print "<td class=\"linenr\">";
        print $cgi->a({ -href => "$blamed#l$orig_lineno",
                        -class => "linenr" },
-- 
1.6.3.3

[PATCH 4/3] gitweb: Use light/dark class also in 'blame' view

[Jakub Narebski]

Instead of using "light2" and "dark2" for class names in 'blame' view
(in place of "light" and "dark" classes in other places) to avoid
changing style on hover in 'blame' view while doing it for other views
(like 'shortlog'), use more advanced CSS, relying on the fact that
more specific selector wins.

While at it add a few comments to gitweb CSS file, and consolidate
some repeated info.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is an RFC because 

1. I am not sure if I did it correctly.  I had to fiddle a bit with CSS
(using "table.blame .light:hover" in place of "table.blame tr.light:hover")
to get the same behaviour (well, the same as far as I have checked it).

2. Commit message could use improvements (single sentence, blergh).

 gitweb/gitweb.css  |   16 +++++++++-------
 gitweb/gitweb.perl |    2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 5e2f629..4e4f8aa 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -226,22 +226,24 @@ th {
 	text-align: left;
 }
 
-tr.light:hover {
-	background-color: #edece6;
-}
-
-tr.dark {
-	background-color: #f6f6f0;
+/* do not change row style on hover for 'blame' view */
+tr.light,
+table.blame .light:hover {
+	background-color: #ffffff;
 }
 
-tr.dark2 {
+tr.dark,
+table.blame .dark:hover {
 	background-color: #f6f6f0;
 }
 
+/* currently both use the same, but it can change */
+tr.light:hover,
 tr.dark:hover {
 	background-color: #edece6;
 }
 
+/* boundary commits in 'blame' view */
 tr.boundary td.sha1 {
 	font-weight: bold;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5336c92..bb7a5a9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4811,7 +4811,7 @@ sub git_blame {
 	git_print_page_path($file_name, $ftype, $hash_base);
 
 	# page body
-	my @rev_color = qw(light2 dark2);
+	my @rev_color = qw(light dark);
 	my $num_colors = scalar(@rev_color);
 	my $current_color = 0;
 	my %metainfo = ();
-- 
1.6.3.3

[RFC PATCH 5/3] gitweb: Incremental blame (proof of concept)

[Jakub Narebski]

This is tweaked up further version of Petr Baudis <pasky@suse.cz> patch, 
which was tweaked up version of Fredrik Kuivinen <frekui@gmail.com>'s
proof of concept patch.  It adds 'blame_incremental' view, which
incrementally displays line data in blame view using JavaScript (AJAX).

This patch does not (contrary to the one by Petr Baudis) enable this
view in gitweb: there are no links leading to 'blame_incremental'
action.  You would have to generate URL 'by hand' (e.g. changing 'blame'
or 'blob' in gitweb URL to 'blame_incremental').  Having links in gitweb
lead to this new action (e.g. by rewriting them like in previous patch),
if JavaScript is enabled in browser, is left for later.

Like earlier patch by Petr Baudis it avoids code duplication, but it goes
one step further and use git_blame_common for ordinary blame view, for
incremental blame, and (which is change from previous patch) for
incremental blame data.

How the 'blame_incremental' view works:
* gitweb generates initial info by putting file contents (from
  git-cat-file) together with line numbers in blame table
* then gitweb makes web browser JavaScript engine call startBlame()
  function from blame.js
* startBlame() opens connection to 'blame_data' view, which in turn
  calls "git blame --incremental" for a file, and streams output of
  git-blame to JavaScript (blame.js)
* blame.js updates line info in blame view, coloring it, and updating
  progress info; note that it has to use 3 colors to ensure that
  different neighbour groups have different styles
* when 'blame_data' ends, and blame.js finishes updating line info,
  it fixes colors to match (as far as possible) ordinary 'blame' view,
  and updates generating time info.

It deals with streamed 'blame_data' server error by notifying about them
in the progress info area (just in case).

Differences between 'blame_incremental' and original 'blame' view:
* 'blame_incremental' always used (partial) query form for links
  generated by JavaScript.  The difference is visible if we use path_info
  link (pass some or all arguments in path_info), e.g. in 'blame' view
  called using:
    http://git.example.com/w/git.git/blame/HEAD:/README
  we have 'linenr' links using the same form:
    http://git.example.com/w/git.git/blame/e83c5163316f89bfbde7d9ab23ca2e25604af290:/README#l4
  while in 'blame_incremental' view called with:
    http://git.example.com/w/git.git/blame_incremental/HEAD:/README
  we have "partial query" form
    http://git.example.com/w/git.git?;a=blame_incremental;hb=e83c5163316f89bfbde7d9ab23ca2e25604af290;f=README#l4
  Changing this would require implementing something akin to href()
  subroutine in JavaScript
* 'blame_incremental' always uses "rowspan" attribute, even if
  rowspan="1".  This simplifies code, and is not visible to user.
+ sometimes 'blame_incremental' and corresponding 'blame' view show
  different widths for "Line" column.  I don't know what is the case
  of this; it might be even bug in web browser I use.

This patch adds GITWEB_BLAMEJS compile configuration option, and
modifies git-instaweb.sh to take blame.js into account, but it does not
update gitweb/README file (as it is only proof of concept patch).  The
code for git-instaweb.sh was taken from Pasky's patch.


This patch also adds showing time (in seconds) it took to generate
a page in page footer (based on example code by Pasky), even though
it is independent change, to be able to evaluate incremental blame in
gitweb better.  In proper patch series it would be independent commit;
and it probably would provide fallback if Time::HiRes is not available
(by e.g. not showing generating time info), even though this is
unlikely.


Some benchmarks:
^^^^^^^^^^^^^^^^
Web server (Apache + mod_cgi) and web browser (Mozilla 1.17.2) both
are running on the same single core single CPU computer (AMD Athlon)

File               | 'blame'[1] | 'blame_incremental'[2]
================================================================
blob.h             |     3.838s | 0.525s +  (3.227s /   4.019s)
GIT-VERSION-GEN    |     4.869s | 0.469s +  (4.359s /   5.134s)
README             |     6.564s | 0.581s +  (4.445s /   5.253s)
revision.c         |    27.817s | 4.562s + (13.619s /  38.307s)
gitweb/gitweb.perl |    81.495s | 9.922s + (50.761s / 242.591s)

Footnotes:
~~~~~~~~~~
[1] Total wall-clock time as returned by gitweb in the page footer.
[2] XXs + (XXs server blame_data / XXs client JavaScript).

Signed-off-by: Fredrik Kuivinen <frekui@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Refrences:
1. Original patch by Frederik Kuivinen
   http://article.gmane.org/gmane.comp.version-control.git/41361
2. Tweaked up version by Petr Baudis
   http://article.gmane.org/gmane.comp.version-control.git/56657
3. First and second version by me
   http://thread.gmane.org/gmane.comp.version-control.git/102657/focus=102712

Adding 'generated in XXX seconds' is based on snipped by Petr Baudis:
  http://article.gmane.org/gmane.comp.version-control.git/83306

REQUEST: Please, if possible, test if 'blame_incremental' works
correctly (without errors in JavaScript console or equivalent)
in other browsers than my old Mozilla 1.17.2 (Gecko / Rhino).
I'd like to have test for:
 * Firefox 2.0, 3.0 and 3.5
 * Internet Explorer 7 and 8 (perhaps also IE6)
 * Safari (and other WebKit based browsers)
 * Konqueror
 * Opera
 * Google Chrome

Changes compared to the last version:
* Perl code (gitweb.perl) updated to latest version, which uses 
  information from "previous" header in "git blame -p" output,
  and which can deal with boundary commits.  JavaScript code
  was updated to do the same.  This change made 'linenr' links
  lead to equivalent views, contrary to previous version.
* JavaScript code (blame.js) uses now the same date format as 'blame'
  view in Perl code (gitweb.perl), i.e. "2005-08-07 21:49:46 +0200"
  where time is in localtime of given timezone.
* JavaScript code (blame.js) got cleaned up and reordered; more
  comments were added, some variables and functions have slightly
  different names, some code got factored out into separate 
  functions, we use switch/case instead of ling if-else-if chain.

TODO list:
* While blame.js should now deal with filenames containing characters
  special to URI like '?', '%' or ';' thanks to use of
  encodeURIComponent(), it doesn't yet do unquoting of space quoted
  filenames, so it won't work correctly for filenames e.g. containing
  space, tab or quote character.
* handleResponse is used both as onreadystatechange and pollTimer;
  if onreadystatechange works for partial responses we can turn off
  the timer.  It is protected from concurrent running by global
  inProgress variable; we could instead pass XMLHttpResponse object
  as a parameter (see comment in startBlame).
* Probably 'blame_data' should use multipart/x-mixed-replace as
  content type, instead of (in addition to?) text/plain.  I am not
  sure about that; I am not knowledgeable in AJAX and Comet.
* Remove timing code, or move it to separate commit, and make it
  optional / configurable (we probably don't want to show such info
  on site open to wide public).
* Move td.error -> .error to separate commit.  td.error is remain 
  from old git-annotate based git_blame; 'blame_interactive' uses
  span.warning to notify about errors (like e.g. JavaScript turned
  off for 'blame_incremental' view).
* Remove or move to separate commit changes which help CPerl
  mode for GNU Emacs to deal with syntax highlighting (#', #").

A few questions about this patch:
* Should progress info be removed after 'blame_data' finishes?
* Should 3-coloring of blame in progress be removed, or perhaps
  just put in a separate patch for better reviewability (and better
  bisectability)?
* Should debug statements (conditional on DEBUG global variable)
  be removed from source?  There is separate mechanism to inform
  user about errors.
* Should we use <element>.className = <value> instead of DOM Core
  <element>.setAttribute('class', <value>)?  What about getter?
* Should we convert some regexps to string manipulation (e.g. split)?
  What is better for JavaScript performance?

Questions for the future:
* Should we split JavaScript scripts into separate files, e.g. one
  with common utilities (gitweb.js), and one dealing specifically with
  'blame_incremental' view (gitweb-blame.js)?  How to configure it
  (some scripts might be loaded in page header, some at the end of
  page to not block progressive loading)?

* Should we use some light-weight JavaScript framework (perhaps loaded
  from Google[1]), instead of / in addition to handcrafted JavaScript (as
  fallback)[2]?  What framework to use: jQuery, Prototype, YUI? CGI::Ajax?

  [1] http://www.google.com/jsapi and google.load, or something
  [2] http://stackoverflow.com/questions/1014203/best-way-to-use-googles-hosted-jquery-but-fall-back-to-my-hosted-library-on-goo

* Perhaps 'blame_data' should pre-process data from git-blame to reduce
  amount of work JavaScript has to do, and e.g. pre-parse data and
  transform to JSON / JSONP?

* To reduce size of JavaScript file user has to download to use
  'blame_incremental' view, while keeping clean and easy to modify
  original code, perhaps we could run some JavaScript minimizing
  tool when building gitweb, something like Douglas Crockford's 
  JSMin[3] (if the tool is available during install / build)?

  [3] http://www.crockford.com/javascript/jsmin.html

* Should we use JSDoc comments?  But no other parts of code use
  similar structured comments (Doxygen, ROBODoc, Natural Docs)...

 Makefile           |    6 +-
 git-instaweb.sh    |    7 +
 gitweb/blame.js    |  561 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.css  |   21 ++-
 gitweb/gitweb.perl |  286 ++++++++++++++++++---------
 5 files changed, 788 insertions(+), 93 deletions(-)
 create mode 100644 gitweb/blame.js

diff --git a/Makefile b/Makefile
index bde27ed..95b577c 100644
--- a/Makefile
+++ b/Makefile
@@ -265,6 +265,7 @@ GITWEB_HOMETEXT = indextext.html
 GITWEB_CSS = gitweb.css
 GITWEB_LOGO = git-logo.png
 GITWEB_FAVICON = git-favicon.png
+GITWEB_BLAMEJS = blame.js
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 
@@ -1406,13 +1407,14 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
 	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
 	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
 	    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+	    -e 's|++GITWEB_BLAMEJS++|$(GITWEB_BLAMEJS)|g' \
 	    -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
 	    -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
 	    $< >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
 
-git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
+git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/blame.js
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -1421,6 +1423,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
 	    -e '/@@GITWEB_CGI@@/d' \
 	    -e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
 	    -e '/@@GITWEB_CSS@@/d' \
+	    -e '/@@GITWEB_BLAMEJS@@/r gitweb/blame.js' \
+	    -e '/@@GITWEB_BLAMEJS@@/d' \
 	    -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 5f4419b..fd6341a 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -331,8 +331,15 @@ gitweb_css () {
 EOFGITWEB
 }
 
+gitweb_blamejs () {
+	cat > "$1" <<\EOFGITWEB
+@@GITWEB_BLAMEJS@@
+EOFGITWEB
+}
+
 gitweb_cgi "$GIT_DIR/gitweb/gitweb.cgi"
 gitweb_css "$GIT_DIR/gitweb/gitweb.css"
+gitweb_blamejs "$GIT_DIR/gitweb/blame.js"
 
 case "$httpd" in
 *lighttpd*)
diff --git a/gitweb/blame.js b/gitweb/blame.js
new file mode 100644
index 0000000..6b89143
--- /dev/null
+++ b/gitweb/blame.js
@@ -0,0 +1,561 @@
+// Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+
+/* ============================================================ */
+/* generic utility functions */
+
+var DEBUG = 0;
+function debug(str) {
+	if (DEBUG) {
+		alert(str);
+	}
+}
+
+// convert month or day of the month to string, padding it with
+// '0' (zero) to two characters width if necessary, e.g. 2 -> '02'
+function zeroPad(n) {
+	if (n < 10) {
+		return '0' + n;
+	} else {
+		return n.toString();
+	}
+}
+
+// pad number N with nonbreakable spaces on the right, to WIDTH characters
+// example: spacePad(12, 3) == '&nbsp;12' ('&nbsp;' is nonbreakable space)
+function spacePad(n,width) {
+	var scale = 1;
+	var prefix = '';
+
+	while (width > 1) {
+		scale *= 10;
+		if (n < scale) {
+			prefix += '&nbsp;';
+		}
+		width--;
+	}
+	return prefix + n;
+}
+
+// create XMLHttpRequest object in cross-browser way
+function createRequestObject() {
+	try {
+		return new XMLHttpRequest();
+	} catch(e) {}
+	try {
+		return new ActiveXObject("Msxml2.XMLHTTP");
+	} catch (e) {}
+	try {
+		return new ActiveXObject("Microsoft.XMLHTTP");
+	} catch (e) {}
+
+	debug("XMLHttpRequest not supported");
+	return null;
+}
+
+/* ============================================================ */
+/* utility/helper functions (and variables) */
+
+var http;       // XMLHttpRequest object
+var projectUrl; // partial query
+
+// 'commits' is an associative map. It maps SHA1s to Commit objects.
+var commits = {};
+
+function Commit(sha1) {
+	this.sha1 = sha1;
+}
+
+/* ............................................................ */
+/* progress info, timing */
+
+var blamedLines = 0;
+var totalLines  = '???';
+var div_progress_bar;
+var div_progress_info;
+
+// how many lines does a file have, used in progress info
+function countLines() {
+	var table =
+		document.getElementById('blame_table') ||
+		document.getElementsByTagName('table')[0];
+
+	if (table) {
+		return table.getElementsByTagName('tr').length - 1; // for header
+	} else {
+		return '...';
+	}
+}
+
+// update progress info and length (width) of progress bar
+function updateProgressInfo() {
+	if (!div_progress_info) {
+		div_progress_info = document.getElementById('progress_info');
+	}
+	if (!div_progress_bar) {
+		div_progress_bar = document.getElementById('progress_bar');
+	}
+	if (!div_progress_info && !div_progress_bar) {
+		return;
+	}
+
+	var percentage = Math.floor(100.0*blamedLines/totalLines);
+
+	if (div_progress_info) {
+		div_progress_info.innerHTML  = blamedLines + ' / ' + totalLines +
+			' ('+spacePad(percentage,3)+'%)';
+	}
+
+	if (div_progress_bar) {
+		div_progress_bar.setAttribute('style', 'width: '+percentage+'%;');
+	}
+}
+
+
+var t_interval_server = '';
+var t0 = new Date();
+
+// write how much it took to generate data, and to run script
+function writeTimeInterval() {
+	var info = document.getElementById('generate_time');
+	if (!info) {
+		return;
+	}
+	var t1 = new Date();
+
+	info.innerHTML += ' + (' +
+		t_interval_server+'s server blame_data / ' +
+		(t1.getTime() - t0.getTime())/1000 + 's client JavaScript)';
+}
+
+/* ............................................................ */
+/* coloring rows during blame_data (git blame --incremental) run */
+
+// used to extract N from colorN, where N is a number,
+var colorRe = new RegExp('color([0-9]*)');
+
+// return N if <tr class="colorN">, otherwise return null
+// (some browsers require CSS class names to begin with letter)
+function getColorNo(tr) {
+	if (!tr) {
+		return null;
+	}
+	var className = tr.getAttribute('class');
+	if (className) {
+		match = colorRe.exec(className);
+		if (match) {
+			return parseInt(match[1],10);
+		}
+	}
+	return null;
+}
+
+// return one of given possible colors (curently least used one)
+// example: chooseColorNoFrom(2, 3) returns 2 or 3
+var colorsFreq = [0, 0, 0];
+// assumes that  1 <= arguments[i] <= colorsFreq.length
+function chooseColorNoFrom() {
+	// choose the color which is least used
+	var colorNo = arguments[0];
+	for (var i = 1; i < arguments.length; i++) {
+		if (colorsFreq[arguments[i]-1] < colorsFreq[colorNo-1]) {
+			colorNo = arguments[i];
+		}
+	}
+	colorsFreq[colorNo-1]++;
+	return colorNo;
+}
+
+// given two neigbour <tr> elements, find color which would be different
+// from color of both of neighbours; used to 3-color blame table
+function findColorNo(tr_prev, tr_next) {
+	var color_prev = getColorNo(tr_prev);
+	var color_next = getColorNo(tr_next);
+
+
+	// neither of neighbours has color set
+	// THEN we can use any of 3 possible colors
+	if (!color_prev && !color_next) {
+		return chooseColorNoFrom(1,2,3);
+	}
+
+	// either both neighbours have the same color,
+	// or only one of neighbours have color set
+	// THEN we can use any color except given
+	var color;
+	if (color_prev == color_next) {
+		color = color_prev; // = color_next;
+	} else if (!color_prev) {
+		color = color_next;
+	} else if (!color_next) {
+		color = color_prev;
+	}
+	if (color) {
+		return chooseColorNoFrom((color % 3) + 1, ((color+1) % 3) + 1);
+	}
+
+	// neighbours have different colors
+	// THEN there is only one color left
+	return (3 - ((color_prev + color_next) % 3));
+}
+
+/* ............................................................ */
+/* coloring rows like 'blame' after 'blame_data' finishes */
+
+// returns true if given row element (tr) is first in commit group
+function isStartOfGroup(tr) {
+	return tr.firstChild.getAttribute('class') == 'sha1';
+}
+
+// change colors to use zebra coloring (2 colors) instead of 3 colors
+// concatenate neighbour commit groups belonging to the same commit
+function fixColorsAndGroups() {
+	var colorClasses = ['light', 'dark'];
+	var linenum = 1;
+	var tr, prev_group;
+	var colorClass = 0;
+
+	while ((tr = document.getElementById('l'+linenum))) {
+		if (isStartOfGroup(tr, linenum, document)) {
+			if (prev_group &&
+			    prev_group.firstChild.firstChild.href ==
+			            tr.firstChild.firstChild.href) {
+				// we have to concatenate groups
+				var rows = prev_group.firstChild.getAttribute('rowspan');
+				// assume that we have rowspan even for rowspan="1"
+				prev_group.firstChild.setAttribute('rowspan',
+					(rows + tr.firstChild.getAttribute('rowspan')));
+				tr.removeChild(tr.firstChild);
+			} else {
+				colorClass = (colorClass + 1) % 2;
+				prev_group = tr;
+			}
+		}
+		var tr_class = colorClasses[colorClass];
+		if (tr.className.indexOf('boundary') != -1)
+			tr_class += ' boundary';
+		tr.setAttribute('class', tr_class);
+		tr.className = tr_class;
+		// Internet Explorer needs this
+		tr.setAttribute('className', tr_class);
+		linenum++;
+	}
+}
+
+/* ............................................................ */
+/* time and data */
+
+// used to extract hours and minutes from timezone info, e.g '-0900'
+var tzRe = new RegExp('^([+-][0-9][0-9])([0-9][0-9])$');
+
+// return date in local time formatted in iso-8601 like format
+// 'yyyy-mm-dd HH:MM:SS +/-ZZZZ' e.g. '2005-08-07 21:49:46 +0200'
+function formatDateIsoTz(epoch, timezoneInfo) {
+	var match = tzRe.exec(timezoneInfo);
+	// date corrected by timezone
+	var localDate = new Date(1000 * (epoch +
+		(parseInt(match[1],10)*3600 + parseInt(match[2],10)*60)));
+	var localDateStr = // e.g. '2005-08-07'
+		localDate.getUTCFullYear()         + '-' +
+		zeroPad(localDate.getUTCMonth()+1) + '-' +
+		zeroPad(localDate.getUTCDate());
+	var localTimeStr = // e.g. '21:49:46'
+		zeroPad(localDate.getUTCHours())   + ':' +
+		zeroPad(localDate.getUTCMinutes()) + ':' +
+		zeroPad(localDate.getUTCSeconds());
+
+	return localDateStr + ' ' + localTimeStr + ' ' + timezoneInfo;
+}
+
+/* ............................................................ */
+/* unquoting/unescaping filenames */
+
+/* TODO */
+
+/* ============================================================ */
+/* main part: parsing response */
+
+// called for each blame entry, as soon as it finishes
+function handleLine(commit) {
+	/* 
+	   This is the structure of the HTML fragment we are working
+	   with:
+
+	   <tr id="l123" class="">
+	     <td class="sha1" title=""><a href=""></a></td>
+	     <td class="linenr"><a class="linenr" href="">123</a></td>
+	     <td class="pre"># times (my ext3 doesn&#39;t).</td>
+	   </tr>
+	*/
+
+	var resline = commit.resline;
+
+	// format date and time string only once per commit
+	if (!commit.info) {
+		/* e.g. 'Kay Sievers, 2005-08-07 21:49:46 +0200' */
+		commit.info = commit.author + ', ' +
+			formatDateIsoTz(commit.authorTime, commit.authorTimezone);
+	}
+
+	// color depends on group of lines, not only on blamed commit
+	var colorNo = findColorNo(
+		document.getElementById('l'+(resline-1)),
+		document.getElementById('l'+(resline+commit.numlines))
+	);
+
+	// loop over lines in commit group
+	for (var i = 0; i < commit.numlines; i++) {
+		var tr = document.getElementById('l'+resline);
+		if (!tr) {
+			debug('tr is null! resline: ' + resline);
+			break;
+		}
+		/*
+			<tr id="l123" class="">
+			  <td class="sha1" title=""><a href=""></a></td>
+			  <td class="linenr"><a class="linenr" href="">123</a></td>
+			  <td class="pre"># times (my ext3 doesn&#39;t).</td>
+			</tr>
+		*/
+		var td_sha1  = tr.firstChild;
+		var a_sha1   = td_sha1.firstChild;
+		var a_linenr = td_sha1.nextSibling.firstChild;
+
+		/* <tr id="l123" class=""> */
+		var tr_class = '';
+		if (colorNo !== null)
+			tr_class = 'color'+colorNo;
+		if (commit.boundary)
+			tr_class += ' boundary';
+		tr.setAttribute('class', tr_class);
+		// Internet Explorer needs this
+		tr.setAttribute('className', tr_class);
+
+		/* <td class="sha1" title="?" rowspan="?"><a href="?">?</a></td> */
+		if (i === 0) {
+			td_sha1.title = commit.info;
+			td_sha1.setAttribute('rowspan', commit.numlines);
+
+			a_sha1.href = projectUrl + ';a=commit;h=' + commit.sha1;
+			a_sha1.innerHTML = commit.sha1.substr(0, 8);
+			if (commit.numlines >= 2) {
+				var br   = document.createElement("br");
+				var text = document.createTextNode(
+					commit.author.match(/\b([A-Z])\B/g).join(''));
+				if (br && text) {
+					td_sha1.appendChild(br);
+					td_sha1.appendChild(text);
+				}
+			}
+		} else {
+			//tr.removeChild(td_sha1); // DOM2 Core way
+			tr.deleteCell(0); // DOM2 HTML way
+		}
+
+		/* <td class="linenr"><a class="linenr" href="?">123</a></td> */
+		linenr_commit =
+			commit.previous ? commit.previous : commit.sha1;
+		linenr_filename =
+			commit.file_parent ? commit.file_parent : commit.filename;
+		a_linenr.href = projectUrl + ';a=blame_incremental' +
+			';hb=' + linenr_commit +
+			';f='  + encodeURIComponent(linenr_filename) +
+			'#l' + (commit.srcline + i);
+
+		resline++;
+		blamedLines++;
+
+		//updateProgressInfo();
+	}
+}
+
+// ----------------------------------------------------------------------
+
+var prevDataLength = -1;
+var nextLine = 0;
+var inProgress = false;
+
+var sha1Re = new RegExp('([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)');
+var infoRe = new RegExp('([a-z-]+) ?(.*)');
+var endRe  = new RegExp('END ?(.*)');
+var curCommit = new Commit();
+
+var pollTimer = null;
+
+function handleResponse() {
+	debug('handleResp ready: ' + http.readyState +
+	      ' respText null?: ' + (http.responseText === null) +
+	      ' progress: ' + inProgress);
+
+	if (http.readyState != 4 && http.readyState != 3) {
+		return;
+	}
+
+	// the server returned error
+	if (http.readyState == 3 && http.status != 200) {
+		return;
+	}
+	if (http.readyState == 4 && http.status != 200) {
+		if (!div_progress_info) {
+			div_progress_info = document.getElementById('progress_info');
+		}
+
+		if (div_progress_info) {
+			div_progress_info.setAttribute('class', 'error');
+			// Internet Explorer needs this
+			div_progress_info.setAttribute('className', 'error');
+			div_progress_info.innerHTML = 'Server error: ' +
+				http.status + ' - ' + (http.statusText || 'Error contacting server');
+		}
+
+		clearInterval(pollTimer);
+		inProgress = false;
+	}
+
+	// In konqueror http.responseText is sometimes null here...
+	if (http.responseText === null) {
+		return;
+	}
+
+	// in case we were called before finished processing
+	if (inProgress) {
+		return;
+	} else {
+		inProgress = true;
+	}
+
+	while (prevDataLength != http.responseText.length) {
+		if (http.readyState == 4 &&
+		    prevDataLength == http.responseText.length) {
+			break;
+		}
+
+		prevDataLength = http.responseText.length;
+		var response = http.responseText.substring(nextLine);
+		var lines = response.split('\n');
+		nextLine = nextLine + response.lastIndexOf('\n') + 1;
+		if (response[response.length-1] != '\n') {
+			lines.pop();
+		}
+
+		for (var i = 0; i < lines.length; i++) {
+			var match = sha1Re.exec(lines[i]);
+			if (match) {
+				var sha1 = match[1];
+				var srcline = parseInt(match[2],10);
+				var resline = parseInt(match[3],10);
+				var numlines = parseInt(match[4],10);
+				var c = commits[sha1];
+				if (!c) {
+					c = new Commit(sha1);
+					commits[sha1] = c;
+				}
+
+				c.srcline = srcline;
+				c.resline = resline;
+				c.numlines = numlines;
+				curCommit = c;
+
+			} else if ((match = infoRe.exec(lines[i]))) {
+				var info = match[1];
+				var data = match[2];
+				switch (info) {
+				case 'filename':
+					curCommit.filename = data; // unquote_maybe
+					// 'filename' information terminates the entry
+					handleLine(curCommit);
+					updateProgressInfo();
+					break;
+				case 'author':
+					curCommit.author = data;
+					break;
+				case 'author-time':
+					curCommit.authorTime = parseInt(data, 10);
+					break;
+				case 'author-tz':
+					curCommit.authorTimezone = data;
+					break;
+				case 'previous':
+					var parts = data.split(' ', 2);
+					curCommit.previous    = parts[0];
+					curCommit.file_parent = parts[1]; // unquote_maybe
+					break;
+				case 'boundary':
+					debug('Boundary commit: '+curCommit.sha1);
+					curCommit.boundary = true;
+					break;
+				} // end switch
+
+			} else if ((match = endRe.exec(lines[i]))) {
+				t_interval_server = match[1];
+				debug('END: '+lines[i]);
+			} else if (lines[i] !== '') {
+				debug('malformed line: ' + lines[i]);
+			}
+		}
+	}
+
+	// did we finish work?
+	if (http.readyState == 4 &&
+	    prevDataLength == http.responseText.length) {
+		clearInterval(pollTimer);
+
+		fixColorsAndGroups();
+		writeTimeInterval();
+		commits = {}; // free memory
+	}
+
+	inProgress = false;
+}
+
+// ============================================================
+// ------------------------------------------------------------
+
+/*
+	Function: startBlame
+
+	Incrementally update line data in blame_incremental view in gitweb.
+
+	Parameters:
+
+		blamedataUrl - URL to server script generating blame data.
+		bUrl -partial URL to project, used to generate links in blame.
+
+	Comments:
+
+	Called from 'blame_incremental' view after loading table with
+	file contents, a base for blame view.
+*/
+function startBlame(blamedataUrl, bUrl) {
+	debug('startBlame('+blamedataUrl+', '+bUrl+')');
+
+	http = createRequestObject();
+	if (!http) {
+		div_progress_info = document.getElementById('progress_info');
+
+		if (div_progress_info) {
+			div_progress_info.setAttribute('class', 'error');
+			// Internet Explorer needs this
+			div_progress_info.setAttribute('className', 'error');
+			div_progress_info.innerHTML = '<b>ERROR:</b> XMLHttpRequest not supported';
+		}
+
+		return;
+	}
+
+	t0 = new Date();
+	projectUrl = bUrl;
+	totalLines = countLines();
+	updateProgressInfo();
+
+	http.open('get', blamedataUrl);
+	http.setRequestHeader('Accept', 'text/plain'); // in case of future changes
+	http.onreadystatechange = handleResponse;
+	//http.onreadystatechange = function() { handleResponse(http); };
+	http.send(null);
+
+	// not all browsers call onreadystatechange event on each server flush
+	if (!DEBUG)
+		pollTimer = setInterval(handleResponse, 1000);
+}
+
+// end of blame.js
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 4e4f8aa..ac9033f 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -248,6 +248,14 @@ tr.boundary td.sha1 {
 	font-weight: bold;
 }
 
+tr.color1:hover { background-color: #e6ede6; }
+tr.color2:hover { background-color: #e6e6ed; }
+tr.color3:hover { background-color: #ede6e6; }
+
+tr.color1 { background-color: #f6fff6; }
+tr.color2 { background-color: #f6f6ff; }
+tr.color3 { background-color: #fff6f6; }
+
 td {
 	padding: 2px 5px;
 	font-size: 100%;
@@ -268,7 +276,7 @@ td.sha1 {
 	font-family: monospace;
 }
 
-td.error {
+.error {
 	color: red;
 	background-color: yellow;
 }
@@ -339,6 +347,17 @@ td.mode {
 	font-family: monospace;
 }
 
+/* progress of blame_interactive */
+div#progress_bar {
+	height: 2px;
+	margin-bottom: -2px;
+	background-color: #d8d9d0;
+}
+div#progress_info {
+	float: right;
+	text-align: right;
+}
+
 /* styling of diffs (patchsets): commitdiff and blobdiff views */
 div.diff.header,
 div.diff.extended_header {
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index bb7a5a9..dea521e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,6 +18,9 @@ use File::Find qw();
 use File::Basename qw(basename);
 binmode STDOUT, ':utf8';
 
+use Time::HiRes qw(gettimeofday tv_interval);
+our $t0 = [gettimeofday];
+
 BEGIN {
 	CGI->compile() if $ENV{'MOD_PERL'};
 }
@@ -90,6 +93,8 @@ our $stylesheet = undef;
 our $logo = "++GITWEB_LOGO++";
 # URI of GIT favicon, assumed to be image/png type
 our $favicon = "++GITWEB_FAVICON++";
+# URI of blame.js
+our $blamejs = "++GITWEB_BLAMEJS++";
 
 # URI and label (title) of GIT logo link
 #our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
@@ -550,6 +555,8 @@ our %cgi_param_mapping = @cgi_param_mapping;
 # we will also need to know the possible actions, for validation
 our %actions = (
 	"blame" => \&git_blame,
+	"blame_incremental" => \&git_blame_incremental,
+	"blame_data" => \&git_blame_data,
 	"blobdiff" => \&git_blobdiff,
 	"blobdiff_plain" => \&git_blobdiff_plain,
 	"blob" => \&git_blob,
@@ -992,7 +999,8 @@ sub href {
 			}
 		}
 	}
-	$href .= "?" . join(';', @result) if scalar @result;
+	$href .= "?" . join(';', @result)
+		if ($params{-partial_query} or scalar @result);
 
 	return $href;
 }
@@ -1716,7 +1724,7 @@ sub format_diff_from_to_header {
 	# no extra formatting for "^--- /dev/null"
 	if (! $diffinfo->{'nparents'}) {
 		# ordinary (single parent) diff
-		if ($line =~ m!^--- "?a/!) {
+		if ($line =~ m!^--- "?a/!) {#"
 			if ($from->{'href'}) {
 				$line = '--- a/' .
 				        $cgi->a({-href=>$from->{'href'}, -class=>"path"},
@@ -3042,13 +3050,13 @@ sub git_header_html {
 	# 'application/xhtml+xml', otherwise send it as plain old 'text/html'.
 	# we have to do this because MSIE sometimes globs '*/*', pretending to
 	# support xhtml+xml but choking when it gets what it asked for.
-	if (defined $cgi->http('HTTP_ACCEPT') &&
-	    $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
-	    $cgi->Accept('application/xhtml+xml') != 0) {
-		$content_type = 'application/xhtml+xml';
-	} else {
+	#if (defined $cgi->http('HTTP_ACCEPT') &&
+	#    $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
+	#    $cgi->Accept('application/xhtml+xml') != 0) {
+	#	$content_type = 'application/xhtml+xml';
+	#} else {
 		$content_type = 'text/html';
-	}
+	#}
 	print $cgi->header(-type=>$content_type, -charset => 'utf-8',
 	                   -status=> $status, -expires => $expires);
 	my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : '';
@@ -3215,6 +3223,14 @@ sub git_footer_html {
 	}
 	print "</div>\n"; # class="page_footer"
 
+	print "<div class=\"page_footer\">\n";
+	print 'This page took '.
+	      '<span id="generate_time" class="time_span">'.
+	      tv_interval($t0, [gettimeofday]).'s'.
+	      '</span>'.
+	      " to generate.\n";
+	print "</div>\n"; # class="page_footer"
+
 	if (-f $site_footer) {
 		insert_file($site_footer);
 	}
@@ -4004,7 +4020,7 @@ sub git_patchset_body {
 	while ($patch_line) {
 
 		# parse "git diff" header line
-		if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ "]*) (.*)$/) {
+		if ($patch_line =~ m/^diff --git (\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\"|[^ "]*) (.*)$/) {#"
 			# $1 is from_name, which we do not use
 			$to_name = unquote($2);
 			$to_name =~ s!^b/!!;
@@ -4768,7 +4784,9 @@ sub git_tag {
 	git_footer_html();
 }
 
-sub git_blame {
+sub git_blame_common {
+	my $format = shift || 'porcelain';
+
 	# permissions
 	gitweb_check_feature('blame')
 		or die_error(403, "Blame view not allowed");
@@ -4790,10 +4808,36 @@ sub git_blame {
 		}
 	}
 
-	# run git-blame --porcelain
-	open my $fd, "-|", git_cmd(), "blame", '-p',
-		$hash_base, '--', $file_name
-		or die_error(500, "Open git-blame failed");
+	my $fd;
+	if ($format eq 'incremental') {
+		# get file contents (as base)
+		open $fd, "-|", git_cmd(), 'cat-file', 'blob', $hash
+			or die_error(500, "Open git-cat-file failed");
+	} elsif ($format eq 'data') {
+		# run git-blame --incremental
+		open $fd, "-|", git_cmd(), "blame", "--incremental",
+			$hash_base, "--", $file_name
+			or die_error(500, "Open git-blame --incremental failed");
+	} else {
+		# run git-blame --porcelain
+		open $fd, "-|", git_cmd(), "blame", '-p',
+			$hash_base, '--', $file_name
+			or die_error(500, "Open git-blame --porcelain failed");
+	}
+
+	# incremental blame data returns early
+	if ($format eq 'data') {
+		print $cgi->header(
+			-type=>"text/plain", -charset => "utf-8",
+			-status=> "200 OK");
+		local $| = 1; # output autoflush
+		print while <$fd>;
+		close $fd
+			or print "ERROR $!\n";
+		print "END ".tv_interval($t0, [gettimeofday])."\n";
+
+		return;
+	}
 
 	# page header
 	git_header_html();
@@ -4804,104 +4848,164 @@ sub git_blame {
 		$cgi->a({-href => href(action=>"history", -replay=>1)},
 		        "history") .
 		" | " .
-		$cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
+		$cgi->a({-href => href(action=>$action, file_name=>$file_name)},
 		        "HEAD");
 	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
 	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	git_print_page_path($file_name, $ftype, $hash_base);
 
 	# page body
+	if ($format eq 'incremental') {
+		print "<noscript>\n<div class=\"error\"><center><b>\n".
+		      "This page requires JavaScript to run\nUse ".
+		      $cgi->a({-href => href(action=>'blame',-replay=>1)}, 'this page').
+		      " instead.\n".
+		      "</b></center></div>\n</noscript>\n";
+
+		print qq!<div id="progress_bar" style="width: 100%; background-color: yellow"></div>\n!;
+	}
+
+	print qq!<div class="page_body">\n!;
+	print qq!<div id="progress_info">... / ...</div>\n!
+		if ($format eq 'incremental');
+	print qq!<table id="blame_table" class="blame" width="100%">\n!.
+	      #qq!<col width="5.5em" /><col width="2.5em" /><col width="*" />\n!.
+	      qq!<thead>\n!.
+	      qq!<tr><th>Commit</th><th>Line</th><th>Data</th></tr>\n!.
+	      qq!</thead>\n!.
+	      qq!<tbody>\n!;
+
 	my @rev_color = qw(light dark);
 	my $num_colors = scalar(@rev_color);
 	my $current_color = 0;
-	my %metainfo = ();
 
-	print <<HTML;
-<div class="page_body">
-<table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
-HTML
- LINE:
-	while (my $line = <$fd>) {
-		chomp $line;
-		# the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
-		# no <lines in group> for subsequent lines in group of lines
-		my ($full_rev, $orig_lineno, $lineno, $group_size) =
-		   ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
-		if (!exists $metainfo{$full_rev}) {
-			$metainfo{$full_rev} = {};
-		}
-		my $meta = $metainfo{$full_rev};
-		my $data;
-		while ($data = <$fd>) {
-			chomp $data;
-			last if ($data =~ s/^\t//); # contents of line
-			if ($data =~ /^(\S+)(?: (.*))?$/) {
-				$meta->{$1} = $2 unless exists $meta->{$1};
-			}
+	if ($format eq 'incremental') {
+		my $color_class = $rev_color[$current_color];
+
+		#contents of a file
+		my $linenr = 0;
+	LINE:
+		while (my $line = <$fd>) {
+			chomp $line;
+			$linenr++;
+
+			print qq!<tr id="l$linenr" class="$color_class">!.
+			      qq!<td class="sha1"><a href=""></a></td>!.
+			      qq!<td class="linenr">!.
+			      qq!<a class="linenr" href="">$linenr</a></td>!;
+			print qq!<td class="pre">! . esc_html($line) . "</td>\n";
+			print qq!</tr>\n!;
 		}
-		my $short_rev = substr($full_rev, 0, 8);
-		my $author = $meta->{'author'};
-		my %date =
-			parse_date($meta->{'author-time'}, $meta->{'author-tz'});
-		my $date = $date{'iso-tz'};
-		if ($group_size) {
-			$current_color = ($current_color + 1) % $num_colors;
-		}
-		my $tr_class = $rev_color[$current_color];
-		$tr_class .= ' boundary' if (exists $meta->{'boundary'});
-		print "<tr id=\"l$lineno\" class=\"$tr_class\">\n";
-		if ($group_size) {
-			print "<td class=\"sha1\"";
-			print " title=\"". esc_html($author) . ", $date\"";
-			print " rowspan=\"$group_size\"" if ($group_size > 1);
-			print ">";
-			print $cgi->a({-href => href(action=>"commit",
-			                             hash=>$full_rev,
-			                             file_name=>$file_name)},
-			              esc_html($short_rev));
-			if ($group_size >= 2) {
-				my @author_initials = ($author =~ /\b([[:upper:]])\B/g);
-				if (@author_initials) {
-					print "<br />" .
-					      esc_html(join('', @author_initials));
-					#           or join('.', ...)
+
+	} else { # porcelain, i.e. ordinary blame
+		my %metainfo = (); # saves information about commits
+
+		# blame data
+	LINE:
+		while (my $line = <$fd>) {
+			chomp $line;
+			# the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
+			# no <lines in group> for subsequent lines in group of lines
+			my ($full_rev, $orig_lineno, $lineno, $group_size) =
+			   ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
+			if (!exists $metainfo{$full_rev}) {
+				$metainfo{$full_rev} = {};
+			}
+			my $meta = $metainfo{$full_rev};
+			my $data;
+			while ($data = <$fd>) {
+				chomp $data;
+				last if ($data =~ s/^\t//); # contents of line
+				if ($data =~ /^(\S+)(?: (.*))?$/) {
+					$meta->{$1} = $2 unless exists $meta->{$1};
 				}
 			}
-			print "</td>\n";
-		}
-		# 'previous' <sha1 of parent commit> <filename at commit>
-		if (exists $meta->{'previous'} &&
-		    $meta->{'previous'} =~ /^([a-fA-F0-9]{40}) (.*)$/) {
-			$meta->{'parent'} = $1;
-			$meta->{'file_parent'} = unquote_maybe($2);
-		}
-		my $linenr_commit =
-			exists($meta->{'parent'}) ?
-			$meta->{'parent'} : $full_rev;
-		my $linenr_filename =
-			exists($meta->{'file_parent'}) ?
-			$meta->{'file_parent'} : unquote_maybe($meta->{'filename'});
-		my $blamed = href(action => 'blame',
-		                  file_name => $linenr_filename,
-		                  hash_base => $linenr_commit);
-		print "<td class=\"linenr\">";
-		print $cgi->a({ -href => "$blamed#l$orig_lineno",
-		                -class => "linenr" },
-		              esc_html($lineno));
-		print "</td>";
-		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
-		print "</tr>\n";
+			my $short_rev = substr($full_rev, 0, 8);
+			my $author = $meta->{'author'};
+			my %date =
+				parse_date($meta->{'author-time'}, $meta->{'author-tz'});
+			my $date = $date{'iso-tz'};
+			if ($group_size) {
+				$current_color = ($current_color + 1) % $num_colors;
+			}
+			my $tr_class = $rev_color[$current_color];
+			$tr_class .= ' boundary' if (exists $meta->{'boundary'});
+			print "<tr id=\"l$lineno\" class=\"$tr_class\">\n";
+			if ($group_size) {
+				print "<td class=\"sha1\"";
+				print " title=\"". esc_html($author) . ", $date\"";
+				print " rowspan=\"$group_size\"" if ($group_size > 1);
+				print ">";
+				print $cgi->a({-href => href(action=>"commit",
+				                             hash=>$full_rev,
+				                             file_name=>$file_name)},
+				              esc_html($short_rev));
+				if ($group_size >= 2) {
+					my @author_initials = ($author =~ /\b([[:upper:]])\B/g);
+					if (@author_initials) {
+						print "<br />" .
+						      esc_html(join('', @author_initials));
+						#           or join('.', ...)
+					}
+				}
+				print "</td>\n";
+			}
+			# 'previous' <sha1 of parent commit> <filename at commit>
+			if (exists $meta->{'previous'} &&
+			    $meta->{'previous'} =~ /^([a-fA-F0-9]{40}) (.*)$/) {
+				$meta->{'parent'} = $1;
+				$meta->{'file_parent'} = unquote_maybe($2);
+			}
+			my $linenr_commit =
+				exists($meta->{'parent'}) ?
+				$meta->{'parent'} : $full_rev;
+			my $linenr_filename =
+				exists($meta->{'file_parent'}) ?
+				$meta->{'file_parent'} : unquote_maybe($meta->{'filename'});
+			my $blamed = href(action => 'blame',
+			                  file_name => $linenr_filename,
+			                  hash_base => $linenr_commit);
+			print "<td class=\"linenr\">";
+			print $cgi->a({ -href => "$blamed#l$orig_lineno",
+			                -class => "linenr" },
+			              esc_html($lineno));
+			print "</td>";
+			print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
+			print "</tr>\n";
+		} # end while
+
 	}
-	print "</table>\n";
-	print "</div>";
+
+	# footer
+	print "</tbody>\n".
+	      "</table>\n"; # class="blame"
+	print "</div>\n";   # class="blame_body"
 	close $fd
 		or print "Reading blob failed\n";
 
-	# page footer
+	if ($format eq 'incremental') {
+		print qq!<script type="text/javascript" src="$blamejs"></script>\n!.
+		      qq!<script type="text/javascript">\n!.
+		      qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
+		      qq!           "!. href(-partial_query=>1) .qq!");\n!.
+		      qq!</script>\n!;
+	}
+
 	git_footer_html();
 }
 
+sub git_blame {
+	git_blame_common();
+}
+
+sub git_blame_incremental {
+	git_blame_common('incremental');
+}
+
+sub git_blame_data {
+	git_blame_common('data');
+}
+
 sub git_tags {
 	my $head = git_get_head_hash($project);
 	git_header_html();
-- 
1.6.3.3

Re: [PATCH 2/3] gitweb: Use "previous" header of git-blame -p in 'blame' view

[Jakub Narebski]

On Fri, 10 July 2009, Jakub Narebski wrote:

> +# if filename is surrounded in double quotes, it need to be unquoted
> +sub unquote_maybe {
> +       my $str = shift;
> +
> +       if ($str =~ /^"(.*)"$/) {
> +               return unquote($1);
> +       }
> +       return $str;
> +}

I'm sorry about that, but this is totally unnecessary, as 
unquote == unquote_maybe (unquotes only when necessary).

-- 
Jakub Narebski
Poland