2011-01-14 Ojan Vafai <ojan@chromium.org>

        Reviewed by Adam Barth.

        tweak line selection in code review tool
        https://bugs.webkit.org/show_bug.cgi?id=52462

        -Improve handling of data-comment-base-line code to deal with
        lines that have multiple values.
        -Make it so that if you click on the line immediately above
        a line that has a comment it will add a new comment instead of
        adding lines to the following comment. If the last selected line
        overlaps existing comment lines though they will still get
        added to the existing comment.

        * code-review.js:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75816 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Websites/bugs.webkit.org/ChangeLog b/Websites/bugs.webkit.org/ChangeLog
index 1be7cb7fccb59680438af8ed41f31aad53d0d693..782ab8e6aa6742d48698afcdb9f95b1c9ac9171a 100644 (file)
--- a/Websites/bugs.webkit.org/ChangeLog
+++ b/Websites/bugs.webkit.org/ChangeLog
@@ -1,3 +1,20 @@
+2011-01-14  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        tweak line selection in code review tool
+        https://bugs.webkit.org/show_bug.cgi?id=52462
+
+        -Improve handling of data-comment-base-line code to deal with
+        lines that have multiple values.
+        -Make it so that if you click on the line immediately above
+        a line that has a comment it will add a new comment instead of
+        adding lines to the following comment. If the last selected line
+        overlaps existing comment lines though they will still get
+        added to the existing comment.
+
+        * code-review.js:
+
 2011-01-14  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Adam Roben.

diff --git a/Websites/bugs.webkit.org/code-review.js b/Websites/bugs.webkit.org/code-review.js
index 18775ff95cbd7848d9a28159fd896554f6cedca9..3555784dff882cdadb5c154429a502cafe0a7cac 100644 (file)
--- a/Websites/bugs.webkit.org/code-review.js
+++ b/Websites/bugs.webkit.org/code-review.js
@@ -914,7 +914,7 @@
     findCommentBlockFor(line).slideUp('fast', function() {
       $(this).remove();
       line.removeAttr('data-has-comment');
-      trimCommentContextToBefore(line);
+      trimCommentContextToBefore(line, line.attr('data-comment-base-line'));
     });
   }
 
@@ -997,23 +997,27 @@
     }
   });
 
-  function contextLinesFor(line_id) {
-    return $('div[data-comment-base-line~="' + line_id + '"]');
+  function contextLinesFor(comment_base_lines, file_diff) {
+    var base_lines = comment_base_lines.split(' ');
+    return $('div[data-comment-base-line]', file_diff).filter(function() {
+      return $(this).attr('data-comment-base-line').split(' ').some(function(item) {
+        return base_lines.indexOf(item) != -1;
+      });
+    });
   }
 
   function numberFrom(line_id) {
     return Number(line_id.replace('line', ''));
   }
 
-  function trimCommentContextToBefore(line) {
-    var base_line_id = line.attr('data-comment-base-line');
+  function trimCommentContextToBefore(line, comment_base_line) {
     var line_to_trim_to = numberFrom(line.attr('id'));
-    contextLinesFor(base_line_id).each(function() {
+    contextLinesFor(comment_base_line, fileDiffFor(line)).each(function() {
       var id = $(this).attr('id');
       if (numberFrom(id) > line_to_trim_to)
         return;
 
-      removeDataCommentBaseLine(this, base_line_id);
+      removeDataCommentBaseLine(this, comment_base_line);
       if (!$(this).attr('data-comment-base-line'))
         $(this).removeClass('commentContext');
     });
@@ -1044,7 +1048,7 @@
   $('.lineNumber').live('click', function() {
     var line = $(this).parents('.Line');
     if (line.hasClass('commentContext'))
-      trimCommentContextToBefore(previousLineFor(line));
+      trimCommentContextToBefore(previousLineFor(line), line.attr('data-comment-base-line'));
   }).live('mousedown', function() {
     in_drag_select = true;
     $(lineFromLineDescendant(this)).addClass('selected');
@@ -1060,21 +1064,22 @@
   }).live('mouseup', function() {
     if (!in_drag_select)
       return;
+
     var selected = $('.selected');
-    var should_add_comment = !nextLineFor(selected.last()).hasClass('commentContext');
+    var already_has_comment = selected.last().hasClass('commentContext');
     selected.addClass('commentContext');
 
-    var id;
-    if (should_add_comment) {
+    var comment_base_line;
+    if (already_has_comment)
+      comment_base_line = selected.last().attr('data-comment-base-line');
+    else {
       var last = lineFromLineDescendant(selected.last()[0]);
       addCommentFor($(last));
-      id = last.id;
-    } else {
-      id = nextLineFor(selected.last())[0].getAttribute('data-comment-base-line');
+      comment_base_line = last.id;
     }
 
     selected.each(function() {
-      addDataCommentBaseLine(this, id);
+      addDataCommentBaseLine(this, comment_base_line);
     });
   });
 
@@ -1091,15 +1096,16 @@
     $(line).attr('data-comment-base-line', parts.join(' '));
   }
 
-  function removeDataCommentBaseLine(line, id) {
+  function removeDataCommentBaseLine(line, comment_base_lines) {
     var val = $(line).attr('data-comment-base-line');
     if (!val)
       return;
 
+    var base_lines = comment_base_lines.split(' ');
     var parts = val.split(' ');
     var newVal = [];
     for (var i = 0; i < parts.length; i++) {
-      if (parts[i] != id)
+      if (base_lines.indexOf(parts[i]) == -1)
         newVal.push(parts[i]);
     }
 
@@ -1124,7 +1130,7 @@
 
   function contextSnippetFor(line, indent) {
     var snippets = []
-    contextLinesFor(line.attr('id')).each(function() {
+    contextLinesFor(line.attr('id'), fileDiffFor(line)).each(function() {
       var action = ' ';
       if ($(this).hasClass('add'))
         action = '+';