Re: [RFC] More diff possibilities in gitweb

[mkoegler@auto.tuwien.ac.at (Martin Koegler)]

On Sat, Mar 17, 2007 at 07:04:54PM +0100, Johannes Schindelin wrote:
> On Sat, 17 Mar 2007, Martin Koegler wrote:
> 
> > The whole is implemented in JavaScript on the client side (tested with 
> > IE 6 and Mozilla).
> 
> This is not acceptable for me. There are too many people blindly running 
> JavaScript everywhere, and we should not enforce such bad behaviour.
>
> At least, you need a way to do it with plain old HTML (which is easy 
> enough).

If the browser supports no JavaScript, the user can use all current
features, except the new diff capabilities.

My first thoughts where also about implementing it inside
gitweb.cgi. This would mean a large change to the code, as all code
which generate links needs to be adapted.

Additionally, selecting a base object would mean, that you must submit
a request to the server and probably return something (probable the
same page, on which you selected the base object).

For me, this would be no problem, but for high load servers (eg. gitweb at
kernel.org), it would incread the system load.

With JavaScript, this step needs no server interaction. Infact, a
client could add the diff feature to any gitweb server with
eg. greasemonkey, by injecting the JavaScript code.

> Also I'd like to know: is there any reason you did not send a proper diff, 
> given that you are so interested in diffs?

I'm not interessted in generating patchs via gitweb. I want to examine
and review differences between various branches/tags/commits/... .

The "pseudo" diff for gitweb.cgi was to illustrate, how to integrate
gitweb.js. The whole thing is under development, so I have not created
a clean version.

As I wrote in my last mail, I used blobdiff to generation tree diffs,
which results in wrong/missing file names. In the mean time, I've
create a first version of a treediff function for gitweb.

mfg Martin Kögler

--- old/gitweb.js   2007-03-17 15:18:23.284317140 +0100
+++ gitweb.js        2007-03-17 22:25:18.254190096 +0100
@@ -259,7 +259,7 @@
   }
   if(c.t=='tree'&&url.a=='tree')
   {
-      url.a='blobdiff';
+      url.a='treediff';
       if(c.h&&url.h)
       {
          url.hb=null;
--- ../mirror/git.git/gitweb/gitweb.perl	2007-03-12 22:06:44.000000000 +0100
+++ gitweb.cgi	2007-03-17 18:41:50.000000000 +0100
@@ -446,6 +446,8 @@
 	"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
@@ -1835,6 +1837,7 @@
 		close $fd;
 	}
 
+	print '<script type="text/javascript" src="gitweb.js"></script>';
 	print "</body>\n" .
 	      "</html>";
 }
@@ -4191,6 +4194,106 @@
 	git_commitdiff('plain');
 }
 
+sub git_treediff {
+	my $format = shift || 'html';
+	$hash ||= $hash_base;
+	$hash_parent ||= $hash_parent_base;
+	if (!defined $hash_parent || !defined $hash)
+	{
+	    die_error(undef, "Missing one of the tree diff parameters");
+	}
+
+	# 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)},
+			        "raw");
+		$formats_nav .= ' | ';
+		$formats_nav .=
+			$cgi->a({-href => href(action=>"tree",
+			                       hash=>$hash)},
+			        "tree");
+		$formats_nav .= ' &lt;-&gt; ';
+		$formats_nav .=
+			$cgi->a({-href => href(action=>"tree",
+			                       hash=>$hash)},
+			        "tree");
+	}
+
+	# 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");
+	}
+
+	# non-textual hash id's can be cached
+	my $expires;
+	if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
+		$expires = "+1d";
+	}
+
+	# write diff message
+	if ($format eq 'html') {
+		git_header_html(undef, $expires);
+
+		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";
+	}
+
+	# write patch
+	if ($format eq 'html') {
+		git_difftree_body(\@difftree, $hash, $hash_parent);
+		print "<br/>\n";
+
+		git_patchset_body($fd, \@difftree, $hash, $hash_parent);
+		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);