[PATCH 0/5] gitweb: Support for arbitrary diffs

[PATCH 0/5] gitweb: Support for arbitrary diffs

[Martin Koegler]

Resend of the complete patch serie (again next).

Patch 1-3 are unchanged.

Patch 4 now only generates the links, if they are enabled.
The header contains a new link to show/hide the links.
The state is remembered via a cookie across pages.

Patch 5 is the foundation for showing the base/diff links
in an other color.

mfg Martin Kögler
PS:
I develop the patches via StGit. If you think that I should publish them
in a different way, please tell me.

[PATCH 1/5] gitweb: Support comparing blobs with different names

[Martin Koegler]

Currently, blobdiff can only compare blobs with different file
names, if no hb/hpb parameters are present.

This patch adds support for comparing two blobs specified by any
combination of hb/f/h and hpb/fp/hp.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 gitweb/gitweb.perl |  148 +++++++++++++++++++---------------------------------
 1 files changed, 53 insertions(+), 95 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b2bae1b..05bfb26 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4645,109 +4645,66 @@ sub git_blobdiff {
 	my $fd;
 	my @difftree;
 	my %diffinfo;
-	my $expires;
-
-	# preparing $fd and %diffinfo for git_patchset_body
-	# new style URI
-	if (defined $hash_base && defined $hash_parent_base) {
-		if (defined $file_name) {
-			# read raw output
-			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-				$hash_parent_base, $hash_base,
-				"--", (defined $file_parent ? $file_parent : ()), $file_name
-				or die_error(undef, "Open git-diff-tree failed");
-			@difftree = map { chomp; $_ } <$fd>;
-			close $fd
-				or die_error(undef, "Reading git-diff-tree failed");
-			@difftree
-				or die_error('404 Not Found', "Blob diff not found");
-
-		} elsif (defined $hash &&
-		         $hash =~ /[0-9a-fA-F]{40}/) {
-			# try to find filename from $hash
-
-			# read filtered raw output
-			open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-				$hash_parent_base, $hash_base, "--"
-				or die_error(undef, "Open git-diff-tree failed");
-			@difftree =
-				# ':100644 100644 03b21826... 3b93d5e7... M	ls-files.c'
-				# $hash == to_id
-				grep { /^:[0-7]{6} [0-7]{6} [0-9a-fA-F]{40} $hash/ }
-				map { chomp; $_ } <$fd>;
-			close $fd
-				or die_error(undef, "Reading git-diff-tree failed");
-			@difftree
-				or die_error('404 Not Found', "Blob diff not found");
+	my $expires = '+1d';
+	my ($from, $to);
 
-		} else {
-			die_error('404 Not Found', "Missing one of the blob diff parameters");
-		}
-
-		if (@difftree > 1) {
-			die_error('404 Not Found', "Ambiguous blob diff specification");
-		}
+	$file_parent ||= $file_name;
 
-		%diffinfo = parse_difftree_raw_line($difftree[0]);
-		$file_parent ||= $diffinfo{'from_file'} || $file_name || $diffinfo{'file'};
-		$file_name   ||= $diffinfo{'to_file'}   || $diffinfo{'file'};
-
-		$hash_parent ||= $diffinfo{'from_id'};
-		$hash        ||= $diffinfo{'to_id'};
-
-		# non-textual hash id's can be cached
-		if ($hash_base =~ m/^[0-9a-fA-F]{40}$/ &&
-		    $hash_parent_base =~ m/^[0-9a-fA-F]{40}$/) {
-			$expires = '+1d';
-		}
+	# non-textual hash id's can be cached
+	if (defined $hash && $hash !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	} elsif (defined $hash_parent && $hash_parent !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	} elsif (defined $hash_base && $hash_base !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	} elsif (defined $hash_parent_base && $hash_parent_base !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	}
+
+	# if hash parameter is missing, read it from the commit.
+	if (defined $hash_base && defined $file_name && !defined $hash) {
+		$hash = git_get_hash_by_path($hash_base, $file_name);
+	}
 
-		# open patch output
-		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-			'-p', ($format eq 'html' ? "--full-index" : ()),
-			$hash_parent_base, $hash_base,
-			"--", (defined $file_parent ? $file_parent : ()), $file_name
-			or die_error(undef, "Open git-diff-tree failed");
+	if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent) {
+	    $hash_parent = git_get_hash_by_path($hash_parent_base, $file_parent);
+	}
+	
+	if (!defined $hash || ! defined $hash_parent) {
+		die_error('404 Not Found', "Missing one of the blob diff parameters");
 	}
 
-	# old/legacy style URI
-	if (!%diffinfo && # if new style URI failed
-	    defined $hash && defined $hash_parent) {
-		# fake git-diff-tree raw output
-		$diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob";
-		$diffinfo{'from_id'} = $hash_parent;
-		$diffinfo{'to_id'}   = $hash;
-		if (defined $file_name) {
-			if (defined $file_parent) {
-				$diffinfo{'status'} = '2';
-				$diffinfo{'from_file'} = $file_parent;
-				$diffinfo{'to_file'}   = $file_name;
-			} else { # assume not renamed
-				$diffinfo{'status'} = '1';
-				$diffinfo{'from_file'} = $file_name;
-				$diffinfo{'to_file'}   = $file_name;
-			}
-		} else { # no filename given
-			$diffinfo{'status'} = '2';
-			$diffinfo{'from_file'} = $hash_parent;
-			$diffinfo{'to_file'}   = $hash;
-		}
+	if (defined $hash_base && defined $file_name) {
+		$to = $hash_base . ':' . $file_name;
+	} else {
+		$to = $hash;
+	}
 
-		# non-textual hash id's can be cached
-		if ($hash =~ m/^[0-9a-fA-F]{40}$/ &&
-		    $hash_parent =~ m/^[0-9a-fA-F]{40}$/) {
-			$expires = '+1d';
-		}
+	if (defined $hash_parent_base && defined $file_parent) {
+		$from = $hash_parent_base . ':' . $file_parent;
+	} else {
+		$from = $hash_parent;
+	}
 
-		# open patch output
-		open $fd, "-|", git_cmd(), "diff", @diff_opts,
-			'-p', ($format eq 'html' ? "--full-index" : ()),
-			$hash_parent, $hash, "--"
-			or die_error(undef, "Open git-diff failed");
-	} else  {
-		die_error('404 Not Found', "Missing one of the blob diff parameters")
-			unless %diffinfo;
+	# fake git-diff-tree raw output
+	$diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob";
+	$diffinfo{'from_id'} = $hash_parent;
+	$diffinfo{'to_id'}   = $hash;
+	if (defined $file_name) {
+		$diffinfo{'status'} = '2';
+		$diffinfo{'from_file'} = $file_parent;
+		$diffinfo{'to_file'}   = $file_name;
+	} else { # no filename given
+		$diffinfo{'status'} = '2';
+		$diffinfo{'from_file'} = $hash_parent;
+		$diffinfo{'to_file'}   = $hash;
 	}
 
+	# open patch output
+	open $fd, "-|", git_cmd(), "diff", @diff_opts, '-p', "--full-index",
+	($format eq 'html' ? "--raw" : ()), $from, $to, "--"
+		or die_error(undef, "Open git-diff failed");
+
 	# header
 	if ($format eq 'html') {
 		my $formats_nav =
@@ -4771,11 +4728,12 @@ sub git_blobdiff {
 		}
 
 	} elsif ($format eq 'plain') {
+		my $patch_file_name = $file_name || $hash;
 		print $cgi->header(
 			-type => 'text/plain',
 			-charset => 'utf-8',
 			-expires => $expires,
-			-content_disposition => 'inline; filename="' . "$file_name" . '.patch"');
+			-content_disposition => 'inline; filename="' . "$patch_file_name" . '.patch"');
 
 		print "X-Git-Url: " . $cgi->self_url() . "\n\n";
 
-- 
1.5.3.rc7.849.g2f5f

[PATCH 2/5] gitweb: support filename prefix in git_patchset_body/git_difftree_body

[Martin Koegler]

git_treediff supports comparing subdirectories. As the output of
git-difftree is missing the path to the compared directories,
the links in the output would be wrong.

The patch adds two new parameters to add the missing path prefix.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 gitweb/gitweb.perl |   88 +++++++++++++++++++++++++++++----------------------
 1 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05bfb26..5f67d73 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1173,10 +1173,10 @@ sub format_diff_from_to_header {
 				        $cgi->a({-href=>href(action=>"blobdiff",
 				                             hash_parent=>$diffinfo->{'from_id'}[$i],
 				                             hash_parent_base=>$parents[$i],
-				                             file_parent=>$from->{'file'}[$i],
+				                             file_parent=>$diffinfo->{'from_prefix'}.$from->{'file'}[$i],
 				                             hash=>$diffinfo->{'to_id'},
 				                             hash_base=>$hash,
-				                             file_name=>$to->{'file'}),
+				                             file_name=>$diffinfo->{'to_prefix'}.$to->{'file'}),
 				                 -class=>"path",
 				                 -title=>"diff" . ($i+1)},
 				                $i+1) .
@@ -1219,7 +1219,7 @@ sub format_diff_cc_simplified {
 		$result .= $cgi->a({-href => href(action=>"blob",
 		                                  hash_base=>$hash,
 		                                  hash=>$diffinfo->{'to_id'},
-		                                  file_name=>$diffinfo->{'to_file'}),
+		                                  file_name=>$diffinfo->{'to_prefix'}.$diffinfo->{'to_file'}),
 		                    -class => "path"},
 		                   esc_path($diffinfo->{'to_file'}));
 	} else {
@@ -2027,7 +2027,7 @@ sub parse_from_to_diffinfo {
 				$from->{'href'}[$i] = href(action=>"blob",
 				                           hash_base=>$parents[$i],
 				                           hash=>$diffinfo->{'from_id'}[$i],
-				                           file_name=>$from->{'file'}[$i]);
+				                           file_name=>$diffinfo->{'from_prefix'}.$from->{'file'}[$i]);
 			} else {
 				$from->{'href'}[$i] = undef;
 			}
@@ -2037,7 +2037,7 @@ sub parse_from_to_diffinfo {
 		if ($diffinfo->{'status'} ne "A") { # not new (added) file
 			$from->{'href'} = href(action=>"blob", hash_base=>$hash_parent,
 			                       hash=>$diffinfo->{'from_id'},
-			                       file_name=>$from->{'file'});
+			                       file_name=>$diffinfo->{'from_prefix'}.$from->{'file'});
 		} else {
 			delete $from->{'href'};
 		}
@@ -2047,7 +2047,7 @@ sub parse_from_to_diffinfo {
 	if (!is_deleted($diffinfo)) { # file exists in result
 		$to->{'href'} = href(action=>"blob", hash_base=>$hash,
 		                     hash=>$diffinfo->{'to_id'},
-		                     file_name=>$to->{'file'});
+		                     file_name=>$diffinfo->{'to_prefix'}.$to->{'file'});
 	} else {
 		delete $to->{'href'};
 	}
@@ -2795,9 +2795,13 @@ sub is_deleted {
 }
 
 sub git_difftree_body {
-	my ($difftree, $hash, @parents) = @_;
+	my ($difftree, $from_prefix, $to_prefix, $hash, @parents) = @_;
 	my ($parent) = $parents[0];
 	my ($have_blame) = gitweb_check_feature('blame');
+
+	$from_prefix = !defined $from_prefix ? '' : $from_prefix.'/';
+	$to_prefix   = !defined $to_prefix   ? '' : $to_prefix . '/';
+
 	print "<div class=\"list_head\">\n";
 	if ($#{$difftree} > 10) {
 		print(($#{$difftree} + 1) . " files changed:\n");
@@ -2854,7 +2858,7 @@ sub git_difftree_body {
 				# file exists in the result (child) commit
 				print "<td>" .
 				      $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
-				                             file_name=>$diff->{'to_file'},
+				                             file_name=>$to_prefix.$diff->{'to_file'},
 				                             hash_base=>$hash),
 				              -class => "list"}, esc_path($diff->{'to_file'})) .
 				      "</td>\n";
@@ -2891,7 +2895,7 @@ sub git_difftree_body {
 					      $cgi->a({-href => href(action=>"blob",
 					                             hash_base=>$hash,
 					                             hash=>$from_hash,
-					                             file_name=>$from_path)},
+					                             file_name=>$from_prefix.$from_path)},
 					              "blob" . ($i+1)) .
 					      " | </td>\n";
 				} else {
@@ -2905,8 +2909,8 @@ sub git_difftree_body {
 					                             hash_parent=>$from_hash,
 					                             hash_base=>$hash,
 					                             hash_parent_base=>$hash_parent,
-					                             file_name=>$diff->{'to_file'},
-					                             file_parent=>$from_path)},
+					                             file_name=>$to_prefix.$diff->{'to_file'},
+					                             file_parent=>$from_prefix.$from_path)},
 					              "diff" . ($i+1)) .
 					      " | </td>\n";
 				}
