Jakub Narebski <jnareb@xxxxxxxxx> writes: > This is tweaked up version of Petr Baudis <pasky@xxxxxxx> patch, which > in turn was tweaked up version of Fredrik Kuivinen <frekui@xxxxxxxxx>'s > proof of concept patch. It adds 'blame_incremental' view, which > incrementally displays line data in blame view using JavaScript (AJAX). [...] > 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? [...] Here is the list of differences between Petr Baudis patch and the one I have just send. No interdiff, as it is artificially large because previous patch was based on much older version, so ranges does not match. Bugs I have made: * I forgot to make some changes for git-instaweb.sh to have support for incremental blame, namely dependency of 'git-instaweb' target in Makefile on gitweb/blame.js, and lack of the following line in git-instaweb.sh: gitweb_blamejs $GIT_DIR/gitweb/blame.js * Pasky's patch added support for href(...,-partial_query=>1) extra parameter, which ensured that gitweb link had '?' in it, and used it to generate 'baseUrl' parameter for startBlame. I have misunderstood what baseUrl is about, and used $my_url there, while it is partial URL for blame links: it is projectUrl. Therefore links in blame table current 'blame_incremental' would not work. I'm sorry about that, I thought I have checked it... Intentionally omitted features: * In patch this one is based on there was fixBlameLinks() JavaScript function (put directly in the HTML head inside <script> element), which was used in body.onLoad event to change 'a=blame' to 'a=blame_incremental' in links marked with class="blamelink". First, this IMHO should be put as separate patch; you can test 'blame_incremental' view by hand-crafting gitweb URL. Second, it would be not enough in current gitweb, as action can be put in path_info. So either fixBlameLinks() should be made work in both cases, or it should be done in different way, for example adding 'js=1' for all links, or doing JavaScript redirect from 'blame' view (although this way we won't be able to view ordinary 'blame' view without turning off JavaScript). Differences in coding of the same features: * In Pasky's patch git_blame (then named git_blame2) and git_blame_incremental were just wrappers around git_blame_common; in this patch git_blame_data is also wrapper (to avoid duplicating permissions and parameter checking code). * Instead of calculating title string for "Commit" column cell, and necessary data for each row, we now calculate it once per commit and save (cache) in 'commits' associative array (hash). This is a bit of performance improvement, similar to the one in "[PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()" for 'blame' view in gitweb. It is just using Date() and string concatenation, and not extra fork. * Variables holding manipulated elements are named a bit differently, and calculated upfront: td_sha1 instead of tr.firstChild a_sha1 instead of ahsAnchor a_linenr instead of lineAnchor * There were a few of style changes in gitweb/blame.js; for example it is used the following style of function definition function functionName(arg1, arg2) { thorough the code. Fixes for bugs in Pasky's patch, and changes related to changes in ordinary 'blame' output: * handleResponse function was called only from XMLHttpRequest onreadystatechange event. Not all browsers call onreadystatechange each time the server flushes output (Firefox does that), so we use a timer to check (poll) for changes. See http://ajaxpatterns.org/HTTP_Streaming#Refactoring_Illustration * The 'linenr' link was to line number commit.srcline, while it should be to (commit.srcline + i), as commit.srcline is _start_ line in group, and not the line equivalent to current line in source. This might be thought a (mis)feature, and not a bug, though. * Currently 'blame' view uses single cell (with rowspan="N") spanning the whole group of lines blaming the same commit, instead of using empty cells for subsequent lines in group. Therefore instead of setting "shaAnchor.innerHTML = '';" to make subsequent cells in "Commit" ('sha1') column empty (or rather to make link element <a> in cell empty), we use "tr.removeChild(td_sha1);" This change was made necessary by changes in the 'blame' view. This also meant that the code checking if lines are in the same group has to be changes; it was refactored into startOfGroup(tr) function. * The title for cells in "Commit" column used UTC (GMT) date and time 'Kay Sievers, 2005-08-07 19:49:46' instead of the localtime format used currently by 'blame' view: 'Kay Sievers, 2005-08-07 21:49:46 +0200' Current code uses neither, but 'commit'-like format: 'Kay Sievers, 2005-08-07 19:49:46 +0000 (21:49:46 +0200)' New features (in short): * 3-coloring of lines with blame data during incremental blaming * Adding author initials below shortened SHA-1 of a commit (if there is place for it, i.e. if group has more than 1 row) * progress indicator: progress info and progress bar * information about how long it took to run 'blame_data', and how long it took to run JavaScript script -- Jakub Narebski Poland ShadeHawk on #git -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
