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 .= ' <-> '; + $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); - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html