@@ -2916,14 +2920,14 @@ sub git_difftree_body {
 			if ($not_deleted) {
 				print $cgi->a({-href => href(action=>"blob",
 				                             hash=>$diff->{'to_id'},
-				                             file_name=>$diff->{'to_file'},
+				                             file_name=>$to_prefix.$diff->{'to_file'},
 				                             hash_base=>$hash)},
 				              "blob");
 				print " | " if ($has_history);
 			}
 			if ($has_history) {
 				print $cgi->a({-href => href(action=>"history",
-				                             file_name=>$diff->{'to_file'},
+				                             file_name=>$to_prefix.$diff->{'to_file'},
 				                             hash_base=>$hash)},
 				              "history");
 			}
@@ -2957,7 +2961,7 @@ sub git_difftree_body {
 			$mode_chng   .= "]</span>";
 			print "<td>";
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff->{'file'}),
+			                             hash_base=>$hash, file_name=>$to_prefix.$diff->{'file'}),
 			              -class => "list"}, esc_path($diff->{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chng</td>\n";
@@ -2969,7 +2973,7 @@ sub git_difftree_body {
 				print " | ";
 			}
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff->{'file'})},
+			                             hash_base=>$hash, file_name=>$to_prefix.$diff->{'file'})},
 			              "blob");
 			print "</td>\n";
 
