[PATCH 1/7] gitweb: show "no difference" message for empty diff

[PATCH 1/7] gitweb: show "no difference" message for empty diff

[Martin Koegler]

Currently, gitweb shows only header and footer, if no differences are
found. This patch adds a "No differences found" message for the html
output.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---
$patch_idx does not work, so I need a new variable.

The css style for the no difference message is useable. If somebody
does not like, feel free to send a patch with a better layout.

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

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 5e40292..fb58bec 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -387,6 +387,13 @@ div.diff.incomplete {
 	color: #cccccc;
 }
 
+div.diff.nodifferences {
+	font-weight: bold;
+	background-color: #edece6;
+
+	padding: 4px;
+	border: 1px solid #d9d8d1;
+}
 
 div.index_include {
 	border: solid #d9d8d1;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c48b35a..cbd8d03 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2398,6 +2398,7 @@ sub git_patchset_body {
 	my ($fd, $difftree, $hash, $hash_parent) = @_;
 
 	my $patch_idx = 0;
+	my $patch_number = 0;
 	my $patch_line;
 	my $diffinfo;
 	my (%from, %to);
@@ -2419,6 +2420,7 @@ sub git_patchset_body {
 		# git diff header
 		#assert($patch_line =~ m/^diff /) if DEBUG;
 		#assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed
+		$patch_number++;
 		push @diff_header, $patch_line;
 
 		# extended diff header
@@ -2581,6 +2583,7 @@ sub git_patchset_body {
 	} continue {
 		print "</div>\n"; # class="patch"
 	}
+	print "<div class=\"diff nodifferences\">No differences found</div>\n" if (!$patch_number);
 
 	print "</div>\n"; # class="patchset"
 }
-- 
1.5.1.1.85.gf1888

Re: [PATCH 1/7] gitweb: show "no difference" message for empty diff

[Jakub Narebski]

On Sun, Apr 15, 2007, Martin Koegler wrote:
> Currently, gitweb shows only header and footer, if no differences are
> found. This patch adds a "No differences found" message for the html
> output.
> 
> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> ---
> $patch_idx does not work, so I need a new variable.

Nice work.

> The css style for the no difference message is useable. If somebody
> does not like, feel free to send a patch with a better layout.

I like it, although I'd rather have "quieter" CSS style for the
"No differences found" message. But it is I think a matter of taste.

Below your patch with my changes.

-- >8 --
From: Martin Koegler <mkoegler@auto.tuwien.ac.at>
Subject: [PATCH 1/7] gitweb: Show "no difference" message for empty diff

Currently, gitweb shows only header and footer, if no differences are
found. This patch adds a "No differences found" message for the html
output.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.css  |    4 ++++
 gitweb/gitweb.perl |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 5e40292..2b023bd 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -387,6 +387,10 @@ div.diff.incomplete {
 	color: #cccccc;
 }
 
+div.diff.nodifferences {
+	font-weight: bold;
+	color: #600000;
+}
 
 div.index_include {
 	border: solid #d9d8d1;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c48b35a..cbd8d03 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2398,6 +2398,7 @@ sub git_patchset_body {
 	my ($fd, $difftree, $hash, $hash_parent) = @_;
 
 	my $patch_idx = 0;
+	my $patch_number = 0;
 	my $patch_line;
 	my $diffinfo;
 	my (%from, %to);
@@ -2419,6 +2420,7 @@ sub git_patchset_body {
 		# git diff header
 		#assert($patch_line =~ m/^diff /) if DEBUG;
 		#assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed
+		$patch_number++;
 		push @diff_header, $patch_line;
 
 		# extended diff header
@@ -2581,6 +2583,7 @@ sub git_patchset_body {
 	} continue {
 		print "</div>\n"; # class="patch"
 	}
+	print "<div class=\"diff nodifferences\">No differences found</div>\n" if (!$patch_number);
 
 	print "</div>\n"; # class="patchset"
 }
-- 
1.5.1

[PATCH 3/7] 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>
---

I renamed the parameters to to/from_prefix.

Modfifing the file name in $diff would require bigger changes and cause new problems:
* new output rewriter for plain view needed
* for modfied files only one file name, but possibly two prefixes

 gitweb/gitweb.perl |   51 ++++++++++++++++++++++++++++++---------------------
 1 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 790041c..1551d95 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2205,8 +2205,12 @@ sub git_print_tree_entry {
 ## functions printing large fragments of HTML
 
 sub git_difftree_body {
-	my ($difftree, $hash, $parent) = @_;
+	my ($difftree, $hash, $parent, $from_prefix, $to_prefix) = @_;
 	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");
@@ -2249,7 +2253,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";
@@ -2261,7 +2265,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";
 
@@ -2269,7 +2273,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";
@@ -2281,15 +2285,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";
 
@@ -2311,7 +2315,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";
@@ -2326,20 +2330,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";
 
@@ -2353,11 +2358,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\">";
@@ -2371,20 +2376,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=>$hash, 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";
 
@@ -2395,7 +2400,7 @@ sub git_difftree_body {
 }
 
 sub git_patchset_body {
-	my ($fd, $difftree, $hash, $hash_parent) = @_;
+	my ($fd, $difftree, $hash, $hash_parent, $from_prefix, $to_prefix) = @_;
 
 	my $patch_idx = 0;
 	my $patch_number = 0;
@@ -2403,6 +2408,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
@@ -2459,17 +2467,18 @@ sub git_patchset_body {
 			}
 			$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
 			$to{'file'}   = $diffinfo->{'to_file'}   || $diffinfo->{'file'};
+
 			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'};
 			}
 			if ($diffinfo->{'status'} ne "D") { # not deleted file
 				$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'};
 			}
-- 
1.5.1.1.85.gf1888

Re: [PATCH 3/7] gitweb: support filename prefix in git_patchset_body/git_difftree_body

[Jakub Narebski]

On Sunday, 15 April 2007, Martin Koegler wrote:
> 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.

You mean that git-diff-tree output for diff between two non-toplevel
trees lacks the path to compared tree, e.g.
  $ git diff-tree <commit1>:<path1> <commit2>:<path2>
lacks <path1> prefix in the from part, and <path2> prefix in to part,
and the goal is to add those missing prefixes in the links, without
changing the visible output of git_patchset_body/git_difftree_body.
 
> The patch adds two new parameters to add the missing path prefix.

...and makes use of them while generating links.

Although I'm not sure if it wouldn't be better to put e.g.
$prefix_from.$diff{'file'} in $diff{'file_from_url'} or something
like that.

> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> ---
> I renamed the parameters to to/from_prefix.
> 
> Modifying the file name in $diff would require bigger changes
> and cause new problems: 
> * new output rewriter for plain view needed
> * for modified files only one file name, but possibly two prefixes

Looks nice, although I think this patch is needed only when we have
"treediff" (and "treediff_plain") views, i.e. when we are able to
compare not only top level trees (like in "commitdiff" view),
or individual files (in "blobdiff" view), but also arbitrary trees.

It can be usefull to compare for example git-gui/ directory in git.git
mainline branch with the root directory from git://repo.or.cz/git-gui
or gitweb/ directory after including gitweb in git repository with the
gitweb.git commits. Although not that useful in first example: there is
as far as I know no development of git-gui directly in git.git
repository so the only difference is that in mainline git-gui stuff
resides in git-gui/ and in git://repo.or.cz/git-gui in top directory

-- 
Jakub Narebski
ShadeHawk on #git
Poland

Re: [PATCH 3/7] gitweb: support filename prefix in git_patchset_body/git_difftree_body

[Martin Koegler]

On Fri, Apr 27, 2007 at 12:55:22PM +0200, Jakub Narebski wrote:
> On Sunday, 15 April 2007, Martin Koegler wrote:
> > 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.
> 
> You mean that git-diff-tree output for diff between two non-toplevel
> trees lacks the path to compared tree, e.g.
>   $ git diff-tree <commit1>:<path1> <commit2>:<path2>
> lacks <path1> prefix in the from part, and <path2> prefix in to part,
> and the goal is to add those missing prefixes in the links, without
> changing the visible output of git_patchset_body/git_difftree_body.

Yes, as I don't want to rewrite the raw view.

> > The patch adds two new parameters to add the missing path prefix.
> 
> ...and makes use of them while generating links.
> 
> Although I'm not sure if it wouldn't be better to put e.g.
> $prefix_from.$diff{'file'} in $diff{'file_from_url'} or something
> like that.

Would this really simplify the code? 

Figuring out, what is stored in $diff and how its elements are used,
requries more time. Adding a prefix is easier to understand.

> Looks nice, although I think this patch is needed only when we have
> "treediff" (and "treediff_plain") views, i.e. when we are able to
> compare not only top level trees (like in "commitdiff" view),
> or individual files (in "blobdiff" view), but also arbitrary trees.

My git_treediff patch supports this. My JavaScript prototyp can be used to
select such a comparison.

mfg Martin Kögler

[PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript

[Martin Koegler]

---
This patch is only to test the other patches. I'm working on reimplementing it in perl.

 gitweb/gitweb.js   |  273 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gitweb/gitweb.perl |    1 +
 2 files changed, 274 insertions(+), 0 deletions(-)
 create mode 100644 gitweb/gitweb.js

diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
new file mode 100644
index 0000000..2e1a94d
--- /dev/null
+++ b/gitweb/gitweb.js
@@ -0,0 +1,273 @@
+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.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.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);
+  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;
+    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 GitAddLinks()
+{
+  var links=document.getElementsByTagName("a");
+  var i;
+  for(i=0;i<links.length;i++)
+    {
+      var link=links[i];
+      if(link.innerHTML=='commit'||link.innerHTML=='tag')
+	{
+	  var url=splitGitURL(link.href);
+	  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')
+	{
+	  var url=splitGitURL(link.href);
+	  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')
+	{
+	  var url=splitGitURL(link.href);
+	  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 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.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");
+}
+
+GitAddLinks();
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c2dc82d..769e755 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1860,6 +1860,7 @@ sub git_footer_html {
 		close $fd;
 	}
 
+	print '<script type="text/javascript" src="gitweb.js"></script>';
 	print "</body>\n" .
 	      "</html>";
 }
-- 
1.5.1.1.85.gf1888

Re: [PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript

[Petr Baudis]

  Hi,

On Sun, Apr 15, 2007 at 10:46:08PM CEST, Martin Koegler wrote:
> ---
> This patch is only to test the other patches. I'm working on reimplementing it in perl.

  I think this is something that I'd quite like to testdrive at
repo.or.cz, but I'm admittelly somewhat lost in all the patches,
resends, new versions, something got applied, etc. - if it's not too big
a hassle for you, would you please care to resend the latest series?

  Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

Re: [PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript

[Martin Koegler]

On Fri, May 18, 2007 at 10:49:35AM +0200, Petr Baudis wrote:
> On Sun, Apr 15, 2007 at 10:46:08PM CEST, Martin Koegler wrote:
> > ---
> > This patch is only to test the other patches. I'm working on reimplementing it in perl.
> 
>   I think this is something that I'd quite like to testdrive at
> repo.or.cz, but I'm admittelly somewhat lost in all the patches,
> resends, new versions, something got applied, etc. - if it's not too big
> a hassle for you, would you please care to resend the latest series?

Againgst what version/repository do you want the patch?

For the new version, I intend to move the JavaScript file into the
head tag of the html page. I could integrate your incremental blame
link rewrite as well. If I'm doing this, we could remove the blamelink
class hack as well as the fixBlameLinks JavaScript function.

mfg Martin Kögler

Re: [PATCH 5/7] gitweb: Prototyp for selecting diffs in JavaScript

[Petr Baudis]

On Sat, May 19, 2007 at 09:57:01AM CEST, Martin Koegler wrote:
> Againgst what version/repository do you want the patch?

Whatever is comfortable for you :-) - next would be ideal for me, but
can be something else as well.

> For the new version, I intend to move the JavaScript file into the
> head tag of the html page. I could integrate your incremental blame
> link rewrite as well. If I'm doing this, we could remove the blamelink
> class hack as well as the fixBlameLinks JavaScript function.

It's not clear at all yet whether incremental blame is viable idea
because I suspect Firefox might be just too horribly slow at working
with DOM for it to make sense for any files of non-trivial size. So I
wouldn't rely on this going in just yet. :-(

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

[PATCH 6/7] gitweb: pass root directory as empty file parameter

[Martin Koegler]

This patch add the automatic propagation of the file name from f to fp
for git_treeview, if fp is undefined. To distinguish it from the root
directory, allow an empty string for this in f/fp.

Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
---

It was requested, that git_treediff should propagate f to fp, if the fp
parameter is missing. So I created this patch.

As any other path could be a valid blob, I use the empty string for
the root directory.

As I personally don't like this, I have split this change into a
seperate patch:

* As git_treediff is new code, not propagating f->fp would not break any
  existing urls. 

* The f->fp propagation currently only happens in blobdiff (The only
  place, where fp is used). Is this enough to make it the standard
  gitweb parameter convention?

* The root tree is specified in git_tree with an missing f parameter.

* I don't know all security implication, if an empty file name is
  passed to all existing git function.

So I would like to drop this patch and pass the root directory as
missing f/fp parameter, which does not allow propagating f to fp.

Comments from other on this topic?

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 769e755..e4d3f8f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -310,14 +310,14 @@ if (defined $project) {
 }
 
 our $file_name = $cgi->param('f');
-if (defined $file_name) {
+if (defined $file_name && $file_name ne '') {
 	if (!validate_pathname($file_name)) {
 		die_error(undef, "Invalid file parameter");
 	}
 }
 
 our $file_parent = $cgi->param('fp');
-if (defined $file_parent) {
+if (defined $file_parent && $file_parent ne '') {
 	if (!validate_pathname($file_parent)) {
 		die_error(undef, "Invalid file parent parameter");
 	}
@@ -4203,6 +4203,10 @@ sub git_treediff {
 					"raw");
 	}
 
+	$file_parent = $file_name if (!defined $file_parent);
+	$file_name = undef if(defined $file_name && $file_name eq '');
+	$file_parent = undef if(defined $file_parent && $file_parent eq '');
+
 	if (!defined $hash) {
 		if (!defined $hash_base) {
 			die_error(undef,'tree parameter missing');
-- 
1.5.1.1.85.gf1888

[PATCH 7/7] gitweb: Adapt gitweb.js to new git_treeview calling convention

[Martin Koegler]

---

Necessary changes for patch 5, if patch 6 is applied.

 gitweb/gitweb.js |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
index 2e1a94d..51eeedb 100644
--- a/gitweb/gitweb.js
+++ b/gitweb/gitweb.js
@@ -105,8 +105,12 @@ function ToURL()
     parts.push("a="+this.a);
   if(this.f)
     parts.push("f="+this.f);
+  else if(this.hb)
+    parts.push("f=");
   if(this.fp)
     parts.push("fp="+this.fp);
+  else if(this.hpb)
+    parts.push("fp=");
   if(this.h)
     parts.push("h="+this.h);
   if(this.hp)
-- 
1.5.1.1.85.gf1888

[PATCH 2/7] 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>
---

I unified all blobdiff variants and added support for comparing blobs
with different names.

If h/hp parameter are missing, I need to generate them with
git_get_hash_by_path, as the are needed for the html header, which is
generated before parsing the git-diff output.

I currently ignore all mode changed, as they are part of the tree. I
don't think that displaying a mode change message justifes two call to
git-ls-tree for each blob diff (Currently it only calls git-ls-tree
for each missing h/hp parameter).

 gitweb/gitweb.perl |  136 ++++++++++++++++------------------------------------
 1 files changed, 41 insertions(+), 95 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cbd8d03..790041c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3902,109 +3902,54 @@ 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");
-
-		} else {
-			die_error('404 Not Found', "Missing one of the blob diff parameters");
-		}
+	my $expires = '+1d';
 
-		if (@difftree > 1) {
-			die_error('404 Not Found', "Ambiguous blob diff specification");
-		}
-
-		%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'};
+	$file_parent ||= $file_name;
 
-		# 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;
+	}
 
-		# 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 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);
 	}
 
-	# 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_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");
+	}
 
-		# 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';
-		}
 
-		# 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,
+	$hash_parent, $hash, "--"
+		or die_error(undef, "Open git-diff failed");
+
 	# header
 	if ($format eq 'html') {
 		my $formats_nav =
@@ -4028,11 +3973,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.1.1.85.gf1888

Re: [PATCH 2/7] gitweb: Support comparing blobs with different names

[Jakub Narebski]

Martin Koegler wrote:
> 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>
> ---

Shouldn't the comment below be also a part of commit message (perhaps
changed to use passive form)?
 
> I unified all blobdiff variants and added support for comparing blobs
> with different names.
> 
> If h/hp parameter are missing, I need to generate them with
> git_get_hash_by_path, as the are needed for the html header, which is
> generated before parsing the git-diff output.
> 
> I currently ignore all mode changes, as they are part of the tree. I
> don't think that displaying a mode change message justifes two call to
> git-ls-tree for each blob diff (Currently it only calls git-ls-tree
> for each missing h/hp parameter).

I have mixed feelings about this. On the one hand side the blobdiff
generation is simplified as we have only one codepath instead of two,
and removed codepath in rare cases (not generated by gitweb currently)
might return incorrect results in the case of requesting diff between
two arbitrary and unrelated files.

On the other hand side we do loose some information, contained in
extended diff header, like mode changes, renames and such.

It's a pity that extended sha-1 syntax to specify blobs, namely the
<tree-ish>:<path> syntax is opaque, and git-diff machinery sees only the
result of it, as if it was called directly with the sha-1 of blob.
It would be nice if "git diff <tree1>:<path1> <tree2>:<path2>" took
the information about file name and mode (permissions) from <tree1>
and <tree2> and included such information in extended git diff header.

Currently we have:

  $ git diff -p HEAD^ HEAD -- gitweb/test/hardlink
  diff --git a/gitweb/test/hardlink b/gitweb/test/hardlink
  old mode 100644
  new mode 100755

but

  $ git diff -p HEAD^:gitweb/test/hardlink HEAD:gitweb/test/hardlink

returns empty diff.


I'd rather have core git support for this first before changing how 
git_blobdiff is implemented in gitweb...

-- 
Jakub Narebski
Poland

Re: [PATCH 2/7] gitweb: Support comparing blobs with different names

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Currently we have:
>
>   $ git diff -p HEAD^ HEAD -- gitweb/test/hardlink
>   diff --git a/gitweb/test/hardlink b/gitweb/test/hardlink
>   old mode 100644
>   new mode 100755
>
> but
>
>   $ git diff -p HEAD^:gitweb/test/hardlink HEAD:gitweb/test/hardlink
>
> returns empty diff.

By "blobdiff", I understand you are talking about the "diff"
link that appears on each path at the bottom of the commit view.

I think the bug is the way gitweb drives the core; it uses the
latter form, when it has perfectly good information to run the
former.  It's not like you have an UI that says "pick any path
from any comit first; after you have picked one, pick the other
arbitrary one; now we run compare them".  The UI says "Give me
diff for this path in this commit".

Re: [PATCH 2/7] gitweb: Support comparing blobs with different names

[Jakub Narebski]

Junio C Hamano <junkio@cox.net> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Currently we have:
>>
>>   $ git diff -p HEAD^ HEAD -- gitweb/test/hardlink
>>   diff --git a/gitweb/test/hardlink b/gitweb/test/hardlink
>>   old mode 100644
>>   new mode 100755
>>
>> but
>>
>>   $ git diff -p HEAD^:gitweb/test/hardlink HEAD:gitweb/test/hardlink
>>
>> returns empty diff.
> 
> By "blobdiff", I understand you are talking about the "diff"
> link that appears on each path at the bottom of the commit view.

By "blobdiff" I mean "blobdiff" action (a=blobdiff), generated by
git_blobdiff subroutine.  The "diff" link that appears for each
path at the bottom of the "commit" view (in the whatchanged / difftree
part), and the "diff to current" link in the "history" view for files
both lead to "blobdiff" view.
 
> I think the bug is the way gitweb drives the core; it uses the
> latter form, when it has perfectly good information to run the
> former.  It's not like you have an UI that says "pick any path
> from any comit first; after you have picked one, pick the other
> arbitrary one; now we run compare them".  The UI says "Give me
> diff for this path in this commit".

_Currently_ gitweb uses git-diff-tree with path limiting when there
is enough information given, which means for all "blobdiff" links
currently generated by gitweb.  It uses git-diff for blobs only if
there is not enough information, e.g. for legacy URL; before moving
to generating diffs with git-diff-tree and git-diff, and all patches
were generated by external /usr/bin/diff, there were no 'hp' and 'hpb'
parameters which are needed for git-diff-tree.

But using git-diff-tree with path limiter to extract diff between
given blobs for "blobdiff" view is not without its caveats.  For example
for a rename you need both pre and post filenames in path limiter, 
otherwise you would get only "half" of rename diff, namely appropriate
creation diff.  And you can select only between blobs which have changed
from one commit (tree) to other; you cannot for example generate diff
between README at 'master' and INSTALL at 'next'.  Although why one 
would want such diff are beyond me... well, except for [overrated] 
completeness...

Martin wants to have ability to generate diff (blobdiff) between two
arbitrary blobs (two arbitrary files at arbitrary revisions). Earlier
patch if I remember correctly introduced yet another codepath; this one
tries to generate everything using one codepath, the git-diff one.
"Pick any path from any comit first; after you have picked one, pick the 
other arbitrary one; now we run compare them".

It adds new feature to gitweb I think almost nobody would use, namely
comparing arbitrary blobs (arbitrary files, with different names), and 
unifies diff generation in git_blobdiff (everything does use git-diff).
But it does so [currently] at the loss of information, namely ignoring 
all mode changes.  The mode changes can be done without lost of 
unification at the cost of performance of always having to do 
git-ls-tree, twice, or at the loss of having git_blobdiff quite 
complicated.

So from me it is slight Nak to this patch, at least unless "git diff 
<tree1>:<path1> <tree2>:<path2>" will take the information about file 
name and mode (permissions) from <tree1> and <tree2>, and included such 
information in extended git diff header.

-- 
Jakub Narebski
Poland

[PATCH 0/4] git-diff: use mode for tree:name syntax (was: Re: [PATCH 2/7] gitweb: Support comparing blobs with different names)

[Martin Koegler]

On Fri, Apr 20, 2007 at 10:49:08PM +0200, Jakub Narebski wrote:
> So from me it is slight Nak to this patch, at least unless "git diff 
> <tree1>:<path1> <tree2>:<path2>" will take the information about file 
> name and mode (permissions) from <tree1> and <tree2>, and included such 
> information in extended git diff header.

I created a prototyp implemention:

* Patch 1:
 add a parameter to get_sha1 to store the mode, if it is known.
 If it is not possible to dermine the mode, it stores S_IFINVALID.

* Patch 2:
 add space for the mode in object_array (unknown mode is S_IFINVALID).

* Patch 3:
 stores the mode for <tree>:<filename> syntax

* Patch 4:
 use mode information for comparing two blobs

The patches are large, as some heavy used functions get a new
parameter. The changes should not increase the CPU time. Patch 2
increases the storage size for each object of object_array by an
unsigned for the mode.

My blobdiff patch will need some small adaptations:
* we need to add --raw -p --full-index as diff arguments
* we must pass $hash_parent_base.':'.$file_parent and
  $hash_base.':'.$file_name to git-diff instead of 
  $hash and $hash_parent, if possible.

mfg Martin Kögler

[PATCH 2/7] 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>
---
New version, as I found a bug in the expiration handling code.

I unified all blobdiff variants and added support for comparing blobs
with different names.

If h/hp parameter are missing, I need to generate them with
git_get_hash_by_path, as the are needed for the html header, which is
generated before parsing the git-diff output.

I currently ignore all mode changed, as they are part of the tree. I
don't think that displaying a mode change message justifes two call to
git-ls-tree for each blob diff (Currently it only calls git-ls-tree
for each missing h/hp parameter).

 gitweb/gitweb.perl |  136 ++++++++++++++++------------------------------------
 1 files changed, 41 insertions(+), 95 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cbd8d03..790041c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3902,109 +3902,54 @@ 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");
-
-		} else {
-			die_error('404 Not Found', "Missing one of the blob diff parameters");
-		}
+	my $expires = '+1d';
 
-		if (@difftree > 1) {
-			die_error('404 Not Found', "Ambiguous blob diff specification");
-		}
-
-		%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'};
+	$file_parent ||= $file_name;
 
-		# 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;
+	}
 
-		# 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 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);
 	}
 
-	# 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_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");
+	}
 
-		# 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';
-		}
 
-		# 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,
+	$hash_parent, $hash, "--"
+		or die_error(undef, "Open git-diff failed");
+
 	# header
 	if ($format eq 'html') {
 		my $formats_nav =
@@ -4028,11 +3973,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.1.1.85.gf1888

Re: [PATCH 2/7] gitweb: Support comparing blobs with different names

[Jakub Narebski]

On Monday, 16 April 2007, Martin Koegler wrote:

> Currently, blobdiff can only compare blobs with different file
> names, if no hb/hpb parameters are present.
> 
Not true. Since commit 5ae917acdf40444945271e5e014cda37e202504e

  "gitweb: Support comparing blobs (files) with different names"

git_blobdiff ('blobdiff' view) suports comparing arbitrary blobs
(also with different file names) when no hp/hpb parameters are
present (although there would be no mode change information),
but supports comparing blobs with different file names in presence
of hp/hpb parameters, provided that it is proper rename diff.

Since this commit you can select rename diff which is part of
'commitdiff' view as a 'blobdiff' view, correctly.

> This patch adds support for comparing two blobs specified by any
> combination of hb/f/h and hpb/fp/hp.
>
But it used to lose mode change information. This info should be
I think in commit message.

Because your work on including mode changes for git-diff with
<tree-ish>:<path-to-blob> extended SHA-1 syntax (thanks a lot!)
I guess that you would want to rewrite this patch.
 
> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> ---
> New version, as I found a bug in the expiration handling code.
> 
> I unified all blobdiff variants and added support for comparing blobs
> with different names.
> 
> If h/hp parameter are missing, I need to generate them with
> git_get_hash_by_path, as the are needed for the html header, which is
> generated before parsing the git-diff output.

git_get_hash_by_path uses git-ls-tree but it does not catch all the info;
perhaps git_get_info_by_path would be called for here.

> I currently ignore all mode changes, as they are part of the tree. I
> don't think that displaying a mode change message justifes two call to
> git-ls-tree for each blob diff (Currently it only calls git-ls-tree
> for each missing h/hp parameter).

That is not a problem now, thanks to your patches... although it would
be nice to have subroutine which would get the difftree information
from extended git diff header (if it is possible).

> +	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;
> +	}

Usually gitweb _changes_ $expires to '+1d' when appropriate, instead
of defaulting to '+1d' and undefining it. But this might be the matter
of style consistency, and personal preferences: one complicated boolean
expression or set of if ... elsif ... conditionals.

The idea is that both TO and FROM are to be immutable; this means
either $hash_parent (or $hash_parent_base) being full SHA-1 and having
filename defined, or if appropriate "parent" hash is not present (because
"parent" hash has precedence over blob hash) it means that $hash
(or $hash_parent) being full SHA-1.

[...]  
> +	if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent) {
> +	    $hash_parent = git_get_hash_by_path($hash_parent_base, $file_parent);
> +	}
[...]
> +	# open patch output
> +	open $fd, "-|", git_cmd(), "diff", @diff_opts,
> +	$hash_parent, $hash, "--"
> +		or die_error(undef, "Open git-diff failed");

You would most probably use now "$hash_base:$file_name" instead of $hash
if $hash_base is defined, i.e.

  defined $hash_base ? "$hash_base:$file_name" : $hash

and similarly for $hash_parent parameter now that <tree>:<path> form
respects mode changes information.

-- 
Jakub Narebski
ShadeHawk on #git
Poland

Re: [PATCH 2/7] gitweb: Support comparing blobs with different names

[Martin Koegler]

On Sun, Apr 29, 2007 at 11:35:49PM +0200, Jakub Narebski wrote:
> On Monday, 16 April 2007, Martin Koegler wrote:
> 
> > Currently, blobdiff can only compare blobs with different file
> > names, if no hb/hpb parameters are present.
> > 

I've already posted a new version of this patch:
http://www.spinics.net/lists/git/msg28812.html

Can you please look at it.

> > Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> > ---
> > New version, as I found a bug in the expiration handling code.
> > 
> > I unified all blobdiff variants and added support for comparing blobs
> > with different names.
> > 
> > If h/hp parameter are missing, I need to generate them with
> > git_get_hash_by_path, as the are needed for the html header, which is
> > generated before parsing the git-diff output.
> 
> git_get_hash_by_path uses git-ls-tree but it does not catch all the info;
> perhaps git_get_info_by_path would be called for here.

I now only need the hash of the blob, if not passed as parameter, to
generate a correct header.

> [...]  
> > +	if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent) {
> > +	    $hash_parent = git_get_hash_by_path($hash_parent_base, $file_parent);
> > +	}
> [...]
> > +	# open patch output
> > +	open $fd, "-|", git_cmd(), "diff", @diff_opts,
> > +	$hash_parent, $hash, "--"
> > +		or die_error(undef, "Open git-diff failed");
> 
> You would most probably use now "$hash_base:$file_name" instead of $hash
> if $hash_base is defined, i.e.
> 
>   defined $hash_base ? "$hash_base:$file_name" : $hash
> 
> and similarly for $hash_parent parameter now that <tree>:<path> form
> respects mode changes information.

Addressed in new patch.

mfg Martin Kögler

[PATCH 4/7] 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.
---

I removed the parent links for now, as they require more discussion.

This patch uses empty fp/f for the root directory and does not propagte f to fp.
Patch 6 adds this.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1551d95..c2dc82d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -450,6 +450,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
@@ -4174,6 +4176,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, $hash_base, $hash_parent_base, $file_name, $file_parent);
+		print "<br/>\n";
+
+		git_patchset_body($fd, \@difftree, $hash_base, $hash_parent_base, $file_name, $file_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);
-- 
1.5.1.1.85.gf1888

[PATCH 4/7] 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.
---
New version, as I found a bug in the expiration handling code.

I removed the parent links for now, as they require more discussion.

This patch uses empty fp/f for the root directory and does not propagte f to fp.
Patch 6 adds this.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1551d95..c2dc82d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -450,6 +450,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
@@ -4174,6 +4176,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, $hash_base, $hash_parent_base, $file_name, $file_parent);
+		print "<br/>\n";
+
+		git_patchset_body($fd, \@difftree, $hash_base, $hash_parent_base, $file_name, $file_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);
-- 
1.5.1.1.85.gf1888