[PATCHv2 00/10] gitweb: 'blame' view improvements

[PATCHv2 00/10] gitweb: 'blame' view improvements

[Jakub Narebski]

This is second version of my improvements to gitweb's 'blame' view,
  Subject: [PATCH 0/3] gitweb: 'blame' view improvements
  Message-Id: <200907102354.43232.jnareb@gmail.com>
  http://article.gmane.org/gmane.comp.version-control.git/123085

including some further improvements, and this time including
preparation and AJAX-y 'blame_incremental' view in series proper.
It also finally creates 'blame_incremental' links (last patch in
series).

The changes are also available in the git repository at:

  git://repo.or.cz/git/jnareb-git.git gitweb/web

Jakub Narebski (10):
  gitweb: Make .error style generic
  gitweb: Mark boundary commits in 'blame' view
  gitweb: Use "previous" header of git-blame -p in 'blame' view
  gitweb: Mark commits with no "previous" in 'blame' view
  gitweb: Add author initials in 'blame' view, a la "git gui blame"
  gitweb: Use light/dark for class names also in 'blame' view
  gitweb: Add -partial_query option to href() subroutine
  gitweb: Add optional "time to generate page" info in footer
  gitweb: Incremental blame (proof of concept)
  gitweb: Create links leading to 'blame_incremental' using JavaScript

 Makefile           |    6 +-
 git-instaweb.sh    |    7 +
 gitweb/blame.js    |  634 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.css  |   43 +++-
 gitweb/gitweb.perl |  332 ++++++++++++++++++++-------
 5 files changed, 930 insertions(+), 92 deletions(-)
 create mode 100644 gitweb/blame.js


Jakub Narebski (10):
  gitweb: Make .error style generic
  gitweb: Mark boundary commits in 'blame' view
  gitweb: Use "previous" header of git-blame -p in 'blame' view
  gitweb: Mark commits with no "previous" in 'blame' view
  gitweb: Add author initials in 'blame' view, a la "git gui blame"
  gitweb: Use light/dark for class names also in 'blame' view
  gitweb: Add -partial_query option to href() subroutine
  gitweb: Add optional "time to generate page" info in footer
  gitweb: Incremental blame (proof of concept)
  gitweb: Create links leading to 'blame_incremental' using JavaScript

 Makefile           |    6 +-
 git-instaweb.sh    |    7 +
 gitweb/blame.js    |  634 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.css  |   43 +++-
 gitweb/gitweb.perl |  332 ++++++++++++++++++++-------
 5 files changed, 930 insertions(+), 92 deletions(-)
 create mode 100644 gitweb/blame.js

[PATCH 01/10] gitweb: Make .error style generic

[Jakub Narebski]

Style for td.error was introduced in 1f1ab5f (gitweb: style done with
stylesheet, 2006-06-20) to replace inline style for errors in old
multi-column "git annotate" based 'blame' view.  This view was then
since removed (replaced by "git-blame" based 'blame' view, with fewer
colums), making this style unused.

Make this style more generic by replacing td.error with .error to make
it apply to any element.  It will be used in 'blame_incremental' view
to show error messages.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It is, as you can see, very simple change.  It makes it easier to
reuse currently "dead" style; unused after removing old git_blame by
Rafael Garcia-Suarez in 3a5b919 (gitweb: remove git_blame and rename
git_blame2 to git_blame, 2008-06-06).

 gitweb/gitweb.css |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index d05bc37..70b7c2f 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -262,7 +262,7 @@ td.sha1 {
 	font-family: monospace;
 }
 
-td.error {
+.error {
 	color: red;
 	background-color: yellow;
 }
-- 
1.6.3.3

[PATCH 02/10] 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 first cell in row, 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 slightly 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.

This is just resend of previous version of patch.

 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 70b7c2f..f47709b 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 7fbd5ff..3078b92 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

[PATCHv2 03/10] 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.
Boundary commits don't have parents, so "git rev-parse <commit>^"
returned literal "<commit>^" (which didn't exists).  Gitweb didn't
detect this situation and passed this result literally as 'hb'
parameter in 'linenr' link.  Following such link currently gives
  400 - Invalid hash base parameter
error; 'hb' parameter is restricted via validate_refname to correct
refnames and doesn't allow for extended SHA-1 syntax.  This bug could
have been fixed alternatively 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.

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

Acked-by: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Note that it does not change "sha1" link from 'commit' to 'commitdiff'
view, as requested by Junio C Hamano.  This is left for later,
separate commit.

New in v2:
 * we don't use totally unnecessary newly introduced unquote_maybe(),
   as unquote == unquote_maybe (unquotes only when necessary).
 * Added ACK from Luben Tuikov (author of 'data mining' behaviour)
   http://article.gmane.org/gmane.comp.version-control.git/123154

BTW. I think as a side effect this patch makes code a bit cleaner.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3078b92..b8a121b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4827,7 +4827,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 +4852,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($2);
+		}
+		my $linenr_commit =
+			exists($meta->{'parent'}) ?
+			$meta->{'parent'} : $full_rev;
+		my $linenr_filename =
+			exists($meta->{'file_parent'}) ?
+			$meta->{'file_parent'} : unquote($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 04/10] gitweb: Mark commits with no "previous" in 'blame' view

[Jakub Narebski]

Use "no-previous" class to mark blamed commits which do not have
"previous" header.  Those are commits in which blamed file was created
(added); this includes boundary commits.  This means that 'linenr'
link leads to blamed commit, not (one of) parent of blamed commit.
Therefore currently line number for such commit uses bold weight font
to denote this situation; the effect is subtle.

Use "multiple-previous" class in the opposite situation, where blamed
commit has multiple "previous" headers (is an evil merge).  Currently
this class is not used for styling.  In this situation 'linenr' link
leads to first of "previous" commits (first parent).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is new commit, which didn't appear in v1 of this series.

It does what marking "boundary" commit meant to do: it marks (via
admittedly quite subtle change of style) "linenr" links which lead to
'blame' view at blamed commit, not at parent commit (because there is
no parent commit).

This patch is after one using "previous" header for finding parents
for "linenr" links, because it also uses (still undocumented)
"previous" header.

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

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index f47709b..4763337 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -242,7 +242,8 @@ tr.dark:hover {
 	background-color: #edece6;
 }
 
-tr.boundary td.sha1 {
+tr.boundary td.sha1,
+tr.no-previous td.linenr {
 	font-weight: bold;
 }
 
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b8a121b..128bddd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4819,7 +4819,7 @@ HTML
 		my ($full_rev, $orig_lineno, $lineno, $group_size) =
 		   ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
 		if (!exists $metainfo{$full_rev}) {
-			$metainfo{$full_rev} = {};
+			$metainfo{$full_rev} = { 'nprevious' => 0 };
 		}
 		my $meta = $metainfo{$full_rev};
 		my $data;
@@ -4829,6 +4829,9 @@ HTML
 			if ($data =~ /^(\S+)(?: (.*))?$/) {
 				$meta->{$1} = $2 unless exists $meta->{$1};
 			}
+			if ($data =~ /^previous /) {
+				$meta->{'nprevious'}++;
+			}
 		}
 		my $short_rev = substr($full_rev, 0, 8);
 		my $author = $meta->{'author'};
@@ -4840,6 +4843,8 @@ HTML
 		}
 		my $tr_class = $rev_color[$current_color];
 		$tr_class .= ' boundary' if (exists $meta->{'boundary'});
