{"thread":{"id":"7285","subject":"[RFC] More diff possibilities in gitweb","startedAt":"2007-03-17T15:17:31Z","lastAt":"2007-03-20T01:41:03Z","messageCount":9,"participants":["Martin Koegler","Johannes Schindelin","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"37321","messageId":"20070317151731.GA32571@auto.tuwien.ac.at","threadId":"7285","inReplyTo":null,"subject":"[RFC] More diff possibilities in gitweb","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-17T15:17:31Z","receivedAt":"2007-03-17T15:17:31Z","isPatch":false,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Compared to eg. viewcvs, gitweb offers fewer possibilies to do a diff.\nWith viewcvs, you can compare arbitrary version of a file; gitweb only\noffers diff to previous (or sometimes current) version.\n\nAs git offers an infinite number of possible (and maybe useful) diffs,\nI added to each each compareable object two links:\n* \"base\": Select this object as parent\n* \"diff\": Diff against the last select parent\n\nThe selected parent object is stored as a cookie, so it is possible to\ndo a diff between objects shown on different pages.\n\nThe whole is implemented in JavaScript on the client side (tested with\nIE 6 and Mozilla). On the server side, only the JavaScript file is\nincluded in the output (see below). The link generation is done\ncompletely in the browser.\n\nThe patch offers the following possibilies:\n* Compare any commit with an other commit\n Passes the hashes of the two commits as h and hp to commitdiff\n function. The output includes the message of one commit, which can\n be confusing.\n* Compare any blob with an other blob\n This feature is based on the blobdiff function. There are two types\n of blob:\n - blobs in the tree view can be compared with any blob in the tree view.\n - blobs in the history view can only be compared with any blob with\n the same file name in the history view.\n* Compare trees\n gitweb currently has no function for comparing trees. I currently\n misused blobdiff for this. The content of the diff is OK, only the\n filenames are wrong/destroyed. Additionally gitweb issus some \"Use\n of uninitialized value\" message to the error log.\n\n A proper implementation would need new function in gitweb.cgi,\n basically git_commitdiff removing the requirement, that the hashes\n must be a commit.\n\nIs there an interest these function?\n\nmfg Martin Kögler\n\nPatch for gitweb.cgi (in function git_footer_html):\n---------------------------------------------------\n \n+ print '';\n print \"\\n\" .\n \"\";\n\nFile gitweb.js:\n---------------\nfunction getCookie(name)\n{\n var name=name+\"=\";\n var c=document.cookie;\n var p=c.indexOf(name);\n if(p==-1)\n return null;\n c=c.substr(p+name.length,c.length);\n p=c.indexOf(\";\");\n if(p==-1)\n return c;\n else\n return c.substr(0, p);\n}\n\nfunction insertAfter(elem, node)\n{\n if(node.nextSibling)\n node.parentNode.insertBefore(elem, node.nextSibling);\n else\n node.parentNode.appendChild(elem);\n}\n\nfunction createLink(href, linktext)\n{\n var l=document.createElement(\"a\");\n l.appendChild(document.createTextNode(linktext));\n l.href=href;\n return l;\n}\n\nfunction createLinkGroup(href1, basetxt, href2, difftxt)\n{\n var l=document.createElement(\"span\");\n l.appendChild(document.createTextNode(\" (\"));\n l.appendChild(createLink(href1, basetxt));\n l.appendChild(document.createTextNode(\" | \"));\n l.appendChild(createLink(href2, difftxt));\n l.appendChild(document.createTextNode(\") \"));\n return l;\n}\n\nfunction GitRef()\n{\n this.t=null;\n this.h=null;\n this.hb=null;\n this.f=null;\n this.ToRef=ToRef;\n}\n\nfunction ToRef()\n{\n var parts=new Array();\n if(this.f)\n parts.push(\"f=\"+this.f);\n if(this.h)\n parts.push(\"h=\"+this.h);\n if(this.hb)\n parts.push(\"hb=\"+this.hb);\n if(this.t)\n parts.push(\"t=\"+this.t);\n return parts.join(\"@\");\n}\n\nfunction splitGitRef(ref)\n{\n var parts=ref.split(\"@\");\n var res=new GitRef();\n var i;\n for(i=0;i1)\n {\n var parts=Urls[1].split(\";\");\n var i;\n for(i=0;i The whole is implemented in JavaScript on the client side (tested with \n> IE 6 and Mozilla).\n\nThis is not acceptable for me. There are too many people blindly running \nJavaScript everywhere, and we should not enforce such bad behaviour.\n\nAt least, you need a way to do it with plain old HTML (which is easy \nenough).\n\nAlso I'd like to know: is there any reason you did not send a proper diff, \ngiven that you are so interested in diffs?\n\nCiao,\nDscho\n"},{"id":"37333","messageId":"ethnog$cf6$1@sea.gmane.org","threadId":"7285","inReplyTo":"20070317151731.GA32571@auto.tuwien.ac.at","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-17T21:51:51Z","receivedAt":"2007-03-17T21:51:51Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"[Cc: Martin Koegler , git@vger.kernel.org]\n\nMartin Koegler wrote:\n\n> The whole is implemented in JavaScript on the client side (tested with\n> IE 6 and Mozilla). On the server side, only the JavaScript file is\n> included in the output (see below). \n\nI'd rather not use JavaScript for that, but perhaps add in the history\nview (shortlog, history) selectbox to select commits (for shortlog, and\nhistory of a directory) or versions of a blob/file (history of a file)\nlike in an \"info\" view e.g. in MoinMoin wiki:\n http://git.or.cz/gitwiki/GitSurvey?action=info\nJavaScript can be used to beautify this like e.g. MediaWiki engine\ndoes:\n http://en.wikipedia.org/w/index.php?title=Darcs&action=history \n(to allow selection of diff only in forward direction).\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"37375","messageId":"20070318141451.GA9607@auto.tuwien.ac.at","threadId":"7285","inReplyTo":"Pine.LNX.4.63.0703171902200.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-18T14:14:51Z","receivedAt":"2007-03-18T14:14:51Z","isPatch":false,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Sat, Mar 17, 2007 at 07:04:54PM +0100, Johannes Schindelin wrote:\n> On Sat, 17 Mar 2007, Martin Koegler wrote:\n> \n> > The whole is implemented in JavaScript on the client side (tested with \n> > IE 6 and Mozilla).\n> \n> This is not acceptable for me. There are too many people blindly running \n> JavaScript everywhere, and we should not enforce such bad behaviour.\n>\n> At least, you need a way to do it with plain old HTML (which is easy \n> enough).\n\nIf the browser supports no JavaScript, the user can use all current\nfeatures, except the new diff capabilities.\n\nMy first thoughts where also about implementing it inside\ngitweb.cgi. This would mean a large change to the code, as all code\nwhich generate links needs to be adapted.\n\nAdditionally, selecting a base object would mean, that you must submit\na request to the server and probably return something (probable the\nsame page, on which you selected the base object).\n\nFor me, this would be no problem, but for high load servers (eg. gitweb at\nkernel.org), it would incread the system load.\n\nWith JavaScript, this step needs no server interaction. Infact, a\nclient could add the diff feature to any gitweb server with\neg. greasemonkey, by injecting the JavaScript code.\n\n> Also I'd like to know: is there any reason you did not send a proper diff, \n> given that you are so interested in diffs?\n\nI'm not interessted in generating patchs via gitweb. I want to examine\nand review differences between various branches/tags/commits/... .\n\nThe \"pseudo\" diff for gitweb.cgi was to illustrate, how to integrate\ngitweb.js. The whole thing is under development, so I have not created\na clean version.\n\nAs I wrote in my last mail, I used blobdiff to generation tree diffs,\nwhich results in wrong/missing file names. In the mean time, I've\ncreate a first version of a treediff function for gitweb.\n\nmfg Martin Kögler\n\n--- old/gitweb.js 2007-03-17 15:18:23.284317140 +0100\n+++ gitweb.js 2007-03-17 22:25:18.254190096 +0100\n@@ -259,7 +259,7 @@\n }\n if(c.t=='tree'&&url.a=='tree')\n {\n- url.a='blobdiff';\n+ url.a='treediff';\n if(c.h&&url.h)\n {\n url.hb=null;\n--- ../mirror/git.git/gitweb/gitweb.perl\t2007-03-12 22:06:44.000000000 +0100\n+++ gitweb.cgi\t2007-03-17 18:41:50.000000000 +0100\n@@ -446,6 +446,8 @@\n \t\"tag\" => \\&git_tag,\n \t\"tags\" => \\&git_tags,\n \t\"tree\" => \\&git_tree,\n+\t\"treediff\" => \\&git_treediff,\n+\t\"treediff_plain\" => \\&git_treediff_plain,\n \t\"snapshot\" => \\&git_snapshot,\n \t\"object\" => \\&git_object,\n \t# those below don't need $project\n@@ -1835,6 +1837,7 @@\n \t\tclose $fd;\n \t}\n \n+\tprint '';\n \tprint \"\\n\" .\n \t \"\";\n }\n@@ -4191,6 +4194,106 @@\n \tgit_commitdiff('plain');\n }\n \n+sub git_treediff {\n+\tmy $format = shift || 'html';\n+\t$hash ||= $hash_base;\n+\t$hash_parent ||= $hash_parent_base;\n+\tif (!defined $hash_parent || !defined $hash)\n+\t{\n+\t die_error(undef, \"Missing one of the tree diff parameters\");\n+\t}\n+\n+\t# we need to prepare $formats_nav before any parameter munging\n+\tmy $formats_nav;\n+\tif ($format eq 'html') {\n+\t\t$formats_nav =\n+\t\t\t$cgi->a({-href => href(action=>\"treediff_plain\",\n+\t\t\t hash=>$hash, hash_parent=>$hash_parent)},\n+\t\t\t \"raw\");\n+\t\t$formats_nav .= ' | ';\n+\t\t$formats_nav .=\n+\t\t\t$cgi->a({-href => href(action=>\"tree\",\n+\t\t\t hash=>$hash)},\n+\t\t\t \"tree\");\n+\t\t$formats_nav .= ' <-> ';\n+\t\t$formats_nav .=\n+\t\t\t$cgi->a({-href => href(action=>\"tree\",\n+\t\t\t hash=>$hash)},\n+\t\t\t \"tree\");\n+\t}\n+\n+\t# read treediff\n+\tmy $fd;\n+\tmy @difftree;\n+\tif ($format eq 'html') {\n+\t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n+\t\t\t\"--no-commit-id\", \"--patch-with-raw\", \"--full-index\",\n+\t\t\t$hash_parent, $hash, \"--\"\n+\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n+\n+\t\twhile (my $line = <$fd>) {\n+\t\t\tchomp $line;\n+\t\t\t# empty line ends raw part of diff-tree output\n+\t\t\tlast unless $line;\n+\t\t\tpush @difftree, $line;\n+\t\t}\n+\n+\t} elsif ($format eq 'plain') {\n+\t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n+\t\t\t'-p', $hash_parent, $hash, \"--\"\n+\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n+\n+\t} else {\n+\t\tdie_error(undef, \"Unknown treediff format\");\n+\t}\n+\n+\t# non-textual hash id's can be cached\n+\tmy $expires;\n+\tif ($hash =~ m/^[0-9a-fA-F]{40}$/) {\n+\t\t$expires = \"+1d\";\n+\t}\n+\n+\t# write diff message\n+\tif ($format eq 'html') {\n+\t\tgit_header_html(undef, $expires);\n+\n+\t\tprint \"

$formats_nav
\\n\";\n+\t\tprint \"
$hash vs $hash_parent
\\n\";\n+\t\tprint \"
\\n\";\n+\n+\t} elsif ($format eq 'plain') {\n+\t\tmy $filename = basename($project) . \"-$hash-$hash_parent.patch\";\n+\n+\t\tprint $cgi->header(\n+\t\t\t-type => 'text/plain',\n+\t\t\t-charset => 'utf-8',\n+\t\t\t-expires => $expires,\n+\t\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"');\n+\t\tprint \"X-Git-Url: \" . $cgi->self_url() . \"\\n\\n\";\n+\t}\n+\n+\t# write patch\n+\tif ($format eq 'html') {\n+\t\tgit_difftree_body(\\@difftree, $hash, $hash_parent);\n+\t\tprint \"
\\n\";\n+\n+\t\tgit_patchset_body($fd, \\@difftree, $hash, $hash_parent);\n+\t\tclose $fd;\n+\t\tprint \"
\\n\"; # class=\"page_body\"\n+\t\tgit_footer_html();\n+\n+\t} elsif ($format eq 'plain') {\n+\t\tlocal $/ = undef;\n+\t\tprint <$fd>;\n+\t\tclose $fd\n+\t\t\tor print \"Reading git-diff-tree failed\\n\";\n+\t}\n+}\n+\n+sub git_treediff_plain {\n+\tgit_treediff('plain');\n+}\n+\n sub git_history {\n \tif (!defined $hash_base) {\n \t\t$hash_base = git_get_head_hash($project);\n"},{"id":"37376","messageId":"20070318150216.GB9607@auto.tuwien.ac.at","threadId":"7285","inReplyTo":"ethnog$cf6$1@sea.gmane.org","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-18T15:02:16Z","receivedAt":"2007-03-18T15:02:16Z","isPatch":false,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Sat, Mar 17, 2007 at 10:51:51PM +0100, Jakub Narebski wrote:\n> Martin Koegler wrote:\n> \n> > The whole is implemented in JavaScript on the client side (tested with\n> > IE 6 and Mozilla). On the server side, only the JavaScript file is\n> > included in the output (see below). \n> \n> I'd rather not use JavaScript for that, but perhaps add in the history\n> view (shortlog, history) selectbox to select commits (for shortlog, and\n> history of a directory) or versions of a blob/file (history of a file)\n> like in an \"info\" view e.g. in MoinMoin wiki:\n> http://git.or.cz/gitwiki/GitSurvey?action=info\n> JavaScript can be used to beautify this like e.g. MediaWiki engine\n> does:\n> http://en.wikipedia.org/w/index.php?title=Darcs&action=history \n> (to allow selection of diff only in forward direction).\n\nLet's focus on the UI first:\n\nUsing a select box, as shown above, exposes only a small part of all\npossible combinations, eg. how to compare commits on different\nbranches.\n\nIn my solution, I add to each link to a compareable object (commit,\ntree, blob) two new link, eg: link \"commit\" becomes \"commit (base |\ndiff)\". \"base\" stores the ID of the object in a cookie. \"diff\"\nretrieves the cookie and compares it with it's associated object.\n\nCurrently, only few combinations are implemented. Future version could\nintegrate tags (as an tag links to one commit). Additionally,\ncomparing tags/commits with a tree could be enabled, as a tag/commit\nlink to one tree.\n\nStoring the base object in a cookie enables the user to use multiple\nwindows/tabs showing, eg. different branches. He can select in on\nwindow the base object, switch to an other window, where he selects\nthe diff option. He can also go back in the browser history, without\nlosing his selection.\n\nAny sugesstions, how the UI can be improved or designed better?\n\nTo the implementation:\n\nThe whole thing can be implemented without any JavaScript on the\nserver side. For an out of tree implementation, my prototype is\nsimpler to maintain, as it needs only a few small changes to gitweb.\n\nIf there is an interesst in this feature (and nobody opposes including\nit), I can reimplement it in perl.\n\nI would do it in the following way:\n1) introduce git_treediff\n2) Add the generation of a base and diff link to each object\n3) If a base link is selected, the server sends a Set-Cookie header\nand redirects to the page, where it came from. So maybe the broser can\nfetch the page out of its cash in some situation.\n4) If a diff link is selected, based on the base object in the cookie,\nthe url to do the diff and redirects the browser to it. This should\nsimplify caching.\n\nIf depending on cookies is a problem for some out there, the base\nobject can be passed in the URLs as fallback. It would require, that\nthe select base object is included in every link, which gitweb\ngenerates.\n\nmfg Martin Kögler\n"},{"id":"37408","messageId":"200703182256.00005.jnareb@gmail.com","threadId":"7285","inReplyTo":"20070318150216.GB9607@auto.tuwien.ac.at","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-18T21:55:59Z","receivedAt":"2007-03-18T21:55:59Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, Mar 18, 2007, Martin Koegler wrote:\n> On Sat, Mar 17, 2007 at 10:51:51PM +0100, Jakub Narebski wrote:\n>> Martin Koegler wrote:\n>> \n>>> The whole is implemented in JavaScript on the client side (tested with\n>>> IE 6 and Mozilla). On the server side, only the JavaScript file is\n>>> included in the output (see below). \n>> \n>> I'd rather not use JavaScript for that, but perhaps add in the history\n>> view (shortlog, history) selectbox to select commits (for shortlog, and\n>> history of a directory) or versions of a blob/file (history of a file)\n>> like in an \"info\" view e.g. in MoinMoin wiki:\n>> http://git.or.cz/gitwiki/GitSurvey?action=info\n>> JavaScript can be used to beautify this like e.g. MediaWiki engine\n>> does:\n>> http://en.wikipedia.org/w/index.php?title=Darcs&action=history \n>> (to allow selection of diff only in forward direction).\n> \n> Let's focus on the UI first:\n> \n> Using a select box, as shown above, exposes only a small part of all\n> possible combinations, eg. how to compare commits on different\n> branches.\n\nI assume you meant \"e.g. it doesn't provide a way to compare commits\non different branches\".\n\nWe can easily add selectbox not only for shortlog and history views,\nbut also for heads view (which would allow to compare tips of different\nbranches) and tags view (which would allow to compare [commits pointed\nto by] different tags).\n\nAlthough there is yet another limitation, namely that shortlog and\nhistory views are divided into pages...\n\n> In my solution, I add to each link to a compareable object (commit,\n> tree, blob) two new link, eg: link \"commit\" becomes \"commit (base |\n> diff)\". \"base\" stores the ID of the object in a cookie. \"diff\"\n> retrieves the cookie and compares it with it's associated object.\n\nWell, I haven't though of it being a problem, because _I_ can have\nalways handcraft an URL to get requested diff from gitweb.\n\nBut after some thinking I guess that your idea has some merit\n_provided_ that \"(base | diff)\" (by the way, what would happen\nif someone would click \"diff\" without setting \"base\" first? is\n\"diff\" inactive, or is some \"base\" default?) is added _only_ when\nweb browser supports it. It means that it should appear only when\nJavaScript is turned on (easy if links are added by JavaScript)\nand when cookies are turned on (I'm not sure if you are checking\nthis).\n\nI guess that we could (under control of gitweb configuration, %feature\nhash or something like that) add links which would lead to server\nsetting a cookie, or adding unused CGI parameters ('hp', 'hpb' and\n'fp' are used only by 'diff' views).\n\n> Currently, only few combinations are implemented. Future version could\n> integrate tags (as an tag links to one commit). Additionally,\n> comparing tags/commits with a tree could be enabled, as a tag/commit\n> link to one tree.\n\nDiffs makes sense only between two commit-ish (commit-ish being commit\nor a tag; head also denotes a commit) for which we need 'h' and 'hp'\nparams, between two tree-ish (tree-ish being tree, commit or a tag)\nfor which we need 'h'/'hb' and 'hp'/'hpb' and usually 'f', sometimes\n'fp', and finnally between two blobs, for which we need 'h'/'hb' and\n'hp'/'hpb' and usually 'f', sometimes 'fp'.\n\nThere makes no sense to compare tree-ish with blob, for example.\n\nBy the way, I'm not that sure if gitweb handle correctly request for\ndiff of two _different_ tree-ish, or two arbitrary different files.\n\n[...]\n> 1) introduce git_treediff\n\nNOTE: git_treediff is in some parts very similar (I think) to\ngit_commitdiff with the exception of not being there commit message,\nnor links to parents, and I'd like very much to avoid code repetition\n(duplication) betweem git_treediff and git_commitdiff if possible.\nOn the other hand git_treediff is similar to git_blob in handling\nits parameters ('h', 'hp' but also 'hb', 'hpb' and 'f', 'fp').\n\n[...]\n> If depending on cookies is a problem for some out there, the base\n> object can be passed in the URLs as fallback. It would require, that\n> the select base object is included in every link, which gitweb\n> generates.\n\nNot much of a problem. Generation of links is consolidated in href()\nsubroutine, and it would be fairly easy to add passing through base\nparameters, if they are needed or not. It means that if 'shortlog' view\nwas passed 'hp' parameter then \"diff\" links (or \"diff \" links,\nor \"diff sel\"/\"diff selected\" links) would lead to diff with given base.\n\n\nP.S. I find it strange that you don't send patches from git. If you want\nto test gitweb, you should have git installed. Why not fetch git.git\nrepository, and send patches from it?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"37421","messageId":"200703190020.55806.jnareb@gmail.com","threadId":"7285","inReplyTo":"20070318141451.GA9607@auto.tuwien.ac.at","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-18T23:20:55Z","receivedAt":"2007-03-18T23:20:55Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Some of discussion is repeated in another subthread\n\nOn Sun, Mar 18, 2007, Martin Koegler wrote:\n> On Sat, Mar 17, 2007 at 07:04:54PM +0100, Johannes Schindelin wrote:\n>> On Sat, 17 Mar 2007, Martin Koegler wrote:\n>> \n>>> The whole is implemented in JavaScript on the client side (tested with \n>>> IE 6 and Mozilla).\n>> \n>> This is not acceptable for me. There are too many people blindly running \n>> JavaScript everywhere, and we should not enforce such bad behaviour.\n>>\n>> At least, you need a way to do it with plain old HTML (which is easy \n>> enough).\n> \n> If the browser supports no JavaScript, the user can use all current\n> features, except the new diff capabilities.\n>\n> My first thoughts where also about implementing it inside\n> gitweb.cgi. This would mean a large change to the code, as all code\n> which generate links needs to be adapted.\n>\n> Additionally, selecting a base object would mean, that you must submit\n> a request to the server and probably return something (probable the\n> same page, on which you selected the base object).\n\nFirst, generating gitweb links in gitweb _all_ goes through href()\nsubroutine, so it would be fairly easy to implement it server side,\ni.e. directly in gitweb.perl. One solution would be to set cookies\nfor parent (base) of diff server side. Another would be to pass\nthrough diff parent (diff base) options (CGI parameters) in non-diff\nviews. It means that for example shortlog, or history, or summary\nview would get 'hp', 'hpb', 'fp' parameters and pass it through\nto \"diff to selected base\" links. But it is a fact that it would\nmean additional request to the server to set the base object.\n\nThe correct solution I think which would satisfy everyone would\nbe to add plain HTML, i.e. server-side solution to \"diff with\nselected base\" if this option is turned on. Then JavaScript would\nreplace it by JavaScript solution (or would add JavaScript solution\nif server-side solution is turned off), again subject to gitweb\nconfiguration. JavaScript would use cookies if possible, and changing\nlocation if not (that is for discussion).\n\n> For me, this would be no problem, but for high load servers (eg. gitweb at\n> kernel.org), it would incread the system load.\n\nI think use JavaScript if possible, and server-side implementation\nif not would be good compromise.\n \n> With JavaScript, this step needs no server interaction. Infact, a\n> client could add the diff feature to any gitweb server with\n> eg. greasemonkey, by injecting the JavaScript code.\n\nWell, we could always put this in contrib/ as Greasmonkey script ;-)\n\n>> Also I'd like to know: is there any reason you did not send a proper diff, \n>> given that you are so interested in diffs?\n> \n> I'm not interessted in generating patchs via gitweb. I want to examine\n> and review differences between various branches/tags/commits/... .\n> \n> The \"pseudo\" diff for gitweb.cgi was to illustrate, how to integrate\n> gitweb.js. The whole thing is under development, so I have not created\n> a clean version.\n\nI'm just wondering why did you not used git for development. You have\nto have git installed to test gitweb, and it would be best to have\ngit.git repository fetched to base your work on newest 'master' version\n(or rather 'origin/master' or 'origin' version). So it would be natural\nto use git to work on gitweb, and to send git patches.\n \n> As I wrote in my last mail, I used blobdiff to generation tree diffs,\n> which results in wrong/missing file names. In the mean time, I've\n> create a first version of a treediff function for gitweb.\n\nWhich is IMVHO long way from ready.\n\n> --- old/gitweb.js 2007-03-17 15:18:23.284317140 +0100\n> +++ gitweb.js 2007-03-17 22:25:18.254190096 +0100\n\nFirst, if you don't use git to generate diffs, could you _please_\nuse equal depth paths patches, i.e. either \n\n --- gitweb.js.orig 2007-03-17 15:18:23.284317140 +0100\n +++ gitweb.js 2007-03-17 22:25:18.254190096 +0100\n\nor\n\n --- old/gitweb.js 2007-03-17 15:18:23.284317140 +0100\n +++ new/gitweb.js 2007-03-17 22:25:18.254190096 +0100\n\n\n> --- ../mirror/git.git/gitweb/gitweb.perl\t2007-03-12 22:06:44.000000000 +0100\n> +++ gitweb.cgi\t2007-03-17 18:41:50.000000000 +0100\n\nThe same as above (or even worse)\n\n> @@ -1835,6 +1837,7 @@\n> \t\tclose $fd;\n> \t}\n> \n> +\tprint '';\n> \tprint \"\\n\" .\n> \t \"\";\n> }\n\nI'd rather have end of line after closing '', and have scripts\nin the head section, not in body.\n\n> @@ -4191,6 +4194,106 @@\n> \tgit_commitdiff('plain');\n> }\n> \n> +sub git_treediff {\n> +\tmy $format = shift || 'html';\n[...]\n\nYou base git_treediff on git_commitdiff. On one hand it is right, as\ntreediff is like commmitdiff, but without commit message, nor link to\ncommit parents.\n\nOn the other hand you should be able to handle 'hb', 'hpb', 'f', 'fp'\nparameters like git_blobdiff does. And making git_difftree work between\ntwo different trees from two different commits (e.g. git-gui/ in git.git\nmainline and / in git-gui.git mainline) is not that easy...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"37552","messageId":"20070319220339.GA26080@auto.tuwien.ac.at","threadId":"7285","inReplyTo":"200703190020.55806.jnareb@gmail.com","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-19T22:03:39Z","receivedAt":"2007-03-19T22:03:39Z","isPatch":false,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"I tried to merge the two thread.\n\nOn Sun, Mar 18, 2007 at 10:55:59PM +0100, Jakub Narebski wrote:\n> On Sun, Mar 18, 2007, Martin Koegler wrote:\n> > On Sat, Mar 17, 2007 at 10:51:51PM +0100, Jakub Narebski wrote:\n> > Let's focus on the UI first:\n> > \n> > Using a select box, as shown above, exposes only a small part of all\n> > possible combinations, eg. how to compare commits on different\n> > branches.\n> \n> I assume you meant \"e.g. it doesn't provide a way to compare commits\n> on different branches\".\n\nYes\n\n> We can easily add selectbox not only for shortlog and history views,\n> but also for heads view (which would allow to compare tips of different\n> branches) and tags view (which would allow to compare [commits pointed\n> to by] different tags).\n> \n> Although there is yet another limitation, namely that shortlog and\n> history views are divided into pages...\n\nTo avoid all those problems, I tried the generic approach with \"(base | diff)\"\n \n> > In my solution, I add to each link to a compareable object (commit,\n> > tree, blob) two new link, eg: link \"commit\" becomes \"commit (base |\n> > diff)\". \"base\" stores the ID of the object in a cookie. \"diff\"\n> > retrieves the cookie and compares it with it's associated object.\n> \n> Well, I haven't though of it being a problem, because _I_ can have\n> always handcraft an URL to get requested diff from gitweb.\n\nI want a comfortable solution. Additionally, this can only be done by\na user, who knows about the internals of gitweb.\n\n> But after some thinking I guess that your idea has some merit\n> _provided_ that \"(base | diff)\" (by the way, what would happen\n> if someone would click \"diff\" without setting \"base\" first? is\n> \"diff\" inactive, or is some \"base\" default?) is added _only_ when\n\nIn my JavaScript solution, the user gets a message box in this case:\n\"no base object selected\".\n\n> web browser supports it. It means that it should appear only when\n> JavaScript is turned on (easy if links are added by JavaScript)\n> and when cookies are turned on (I'm not sure if you are checking\n> this).\n\nNo, there is no check at the moment. Buts this is basically checking\nthe navigator.cookieEnabled property. A user could only reject some\ncookies, but catching this would probably be too complicated.\n\n> I guess that we could (under control of gitweb configuration, %feature\n> hash or something like that) add links which would lead to server\n> setting a cookie, or adding unused CGI parameters ('hp', 'hpb' and\n> 'fp' are used only by 'diff' views).\n\nYes, but I would you a new set of parameters for it (eg bf, bh, bhb), so that a\npossible (future) fallback version without cookies can reuse them.\n \n> > Currently, only few combinations are implemented. Future version could\n> > integrate tags (as an tag links to one commit). Additionally,\n> > comparing tags/commits with a tree could be enabled, as a tag/commit\n> > link to one tree.\n> \n> Diffs makes sense only between two commit-ish (commit-ish being commit\n> or a tag; head also denotes a commit) for which we need 'h' and 'hp'\n> params, between two tree-ish (tree-ish being tree, commit or a tag)\n> for which we need 'h'/'hb' and 'hp'/'hpb' and usually 'f', sometimes\n\nFor tree diffs, I currently only support h/hp. Using hb/hbp and f/fp\nwould require, that both trees share the same file name.\n\n> 'fp', and finnally between two blobs, for which we need 'h'/'hb' and\n> 'hp'/'hpb' and usually 'f', sometimes 'fp'.\n\nFor blob diffs, I prefer to use only h/bp (and f/fp for showing nice\nfile names). I fall back to hp/hb and f for the blobs in the history\nviews (where h is not available), which has the drawback, that both\nblobs must have the same name.\n\n> There makes no sense to compare tree-ish with blob, for example.\n\nYes.\n\n> By the way, I'm not that sure if gitweb handle correctly request for\n> diff of two _different_ tree-ish, or two arbitrary different files.\n\nIf you use not hb/hbp, this works.\n\n> > 1) introduce git_treediff\n> \n> NOTE: git_treediff is in some parts very similar (I think) to\n> git_commitdiff with the exception of not being there commit message,\n> nor links to parents, and I'd like very much to avoid code repetition\n> (duplication) betweem git_treediff and git_commitdiff if possible.\n> On the other hand git_treediff is similar to git_blob in handling\n> its parameters ('h', 'hp' but also 'hb', 'hpb' and 'f', 'fp').\n\nI created git_treediff out of git_commitdiff.\n\n> > If depending on cookies is a problem for some out there, the base\n> > object can be passed in the URLs as fallback. It would require, that\n> > the select base object is included in every link, which gitweb\n> > generates.\n> \n> Not much of a problem. Generation of links is consolidated in href()\n> subroutine, and it would be fairly easy to add passing through base\n> parameters, if they are needed or not. It means that if 'shortlog' view\n> was passed 'hp' parameter then \"diff\" links (or \"diff \" links,\n> or \"diff sel\"/\"diff selected\" links) would lead to diff with given base.\n\n> First, generating gitweb links in gitweb _all_ goes through href()\n> subroutine, so it would be fairly easy to implement it server side,\n> i.e. directly in gitweb.perl. \n\nThis has on complication: href only generats the url. The link is\ngenerated in the caller. We would need a new wrapper for this.\n\n[...]\n\n> > For me, this would be no problem, but for high load servers (eg. gitweb at\n> > kernel.org), it would incread the system load.\n> \n> I think use JavaScript if possible, and server-side implementation\n> if not would be good compromise.\n\nI'll try to implement a solution for this.\n\n> I'm just wondering why did you not used git for development. You have\n> to have git installed to test gitweb, and it would be best to have\n> git.git repository fetched to base your work on newest 'master' version\n> (or rather 'origin/master' or 'origin' version). So it would be natural\n> to use git to work on gitweb, and to send git patches.\n\ngitweb.cgi is generated out of gitweb.perl. Then I need to manually\nchange some parameters in it and put it on a webserver. For\ndeveloping, it's easier for me to edit the cgi-script on the\nserver. In a diff of gitweb.cgi, I have to remove the unrelated\nchanges, for eg. setting paths parameters.\n\nI would be interessted, how git can help me in this workflow?\n\n> > As I wrote in my last mail, I used blobdiff to generation tree diffs,\n> > which results in wrong/missing file names. In the mean time, I've\n> > create a first version of a treediff function for gitweb.\n> \n> Which is IMVHO long way from ready.\n> \n> > --- old/gitweb.js 2007-03-17 15:18:23.284317140 +0100\n> > +++ gitweb.js 2007-03-17 22:25:18.254190096 +0100\n> \n> First, if you don't use git to generate diffs, could you _please_\n> use equal depth paths patches, i.e. either \n\nI'll try to produce better patches in the future.\n\n> > @@ -1835,6 +1837,7 @@\n> > \t\tclose $fd;\n> > \t}\n> > \n> > +\tprint '';\n> > \tprint \"\\n\" .\n> > \t \"\";\n> > }\n> \n> I'd rather have end of line after closing '', and have scripts\n> in the head section, not in body.\n\nFor my prototyp, it was easier put the script at the end. Putting it\nin the header would require, that I call the JavaScript load function\nin the body onload event. In the end version, I'll change that.\n\n> > @@ -4191,6 +4194,106 @@\n> > \tgit_commitdiff('plain');\n> > }\n> > \n> > +sub git_treediff {\n> > +\tmy $format = shift || 'html';\n> [...]\n> \n> You base git_treediff on git_commitdiff. On one hand it is right, as\n> treediff is like commmitdiff, but without commit message, nor link to\n> commit parents.\n> \n> On the other hand you should be able to handle 'hb', 'hpb', 'f', 'fp'\n> parameters like git_blobdiff does. And making git_difftree work between\n> two different trees from two different commits (e.g. git-gui/ in git.git\n> mainline and / in git-gui.git mainline) is not that easy...\n\nCompaing this (if git-gui.git history is part of git.git), already\nworks. It's currently simple, as I ignore hb. The diff would be\nrelativ (that means, that git-gui/ in git.git would be a/ and / in\ngit-gui.git would be b/). \n\nDo git-diff-tree offers an option to compare in two trees two\ndifferent file names? If not, using hb would require, that both trees\nhave the same name. In this case, as all tree links seems to have h, I\nsee no use for hb/hpb.\n\nmfg Martin Kögler\n"},{"id":"37569","messageId":"200703200241.04738.jnareb@gmail.com","threadId":"7285","inReplyTo":"20070319220339.GA26080@auto.tuwien.ac.at","subject":"Re: [RFC] More diff possibilities in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-20T01:41:03Z","receivedAt":"2007-03-20T01:41:03Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, Mar 19, 2007, Martin Koegler wrote:\n> I tried to merge the two threads.\n> \n> On Sun, Mar 18, 2007 at 10:55:59PM +0100, Jakub Narebski wrote:\n>> On Sun, Mar 18, 2007, Martin Koegler wrote:\n>>> On Sat, Mar 17, 2007 at 10:51:51PM +0100, Jakub Narebski wrote:\n>>>\n>>> Let's focus on the UI first:\n>>> \n>>> Using a select box, as shown above, exposes only a small part of all\n>>> possible combinations, eg. how to compare commits on different\n>>> branches.\n>> \n>> I assume you meant \"e.g. it doesn't provide a way to compare commits\n>> on different branches\".\n> \n> Yes\n[...]\n>> But after some thinking I guess that your idea has some merit\n>> _provided_ that \"(base | diff)\" (by the way, what would happen\n>> if someone would click \"diff\" without setting \"base\" first? is\n>> \"diff\" inactive, or is some \"base\" default?) is added _only_ when\n> \n> In my JavaScript solution, the user gets a message box in this case:\n> \"no base object selected\".\n\nThis is a bad idea from UI point of view. Users don't like message boxes,\nunless really, really necessary.\n\nWe can either do like MediaWiki engine does, i.e. have default base, be\nit current version (tip of shortlog, history) or the previous (parent,\nor rather first simplified parent) version. Or we can make \"diff\" part\nof \"(base | diff)\" inactive and grayed out till base is selected, which\nshould be fairly easy in JavaScript.\n \n>> web browser supports it. It means that it should appear only when\n>> JavaScript is turned on (easy if links are added by JavaScript)\n>> and when cookies are turned on (I'm not sure if you are checking\n>> this).\n> \n> No, there is no check at the moment. Buts this is basically checking\n> the navigator.cookieEnabled property. A user could only reject some\n> cookies, but catching this would probably be too complicated.\n\nPlease make it so.\n\n>> I guess that we could (under control of gitweb configuration, %feature\n>> hash or something like that) add links which would lead to server\n>> setting a cookie, or adding unused CGI parameters ('hp', 'hpb' and\n>> 'fp' are used only by 'diff' views).\n[...]\n\n>>> If depending on cookies is a problem for some out there, the base\n>>> object can be passed in the URLs as fallback. It would require, that\n>>> the select base object is included in every link, which gitweb\n>>> generates.\n>> \n>> Not much of a problem. Generation of links is consolidated in href()\n>> subroutine, and it would be fairly easy to add passing through base\n>> parameters, if they are needed or not. It means that if 'shortlog' view\n>> was passed 'hp' parameter then \"diff\" links (or \"diff \" links,\n>> or \"diff sel\"/\"diff selected\" links) would lead to diff with given base.\n> \n>> First, generating gitweb links in gitweb _all_ goes through href()\n>> subroutine, so it would be fairly easy to implement it server side,\n>> i.e. directly in gitweb.perl. \n> \n> This has on complication: href only generats the url. The link is\n> generated in the caller. We would need a new wrapper for this.\n> \n> [...]\n\nI was thinking about the following solution: by default \"base\" part\nof server-side plain HTML \"(base | diff)\" would lead to current URL\nbut with 'hp' and if necessary also 'hpb', 'fp' CGI parameters added,\n(e.g. if current URL is\n p=project.git;a=shortlog;h=next\nthen \"base\" link somewhere deeper in the \"shortlog\" would be:\n p=project.git;a=shortlog;h=next;hp=\ni.e. with appropriate 'hb' param added). This of course would mean\nthat \"base\" link would mean additional request for the server. And\nnewly served page would have modified \"diff\" part of \"(base | diff)\"\nto have just set base as base.\n\nIf non-diff view has diff-related ('hp', 'hpb', 'fp') parameters set,\nthey are passed to \"diff\" part of \"(base | diff)\" links; if they\nare not set, \"diff\" part is inactive (unless there is some default).\n\nOf course when we finally view requested diff, the selected base is\nlost... unless we save it in a cookie, server-side.\n\n>>> Currently, only few combinations are implemented. Future version could\n>>> integrate tags (as an tag links to one commit). Additionally,\n>>> comparing tags/commits with a tree could be enabled, as a tag/commit\n>>> link to one tree.\n>> \n>> Diffs makes sense only between two commit-ish (commit-ish being commit\n>> or a tag; head also denotes a commit) for which we need 'h' and 'hp'\n>> params, between two tree-ish (tree-ish being tree, commit or a tag)\n>> for which we need 'h'/'hb' and 'hp'/'hpb' and usually 'f', sometimes\n> \n> For tree diffs, I currently only support h/hp. Using hb/hbp and f/fp\n> would require, that both trees share the same file name.\n\nNot true. You can use extended sha1 syntax to specify trees by\nrevision (commit/base hash) and directory name (path): see \n\"SPECIFYING REVISIONS\" section in git-rev-parse(1):\n\n * A suffix : followed by a path; this names the blob or tree at the given\n path in the tree-ish object named by the part before the colon.\n\nSo if 'h=' is given, we use it's value; if only \n'hb=' and 'f=' are given, use\n: to specify tree. Not hard.\n\n>> 'fp', and finnally between two blobs, for which we need 'h'/'hb' and\n>> 'hp'/'hpb' and usually 'f', sometimes 'fp'.\n> \n> For blob diffs, I prefer to use only h/hp (and f/fp for showing nice\n> file names). I fall back to hb/hpb and f for the blobs in the history\n> views (where h is not available), which has the drawback, that both\n> blobs must have the same name.\n\nI think it works. If I remember correctly in commitdiff and commit views,\nahen file modification is detected as rename the blobdiff link is\ngenerated with both 'h' (for blob hash) and 'hb' (for commit hash),\nand 'hp' and 'hbp', and with 'f' and 'fp'; and it just works.\n\n>> By the way, I'm not that sure if gitweb handle correctly request for\n>> diff of two _different_ tree-ish, or two arbitrary different files.\n> \n> If you use not hb/hbp, this works.\n\nSee above.\n\n>>> 1) introduce git_treediff\n>> \n>> NOTE: git_treediff is in some parts very similar (I think) to\n>> git_commitdiff with the exception of not being there commit message,\n>> nor links to parents, and I'd like very much to avoid code repetition\n>> (duplication) betweem git_treediff and git_commitdiff if possible.\n>> On the other hand git_treediff is similar to git_blob in handling\n>> its parameters ('h', 'hp' but also 'hb', 'hpb' and 'f', 'fp').\n> \n> I created git_treediff out of git_commitdiff.\n\nPerhaps some code could be shared, i.e. separated into common subroutine,\nor git_commitdiff could be generalized to take also trees?\n\n>>> For me, this would be no problem, but for high load servers (eg. gitweb at\n>>> kernel.org), it would incread the system load.\n>> \n>> I think use JavaScript if possible, and server-side implementation\n>> if not would be good compromise.\n> \n> I'll try to implement a solution for this.\n\nThanks in advance.\n\n>> I'm just wondering why did you not used git for development. You have\n>> to have git installed to test gitweb, and it would be best to have\n>> git.git repository fetched to base your work on newest 'master' version\n>> (or rather 'origin/master' or 'origin' version). So it would be natural\n>> to use git to work on gitweb, and to send git patches.\n> \n> gitweb.cgi is generated out of gitweb.perl. Then I need to manually\n> change some parameters in it and put it on a webserver. For\n> developing, it's easier for me to edit the cgi-script on the\n> server. In a diff of gitweb.cgi, I have to remove the unrelated\n> changes, for eg. setting paths parameters.\n> \n> I would be interessted, how git can help me in this workflow?\n\nThere are two solutions. First is to use script to generate gitweb.cgi\nout of (just modified) gitweb.perl, setting the parameters while at\nit, and install it. See attached gitweb-update.sh which I use for this.\nAnother would be to use config file to set appropriate parameters;\nsee attached gitweb_config.perl which I use for similar purposes.\n\n>>> @@ -1835,6 +1837,7 @@\n>>> \t\tclose $fd;\n>>> \t}\n>>> \n>>> +\tprint '';\n>>> \tprint \"\\n\" .\n>>> \t \"\";\n>>> }\n>> \n>> I'd rather have end of line after closing '', and have scripts\n>> in the head section, not in body.\n> \n> For my prototype, it was easier put the script at the end. Putting it\n> in the header would require, that I call the JavaScript load function\n> in the body onload event. In the end version, I'll change that.\n\nI can understand this, but I think patches _for the inclusion_ should\nhave scripts included in the header.\n\n[...]\n> Do git-diff-tree offers an option to compare in two trees two\n> different file names? If not, using hb would require, that both trees\n> have the same name. In this case, as all tree links seems to have h, I\n> see no use for hb/hpb.\n\nSee above. You can use hb:f and hpb:fp (or hpb:f if fp is not set,\nmeaning fp=f).\n\n-- \nJakub Narebski\nPoland\n\n\n#!/bin/bash\n\n#BASEDIR=\"/home/jnareb/\"\n\n#BINDIR=\"/usr/bin\"\nBINDIR=\"/home/local/git\"\n\nfunction make_gitweb()\n{\n\tpushd \"/home/jnareb/git/\"\n\n\tmake GITWEB_PROJECTROOT=\"/home/local/scm\" \\\n\t GITWEB_CSS=\"/gitweb/gitweb.css\" \\\n\t GITWEB_LOGO=\"/gitweb/git-logo.png\" \\\n\t GITWEB_FAVICON=\"/gitweb/git-favicon.png\" \\\n\t bindir=$BINDIR \\\n\t gitweb/gitweb.cgi\n\n\tpopd\n}\n\nfunction copy_gitweb()\n{\n\tcp -fv /home/jnareb/git/gitweb/gitweb.{cgi,css} /home/local/gitweb/\n}\n\nmake_gitweb\ncopy_gitweb\n\n# end of gitweb-update.sh\n\n\n#!/usr/bin/perl\n\n# gitweb configuration\n\nour $version = \"current\";\n#our $GIT = \"/usr/bin/git\";\nour $GIT = \"/home/local/git/git\";\nour $projectroot = \"/home/local/scm\";\nour $home_link_str = \"projects\";\nour $site_name = \"[localhost]\";\nour $site_header = \"\";\nour $site_footer = \"\";\nour $home_text = \"indextext.html\";\nour @stylesheets = (\"file:///home/jnareb/git/gitweb/gitweb.css\");\nour $logo = \"file:///home/jnareb/git/gitweb/git-logo.png\";\nour $favicon = \"file:///home/jnareb/git/gitweb/git-favicon.png\";\nour $projects_list = \"\";\nour $export_ok = \"\";\nour $strict_export = \"\";\nour @git_base_url_list = (\"/home/jnareb/git\");\n"}]}