[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
abarth at webkit.org
abarth at webkit.org
Wed Dec 22 12:52:47 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit e10d2309929fd6e5e1f2c178416c6ee7f2d7a75c
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Aug 31 23:31:02 2010 +0000
2010-08-31 Adam Barth <abarth at webkit.org>
Reviewed by Eric Seidel.
[reviewtool] Make it easy to scroll through review comments
https://bugs.webkit.org/show_bug.cgi?id=45002
This patch lets you scroll through review comments using "n" (for next)
and "p" (for previous). It also attributes comments to their authors.
* PrettyPatch/PrettyPatch.rb:
* code-review.js:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66553 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/BugsSite/ChangeLog b/BugsSite/ChangeLog
index 6bbc741..b5a0db8 100644
--- a/BugsSite/ChangeLog
+++ b/BugsSite/ChangeLog
@@ -2,6 +2,19 @@
Reviewed by Eric Seidel.
+ [reviewtool] Make it easy to scroll through review comments
+ https://bugs.webkit.org/show_bug.cgi?id=45002
+
+ This patch lets you scroll through review comments using "n" (for next)
+ and "p" (for previous). It also attributes comments to their authors.
+
+ * PrettyPatch/PrettyPatch.rb:
+ * code-review.js:
+
+2010-08-31 Adam Barth <abarth at webkit.org>
+
+ Reviewed by Eric Seidel.
+
[reviewtool] Show previous comments inline in diff
https://bugs.webkit.org/show_bug.cgi?id=44977
diff --git a/BugsSite/PrettyPatch/PrettyPatch.rb b/BugsSite/PrettyPatch/PrettyPatch.rb
index e893267..27a5843 100644
--- a/BugsSite/PrettyPatch/PrettyPatch.rb
+++ b/BugsSite/PrettyPatch/PrettyPatch.rb
@@ -194,6 +194,14 @@ h1 :hover {
background-color: #ffd;
}
+.author {
+ font-style: italic;
+}
+
+.focused {
+ border: 1px solid blue;
+}
+
.comment {
position: relative;
}
@@ -203,17 +211,6 @@ h1 :hover {
height: 6em;
}
-.reply {
- font-family: sans-serif;
- float: right;
- color: #999;
- display: none;
-}
-
-.hot .reply {
- display: block;
-}
-
body {
margin-bottom: 40px;
}
@@ -232,6 +229,10 @@ body {
float: right;
}
+#toolbar .commentStatus {
+ font-style:italic
+}
+
.winter {
position: fixed;
z-index: 5;
@@ -277,7 +278,7 @@ body {
}
</style>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.min.js"></script>
-<script src="code-review.js?version=4"></script>
+<script src="code-review.js?version=5"></script>
EOF
def self.revisionOrDescription(string)
diff --git a/BugsSite/code-review.js b/BugsSite/code-review.js
index 5b065f8..4713e5e 100644
--- a/BugsSite/code-review.js
+++ b/BugsSite/code-review.js
@@ -14,7 +14,6 @@
if (!attachment_id)
return;
- var current_comment = {}
var next_line_id = 0;
function idForLine(number) {
@@ -82,15 +81,16 @@
var files = {}
- function addPreviousComment(line, comment_text) {
+ function addPreviousComment(line, author, comment_text) {
var comment_block = $('<div data-comment-for="' + line.attr('id') + '" class="previous_comment"></div>');
- comment_block.text(comment_text).prepend('<div class="reply">Click to reply</div>');
- comment_block.each(hoverify).click(addCommentField);
+ var author_block = $('<div class="author"></div>').text(author + ':');
+ comment_block.text(comment_text).prepend(author_block).each(hoverify).click(addCommentField);
insertCommentFor(line, comment_block);
}
function displayPreviousComments(comments) {
for (var i = 0; i < comments.length; ++i) {
+ var author = comments[i].author;
var file_name = comments[i].file_name;
var line_number = comments[i].line_number;
var comment_text = comments[i].comment_text;
@@ -110,12 +110,18 @@
if ($(this).text() != line_number)
return;
var line = $(this).parent();
- addPreviousComment(line, comment_text);
+ addPreviousComment(line, author, comment_text);
});
}
+ if (comments.length == 0)
+ return;
+ descriptor = comments.length + ' comment';
+ if (comments.length > 1)
+ descriptor += 's';
+ $('#toolbar .commentStatus').text('This patch has ' + descriptor + '. Scroll through them with the "n" and "p" keys.');
}
- function scanForComments(text) {
+ function scanForComments(author, text) {
var comments = []
var lines = text.split('\n');
for (var i = 0; i < lines.length; ++i) {
@@ -134,8 +140,10 @@
comment_lines.push(lines[i]);
++i;
}
+ --i; // Decrement i because the for loop will increment it again in a second.
var comment_text = comment_lines.join('\n');
comments.push({
+ 'author': author,
'file_name': file_name,
'line_number': line_number,
'comment_text': comment_text
@@ -154,7 +162,7 @@
var text = $(this).find('.bz_comment_text').text();
var comment_marker = '(From update of attachment ' + attachment_id + ' .details.)';
if (text.match(comment_marker))
- $.merge(comments, scanForComments(text));
+ $.merge(comments, scanForComments(author, text));
});
displayPreviousComments(comments);
});
@@ -172,24 +180,62 @@
$(document).ready(function() {
crawlDiff();
fetchHistory();
- $(document.body).prepend('<div id="toolbar"><div class="actions"><button id="post_comments">Prepare comments</button></div><div class="help">Double-click a line to add a comment.</div></div>');
+ $(document.body).prepend('<div id="toolbar"><div class="actions"><button id="post_comments">Prepare comments</button></div><span class="commentStatus"></span> <span class="help">Double-click a line to add a comment.</span></div>');
$(document.body).prepend('<div id="comment_form" class="inactive"><div class="winter"></div><div class="lightbox"><iframe src="attachment.cgi?id=' + attachment_id + '&action=reviewform"></iframe></div></div>');
});
- $('.comment textarea').live('keydown', function() {
- var line_id = $(this).attr('data-comment-for');
- current_comment[line_id] = $(this).val();
- });
-
$('.comment .delete').live('click', function() {
var line_id = $(this).parentsUntil('.comment').parent().find('textarea').attr('data-comment-for');
- delete current_comment[line_id];
var line = $('#' + line_id)
findCommentBlockFor(line).remove();
line.removeAttr('data-has-comment');
trimCommentContextToBefore(line);
});
+ function focusOn(comment) {
+ $('.focused').removeClass('focused');
+ if (comment.length == 0)
+ return;
+ $(document).scrollTop(comment.addClass('focused').position().top - window.innerHeight/2);
+ }
+
+ function focusNextComment() {
+ var comments = $('.previous_comment');
+ if (comments.length == 0)
+ return;
+ var index = comments.index($('.focused'));
+ // Notice that -1 gets mapped to 0.
+ focusOn($(comments.get(index + 1)));
+ }
+
+ function focusPreviousComment() {
+ var comments = $('.previous_comment');
+ if (comments.length == 0)
+ return;
+ var index = comments.index($('.focused'));
+ if (index == -1)
+ index = comments.length;
+ if (index == 0) {
+ focusOn([]);
+ return;
+ }
+ focusOn($(comments.get(index - 1)));
+ }
+
+ var kCharCodeForN = 'n'.charCodeAt(0);
+ var kCharCodeForP = 'p'.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();
+ });
+
function contextLinesFor(line) {
var context = [];
while (line.hasClass('commentContext')) {
@@ -289,6 +335,8 @@
});
$('#comment_form').removeClass('inactive');
var comment = comments_in_context.join('\n\n');
+ if (comment != '')
+ comment = 'View in context: ' + window.location + '\n\n' + comment;
$('#comment_form').find('iframe').contents().find('#comment').val(comment);
});
})();
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list