+		$tr_class .= ' no-previous' if ($meta->{'nprevious'} == 0);
+		$tr_class .= ' multiple-previous' if ($meta->{'nprevious'} > 1);
 		print "<tr id=\"l$lineno\" class=\"$tr_class\">\n";
 		if ($group_size) {
 			print "<td class=\"sha1\"";
-- 
1.6.3.3

[PATCHv2 05/10] 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 that blame the same commit has 2 or more lines in it.

Initials are extracted using i18n /\b([[:upper:]])\B/g regexp.

Additionally initials help to distinguish boundary commits, as they
use bold weight font too (in addition to shortened SHA-1 of commit).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version of patch differs from previous version only in more
detailed commit message.  The change it introduces are identical.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 128bddd..ea1ab5f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4855,6 +4855,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

[PATCH/RFC 06/10] gitweb: Use light/dark for class names 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 patch is identical to previous version (patch 4/3 in previous
version of this series).

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  |   17 ++++++++++-------
 gitweb/gitweb.perl |    2 +-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 4763337..8f68fe3 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -226,22 +226,25 @@ 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 */
+/* and commits without "previous" */
 tr.boundary td.sha1,
 tr.no-previous td.linenr {
 	font-weight: bold;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ea1ab5f..2cb60be 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4801,7 +4801,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

[PATCH 07/10] gitweb: Add -partial_query option to href() subroutine

[Jakub Narebski]

href(..., -partial_query=>1) is meant to generate links which have not
all parameters filled, and which can be completed by simply appending
';<param>=<value>'.  This feature was implemented for future AJAX-y
'blame_incremental' in JavaScript.

Originally by Petr Baudis as part of "gitweb: Incremental blame"
patch, to deal with path_info URLs in JavaScript part easily and
correctly.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This change is extracted from incremental blame patch by Petr Baudis.
In previous version of this series it was part of 5/3 patch
introducing 'blame_incremental' view.

However as you can see in incremental blame patch by Matrin Koegler:
  http://thread.gmane.org/gmane.comp.version-control.git/47902/focus=47905

this is not strictly necessary. JavaScript (which is sole user of this
'-partial-query' parameter) can simply check if there is '?' character
in link already.  On the other hand side it makes JavaScript code a bit
simpler.

 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2cb60be..0d91ac7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -992,7 +992,8 @@ sub href {
 			}
 		}
 	}
-	$href .= "?" . join(';', @result) if scalar @result;
+	$href .= "?" . join(';', @result)
+		if ($params{-partial_query} or scalar @result);
 
 	return $href;
 }
-- 
1.6.3.3

[PATCH 08/10] gitweb: Add optional "time to generate page" info in footer

[Jakub Narebski]

Add "This page took XXXs and Y git commands to generate" to page
footer, if global feature 'timed' is enabled (disabled by default).
Requires Time::HiRes installed for high precision 'wallclock' time.

This code is based on example code by Petr 'Pasky' Baudis.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch was extracted from 5/3 patch implementing interactive
blame, and modified to be conditional on 'timed' feature, and to show
also number of git commands used.

Note that setting $t0 variable should be fairly early to have running
time of the whole script.  The same for $number_of_git_cmds.

Current formatting is very basic, just like before.  It simply uses 
'page_footer' style.

Variable names and name of feature is up to debate.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0d91ac7..bd77b31 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,6 +18,12 @@ use File::Find qw();
 use File::Basename qw(basename);
 binmode STDOUT, ':utf8';
 
+our $t0;
+if (eval { require Time::HiRes; 1; }) {
+	$t0 = [Time::HiRes::gettimeofday()];
+}
+our $number_of_git_cmds = 0;
+
 BEGIN {
 	CGI->compile() if $ENV{'MOD_PERL'};
 }