@@ -2977,7 +2981,7 @@ sub git_difftree_body {
 			my $mode_chng = "<span class=\"file_status deleted\">[deleted $from_file_type]</span>";
 			print "<td>";
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
-			                             hash_base=>$parent, file_name=>$diff->{'file'}),
+			                             hash_base=>$parent, file_name=>$from_prefix.$diff->{'file'}),
 			               -class => "list"}, esc_path($diff->{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chng</td>\n";
@@ -2989,15 +2993,15 @@ sub git_difftree_body {
 				print " | ";
 			}
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
-			                             hash_base=>$parent, file_name=>$diff->{'file'})},
+			                             hash_base=>$parent, file_name=>$from_prefix.$diff->{'file'})},
 			              "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
-				                             file_name=>$diff->{'file'})},
+				                             file_name=>$from_prefix.$diff->{'file'})},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$parent,
-			                             file_name=>$diff->{'file'})},
+			                             file_name=>$from_prefix.$diff->{'file'})},
 			              "history");
 			print "</td>\n";
 
@@ -3019,7 +3023,7 @@ sub git_difftree_body {
 			}
 			print "<td>";
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff->{'file'}),
+			                             hash_base=>$hash, file_name=>$to_prefix.$diff->{'file'}),
 			              -class => "list"}, esc_path($diff->{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chnge</td>\n";
@@ -3034,20 +3038,21 @@ sub git_difftree_body {
 				print $cgi->a({-href => href(action=>"blobdiff",
 				                             hash=>$diff->{'to_id'}, hash_parent=>$diff->{'from_id'},
 				                             hash_base=>$hash, hash_parent_base=>$parent,
-				                             file_name=>$diff->{'file'})},
+				                             file_name=>$to_prefix.$diff->{'file'},
+				                             file_parent=>$from_prefix.$diff->{'file'})},
 				              "diff") .
 				      " | ";
 			}
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff->{'file'})},
+			                             hash_base=>$hash, file_name=>$to_prefix.$diff->{'file'})},
 			               "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
-				                             file_name=>$diff->{'file'})},
+				                             file_name=>$to_prefix.$diff->{'file'})},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
-			                             file_name=>$diff->{'file'})},
+			                             file_name=>$to_prefix.$diff->{'file'})},
 			              "history");
 			print "</td>\n";
 
@@ -3061,11 +3066,11 @@ sub git_difftree_body {
 			}
 			print "<td>" .
 			      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-			                             hash=>$diff->{'to_id'}, file_name=>$diff->{'to_file'}),
+			                             hash=>$diff->{'to_id'}, file_name=>$to_prefix.$diff->{'to_file'}),
 			              -class => "list"}, esc_path($diff->{'to_file'})) . "</td>\n" .
 			      "<td><span class=\"file_status $nstatus\">[$nstatus from " .
 			      $cgi->a({-href => href(action=>"blob", hash_base=>$parent,
-			                             hash=>$diff->{'from_id'}, file_name=>$diff->{'from_file'}),
+			                             hash=>$diff->{'from_id'}, file_name=>$from_prefix.$diff->{'from_file'}),
 			              -class => "list"}, esc_path($diff->{'from_file'})) .
 			      " with " . (int $diff->{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
 			      "<td class=\"link\">";
@@ -3079,20 +3084,20 @@ sub git_difftree_body {
 				print $cgi->a({-href => href(action=>"blobdiff",
 				                             hash=>$diff->{'to_id'}, hash_parent=>$diff->{'from_id'},
 				                             hash_base=>$hash, hash_parent_base=>$parent,
-				                             file_name=>$diff->{'to_file'}, file_parent=>$diff->{'from_file'})},
+				                             file_name=>$to_prefix.$diff->{'to_file'}, file_parent=>$from_prefix.$diff->{'from_file'})},
 				              "diff") .
 				      " | ";
 			}
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
-			                             hash_base=>$parent, file_name=>$diff->{'to_file'})},
+			                             hash_base=>$parent, file_name=>$to_prefix.$diff->{'to_file'})},
 			              "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
-				                             file_name=>$diff->{'to_file'})},
+				                             file_name=>$to_prefix.$diff->{'to_file'})},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
-			                            file_name=>$diff->{'to_file'})},
+			                            file_name=>$to_prefix.$diff->{'to_file'})},
 			              "history");
 			print "</td>\n";
 
