From 9317bafd74c91159374903ac69cb25af9e3f2447 Mon Sep 17 00:00:00 2001
From: Wyatt Allen <wyatta@google.com>
Date: Fri, 15 Jul 2016 17:08:22 -0700
Subject: [PATCH] Upgrades and tests for GrDiffBuilder utility methods

A new method is added to the GrDiffBuilder class named
`_renderContentByRange`. It will replace DIV.contentText elements with
newly-rendered versions for the given line-range and side.

Two utility methods found on GR-DIFF-BUILDER are pushed down into the
GrDiffBuilder class. In particular, `getContentByLine` is moved as-is
and a wrapper is added to original place. `getContentsByLineRange` is
moved and converted to be more general. A wrapper is put in its original
location which follows the original signature. These moves make the
functions available to other methods of the class.

Tests are added for these new and existing methods.

Change-Id: I77634d05828756c46b802f9ec789ab767faca3cf
---
 .../diff/gr-diff-builder/gr-diff-builder.html | 22 ++---
 .../diff/gr-diff-builder/gr-diff-builder.js   | 60 +++++++++++++
 .../gr-diff-builder/gr-diff-builder_test.html | 88 +++++++++++++++++++
 3 files changed, 153 insertions(+), 17 deletions(-)

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 5f1fd90178..bf787088a3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -142,10 +142,7 @@ limitations under the License.
         },
 
         getContentByLine: function(lineNumber, opt_side, opt_root) {
-          var root = Polymer.dom(opt_root || this.diffElement);
-          var sideSelector = !!opt_side ? ('.' + opt_side) : '';
-          return root.querySelector('td.lineNum[data-value="' + lineNumber +
-              '"]' + sideSelector + ' ~ td.content .contentText');
+          return this._builder.getContentByLine(lineNumber, opt_side, opt_root);
         },
 
         getContentByLineEl: function(lineEl) {
@@ -162,19 +159,10 @@ limitations under the License.
         },
 
         getContentsByLineRange: function(startLine, endLine, opt_side) {
-          var groups =
-              this._builder.getGroupsByLineRange(startLine, endLine, opt_side);
-          // In each group, search Element for lines in range.
-          return groups.reduce((function(acc, group) {
-            for (var line = startLine; line <= endLine; line++) {
-              var content =
-                  this.getContentByLine(line, opt_side, group.element);
-              if (content) {
-                acc.push(content);
-              }
-            }
-            return acc;
-          }).bind(this), []);
+          var result = [];
+          this._builder.findLinesByRange(startLine, endLine, opt_side, null,
+              result);
+          return result;
         },
 
         getCommentThreadByLine: function(lineNumber, opt_side, opt_root) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 79188373f9..fb2d230a6e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -109,6 +109,66 @@
     return groups;
   };
 