@@ -394,6 +400,13 @@ our %feature = (
 		'sub' => \&feature_avatar,
 		'override' => 0,
 		'default' => ['']},
+
+	# Enable displaying how much time and how many git commands
+	# it took to generate and display page.  Disabled by default.
+	# Project specific override is not supported.
+	'timed' => {
+		'override' => 0,
+		'default' => [0]},
 );
 
 sub gitweb_get_feature {
@@ -507,6 +520,7 @@ if (-e $GITWEB_CONFIG) {
 
 # version of the core git binary
 our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
+$number_of_git_cmds++;
 
 $projects_list ||= $projectroot;
 
@@ -1956,6 +1970,7 @@ sub get_feed_info {
 
 # returns path to the core git executable and the --git-dir parameter as list
 sub git_cmd {
+	$number_of_git_cmds++;
 	return $GIT, '--git-dir='.$git_dir;
 }
 
@@ -3206,6 +3221,20 @@ sub git_footer_html {
 	}
 	print "</div>\n"; # class="page_footer"
 
+	if (defined $t0 && gitweb_check_feature('timed')) {
+		print "<div id=\"generate_info\" class=\"page_footer\">\n";
+		print 'This page took '.
+		      '<span id="generate_time" class="time_span">'.
+		      Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).'s'.
+		      '</span>'.
+		      ' and '.
+		      '<span id="generate_cmd">'.
+		      $number_of_git_cmds.
+		      '</span> git commands '.
+		      " to generate.\n";
+		print "</div>\n"; # class="page_footer"
+	}
+
 	if (-f $site_footer) {
 		insert_file($site_footer);
 	}
-- 
1.6.3.3

[PATCHv2/RFC 09/10] gitweb: Incremental blame (proof of concept)

[Jakub Narebski]

This is tweaked up version of Petr Baudis <pasky@suse.cz> patch, which
in turn 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 Per 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 from gitweb.perl in JavaScript
* 'blame_incremental' always uses "rowspan" appribite, even if
  rowspan="1".  This simplifies code, and is not visible to user.

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.

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/47614
   http://article.gmane.org/gmane.comp.version-control.git/56657
3. New link rewriting and some optimization in Matrin Koegler
   series introducing some JavaScript support in Git
   http://thread.gmane.org/gmane.comp.version-control.git/47902/focus=47905   
4. First, second and third version by me (Jakub Narebski)
   http://thread.gmane.org/gmane.comp.version-control.git/102657/focus=102712
   http://article.gmane.org/gmane.comp.version-control.git/123202

Changes compared to the last version:
* simplified spacePad function in blame.js; add padLeft (unused)
* deal with lack of 'timed' info from server, and with expanded
  information including number of git commands used
* a few style fixes (like declaring previously un-declared variables). 
* fixed escape code sequences; they are different for JavaScript than
  for Perl (and for C).
* refactor communicating error message to user into errorInfo(str)
* use /regex/ instead of new RegExp("regex")
* use DOM2 HTML (faster) in place of DOM2 Core when possible,
  e.g. elem.className instead of elem.getAttribute('class') 
* use === instead of == and !== instead of !=
* first column cells doesn't need now (at least in theory) 'rowspan'
  attribute to be present (set)
* instead of creating list of classes in fixColorsAndGroups(), replace
  colorN by light/dark in class name(s).
* add unquote() to deal with quoted filenames (e.g. with TAB character
  encoded as C escape sequence "\t")
* set and deal with "no-previous" and "multiple-previous" classes;
  this was somewhat-ported from 'blame' view in gitweb.perl

TODO list:
* Using 'application/xhtml+xml' Content-Type, together with 
  XHTML 1.0 Strict doctype turns on strict validation (and not
  continuing on errors, if possible) in some web browsers.
  Unfortunately the web browser I use (Mozilla 1.17.2) turns on
  strict mode also for _JavaScript_, which means no innerHTML.
  
  In current patch gitweb is modified to always return 'text/html'.
  Better solution would be to return 'text/html' only for
  'blame_incremental' view, or replace innerHTML by DOM2 manipulations
  (which are unfortunately usually slower).

* 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 or move to separate commit changes which help CPerl
  mode for GNU Emacs to deal with syntax highlighting (#', #").
  Best via simplifying complex regexp, as in "Perl best Practices".
* Remove (fade out) progress bar and progress info after blame
  incremental finished run (left for a separate commit).
* Instead of running startBlame, put it in window.onload handler

 Makefile           |    6 +-
 git-instaweb.sh    |    7 +
 gitweb/blame.js    |  632 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.css  |   19 ++
 gitweb/gitweb.perl |  287 ++++++++++++++++--------
 5 files changed, 856 insertions(+), 95 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..5a8a29f
--- /dev/null
+++ b/gitweb/blame.js
@@ -0,0 +1,632 @@
+// 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 left, to WIDTH characters
+// example: spacePad(12, 3) == '&nbsp;12' ('&nbsp;' is nonbreakable space)
+function spacePad(n, width) {
+	var prefix = '';
+
+	width -= n.toString().length;
+	while (width > 1) {
+		prefix += '&nbsp;';
+		width--;
+	}
+	return prefix + n;
+}
+
+/**
+ * @param {string} input: input value converted to string.
+ * @param {number} size: desired length of output.
+ * @param {string} ch: single character to prefix to s.
+ */
+function padLeft(input, size, ch) {
+	var s = input + "";
+	while (s.length < size) {
+		s = ch + s;
+	}
+	return s;
+}
+
+// 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 = {};
+
+// constructor for Commit objects, used in 'blame'
+function Commit(sha1) {
+	this.sha1 = sha1;
+	this.nprevious = 0; /* blame-specific */
+}
+
+/* ............................................................ */
+/* progress info, timing, error reporting */
+
+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.firstChild.data = ... /* text node */ ???
+		div_progress_info.innerHTML  = blamedLines + ' / ' + totalLines +
+			' (' + spacePad(percentage, 3) + '%)';
+	}
+
+	if (div_progress_bar) {
+		//div_progress_bar.setAttribute('style', 'width: '+percentage+'%;');
+		div_progress_bar.style.width = percentage + '%';
+	}
+}
+
+
+var t_interval_server = '';
+var cmds_server = '';
+var t0 = new Date();
+
+// write how much it took to generate data, and to run script
+function writeTimeInterval() {
+	var info_time = document.getElementById('generate_time');
+	if (!info_time || !t_interval_server) {
+		return;
+	}
+	var t1 = new Date();
+	//info_time.firstChild.data = ... /* text node */ ???
+	info_time.innerHTML += ' + (' +
+		t_interval_server + 's server blame_data / ' +
+		(t1.getTime() - t0.getTime())/1000 + 's client JavaScript)';
+
+	var info_cmds = document.getElementById('generate_cmd');
+	if (!info_time || !cmds_server) {
+		return;
+	}
+	info_cmds.innerHTML += ' + ' + cmds_server;
+}
+
+// show an error message alert to user within page
+function errorInfo(str) {
+	if (!div_progress_info) {
+		div_progress_info = document.getElementById('progress_info');
+	}
+	if (div_progress_info) {
+		div_progress_info.className = 'error';
+		div_progress_info.innerHTML = str; /* can contain HTML */
+	}
+}
+
+/* ............................................................ */
+/* coloring rows during blame_data (git blame --incremental) run */
+
+// used to extract N from colorN, where N is a number,
+var colorRe = /\bcolor([0-9]*)\b/;
+
+// 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.className;
+	if (className) {
+		var 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.className === '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;
+	var table =
+		document.getElementById('blame_table') ||
+		document.getElementsByTagName('table')[0];
+
+	while ((tr = document.getElementById('l'+linenum))) {
+	// index origin is 0, which is table header
+	//while ((tr = table.rows[linenum])) {
+		if (isStartOfGroup(tr, linenum, document)) {
+			if (prev_group &&
+			    prev_group.firstChild.firstChild.href ===
+			            tr.firstChild.firstChild.href) {
+				// we have to concatenate groups
+				var prev_rows = prev_group.firstChild.rowSpan || 1;
+				var curr_rows =         tr.firstChild.rowSpan || 1;
+				prev_group.firstChild.rowSpan = prev_rows + curr_rows;
+				//tr.removeChild(tr.firstChild);
+				tr.deleteCell(0); // DOM2 HTML way
+			} else {
+				colorClass = (colorClass + 1) % 2;
+				prev_group = tr;
+			}
+		}
+		var tr_class = tr.className;
+		tr.className = tr_class.replace(colorRe, colorClasses[colorClass]);
+		linenum++;
+	}
+}
+
+/* ............................................................ */
+/* time and data */
+
+// used to extract hours and minutes from timezone info, e.g '-0900'
+var tzRe = /^([+-][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 formatDateISOLocal(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 */
+
+var escCodeRe = /\\([^0-7]|[0-7]{1,3})/g;
+
+// unquote maybe git-quoted filename
+function unquote(str) {
+	function unq(seq) {
+		var es = { // character escape codes, aka escape sequences
+			t: "\t",   // tab            (HT, TAB)
+			n: "\n",   // newline        (NL)
+			r: "\r",   // return         (CR)
+			f: "\f",   // form feed      (FF)
+			b: "\b",   // backspace      (BS)
+			a: "\x07", // alarm (bell)   (BEL)
+			e: "\x1B", // escape         (ESC)
+			v: "\v"    // vertical tab   (VT)
+		};
+
+		if (seq.search(/^[0-7]{1,3}$/) !== -1) {
+			// octal char sequence
+			return String.fromCharCode(parseInt(seq, 8));
+		} else if (seq in es) {
+			// C escape sequence, aka character escape code
+			return es[seq];
+		}
+		// quoted ordinary character
+		return seq;
+	}
+
+	var match = str.match(/^\"(.*)\"$/);
+	if (match) {
+		str = match[1];
+		// perhaps str = eval('"'+str+'"'); would be enough?
+		str = str.replace(escCodeRe,
+			function (substr, p1, offset, s) { return unq(p1); });
+	}
+	return str;
+}
+
+/* ============================================================ */
+/* 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 + ', ' +
+			formatDateISOLocal(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';
+		}
+		if (commit.nprevious === 0) {
+			tr_class += ' no-previous';
+		} else if (commit.nprevious > 1) {
+			tr_class += ' multiple-previous';
+		}
+		tr.className = tr_class;
+
+		/* <td class="sha1" title="?" rowspan="?"><a href="?">?</a></td> */
+		if (i === 0) {
+			td_sha1.title = commit.info;
+			td_sha1.rowSpan = commit.numlines;
+
+			a_sha1.href = projectUrl + ';a=commit;h=' + commit.sha1;
+			//a_sha1.firstChild.data = ... /* text node */ ???
+			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> */
+		var linenr_commit =
+			('previous' in commit ? commit.previous : commit.sha1);
+		var linenr_filename =
+			('file_parent' in commit ? 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 = /^([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)/;
+var infoRe = /^([a-z-]+) ?(.*)/;
+var endRe  = /^END ?([^ ]*) ?(.*)/;
+var curCommit = new Commit();
+
+var pollTimer = null;
+
+// handler for XMLHttpRequest onreadystatechange events
+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');
+		}
+
+		errorInfo('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 = unquote(data);
+					// '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':
+					curCommit.nprevious++;
+					if (!'previous' in curCommit) {
+						var parts = data.split(' ', 2);
+						curCommit.previous    = parts[0];
+						curCommit.file_parent = unquote(parts[1]);
+					}
+					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];
+				cmds_server = match[2];
+				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) {
+		errorInfo('<b>ERROR:</b> XMLHttpRequest not supported');
+		return;
+	}
+
+	t0 = new Date();
+	projectUrl = bUrl;
+	if ((div_progress_bar = document.getElementById('progress_bar'))) {
+		div_progress_bar.setAttribute('style', 'width: 100%;');
+		//div_progress_bar.style.value = 'width: 100%;';
+	}
+	totalLines = countLines();
+	updateProgressInfo();
+
+	http.open('get', blamedataUrl);
+	http.setRequestHeader('Accept', 'text/plain'); // in case of future changes
+	// perhaps also 'multipart/x-mixed-replace'
+	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 8f68fe3..8b05604 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -250,6 +250,14 @@ tr.no-previous td.linenr {
 	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%;
@@ -341,6 +349,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 bd77b31..036f8da 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -96,6 +96,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/";
@@ -564,6 +566,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,
@@ -1721,7 +1725,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"},
@@ -3048,13 +3052,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'}" : '';
@@ -4024,7 +4028,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/!!;
@@ -4788,7 +4792,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");
@@ -4810,10 +4816,43 @@ 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';
+		if (defined $t0 && gitweb_check_feature('timed')) {
+			print ' '.
+			      Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).
+			      ' '.$number_of_git_cmds;
+		}
+		print "\n";
+
+		return;
+	}
 
 	# page header
 	git_header_html();
@@ -4824,109 +4863,169 @@ 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} = { 'nprevious' => 0 };
-		}
-		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!;
+		}
+
+	} 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} = { 'nprevious' => 0 };
 			}