@@ -3104,7 +3109,7 @@ sub git_difftree_body {
 }
 
 sub git_patchset_body {
-	my ($fd, $difftree, $hash, @hash_parents) = @_;
+	my ($fd, $difftree, $from_prefix, $to_prefix, $hash, @hash_parents) = @_;
 	my ($hash_parent) = $hash_parents[0];
 
 	my $patch_idx = 0;
@@ -3113,6 +3118,9 @@ sub git_patchset_body {
 	my $diffinfo;
 	my (%from, %to);
 
+	$from_prefix = !defined $from_prefix ? '' : $from_prefix.'/';
+	$to_prefix   = !defined $to_prefix   ? '' : $to_prefix . '/';
+
 	print "<div class=\"patchset\">\n";
 
 	# skip to first patch
@@ -3160,6 +3168,8 @@ sub git_patchset_body {
 		    $diffinfo->{'to_id'} eq $to_id) {
 			# this is continuation of a split patch
 			print "<div class=\"patch cont\">\n";
+			$diffinfo->{'from_prefix'} = $from_prefix;
+			$diffinfo->{'to_prefix'} = $to_prefix;
 		} else {
 			# advance raw git-diff output if needed
 			$patch_idx++ if defined $diffinfo;
@@ -3191,6 +3201,8 @@ sub git_patchset_body {
 				}
 			} until (!defined $to_name || $to_name eq $diffinfo->{'to_file'} ||
 			         $patch_idx > $#$difftree);
+			$diffinfo->{'from_prefix'} = $from_prefix;
+			$diffinfo->{'to_prefix'} = $to_prefix;
 			# modifies %from, %to hashes
 			parse_from_to_diffinfo($diffinfo, \%from, \%to, @hash_parents);
 			if ($diffinfo->{'nparents'}) {
@@ -3205,7 +3217,7 @@ sub git_patchset_body {
 						$from{'href'}[$i] = href(action=>"blob",
 						                         hash_base=>$hash_parents[$i],
 						                         hash=>$diffinfo->{'from_id'}[$i],
-						                         file_name=>$from{'file'}[$i]);
+						                         file_name=>$from_prefix.$from{'file'}[$i]);
 					} else {
 						$from{'href'}[$i] = undef;
 					}
@@ -3215,7 +3227,7 @@ sub git_patchset_body {
 				if ($diffinfo->{'status'} ne "A") { # not new (added) file
 					$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
 					                     hash=>$diffinfo->{'from_id'},
-					                     file_name=>$from{'file'});
+					                     file_name=>$from_prefix.$from{'file'});
 				} else {
 					delete $from{'href'};
 				}
@@ -3225,7 +3237,7 @@ sub git_patchset_body {
 			if (!is_deleted($diffinfo)) { # file exists in result
 				$to{'href'} = href(action=>"blob", hash_base=>$hash,
 				                   hash=>$diffinfo->{'to_id'},
-				                   file_name=>$to{'file'});
+				                   file_name=>$to_prefix.$to{'file'});
 			} else {
 				delete $to{'href'};
 			}
@@ -4587,7 +4599,7 @@ sub git_commit {
 	git_print_log($co{'comment'});
 	print "</div>\n";
 
-	git_difftree_body(\@difftree, $hash, @$parents);
+	git_difftree_body(\@difftree, undef, undef, $hash, @$parents);
 
 	git_footer_html();
 }
@@ -4745,7 +4757,7 @@ sub git_blobdiff {
 	if ($format eq 'html') {
 		print "<div class=\"page_body\">\n";
 
-		git_patchset_body($fd, [ \%diffinfo ], $hash_base, $hash_parent_base);
+		git_patchset_body($fd, [ \%diffinfo ], undef, undef, $hash_base, $hash_parent_base);
 		close $fd;
 
 		print "</div>\n"; # class="page_body"
@@ -4928,11 +4940,11 @@ TEXT
 	if ($format eq 'html') {
 		my $use_parents = !defined $hash_parent ||
 			$hash_parent eq '-c' || $hash_parent eq '--cc';
-		git_difftree_body(\@difftree, $hash,
+		git_difftree_body(\@difftree, undef, undef, $hash,
 		                  $use_parents ? @{$co{'parents'}} : $hash_parent);
 		print "<br/>\n";
 
-		git_patchset_body($fd, \@difftree, $hash,
+		git_patchset_body($fd, \@difftree, undef, undef, $hash,
 		                  $use_parents ? @{$co{'parents'}} : $hash_parent);
 		close $fd;
 		print "</div>\n"; # class="page_body"
-- 
1.5.3.rc7.849.g2f5f

[PATCH 3/5] gitweb: Add treediff view

[Martin Koegler]

git_treediff supports comparing different trees. A tree can be specified
either as hash or as base hash and filename.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 gitweb/gitweb.perl |  116 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 116 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5f67d73..4081f51 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -549,6 +549,8 @@ my %actions = (
 	"tag" => \&git_tag,
 	"tags" => \&git_tags,
 	"tree" => \&git_tree,
+	"treediff" => \&git_treediff,
+	"treediff_plain" => \&git_treediff_plain,
 	"snapshot" => \&git_snapshot,
 	"object" => \&git_object,
 	# those below don't need $project
@@ -4962,6 +4964,120 @@ sub git_commitdiff_plain {
 	git_commitdiff('plain');
 }
 
+sub git_treediff {
+	my $format = shift || 'html';
+	my $expires = '+1d';
+
+	# non-textual hash id's can be cached
+	if (defined $hash && $hash !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	} elsif (defined $hash_parent && $hash_parent !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	} elsif (defined $hash_base && $hash_base !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	} elsif (defined $hash_parent_base && $hash_parent_base !~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = undef;
+	}
+
+	# we need to prepare $formats_nav before any parameter munging
+	my $formats_nav;
+	if ($format eq 'html') {
+		$formats_nav =
+			$cgi->a({-href => href(action=>"treediff_plain",
+								   hash=>$hash, hash_parent=>$hash_parent,
+								   hash_base=>$hash_base, hash_parent_base=>$hash_parent_base,
+								   file_name=>$file_name, file_parent=>$file_parent)},
+					"raw");
+	}
+
+	if (!defined $hash) {
+		if (!defined $hash_base) {
+			die_error(undef,'tree parameter missing');
+		}
+		$hash = $hash_base;
+		$hash .= ":".$file_name if (defined $file_name);
+	}
+	
+	if (!defined $hash_parent) {
+		if (!defined $hash_parent_base) {
+			die_error(undef,'tree parameter missing');
+		}
+		$hash_parent = $hash_parent_base;
+		$hash_parent .= ":".$file_parent if (defined $file_parent);
+	}
+
+	# read treediff
+	my $fd;
+	my @difftree;
+	if ($format eq 'html') {
+		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+		"--no-commit-id", "--patch-with-raw", "--full-index",
+		$hash_parent, $hash, "--"
+			or die_error(undef, "Open git-diff-tree failed");
+		
+		while (my $line = <$fd>) {
+			chomp $line;
+			# empty line ends raw part of diff-tree output
+			last unless $line;
+			push @difftree, $line;
+		}
+		
+	} elsif ($format eq 'plain') {
+		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+		'-p', $hash_parent, $hash, "--"
+			or die_error(undef, "Open git-diff-tree failed");
+		
+	} else {
+		die_error(undef, "Unknown treediff format");
+	}
+
+	# write header
+	if ($format eq 'html') {
+		git_header_html(undef, $expires);
+		if (defined $hash_base && (my %co = parse_commit($hash_base))) {
+			git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+			git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
+		} else {
+			print "<div class=\"page_nav\"><br/>$formats_nav<br/></div>\n";
+			print "<div class=\"title\">$hash vs $hash_parent</div>\n";
+		}
+		print "<div class=\"page_body\">\n";
+		
+	} elsif ($format eq 'plain') {
+		my $filename = basename($project) . "-$hash-$hash_parent.patch";
+		
+		print $cgi->header(
+						   -type => 'text/plain',
+						   -charset => 'utf-8',
+						   -expires => $expires,
+						   -content_disposition => 'inline; filename="' . "$filename" . '"');
+		
+		print "X-Git-Url: " . $cgi->self_url() . "\n\n";
+		print "---\n\n";
+	}
+	
+	# write patch
+	if ($format eq 'html') {
+		git_difftree_body(\@difftree, $file_parent, $file_name, $hash_base, $hash_parent_base);
+		print "<br/>\n";
+
+		git_patchset_body($fd, \@difftree, $file_parent, $file_name, $hash_base, $hash_parent_base);
+		close $fd;
+		print "</div>\n"; # class="page_body"
+		git_footer_html();
+
+	} elsif ($format eq 'plain') {
+		local $/ = undef;
+		print <$fd>;
+		close $fd
+			or print "Reading git-diff-tree failed\n";
+	}
+}
+
+sub git_treediff_plain {
+	git_treediff('plain');
+}
+
 sub git_history {
 	if (!defined $hash_base) {
 		$hash_base = git_get_head_hash($project);
-- 
1.5.3.rc7.849.g2f5f

[PATCH 4/5] gitweb: Selecting diffs in JavaScript

[Martin Koegler]

The adds support for selecting arbitrary diffs, if the client browser supports
JavaScript.

It is no possible to hide/show all diff links.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
 Makefile           |    6 +-
 git-instaweb.sh    |    7 +
 gitweb/gitweb.js   |  360 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.perl |   11 ++
 4 files changed, 383 insertions(+), 1 deletions(-)
 create mode 100644 gitweb/gitweb.js

diff --git a/Makefile b/Makefile
index 2decdfb..1582dfa 100644
--- a/Makefile
+++ b/Makefile
@@ -168,6 +168,7 @@ GITWEB_HOMETEXT = indextext.html
 GITWEB_CSS = gitweb.css
 GITWEB_LOGO = git-logo.png
 GITWEB_FAVICON = git-favicon.png
+GITWEB_JS = gitweb.js
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 
@@ -817,13 +818,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_JS++|$(GITWEB_JS)|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/gitweb.js
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -832,6 +834,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_JS@@/r gitweb/gitweb.js' \
+	    -e '/@@GITWEB_JS@@/d' \
 	    $@.sh > $@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@
diff --git a/git-instaweb.sh b/git-instaweb.sh
index b79c6b6..960486e 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -227,8 +227,15 @@ gitweb_css () {
 EOFGITWEB
 }
 
+gitweb_js () {
+	cat > "$1" <<\EOFGITWEB
+@@GITWEB_JS@@
+EOFGITWEB
+}
+
 gitweb_cgi $GIT_DIR/gitweb/gitweb.cgi
 gitweb_css $GIT_DIR/gitweb/gitweb.css
+gitweb_js $GIT_DIR/gitweb/gitweb.js
 
 case "$httpd" in
 *lighttpd*)
diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
new file mode 100644
index 0000000..fd4178a
--- /dev/null
+++ b/gitweb/gitweb.js
@@ -0,0 +1,360 @@
+/* Javascript functions for gitweb
+
+  (C) 2007 Martin Koegler <mkoegler@auto.tuwien.ac.at>
+
+  This file is licensed under the GPL v2, or (at your option) any later version.
+*/
+
+function getCookie (name)
+{
+  var name = name + "=";
+  var c = document.cookie;
+  var p = c.indexOf (name);
+  if (p == -1)
+    return null;
+  c = c.substr (p + name.length, c.length);
+  p = c.indexOf (";");
+  if (p == -1)
+    return c;
+  else
+    return c.substr (0, p);
+}
+
+function insertAfter (elem, node)
+{
+  if (node.nextSibling)
+    node.parentNode.insertBefore (elem, node.nextSibling);
+  else
+    node.parentNode.appendChild (elem);
+}
+
+function createLink (href, linktext)
+{
+  var l = document.createElement ("a");
+  l.appendChild (document.createTextNode (linktext));
+  l.href = href;
+  return l;
+}
+
+function createLinkGroup (href1, basetxt, href2, difftxt)
+{
+  var l = document.createElement ("span");
+  l.className = 'difflinks';
+  l.appendChild (document.createTextNode (" ("));
+  l.appendChild (createLink (href1, basetxt));
+  l.appendChild (document.createTextNode (" | "));
+  l.appendChild (createLink (href2, difftxt));
+  l.appendChild (document.createTextNode (") "));
+  return l;
+}
+
+function GitRef ()
+{
+  this.t = null;
+  this.h = null;
+  this.hb = null;
+  this.f = null;
+  this.p = null;
+  this.ToRef = ToRef;
+}
+
+function ToRef ()
+{
+  var parts = new Array ();
+  if (this.f)
+    parts.push ("f=" + this.f);
+  if (this.h)
+    parts.push ("h=" + this.h);
+  if (this.hb)
+    parts.push ("hb=" + this.hb);
+  if (this.t)
+    parts.push ("t=" + this.t);
+  if (this.p)
+    parts.push ("p=" + this.p);
+  return parts.join ("@");
+}
+
+function splitGitRef (ref)
+{
+  var parts = ref.split ("@");
+  var res = new GitRef ();
+  var i;
+  for (i = 0; i < parts.length; i++)
+    {
+      var p = parts[i].split ("=");
+      res[p[0]] = p[1];
+    }
+  return res;
+}
+
+function GitURL (base)
+{
+  this.base = base;
+  this.p = null;
+  this.a = null;
+  this.f = null;
+  this.fp = null;
+  this.h = null;
+  this.hp = null;
+  this.hb = null;
+  this.hpb = null;
+  this.pg = null;
+  this.o = null;
+  this.s = null;
+  this.st = null;
+  this.ToURL = ToURL;
+  this.ToRef = UrlToRef;
+  this.ToDUrl = ToDUrl;
+}
+
+function ToURL ()
+{
+  var parts = new Array ();
+  if (this.p)
+    parts.push ("p=" + this.p);
+  if (this.a)
+    parts.push ("a=" + this.a);
+  if (this.f)
+    parts.push ("f=" + this.f);
+  if (this.fp)
+    parts.push ("fp=" + this.fp);
+  if (this.h)
+    parts.push ("h=" + this.h);
+  if (this.hp)
+    parts.push ("hp=" + this.hp);
+  if (this.hb)
+    parts.push ("hb=" + this.hb);
+  if (this.hpb)
+    parts.push ("hpb=" + this.hpb);
+  if (this.o)
+    parts.push ("o=" + this.o);
+  if (this.s)
+    parts.push ("s=" + this.s);
+  if (this.st)
+    parts.push ("st=" + this.st);
+  return this.base + "?" + parts.join (";");
+}
+
+function UrlToRef (type)
+{
+  var res = new GitRef;
+  res.f = this.f;
+  res.h = this.h;
+  res.hb = this.hb;
+  res.t = type;
+  res.p = this.p;
+  return res.ToRef ();
+}
+
+function ToDUrl (type)
+{
+  var res = new GitURL (this.base);
+  res.f = this.f;
+  res.h = this.h;
+  res.hb = this.hb;
+  res.p = this.p;
+  res.a = type;
+  return res.ToURL ();
+}
+
+function splitGitURL (url)
+{
+  var Urls = url.split ("?");
+  var res = new GitURL (Urls[0]);
+  if (Urls.length > 1)
+    {
+      var parts = Urls[1].split (";");
+      var i;
+      for (i = 0; i < parts.length; i++)
+	{
+	  var p = parts[i].split ("=");
+	  res[p[0]] = p[1];
+	}
+    }
+  return res;
+}
+
+function base (ref)
+{
+  document.cookie = "basename=" + ref;
+}
+
+function diff (url)
+{
+  var c = getCookie ("basename");
+  if (!c)
+    {
+      alert ("no diff base selected");
+      return;
+    }
+  c = splitGitRef (c);
+  url = splitGitURL (url);
+
+  if (c.p != url.p)
+    {
+      alert ("base object in an other repository");
+      return;
+    }
+
+  if (c.t == 'commit' && url.a == 'commit')
+    {
+      url.a = 'commitdiff';
+      if (!c.h || !url.h)
+	{
+	  alert ("commit diff not possible");
+	  return;
+	}
+      url.hb = null;
+      url.f = null;
+      url.hp = c.h;
+      document.location.href = url.ToURL ();
+      return;
+    }
+  if (c.t == 'blob' && url.a == 'blob')
+    {
+      url.a = 'blobdiff';
+      url.hp = c.h;
+      url.hpb = c.hb;
+      url.fp = c.f;
+      document.location.href = url.ToURL ();
+      return;
+    }
+  if (c.t == 'tree' && url.a == 'tree')
+    {
+      url.a = 'treediff';
+      url.hpb = c.hb;
+      url.hp = c.h;
+      url.fp = c.f;
+      document.location.href = url.ToURL ();
+      return;
+    }
+  if (c.t == 'commit' && url.a == 'tree')
+    {
+      url.a = 'treediff';
+      url.hpb = c.h;
+      url.hp = null;
+      url.fp = null;
+      document.location.href = url.ToURL ();
+      return;
+    }
+  if (c.t == 'tree' && url.a == 'commit')
+    {
+      url.a = 'treediff';
+      url.hpb = c.hb;
+      url.hp = c.h;
+      url.fp = c.f;
+      url.hb = url.h;
+      url.h = null;
+      document.location.href = url.ToURL ();
+      return;
+    }
+  alert ("diff not possible");
+}
+
+function GitAddLinks ()
+{
+  var links = document.getElementsByTagName ("a");
+  var i;
+
+  for (i = 0; i < links.length; i++)
+    {
+      var link = links[i];
+      var url = splitGitURL (link.href);
+      if (link.innerHTML == 'commit' || link.innerHTML == 'tag')
+	{
+	  if (!url.h)
+	    continue;
+	  var l =
+	    createLinkGroup ("javascript:base('" + url.ToRef ('commit') +
+			     "')", "base",
+			     "javascript:diff('" + url.ToDUrl ('commit') +
+			     "')", "diff");
+	  insertAfter (l, link);
+	}
+      if (link.innerHTML == 'blob')
+	{
+	  if (!url.h && !(url.hb && url.f))
+	    continue;
+	  var l =
+	    createLinkGroup ("javascript:base('" + url.ToRef ('blob') + "')",
+			     "base",
+			     "javascript:diff('" + url.ToDUrl ('blob') + "')",
+			     "diff");
+	  insertAfter (l, link);
+	}
+      if (link.innerHTML == 'tree')
+	{
+	  if (!url.h && !(url.hb && url.f))
+	    continue;
+	  var l =
+	    createLinkGroup ("javascript:base('" + url.ToRef ('tree') + "')",
+			     "base",
+			     "javascript:diff('" + url.ToDUrl ('tree') + "')",
+			     "diff");
+	  insertAfter (l, link);
+	}
+    }
+}
+
+function ShowHideLinks (action)
+{
+  var tags = document.getElementsByTagName ("span");
+
+  for (i = 0; i < tags.length; i++)
+    {
+      var tag = tags[i];
+      if (tag.className == 'difflinks')
+	{
+	  if (action)
+	    tag.style.display = 'inline';
+	  else
+	    tag.style.display = 'none';
+	}
+    }
+
+}
+
+var LinkState=-1;
+var saveonload;
+
+function GitShowLinks ()
+{
+  if (LinkState == -1)
+    {
+      GitAddLinks();
+      LinkState = 1;
+      document.cookie = "showdiff=1";
+      document.getElementById ('difflinks').innerHTML = 'hide difflinks';
+    }	
+  else if (LinkState == 1)
+    {
+      ShowHideLinks (false);
+      LinkState = 0;
+      document.cookie = "showdiff=0";
+      document.getElementById ('difflinks').innerHTML = 'show difflinks';
+    }
+  else
+    {
+      ShowHideLinks (true);
+      LinkState = 1;
+      document.cookie = "showdiff=1";
+      document.getElementById ('difflinks').innerHTML = 'hide difflinks';
+    }
+}
+
+function GitAddLinkElement ()
+{
+    document.getElementById ('difflinkdiv').innerHTML = 
+      ' | <a id="difflinks" href="javascript:GitShowLinks()">show difflinks</a>';
+    var show = getCookie ('showdiff');
+    if (show == '1')
+      {
+	saveonload = window.onload;
+	window.onload = function ()
+	  {
+	    if (saveonload)
+	       saveonload();
+	    GitShowLinks();
+	  }
+      }
+}
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4081f51..2531133 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -61,6 +61,8 @@ our $stylesheet = undef;
 our $logo = "++GITWEB_LOGO++";
 # URI of GIT favicon, assumed to be image/png type
 our $favicon = "++GITWEB_FAVICON++";
+# URI of gitweb.js
+our $gitwebjs = "++GITWEB_JS++";
 
 # URI and label (title) of GIT logo link
 #our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
@@ -2313,6 +2315,10 @@ EOF
 		print qq(<link rel="shortcut icon" href="$favicon" type="image/png"/>\n);
 	}
 
+	if (defined $gitwebjs) {
+		print qq(<script src="$gitwebjs" type="text/javascript"></script>\n);
+	}
+
 	print "</head>\n" .
 	      "<body>\n";
 
@@ -2449,6 +2455,11 @@ sub git_print_page_nav {
 		 map { $_ eq $current ?
 		       $_ : $cgi->a({-href => href(%{$arg{$_}})}, "$_")
 		 } @navs);
+
+	if (defined $gitwebjs) {
+		print '<span id="difflinkdiv"></span><script type="text/javascript">GitAddLinkElement();</script>';
+	}
+
 	print "<br/>\n$extra<br/>\n" .
 	      "</div>\n";
 }
-- 
1.5.3.rc7.849.g2f5f

[PATCH 5/5] Show Difflinks in an other color

[Martin Koegler]

Suggested by Petr Baudis

The style for the links was randomly selected.

No sign-off, as somebody else should select a suiteable style.
---
 gitweb/gitweb.css |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 1b88879..ac15c0e 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -499,3 +499,7 @@ span.match {
 div.binary {
 	font-style: italic;
 }
+
+span.difflinks a {
+	color: #ff0000;
+}
-- 
1.5.3.rc7.849.g2f5f

Re: [PATCH 0/5] gitweb: Support for arbitrary diffs

[Jakub Narebski]

[Cc: Martin Koegler <mkoegler@auto.tuwien.ac.at>,
     Petr Baudis <pasky@suse.cz>, git@vger.kernel.org]

Martin Koegler wrote:

> Patch 4 now only generates the links, if they are enabled.
> The header contains a new link to show/hide the links.
> The state is remembered via a cookie across pages.

I'd rather have patch adding cookie management (are they only JavaScript,
or also server side?) as a separate patch.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 0/5] gitweb: Support for arbitrary diffs

[Petr Baudis]

On Mon, Sep 03, 2007 at 02:19:05AM CEST, Jakub Narebski wrote:
> [Cc: Martin Koegler <mkoegler@auto.tuwien.ac.at>,
>      Petr Baudis <pasky@suse.cz>, git@vger.kernel.org]
> 
> Martin Koegler wrote:
> 
> > Patch 4 now only generates the links, if they are enabled.
> > The header contains a new link to show/hide the links.
> > The state is remembered via a cookie across pages.
> 
> I'd rather have patch adding cookie management (are they only JavaScript,
> or also server side?) as a separate patch.

This is purely in-Javascript thing, so I don't think a separate patch
makes much sense.

To hijack this post a bit, another patch in the queue (the incremental
blame thingie) introduces blame.js. Do you think that we should keep the
.js files separate, or instead have one big gitweb.js with everything?
I'm inclined to the second possibility in order to reduce the number of
requests, but it comes at a price of slightly worse maintainability.

-- 
				Petr "Pasky" Baudis
Early to rise and early to bed makes a male healthy and wealthy and dead.
                -- James Thurber

Re: [PATCH 0/5] gitweb: Support for arbitrary diffs

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> To hijack this post a bit, another patch in the queue (the incremental
> blame thingie) introduces blame.js. Do you think that we should keep the
> .js files separate, or instead have one big gitweb.js with everything?
> I'm inclined to the second possibility in order to reduce the number of
> requests, but it comes at a price of slightly worse maintainability.

Is it a possibility to concatenate them at the installation
time, if a single huge file becomes a problem?

Re: [PATCH 0/5] gitweb: Support for arbitrary diffs

[Jakub Narebski]

On Mon, 3 September 2007, Petr "Pasky" Baudis wrote:

> To hijack this post a bit, another patch in the queue (the incremental
> blame thingie) introduces blame.js. Do you think that we should keep the
> .js files separate, or instead have one big gitweb.js with everything?
> I'm inclined to the second possibility in order to reduce the number of
> requests, but it comes at a price of slightly worse maintainability.

On the other hand if we have blame.js separate, we could load it
(require it) only for the 'blame' view, it means only when needed.

gitweb.js would contain JavaScript code used by all (or almost all)
views, then...

I don't think gitweb.js would be as large as gitweb.perl, if we are
talking about maintability ;-)

-- 
Jakub Narebski
Poland

Re: [PATCH 0/5] gitweb: Support for arbitrary diffs

[Martin Koegler]

On Mon, Sep 03, 2007 at 10:23:41AM +0200, Jakub Narebski wrote:
> On Mon, 3 September 2007, Petr "Pasky" Baudis wrote:
> 
> > To hijack this post a bit, another patch in the queue (the incremental
> > blame thingie) introduces blame.js. Do you think that we should keep the
> > .js files separate, or instead have one big gitweb.js with everything?
> > I'm inclined to the second possibility in order to reduce the number of
> > requests, but it comes at a price of slightly worse maintainability.

Why is the maintainablility reduced?

gitweb.perl is also a collection of different function, which are kept
in one file. Where is the difference to a javascript file?

Keeping everything into one file will make it more likely, that the
code is not developed totally different (in the terms of code style,
variable/function names, ...).

In the moment, there is the problem, which patch should introduce the
js file. We need a common base patch, which introduces an empty
gitweb.js.

Keeping the two functions separate has problems. Every patch need to
hook into the onload event. To avoid merge conflicts, my current patch
saves the old handler and overwrite it with its on version, which
calls back into the saved handler.

If we would have on js file, we can add in the base patch an empty
JavaScript function for this.

Then Hooking in the onload event would mean, to only add some new
lines to the function. The resulting merge conflicts are
easier to resolve (as they affect independet lines) compared to
the current '<body onload="hook1(); hook2();">'

> On the other hand if we have blame.js separate, we could load it
> (require it) only for the 'blame' view, it means only when needed.
> 
> gitweb.js would contain JavaScript code used by all (or almost all)
> views, then...
> 
> I don't think gitweb.js would be as large as gitweb.perl, if we are
> talking about maintability ;-)

The size of gitweb.js should not matter. On a modern browser, the
first request will fetch the whole Javascript file. For subseqent
request, the webserver returns "not modified". Having two javascript
file means two checks.

mfg Martin Kögler

Re: [PATCH 0/5] gitweb: Support for arbitrary diffs

[Catalin Marinas]

Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:
> I develop the patches via StGit. If you think that I should publish them
> in a different way, please tell me.

You could use 'stg mail' to send the patches and it makes each patch a
direct child of the main series description (PATCH 0/5) rather than a
child of the previous patch. It looks nicer with a threaded view.

-- 
Catalin