+  GrDiffBuilder.prototype.getContentByLine = function(lineNumber, opt_side,
+      opt_root) {
+    var root = Polymer.dom(opt_root || this._outputEl);
+    var sideSelector = !!opt_side ? ('.' + opt_side) : '';
+    return root.querySelector('td.lineNum[data-value="' + lineNumber +
+        '"]' + sideSelector + ' ~ td.content .contentText');
+  };
+
+  /**
+   * Find line elements or line objects by a range of line numbers and a side.
+   *
+   * @param {Number} start The first line number
+   * @param {Number} end The last line number
+   * @param {String} opt_side The side of the range. Either 'left' or 'right'.
+   * @param {Array<GrDiffLine>} out_lines The output list of line objects. Use
+   *     null if not desired.
+   * @param  {Array<HTMLElement>} out_elements The output list of line elements.
+   *     Use null if not desired.
+   */
+  GrDiffBuilder.prototype.findLinesByRange = function(start, end, opt_side,
+      out_lines, out_elements) {
+    var groups = this.getGroupsByLineRange(start, end, opt_side);
+
+    groups.forEach(function(group) {
+      group.lines.forEach(function(line) {
+        if ((opt_side === 'left' && line.type === GrDiffLine.Type.ADD) ||
+            (opt_side === 'right' && line.type === GrDiffLine.Type.REMOVE)) {
+          return;
+        }
+        var lineNumber = opt_side === 'left' ?
+            line.beforeNumber : line.afterNumber;
+        if (lineNumber < start || lineNumber > end) { return; }
+
+        if (out_lines) { out_lines.push(line); }
+        if (out_elements) {
+          var content = this.getContentByLine(lineNumber, opt_side,
+              group.element);
+          if (content) { out_elements.push(content); }
+        }
+      }.bind(this));
+    }.bind(this));
+  };
+
+  /**
+   * Re-renders the DIV.contentText alement for the given side and range of diff
+   * content.
+   */
+  GrDiffBuilder.prototype._renderContentByRange = function(start, end, side) {
+    var lines = [];
+    var elements = [];
+    var line;
+    var el;
+    this.findLinesByRange(start, end, side, lines, elements);
+    for (var i = 0; i < lines.length; i++) {
+      line = lines[i];
+      el = elements[i];
+      el.parentElement.replaceChild(this._createTextEl(line).firstChild, el);
+    }
+  };
+
   GrDiffBuilder.prototype.getSectionsByLineRange = function(
       startLine, endLine, opt_side) {
     return this.getGroupsByLineRange(startLine, endLine, opt_side).map(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 727ae9bc89..12c8f3c5ca 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -23,7 +23,9 @@ limitations under the License.
 <script src="../gr-diff/gr-diff-line.js"></script>
 <script src="../gr-diff/gr-diff-group.js"></script>
 <script src="gr-diff-builder.js"></script>
+<script src="../../../scripts/util.js"></script>
 
+<link rel="import" href="../gr-diff-cursor/mock-diff-response_test.html">
 <link rel="import" href="gr-diff-builder.html">
 
 <test-fixture id="basic">
@@ -40,6 +42,14 @@ limitations under the License.
   </template>
 </test-fixture>
 
+<test-fixture id="mock-diff">
+  <template>
+    <gr-diff-builder view-mode="SIDE_BY_SIDE">
+      <table id="diffTable"></table>
+    </gr-diff-builder>
+  </template>
+</test-fixture>
+
 <script>
   suite('gr-diff-builder tests', function() {
     var element;
@@ -476,5 +486,83 @@ limitations under the License.
         assert.strictEqual(sections[1], section[1]);
       });
     });
+
+    suite('mock-diff', function() {
+      var element;
+      var builder;
+      var diff;
+
+      setup(function(done) {
+        element = fixture('mock-diff');
+        diff = document.createElement('mock-diff-response').diffResponse;
+
+        var prefs = {
+          line_length: 80,
+          show_tabs: true,
+          tab_size: 4,
+        };
+
+        function doneHandler() {
+          element.removeEventListener('render', doneHandler);
+          builder = element._builder;
+          done();
+        }
+        element.addEventListener('render', doneHandler);
+
+        element.render(diff, {left: [], right: []}, prefs);
+      });
+
+      test('getContentByLine', function() {
+        var actual;
+
+        actual = builder.getContentByLine(2, 'left');
+        assert.equal(actual.textContent, diff.content[0].ab[1]);
+
+        actual = builder.getContentByLine(2, 'right');
+        assert.equal(actual.textContent, diff.content[0].ab[1]);
+
+        actual = builder.getContentByLine(5, 'left');
+        assert.equal(actual.textContent, diff.content[2].ab[0]);
+
+        actual = builder.getContentByLine(5, 'right');
+        assert.equal(actual.textContent, diff.content[1].b[0]);
+      });
+
+      test('findLinesByRange', function() {
+        var lines = [];
+        var elems = [];
+        var start = 6;
+        var end = 10;
+        var count = end - start + 1;
+
+        builder.findLinesByRange(start, end, 'right', lines, elems);
+
+        assert.equal(lines.length, count);
+        assert.equal(elems.length, count);
+
+        for (var i = 0; i < 5; i++) {
+          assert.instanceOf(lines[i], GrDiffLine);
+          assert.equal(lines[i].afterNumber, start + i);
+          assert.instanceOf(elems[i], HTMLElement);
+          assert.equal(lines[i].text, elems[i].textContent);
+        }
+      });
+
+      test('_renderContentByRange', function() {
+        var spy = sinon.spy(builder, '_createTextEl');
+        var start = 9;
+        var end = 14;
+        var count = end - start + 1;
+
+        builder._renderContentByRange(start, end, 'left');
+
+        assert.equal(spy.callCount, count);
+        spy.getCalls().forEach(function(call, i) {
+          assert.equal(call.args[0].beforeNumber, start + i);
+        });
+
+        spy.restore();
+      });
+    });
   });
 </script>