git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Supporting more gitweb diff possibities
@ 2007-03-25 20:26 Martin Koegler
  2007-03-26 17:11 ` Jakub Narebski
  0 siblings, 1 reply; 2+ messages in thread
From: Martin Koegler @ 2007-03-25 20:26 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

I have done a first series of 6 patches, which improves blobdiff and
adds treediff.

[PATCH] gitweb: show no difference message

This patch shows an "no difference" message instead of nothing for
equal objects.

[PATCH] gitweb: Support comparing blobs with different names

This patch adds support for comparing objects with different file
names using hb/hbp.

[PATCH] gitweb: link base commit (hpb) to blobdiff output

Add link the parent commit, as there is currently no such link.

[PATCH] gitweb: support filename prefix in git_patchset_body
[PATCH] gitweb: support filename prefix in git_difftree_body
[PATCH] gitweb: Add treediff

These 3 patches add the treediff method. Its a complete reworked
verion. As git-diff-tree outputs relativ patches (discards part of the
compared tree objects), the first two patches are necessary to produce
correct links in the treediff output.

I do not see many possibilties for code sharing with git_commitdiff:
The only large portion of common code is calling git-diff-tree. I
don't think that this would justify the more complex code.

To test these patches, I still use a JavaScript implementation (see
below). I'm working on the server side implementation.

The next patch, I'm currently working on, will introduce a wrapper for all
calls to
$cgi->a({-href => href([Parameter])}, [Title]) 

mfg Martin Kögler

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 71d0933..db58319 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1837,6 +1837,7 @@ sub git_footer_html {
 		close $fd;
 	}
 
+	print '<script type="text/javascript" src="gitweb.js"></script>';
 	print "</body>\n" .
 	      "</html>";
 }


gitweb.js:
----------
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.h=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();

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: Supporting more gitweb diff possibities
  2007-03-25 20:26 Supporting more gitweb diff possibities Martin Koegler
@ 2007-03-26 17:11 ` Jakub Narebski
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Narebski @ 2007-03-26 17:11 UTC (permalink / raw)
  To: Martin Koegler; +Cc: git

On Sun, Mar 25, 2007, Martin Koegler wrote:

> I have done a first series of 6 patches, which improves blobdiff and
> adds treediff.

Just a few nits.

First, I think it would be better to have individual patches to be
replies to this cover letter, either directly (all patch mails
are replies to cover letter) or indirectly (i.e. chained, the later
patch is reply to earlier patch, first patch ir reply to cover letter).

Second, it is a good idea to _number_ patches in series, i.e. have
  "[PATCH 0/6] Supporting more gitweb diff possibities"
as cover letter subject, and number patches respectively. The option
`-n' of git-format-patch (together with --start-number if you make
patches one by one) takes care of that.

> [PATCH] gitweb: show no difference message

This should be I think: 
  "[PATCH 1/6] gitweb: show "no difference" message for empty diff"

> This patch shows an "no difference" message instead of nothing for
> equal objects.

This is a good patch, worth doing even without rest of the patches.

Currently we have only one place (I think) where gitweb can generate
link to "blobdiff", namely "diff to parent" link in "history" view
for plain file, when e.g. some change was (explicitely or accidentally)
reverted.

The few comments about style (variable naming, CSS style for 
"no differences" message) are in the reply to the patch.

> [PATCH] gitweb: Support comparing blobs with different names
> 
> This patch adds support for comparing objects with different file
> names using hb/hbp.

Good idea; I have replied with an alternate solution, involving adding
'fp' to git-diff-tree path limit instead of git-diff with hpb:fp.

I'm sorry for the confusing advice wrt. git_blobdiff and rename diffs.

> [PATCH] gitweb: link base commit (hpb) to blobdiff output
> 
> Add link the parent commit, as there is currently no such link.

I'm rather ambivalent about this patch. Perhaps there is currently no such
link, but you can always click on one of the links to parent blob, and
from blob view to commit, commitdiff or tree.

By the way, the "(from: _commitdiff_)" link in "commitdiff" view
(and similar one in "commit" view) is part of the "next link" family
of "(parent: _commitdiff_)" and "(merge: _commitdiff_ _commitdiff_)"
which allow to go down the line of parents without need to change
views (or go to the 'parent' header in the case of "commit" view),
just like you would press PageDown during "git log" or "git log -p"
invocation from the command line. "(from: _commitdiff_)" was added
only for completeness (and because it was easy to do).

> [PATCH] gitweb: support filename prefix in git_patchset_body
> [PATCH] gitweb: support filename prefix in git_difftree_body

I'm not sure if those patches should or should not be concatenated
together in one commit.

See further comments on style (variable naming) and alternate solution
in the comments to first patch of those two: they refer to both patches.

> [PATCH] gitweb: Add treediff

Shouldn't it be:
  "[PATCH 6/6] gitweb: Add "treediff" and "treediff_plain" views"

> These 3 patches add the treediff method. Its a complete reworked
> verion. As git-diff-tree outputs relative patches (discards part of the
> compared tree objects), the first two patches are necessary to produce
> correct links in the treediff output.

See comments for that patch.

> I do not see many possibilties for code sharing with git_commitdiff:
> The only large portion of common code is calling git-diff-tree. I
> don't think that this would justify the more complex code.

I was not thinking about using git_commitdiff to generate "treediff"
view, but rather about extracting the common code into separate subroutine.
But it might (or might not) be not worth the hassle. And it can always
be done in separate "refactoring" patch.

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-03-26 17:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-25 20:26 Supporting more gitweb diff possibities Martin Koegler
2007-03-26 17:11 ` Jakub Narebski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).