-			if ($data =~ /^previous /) {
-				$meta->{'nprevious'}++;
+			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 ($data =~ /^previous /) {
+					$meta->{'nprevious'}++;
+				}
 			}
-		}
-		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'});
-		$tr_class .= ' no-previous' if ($meta->{'nprevious'} == 0);
-		$tr_class .= ' multiple-previous' if ($meta->{'nprevious'} > 1);
-		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('.', ...)
+			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'});
+			$tr_class .= ' no-previous' if ($meta->{'nprevious'} == 0);
+			$tr_class .= ' multiple-previous' if ($meta->{'nprevious'} > 1);
+			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";
 			}
-			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($2);
-		}
-		my $linenr_commit =
-			exists($meta->{'parent'}) ?
-			$meta->{'parent'} : $full_rev;
-		my $linenr_filename =
-			exists($meta->{'file_parent'}) ?
-			$meta->{'file_parent'} : unquote($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";
+			# '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($2);
+			}
+			my $linenr_commit =
+				exists($meta->{'parent'}) ?
+				$meta->{'parent'} : $full_rev;
+			my $linenr_filename =
+				exists($meta->{'file_parent'}) ?
+				$meta->{'file_parent'} : unquote($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

[PATCH/RFC 10/10] gitweb: Create links leading to 'blame_incremental' using JavaScript

[Jakub Narebski]

The new 'blame_incremental' view requires JavaScript to run.  Not all
web browsers implement JavaScript (e.g. text browsers such as Lynx),
and not all users have JavaScript enabled.  Therefore instead of
unconditionally link to 'blame_incremental' view, we use JavaScript to
convert those links to lead to view utilizing JavaScript, by adding
'js=1' to link.

The only JavaScript-aware/using view is currently 'blame_incremental'.
As first, it might want to have links to non-JavaScript version, and
second, it should also use window.onload, we do not add nor run
fixLinks() for such views (currently hardcoded 'blame_incremental')

Possible enhancement would be to do JavaScript redirect by setting
window.location instead of modifying $format and $action in
git_blame_common() subroutine.


This idea was originally implemented by Petr Baudis in
  http://article.gmane.org/gmane.comp.version-control.git/47614
but it added <script> element with fixBlameLinks() function in page
header, to be added as onload event using 'onload' attribute of HTML
'body' element: <body onload="fixBlameLinks();">.  This version adds
script at then end of page (in the page footer), and uses JavaScript
'window.onload=fixLinks();'.  Also in Petr version only links marked
with 'blamelink' class were modified, and they were modified by
replacing "a=blame" by "a=blame_incremental"... which doesn't work for
path_info links, and might replace wrong part if there is "a=blame" in
project name, ref name or file name.

Slightly different solution was implemented by Martin Koegler in
  http://thread.gmane.org/gmane.comp.version-control.git/47902/focus=47905
Here GitAddLinks() function was in gitweb.js file, not as contents of
<script> element.  This might be a better solution (although I think
it would be better to split JavaScript file and load only parts that
are required).  It was also included in page header (in <head>
element) though, which means waiting for a script to load (and run).
It was smarter in that to "fix" (modify) link, it split URL, modified
value of 'a' parameter, and then recreated modified link.  It avoids
trouble with "a=blame" as substring in project name or file name, but
it doesn't work with path_info URL/link in the way it was written.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new, and didn't appear in any way in any of my earlier
series dealing with incremental blame view.

TODO list:
* Extract and remove unrelated changes, like updating copyright
* Perhaps put fixLinks() function in separate file gitweb.js.
  Should gitweb use single JavaScript file, or should it be split into
  more than one file?
* Better solution to "don't invoke for JavaScript-aware actions"
  problem.  Currently hardcoded 'blame_incremental'.

 gitweb/blame.js    |    8 +++++---
 gitweb/gitweb.perl |   21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/gitweb/blame.js b/gitweb/blame.js
index 5a8a29f..d8ecee1 100644
--- a/gitweb/blame.js
+++ b/gitweb/blame.js
@@ -1,4 +1,6 @@
 // Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+//               2007, Petr Baudis <pasky@suse.cz>
+//          2008-2009, Jakub Narebski <jnareb@gmail.com>
 
 /* ============================================================ */
 /* generic utility functions */
@@ -36,7 +38,7 @@ function spacePad(n, width) {
 /**
  * @param {string} input: input value converted to string.
  * @param {number} size: desired length of output.
- * @param {string} ch: single character to prefix to s.
+ * @param {string} ch: single character to prefix to string.
  */
 function padLeft(input, size, ch) {
 	var s = input + "";
@@ -611,14 +613,14 @@ function startBlame(blamedataUrl, bUrl) {
 	projectUrl = bUrl;
 	if ((div_progress_bar = document.getElementById('progress_bar'))) {
 		div_progress_bar.setAttribute('style', 'width: 100%;');
-		//div_progress_bar.style.value = 'width: 100%;';
+		//div_progress_bar.style.value = 'width: 100%;'; // doesn't work
 	}
 	totalLines = countLines();
 	updateProgressInfo();
 
 	http.open('get', blamedataUrl);
 	http.setRequestHeader('Accept', 'text/plain'); // in case of future changes
-	// perhaps also 'multipart/x-mixed-replace'
+	// perhaps also, in the future, 'multipart/x-mixed-replace' (not standard)
 	http.onreadystatechange = handleResponse;
 	//http.onreadystatechange = function () { handleResponse(http); };
 	http.send(null);
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 036f8da..38492d0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3243,6 +3243,23 @@ sub git_footer_html {
 		insert_file($site_footer);
 	}
 
+	if ($action ne 'blame_incremental') {
+		print <<'HTML';
+<script type="text/javascript">/* <![CDATA[ */
+function fixLinks() {
+	//var allLinks = document.getElementsByTagName("a");
+	var allLinks = document.links;
+	for (var i = 0; i < allLinks.length; i++) {
+		var link = allLinks[i];
+		link.href +=
+			(link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1';
+	}
+}
+window.onload = fixLinks;
+/* ]]> */</script>
+HTML
+	}
+
 	print "</body>\n" .
 	      "</html>";
 }
@@ -4794,6 +4811,10 @@ sub git_tag {
 
 sub git_blame_common {
 	my $format = shift || 'porcelain';
+	if ($format eq 'porcelain' && $cgi->param('js')) {
+		$format = 'incremental';
+		$action = 'blame_incremental'; # for page title etc
+	}
 
 	# permissions
 	gitweb_check_feature('blame')
-- 
1.6.3.3

Re: [PATCHv2 00/10] gitweb: 'blame' view improvements

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> This is second version of my improvements to gitweb's 'blame' view,
>   Subject: [PATCH 0/3] gitweb: 'blame' view improvements
>   Message-Id: <200907102354.43232.jnareb@gmail.com>
>   http://article.gmane.org/gmane.comp.version-control.git/123085
>
> including some further improvements, and this time including
> preparation and AJAX-y 'blame_incremental' view in series proper.
> It also finally creates 'blame_incremental' links (last patch in
> series).

I understand that this series replaces the four-patch series between
"git log --oneline a6be48b^..3643cc0"---am I correct?

Re: [PATCHv2 00/10] gitweb: 'blame' view improvements

[Jakub Narebski]

Dnia sobota 25. lipca 2009 01:47, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > This is second version of my improvements to gitweb's 'blame' view,
> >   Subject: [PATCH 0/3] gitweb: 'blame' view improvements
> >   Message-Id: <200907102354.43232.jnareb@gmail.com>
> >   http://article.gmane.org/gmane.comp.version-control.git/123085
> >
> > including some further improvements, and this time including
> > preparation and AJAX-y 'blame_incremental' view in series proper.
> > It also finally creates 'blame_incremental' links (last patch in
> > series).
> 
> I understand that this series replaces the four-patch series between
> "git log --oneline a6be48b^..3643cc0"---am I correct?

Yes, it does.  In particular it fixes my mistake about need for in 
unquote_maybe in "gitweb: Use "previous" header of git-blame -p in 
'blame' view".  And that is the only change (except extending commit 
message) in a6be48b^..3643cc0.

I see that I forgot to mark "gitweb: Add -partial_query option to href() 
subroutine" as RFC; it isn't I think strictly necessary.  But that is 
why I put it in separate commit: to easy delete it or revert if deemed 
not needed.

-- 
Jakub Narebski
Poland

Re: [PATCH 02/10] gitweb: Mark boundary commits in 'blame' view

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> 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 first cell in row, that contains SHA-1 of a commit).
> ...
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 70b7c2f..f47709b 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;
> +}
> +

"boundary" means that "blame low..hight file" attributed the line to the
"low" commit, not because the commit introduced the line, but because the
user said not to bother digging further.

I had an assumption that in such a bounded blame, lines attributed to the
boundary commit are not very interesting (they belong to a distant stable
past that the user does not care much about, as opposed to more recent
breakages), and that is exactly the same reasoning behind the -b option of
"git blame" command.

I would have expected the boundary to be shown in weaker decoration
(e.g. gray letters as opposed to black), not in stronger annotation.

Perhaps you are talking about something different?  I am a bit puzzled.

Re: [PATCH 02/10] gitweb: Mark boundary commits in 'blame' view

[Jakub Narebski]

On Sat, 25 July 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > 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 first cell in row, that contains SHA-1 of a commit).
> > ...
> > diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> > index 70b7c2f..f47709b 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;
> > +}
> > +
> 
> "boundary" means that "blame low..hight file" attributed the line to the
> "low" commit, not because the commit introduced the line, but because the
> user said not to bother digging further.

Well, currently 'blame' view in gitweb doesn't allow to limit revision
range from below, i.e. to state "low" commit; it doesn't use 'hpb' 
(hash_parent_base) parameter.  So boundary commit means root commit.

> 
> I had an assumption that in such a bounded blame, lines attributed to the
> boundary commit are not very interesting (they belong to a distant stable
> past that the user does not care much about, as opposed to more recent
> breakages), and that is exactly the same reasoning behind the -b option of
> "git blame" command.
> 
> I would have expected the boundary to be shown in weaker decoration
> (e.g. gray letters as opposed to black), not in stronger annotation.

Well, weaker decoration is, I think, actually harder to do in CSS...

> Perhaps you are talking about something different?  I am a bit puzzled.

Well, I have thought that only boundary commits can be without previous
[blame] commit, but I noticed that it is not the case: see 04/10.  But
some of that remains of my mistaken belief can resonate in commit 
message... ;-)

