opendev/gerrit
Code Issues Proposed changes

Upgrades and tests for GrDiffBuilder utility methods

Browse Source 
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
This commit is contained in:
Wyatt Allen
2016-07-15 17:08:22 -07:00
parent 33df005810
commit 9317bafd74
3 changed files with 153 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
View File

@@ -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) {

60
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
View File

@@ -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(

88
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
View File

@@ -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>