opendev/gerrit
Code Issues Proposed changes

Add context controls to gr-new-diff

Browse Source 
+ This only covers collapsing context according to the
  diff preferences. Clicking on the control does nothing
  in this change.

Change-Id: I82e07d1ba55630351b1f79db5f44ff81e2fafecd
This commit is contained in:
Andrew Bonventre
2016-03-15 18:58:46 -04:00
parent 2545752989
commit f772ec8a32
9 changed files with 265 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
polygerrit-ui/app/elements/diff/gr-new-diff/gr-diff-builder-side-by-side.js
View File

@@ -14,8 +14,8 @@
(function(window, GrDiffBuilder) {
'use strict';
function GrDiffBuilderSideBySide(diff, outputEl) {
GrDiffBuilder.call(this, diff, outputEl);
function GrDiffBuilderSideBySide(diff, prefs, outputEl) {
GrDiffBuilder.call(this, diff, prefs, outputEl);
}
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderSideBySide.prototype.constructor = GrDiffBuilderSideBySide;
@@ -23,8 +23,9 @@
GrDiffBuilderSideBySide.prototype._emitGroup = function(group,
opt_beforeSection) {
var sectionEl = this._createElement('tbody', 'section');
sectionEl.classList.add(group.type);
var pairs = group.getSideBySidePairs();
for (var i = 0; i < pairs.length; ++i) {
for (var i = 0; i < pairs.length; i++) {
sectionEl.appendChild(this._createRow(pairs[i].left, pairs[i].right));
}
this._outputEl.insertBefore(sectionEl, opt_beforeSection);
@@ -45,7 +46,12 @@
}
row.appendChild(this._createLineEl(line, lineNumber, line.type));
row.appendChild(this._createTextEl(line));
var action = this._createContextControl(row, line);
if (action) {
row.appendChild(action);
} else {
row.appendChild(this._createTextEl(line));
}
return row;
};

12
polygerrit-ui/app/elements/diff/gr-new-diff/gr-diff-builder-unified.js
View File

@@ -14,8 +14,8 @@
(function(window, GrDiffBuilder) {
'use strict';
function GrDiffBuilderUnified(diff, outputEl) {
GrDiffBuilder.call(this, diff, outputEl);
function GrDiffBuilderUnified(diff, prefs, outputEl) {
GrDiffBuilder.call(this, diff, prefs, outputEl);
}
GrDiffBuilderUnified.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderUnified.prototype.constructor = GrDiffBuilderUnified;
@@ -36,7 +36,13 @@
GrDiffLine.Type.REMOVE));
row.appendChild(this._createLineEl(line, line.afterNumber,
GrDiffLine.Type.ADD));
row.appendChild(this._createTextEl(line));
var action = this._createContextControl(row, line);
if (action) {
row.appendChild(action);
} else {
row.appendChild(this._createTextEl(line));
}
return row;
};

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

@@ -14,11 +14,12 @@
(function(window, GrDiffGroup, GrDiffLine) {
'use strict';
function GrDiffBuilder(diff, outputEl) {
function GrDiffBuilder(diff, prefs, outputEl) {
this._prefs = prefs;
this._outputEl = outputEl;
this._groups = [];
this._processContent(diff.content, this._groups);
this._processContent(diff.content, this._groups, prefs.context);
}
GrDiffBuilder.GroupType = {
@@ -37,48 +38,122 @@
throw Error('Subclasses must implement emitGroup');
},
GrDiffBuilder.prototype._processContent = function(content, groups) {
var leftLineNum = 0;
var rightLineNum = 0;
GrDiffBuilder.prototype._processContent = function(content, groups, context) {
var WHOLE_FILE = -1;
context = content.length > 1 ? context : WHOLE_FILE;
var lineNums = {
left: 0,
right: 0,
};
for (var i = 0; i < content.length; i++) {
var group = content[i];
var lines = [];
if (group[GrDiffBuilder.GroupType.BOTH] !== undefined) {
var rows = group[GrDiffBuilder.GroupType.BOTH];
for (var j = 0; j < rows.length; j++) {
var line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.text = rows[j];
line.beforeNumber = ++leftLineNum;
line.afterNumber = ++rightLineNum;
lines.push(line);
this._appendCommonLines(rows, lines, lineNums);
var hiddenRange = [context, rows.length - context];
if (i === 0) {
hiddenRange[0] = 0;
} else if (i === content.length - 1) {
hiddenRange[1] = rows.length;
}
if (context !== WHOLE_FILE && hiddenRange[1] - hiddenRange[0] > 0) {
this._insertContextGroups(groups, lines, hiddenRange);
} else {
groups.push(new GrDiffGroup(GrDiffGroup.Type.BOTH, lines));
}
groups.push(new GrDiffGroup(GrDiffGroup.Type.BOTH, lines));
continue;
}
if (group[GrDiffBuilder.GroupType.REMOVED] !== undefined) {
var rows = group[GrDiffBuilder.GroupType.REMOVED];
for (var j = 0; j < rows.length; j++) {
var line = new GrDiffLine(GrDiffLine.Type.REMOVE);
line.text = rows[j];
line.beforeNumber = ++leftLineNum;
lines.push(line);
}
this._appendRemovedLines(group[GrDiffBuilder.GroupType.REMOVED], lines,
lineNums);
}
if (group[GrDiffBuilder.GroupType.ADDED] !== undefined) {
var rows = group[GrDiffBuilder.GroupType.ADDED];
for (var j = 0; j < rows.length; j++) {
var line = new GrDiffLine(GrDiffLine.Type.ADD);
line.text = rows[j];
line.afterNumber = ++rightLineNum;
lines.push(line);
}
this._appendAddedLines(group[GrDiffBuilder.GroupType.ADDED], lines,
lineNums);
}
groups.push(new GrDiffGroup(GrDiffGroup.Type.DELTA, lines));
}
};
GrDiffBuilder.prototype._insertContextGroups = function(groups, lines,
hiddenRange) {
// TODO: Split around comments as well.
var linesBeforeCtx = lines.slice(0, hiddenRange[0]);
var hiddenLines = lines.slice(hiddenRange[0], hiddenRange[1]);
var linesAfterCtx = lines.slice(hiddenRange[1]);
if (linesBeforeCtx.length > 0) {
groups.push(new GrDiffGroup(GrDiffGroup.Type.BOTH, linesBeforeCtx));
}
var ctxLine = new GrDiffLine(GrDiffLine.Type.CONTEXT_CONTROL);
ctxLine.contextLines = hiddenLines;
groups.push(new GrDiffGroup(GrDiffGroup.Type.CONTEXT_CONTROL,
[ctxLine]));
if (linesAfterCtx.length > 0) {
groups.push(new GrDiffGroup(GrDiffGroup.Type.BOTH, linesAfterCtx));
}
};
GrDiffBuilder.prototype._appendCommonLines = function(rows, lines, lineNums) {
for (var i = 0; i < rows.length; i++) {
var line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.text = rows[i];
line.beforeNumber = ++lineNums.left;
line.afterNumber = ++lineNums.right;
lines.push(line);
}
};
GrDiffBuilder.prototype._appendRemovedLines = function(rows, lines,
lineNums) {
for (var i = 0; i < rows.length; i++) {
var line = new GrDiffLine(GrDiffLine.Type.REMOVE);
line.text = rows[i];
line.beforeNumber = ++lineNums.left;
lines.push(line);
}
};
GrDiffBuilder.prototype._appendAddedLines = function(rows, lines, lineNums) {
for (var i = 0; i < rows.length; i++) {
var line = new GrDiffLine(GrDiffLine.Type.ADD);
line.text = rows[i];
line.afterNumber = ++lineNums.right;
lines.push(line);
}
};
GrDiffBuilder.prototype._createContextControl = function(section, line) {
if (!line.contextLines.length) {
return null;
}
var td = this._createElement('td');
var button = this._createElement('gr-button', 'showContext');
button.setAttribute('link', true);
var commonLines = line.contextLines.length;
var text = 'Show ' + commonLines + ' common line';
if (commonLines > 1) {
text += 's';
}
text += '...';
button.textContent = text;
button.addEventListener('tap', function(e) {
e.detail = {section: section, line: line};
// Let it bubble up the DOM tree.
});
td.appendChild(button);
return td;
};
GrDiffBuilder.prototype._createBlankSideEl = function() {
var td = this._createElement('td');
td.setAttribute('colspan', '2');
@@ -87,8 +162,10 @@
GrDiffBuilder.prototype._createLineEl = function(line, number, type) {
var td = this._createElement('td', 'lineNum');
if (line.type === GrDiffLine.Type.BOTH || line.type == type) {
td.setAttribute('data-line-num', number);
if (line.type === GrDiffLine.Type.CONTEXT_CONTROL) {
td.setAttribute('data-value', '@@');
} else if (line.type === GrDiffLine.Type.BOTH || line.type == type) {
td.setAttribute('data-value', number);
}
return td;
};

104
polygerrit-ui/app/elements/diff/gr-new-diff/gr-diff-builder_test.html
View File

@@ -52,7 +52,7 @@ limitations under the License.
},
];
var groups = [];
GrDiffBuilder.prototype._processContent(content, groups);
GrDiffBuilder.prototype._processContent(content, groups, -1);
assert.equal(groups.length, 3);
@@ -99,5 +99,107 @@ limitations under the License.
]);
});
test('insert context groups', function() {
var content = [
{ab: []},
{a: ['all work and no play make andybons a dull boy']},
{ab: []},
{b: ['elgoog elgoog elgoog']},
{ab: []},
];
for (var i = 0; i < 100; i++) {
content[0].ab.push('all work and no play make jack a dull boy');
content[4].ab.push('all work and no play make jill a dull girl');
}
for (var i = 0; i < 5; i++) {
content[2].ab.push('no tv and no beer make homer go crazy');
}
var groups = [];
var context = 10;
GrDiffBuilder.prototype._processContent(content, groups, context);
assert.equal(groups[0].type, GrDiffGroup.Type.CONTEXT_CONTROL);
assert.equal(groups[0].lines[0].contextLines.length, 90);
groups[0].lines[0].contextLines.forEach(function(l) {
assert.equal(l.text, content[0].ab[0]);
});
assert.equal(groups[1].type, GrDiffGroup.Type.BOTH);
assert.equal(groups[1].lines.length, context);
groups[1].lines.forEach(function(l) {
assert.equal(l.text, content[0].ab[0]);
});
assert.equal(groups[2].type, GrDiffGroup.Type.DELTA);
assert.equal(groups[2].lines.length, 1);
assert.equal(groups[2].removes.length, 1);
assert.equal(groups[2].removes[0].text,
'all work and no play make andybons a dull boy');
assert.equal(groups[3].type, GrDiffGroup.Type.BOTH);
assert.equal(groups[3].lines.length, 5);
groups[3].lines.forEach(function(l) {
assert.equal(l.text, content[2].ab[0]);
});
assert.equal(groups[4].type, GrDiffGroup.Type.DELTA);
assert.equal(groups[4].lines.length, 1);
assert.equal(groups[4].adds.length, 1);
assert.equal(groups[4].adds[0].text, 'elgoog elgoog elgoog');
assert.equal(groups[5].type, GrDiffGroup.Type.BOTH);
assert.equal(groups[5].lines.length, context);
groups[5].lines.forEach(function(l) {
assert.equal(l.text, content[4].ab[0]);
});
assert.equal(groups[6].type, GrDiffGroup.Type.CONTEXT_CONTROL);
assert.equal(groups[6].lines[0].contextLines.length, 90);
groups[6].lines[0].contextLines.forEach(function(l) {
assert.equal(l.text, content[4].ab[0]);
});
content = [
{a: ['all work and no play make andybons a dull boy']},
{ab: []},
{b: ['elgoog elgoog elgoog']},
];
for (var i = 0; i < 50; i++) {
content[1].ab.push('no tv and no beer make homer go crazy');
}
groups = [];
GrDiffBuilder.prototype._processContent(content, groups, 10);
assert.equal(groups[0].type, GrDiffGroup.Type.DELTA);
assert.equal(groups[0].lines.length, 1);
assert.equal(groups[0].removes.length, 1);
assert.equal(groups[0].removes[0].text,
'all work and no play make andybons a dull boy');
assert.equal(groups[1].type, GrDiffGroup.Type.BOTH);
assert.equal(groups[1].lines.length, context);
groups[1].lines.forEach(function(l) {
assert.equal(l.text, content[1].ab[0]);
});
assert.equal(groups[2].type, GrDiffGroup.Type.CONTEXT_CONTROL);
assert.equal(groups[2].lines[0].contextLines.length, 30);
groups[2].lines[0].contextLines.forEach(function(l) {
assert.equal(l.text, content[1].ab[0]);
});
assert.equal(groups[3].type, GrDiffGroup.Type.BOTH);
assert.equal(groups[3].lines.length, context);
groups[3].lines.forEach(function(l) {
assert.equal(l.text, content[1].ab[0]);
});
assert.equal(groups[4].type, GrDiffGroup.Type.DELTA);
assert.equal(groups[4].lines.length, 1);
assert.equal(groups[4].adds.length, 1);
assert.equal(groups[4].adds[0].text, 'elgoog elgoog elgoog');
});
});
</script>

6
polygerrit-ui/app/elements/diff/gr-new-diff/gr-diff-group.js
View File

@@ -27,15 +27,15 @@
GrDiffGroup.Type = {
BOTH: 'both',
CONTEXT_CONTROL: 'contextControl',
DELTA: 'delta',
HEADER: 'header',
};
GrDiffGroup.prototype.addLine = function(line) {
this.lines.push(line);
var notDelta = (this.type === GrDiffGroup.Type.BOTH ||
this.type === GrDiffGroup.Type.HEADER);
this.type === GrDiffGroup.Type.CONTEXT_CONTROL);
if (notDelta && (line.type === GrDiffLine.Type.ADD ||
line.type === GrDiffLine.Type.REMOVE)) {
throw Error('Cannot add delta line to a non-delta group.');
@@ -50,7 +50,7 @@
GrDiffGroup.prototype.getSideBySidePairs = function() {
if (this.type === GrDiffGroup.Type.BOTH ||
this.type === GrDiffGroup.Type.HEADER) {
this.type === GrDiffGroup.Type.CONTEXT_CONTROL) {
return this.lines.map(function(line) {
return {
left: line,

4
polygerrit-ui/app/elements/diff/gr-new-diff/gr-diff-group_test.html
View File

@@ -72,7 +72,7 @@ limitations under the License.
{left: l3, right: l3},
]);
group = new GrDiffGroup(GrDiffGroup.Type.HEADER, [l1, l2, l3]);
group = new GrDiffGroup(GrDiffGroup.Type.CONTEXT_CONTROL, [l1, l2, l3]);
assert.deepEqual(group.lines, [l1, l2, l3]);
assert.deepEqual(group.adds, []);
assert.deepEqual(group.removes, []);
@@ -95,7 +95,7 @@ limitations under the License.
assert.throws(group.addLine.bind(group, l2));
assert.doesNotThrow(group.addLine.bind(group, l3));
group = new GrDiffGroup(GrDiffGroup.Type.HEADER);
group = new GrDiffGroup(GrDiffGroup.Type.CONTEXT_CONTROL);
assert.throws(group.addLine.bind(group, l1));
assert.throws(group.addLine.bind(group, l2));
assert.doesNotThrow(group.addLine.bind(group, l3));

2
polygerrit-ui/app/elements/diff/gr-new-diff/gr-diff-line.js
View File

@@ -16,6 +16,7 @@
function GrDiffLine(type) {
this.type = type;
this.contextLines = [];
}
GrDiffLine.prototype.beforeNumber = 0;
@@ -28,6 +29,7 @@
ADD: 'add',
BOTH: 'both',
BLANK: 'blank',
CONTEXT_CONTROL: 'contextControl',
REMOVE: 'remove',
};

17
polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.html
View File

@@ -50,6 +50,7 @@ limitations under the License.
display: flex;
font: 12px var(--monospace-font-family);
overflow-x: auto;
will-change: transform;
}
table {
border-collapse: collapse;
@@ -67,7 +68,7 @@ limitations under the License.
text-align: right;
}
.lineNum:before {
content: attr(data-line-num);
content: attr(data-value);
}
.content {
overflow: hidden;
@@ -79,6 +80,18 @@ limitations under the License.
.remove {
background-color: var(--dark-remove-highlight-color);
}
.contextControl,
.contextControl .lineNum {
color: #849;
background-color: #fef;
}
.contextControl gr-button {
font-family: var(--monospace-font-family);
text-decoration: none;
}
.contextControl td:not(.lineNum) {
text-align: center;
}
</style>
<div class="loading" hidden$="[[!_loading]]">Loading...</div>
<div hidden$="[[_loading]]" hidden>
@@ -101,7 +114,7 @@ limitations under the License.
on-cancel="_handlePrefsCancel"></gr-diff-preferences>
</gr-overlay>
<div class="diffContainer">
<div class="diffContainer" on-tap="_handleTap">
<table id="diffTable"></table>
</div>
</div>

26
polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.js
View File

@@ -39,12 +39,13 @@
},
_viewMode: {
type: String,
value: DiffViewMode.SIDE_BY_SIDE,
value: DiffViewMode.UNIFIED,
},
_diff: Object,
},
observers: [
'_prefsChanged(prefs.*)',
'_render(_diff, prefs.*)',
],
reload: function() {
@@ -52,17 +53,22 @@
this._loading = true;
return this._getDiff().then(function(diff) {
var builder = this._getDiffBuilder(diff);
builder.emitDiff(diff.content);
this._diff = diff;
this._loading = false;
}.bind(this));
},
_prefsChanged: function(changeRecord) {
var prefs = changeRecord.base;
_handleTap: function(e) {
console.table(e)
},
_render: function(diff, prefsChangeRecord) {
var prefs = prefsChangeRecord.base;
this.customStyle['--content-width'] = prefs.line_length + 'ch';
this.updateStyles();
var builder = this._getDiffBuilder(diff, prefs);
builder.emitDiff(diff.content);
},
_getDiff: function() {
@@ -73,11 +79,11 @@
this.path);
},
_getDiffBuilder: function(diff) {
_getDiffBuilder: function(diff, prefs) {
if (this._viewMode === DiffViewMode.SIDE_BY_SIDE) {
return new GrDiffBuilderSideBySide(diff, this.$.diffTable);
return new GrDiffBuilderSideBySide(diff, prefs, this.$.diffTable);
} else if (this._viewMode === DiffViewMode.UNIFIED) {
return new GrDiffBuilderUnified(diff, this.$.diffTable);
return new GrDiffBuilderUnified(diff, prefs, this.$.diffTable);
}
throw Error('Unsupported diff view mode: ' + this._viewMode);
},