From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Narebski Subject: [RFC/PATCH 4/3] gitweb: Incremental blame (proof of concept) Date: Wed, 10 Dec 2008 21:11:18 +0100 Message-ID: <20081210200908.16899.36727.stgit@localhost.localdomain> References: <20081209223703.28106.29198.stgit@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: Luben Tuikov , Nanako Shiraishi , Petr Baudis , Fredrik Kuivinen , Fredrik Kuivinen , Petr Baudis , Jakub Narebski To: git@vger.kernel.org X-From: git-owner@vger.kernel.org Wed Dec 10 21:13:06 2008 Return-path: Envelope-to: gcvg-git-2@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1LAVQV-0008Or-QH for gcvg-git-2@gmane.org; Wed, 10 Dec 2008 21:12:56 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755453AbYLJULh (ORCPT ); Wed, 10 Dec 2008 15:11:37 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755452AbYLJULh (ORCPT ); Wed, 10 Dec 2008 15:11:37 -0500 Received: from ti-out-0910.google.com ([209.85.142.191]:55117 "EHLO ti-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752794AbYLJULe (ORCPT ); Wed, 10 Dec 2008 15:11:34 -0500 Received: by ti-out-0910.google.com with SMTP id b6so419810tic.23 for ; Wed, 10 Dec 2008 12:11:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:from:subject:to:cc :date:message-id:in-reply-to:references:user-agent:mime-version :content-type:content-transfer-encoding; bh=VNLL69WKv+BhA1x06tl5Cp4mqpQHogIQwQ2LsuoeX+Q=; b=I8XbNDX9AJqc70V6hA7t6aKqQ+fug/qeBC/6WFnvt59JQ/5e+C4UUiYj3pqQF325iz FKp3aiKaXNL5aDDSy+LlJbjjNMknguk4ej2cyL1xGq47hi8mRKSIB4bRkQnijegXlUuI O9+S1Kw6/hQ55IvkcyGGJBzw3aoZc4LWcOn7k= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:subject:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-type:content-transfer-encoding; b=iRy0TgN12jBSiEr5kuAt9b8NvZdib+XZSw/pv79Q8AkmxxeQjNC9pqCaelsnOSef/g +UOtXyhd2DhG2A9DS5MOA+HiEcq7pfyaetd6nR9XpfqY7CHPzPlgbij+Yr5nUcTbfx/I Kebrto9b+F9PoS7MnbrDgxS/JkMNBDoUmn/0Q= Received: by 10.110.42.17 with SMTP id p17mr2505422tip.42.1228939891980; Wed, 10 Dec 2008 12:11:31 -0800 (PST) Received: from localhost.localdomain (abxf60.neoplus.adsl.tpnet.pl [83.8.255.60]) by mx.google.com with ESMTPS id 25sm2357904tif.6.2008.12.10.12.11.23 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 10 Dec 2008 12:11:28 -0800 (PST) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by localhost.localdomain (8.13.4/8.13.4) with ESMTP id mBAKBIYt016943; Wed, 10 Dec 2008 21:11:19 +0100 In-Reply-To: <20081209223703.28106.29198.stgit@localhost.localdomain> User-Agent: StGIT/0.14.3 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: This is tweaked up version of Petr Baudis patch, which in turn was tweaked up version of Fredrik Kuivinen 's proof of concept patch. It adds 'blame_incremental' view, which incrementally displays line data in blame view using JavaScript (AJAX). The original patch by Fredrik Kuivinen has been lightly tested in a couple of browsers (Firefox, Mozilla, Konqueror, Galeon, Opera and IE6). The next patch by Petr Baudis has been tested in Firefox and Epiphany. This patch has been lightly tested in Mozilla 1.17.2 and Konqueror 3.5.3. This patch does not (contrary to the one by Petr Baudis) enable this view in gitweb: there are no links leading to 'blame_incremental' action. You would have to generate URL 'by hand' (e.g. changing 'blame' or 'blob' in gitweb URL to 'blame_incremental'). Having links in gitweb lead to this new action (e.g. by rewriting them like in previous patch), if JavaScript is enabled in browser, is left for later. Like earlier patch by Per Baudis it avoids code duplication, but it goes one step further and use git_blame_common for ordinary blame view, for incremental blame, and for incremental blame data. How the 'blame_incremental' view works: * gitweb generates initial info by putting file contents (from git-cat-file) together with line numbers in blame table * then gitweb makes web browser JavaScript engine call startBlame() function from blame.js * startBlame() opens connection to 'blame_data' view, which in turn calls "git blame --incremental" for a file, and streams output of git-blame to JavaScript (blame.js) * blame.js updates line info in blame view, coloring it, and updating progress info; note that it has to use 3 colors to ensure that different neighbour groups have different styles * when 'blame_data' ends, and blame.js finishes updating line info, it fixes colors to match (as far as possible) ordinary 'blame' view, and updates generating time info. This code uses http://ajaxpatterns.org/HTTP_Streaming pattern. It deals with streamed 'blame_data' server error by notifying about them in the progress info area (just in case). This patch adds GITWEB_BLAMEJS compile configuration option, and modifies git-instaweb.sh to take blame.js into account, but it does not update gitweb/README file (as it is only proof of concept patch). The code for git-instaweb.sh was taken from Pasky's patch. While at it, this patch uniquifies td.dark and td.dark2 style: they differ only in that td.dark2 doesn't have style for :hover. This patch also adds showing time (in seconds) it took to generate a page in page footer (based on example code by Pasky), even though it is independent change, to be able to evaluate incremental blame in gitweb better. In proper patch series it would be independent commit; and it probably would provide fallback if Time::HiRes is not available (by e.g. not showing generating time info), even though this is unlikely. Cc: Fredrik Kuivinen Signed-off-by: Petr Baudis Signed-off-by: Jakub Narebski --- I'm sorry if you have received duplicate copy of this message... NOTE: This patch is RFC proof of concept patch!: it should be split onto many smaller patches for easy review (and bug finding) in version meant to be applied. [Please tell me if some of info below should be put in the commit message instead of here] Patch by Petr Baudis this one is based on: http://permalink.gmane.org/gmane.comp.version-control.git/56657 Original patch by Fredrik Kuivinen: http://article.gmane.org/gmane.comp.version-control.git/41361 Snippet adding 'generated in' to gitweb, by Petr Baudis: http://article.gmane.org/gmane.comp.version-control.git/83306 Should I post interdiff to Petr Baudis patch, and comments about difference between them? This post is quite long as it is now... Differences between 'blame' and 'blame_incremental' output: * Line number links in 'blame' lead to parent of blamed commit, i.e. to the view where given line _changed_; this behavior was introduced in 244a70e (Blame "linenr" link jumps to previous state at "orig_lineno") by Luben Tuikov on Jan 2008 to make data mining possible. Currently 'blame_incremental' lead to version at blamed commit; this would be hard to change without opening another stream, or without having gitweb accept ^ for 'hb' (hash_base) parameter. * The title attribute (shown in popup on mouseover) for "sha1" cell for 'blame_incremental' view looks like the following: 'Kay Sievers, 2005-08-07 19:49:46 +0000 (21:49:46 +0200)' more like the date format used in 'commit' view, rather than shorter 'Kay Sievers, 2005-08-07 21:49:46 +0200' This is a bit of accident, as in originla patch(es) there was error where the time and date shown were for UTC (GMT), and not for shown together timezone, i.e. 'Kay Sievers, 2005-08-07 19:49:46 +0200' and I have added local time instead of replacing it. But perhaps it is 'blame' view that should change format? * In 'blame_incremental' the cell () with sha1 has rowspan attribute set even if it is at its default value rowspan="1". This should be very easy to fix. * The 'blame_incremental' view has new feature inspired by output of "git gui blame ", that if group of lines blamed to the same commit has more than two lines, then below shortened sha-1 of a commit it is shown shortened author (initials, in uppercase). If you think it is worth it, this feature should be fairly easy to port to ordinary non-incremental 'blame' view. * Sometimes "git blame --porcelain" and "git blame --incremental" can give different output. Compare for example 'blame' and 'blame_incremental' view for GIT-VERSION-GEN in git.git repository, and note that in 'blame_incremental' the last two groups are for the same commit (compare bottom parts of pages). 'blame_incremental' currently does not consolidate groups; if it did that, it should do it before fixColors() * During filling blame info 'blame_incremental' view uses three colors (three styles: color1, color2, color3) instead of two color zebra coloring (two styles: light2, dark2) to ensure that the color of current group is different from both of its neighbours. This is non-issue: this is fixed at the end (although intermediate versions of blame.js script didn't fo fixColors() to make it easier to check if the 3-coloring is correct). * The 'blame_incremental' view contains unique progress bar and progress report; perhaps they should vanish after succesfull loading of all data? Bugs and suspected bugs in Mozilla 1.17.2 (my main browser); perhaps those got corrected, as 1.17.2 is ancient (Gecko/20050923) version: * HTML 4.01 Transitional specification at W3C states only ID and NAME tokens must begin with a letter ([A-Za-z]); it looks like class named "1" or "2" or "3" has style specified by CSS ignored. * With XHTML 1.0 DTD and application/xhtml+xml content type, if there were elements in blame table (currently commented out in the source), then row with column headings (with elements) was not visible. * With XHTML 1.0 DTD and application/xhtml+xml content type, if there was an error in JavaScript, instead of having it visible as error message in JavaScript Console, Mozilla behaved as if the script wasn't there at all. * With XHTML 1.0 DTD and application/xhtml+xml content type, setting innerHTML doesn't work: it raises cryptic JavaScript exception: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMNSHTMLElement.innerHTML] Correct solution would be to use only DOM, or rather depending on what web browser supports, use either .innerHTML (which is proprietary Microsoft extension) or DOM (which is standard but not all browser use it). Current *workaround* is to simply always use 'text/html' content type, and HTML 4.01 DTD. I wonder whether innerHTML or DOM is faster; and how many of web browser implements other similar properties like innerText, outerHTML and insertAdjacentHTML. * XHTML 1.0 doesn't allow for trick with using HTML comments to hide contents of \n!. + qq!\n!; + } + git_footer_html(); } +sub git_blame { + git_blame_common(); +} + +sub git_blame_incremental { + git_blame_common('incremental'); +} + +sub git_blame_data { + git_blame_common('data'); +} + sub git_tags { my $head = git_get_head_hash($project); git_header_html(); -- Stacked GIT 0.14.3 git version 1.6.0.4