[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:48:40 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 105eeafabcf9762e6bbed4013cf2fdd13e21c9e0
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Aug 30 22:52:27 2010 +0000
2010-08-30 Adam Barth <abarth at webkit.org>
Reviewed by Eric Seidel.
[review tool] Let reviewer select how much context to show in snippet
https://bugs.webkit.org/show_bug.cgi?id=44905
We now highlight the context for a comment in yellow on the left (where
the line numbers are). Clicking a line number expands or contracts the
amount of context, as appropriate. Informal user testing indicates
that we might want to support drag as well.
This patch also changes the "open a comment box here" action to
double-click to avoid issues with mis-clicks.
* PrettyPatch/PrettyPatch.rb:
* code-review.js:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66420 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/BugsSite/ChangeLog b/BugsSite/ChangeLog
index f50caeb..ef170f0 100644
--- a/BugsSite/ChangeLog
+++ b/BugsSite/ChangeLog
@@ -1,3 +1,21 @@
+2010-08-30 Adam Barth <abarth at webkit.org>
+
+ Reviewed by Eric Seidel.
+
+ [review tool] Let reviewer select how much context to show in snippet
+ https://bugs.webkit.org/show_bug.cgi?id=44905
+
+ We now highlight the context for a comment in yellow on the left (where
+ the line numbers are). Clicking a line number expands or contracts the
+ amount of context, as appropriate. Informal user testing indicates
+ that we might want to support drag as well.
+
+ This patch also changes the "open a comment box here" action to
+ double-click to avoid issues with mis-clicks.
+
+ * PrettyPatch/PrettyPatch.rb:
+ * code-review.js:
+
2010-08-29 Adam Barth <abarth at webkit.org>
Attempt to make Sam's life easier by not opening a comment text field
diff --git a/BugsSite/PrettyPatch/PrettyPatch.rb b/BugsSite/PrettyPatch/PrettyPatch.rb
index 28687ed..ab8fb89 100644
--- a/BugsSite/PrettyPatch/PrettyPatch.rb
+++ b/BugsSite/PrettyPatch/PrettyPatch.rb
@@ -203,17 +203,6 @@ h1 :hover {
height: 6em;
}
-.comment .actions {
- position: absolute;
- right: 3px;
- top: 3px;
- display: none;
-}
-
-.hot .actions {
- display: block;
-}
-
.reply {
font-family: sans-serif;
float: right;
@@ -231,7 +220,6 @@ body {
#toolbar {
position: fixed;
- text-align: right;
padding: 5px;
bottom: 0;
left: 0;
@@ -240,6 +228,10 @@ body {
background-color: #eee;
}
+#toolbar .actions {
+ float: right;
+}
+
.winter {
position: fixed;
z-index: 5;
@@ -268,6 +260,15 @@ body {
width: 100%;
height: 100%;
}
+
+help {
+ color: gray;
+ font-style: italic;
+}
+
+.commentContext .lineNumber {
+ background-color: yellow;
+}
</style>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.min.js"></script>
<script src="code-review.js"></script>
diff --git a/BugsSite/code-review.js b/BugsSite/code-review.js
index 9edae76..cc1912d 100644
--- a/BugsSite/code-review.js
+++ b/BugsSite/code-review.js
@@ -1,6 +1,4 @@
(function() {
- var kDeleteImage = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAMAAAAoLQ9TAAAAA3NCSVQICAjb4U/gAAAAn1BMVEX////4YmT/dnnyTE//dnn9bnH6am34YmT3XWD2WVv2VVjsOj3oMDLlJyrjICL2VVjzUVTwR0ruPT/iHB72WVvwR0rzUVT/h4r/gob/foH/eXv/dnn/cnT9bnH/bG76am3/Zmb6ZGf4YmT3XWD/WFv2WVv/VFf2VVj0TVDyTE/2SkzwR0rvREfuQUPuPT/sOj3rNDboMDLnLTDlJyrjICIhCpwnAAAANXRSTlMAESIiMzMzMzMzMzMzMzNERERERHd3qv///////////////////////////////////////0mgXpwAAAAJcEhZcwAAHngAAB54AcurAx8AAAAYdEVYdFNvZnR3YXJlAEFkb2JlIEZpcmV3b3Jrc0+zH04AAACVSURBVBiVbczXFoIwDAbguHGi4mqbWugQZInj/Z9NSuXAhblJvuTkB+jV4NeHY9e9g+/M2KSxFKdRY0JwWltxoo72gvRMxcxTgqrM/Qp2QWmdt+kRJ5SyzgCGao09zw3TN8yWnSNEfo3LVWdTPJIwqdbWCyN5XABUeZi+NvViG0trgHeRPgM77O6l+/04A+zb9AD+1Bf6lg3jQQJJTgAAAABJRU5ErkJggg==';
-
function determineAttachmentID() {
try {
return /id=(\d+)/.exec(window.location.search)[1]
@@ -67,17 +65,17 @@
function addCommentField() {
var id = $(this).attr('data-comment-for');
- if (!id)
+ if (!id) {
id = this.id;
+ $(this).addClass('commentContext');
+ }
var line = $('#' + id);
if (line.attr('data-has-comment'))
return;
- if (!window.getSelection().isCollapsed)
- return; // If there's a selection, we assume the user wants to copy the text.
line.attr('data-has-comment', 'true');
- var comment_block = $('<div class="comment"><div class="actions"><img class="delete" src="' + kDeleteImage + '"></div><textarea data-comment-for="' + id + '"></textarea></div>');
+
+ var comment_block = $('<div class="comment"><textarea data-comment-for="' + id + '"></textarea><div class="actions"><button class="delete">Delete</button></div></div>');
insertCommentFor(line, comment_block);
- comment_block.each(hoverify);
comment_block.children('textarea').focus();
}
@@ -96,9 +94,9 @@
}
$(document).ready(function() {
- $('.Line').each(idify).each(hoverify).click(addCommentField);
+ $('.Line').each(idify).each(hoverify).dblclick(addCommentField);
$(previous_comments).each(displayPreviousComments);
- $(document.body).prepend('<div id="toolbar"><button id="post_comments">Prepare comments</button></div>');
+ $(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="comment_form" class="inactive"><div class="winter"></div><div class="lightbox"><iframe src="attachment.cgi?id=' + attachment_id + '&action=reviewform"></iframe></div></div>');
});
@@ -108,25 +106,72 @@
});
$('.comment .delete').live('click', function() {
- var line_id = $(this).parentsUntil('.comment').next().attr('data-comment-for');
+ 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 contextLinesFor(line) {
+ var context = [];
+ while (line.hasClass('commentContext')) {
+ $.merge(context, line);
+ line = line.prev();
+ }
+ return $(context.reverse());
+ }
+
+ function trimCommentContextToBefore(line) {
+ while (line.hasClass('commentContext') && line.attr('data-has-comment') != 'true') {
+ line.removeClass('commentContext');
+ line = line.prev();
+ }
+ }
+
+ function tryToExpandCommentContextTo(line) {
+ var context_line = line;
+ while (context_line.length != 0) {
+ context_line = context_line.next();
+ if (context_line.hasClass('commentContext'))
+ break;
+ }
+ if (context_line.length == 0)
+ return; // No comment context to expand.
+ line.addClass('commentContext').nextUntil('.commentContext').addClass('commentContext');
+ }
+
+ $('.lineNumber').live('click', function() {
+ var line = $(this).parent();
+ if (line.hasClass('commentContext'))
+ trimCommentContextToBefore(line.prev());
+ else
+ tryToExpandCommentContextTo(line);
});
function contextSnippetFor(line) {
- var file_name = line.parent().prev().text();
- var line_number = line.hasClass('remove') ? line.children('.from').text() : line.children('.to').text();
+ var snippets = []
+ contextLinesFor(line).each(function() {
+ var action = ' ';
+ if ($(this).hasClass('add'))
+ action = '+';
+ else if ($(this).hasClass('remove'))
+ action = '-';
+ var text = $(this).children('.text').text();
+ snippets.push('> ' + action + text);
+ });
+ return snippets.join('\n');
+ }
- var action = ' ';
- if (line.hasClass('add'))
- action = '+';
- else if (line.hasClass('remove'))
- action = '-';
+ function fileNameFor(line) {
+ return line.parentsUntil('.FileDiff').parent().find('h1').text();
+ }
- var text = line.children('.text').text();
- return '> ' + file_name + ':' + line_number + '\n> ' + action + text;
+ function snippetFor(line) {
+ var file_name = fileNameFor(line);
+ var line_number = line.hasClass('remove') ? line.children('.from').text() : line.children('.to').text();
+ return '> ' + file_name + ':' + line_number + '\n' + contextSnippetFor(line);
}
$('#comment_form .winter').live('click', function() {
@@ -138,7 +183,7 @@
forEachLine(function(line) {
if (line.attr('data-has-comment') != 'true')
return;
- var snippet = contextSnippetFor(line);
+ var snippet = snippetFor(line);
var comment = findCommentBlockFor(line).children('textarea').val();
if (comment == '')
return;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list