Introduce diff builder for binary files
Binary files cannot be diffed like text, but for some binary changes,
the diff algorithm does yield binary differences as text. With this
change, the diff builder and processor are taught to render a special
message for (non-image) binary diffs, and will ignore the text
representation of the delta.
Bug: Issue 4031
Change-Id: I2dcdbe9def006de827a37c35c42606bc1c9cf4fc
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
new file mode 100644
index 0000000..4f67142
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
@@ -0,0 +1,44 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the 'License');
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an 'AS IS' BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+(function(window, GrDiffBuilderSideBySide) {
+ 'use strict';
+
+ // Prevent redefinition.
+ if (window.GrDiffBuilderBinary) { return; }
+
+ function GrDiffBuilderBinary(diff, comments, prefs, projectName, outputEl) {
+ GrDiffBuilder.call(this, diff, comments, prefs, projectName, outputEl);
+ console.log('binary village');
+ }
+
+ GrDiffBuilderBinary.prototype = Object.create(GrDiffBuilder.prototype);
+ GrDiffBuilderBinary.prototype.constructor = GrDiffBuilderBinary;
+
+ // This method definition is a no-op to satisfy the parent type.
+ GrDiffBuilderBinary.prototype.addColumns = function(outputEl, fontSize) {};
+
+ GrDiffBuilderBinary.prototype.buildSectionElement = function() {
+ const section = this._createElement('tbody', 'binary-diff');
+ const row = this._createElement('tr');
+ const cell = this._createElement('td');
+ const label = this._createElement('label');
+ label.textContent = 'Difference in binary files';
+ cell.appendChild(label);
+ row.appendChild(cell);
+ section.appendChild(row);
+ return section;
+ };
+
+ window.GrDiffBuilderBinary = GrDiffBuilderBinary;
+})(window, GrDiffBuilderSideBySide);
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 1cfab26..7c35fb5 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
@@ -44,6 +44,7 @@
<script src="gr-diff-builder-side-by-side.js"></script>
<script src="gr-diff-builder-unified.js"></script>
<script src="gr-diff-builder-image.js"></script>
+ <script src="gr-diff-builder-binary.js"></script>
<script>
(function() {
'use strict';
@@ -141,11 +142,12 @@
this._builder.addColumns(this.diffElement, prefs.font_size);
const reporting = this.$.reporting;
+ const isBinary = !!(this.isImageDiff || this.diff.binary);
reporting.time(TimingLabel.TOTAL);
reporting.time(TimingLabel.CONTENT);
this.dispatchEvent(new CustomEvent('render-start', {bubbles: true}));
- return this.$.processor.process(this.diff.content, this.isImageDiff)
+ return this.$.processor.process(this.diff.content, isBinary)
.then(() => {
if (this.isImageDiff) {
this._builder.renderDiffImages();
@@ -255,6 +257,10 @@
return new GrDiffBuilderImage(diff, comments, prefs,
this.projectName, this.diffElement, this.baseImage,
this.revisionImage);
+ } else if (diff.binary) {
+ // If the diff is binary, but not an image.
+ return new GrDiffBuilderBinary(diff, comments, prefs,
+ this.projectName, this.diffElement);
} else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
return new GrDiffBuilderSideBySide(diff, comments, prefs,
this.projectName, this.diffElement, this._layers);
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 9434487..499e33b 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
@@ -104,7 +104,7 @@
* @param {Object} group
*/
GrDiffBuilder.prototype.buildSectionElement = function() {
- throw Error('Subclasses must implement buildGroupElement');
+ throw Error('Subclasses must implement buildSectionElement');
};
GrDiffBuilder.prototype.emitGroup = function(group, opt_beforeSection) {
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 952c063..e9b1b40 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
@@ -752,6 +752,63 @@
});
});
+ suite('rendering text, images and binary files', () => {
+ let processStub;
+ let comments;
+ let prefs;
+ let content;
+
+ setup(() => {
+ element = fixture('basic');
+ element.viewMode = 'SIDE_BY_SIDE';
+ processStub = sandbox.stub(element.$.processor, 'process')
+ .returns(Promise.resolve());
+ sandbox.stub(element, '_anyLineTooLong').returns(true);
+ comments = {left: [], right: []};
+ prefs = {
+ line_length: 10,
+ show_tabs: true,
+ tab_size: 4,
+ context: -1,
+ syntax_highlighting: true,
+ };
+ content = [{
+ a: ['all work and no play make andybons a dull boy'],
+ b: ['elgoog elgoog elgoog'],
+ }, {
+ ab: [
+ 'Non eram nescius, Brute, cum, quae summis ingeniis ',
+ 'exquisitaque doctrina philosophi Graeco sermone tractavissent',
+ ],
+ }];
+ });
+
+ test('text', () => {
+ element.diff = {content};
+ return element.render(comments, prefs).then(() => {
+ assert.isTrue(processStub.calledOnce);
+ assert.isFalse(processStub.lastCall.args[1]);
+ });
+ });
+
+ test('image', () => {
+ element.diff = {content, binary: true};
+ element.isImageDiff = true;
+ return element.render(comments, prefs).then(() => {
+ assert.isTrue(processStub.calledOnce);
+ assert.isTrue(processStub.lastCall.args[1]);
+ });
+ });
+
+ test('binary', () => {
+ element.diff = {content, binary: true};
+ return element.render(comments, prefs).then(() => {
+ assert.isTrue(processStub.calledOnce);
+ assert.isTrue(processStub.lastCall.args[1]);
+ });
+ });
+ });
+
suite('rendering', () => {
let content;
let outputEl;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index bca6bea..230cea9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -104,12 +104,13 @@
* @return {Promise} A promise that resolves when the diff is completely
* processed.
*/
- process(content, isImageDiff) {
+ process(content, isBinary) {
this.groups = [];
this.push('groups', this._makeFileComments());
- // If image diff, only render the file lines.
- if (isImageDiff) { return Promise.resolve(); }
+ // If it's a binary diff, we won't be rendering hunks of text differences
+ // so finish processing.
+ if (isBinary) { return Promise.resolve(); }
return new Promise(resolve => {
const state = {
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 f32234a..4791a3c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -71,7 +71,8 @@
max-width: 50em;
outline: 1px solid #ccc;
}
- .image-diff label {
+ .image-diff label,
+ .binary-diff label {
font-family: var(--font-family);
font-style: italic;
}
