* [PATCHv5 2/5] gitweb: Incremental blame (using JavaScript)
From: Jakub Narebski @ 2009-09-01 11:39 UTC (permalink / raw)
To: git
Cc: Petr Baudis, Fredrik Kuivinen, Giuseppe Bilotta, Luben Tuikov,
Martin Koegler, Jakub Narebski
In-Reply-To: <1251805160-5303-2-git-send-email-jnareb@gmail.com>
Add 'blame_incremental' view, which uses "git blame --incremental"
and JavaScript (Ajax), where 'blame' use "git blame --porcelain".
* 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 gitweb.js
* startBlame() opens XMLHttpRequest connection to 'blame_data' view,
which in turn calls "git blame --incremental" for a file, and
streams output of git-blame to JavaScript (gitweb.js)
* XMLHttpRequest event handler updates line info in blame view as soon
as it gets data from 'blame_data' (from server), and it also updates
progress info
* when 'blame_data' ends, and gitweb.js finishes updating line info,
it fixes colors to match (as far as possible) ordinary 'blame' view,
and updates information about how long it took to generate page.
Gitweb deals with streamed 'blame_data' server errors by displaying
them in the progress info area (just in case).
The 'blame_incremental' view tries to be equivalent to 'blame' action;
there are however a few differences in output between 'blame' and
'blame_incremental' view:
* 'blame_incremental' always used query form for this part of link(s)
which is generated by JavaScript code. The difference is visible
if we use path_info link (pass some or all arguments in path_info).
Changing this would require implementing something akin to href()
subroutine from gitweb.perl in JavaScript (in gitweb.js).
* 'blame_incremental' always uses "rowspan" attribute, even if
rowspan="1". This simplifies code, and is not visible to user.
* The progress bar and progress info are still there even after
JavaScript part of 'blame_incremental' finishes work.
Note that currently no link generated by gitweb leads to this new
view.
This code is based on patch by Petr Baudis <pasky@suse.cz> patch, which
in turn was tweaked up version of Fredrik Kuivinen <frekui@gmail.com>'s
proof of concept patch.
This patch adds GITWEB_JS compile configuration option, and modifies
git-instaweb.sh to take gitweb.js into account. The code for
git-instaweb.sh was taken from Pasky's patch.
Signed-off-by: Fredrik Kuivinen <frekui@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
References:
1. Original patch by Frederik Kuivinen
http://article.gmane.org/gmane.comp.version-control.git/41361
2. Tweaked up version by Petr Baudis
http://article.gmane.org/gmane.comp.version-control.git/47614
http://article.gmane.org/gmane.comp.version-control.git/56657
3. New link rewriting and some optimization in Matrin Koegler
series introducing some JavaScript support in Git
http://thread.gmane.org/gmane.comp.version-control.git/47902/focus=47905
4. My earlier patches
http://thread.gmane.org/gmane.comp.version-control.git/102657/focus=102712
http://article.gmane.org/gmane.comp.version-control.git/123202
http://thread.gmane.org/gmane.comp.version-control.git/123957/focus=123968
http://thread.gmane.org/gmane.comp.version-control.git/125096/focus=125717
Changes compared to last version (v4):
* The file with JavaScript code for 'blame_incremental' got renamed
from 'blame.js' to 'gitweb.js' -- it is now meant to contain all
JavaScript code, as per Google and Yahoo! guidelines (there should
be one file with JavaScript code, to be cached, as JavaScript
loading is blocking).
* Move coloring rows duting 'blame_data' run to a separate commit.
* The debug(str) function and its use (even commented out) was removed;
the code is now meant to be in production, and should have leftover
debugging statements.
* spacePad(input, width) function, which pads with ' ' got
replaced by more generic padLeftStr(input, width, padstr) function.
* Added information about global variables used by function via
@globals annotation (non in JSDoc standard).
* Make prevDataLength and nextReadPos properties (fields) of
XMLHttpRequest object, instead of being global variables.
* Pass xhr as parameter to handleError and responseLoaded functions
(it shadows xhr as a global variable).
* Move setting onreadystatechange handler before xhr.open()
* Add information about GITWEB_JS Makefile configuration variable to
gitweb/README
* Remove unnecessary 'reminder' comments in gitweb.js
TODO for future commits:
* Use W3C Progress Events (progress, error, load) in addition to
currently used readystatechange event
http://www.w3.org/TR/progress-events/
http://www.w3.org/TR/XMLHttpRequest2/
http://www.nczonline.net/blog/2009/07/09/firefox-35firebug-xmlhttprequest-and-readystatechange-bug/
if XMLHttpResponse supports it.
* handleResponse is used both as onreadystatechange and pollTimer;
if onreadystatechange works for partial responses we can turn off
the timer.
* JavaScript is single theraded, therefore there is no need for
critical section; remove inProgress global variable (flag), and
busy-read while loop.
* Remove (fade out) progress bar and progress info after blame
incremental finished run, if possible with large amount of code to
deal with browser incompatibilities.
TODO and possible extensions:
* Profile gitweb.js using YUI Profiler (or other JavaScript profile tool)
http://developer.yahoo.com/yui/profiler/
to check whether performance improvements described in articles
on NCZOnline blog by Nicholas C. Zakas are required, and would help
http://www.nczonline.net/blog/
* Get rid of global variables, if possible.
* Instead of running startBlame, put it in window.onload handler.
Roads not taken (perhaps that should be part of commit message?):
* Move most (or all) of "git blame --incremental" output parsing to
server side, and instead of sending direct output in text/plain,
send processed data in JSON format, e.g.
{"commit": {
"sha1": "e83c5163316f89bfbde7d9ab23ca2e25604af290",
"info": "Kay Sievers, 2005-08-07 21:49:46 +0200",
"author-initials": "KS",
...
},
"src-line": 13,
"dst-line": 16,
"numlines": 3,
"filename": "README"
}
(line wrapping added for readibility). This would require however
taking care on Perl side to send properly formatted JSON, and on
JavaScript side including json2.js code to read JSON in gitweb.js
(unless we rely on eval).
* Using some lightweight JavaScript library (framework), like jQuery,
Prototype, ExtJS, MooTools, etc. One one hand side this means not
having to worry about browser incompatibilities as this would be
taken care of by library; on the other hand side we want gitweb to
have as few dependences as possible.
* Use 'multipart/x-mixed-replace': first it is not standard, second
I don't think this would be a good solution for our problem.
Makefile | 6 +-
git-instaweb.sh | 7 +
gitweb/README | 4 +
gitweb/gitweb.css | 11 +
gitweb/gitweb.js | 726 ++++++++++++++++++++++++++++++++++++++++++++++++++++
gitweb/gitweb.perl | 272 ++++++++++++++------
6 files changed, 939 insertions(+), 87 deletions(-)
create mode 100644 gitweb/gitweb.js
diff --git a/Makefile b/Makefile
index a614347..407b35c 100644
--- a/Makefile
+++ b/Makefile
@@ -261,6 +261,7 @@ GITWEB_HOMETEXT = indextext.html
GITWEB_CSS = gitweb.css
GITWEB_LOGO = git-logo.png
GITWEB_FAVICON = git-favicon.png
+GITWEB_JS = gitweb.js
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
@@ -1393,13 +1394,14 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
-e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
-e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
-e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
+ -e 's|++GITWEB_JS++|$(GITWEB_JS)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
$< >$@+ && \
chmod +x $@+ && \
mv $@+ $@
-git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
+git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css gitweb/gitweb.js
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -1408,6 +1410,8 @@ git-instaweb: git-instaweb.sh gitweb/gitweb.cgi gitweb/gitweb.css
-e '/@@GITWEB_CGI@@/d' \
-e '/@@GITWEB_CSS@@/r gitweb/gitweb.css' \
-e '/@@GITWEB_CSS@@/d' \
+ -e '/@@GITWEB_JS@@/r gitweb/gitweb.js' \
+ -e '/@@GITWEB_JS@@/d' \
-e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \
$@.sh > $@+ && \
chmod +x $@+ && \
diff --git a/git-instaweb.sh b/git-instaweb.sh
index d96eddb..701c0d7 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -375,8 +375,15 @@ gitweb_css () {
EOFGITWEB
}
+gitweb_js () {
+ cat > "$1" <<\EOFGITWEB
+@@GITWEB_JS@@
+EOFGITWEB
+}
+
gitweb_cgi "$GIT_DIR/gitweb/gitweb.cgi"
gitweb_css "$GIT_DIR/gitweb/gitweb.css"
+gitweb_js "$GIT_DIR/gitweb/gitweb.js"
case "$httpd" in
*lighttpd*)
diff --git a/gitweb/README b/gitweb/README
index 66c6a93..b69b0e5 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -92,6 +92,10 @@ You can specify the following configuration variables when building GIT:
web browsers that support favicons (website icons) may display them
in the browser's URL bar and next to site name in bookmarks). Relative
to base URI of gitweb. [Default: git-favicon.png]
+ * GITWEB_JS
+ Points to the localtion where you put gitweb.js on your web server
+ (or to be more generic URI of JavaScript code used by gitweb).
+ Relative to base URI of gitweb. [Default: gitweb.js]
* GITWEB_CONFIG
This Perl file will be loaded using 'do' and can be used to override any
of the options above as well as some other options -- see the "Runtime
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 00e2e4c..69ef119 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -353,6 +353,17 @@ td.mode {
font-family: monospace;
}
+/* progress of blame_interactive */
+div#progress_bar {
+ height: 2px;
+ margin-bottom: -2px;
+ background-color: #d8d9d0;
+}
+div#progress_info {
+ float: right;
+ text-align: right;
+}
+
/* styling of diffs (patchsets): commitdiff and blobdiff views */
div.diff.header,
div.diff.extended_header {
diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
new file mode 100644
index 0000000..c8411e7
--- /dev/null
+++ b/gitweb/gitweb.js
@@ -0,0 +1,726 @@
+// Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+// 2007, Petr Baudis <pasky@suse.cz>
+// 2008-2009, Jakub Narebski <jnareb@gmail.com>
+
+/**
+ * @fileOverview JavaScript code for gitweb (git web interface).
+ * @license GPLv2 or later
+ */
+
+/*
+ * This code uses DOM methods instead of (nonstandard) innerHTML
+ * to modify page.
+ *
+ * innerHTML is non-standard IE extension, though supported by most
+ * browsers; however Firefox up to version 1.5 didn't implement it in
+ * a strict mode (application/xml+xhtml mimetype).
+ *
+ * Also my simple benchmarks show that using elem.firstChild.data =
+ * 'content' is slightly faster than elem.innerHTML = 'content'. It
+ * is however more fragile (text element fragment must exists), and
+ * less feature-rich (we cannot add HTML).
+ *
+ * Note that DOM 2 HTML is preferred over generic DOM 2 Core; the
+ * equivalent using DOM 2 Core is usually shown in comments.
+ */
+
+
+/* ============================================================ */
+/* generic utility functions */
+
+
+/**
+ * pad number N with nonbreakable spaces on the left, to WIDTH characters
+ * example: padLeftStr(12, 3, ' ') == ' 12'
+ * (' ' is nonbreakable space)
+ *
+ * @param {Number|String} input: number to pad
+ * @param {Number} width: visible width of output
+ * @param {String} str: string to prefix to string, e.g. ' '
+ * @returns {String} INPUT prefixed with (WIDTH - INPUT.length) x STR
+ */
+function padLeftStr(input, width, str) {
+ var prefix = '';
+
+ width -= input.toString().length;
+ while (width > 1) {
+ prefix += str;
+ width--;
+ }
+ return prefix + input;
+}
+
+/**
+ * Pad INPUT on the left to SIZE width, using given padding character CH,
+ * for example padLeft('a', 3, '_') is '__a'.
+ *
+ * @param {String} input: input value converted to string.
+ * @param {Number} width: desired length of output.
+ * @param {String} ch: single character to prefix to string.
+ *
+ * @returns {String} Modified string, at least SIZE length.
+ */
+function padLeft(input, width, ch) {
+ var s = input + "";
+ while (s.length < width) {
+ s = ch + s;
+ }
+ return s;
+}
+
+/**
+ * Create XMLHttpRequest object in cross-browser way
+ * @returns XMLHttpRequest object, or null
+ */
+function createRequestObject() {
+ try {
+ return new XMLHttpRequest();
+ } catch (e) {}
+ try {
+ return window.createRequest();
+ } catch (e) {}
+ try {
+ return new ActiveXObject("Msxml2.XMLHTTP");
+ } catch (e) {}
+ try {
+ return new ActiveXObject("Microsoft.XMLHTTP");
+ } catch (e) {}
+
+ return null;
+}
+
+/* ============================================================ */
+/* utility/helper functions (and variables) */
+
+var xhr; // XMLHttpRequest object
+var projectUrl; // partial query + separator ('?' or ';')
+
+// 'commits' is an associative map. It maps SHA1s to Commit objects.
+var commits = {};
+
+/**
+ * constructor for Commit objects, used in 'blame'
+ * @class Represents a blamed commit
+ * @param {String} sha1: SHA-1 identifier of a commit
+ */
+function Commit(sha1) {
+ if (this instanceof Commit) {
+ this.sha1 = sha1;
+ this.nprevious = 0; /* number of 'previous', effective parents */
+ } else {
+ return new Commit(sha1);
+ }
+}
+
+/* ............................................................ */
+/* progress info, timing, error reporting */
+
+var blamedLines = 0;
+var totalLines = '???';
+var div_progress_bar;
+var div_progress_info;
+
+/**
+ * Detects how many lines does a blamed file have,
+ * This information is used in progress info
+ *
+ * @returns {Number|String} Number of lines in file, or string '...'
+ */
+function countLines() {
+ var table =
+ document.getElementById('blame_table') ||
+ document.getElementsByTagName('table')[0];
+
+ if (table) {
+ return table.getElementsByTagName('tr').length - 1; // for header
+ } else {
+ return '...';
+ }
+}
+
+/**
+ * update progress info and length (width) of progress bar
+ *
+ * @globals div_progress_info, div_progress_bar, blamedLines, totalLines
+ */
+function updateProgressInfo() {
+ if (!div_progress_info) {
+ div_progress_info = document.getElementById('progress_info');
+ }
+ if (!div_progress_bar) {
+ div_progress_bar = document.getElementById('progress_bar');
+ }
+ if (!div_progress_info && !div_progress_bar) {
+ return;
+ }
+
+ var percentage = Math.floor(100.0*blamedLines/totalLines);
+
+ if (div_progress_info) {
+ div_progress_info.firstChild.data = blamedLines + ' / ' + totalLines +
+ ' (' + padLeftStr(percentage, 3, ' ') + '%)';
+ }
+
+ if (div_progress_bar) {
+ //div_progress_bar.setAttribute('style', 'width: '+percentage+'%;');
+ div_progress_bar.style.width = percentage + '%';
+ }
+}
+
+
+var t_interval_server = '';
+var cmds_server = '';
+var t0 = new Date();
+
+/**
+ * write how much it took to generate data, and to run script
+ *
+ * @globals t0, t_interval_server, cmds_server
+ */
+function writeTimeInterval() {
+ var info_time = document.getElementById('generating_time');
+ if (!info_time || !t_interval_server) {
+ return;
+ }
+ var t1 = new Date();
+ info_time.firstChild.data += ' + (' +
+ t_interval_server + ' sec server blame_data / ' +
+ (t1.getTime() - t0.getTime())/1000 + ' sec client JavaScript)';
+
+ var info_cmds = document.getElementById('generating_cmd');
+ if (!info_time || !cmds_server) {
+ return;
+ }
+ info_cmds.firstChild.data += ' + ' + cmds_server;
+}
+
+/**
+ * show an error message alert to user within page (in prohress info area)
+ * @param {String} str: plain text error message (no HTML)
+ *
+ * @globals div_progress_info
+ */
+function errorInfo(str) {
+ if (!div_progress_info) {
+ div_progress_info = document.getElementById('progress_info');
+ }
+ if (div_progress_info) {
+ div_progress_info.className = 'error';
+ div_progress_info.firstChild.data = str;
+ }
+}
+
+/* ............................................................ */
+/* coloring rows like 'blame' after 'blame_data' finishes */
+
+/**
+ * returns true if given row element (tr) is first in commit group
+ * to be used only after 'blame_data' finishes (after processing)
+ *
+ * @param {HTMLElement} tr: table row
+ * @returns {Boolean} true if TR is first in commit group
+ */
+function isStartOfGroup(tr) {
+ return tr.firstChild.className === 'sha1';
+}
+
+var colorRe = /(?:light|dark)/;
+
+/**
+ * change colors to use zebra coloring (2 colors) instead of 3 colors
+ * concatenate neighbour commit groups belonging to the same commit
+ *
+ * @globals colorRe
+ */
+function fixColorsAndGroups() {
+ var colorClasses = ['light', 'dark'];
+ var linenum = 1;
+ var tr, prev_group;
+ var colorClass = 0;
+ var table =
+ document.getElementById('blame_table') ||
+ document.getElementsByTagName('table')[0];
+
+ while ((tr = document.getElementById('l'+linenum))) {
+ // index origin is 0, which is table header; start from 1
+ //while ((tr = table.rows[linenum])) { // <- it is slower
+ if (isStartOfGroup(tr, linenum, document)) {
+ if (prev_group &&
+ prev_group.firstChild.firstChild.href ===
+ tr.firstChild.firstChild.href) {
+ // we have to concatenate groups
+ var prev_rows = prev_group.firstChild.rowSpan || 1;
+ var curr_rows = tr.firstChild.rowSpan || 1;
+ prev_group.firstChild.rowSpan = prev_rows + curr_rows;
+ //tr.removeChild(tr.firstChild);
+ tr.deleteCell(0); // DOM2 HTML way
+ } else {
+ colorClass = (colorClass + 1) % 2;
+ prev_group = tr;
+ }
+ }
+ var tr_class = tr.className;
+ tr.className = tr_class.replace(colorRe, colorClasses[colorClass]);
+ linenum++;
+ }
+}
+
+/* ............................................................ */
+/* time and data */
+
+/**
+ * used to extract hours and minutes from timezone info, e.g '-0900'
+ * @constant
+ */
+var tzRe = /^([+-][0-9][0-9])([0-9][0-9])$/;
+
+/**
+ * return date in local time formatted in iso-8601 like format
+ * 'yyyy-mm-dd HH:MM:SS +/-ZZZZ' e.g. '2005-08-07 21:49:46 +0200'
+ *
+ * @param {Number} epoch: seconds since '00:00:00 1970-01-01 UTC'
+ * @param {String} timezoneInfo: numeric timezone '(+|-)HHMM'
+ * @returns {String} date in local time in iso-8601 like format
+ *
+ * @globals tzRe
+ */
+function formatDateISOLocal(epoch, timezoneInfo) {
+ var match = tzRe.exec(timezoneInfo);
+ // date corrected by timezone
+ var localDate = new Date(1000 * (epoch +
+ (parseInt(match[1],10)*3600 + parseInt(match[2],10)*60)));
+ var localDateStr = // e.g. '2005-08-07'
+ localDate.getUTCFullYear() + '-' +
+ padLeft(localDate.getUTCMonth()+1, 2, '0') + '-' +
+ padLeft(localDate.getUTCDate(), 2, '0');
+ var localTimeStr = // e.g. '21:49:46'
+ padLeft(localDate.getUTCHours(), 2, '0') + ':' +
+ padLeft(localDate.getUTCMinutes(), 2, '0') + ':' +
+ padLeft(localDate.getUTCSeconds(), 2, '0');
+
+ return localDateStr + ' ' + localTimeStr + ' ' + timezoneInfo;
+}
+
+/* ............................................................ */
+/* unquoting/unescaping filenames */
+
+/**#@+
+ * @constant
+ */
+var escCodeRe = /\\([^0-7]|[0-7]{1,3})/g;
+var octEscRe = /^[0-7]{1,3}$/;
+var maybeQuotedRe = /^\"(.*)\"$/;
+/**#@-*/
+
+/**
+ * unquote maybe git-quoted filename
+ * e.g. 'aa' -> 'aa', '"a\ta"' -> 'a a'
+ *
+ * @param {String} str: git-quoted string
+ * @returns {String} Unquoted and unescaped string
+ *
+ * @globals escCodeRe, octEscRe, maybeQuotedRe
+ */
+function unquote(str) {
+ function unq(seq) {
+ var es = {
+ // character escape codes, aka escape sequences (from C)
+ // replacements are to some extent JavaScript specific
+ t: "\t", // tab (HT, TAB)
+ n: "\n", // newline (NL)
+ r: "\r", // return (CR)
+ f: "\f", // form feed (FF)
+ b: "\b", // backspace (BS)
+ a: "\x07", // alarm (bell) (BEL)
+ e: "\x1B", // escape (ESC)
+ v: "\v" // vertical tab (VT)
+ };
+
+ if (seq.search(octEscRe) !== -1) {
+ // octal char sequence
+ return String.fromCharCode(parseInt(seq, 8));
+ } else if (seq in es) {
+ // C escape sequence, aka character escape code
+ return es[seq];
+ }
+ // quoted ordinary character
+ return seq;
+ }
+
+ var match = str.match(maybeQuotedRe);
+ if (match) {
+ str = match[1];
+ // perhaps str = eval('"'+str+'"'); would be enough?
+ str = str.replace(escCodeRe,
+ function (substr, p1, offset, s) { return unq(p1); });
+ }
+ return str;
+}
+
+/* ============================================================ */
+/* main part: parsing response */
+
+/**
+ * Function called for each blame entry, as soon as it finishes.
+ * It updates page via DOM manipulation, adding sha1 info, etc.
+ *
+ * @param {Commit} commit: blamed commit
+ * @param {Object} group: object representing group of lines,
+ * which blame the same commit (blame entry)
+ *
+ * @globals blamedLines
+ */
+function handleLine(commit, group) {
+ /*
+ This is the structure of the HTML fragment we are working
+ with:
+
+ <tr id="l123" class="">
+ <td class="sha1" title=""><a href=""> </a></td>
+ <td class="linenr"><a class="linenr" href="">123</a></td>
+ <td class="pre"># times (my ext3 doesn't).</td>
+ </tr>
+ */
+
+ var resline = group.resline;
+
+ // format date and time string only once per commit
+ if (!commit.info) {
+ /* e.g. 'Kay Sievers, 2005-08-07 21:49:46 +0200' */
+ commit.info = commit.author + ', ' +
+ formatDateISOLocal(commit.authorTime, commit.authorTimezone);
+ }
+
+ // loop over lines in commit group
+ for (var i = 0; i < group.numlines; i++, resline++) {
+ var tr = document.getElementById('l'+resline);
+ if (!tr) {
+ break;
+ }
+ /*
+ <tr id="l123" class="">
+ <td class="sha1" title=""><a href=""> </a></td>
+ <td class="linenr"><a class="linenr" href="">123</a></td>
+ <td class="pre"># times (my ext3 doesn't).</td>
+ </tr>
+ */
+ var td_sha1 = tr.firstChild;
+ var a_sha1 = td_sha1.firstChild;
+ var a_linenr = td_sha1.nextSibling.firstChild;
+
+ /* <tr id="l123" class=""> */
+ var tr_class = 'light'; // or tr.className
+ if (commit.boundary) {
+ tr_class += ' boundary';
+ }
+ if (commit.nprevious === 0) {
+ tr_class += ' no-previous';
+ } else if (commit.nprevious > 1) {
+ tr_class += ' multiple-previous';
+ }
+ tr.className = tr_class;
+
+ /* <td class="sha1" title="?" rowspan="?"><a href="?">?</a></td> */
+ if (i === 0) {
+ td_sha1.title = commit.info;
+ td_sha1.rowSpan = group.numlines;
+
+ a_sha1.href = projectUrl + 'a=commit;h=' + commit.sha1;
+ a_sha1.firstChild.data = commit.sha1.substr(0, 8);
+ if (group.numlines >= 2) {
+ var fragment = document.createDocumentFragment();
+ var br = document.createElement("br");
+ var text = document.createTextNode(
+ commit.author.match(/\b([A-Z])\B/g).join(''));
+ if (br && text) {
+ var elem = fragment || td_sha1;
+ elem.appendChild(br);
+ elem.appendChild(text);
+ if (fragment) {
+ td_sha1.appendChild(fragment);
+ }
+ }
+ }
+ } else {
+ //tr.removeChild(td_sha1); // DOM2 Core way
+ tr.deleteCell(0); // DOM2 HTML way
+ }
+
+ /* <td class="linenr"><a class="linenr" href="?">123</a></td> */
+ var linenr_commit =
+ ('previous' in commit ? commit.previous : commit.sha1);
+ var linenr_filename =
+ ('file_parent' in commit ? commit.file_parent : commit.filename);
+ a_linenr.href = projectUrl + 'a=blame_incremental' +
+ ';hb=' + linenr_commit +
+ ';f=' + encodeURIComponent(linenr_filename) +
+ '#l' + (group.srcline + i);
+
+ blamedLines++;
+
+ //updateProgressInfo();
+ }
+}
+
+// ----------------------------------------------------------------------
+
+var inProgress = false; // are we processing response
+
+/**#@+
+ * @constant
+ */
+var sha1Re = /^([0-9a-f]{40}) ([0-9]+) ([0-9]+) ([0-9]+)/;
+var infoRe = /^([a-z-]+) ?(.*)/;
+var endRe = /^END ?([^ ]*) ?(.*)/;
+/**@-*/
+
+var curCommit = new Commit();
+var curGroup = {};
+
+var pollTimer = null;
+
+/**
+ * Parse output from 'git blame --incremental [...]', received via
+ * XMLHttpRequest from server (blamedataUrl), and call handleLine
+ * (which updates page) as soon as blame entry is completed.
+ *
+ * @param {String[]} lines: new complete lines from blamedata server
+ *
+ * @globals commits, curCommit, curGroup, t_interval_server, cmds_server
+ * @globals sha1Re, infoRe, endRe
+ */
+function processBlameLines(lines) {
+ var match;
+
+ for (var i = 0, len = lines.length; i < len; i++) {
+
+ if ((match = sha1Re.exec(lines[i]))) {
+ var sha1 = match[1];
+ var srcline = parseInt(match[2], 10);
+ var resline = parseInt(match[3], 10);
+ var numlines = parseInt(match[4], 10);
+
+ var c = commits[sha1];
+ if (!c) {
+ c = new Commit(sha1);
+ commits[sha1] = c;
+ }
+ curCommit = c;
+
+ curGroup.srcline = srcline;
+ curGroup.resline = resline;
+ curGroup.numlines = numlines;
+
+ } else if ((match = infoRe.exec(lines[i]))) {
+ var info = match[1];
+ var data = match[2];
+ switch (info) {
+ case 'filename':
+ curCommit.filename = unquote(data);
+ // 'filename' information terminates the entry
+ handleLine(curCommit, curGroup);
+ updateProgressInfo();
+ break;
+ case 'author':
+ curCommit.author = data;
+ break;
+ case 'author-time':
+ curCommit.authorTime = parseInt(data, 10);
+ break;
+ case 'author-tz':
+ curCommit.authorTimezone = data;
+ break;
+ case 'previous':
+ curCommit.nprevious++;
+ // store only first 'previous' header
+ if (!'previous' in curCommit) {
+ var parts = data.split(' ', 2);
+ curCommit.previous = parts[0];
+ curCommit.file_parent = unquote(parts[1]);
+ }
+ break;
+ case 'boundary':
+ curCommit.boundary = true;
+ break;
+ } // end switch
+
+ } else if ((match = endRe.exec(lines[i]))) {
+ t_interval_server = match[1];
+ cmds_server = match[2];
+
+ } else if (lines[i] !== '') {
+ // malformed line
+
+ } // end if (match)
+
+ } // end for (lines)
+}
+
+/**
+ * Process new data and return pointer to end of processed part
+ *
+ * @param {String} unprocessed: new data (from nextReadPos)
+ * @param {Number} nextReadPos: end of last processed data
+ * @return {Number} end of processed data (new value for nextReadPos)
+ */
+function processData(unprocessed, nextReadPos) {
+ var lastLineEnd = unprocessed.lastIndexOf('\n');
+ if (lastLineEnd !== -1) {
+ var lines = unprocessed.substring(0, lastLineEnd).split('\n');
+ nextReadPos += lastLineEnd + 1 /* 1 == '\n'.length */;
+
+ processBlameLines(lines);
+ } // end if
+
+ return nextReadPos;
+}
+
+/**
+ * Handle XMLHttpRequest errors
+ *
+ * @param {XMLHttpRequest} xhr: XMLHttpRequest object
+ *
+ * @globals pollTimer, commits, inProgress
+ */
+function handleError(xhr) {
+ errorInfo('Server error: ' +
+ xhr.status + ' - ' + (xhr.statusText || 'Error contacting server'));
+
+ clearInterval(pollTimer);
+ commits = {}; // free memory
+
+ inProgress = false;
+}
+
+/**
+ * Called after XMLHttpRequest finishes (loads)
+ *
+ * @param {XMLHttpRequest} xhr: XMLHttpRequest object (unused)
+ *
+ * @globals pollTimer, commits, inProgress
+ */
+function responseLoaded(xhr) {
+ clearInterval(pollTimer);
+
+ fixColorsAndGroups();
+ writeTimeInterval();
+ commits = {}; // free memory
+
+ inProgress = false;
+}
+
+/**
+ * handler for XMLHttpRequest onreadystatechange event
+ * @see startBlame
+ *
+ * @globals xhr, inProgress
+ */
+function handleResponse() {
+
+ /*
+ * xhr.readyState
+ *
+ * Value Constant (W3C) Description
+ * -------------------------------------------------------------------
+ * 0 UNSENT open() has not been called yet.
+ * 1 OPENED send() has not been called yet.
+ * 2 HEADERS_RECEIVED send() has been called, and headers
+ * and status are available.
+ * 3 LOADING Downloading; responseText holds partial data.
+ * 4 DONE The operation is complete.
+ */
+
+ if (xhr.readyState !== 4 && xhr.readyState !== 3) {
+ return;
+ }
+
+ // the server returned error
+ if (xhr.readyState === 3 && xhr.status !== 200) {
+ return;
+ }
+ if (xhr.readyState === 4 && xhr.status !== 200) {
+ handleError(xhr);
+ return;
+ }
+
+ // In konqueror xhr.responseText is sometimes null here...
+ if (xhr.responseText === null) {
+ return;
+ }
+
+ // in case we were called before finished processing
+ if (inProgress) {
+ return;
+ } else {
+ inProgress = true;
+ }
+
+ // extract new whole (complete) lines, and process them
+ while (xhr.prevDataLength !== xhr.responseText.length) {
+ if (xhr.readyState === 4 &&
+ xhr.prevDataLength === xhr.responseText.length) {
+ break;
+ }
+
+ xhr.prevDataLength = xhr.responseText.length;
+ var unprocessed = xhr.responseText.substring(xhr.nextReadPos);
+ xhr.nextReadPos = processData(unprocessed, xhr.nextReadPos);
+ } // end while
+
+ // did we finish work?
+ if (xhr.readyState === 4 &&
+ xhr.prevDataLength === xhr.responseText.length) {
+ responseLoaded(xhr);
+ }
+
+ inProgress = false;
+}
+
+// ============================================================
+// ------------------------------------------------------------
+
+/**
+ * Incrementally update line data in blame_incremental view in gitweb.
+ *
+ * @param {String} blamedataUrl: URL to server script generating blame data.
+ * @param {String} bUrl: partial URL to project, used to generate links.
+ *
+ * Called from 'blame_incremental' view after loading table with
+ * file contents, a base for blame view.
+ *
+ * @globals xhr, t0, projectUrl, div_progress_bar, totalLines, pollTimer
+*/
+function startBlame(blamedataUrl, bUrl) {
+
+ xhr = createRequestObject();
+ if (!xhr) {
+ errorInfo('ERROR: XMLHttpRequest not supported');
+ return;
+ }
+
+ t0 = new Date();
+ projectUrl = bUrl + (bUrl.indexOf('?') === -1 ? '?' : ';');
+ if ((div_progress_bar = document.getElementById('progress_bar'))) {
+ //div_progress_bar.setAttribute('style', 'width: 100%;');
+ div_progress_bar.style.cssText = 'width: 100%;';
+ }
+ totalLines = countLines();
+ updateProgressInfo();
+
+ /* add extra properties to xhr object to help processing response */
+ xhr.prevDataLength = -1; // used to detect if we have new data
+ xhr.nextReadPos = 0; // where unread part of response starts
+
+ xhr.onreadystatechange = handleResponse;
+ //xhr.onreadystatechange = function () { handleResponse(xhr); };
+
+ xhr.open('GET', blamedataUrl);
+ xhr.setRequestHeader('Accept', 'text/plain');
+ xhr.send(null);
+
+ // not all browsers call onreadystatechange event on each server flush
+ // poll response using timer every second to handle this issue
+ pollTimer = setInterval(xhr.onreadystatechange, 1000);
+}
+
+// end of gitweb.js
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 08d410d..88c91ff 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -96,6 +96,8 @@ our $stylesheet = undef;
our $logo = "++GITWEB_LOGO++";
# URI of GIT favicon, assumed to be image/png type
our $favicon = "++GITWEB_FAVICON++";
+# URI of gitweb.js (JavaScript code for gitweb)
+our $javascript = "++GITWEB_JS++";
# URI and label (title) of GIT logo link
#our $logo_url = "http://www.kernel.org/pub/software/scm/git/docs/";
@@ -575,6 +577,8 @@ our %cgi_param_mapping = @cgi_param_mapping;
# we will also need to know the possible actions, for validation
our %actions = (
"blame" => \&git_blame,
+ "blame_incremental" => \&git_blame_incremental,
+ "blame_data" => \&git_blame_data,
"blobdiff" => \&git_blobdiff,
"blobdiff_plain" => \&git_blobdiff_plain,
"blob" => \&git_blob,
@@ -4800,7 +4804,9 @@ sub git_tag {
git_footer_html();
}
-sub git_blame {
+sub git_blame_common {
+ my $format = shift || 'porcelain';
+
# permissions
gitweb_check_feature('blame')
or die_error(403, "Blame view not allowed");
@@ -4822,10 +4828,43 @@ sub git_blame {
}
}
- # run git-blame --porcelain
- open my $fd, "-|", git_cmd(), "blame", '-p',
- $hash_base, '--', $file_name
- or die_error(500, "Open git-blame failed");
+ my $fd;
+ if ($format eq 'incremental') {
+ # get file contents (as base)
+ open $fd, "-|", git_cmd(), 'cat-file', 'blob', $hash
+ or die_error(500, "Open git-cat-file failed");
+ } elsif ($format eq 'data') {
+ # run git-blame --incremental
+ open $fd, "-|", git_cmd(), "blame", "--incremental",
+ $hash_base, "--", $file_name
+ or die_error(500, "Open git-blame --incremental failed");
+ } else {
+ # run git-blame --porcelain
+ open $fd, "-|", git_cmd(), "blame", '-p',
+ $hash_base, '--', $file_name
+ or die_error(500, "Open git-blame --porcelain failed");
+ }
+
+ # incremental blame data returns early
+ if ($format eq 'data') {
+ print $cgi->header(
+ -type=>"text/plain", -charset => "utf-8",
+ -status=> "200 OK");
+ local $| = 1; # output autoflush
+ print while <$fd>;
+ close $fd
+ or print "ERROR $!\n";
+
+ print 'END';
+ if (defined $t0 && gitweb_check_feature('timed')) {
+ print ' '.
+ Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).
+ ' '.$number_of_git_cmds;
+ }
+ print "\n";
+
+ return;
+ }
# page header
git_header_html();
@@ -4836,109 +4875,170 @@ sub git_blame {
$cgi->a({-href => href(action=>"history", -replay=>1)},
"history") .
" | " .
- $cgi->a({-href => href(action=>"blame", file_name=>$file_name)},
+ $cgi->a({-href => href(action=>$action, file_name=>$file_name)},
"HEAD");
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
git_print_page_path($file_name, $ftype, $hash_base);
# page body
+ if ($format eq 'incremental') {
+ print "<noscript>\n<div class=\"error\"><center><b>\n".
+ "This page requires JavaScript to run.\n Use ".
+ $cgi->a({-href => href(action=>'blame',-replay=>1)},
+ 'this page').
+ " instead.\n".
+ "</b></center></div>\n</noscript>\n";
+
+ print qq!<div id="progress_bar" style="width: 100%; background-color: yellow"></div>\n!;
+ }
+
+ print qq!<div class="page_body">\n!;
+ print qq!<div id="progress_info">... / ...</div>\n!
+ if ($format eq 'incremental');
+ print qq!<table id="blame_table" class="blame" width="100%">\n!.
+ #qq!<col width="5.5em" /><col width="2.5em" /><col width="*" />\n!.
+ qq!<thead>\n!.
+ qq!<tr><th>Commit</th><th>Line</th><th>Data</th></tr>\n!.
+ qq!</thead>\n!.
+ qq!<tbody>\n!;
+
my @rev_color = qw(light dark);
my $num_colors = scalar(@rev_color);
my $current_color = 0;
- my %metainfo = ();
- print <<HTML;
-<div class="page_body">
-<table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
-HTML
- LINE:
- while (my $line = <$fd>) {
- chomp $line;
- # the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
- # no <lines in group> for subsequent lines in group of lines
- my ($full_rev, $orig_lineno, $lineno, $group_size) =
- ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
- if (!exists $metainfo{$full_rev}) {
- $metainfo{$full_rev} = { 'nprevious' => 0 };
- }
- my $meta = $metainfo{$full_rev};
- my $data;
- while ($data = <$fd>) {
- chomp $data;
- last if ($data =~ s/^\t//); # contents of line
- if ($data =~ /^(\S+)(?: (.*))?$/) {
- $meta->{$1} = $2 unless exists $meta->{$1};
+ if ($format eq 'incremental') {
+ my $color_class = $rev_color[$current_color];
+
+ #contents of a file
+ my $linenr = 0;
+ LINE:
+ while (my $line = <$fd>) {
+ chomp $line;
+ $linenr++;
+
+ print qq!<tr id="l$linenr" class="$color_class">!.
+ qq!<td class="sha1"><a href=""> </a></td>!.
+ qq!<td class="linenr">!.
+ qq!<a class="linenr" href="">$linenr</a></td>!;
+ print qq!<td class="pre">! . esc_html($line) . "</td>\n";
+ print qq!</tr>\n!;
+ }
+
+ } else { # porcelain, i.e. ordinary blame
+ my %metainfo = (); # saves information about commits
+
+ # blame data
+ LINE:
+ while (my $line = <$fd>) {
+ chomp $line;
+ # the header: <SHA-1> <src lineno> <dst lineno> [<lines in group>]
+ # no <lines in group> for subsequent lines in group of lines
+ my ($full_rev, $orig_lineno, $lineno, $group_size) =
+ ($line =~ /^([0-9a-f]{40}) (\d+) (\d+)(?: (\d+))?$/);
+ if (!exists $metainfo{$full_rev}) {
+ $metainfo{$full_rev} = { 'nprevious' => 0 };
}
- if ($data =~ /^previous /) {
- $meta->{'nprevious'}++;
+ my $meta = $metainfo{$full_rev};
+ my $data;
+ while ($data = <$fd>) {
+ chomp $data;
+ last if ($data =~ s/^\t//); # contents of line
+ if ($data =~ /^(\S+)(?: (.*))?$/) {
+ $meta->{$1} = $2 unless exists $meta->{$1};
+ }
+ if ($data =~ /^previous /) {
+ $meta->{'nprevious'}++;
+ }
}
- }
- my $short_rev = substr($full_rev, 0, 8);
- my $author = $meta->{'author'};
- my %date =
- parse_date($meta->{'author-time'}, $meta->{'author-tz'});
- my $date = $date{'iso-tz'};
- if ($group_size) {
- $current_color = ($current_color + 1) % $num_colors;
- }
- my $tr_class = $rev_color[$current_color];
- $tr_class .= ' boundary' if (exists $meta->{'boundary'});
- $tr_class .= ' no-previous' if ($meta->{'nprevious'} == 0);
- $tr_class .= ' multiple-previous' if ($meta->{'nprevious'} > 1);
- print "<tr id=\"l$lineno\" class=\"$tr_class\">\n";
- if ($group_size) {
- print "<td class=\"sha1\"";
- print " title=\"". esc_html($author) . ", $date\"";
- print " rowspan=\"$group_size\"" if ($group_size > 1);
- print ">";
- print $cgi->a({-href => href(action=>"commit",
- hash=>$full_rev,
- file_name=>$file_name)},
- esc_html($short_rev));
- if ($group_size >= 2) {
- my @author_initials = ($author =~ /\b([[:upper:]])\B/g);
- if (@author_initials) {
- print "<br />" .
- esc_html(join('', @author_initials));
- # or join('.', ...)
+ my $short_rev = substr($full_rev, 0, 8);
+ my $author = $meta->{'author'};
+ my %date =
+ parse_date($meta->{'author-time'}, $meta->{'author-tz'});
+ my $date = $date{'iso-tz'};
+ if ($group_size) {
+ $current_color = ($current_color + 1) % $num_colors;
+ }
+ my $tr_class = $rev_color[$current_color];
+ $tr_class .= ' boundary' if (exists $meta->{'boundary'});
+ $tr_class .= ' no-previous' if ($meta->{'nprevious'} == 0);
+ $tr_class .= ' multiple-previous' if ($meta->{'nprevious'} > 1);
+ print "<tr id=\"l$lineno\" class=\"$tr_class\">\n";
+ if ($group_size) {
+ print "<td class=\"sha1\"";
+ print " title=\"". esc_html($author) . ", $date\"";
+ print " rowspan=\"$group_size\"" if ($group_size > 1);
+ print ">";
+ print $cgi->a({-href => href(action=>"commit",
+ hash=>$full_rev,
+ file_name=>$file_name)},
+ esc_html($short_rev));
+ if ($group_size >= 2) {
+ my @author_initials = ($author =~ /\b([[:upper:]])\B/g);
+ if (@author_initials) {
+ print "<br />" .
+ esc_html(join('', @author_initials));
+ # or join('.', ...)
+ }
}
+ print "</td>\n";
}
- print "</td>\n";
- }
- # 'previous' <sha1 of parent commit> <filename at commit>
- if (exists $meta->{'previous'} &&
- $meta->{'previous'} =~ /^([a-fA-F0-9]{40}) (.*)$/) {
- $meta->{'parent'} = $1;
- $meta->{'file_parent'} = unquote($2);
- }
- my $linenr_commit =
- exists($meta->{'parent'}) ?
- $meta->{'parent'} : $full_rev;
- my $linenr_filename =
- exists($meta->{'file_parent'}) ?
- $meta->{'file_parent'} : unquote($meta->{'filename'});
- my $blamed = href(action => 'blame',
- file_name => $linenr_filename,
- hash_base => $linenr_commit);
- print "<td class=\"linenr\">";
- print $cgi->a({ -href => "$blamed#l$orig_lineno",
- -class => "linenr" },
- esc_html($lineno));
- print "</td>";
- print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
- print "</tr>\n";
+ # 'previous' <sha1 of parent commit> <filename at commit>
+ if (exists $meta->{'previous'} &&
+ $meta->{'previous'} =~ /^([a-fA-F0-9]{40}) (.*)$/) {
+ $meta->{'parent'} = $1;
+ $meta->{'file_parent'} = unquote($2);
+ }
+ my $linenr_commit =
+ exists($meta->{'parent'}) ?
+ $meta->{'parent'} : $full_rev;
+ my $linenr_filename =
+ exists($meta->{'file_parent'}) ?
+ $meta->{'file_parent'} : unquote($meta->{'filename'});
+ my $blamed = href(action => 'blame',
+ file_name => $linenr_filename,
+ hash_base => $linenr_commit);
+ print "<td class=\"linenr\">";
+ print $cgi->a({ -href => "$blamed#l$orig_lineno",
+ -class => "linenr" },
+ esc_html($lineno));
+ print "</td>";
+ print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
+ print "</tr>\n";
+ } # end while
+
}
- print "</table>\n";
- print "</div>";
+
+ # footer
+ print "</tbody>\n".
+ "</table>\n"; # class="blame"
+ print "</div>\n"; # class="blame_body"
close $fd
or print "Reading blob failed\n";
- # page footer
+ if ($format eq 'incremental') {
+ print qq!<script type="text/javascript" src="$javascript"></script>\n!.
+ qq!<script type="text/javascript">\n!.
+ qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
+ qq! "!. href() .qq!");\n!.
+ qq!</script>\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();
--
1.6.3.3
^ permalink raw reply related
* [PATCHv1 3/5] gitweb: Colorize 'blame_incremental' view during processing
From: Jakub Narebski @ 2009-09-01 11:39 UTC (permalink / raw)
To: git
Cc: Petr Baudis, Fredrik Kuivinen, Giuseppe Bilotta, Luben Tuikov,
Martin Koegler, Jakub Narebski
In-Reply-To: <1251805160-5303-3-git-send-email-jnareb@gmail.com>
This requires using 3 colors, not only two, to be able to choose color
different from colors of up to 2 neigbours.
gitweb.js select least used color, if more than one color is possible.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This was earlier part of previous commit (adding 'blame_incremental'
view).
gitweb/gitweb.css | 5 ++
gitweb/gitweb.js | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 110 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 69ef119..977a013 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -262,6 +262,11 @@ tr.no-previous td.linenr {
font-weight: bold;
}
+/* for 'blame_incremental', during processing */
+tr.color1 { background-color: #f6fff6; }
+tr.color2 { background-color: #f6f6ff; }
+tr.color3 { background-color: #fff6f6; }
+
td {
padding: 2px 5px;
font-size: 100%;
diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
index c8411e7..bf38216 100644
--- a/gitweb/gitweb.js
+++ b/gitweb/gitweb.js
@@ -211,6 +211,101 @@ function errorInfo(str) {
}
/* ............................................................ */
+/* coloring rows during blame_data (git blame --incremental) run */
+
+/**
+ * used to extract N from 'colorN', where N is a number,
+ * @constant
+ */
+var colorRe = /\bcolor([0-9]*)\b/;
+
+/**
+ * return N if <tr class="colorN">, otherwise return null
+ * (some browsers require CSS class names to begin with letter)
+ *
+ * @param {HTMLElement} tr: table row element to check
+ * @param {String} tr.className: 'class' attribute of tr element
+ * @returns {Number|null} N if tr.className == 'colorN', otherwise null
+ *
+ * @globals colorRe
+ */
+function getColorNo(tr) {
+ if (!tr) {
+ return null;
+ }
+ var className = tr.className;
+ if (className) {
+ var match = colorRe.exec(className);
+ if (match) {
+ return parseInt(match[1], 10);
+ }
+ }
+ return null;
+}
+
+var colorsFreq = [0, 0, 0];
+/**
+ * return one of given possible colors (curently least used one)
+ * example: chooseColorNoFrom(2, 3) returns 2 or 3
+ *
+ * @param {Number[]} arguments: one or more numbers
+ * assumes that 1 <= arguments[i] <= colorsFreq.length
+ * @returns {Number} Least used color number from arguments
+ * @globals colorsFreq
+ */
+function chooseColorNoFrom() {
+ // choose the color which is least used
+ var colorNo = arguments[0];
+ for (var i = 1; i < arguments.length; i++) {
+ if (colorsFreq[arguments[i]-1] < colorsFreq[colorNo-1]) {
+ colorNo = arguments[i];
+ }
+ }
+ colorsFreq[colorNo-1]++;
+ return colorNo;
+}
+
+/**
+ * given two neigbour <tr> elements, find color which would be different
+ * from color of both of neighbours; used to 3-color blame table
+ *
+ * @param {HTMLElement} tr_prev
+ * @param {HTMLElement} tr_next
+ * @returns {Number} color number N such that
+ * colorN != tr_prev.className && colorN != tr_next.className
+ */
+function findColorNo(tr_prev, tr_next) {
+ var color_prev = getColorNo(tr_prev);
+ var color_next = getColorNo(tr_next);
+
+
+ // neither of neighbours has color set
+ // THEN we can use any of 3 possible colors
+ if (!color_prev && !color_next) {
+ return chooseColorNoFrom(1,2,3);
+ }
+
+ // either both neighbours have the same color,
+ // or only one of neighbours have color set
+ // THEN we can use any color except given
+ var color;
+ if (color_prev === color_next) {
+ color = color_prev; // = color_next;
+ } else if (!color_prev) {
+ color = color_next;
+ } else if (!color_next) {
+ color = color_prev;
+ }
+ if (color) {
+ return chooseColorNoFrom((color % 3) + 1, ((color+1) % 3) + 1);
+ }
+
+ // neighbours have different colors
+ // THEN there is only one color left
+ return (3 - ((color_prev + color_next) % 3));
+}
+
+/* ............................................................ */
/* coloring rows like 'blame' after 'blame_data' finishes */
/**
@@ -224,8 +319,6 @@ function isStartOfGroup(tr) {
return tr.firstChild.className === 'sha1';
}
-var colorRe = /(?:light|dark)/;
-
/**
* change colors to use zebra coloring (2 colors) instead of 3 colors
* concatenate neighbour commit groups belonging to the same commit
@@ -391,6 +484,12 @@ function handleLine(commit, group) {
formatDateISOLocal(commit.authorTime, commit.authorTimezone);
}
+ // color depends on group of lines, not only on blamed commit
+ var colorNo = findColorNo(
+ document.getElementById('l'+(resline-1)),
+ document.getElementById('l'+(resline+group.numlines))
+ );
+
// loop over lines in commit group
for (var i = 0; i < group.numlines; i++, resline++) {
var tr = document.getElementById('l'+resline);
@@ -409,7 +508,10 @@ function handleLine(commit, group) {
var a_linenr = td_sha1.nextSibling.firstChild;
/* <tr id="l123" class=""> */
- var tr_class = 'light'; // or tr.className
+ var tr_class = '';
+ if (colorNo !== null) {
+ tr_class = 'color'+colorNo;
+ }
if (commit.boundary) {
tr_class += ' boundary';
}
--
1.6.3.3
^ permalink raw reply related
* [PATCHv3/RFC 4/5] gitweb: Create links leading to 'blame_incremental' using JavaScript
From: Jakub Narebski @ 2009-09-01 11:39 UTC (permalink / raw)
To: git
Cc: Petr Baudis, Fredrik Kuivinen, Giuseppe Bilotta, Luben Tuikov,
Martin Koegler, Jakub Narebski
In-Reply-To: <1251805160-5303-4-git-send-email-jnareb@gmail.com>
The new 'blame_incremental' view requires JavaScript to run. Not all
web browsers implement JavaScript (e.g. text browsers such as Lynx),
and not all users have JavaScript enabled. Therefore instead of
unconditionally linking to 'blame_incremental' view, we use JavaScript
to convert those links to lead to view utilizing JavaScript, by adding
'js=1' to link.
Currently the only action that takes 'js=1' into account is 'blame',
which then acts as if it was called as 'blame_incremental' action.
Possible enhancement would be to do JavaScript redirect by setting
window.location instead of modifying $format and $action in
git_blame_common() subroutine.
The only JavaScript-aware/using view is currently 'blame_incremental'.
While at it move reading JavaScript to git_footer_html() subroutine.
Note that in this view we do not add 'js=1' currently (even though
perhaps we should; note that for consistency we should also add 'js=1'
in links added by JavaScript part of 'blame_incremental').
This idea was originally implemented by Petr Baudis in
http://article.gmane.org/gmane.comp.version-control.git/47614
but it added <script> element with fixBlameLinks() function in page
header, to be added as onload event using 'onload' attribute of HTML
'body' element: <body onload="fixBlameLinks();">. This version adds
script at then end of page (in the page footer), and uses JavaScript
'window.onload=fixLinks();'. Also in Petr version only links marked
with 'blamelink' class were modified, and they were modified by
replacing "a=blame" by "a=blame_incremental"... which doesn't work for
path_info links, and might replace wrong part if there is "a=blame" in
project name, ref name or file name.
Slightly different solution was implemented by Martin Koegler in
http://thread.gmane.org/gmane.comp.version-control.git/47902/focus=47905
Here GitAddLinks() function was in gitweb.js file, not as contents of
<script> element. It was also included in page header (in <head>
element) though, which means waiting for a script to load (and run).
It was smarter in that to "fix" (modify) link, it split URL, modified
value of 'a' parameter, and then recreated modified link. It avoids
trouble with "a=blame" as substring in project name or file name, but
it doesn't work with path_info URL/link in the way it was written.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is in RFC stage.
Currently fixLinks() is not invoked for 'blame_incremental', but links
generated by startBlame (or to be more exact by event handler in
gitweb.js) lead to 'blame_incremental' directly. Also the list of
"JavaScript-aware actions" is currently hardcoded to
'blame_incremental' (and relies on 'blame' + 'js=1' to change
$action).
Also fixLinks() adds 'js=1' to too many links, including $home_link,
and links to feeds (where JavaScript is not supported anyway).
Alternate solution would be to mark links which lead to action which
has JavaScript-requiring alternative, either marking it via 'class'
attribute, or providing for example 'alt_href' or 'jshref' attribute
with link to JavaScript-requiring variant.
Differences from previous version (v2):
* fixLinks() function is put in gitweb.js together with all JavaScript
code required for 'blame_incremental' view, instead of being defined
inside <script> element.
* Running startBlame() was moved to git_footer_html, instead of being
in git_blame_common
* The link to plain 'blame' view (which do not require JavaScript
support enabled in web browser) inside <noscript> element is marked
with 'js=0'.
* fixLinks() now doesn't add 'js=1' if links ends with '[?;]js=[01]'
(so that e.g. 'js=0' means link to view which do not require
JavaScript).
* performance: calculate number of elements in a list before loop
TODO for future commits:
* Use 'click' event to change links to jave 'js=1' parameter appended;
this way we would check if JavaScript is enabled at the moment of
following (clicking) link, not at the moment of loading the page.
Unfortunately adding event listeners (much better solution than
providing/adding 'onclick' attribute) is different in different
browsers. Some kind of wrapper around browser incompatibilities
would be required.
gitweb/gitweb.js | 34 ++++++++++++++++++++++++++++++++++
gitweb/gitweb.perl | 28 +++++++++++++++++++---------
2 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/gitweb/gitweb.js b/gitweb/gitweb.js
index bf38216..66fd64e 100644
--- a/gitweb/gitweb.js
+++ b/gitweb/gitweb.js
@@ -7,6 +7,39 @@
* @license GPLv2 or later
*/
+/* ============================================================ */
+/* functions for generic gitweb actions and views */
+
+/**
+ * used to check if link has 'js' query parameter already (at end),
+ * and other reasons to not add 'js=1' param at the end of link
+ * @constant
+ */
+var jsExceptionsRe = /[;?]js=[01]$/;
+
+/**
+ * Add '?js=1' or ';js=1' to the end of every link in the document
+ * that doesn't have 'js' query parameter set already.
+ *
+ * Links with 'js=1' lead to JavaScript version of given action, if it
+ * exists (currently there is only 'blame_incremental' for 'blame')
+ *
+ * @globals jsExceptionsRe
+ */
+function fixLinks() {
+ var allLinks = document.getElementsByTagName("a") || document.links;
+ for (var i = 0, len = allLinks.length; i < len; i++) {
+ var link = allLinks[i];
+ if (!jsExceptionsRe.test(link)) { // =~ /[;?]js=[01]$/;
+ link.href +=
+ (link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1';
+ }
+ }
+}
+
+
+/* ============================================================ */
+
/*
* This code uses DOM methods instead of (nonstandard) innerHTML
* to modify page.
@@ -89,6 +122,7 @@ function createRequestObject() {
return null;
}
+
/* ============================================================ */
/* utility/helper functions (and variables) */
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 88c91ff..0adfd3f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -571,6 +571,8 @@ our @cgi_param_mapping = (
snapshot_format => "sf",
extra_options => "opt",
search_use_regexp => "sr",
+ # this must be last entry (for manipulation from JavaScript)
+ javascript => "js"
);
our %cgi_param_mapping = @cgi_param_mapping;
@@ -3255,6 +3257,18 @@ sub git_footer_html {
insert_file($site_footer);
}
+ print qq!<script type="text/javascript" src="$javascript"></script>\n!;
+ if ($action eq 'blame_incremental') {
+ print qq!<script type="text/javascript">\n!.
+ qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
+ qq! "!. href() .qq!");\n!.
+ qq!</script>\n!;
+ } else {
+ print qq!<script type="text/javascript">\n!.
+ qq!window.onload = fixLinks;\n!.
+ qq!</script>\n!;
+ }
+
print "</body>\n" .
"</html>";
}
@@ -4806,6 +4820,10 @@ sub git_tag {
sub git_blame_common {
my $format = shift || 'porcelain';
+ if ($format eq 'porcelain' && $cgi->param('js')) {
+ $format = 'incremental';
+ $action = 'blame_incremental'; # for page title etc
+ }
# permissions
gitweb_check_feature('blame')
@@ -4885,7 +4903,7 @@ sub git_blame_common {
if ($format eq 'incremental') {
print "<noscript>\n<div class=\"error\"><center><b>\n".
"This page requires JavaScript to run.\n Use ".
- $cgi->a({-href => href(action=>'blame',-replay=>1)},
+ $cgi->a({-href => href(action=>'blame',javascript=>0,-replay=>1)},
'this page').
" instead.\n".
"</b></center></div>\n</noscript>\n";
@@ -5016,14 +5034,6 @@ sub git_blame_common {
close $fd
or print "Reading blob failed\n";
- if ($format eq 'incremental') {
- print qq!<script type="text/javascript" src="$javascript"></script>\n!.
- qq!<script type="text/javascript">\n!.
- qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
- qq! "!. href() .qq!");\n!.
- qq!</script>\n!;
- }
-
git_footer_html();
}
--
1.6.3.3
^ permalink raw reply related
* [PATCHv1/RFC 5/5] gitweb: Minify gitweb.js if JSMIN is defined
From: Jakub Narebski @ 2009-09-01 11:39 UTC (permalink / raw)
To: git
Cc: Petr Baudis, Fredrik Kuivinen, Giuseppe Bilotta, Luben Tuikov,
Martin Koegler, Jakub Narebski
In-Reply-To: <1251805160-5303-5-git-send-email-jnareb@gmail.com>
It requires that $JSMIN command can function as a filter.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in series. Comments welcome.
I have not tested it extensively, but JSMIN=cat seems to work
correctly.
Makefile | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 407b35c..f149a36 100644
--- a/Makefile
+++ b/Makefile
@@ -194,6 +194,9 @@ all::
# memory allocators with the nedmalloc allocator written by Niall Douglas.
#
# Define NO_REGEX if you have no or inferior regex support in your C library.
+#
+# Define JSMIN to point to JavaScript minifier that functions as
+# a filter to have gitweb.js minified.
GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -246,6 +249,9 @@ lib = lib
# DESTDIR=
pathsep = :
+# JavaScript minifier invocation that can function as filter
+JSMIN =
+
# default configuration for gitweb
GITWEB_CONFIG = gitweb_config.perl
GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf
@@ -261,7 +267,11 @@ GITWEB_HOMETEXT = indextext.html
GITWEB_CSS = gitweb.css
GITWEB_LOGO = git-logo.png
GITWEB_FAVICON = git-favicon.png
+ifdef JSMIN
+GITWEB_JS = gitweb.min.js
+else
GITWEB_JS = gitweb.js
+endif
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
@@ -1374,8 +1384,13 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
chmod +x $@+ && \
mv $@+ $@
+ifdef JSMIN
+OTHER_PROGRAMS += gitweb/gitweb.cgi gitweb/gitweb.min.js
+gitweb/gitweb.cgi: gitweb/gitweb.perl gitweb/gitweb.min.js
+else
OTHER_PROGRAMS += gitweb/gitweb.cgi
gitweb/gitweb.cgi: gitweb/gitweb.perl
+endif
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
@@ -1426,6 +1441,11 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
mv $@+ $@
endif # NO_PERL
+ifdef JSMIN
+gitweb/gitweb.min.js: gitweb/gitweb.js
+ $(QUIET_GEN)$(JSMIN) <$< >$@
+endif # JSMIN
+
configure: configure.ac
$(QUIET_GEN)$(RM) $@ $<+ && \
sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
--
1.6.3.3
^ permalink raw reply related
* [PATCHv2 1/5] gitweb: Add optional "time to generate page" info in footer
From: Jakub Narebski @ 2009-09-01 11:39 UTC (permalink / raw)
To: git
Cc: Petr Baudis, Fredrik Kuivinen, Giuseppe Bilotta, Luben Tuikov,
Martin Koegler, Jakub Narebski
In-Reply-To: <1251805160-5303-1-git-send-email-jnareb@gmail.com>
Add "This page took XXX seconds and Y git commands to generate"
to page footer, if global feature 'timed' is enabled (disabled
by default). Requires Time::HiRes installed for high precision
'wallclock' time.
Note that Time::HiRes is being required unconditionally of 'timed'
feature, because setting $t0 variable should be fairly early to have
running time of the whole script. If Time::HiRes module was required
only if 'timed' feature is enabled, the earliest place where starting
time ($t0) could be calculated would be after reading gitweb config,
making "time to generate page" info inaccurate.
This code is based on example code by Petr 'Pasky' Baudis.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version adds custom formatting for "time to generate page"
information in the page footer, instead of reusing 'page_footer'
style. It is now clearly separated from the gitweb footer proper.
While at it the ids were changed from "generate_info" etc. to
"generating_info" etc.
There is still a question whether Time::HiRes should be require'd
unconditionally, but otherwise this patch is out of RFC stage.
Sidenote: I have noticed that gitweb's CSS rules are unnecessary
strict, making rendering slower because of extra non needed checks
(the rule is that selectors should be as simple as possible).
gitweb/gitweb.css | 7 +++++++
gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index b59fcaf..00e2e4c 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -75,6 +75,13 @@ div.page_footer_text {
font-style: italic;
}
+div#generating_info {
+ margin: 4px;
+ font-size: smaller;
+ text-align: center;
+ color: #505050;
+}
+
div.page_body {
padding: 8px;
font-family: monospace;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ff2d42a..08d410d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,6 +18,12 @@ use File::Find qw();
use File::Basename qw(basename);
binmode STDOUT, ':utf8';
+our $t0;
+if (eval { require Time::HiRes; 1; }) {
+ $t0 = [Time::HiRes::gettimeofday()];
+}
+our $number_of_git_cmds = 0;
+
BEGIN {
CGI->compile() if $ENV{'MOD_PERL'};
}
@@ -404,6 +410,13 @@ our %feature = (
'sub' => \&feature_avatar,
'override' => 0,
'default' => ['']},
+
+ # Enable displaying how much time and how many git commands
+ # it took to generate and display page. Disabled by default.
+ # Project specific override is not supported.
+ 'timed' => {
+ 'override' => 0,
+ 'default' => [0]},
);
sub gitweb_get_feature {
@@ -518,6 +531,7 @@ if (-e $GITWEB_CONFIG) {
# version of the core git binary
our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";
+$number_of_git_cmds++;
$projects_list ||= $projectroot;
@@ -1969,6 +1983,7 @@ sub get_feed_info {
# returns path to the core git executable and the --git-dir parameter as list
sub git_cmd {
+ $number_of_git_cmds++;
return $GIT, '--git-dir='.$git_dir;
}
@@ -3218,6 +3233,20 @@ sub git_footer_html {
}
print "</div>\n"; # class="page_footer"
+ if (defined $t0 && gitweb_check_feature('timed')) {
+ print "<div id=\"generating_info\">\n";
+ print 'This page took '.
+ '<span id="generating_time" class="time_span">'.
+ Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).
+ ' seconds </span>'.
+ ' and '.
+ '<span id="generating_cmd">'.
+ $number_of_git_cmds.
+ '</span> git commands '.
+ " to generate.\n";
+ print "</div>\n"; # class="page_footer"
+ }
+
if (-f $site_footer) {
insert_file($site_footer);
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 0/5] gitweb: Incremental blame series (1 Sep 09)
From: Jakub Narebski @ 2009-09-01 11:39 UTC (permalink / raw)
To: git
Cc: Petr Baudis, Fredrik Kuivinen, Giuseppe Bilotta, Luben Tuikov,
Martin Koegler, Jakub Narebski
This series is meant to replace the 'jn/gitweb-blame' topic branch
(it is merged into 'pu', b08ae28).
Jakub Narebski (5):
gitweb: Add optional "time to generate page" info in footer
gitweb: Incremental blame (using JavaScript)
gitweb: Colorize 'blame_incremental' view during processing
gitweb: Create links leading to 'blame_incremental' using JavaScript
gitweb: Minify gitweb.js if JSMIN is defined
The "time to generate page" info got its own style, and don't looks
bolted on now. Incremental blame patch is no longer marked as proof
of concept, or as a work in progress; I think it is mature enough.
Last two patches (creating links and JSMIN) are in active development,
and should be considered work in progress (or even proof of concept
code).
This series has 3-color colorizing of 'blame_incremental' view is put
into separate patch. JavaScript code used to create links leading to
'blame_incremental' view is now put in gitweb.js, which is meant to
contain all JavaScript code used by Git.
The last patch (modifying Makefile) was added because of concern that
heavily commented gitweb.js could be a burden on server.
Makefile | 26 ++-
git-instaweb.sh | 7 +
gitweb/README | 4 +
gitweb/gitweb.css | 23 ++
gitweb/gitweb.js | 862 ++++++++++++++++++++++++++++++++++++++++++++++++++++
gitweb/gitweb.perl | 311 ++++++++++++++------
6 files changed, 1146 insertions(+), 87 deletions(-)
create mode 100644 gitweb/gitweb.js
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv4] git apply: option to ignore whitespace differences
From: Giuseppe Bilotta @ 2009-09-01 9:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Felipe Contreras, Nanako Shiraishi
In-Reply-To: <7vocpvvxaw.fsf@alter.siamese.dyndns.org>
On Tue, Sep 1, 2009 at 11:17 AM, Junio C Hamano<gitster@pobox.com> wrote:
>
> Why do you need imglen[] vla here? IOW, can't the above be simply like
> this?
>
> diff --git a/builtin-apply.c b/builtin-apply.c
> index ae11b41..c8372a0 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -1874,20 +1874,18 @@ static int match_fragment(struct image *img,
> if (ws_ignore_action == ignore_ws_change) {
> size_t imgoff = 0;
> size_t preoff = 0;
> size_t postlen = postimage->len;
> - size_t imglen[preimage->nr];
> for (i = 0; i < preimage->nr; i++) {
> size_t prelen = preimage->line[i].len;
> + size_t imglen = img->line[try_lno+i].len;
>
> - imglen[i] = img->line[try_lno+i].len;
> - if (!fuzzy_matchlines(
> - img->buf + try + imgoff, imglen[i],
> - preimage->buf + preoff, prelen))
> + if (!fuzzy_matchlines(img->buf + try + imgoff, imglen,
> + preimage->buf + preoff, prelen))
> return 0;
> if (preimage->line[i].flag & LINE_COMMON)
> - postlen += imglen[i] - prelen;
> - imgoff += imglen[i];
> + postlen += imglen - prelen;
> + imgoff += imglen;
> preoff += prelen;
> }
>
> /*
> @@ -1899,9 +1897,9 @@ static int match_fragment(struct image *img,
> */
> fixed_buf = xmalloc(imgoff);
> memcpy(fixed_buf, img->buf + try, imgoff);
> for (i = 0; i < preimage->nr; i++)
> - preimage->line[i].len = imglen[i];
> + preimage->line[i].len = img->line[try_lno+i].len;
Yep, I think that would do it. I'm not sure why I was doing it that
other way. Maybe a leftover from when I was still getting confident
with the code and I hadn't yet found the var that held the initial
match line, or something like that.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Error with git svn show-ignore: forbidden access
From: Yann Simon @ 2009-09-01 9:46 UTC (permalink / raw)
To: git
Hi,
with git version 1.6.4:
$ git svn show-ignore > .gitignore
RA layer request failed: Server sent unexpected return value (403
Forbidden) in response to PROPFIND request for
'/repos/XXX/YYY/ZZZ/trunk/aaa' at /usr/lib/git-core/git-svn line 2243
Is git svn show-ignore making request to the svn server?
I tried also with the --no-minimize-url option but get as answer:
$ git svn --no-minimize-url show-ignore
Unknown option: no-minimize-url
Thanks for the help
Yann
^ permalink raw reply
* Re: [PATCHv4] git apply: option to ignore whitespace differences
From: Junio C Hamano @ 2009-09-01 9:17 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Felipe Contreras, Nanako Shiraishi
In-Reply-To: <1249384609-30240-1-git-send-email-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 39dc96a..7ec5b8b 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -1773,12 +1866,57 @@ static int match_fragment(struct image *img,
> !memcmp(img->buf + try, preimage->buf, preimage->len))
> return 1;
>
> + /*
> + * No exact match. If we are ignoring whitespace, run a line-by-line
> + * fuzzy matching. We collect all the line length information because
> + * we need it to adjust whitespace if we match.
> + */
> + if (ws_ignore_action == ignore_ws_change) {
> + size_t imgoff = 0;
> + size_t preoff = 0;
> + size_t postlen = postimage->len;
> + size_t imglen[preimage->nr];
> + for (i = 0; i < preimage->nr; i++) {
> + imglen[i] = img->line[try_lno+i].len;
> + size_t prelen = preimage->line[i].len;
> + if (!fuzzy_matchlines(
> + img->buf + try + imgoff, imglen[i],
> + preimage->buf + preoff, prelen))
> + return 0;
> + if (preimage->line[i].flag & LINE_COMMON)
> + postlen += imglen[i] - prelen;
> + imgoff += imglen[i];
> + preoff += prelen;
> + }
> +
> + /*
> + * Ok, the preimage matches with whitespace fuzz. Update it and
> + * the common postimage lines to use the same whitespace as the
> + * target. imgoff now holds the true length of the target that
> + * matches the preimage, and we need to update the line lengths
> + * of the preimage to match the target ones.
> + */
> + fixed_buf = xmalloc(imgoff);
> + memcpy(fixed_buf, img->buf + try, imgoff);
> + for (i = 0; i < preimage->nr; i++)
> + preimage->line[i].len = imglen[i];
> +
> + /*
> + * Update the preimage buffer and the postimage context lines.
> + */
> + update_pre_post_images(preimage, postimage,
> + fixed_buf, imgoff, postlen);
> + return 1;
> + }
> +
Why do you need imglen[] vla here? IOW, can't the above be simply like
this?
diff --git a/builtin-apply.c b/builtin-apply.c
index ae11b41..c8372a0 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1874,20 +1874,18 @@ static int match_fragment(struct image *img,
if (ws_ignore_action == ignore_ws_change) {
size_t imgoff = 0;
size_t preoff = 0;
size_t postlen = postimage->len;
- size_t imglen[preimage->nr];
for (i = 0; i < preimage->nr; i++) {
size_t prelen = preimage->line[i].len;
+ size_t imglen = img->line[try_lno+i].len;
- imglen[i] = img->line[try_lno+i].len;
- if (!fuzzy_matchlines(
- img->buf + try + imgoff, imglen[i],
- preimage->buf + preoff, prelen))
+ if (!fuzzy_matchlines(img->buf + try + imgoff, imglen,
+ preimage->buf + preoff, prelen))
return 0;
if (preimage->line[i].flag & LINE_COMMON)
- postlen += imglen[i] - prelen;
- imgoff += imglen[i];
+ postlen += imglen - prelen;
+ imgoff += imglen;
preoff += prelen;
}
/*
@@ -1899,9 +1897,9 @@ static int match_fragment(struct image *img,
*/
fixed_buf = xmalloc(imgoff);
memcpy(fixed_buf, img->buf + try, imgoff);
for (i = 0; i < preimage->nr; i++)
- preimage->line[i].len = imglen[i];
+ preimage->line[i].len = img->line[try_lno+i].len;
/*
* Update the preimage buffer and the postimage context lines.
*/
^ permalink raw reply related
* Re: stash --dwim safety
From: Jeff King @ 2009-09-01 6:57 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, git
In-Reply-To: <vpqocpv2n93.fsf@bauges.imag.fr>
On Tue, Sep 01, 2009 at 08:27:20AM +0200, Matthieu Moy wrote:
> I was actually thinking of being a little more paranoid to prevent
> accidental "stash save": we could refuse to create a named stash when
> the "save" command is not given. The case I hadn't thought of was "git
> stash -q apply", which has 99% chances of being a typo for "git stash
> apply -q", and which would mean "create a stash named apply, quietly".
I like that. I think it addresses Dscho's concern with mistakes causing
an unexpected stash, and it is actually more consistent with the current
rule (that named stashes need an explicit 'save'). IOW, it is actually a
bit confusing that "git stash foo" doesn't work, but "git stash -k foo"
does.
-Peff
^ permalink raw reply
* Re: [PATCH] fast-import.c: Silence build warning
From: Alex Riesen @ 2009-09-01 6:30 UTC (permalink / raw)
To: Michael Wookey; +Cc: Junio C Hamano, git
In-Reply-To: <d2e97e800908311655t553d6c4bo6ed45fe37819c1d8@mail.gmail.com>
On Tue, Sep 1, 2009 at 01:55, Michael Wookey<michaelwookey@gmail.com> wrote:
>> Otherwise a clever compiler has every right to complain "the variable
>> unused is assigned but never used."
>
> I get no other warnings, so does that make gcc less than clever? ;-)
It only does what it is instructed to do: the function is annotated with
warn_unused_result attribute. What really is annoying is someones
choice of the functions to annotate.
^ permalink raw reply
* Re: stash --dwim safety
From: Matthieu Moy @ 2009-09-01 6:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3a77dx5b.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> It turns out that the rework was simple enough, so I did it myself. Among
> his 3 patch series, an equivalent to the first one ("save -keep" can be
> written as "save -k" for brevity) were already in, and the second one
> (default to "save" if we see any option before command word) was unsafe
> without the third one (reject unknown option to "save"), so it ended up as
> a single patch that is a combination of the latter two patches.
Thanks, lack of time on my side to work on this, sorry.
I was actually thinking of being a little more paranoid to prevent
accidental "stash save": we could refuse to create a named stash when
the "save" command is not given. The case I hadn't thought of was "git
stash -q apply", which has 99% chances of being a typo for "git stash
apply -q", and which would mean "create a stash named apply, quietly".
> +# The default command is "save"
> +case "$1" in
> +-*)
> + set "save" "$@"
> + ;;
> +esac
So, that could become something like
default_to_save=t
for arg in "$@"; do
case "$arg" in
-*)
;;
*)
default_to_save=
esac
done
if [ "$default_to_save" = t ]; then
set "save" "$@"
fi
(untested)
--
Matthieu
^ permalink raw reply
* Re: from local to github
From: Matthieu Moy @ 2009-09-01 6:14 UTC (permalink / raw)
To: sigbackup; +Cc: git
In-Reply-To: <48b054040908311346y853b917l44a7e5d4310501b@mail.gmail.com>
sigbackup <sigbackup@gmail.com> writes:
> Hello guys,
> I'm a git newbie and I'm looking for some good references about using
> git locally (on Mac) and synchronize my repositories to my github
> account and from there to a Win2003 production server.
github probably told you how to clone your github repo on your local
machine. Then, use "git pull" and "git push" to get changes from
github and to send your changes there.
This is very much the standard way to use Git, so I guess reading the
doc can help ;-).
--
Matthieu
^ permalink raw reply
* stash --dwim safety (was Re: What's cooking in git.git (Aug 2009, #06; Sun, 30))
From: Junio C Hamano @ 2009-09-01 5:58 UTC (permalink / raw)
To: git; +Cc: Matthieu Moy
In-Reply-To: <7viqg48nxi.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> [Stalled]
>
> * js/stash-dwim (2009-07-27) 1 commit.
> * tr/reset-checkout-patch (2009-08-27) 9 commits.
>
> There was a discussion on better DWIMmery for the above two topics to (1)
> forbid "git stash save --anything-with-dash" and (2) redirect with any
> option "git stash --opt" to "git stash save --opt", to keep it flexible
> and safe at the same time. I think it is a sane thing to do, but nothing
> has happened lately.
Actually, I was at fault giving up on Matthieu's patch without studying it
after seeing the phrase "this series replaces", when the two topics the
series tried to replace were already in 'next'.
It turns out that the rework was simple enough, so I did it myself. Among
his 3 patch series, an equivalent to the first one ("save -keep" can be
written as "save -k" for brevity) were already in, and the second one
(default to "save" if we see any option before command word) was unsafe
without the third one (reject unknown option to "save"), so it ended up as
a single patch that is a combination of the latter two patches.
This applies on top of tr/reset-checkout-patch branch, 14c674e (Make test
case number unique, 2009-08-27).
-- >8 --
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Tue, 18 Aug 2009 23:38:40 +0200
Subject: [PATCH] stash: simplify defaulting to "save" and reject unknown options
With the earlier DWIM patches, certain combination of options defaulted
to the "save" command correctly while certain equally valid combination
did not. For example, "git stash -k" were Ok but "git stash -q -k" did
not work.
This makes the logic of defaulting to "save" much simpler. If the first
argument begins with a '-', it is clear that there is no command word,
and we default to "save" subcommand.
This also teaches "git stash save" to reject an unknown option. This is
to keep a mistyped "git stash save --quite" from creating a stash with a
message "--quite", and this safety is more important with the new logic
to default to "save" with any option-looking argument without an explicit
comand word.
[jc: this is based on Matthieu's 3-patch series, and he takes all the
credit; if I have introduced bugs while reworking they are mine]
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-stash.txt | 1 -
git-stash.sh | 22 ++++++++++++++++++----
t/t3903-stash.sh | 11 +++++++++++
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 1c4ed41..5d4cce3 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -14,7 +14,6 @@ SYNOPSIS
'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
'git stash' branch <branchname> [<stash>]
'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet] [<message>]]
-'git stash' [-p|--patch|-k|--keep-index]
'git stash' clear
'git stash' create
diff --git a/git-stash.sh b/git-stash.sh
index 9fd7289..ff71507 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -8,7 +8,6 @@ USAGE="list [<options>]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: $dashless branch <branchname> [<stash>]
or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]
- or: $dashless [-k|--keep-index]
or: $dashless clear"
SUBDIRECTORY_OK=Yes
@@ -146,6 +145,14 @@ save_stash () {
-q|--quiet)
GIT_QUIET=t
;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ echo "error: unknown option for 'stash save': $1"
+ usage
+ ;;
*)
break
;;
@@ -355,6 +362,13 @@ apply_to_branch () {
drop_stash $stash
}
+# The default command is "save"
+case "$1" in
+-*)
+ set "save" "$@"
+ ;;
+esac
+
# Main command set
case "$1" in
list)
@@ -406,9 +420,9 @@ branch)
apply_to_branch "$@"
;;
*)
- case $#,"$1","$2" in
- 0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)
- save_stash "$@" &&
+ case $# in
+ 0)
+ save_stash &&
say '(To restore them type "git stash apply")'
;;
*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e16ad93..5514f74 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -208,4 +208,15 @@ test_expect_success 'stash -k' '
test bar,bar4 = $(cat file),$(cat file2)
'
+test_expect_success 'stash --invalid-option' '
+ echo bar5 > file &&
+ echo bar6 > file2 &&
+ git add file2 &&
+ test_must_fail git stash --invalid-option &&
+ test_must_fail git stash save --invalid-option &&
+ test bar5,bar6 = $(cat file),$(cat file2) &&
+ git stash -- -message-starting-with-dash &&
+ test bar,bar2 = $(cat file),$(cat file2)
+'
+
test_done
--
1.6.4.2.295.g9bcb
^ permalink raw reply related
* [PATCH] Style fixes, add a space after if/for/while.
From: Brian Gianforcaro @ 2009-09-01 5:35 UTC (permalink / raw)
To: git, gitster
The majority of code in core git appears to use a single
space after if/for/while. This is an attempt to bring more
code to this standard. These are entirely cosmetic changes.
Signed-off-by: Brian Gianforcaro <b.gianfo@gmail.com>
---
builtin-blame.c | 2 +-
builtin-clone.c | 2 +-
builtin-for-each-ref.c | 2 +-
builtin-pack-objects.c | 2 +-
builtin-remote.c | 4 ++--
builtin-shortlog.c | 2 +-
connect.c | 2 +-
diff.c | 2 +-
pack-redundant.c | 28 ++++++++++++++--------------
remote.c | 2 +-
upload-pack.c | 2 +-
var.c | 4 ++--
wt-status.c | 2 +-
13 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index fd6ca51..7512773 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -1348,7 +1348,7 @@ static void get_ac_line(const char *inbuf, const char *what,
/*
* Now, convert both name and e-mail using mailmap
*/
- if(map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
+ if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
/* Add a trailing '>' to email, since map_user returns plain emails
Note: It already has '<', since we replace from mail+1 */
mailpos = memchr(mail, '\0', mail_len);
diff --git a/builtin-clone.c b/builtin-clone.c
index 0d2b4a8..991a7ae 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -515,7 +515,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_upload_pack);
refs = transport_get_remote_refs(transport);
- if(refs)
+ if (refs)
transport_fetch_refs(transport, refs);
}
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index d7cc8ca..a5a83f1 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -576,7 +576,7 @@ static void populate_value(struct refinfo *ref)
if (!prefixcmp(name, "refname"))
refname = ref->refname;
- else if(!prefixcmp(name, "upstream")) {
+ else if (!prefixcmp(name, "upstream")) {
struct branch *branch;
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index c433748..c918d4b 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1808,7 +1808,7 @@ static void prepare_pack(int window, int depth)
static int git_pack_config(const char *k, const char *v, void *cb)
{
- if(!strcmp(k, "pack.window")) {
+ if (!strcmp(k, "pack.window")) {
window = git_config_int(k, v);
return 0;
}
diff --git a/builtin-remote.c b/builtin-remote.c
index 008abfe..0777dd7 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -385,7 +385,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
fetch_map, 1);
- for(ref = matches; ref; ref = ref->next)
+ for (ref = matches; ref; ref = ref->next)
string_list_append(abbrev_branch(ref->name), &states->heads);
free_refs(fetch_map);
@@ -484,7 +484,7 @@ static int read_remote_branches(const char *refname,
const char *symref;
strbuf_addf(&buf, "refs/remotes/%s", rename->old);
- if(!prefixcmp(refname, buf.buf)) {
+ if (!prefixcmp(refname, buf.buf)) {
item = string_list_append(xstrdup(refname), rename->remote_branches);
symref = resolve_ref(refname, orig_sha1, 1, &flag);
if (flag & REF_ISSYMREF)
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 6a3812e..4d4a3c8 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -56,7 +56,7 @@ static void insert_one_record(struct shortlog *log,
/* copy author name to namebuf, to support matching on both name and email */
memcpy(namebuf, author, boemail - author);
len = boemail - author;
- while(len > 0 && isspace(namebuf[len-1]))
+ while (len > 0 && isspace(namebuf[len-1]))
len--;
namebuf[len] = 0;
diff --git a/connect.c b/connect.c
index 76e5427..7945e38 100644
--- a/connect.c
+++ b/connect.c
@@ -513,7 +513,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
signal(SIGCHLD, SIG_DFL);
host = strstr(url, "://");
- if(host) {
+ if (host) {
*host = '\0';
protocol = get_protocol(url);
host += 3;
diff --git a/diff.c b/diff.c
index cd35e0c..e1be189 100644
--- a/diff.c
+++ b/diff.c
@@ -2691,7 +2691,7 @@ static int parse_num(const char **cp_p)
num = 0;
scale = 1;
dot = 0;
- for(;;) {
+ for (;;) {
ch = *cp;
if ( !dot && ch == '.' ) {
scale = 1;
diff --git a/pack-redundant.c b/pack-redundant.c
index 48a12bc..a3fad74 100644
--- a/pack-redundant.c
+++ b/pack-redundant.c
@@ -55,7 +55,7 @@ static inline struct llist_item *llist_item_get(void)
} else {
int i = 1;
new = xmalloc(sizeof(struct llist_item) * BLKSIZE);
- for(;i < BLKSIZE; i++) {
+ for (;i < BLKSIZE; i++) {
llist_item_put(&new[i]);
}
}
@@ -64,7 +64,7 @@ static inline struct llist_item *llist_item_get(void)
static void llist_free(struct llist *list)
{
- while((list->back = list->front)) {
+ while ((list->back = list->front)) {
list->front = list->front->next;
llist_item_put(list->back);
}
@@ -146,7 +146,7 @@ static inline struct llist_item *llist_insert_sorted_unique(struct llist *list,
if (cmp > 0) { /* we insert before this entry */
return llist_insert(list, prev, sha1);
}
- if(!cmp) { /* already exists */
+ if (!cmp) { /* already exists */
return l;
}
prev = l;
@@ -168,7 +168,7 @@ redo_from_start:
int cmp = hashcmp(l->sha1, sha1);
if (cmp > 0) /* not in list, since sorted */
return prev;
- if(!cmp) { /* found */
+ if (!cmp) { /* found */
if (prev == NULL) {
if (hint != NULL && hint != list->front) {
/* we don't know the previous element */
@@ -218,7 +218,7 @@ static inline struct pack_list * pack_list_insert(struct pack_list **pl,
static inline size_t pack_list_size(struct pack_list *pl)
{
size_t ret = 0;
- while(pl) {
+ while (pl) {
ret++;
pl = pl->next;
}
@@ -396,7 +396,7 @@ static size_t get_pack_redundancy(struct pack_list *pl)
return 0;
while ((subset = pl->next)) {
- while(subset) {
+ while (subset) {
ret += sizeof_union(pl->pack, subset->pack);
subset = subset->next;
}
@@ -427,7 +427,7 @@ static void minimize(struct pack_list **min)
pl = local_packs;
while (pl) {
- if(pl->unique_objects->size)
+ if (pl->unique_objects->size)
pack_list_insert(&unique, pl);
else
pack_list_insert(&non_unique, pl);
@@ -479,7 +479,7 @@ static void minimize(struct pack_list **min)
*min = min_perm;
/* add the unique packs to the list */
pl = unique;
- while(pl) {
+ while (pl) {
pack_list_insert(min, pl);
pl = pl->next;
}
@@ -516,7 +516,7 @@ static void cmp_local_packs(void)
struct pack_list *subset, *pl = local_packs;
while ((subset = pl)) {
- while((subset = subset->next))
+ while ((subset = subset->next))
cmp_two_packs(pl, subset);
pl = pl->next;
}
@@ -608,23 +608,23 @@ int main(int argc, char **argv)
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
- if(!strcmp(arg, "--")) {
+ if (!strcmp(arg, "--")) {
i++;
break;
}
- if(!strcmp(arg, "--all")) {
+ if (!strcmp(arg, "--all")) {
load_all_packs = 1;
continue;
}
- if(!strcmp(arg, "--verbose")) {
+ if (!strcmp(arg, "--verbose")) {
verbose = 1;
continue;
}
- if(!strcmp(arg, "--alt-odb")) {
+ if (!strcmp(arg, "--alt-odb")) {
alt_odb = 1;
continue;
}
- if(*arg == '-')
+ if (*arg == '-')
usage(pack_redundant_usage);
else
break;
diff --git a/remote.c b/remote.c
index c3ada2d..4b5b905 100644
--- a/remote.c
+++ b/remote.c
@@ -1038,7 +1038,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
case 0:
if (!memcmp(dst_value, "refs/", 5))
matched_dst = make_linked_ref(dst_value, dst_tail);
- else if((dst_guess = guess_ref(dst_value, matched_src)))
+ else if ((dst_guess = guess_ref(dst_value, matched_src)))
matched_dst = make_linked_ref(dst_guess, dst_tail);
else
error("unable to push to unqualified destination: %s\n"
diff --git a/upload-pack.c b/upload-pack.c
index 4d8be83..dacbc76 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -427,7 +427,7 @@ static int get_common_commits(void)
save_commit_buffer = 0;
- for(;;) {
+ for (;;) {
int len = packet_read_line(0, line, sizeof(line));
reset_timeout();
diff --git a/var.c b/var.c
index 7362ed8..a807dea 100644
--- a/var.c
+++ b/var.c
@@ -21,7 +21,7 @@ static struct git_var git_vars[] = {
static void list_vars(void)
{
struct git_var *ptr;
- for(ptr = git_vars; ptr->read; ptr++) {
+ for (ptr = git_vars; ptr->read; ptr++) {
printf("%s=%s\n", ptr->name, ptr->read(IDENT_WARN_ON_NO_NAME));
}
}
@@ -31,7 +31,7 @@ static const char *read_var(const char *var)
struct git_var *ptr;
const char *val;
val = NULL;
- for(ptr = git_vars; ptr->read; ptr++) {
+ for (ptr = git_vars; ptr->read; ptr++) {
if (strcmp(var, ptr->name) == 0) {
val = ptr->read(IDENT_ERROR_ON_NO_NAME);
break;
diff --git a/wt-status.c b/wt-status.c
index 63598ce..3395456 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -326,7 +326,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
setup_standard_excludes(&dir);
fill_directory(&dir, NULL);
- for(i = 0; i < dir.nr; i++) {
+ for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
if (!cache_name_is_other(ent->name, ent->len))
continue;
--
1.6.3.3
^ permalink raw reply related
* Re: What's cooking in git.git (Aug 2009, #05; Wed, 26)
From: Daniel Barkalow @ 2009-09-01 4:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhbvnzhdq.fsf@alter.siamese.dyndns.org>
On Mon, 31 Aug 2009, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Sat, 29 Aug 2009, Junio C Hamano wrote:
> >
> >> ..., if only to avoid confusion with our own earlier misdesigned
> >> syntax git+ssh://), so the canonical syntax would be:
> >
> > (with the syntax <helper>+, "git+ssh://" would specify the helper "git",
> > which is as good an explicit identifier of the internal handling as any)
>
> Sure but how would you explain "ssh+git://" then ;-)?
That's just weird, and looks wrong to me, like ssh tunnelled over the git
native protocol. I think, if we support it, it would have to be another
special case.
> Luckily neither is advertised in our documentation set as far as I can
> tell, so I do not think it is a huge deal between + vs ::, but as Peff
> says in
>
> http://thread.gmane.org/gmane.comp.version-control.git/125615
>
> I think the latter is probably less problematic.
>
> With plus, a helper that talks with a Subversion repository whose native
> URL is http://host/path would look like svn+http://host/path, which is
> reasonable. When talking the Subversion protocol over SSH, however, the
> native URL for the repository would be svn+ssh://host/path, so the URL
> with helper name on our side becomes svn+svn+ssh://host/path. We could
> recognize "svn+" part and implicitly pass the whole thing to svn helper
> upon seeing svn+ssh://host/path, but I do not think we would want to make
> the dispatcher too familiar with what the backends do. Using something
> other than plus sign would avoid this issue.
I was thinking that the dispatcher would always pass the whole thing, and
the svn helper would have to deal with svn+http:// and svn+svn:// as well
as svn+ssh://. The dispatcher shouldn't be familiar with what the backends
do, but the backends could stand to be familiar with what might be there
just to get to them. On the other hand, I don't have a good intuition for
what makes sense to users of such systems, since I've only extensively
used cvs, git, and perforce.
> > If the policy is that we're going to have "traditionally supported"
> > schemes, where the internal code knows what helper supports them, I can
> > fix up the series so that the curl-based helper is named "curl", and we
> > can say that the check for "http://", "ftp://", and "https://" is
> > recognizing traditionally-supported schemes, and we can defer coming up
> > with what the syntax for the explicit handler selection is. (For that
> > matter, if there's a // after the colon, it's obviously not a
> > handy ssh-style location, since the second slash would do nothing)
>
> That sounds like a sane approach to first get the "eject curl from
> builtin" out the door. We might extend the dispatcher in the future by
> changing "traditionally supported" criterion to "commonly used",
> e.g. recognize "svn+ssh://" as something the svn helper would want to
> handle, but that is a future extension we do not have to address right
> now.
Fair enough.
> >> After you explained this in the thread (I think for the second time), I
> >> see no problem with this, except that I think to support this we should
> >> notice and raise an error when we see a remote has both vcs and url,
> >> because the only reason we would want to say "vcs", if I recall your
> >> explanation correctly, is that such a transport does not have the concept
> >> of URL, i.e. a well defined single string that identifies the repository.
> >
> > A user who mostly uses Perforce as a foreign repositories but is using a
> > SVN repo on occasion might want to use "vcs" regardless, but I agree with
> > forcing the helper to use a different option for the case of a URL that
> > git isn't going to look at. That is, you ought to be able to use:
> >
> > [remote "origin"]
> > vcs = svn
> > (something) = http://svn.savannah.gnu.org/...
> >
> > But "(something)" shouldn't be "url".
>
> I actuallly do not have a strong opinion on this one either way. I said
> "I think" when I suggested it, but it was actually without thinking too
> deeply, hoping that you would come up with a good counter-argument.
>
> For example, if we envision that for most of the helpers there will be one
> primary string that identifies the repository, but the primary string
> alone is not enough for the helper without some auxiliary information, it
> would be natural to use remote.$name.url for that primary string. I do
> not know if that would be the case, but I was hoping that you would have a
> better intuition[*1*], as you have thought this topic through a lot longer
> and deeper than I have. So I'd rather leave the decision on that "no
> vcs/url at the same time restriction" up to you. It is in general easier
> to start more strict and then loosen the restiction later, than the other
> way around, when we cannot decide, though.
Agreed.
> Thanks.
>
> [Footnote]
>
> *1* What I mean by intuition is that you do not have to have the right
> answer backed by research _now_, but have a good guess as to what the
> right answer would be.
I don't really have a particularly good guess about the case of vcs and a
URL, because I haven't used systems that are not git and use URLs.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] fast-import.c: Silence build warning
From: Stephen Boyd @ 2009-09-01 4:06 UTC (permalink / raw)
To: Michael Wookey; +Cc: Junio C Hamano, git
In-Reply-To: <d2e97e800908311655t553d6c4bo6ed45fe37819c1d8@mail.gmail.com>
Michael Wookey wrote:
> 2009/9/1 Junio C Hamano <gitster@pobox.com>:
>> Isn't this typically done by casting the expression to (void)?
>
> I originally tried that - the compiler still complains.
>
>> Otherwise a clever compiler has every right to complain "the variable
>> unused is assigned but never used.
>
> I get no other warnings, so does that make gcc less than clever? ;-)
I noticed this warning recently too when I upgraded my box and a flurry
of fwrite() unused warnings came up. Looks like ubuntu patches that
issue[1] by arguing it's a valid programming style to
fwrite/fflush/ferror. Perhaps this programming style could follow a
similar reasoning?
It gets better though. Commit c55fae4 (fast-import.c: stricter strtoul
check, silence compiler warning, 2008-12-21) made this change already.
Then commit eb3a9dd (Remove unused function scope local variables,
2009-03-07) came by and removed it. Unless the definition of strtoul
drops the attribute I fear we'll keep going back and forth.
-- Footnotes --
[1] https://lists.ubuntu.com/archives/ubuntu-devel/2009-March/027832.html
^ permalink raw reply
* Re: Improve on 'approxidate'
From: Linus Torvalds @ 2009-09-01 3:27 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20090830223558.GA29807@coredump.intra.peff.net>
On Sun, 30 Aug 2009, Jeff King wrote:
>
> This breaks relative dates like "3.months.ago", because
> approxidate_alpha needs to see the "current" date in tm (and now it sees
> -1, subtracts from it, and assumes we are just crossing a year boundary
> because of the negative). 3.years.ago is also broken, but I don't think
> 3.days.ago is.
Gaah. Thanks for noticing and the fixes. I had tested the relative modes,
but only the "fixed offset" ones (days, hours, minutes, seconds), not the
months and years cases.
Linus
^ permalink raw reply
* Re: [PATCH v2 3/4] tests: add date printing and parsing tests
From: Jeff King @ 2009-09-01 3:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Alex Riesen, git
In-Reply-To: <20090831023015.GC5507@coredump.intra.peff.net>
On Sun, Aug 30, 2009 at 10:30:15PM -0400, Jeff King wrote:
> - We confirm that the improvements in b5373e9 and 1bddb25
> work.
Ugh. I just realized (when explaining how awesome git resurrect was in
another mail) that I managed to bungle these commit hashes (and the one
mentioned in the following patch).
What happened is that I was building on the topic branch and lazily did
a "rebase -i origin" to fix up my patches. I left the first two patches
untouched, of course, but they still ended up with new committer
information.
As my patches are merged to 'next' already, I think it is too late to
fixup the commit message. But for posterity, the correct referenced
commits are 9029055 and 36e4986.
Caveat rebaser.
-Peff
^ permalink raw reply
* Re: A note from the maintainer: Follow-up questions (MaintNotes)
From: Jeff King @ 2009-09-01 2:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Chanters, git
In-Reply-To: <7v8wgzla02.fsf@alter.siamese.dyndns.org>
On Mon, Aug 31, 2009 at 06:38:37PM -0700, Junio C Hamano wrote:
> a71f64a Merge branch 'pk/import-dirs' into pu
> ce6cd39 Merge branch 'jh/cvs-helper' into pu
> ...
> 2178d02 Merge branch 'jc/log-tz' into pu
> ...
> 927d129 Merge branch 'lt/approxidate' into jch
> 35ada54 Merge branch 'tr/reset-checkout-patch' into jch
> d82f86c Merge branch 'db/vcs-helper' (early part) into jch
>
> So if you for example happen to be interested in jc/log-tz topic,
> you would do something like:
>
> $ git checkout -b jc/log-tz 2178d02^2
> $ git log -p master..
>
> to check out, and view what changes the topic introduces.
Another alternative: Thomas Rast wrote a handy script called
'git-resurrect' which does this for you. It's carried in the contrib/
section right now. I used it just the other day to pull out the
lt/approxidate topic branch, which I then built some follow-up patches
for. Usage is something like:
# contrib scripts aren't installed by "make install",
# so put it somewhere in your PATH
$ cp contrib/git-resurrect.sh ~/bin/git-resurrect
$ git resurrect -m lt/approxidate
** Candidates for lt/approxidate **
931e8e2 [22 hours ago] fix approxidate parsing of relative months and years
** Restoring lt/approxidate to 931e8e2 fix approxidate parsing of relative months and years
-Peff
^ permalink raw reply
* Re: Troubles building man pages
From: Junio C Hamano @ 2009-09-01 2:44 UTC (permalink / raw)
To: Brian S. Schang; +Cc: git
In-Reply-To: <4A9C8750.60308@lists.schang.net>
"Brian S. Schang" <git@lists.schang.net> writes:
>> ASCIIDOC git-am.xml
>> XMLTO git-am.1
>> I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd
>> /raida/packages/git/Documentation/git-am.xml:2: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
>> D DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
>
> I have read through INSTALL and Documentation/README. I have tried to
> cover all dependencies, but I suspect that I'm missing something
> simple.
>
> Note that 'git' itself compiles fine and I can build the HTML pages
> fine also. It seems to be the man pages that cause the problem.
>
> Note that I am using openSUSE 11.1. I was able to build the man pages
> on my older openSUSE 11.0 machine, but not any longer.
The following was an ancient experience of mine with a different distro
but I am reasonably sure it would point you in the right direction.
http://article.gmane.org/gmane.comp.version-control.git/107387
^ permalink raw reply
* Troubles building man pages
From: Brian S. Schang @ 2009-09-01 2:30 UTC (permalink / raw)
To: git
Hello:
I am having problem building the man pages for git v1.6.4.2. I am
getting the follow stream of errors: (sorry for the long lines)
> # make ASCIIDOC8=Yes man
> GIT_VERSION = 1.6.4.2
> make -C Documentation man
> make[1]: Entering directory `/raida/packages/git/Documentation'
> GEN doc.dep
> make[2]: Entering directory `/raida/packages/git'
> make[2]: `GIT-VERSION-FILE' is up to date.
> make[2]: Leaving directory `/raida/packages/git'
> make[1]: Leaving directory `/raida/packages/git/Documentation'
> make[1]: Entering directory `/raida/packages/git/Documentation'
> make[2]: Entering directory `/raida/packages/git'
> make[2]: `GIT-VERSION-FILE' is up to date.
> make[2]: Leaving directory `/raida/packages/git'
> ASCIIDOC git-add.xml
> XMLTO git-add.1
> I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd
> /raida/packages/git/Documentation/git-add.xml:2: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
> D DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
> ^
> ASCIIDOC git-am.xml
> XMLTO git-am.1
> I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd
> /raida/packages/git/Documentation/git-am.xml:2: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
> D DocBook XML V4.2//EN" "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
I have read through INSTALL and Documentation/README. I have tried to
cover all dependencies, but I suspect that I'm missing something simple.
Note that 'git' itself compiles fine and I can build the HTML pages fine
also. It seems to be the man pages that cause the problem.
Note that I am using openSUSE 11.1. I was able to build the man pages on
my older openSUSE 11.0 machine, but not any longer.
I would appreciate any advice you are willing to share.
Thank you.
--
Brian Schang
^ permalink raw reply
* Re: A note from the maintainer: Follow-up questions (MaintNotes)
From: Junio C Hamano @ 2009-09-01 1:38 UTC (permalink / raw)
To: David Chanters; +Cc: git
In-Reply-To: <ac3d41850908301519s2cf8a45auf11fb4c9285c0cb5@mail.gmail.com>
David Chanters <david.chanters@googlemail.com> writes:
> I'd often wondered when I have read various posts of the git mailing
> list on gmane, just how it is I am supposed to track:
>
> dc/some-topic-feature
>
> ... Junio, are these topic branches ones you actively have somewhere
> in your own private checkout? Yes, I appreciate that when I read a
> given post to the mailing list, you or other people will sometimes
> make reference to these topic branches, but what do I do if I am
> interested in finding out about one of them?
$ git log --oneline --first-parent origin/master..origin/pu
would be a handy way to view where the tip of each branch is.
a71f64a Merge branch 'pk/import-dirs' into pu
ce6cd39 Merge branch 'jh/cvs-helper' into pu
...
2178d02 Merge branch 'jc/log-tz' into pu
...
927d129 Merge branch 'lt/approxidate' into jch
35ada54 Merge branch 'tr/reset-checkout-patch' into jch
d82f86c Merge branch 'db/vcs-helper' (early part) into jch
So if you for example happen to be interested in jc/log-tz topic,
you would do something like:
$ git checkout -b jc/log-tz 2178d02^2
$ git log -p master..
to check out, and view what changes the topic introduces.
Some hawk-eyed people may have already noticed this, but I recently
updated the script I used to maintain the "What's cooking" messages, and
the entries come with when and at what commit each part of the series has
been merged to 'next'. For example, the lt/approxidate topic reads like
this:
* lt/approxidate (2009-08-30) 6 commits
(merged to 'next' on 2009-08-30 at e016e3d)
+ fix approxidate parsing of relative months and years
+ tests: add date printing and parsing tests
+ refactor test-date interface
+ Add date formatting and parsing functions relative to a given time
(merged to 'next' on 2009-08-26 at 62853f9)
+ Further 'approxidate' improvements
+ Improve on 'approxidate'
Time flows from bottom to top in this output, so this tells us that the
first two patches in the series has been in 'next' for five days or so,
and the tests and a fix in 4 follow-up patches came later, merged to
'next' yesterday. You can use
$ git checkout -b lt/approxidate e016e3d^2
to get at its tip.
> ..., how do you go about creating and
> maintaining these topic branches -- are you making heavy use of "git
> am"
Save patches from the list in mbox, review them in the mbox while fixing
trivial breakages, and finally:
$ git checkout -b ai/topic-name master
$ git am -s that.mbox
where "ai" is typically the author's initial, and topic-name names the
topic just like you would name a function. A topic typically forks from
the tip of master if it is a new feature, or a much older commit in maint
if it is a fix (and in such a case, topic-name typically begins with
a string "maint-").
> I ask because of the following snippet from "MaintNotes":
>
> The two branches "master" and "maint" are never rewound, and
> "next" usually will not be either (this automatically means the
> topics that have been merged into "next" are usually not
> rebased, and you can find the tip of topic branches you are
> interested in from the output of "git log next"). You should be
> able to safely track them.
>
> I am not sure if there's any real use-case for this, but I will ask
> anyway: is the above saying that I am able to *checkout* one of these
> topic-branches just from their presence in "next" alone? I appreciate
> that the point is somewhat moot since the topic branch has already
> been merged into "next", but I can surely see this as a really useful
> way for people to manage topic-branches in a shared environment:
> people can simply pick a topic branch out from the integrated one --
> in this case "next".
Surely, see above. And by checking out the topic alone, you can test and
enhance it in isolation.
If you come up with a follow-up patch based on one particular topic, in
other words, building on ai/topic-name created like the above example, as
opposed to building on 'next', it would be easier for me to integrate it,
too, because the way I accept a follow-up patch to a particular topic is
by doing this:
$ git checkout ai/topic-name
$ git am -s followup-mail.mbox
The result will be tested in isolation and if it is good it would be
merged to 'next' again.
$ git merge ai/topic-name
> I'm obviously missing something here -- but why is rebasing these
> existing topic branches (I assume on top of "pu") more useful than
> just merging them into "pu" -- like you do with "next"?
Somebody may send a few patches [PATCH 1/3] thru [PATCH 3/3] whose
quality is sub-par but tries to tackle a good problem. I'd queue it in
'pu'.
$ git checkout -b dc/cool-feature
$ git am -s david-chanters.mbox
$ git checkout pu
$ git merge dc/cool-feature
In a few days, people would notice that the series has a lot of room for
improvement, and offer suggestions. You would send replacement patches
based on the review. Perhaps you squashed the first two patches from the
original into one, and added another patch for test suite, and the new
series is marked as [PATCH v2 1/3] thru [PATCH v2 3/3].
In general, the early attempts of a topic that have never been merged to
'next' are not worth keeping in the public history. The original author
is free to keep them in his private tree, but there is no point in forever
keeping earlier mistakes, bugs, typos and implementation based on a flawed
design, all of which later were corrected in _my_ history.
So I would replace the whole topic with the new series.
$ git checkout dc/cool-feature
$ git reset --hard HEAD~3
$ git am -s david-chanters-v2.mbox
The whole point of replacing the contents of this topic was to get rid of
the mistakes in your earlier round, so it does not make much sense to
merge this to 'pu' without rewinding 'pu' first.
After the topic is merged to 'next', obviously I cannot sanely rewind and
rebuild it. Everything goes incremental from then on (see the earlier
example of applying followup-mail.mbox).
> If regular readers of the git mailing list wish to track this topic
> branch, can they do so from you only until it's merged into "next"?
Sorry, but I am not sure if I understand the question.
> And a related question: If you decide a given topic in pu is declared
> to "be dropped", is this done by rebasing (as you mentioned earlier)
> so as to remove any trace of the topic branch ever having been in
> "pu", or am I reading too much into "dropping" here? :)
Essentially, I keep a list of branches that are in 'next' and another list
of branches that are in 'pu'. At the end of each integration cycle, I do
a rough equivalent of:
$ git checkout next
$ git merge ..a good topic that is not fully in next..
$ git merge ..another good topic that is not fully in next..
$ make test ;# make sure next is ok
$ git branch -f pu ;# get rid of everything in pu
$ git checkout pu
$ for branch in ..the list of branches to be in pu..
do
git merge $branch || break
done
Discarding a topic from 'pu' simply means I do not merge the topic in the
last loop.
^ permalink raw reply
* Re: clong an empty repo over ssh causes (harmless) fatal
From: Jeff King @ 2009-09-01 1:08 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Björn Steinbrink, Matthieu Moy, Sitaram Chamarty, git
In-Reply-To: <fabb9a1e0908311550r1b549eb2k2df65c188a0ea6a0@mail.gmail.com>
On Tue, Sep 01, 2009 at 12:50:25AM +0200, Sverre Rabbelier wrote:
> 2009/9/1 Jeff King <peff@peff.net>:
> > AFAICT, this problem goes back to v1.6.2, the first version which
> > handled empty clones. So I blame Sverre. ;)
>
> Eep :(. Any idea what is going on?
Yeah. We call upload-pack on the remote side, realize there are no refs,
and then we just stop talking. Meanwhile upload-pack is waiting for a
packet to say "these are the refs that I want". So the client really
needs to send an extra packet saying "list of refs is finished".
The patch below seems to work for me, but I'm a little concerned how it
might impact other transports. It actually calls the transport's
fetch method when we have no refs that we want. So each transport must
recognize that we want zero refs and do the appropriate thing. In this
case, for the git protocol, we want to:
- do a packet_flush to signal "no more refs" to the remote side
- be aware that we might have zero refs and avoid establishing a new
connection in that case
Other transports might need to be tweaked similarly, but I don't have
time to test at the moment.
diff --git a/builtin-clone.c b/builtin-clone.c
index 0d2b4a8..f198c01 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -515,8 +515,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_upload_pack);
refs = transport_get_remote_refs(transport);
- if(refs)
- transport_fetch_refs(transport, refs);
+ transport_fetch_refs(transport, refs);
}
if (refs) {
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 629735f..04a3776 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -803,6 +803,8 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
nr_heads = remove_duplicates(nr_heads, heads);
if (!ref) {
packet_flush(fd[1]);
+ if (!nr_heads)
+ return NULL;
die("no matching remote head");
}
ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
diff --git a/transport.c b/transport.c
index f2bd998..25e8946 100644
--- a/transport.c
+++ b/transport.c
@@ -512,6 +512,8 @@ static int fetch_refs_via_pack(struct transport *transport,
origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
if (!data->conn) {
+ if (!nr_heads)
+ return 0;
connect_setup(transport, 0, 0);
get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);
}
^ permalink raw reply related
* Re: [PATCH] fast-import.c: Silence build warning
From: Michael Wookey @ 2009-08-31 23:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfxb7y2h3.fsf@alter.siamese.dyndns.org>
2009/9/1 Junio C Hamano <gitster@pobox.com>:
> Michael Wookey <michaelwookey@gmail.com> writes:
>
>> gcc 4.3.3 (Ubuntu 9.04) warns that the return value of strtoul() was not
>> checked by issuing the following notice:
>>
>> warning: ignoring return value of ‘strtoul’, declared with attribute
>> warn_unused_result
>>
>> Provide a dummy variable to keep the compiler happy.
>>
>> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
>> ---
>> fast-import.c | 5 +++--
>> 1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fast-import.c b/fast-import.c
>> index 7ef9865..1386e75 100644
>> --- a/fast-import.c
>> +++ b/fast-import.c
>> @@ -1744,10 +1744,11 @@ static int validate_raw_date(const char *src,
>> char *result, int maxlen)
>> {
>> const char *orig_src = src;
>> char *endp;
>> + unsigned long int unused;
>>
>> errno = 0;
>>
>> - strtoul(src, &endp, 10);
>> + unused = strtoul(src, &endp, 10);
>
> Isn't this typically done by casting the expression to (void)?
I originally tried that - the compiler still complains.
> Otherwise a clever compiler has every right to complain "the variable
> unused is assigned but never used."
I get no other warnings, so does that make gcc less than clever? ;-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox