Adds diff traversal helper and make diff object a builder property
Adds a polymorphic method to GrDiffBuilder subclasses named
`_getNextContentOnSide` which gets the a content element by traversing
from its preceding content on the same side. This method is dramatically
faster than the query-based method when diff sections are large.
In preparation for the syntax highlighting change, the gr-diff-builder
is refactored to use a property for the diff object, rather than
accepting it as a parameter to the `render` function.
Tests are included for new functions.
Change-Id: Ifd0edb530a303de2626d55a691c3ba1eaed6167c
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
index 0d9ecbc..1cb8cc7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
@@ -66,5 +66,17 @@
}
};
+ GrDiffBuilderSideBySide.prototype._getNextContentOnSide = function(
+ content, side) {
+ var tr = content.parentElement.parentElement;
+ var content;
+ while (tr = tr.nextSibling) {
+ content = tr.querySelector(
+ 'td.content .contentText[data-side="' + side + '"]');
+ if (content) { return content; }
+ }
+ return null;
+ };
+
window.GrDiffBuilderSideBySide = GrDiffBuilderSideBySide;
})(window, GrDiffBuilder);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
index d9c9b1b..960bf46 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
@@ -59,5 +59,19 @@
return row;
};
+ GrDiffBuilderUnified.prototype._getNextContentOnSide = function(
+ content, side) {
+ var tr = content.parentElement.parentElement;
+ var content;
+ while (tr = tr.nextSibling) {
+ if (tr.classList.contains('both') || (
+ (side === 'left' && tr.classList.contains('remove')) ||
+ (side === 'right' && tr.classList.contains('add')))) {
+ return tr.querySelector('.contentText');
+ }
+ }
+ return null;
+ };
+
window.GrDiffBuilderUnified = GrDiffBuilderUnified;
})(window, GrDiffBuilder);
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 0aa5c94..3cad9d9 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
@@ -55,6 +55,7 @@
*/
properties: {
+ diff: Object,
viewMode: String,
comments: Object,
isImageDiff: Boolean,
@@ -81,11 +82,11 @@
];
},
- render: function(diff, comments, prefs) {
+ render: function(comments, prefs) {
// Stop the processor (if it's running).
this.$.processor.cancel();
- this._builder = this._getDiffBuilder(diff, comments, prefs);
+ this._builder = this._getDiffBuilder(this.diff, comments, prefs);
this.$.processor.context = prefs.context;
this.$.processor.keyLocations = this._getCommentLocations(comments);
@@ -93,7 +94,7 @@
this._clearDiffContent();
console.time('diff render');
- this.$.processor.process(diff.content).then(function() {
+ return this.$.processor.process(this.diff.content).then(function() {
if (this.isImageDiff) {
this._builder.renderDiffImages();
}
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 389d290..816e3a4 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
@@ -139,6 +139,7 @@
var groups = this.getGroupsByLineRange(start, end, opt_side);
groups.forEach(function(group) {
+ var content = null;
group.lines.forEach(function(line) {
if ((opt_side === 'left' && line.type === GrDiffLine.Type.ADD) ||
(opt_side === 'right' && line.type === GrDiffLine.Type.REMOVE)) {
@@ -150,8 +151,12 @@
if (out_lines) { out_lines.push(line); }
if (out_elements) {
- var content = this.getContentByLine(lineNumber, opt_side,
- group.element);
+ if (content) {
+ content = this._getNextContentOnSide(content, opt_side);
+ } else {
+ content = this.getContentByLine(lineNumber, opt_side,
+ group.element);
+ }
if (content) { out_elements.push(content); }
}
}.bind(this));
@@ -537,5 +542,16 @@
this._renderContentByRange(start, end, side);
};
+ /**
+ * Finds the next DIV.contentText element following the given element, and on
+ * the same side. Will only search within a group.
+ * @param {HTMLElement} content
+ * @param {String} side Either 'left' or 'right'
+ * @return {HTMLElement}
+ */
+ GrDiffBuilder.prototype._getNextContentOnSide = function(content, side) {
+ throw Error('Subclasses must implement _getNextContentOnSide');
+ };
+
window.GrDiffBuilder = GrDiffBuilder;
})(window, GrDiffGroup, GrDiffLine);
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 f3d24f7..753ddaa 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
@@ -459,7 +459,8 @@
};
return builder;
});
- element.render({content: content}, {left: [], right: []}, prefs);
+ element.diff = {content: content};
+ element.render({left: [], right: []}, prefs);
});
test('renderSection', function() {
@@ -494,25 +495,23 @@
var element;
var builder;
var diff;
+ var prefs;
setup(function(done) {
element = fixture('mock-diff');
diff = document.createElement('mock-diff-response').diffResponse;
+ element.diff = diff;
- var prefs = {
+ prefs = {
line_length: 80,
show_tabs: true,
tab_size: 4,
};
- function doneHandler() {
- element.removeEventListener('render', doneHandler);
+ element.render({left: [], right: []}, prefs).then(function() {
builder = element._builder;
done();
- }
- element.addEventListener('render', doneHandler);
-
- element.render(diff, {left: [], right: []}, prefs);
+ });
});
test('getContentByLine', function() {
@@ -566,6 +565,70 @@
spy.restore();
});
+
+ test('_getNextContentOnSide side-by-side left', function() {
+ var startElem = builder.getContentByLine(5, 'left',
+ element.$.diffTable);
+ var expectedStartString = diff.content[2].ab[0];
+ var expectedNextString = diff.content[2].ab[1];
+ assert.equal(startElem.textContent, expectedStartString);
+
+ var nextElem = builder._getNextContentOnSide(startElem,
+ 'left');
+ assert.equal(nextElem.textContent, expectedNextString);
+ });
+
+ test('_getNextContentOnSide side-by-side right', function() {
+ var startElem = builder.getContentByLine(5, 'right',
+ element.$.diffTable);
+ var expectedStartString = diff.content[1].b[0];
+ var expectedNextString = diff.content[1].b[1];
+ assert.equal(startElem.textContent, expectedStartString);
+
+ var nextElem = builder._getNextContentOnSide(startElem,
+ 'right');
+ assert.equal(nextElem.textContent, expectedNextString);
+ });
+
+ test('_getNextContentOnSide unified left', function(done) {
+ // Re-render as unified:
+ element.viewMode = 'UNIFIED_DIFF';
+ element.render({left: [], right: []}, prefs).then(function() {
+ builder = element._builder;
+
+ var startElem = builder.getContentByLine(5, 'left',
+ element.$.diffTable);
+ var expectedStartString = diff.content[2].ab[0];
+ var expectedNextString = diff.content[2].ab[1];
+ assert.equal(startElem.textContent, expectedStartString);
+
+ var nextElem = builder._getNextContentOnSide(startElem,
+ 'left');
+ assert.equal(nextElem.textContent, expectedNextString);
+
+ done();
+ });
+ });
+
+ test('_getNextContentOnSide unified right', function(done) {
+ // Re-render as unified:
+ element.viewMode = 'UNIFIED_DIFF';
+ element.render({left: [], right: []}, prefs).then(function() {
+ builder = element._builder;
+
+ var startElem = builder.getContentByLine(5, 'right',
+ element.$.diffTable);
+ var expectedStartString = diff.content[1].b[0];
+ var expectedNextString = diff.content[1].b[1];
+ assert.equal(startElem.textContent, expectedStartString);
+
+ var nextElem = builder._getNextContentOnSide(startElem,
+ 'right');
+ assert.equal(nextElem.textContent, expectedNextString);
+
+ done();
+ });
+ });
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 2e7fd56..33322df 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -151,6 +151,7 @@
<gr-diff-builder
id="diffBuilder"
comments="[[_comments]]"
+ diff="[[_diff]]"
view-mode="[[viewMode]]"
is-image-diff="[[isImageDiff]]"
base-image="[[_baseImage]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 83ad58a..b0c1731 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -336,7 +336,7 @@
},
_render: function() {
- this.$.diffBuilder.render(this._diff, this._comments, this.prefs);
+ this.$.diffBuilder.render(this._comments, this.prefs);
},
_clearDiffContent: function() {