-- 
Jakub Narebski
Poland

Re: [PATCH 02/10] gitweb: Mark boundary commits in 'blame' view

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> On Sat, 25 July 2009, Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>> 
>> > 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 first cell in row, that contains SHA-1 of a commit).
>> > ...
>> > diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
>> > index 70b7c2f..f47709b 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;
>> > +}
>> > +
>> 
>> "boundary" means that "blame low..hight file" attributed the line to the
>> "low" commit, not because the commit introduced the line, but because the
>> user said not to bother digging further.
>
> Well, currently 'blame' view in gitweb doesn't allow to limit revision
> range from below, i.e. to state "low" commit; it doesn't use 'hpb' 
> (hash_parent_base) parameter.  So boundary commit means root commit.

Yes, but I think it is the same thing.  The initial import of a tarball,
that is "a distant stable past as opposed to more recent breakages".

Re: [PATCH/RFC 10/10] gitweb: Create links leading to 'blame_incremental' using JavaScript

[Martin Koegler]

On Sat, Jul 25, 2009 at 12:44:10AM +0200, Jakub Narebski wrote:
> TODO list:
> * Perhaps put fixLinks() function in separate file gitweb.js.
>   Should gitweb use single JavaScript file, or should it be split into
>   more than one file?

The same question can be asked for gitweb itself:

