[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198
ojan at chromium.org
ojan at chromium.org
Sun Feb 20 22:52:59 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit fc99592beca32b28fc88bbc7b2a9bac1dcd380c0
Author: ojan at chromium.org <ojan at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jan 12 23:27:42 2011 +0000
2011-01-12 Ojan Vafai <ojan at chromium.org>
Reviewed by Adam Barth.
Bugzilla: Add keyboard shortcuts to jump to next change
https://bugs.webkit.org/show_bug.cgi?id=52305
Comments and diff blocks go in the same queue. If you have a
comment focused, then j/k will focus the next/prev diff block
with respect to that comment.
* PrettyPatch/PrettyPatch.rb:
* code-review.js:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75646 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Websites/bugs.webkit.org/ChangeLog b/Websites/bugs.webkit.org/ChangeLog
index 6e2ee04..6b5f678 100644
--- a/Websites/bugs.webkit.org/ChangeLog
+++ b/Websites/bugs.webkit.org/ChangeLog
@@ -2,6 +2,20 @@
Reviewed by Adam Barth.
+ Bugzilla: Add keyboard shortcuts to jump to next change
+ https://bugs.webkit.org/show_bug.cgi?id=52305
+
+ Comments and diff blocks go in the same queue. If you have a
+ comment focused, then j/k will focus the next/prev diff block
+ with respect to that comment.
+
+ * PrettyPatch/PrettyPatch.rb:
+ * code-review.js:
+
+2011-01-12 Ojan Vafai <ojan at chromium.org>
+
+ Reviewed by Adam Barth.
+
show shared lines on both sides in code review tool
https://bugs.webkit.org/show_bug.cgi?id=52308
diff --git a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
index 84b1ff2..6c8cb3c 100644
--- a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
+++ b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
@@ -374,7 +374,8 @@ body {
}
.focused {
- border: 1px solid blue;
+ outline: 1px solid blue;
+ outline-offset: -1px;
}
.statusBubble {
diff --git a/Websites/bugs.webkit.org/code-review.js b/Websites/bugs.webkit.org/code-review.js
index b0786f7..696a08a 100644
--- a/Websites/bugs.webkit.org/code-review.js
+++ b/Websites/bugs.webkit.org/code-review.js
@@ -97,18 +97,18 @@
});
}
- function diffSectionFrom(line) {
+ function fileDiffFor(line) {
return line.parents('.FileDiff');
}
function activeCommentFor(line) {
// Scope to the diffSection as a performance improvement.
- return $('textarea[data-comment-for~="' + line[0].id + '"]', diffSectionFrom(line));
+ return $('textarea[data-comment-for~="' + line[0].id + '"]', fileDiffFor(line));
}
function previousCommentsFor(line) {
// Scope to the diffSection as a performance improvement.
- return $('div[data-comment-for~="' + line[0].id + '"].previousComment', diffSectionFrom(line));
+ return $('div[data-comment-for~="' + line[0].id + '"].previousComment', fileDiffFor(line));
}
function findCommentPositionFor(line) {
@@ -189,12 +189,17 @@
addPreviousComment(line, author, comment_text);
});
}
- if (comments.length == 0)
+
+ var help_text = 'Scroll though diffs with the "j" and "k" keys.';
+ if (comments.length == 0) {
+ $('#message .commentStatus').text(help_text);
return;
+ }
+
descriptor = comments.length + ' comment';
if (comments.length > 1)
descriptor += 's';
- $('#message .commentStatus').text('This patch has ' + descriptor + '. Scroll through them with the "n" and "p" keys.');
+ $('#message .commentStatus').text('This patch has ' + descriptor + '. Scroll through them with the "n" and "p" keys. ' + help_text);
}
function scanForComments(author, text) {
@@ -934,48 +939,105 @@
findCommentBlockFor(line).hide().after($('<div class="frozenComment"></div>').text(comment_textarea.val()));
});
- function focusOn(comment) {
+ function focusOn(node) {
$('.focused').removeClass('focused');
- if (comment.length == 0)
+ if (node.length == 0)
return;
- $(document).scrollTop(comment.addClass('focused').position().top - window.innerHeight/2);
+ $(document).scrollTop(node.addClass('focused').position().top - window.innerHeight / 2);
}
- function focusNextComment() {
- var comments = $('.previousComment');
- if (comments.length == 0)
- return;
- var index = comments.index($('.focused'));
- // Notice that -1 gets mapped to 0.
- focusOn($(comments.get(index + 1)));
+ function diffBlockEndPoint(focusable_nodes, line, line_offset, is_backward) {
+ if (!line_offset && is_backward)
+ return line;
+
+ var offset = is_backward ? -1 : 1;
+
+ // If we're at a comment, get a Line in the diff block that contains the comment.
+ if (line.hasClass('previousComment')) {
+ var line_for_comment = $('#' + line.attr('data-comment-for'));
+ line_offset = focusable_nodes.index(line_for_comment);
+
+ // If the comment is not inside a diff block, return the comment node.
+ if (line_offset == -1)
+ return line;
+
+ line = line_for_comment;
+ }
+
+ // Find the Line at the beginning/end of this contiguous block of lines.
+ // Contiguous blocks of lines have contiguous IDs and are contained in the same FileDiff.
+ // Skip over comment nodes.
+ var id = numberFrom(line.attr('id'));
+ var next_node = $(focusable_nodes[line_offset + offset]);
+ while (next_node.size()
+ && (!next_node.hasClass('Line') || next_node.attr('id') == 'line' + (id + offset))
+ && fileDiffFor(line)[0] == fileDiffFor(next_node)[0]) {
+ if (next_node.hasClass('Line')) {
+ line = next_node;
+ id += offset;
+ }
+
+ line_offset += offset;
+ next_node = $(focusable_nodes[line_offset + offset]);
+ }
+ return line;
}
- function focusPreviousComment() {
- var comments = $('.previousComment');
- if (comments.length == 0)
- return;
- var index = comments.index($('.focused'));
- if (index == -1)
- index = comments.length;
- if (index == 0) {
- focusOn([]);
+ function focusNext(className, is_backward) {
+ var focusable_nodes = $('.previousComment,.Line.add,.Line.remove');
+ var focused_node = $('.focused');
+ var index = focusable_nodes.index(focused_node);
+ if (is_backward && (!index || index == -1))
return;
+
+ if (focused_node.size() && className == 'Line') {
+ focused_node = diffBlockEndPoint(focusable_nodes, focused_node, index, is_backward);
+ index = focusable_nodes.index(focused_node);
+ }
+
+ var end = is_backward ? 0 : focusable_nodes.size();
+ var offset = is_backward ? -1 : 1;
+ for (var i = index + offset; i != end; i = i + offset) {
+ var node = $(focusable_nodes[i]);
+ if (node.hasClass(className)) {
+ if (className == 'Line') {
+ // Pass in true for is_backward because we always want to focus the start of the diff block.
+ node = diffBlockEndPoint(focusable_nodes, node, i, true);
+ }
+ focusOn(node);
+ return;
+ }
}
- focusOn($(comments.get(index - 1)));
}
var kCharCodeForN = 'n'.charCodeAt(0);
var kCharCodeForP = 'p'.charCodeAt(0);
+ var kCharCodeForJ = 'j'.charCodeAt(0);
+ var kCharCodeForK = 'k'.charCodeAt(0);
$('body').live('keypress', function() {
// FIXME: There's got to be a better way to avoid seeing these keypress
// events.
if (event.target.nodeName == 'TEXTAREA')
return;
- if (event.charCode == kCharCodeForN)
- focusNextComment();
- else if (event.charCode == kCharCodeForP)
- focusPreviousComment();
+
+ switch (event.charCode) {
+ case kCharCodeForN:
+ focusNext('previousComment', false);
+ break;
+
+ case kCharCodeForP:
+ focusNext('previousComment', true);
+ break;
+
+ case kCharCodeForJ:
+ focusNext('Line', false);
+ break;
+
+ case kCharCodeForK:
+ focusNext('Line', true);
+ break;
+ }
});
function contextLinesFor(line_id) {
@@ -1117,7 +1179,7 @@
}
function fileNameFor(line) {
- return line.parentsUntil('.FileDiff').parent().find('h1').text();
+ return fileDiffFor(line).find('h1').text();
}
function indentFor(depth) {
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list