Why is it a single perl file and not splited in many different
modules?

mfg Martin Kögler

Re: [PATCHv2/RFC 09/10] gitweb: Incremental blame (proof of concept)

[Jakub Narebski]

On Sat, 25 July 2009, Jakub Narebski wrote:

> * set and deal with "no-previous" and "multiple-previous" classes;
>   this was somewhat-ported from 'blame' view in gitweb.perl

Note that it is something that we unfortunately would have to deal with, 
namely playing catch-up with new features in gitweb's 'blame' view, 
"duplicating" quite a bit of code (well, rewriting Perl code in 
JavaScript).  But this is, I think, unfortunately unavoidable, unless 
we move to generating JavaScript code from Perl, e.g. using CGI::Ajax
(something that GWT does with Java and JavaScript).

-- 
Jakub Narebski
Poland

Re: [PATCH/RFC 10/10] gitweb: Create links leading to 'blame_incremental' using JavaScript

[Jakub Narebski]

On Sat, 25 Jul 2009, Martin Koegler wrote:
> On Sat, Jul 25, 2009 at 12:44:10AM +0200, Jakub Narebski wrote:

> > TODO list:
> > * Perhaps put fixLinks() function in separate file gitweb.js.
> >   Should gitweb use single JavaScript file, or should it be split into
> >   more than one file?
> 
> The same question can be asked for gitweb itself:

Well, there is one important difference: gitweb itself is not send
over network to client.  JavaScript is.  (Although I'm not sure how
great it is of an issue, with browsers caching JavaScript.  Perhaps
one single file would be better idea.)

> 
> Why is it a single perl file and not splited in many different
> modules?

There are a few causes.

1. Gitweb (then gitweb.cgi) started as single file.  There is a bit
   of resistance to changing this, especially that splitting it might
   make it harder to interate changes from other people who still use
   single file gitweb (see for example gitweb fork at git.kernel.org
   history).

2. Having it all in single file make its easy to install and update.
   Well, it made more sense when only way to configure gitweb was to
   edit gitweb.cgi.  Now building gitweb.cgi is the task for build
   system, and the only thing left is to copy files in correct place
   (I think that there are distribution specific packages which makes
   installing gitweb as easy as "xxx install gitweb").

3. You would have to decide _how_ to split it into many different 
   modules.  Do you know any good examples?

-- 
Jakub Narebski
Poland

Re: [PATCH/RFC 10/10] gitweb: Create links leading to 'blame_incremental' using JavaScript

[Martin Koegler]

On Sun, Jul 26, 2009 at 12:06:11PM +0200, Jakub Narebski wrote:
> On Sat, 25 Jul 2009, Martin Koegler wrote:
> > On Sat, Jul 25, 2009 at 12:44:10AM +0200, Jakub Narebski wrote:
> 
> > > TODO list:
> > > * Perhaps put fixLinks() function in separate file gitweb.js.
> > >   Should gitweb use single JavaScript file, or should it be split into
> > >   more than one file?
> > 
> > The same question can be asked for gitweb itself:
> 
> Well, there is one important difference: gitweb itself is not send
> over network to client.  JavaScript is.  (Although I'm not sure how
> great it is of an issue, with browsers caching JavaScript.  Perhaps
> one single file would be better idea.)

More files mean more request on the server. If the browser is
configured to check at each request, it will issue a GET for each
JavaScript file, which will be answered by a 304 after the first
request. In the "automatic mode", the browser waits for some time
(determined by a heuristic), before it will issue a GET for each file
request again.

So in my option, on (bigger) file is better, as it means fewer request.

> > 
> > Why is it a single perl file and not splited in many different
> > modules?
> 
> 2. Having it all in single file make its easy to install and update.
>    Well, it made more sense when only way to configure gitweb was to
>    edit gitweb.cgi.  Now building gitweb.cgi is the task for build
>    system, and the only thing left is to copy files in correct place
>    (I think that there are distribution specific packages which makes
>    installing gitweb as easy as "xxx install gitweb").

Yes, there are gitweb packages, which automaticially server
repositories under a specific path (eg. /srv/git). For such packages,
the js layout is irrelavant.

When manually installing, copying only one javascript file simplifies
the deployment.

> 3. You would have to decide _how_ to split it into many different 
>    modules.  Do you know any good examples?

Javascript uses on global namespace. If it is one file, its implicitly
clear, that everything (functions, variables) are in one scope.  If
you split it into multiple files, you have to remember, what the other
files contain.

mfg Martin Kögler

Re: [PATCH/RFC 10/10] gitweb: Create links leading to 'blame_incremental' using JavaScript

[Jakub Narebski]

On Mon, 27 July 2009, Martin Koegler wrote:
> On Sun, Jul 26, 2009 at 12:06:11PM +0200, Jakub Narebski wrote:
>> On Sat, 25 Jul 2009, Martin Koegler wrote:
>>> On Sat, Jul 25, 2009 at 12:44:10AM +0200, Jakub Narebski wrote:
>> 
>>>> TODO list:
>>>> * Perhaps put fixLinks() function in separate file gitweb.js.
>>>>   Should gitweb use single JavaScript file, or should it be split into
>>>>   more than one file?
>>> 
>>> The same question can be asked for gitweb itself:
>> 
>> Well, there is one important difference: gitweb itself is not send
>> over network to client.  JavaScript is.  (Although I'm not sure how
>> great it is of an issue, with browsers caching JavaScript.  Perhaps
>> one single file would be better idea.)
> 
> More files mean more request on the server. If the browser is
> configured to check at each request, it will issue a GET for each
> JavaScript file, which will be answered by a 304 after the first
> request. In the "automatic mode", the browser waits for some time
> (determined by a heuristic), before it will issue a GET for each file
> request again.
> 
> So in my option, on (bigger) file is better, as it means fewer request.

I didn't means splitting JavaScript file based on functionality, like
one would do with modules in C, for example.  I was wondering if it 
would be worth to keep gitweb.js with common utility functions and 
functions used in all or nearly all views separate from gitweb-blame.js
which would be loaded and used only for 'blame_incremental' view.

But I am not sure if it is worth complications with build procedure:
installation and configuration.  With single JavaScript file it is
enough to have GITWEB_JAVASCRIPT or GITWEB_JS build variable/option,
and that would be enough.  Not so if we have gitweb-blame.js and 
perhaps other such files.


BTW. we can minify JavaScript file during the build (for example using
JSMin, or one of its derivatives), and e.g. use "gitweb.min.js" in
HTML generated by gitweb.perl / gitweb.cgi.

>>> 
>>> Why is it a single perl file and not splited in many different
>>> modules?
>> 
>> 2. Having it all in single file make its easy to install and update.
>>    Well, it made more sense when only way to configure gitweb was to
>>    edit gitweb.cgi.  Now building gitweb.cgi is the task for build
>>    system, and the only thing left is to copy files in correct place
>>    (I think that there are distribution specific packages which makes
>>    installing gitweb as easy as "xxx install gitweb").
> 
> Yes, there are gitweb packages, which automaticially server
> repositories under a specific path (eg. /srv/git). For such packages,
> the js layout is irrelavant.
> 
> When manually installing, copying only one javascript file simplifies
> the deployment.

OTOH if we split Perl source of gitweb (it is second in size only to
gitk; fourth in size git-gui got already split into smaller modules)
then dealing with multiple JavaScript wouldn't be the problem in build.

> 
>> 3. You would have to decide _how_ to split it into many different 
>>    modules.  Do you know any good examples?
> 
> Javascript uses on global namespace. If it is one file, its implicitly
> clear, that everything (functions, variables) are in one scope.  If
> you split it into multiple files, you have to remember, what the other
> files contain.

True.  You can however use single global object and store what would
be global variables as properties (fields) of such object...

Besides I'm not advocating splitting into many small files.  Only that
much files as really necessary.

-- 
Jakub Narebski
Poland