Refactoring only: Split up the diff builder into two classes
GrDiffBuilder had two responsibilities:
1. Manage the data and the process for building a diff.
2. Create diff elements programmatically.
This change splits the builder up into two classes. It adds
`GrDiffBuilderLegacy` as a subclass of `GrDiffBuilder`.
The goal is to create alternative diff builders that are based on (Lit)
components instead of creating elements programmatically. For doing that
we will be re-using `GrDiffBuilder`, but replacing
`GrDiffBuilderLegacy`. That is why we need the split, although splitting
up large classes generally makes sense. :-)
We are also moving some stateless utilities into `gr-diff-utils.ts`.
Release-Notes: skip
Change-Id: Iaf92695139135edafdc81922fbf2435cfe2832c4
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
index ec41fc8..6699b57 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
@@ -14,12 +14,10 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
import {GrDiffBuilderUnified} from './gr-diff-builder-unified';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
import {queryAndAssert} from '../../../utils/common-util';
-import {RenderPreferences} from '../../../api/diff';
import {createElementDiff} from '../gr-diff/gr-diff-utils';
export class GrDiffBuilderBinary extends GrDiffBuilderUnified {
@@ -34,12 +32,10 @@
override buildSectionElement(): HTMLElement {
const section = createElementDiff('tbody', 'binary-diff');
const line = new GrDiffLine(GrDiffLineType.BOTH, 'FILE', 'FILE');
- const fileRow = this._createRow(line);
+ const fileRow = this.createRow(line);
const contentTd = queryAndAssert(fileRow, 'td.both.file')!;
contentTd.textContent = ' Difference in binary files';
section.appendChild(fileRow);
return section;
}
-
- override updateRenderPrefs(_renderPrefs: RenderPreferences) {}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
index bbebb21..8421620 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -588,7 +588,7 @@
setBlame(blame: BlameInfo[] | null) {
if (!this._builder) return;
- this._builder.setBlame(blame);
+ this._builder.setBlame(blame ?? []);
}
updateRenderPrefs(renderPrefs: RenderPreferences) {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.js b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.js
index feee78f..7ee2f17 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.js
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.js
@@ -28,6 +28,7 @@
import {DiffViewMode, Side} from '../../../api/diff.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {afterNextRender} from '@polymer/polymer/lib/utils/render-status';
+import {GrDiffBuilderLegacy} from './gr-diff-builder-legacy.js';
const basicFixture = fixtureFromTemplate(html`
<gr-diff-builder>
@@ -67,48 +68,7 @@
stubRestApi('getProjectConfig').returns(Promise.resolve({}));
stubBaseUrl('/r');
prefs = {...DEFAULT_PREFS};
- builder = new GrDiffBuilder({content: []}, prefs);
- });
-
- test('newlines 1', () => {
- let text = 'abcdef';
-
- assert.equal(builder._formatText(text, 'NONE', 4, 10).innerHTML, text);
- text = 'a'.repeat(20);
- assert.equal(builder._formatText(text, 'NONE', 4, 10).innerHTML,
- 'a'.repeat(10) +
- LINE_BREAK_HTML +
- 'a'.repeat(10));
- });
-
- test('newlines 2', () => {
- const text = '<span class="thumbsup">๐</span>';
- assert.equal(builder._formatText(text, 'NONE', 4, 10).innerHTML,
- '<span clas' +
- LINE_BREAK_HTML +
- 's="thumbsu' +
- LINE_BREAK_HTML +
- 'p">๐</span' +
- LINE_BREAK_HTML +
- '>');
- });
-
- test('newlines 3', () => {
- const text = '01234\t56789';
- assert.equal(builder._formatText(text, 'NONE', 4, 10).innerHTML,
- '01234' + builder._getTabWrapper(3).outerHTML + '56' +
- LINE_BREAK_HTML +
- '789');
- });
-
- test('newlines 4', () => {
- const text = '๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐';
- assert.equal(builder._formatText(text, 'NONE', 4, 20).innerHTML,
- '๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐' +
- LINE_BREAK_HTML +
- '๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐' +
- LINE_BREAK_HTML +
- '๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐๐');
+ builder = new GrDiffBuilderLegacy({content: []}, prefs);
});
test('line_length applied with <wbr> if line_wrapping is true', () => {
@@ -117,7 +77,7 @@
const line = {text, highlights: []};
const expected = 'a'.repeat(50) + WBR_HTML + 'a';
- const result = builder._createTextEl(undefined, line).firstChild.innerHTML;
+ const result = builder.createTextEl(undefined, line).firstChild.innerHTML;
assert.equal(result, expected);
});
@@ -127,7 +87,7 @@
const line = {text, highlights: []};
const expected = 'a'.repeat(50) + LINE_BREAK_HTML + 'a';
- const result = builder._createTextEl(undefined, line).firstChild.innerHTML;
+ const result = builder.createTextEl(undefined, line).firstChild.innerHTML;
assert.equal(result, expected);
});
@@ -152,10 +112,10 @@
});
});
- test('_createTextEl linewrap with tabs', () => {
+ test('createTextEl linewrap with tabs', () => {
const text = '\t'.repeat(7) + '!';
const line = {text, highlights: []};
- const el = builder._createTextEl(undefined, line);
+ const el = builder.createTextEl(undefined, line);
assert.equal(el.innerText, text);
// With line length 10 and tab size 2, there should be a line break
// after every two tabs.
@@ -166,70 +126,6 @@
newlineEl);
});
- test('text length with tabs and unicode', () => {
- function expectTextLength(text, tabSize, expected) {
- // Formatting to |expected| columns should not introduce line breaks.
- const result = builder._formatText(text, 'NONE', tabSize, expected);
- assert.isNotOk(result.querySelector('.contentText > .br'),
- ` Expected the result of: \n` +
- ` _formatText(${text}', 'NONE', ${tabSize}, ${expected})\n` +
- ` to not contain a br. But the actual result HTML was:\n` +
- ` '${result.innerHTML}'\nwhereupon`);
-
- // Increasing the line limit should produce the same markup.
- assert.equal(
- builder._formatText(text, 'NONE', tabSize, Infinity).innerHTML,
- result.innerHTML);
- assert.equal(
- builder._formatText(text, 'NONE', tabSize, expected + 1).innerHTML,
- result.innerHTML);
-
- // Decreasing the line limit should introduce line breaks.
- if (expected > 0) {
- const tooSmall = builder._formatText(text,
- 'NONE', tabSize, expected - 1);
- assert.isOk(tooSmall.querySelector('.contentText > .br'),
- ` Expected the result of: \n` +
- ` _formatText(${text}', ${tabSize}, ${expected - 1})\n` +
- ` to contain a br. But the actual result HTML was:\n` +
- ` '${tooSmall.innerHTML}'\nwhereupon`);
- }
- }
- expectTextLength('12345', 4, 5);
- expectTextLength('\t\t12', 4, 10);
- expectTextLength('abc๐ข123', 4, 7);
- expectTextLength('abc\t', 8, 8);
- expectTextLength('abc\t\t', 10, 20);
- expectTextLength('', 10, 0);
- // 17 Thai combining chars.
- expectTextLength('เธเนเนเนเนเนเนเนเนเนเนเนเนเนเนเนเน', 4, 17);
- expectTextLength('abc\tde', 10, 12);
- expectTextLength('abc\tde\t', 10, 20);
- expectTextLength('\t\t\t\t\t', 20, 100);
- });
-
- test('tab wrapper insertion', () => {
- const html = 'abc\tdef';
- const tabSize = builder._prefs.tab_size;
- const wrapper = builder._getTabWrapper(tabSize - 3);
- assert.ok(wrapper);
- assert.equal(wrapper.innerText, '\t');
- assert.equal(
- builder._formatText(html, 'NONE', tabSize, Infinity).innerHTML,
- 'abc' + wrapper.outerHTML + 'def');
- });
-
- test('tab wrapper style', () => {
- const pattern = new RegExp('^<span class="style-scope gr-diff tab" ' +
- 'style="((?:-moz-)?tab-size: (\\d+);.?)+">\\t<\\/span>$');
-
- for (const size of [1, 3, 8, 55]) {
- const html = builder._getTabWrapper(size).outerHTML;
- expect(html).to.match(pattern);
- assert.equal(html.match(pattern)[2], size);
- }
- });
-
test('_handlePreferenceError throws with invalid preference', () => {
element.prefs = {tab_size: 0};
assert.throws(() => element._getDiffBuilder());
@@ -244,14 +140,14 @@
`Fix in diff preferences`);
});
- suite('_isTotal', () => {
+ suite('isTotal', () => {
test('is total for add', () => {
const lines = [];
for (let idx = 0; idx < 10; idx++) {
lines.push(new GrDiffLine(GrDiffLineType.ADD));
}
const group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
- assert.isTrue(GrDiffBuilder.prototype._isTotal(group));
+ assert.isTrue(GrDiffBuilder.prototype.isTotal(group));
});
test('is total for remove', () => {
@@ -260,12 +156,12 @@
lines.push(new GrDiffLine(GrDiffLineType.REMOVE));
}
const group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
- assert.isTrue(GrDiffBuilder.prototype._isTotal(group));
+ assert.isTrue(GrDiffBuilder.prototype.isTotal(group));
});
test('not total for empty', () => {
const group = new GrDiffGroup({type: GrDiffGroupType.BOTH});
- assert.isFalse(GrDiffBuilder.prototype._isTotal(group));
+ assert.isFalse(GrDiffBuilder.prototype.isTotal(group));
});
test('not total for non-delta', () => {
@@ -274,7 +170,7 @@
lines.push(new GrDiffLine(GrDiffLineType.BOTH));
}
const group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
- assert.isFalse(GrDiffBuilder.prototype._isTotal(group));
+ assert.isFalse(GrDiffBuilder.prototype.isTotal(group));
});
});
@@ -1003,13 +899,13 @@
}
});
- test('_renderContentByRange', () => {
- const spy = sinon.spy(builder, '_createTextEl');
+ test('renderContentByRange', () => {
+ const spy = sinon.spy(builder, 'createTextEl');
const start = 9;
const end = 14;
const count = end - start + 1;
- builder._renderContentByRange(start, end, 'left');
+ builder.renderContentByRange(start, end, 'left');
assert.equal(spy.callCount, count);
spy.getCalls().forEach((call, i) => {
@@ -1017,10 +913,10 @@
});
});
- test('_renderContentByRange notexistent elements', () => {
- const spy = sinon.spy(builder, '_createTextEl');
+ test('renderContentByRange notexistent elements', () => {
+ const spy = sinon.spy(builder, 'createTextEl');
- sinon.stub(builder, '_getLineNumberEl').returns(
+ sinon.stub(builder, 'getLineNumberEl').returns(
document.createElement('div')
);
sinon.stub(builder, 'findLinesByRange').callsFake(
@@ -1039,29 +935,29 @@
lines.push(new GrDiffLine(GrDiffLineType.BOTH));
});
- builder._renderContentByRange(1, 10, 'left');
+ builder.renderContentByRange(1, 10, 'left');
// Should be called only once because only one line had a corresponding
// element.
assert.equal(spy.callCount, 1);
});
- test('_getLineNumberEl side-by-side left', () => {
+ test('getLineNumberEl side-by-side left', () => {
const contentEl = builder.getContentByLine(5, 'left',
element.$.diffTable);
- const lineNumberEl = builder._getLineNumberEl(contentEl, 'left');
+ const lineNumberEl = builder.getLineNumberEl(contentEl, 'left');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('left'));
});
- test('_getLineNumberEl side-by-side right', () => {
+ test('getLineNumberEl side-by-side right', () => {
const contentEl = builder.getContentByLine(5, 'right',
element.$.diffTable);
- const lineNumberEl = builder._getLineNumberEl(contentEl, 'right');
+ const lineNumberEl = builder.getLineNumberEl(contentEl, 'right');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('right'));
});
- test('_getLineNumberEl unified left', async () => {
+ test('getLineNumberEl unified left', async () => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
await element.render(keyLocations);
@@ -1069,12 +965,12 @@
const contentEl = builder.getContentByLine(5, 'left',
element.$.diffTable);
- const lineNumberEl = builder._getLineNumberEl(contentEl, 'left');
+ const lineNumberEl = builder.getLineNumberEl(contentEl, 'left');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('left'));
});
- test('_getLineNumberEl unified right', async () => {
+ test('getLineNumberEl unified right', async () => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
await element.render(keyLocations);
@@ -1082,36 +978,36 @@
const contentEl = builder.getContentByLine(5, 'right',
element.$.diffTable);
- const lineNumberEl = builder._getLineNumberEl(contentEl, 'right');
+ const lineNumberEl = builder.getLineNumberEl(contentEl, 'right');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('right'));
});
- test('_getNextContentOnSide side-by-side left', () => {
+ test('getNextContentOnSide side-by-side left', () => {
const startElem = builder.getContentByLine(5, 'left',
element.$.diffTable);
const expectedStartString = diff.content[2].ab[0];
const expectedNextString = diff.content[2].ab[1];
assert.equal(startElem.textContent, expectedStartString);
- const nextElem = builder._getNextContentOnSide(startElem,
+ const nextElem = builder.getNextContentOnSide(startElem,
'left');
assert.equal(nextElem.textContent, expectedNextString);
});
- test('_getNextContentOnSide side-by-side right', () => {
+ test('getNextContentOnSide side-by-side right', () => {
const startElem = builder.getContentByLine(5, 'right',
element.$.diffTable);
const expectedStartString = diff.content[1].b[0];
const expectedNextString = diff.content[1].b[1];
assert.equal(startElem.textContent, expectedStartString);
- const nextElem = builder._getNextContentOnSide(startElem,
+ const nextElem = builder.getNextContentOnSide(startElem,
'right');
assert.equal(nextElem.textContent, expectedNextString);
});
- test('_getNextContentOnSide unified left', async () => {
+ test('getNextContentOnSide unified left', async () => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
await element.render(keyLocations);
@@ -1123,12 +1019,12 @@
const expectedNextString = diff.content[2].ab[1];
assert.equal(startElem.textContent, expectedStartString);
- const nextElem = builder._getNextContentOnSide(startElem,
+ const nextElem = builder.getNextContentOnSide(startElem,
'left');
assert.equal(nextElem.textContent, expectedNextString);
});
- test('_getNextContentOnSide unified right', async () => {
+ test('getNextContentOnSide unified right', async () => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
await element.render(keyLocations);
@@ -1140,22 +1036,10 @@
const expectedNextString = diff.content[1].b[1];
assert.equal(startElem.textContent, expectedStartString);
- const nextElem = builder._getNextContentOnSide(startElem,
+ const nextElem = builder.getNextContentOnSide(startElem,
'right');
assert.equal(nextElem.textContent, expectedNextString);
});
-
- test('escaping HTML', () => {
- let input = '<script>alert("XSS");<' + '/script>';
- let expected = '<script>alert("XSS");</script>';
- let result = builder._formatText(input, 1, Infinity).innerHTML;
- assert.equal(result, expected);
-
- input = '& < > " \' / `';
- expected = '& < > " \' / `';
- result = builder._formatText(input, 1, Infinity).innerHTML;
- assert.equal(result, expected);
- });
});
suite('blame', () => {
@@ -1169,73 +1053,68 @@
});
test('setBlame attempts to render each blamed line', () => {
- const getBlameStub = sinon.stub(builder, '_getBlameByLineNum')
+ const getBlameStub = sinon.stub(builder, 'getBlameTdByLine')
.returns(null);
builder.setBlame(mockBlame);
assert.equal(getBlameStub.callCount, 32);
});
- test('_getBlameCommitForBaseLine', () => {
- sinon.stub(builder, '_getBlameByLineNum').returns(null);
+ test('getBlameCommitForBaseLine', () => {
+ sinon.stub(builder, 'getBlameTdByLine').returns(undefined);
builder.setBlame(mockBlame);
- assert.isOk(builder._getBlameCommitForBaseLine(1));
- assert.equal(builder._getBlameCommitForBaseLine(1).id, 'commit 1');
+ assert.isOk(builder.getBlameCommitForBaseLine(1));
+ assert.equal(builder.getBlameCommitForBaseLine(1).id, 'commit 1');
- assert.isOk(builder._getBlameCommitForBaseLine(11));
- assert.equal(builder._getBlameCommitForBaseLine(11).id, 'commit 1');
+ assert.isOk(builder.getBlameCommitForBaseLine(11));
+ assert.equal(builder.getBlameCommitForBaseLine(11).id, 'commit 1');
- assert.isOk(builder._getBlameCommitForBaseLine(32));
- assert.equal(builder._getBlameCommitForBaseLine(32).id, 'commit 2');
+ assert.isOk(builder.getBlameCommitForBaseLine(32));
+ assert.equal(builder.getBlameCommitForBaseLine(32).id, 'commit 2');
- assert.isNull(builder._getBlameCommitForBaseLine(33));
+ assert.isUndefined(builder.getBlameCommitForBaseLine(33));
});
- test('_getBlameCommitForBaseLine w/o blame returns null', () => {
- assert.isNull(builder._getBlameCommitForBaseLine(1));
- assert.isNull(builder._getBlameCommitForBaseLine(11));
- assert.isNull(builder._getBlameCommitForBaseLine(31));
+ test('getBlameCommitForBaseLine w/o blame returns null', () => {
+ assert.isUndefined(builder.getBlameCommitForBaseLine(1));
+ assert.isUndefined(builder.getBlameCommitForBaseLine(11));
+ assert.isUndefined(builder.getBlameCommitForBaseLine(31));
});
- test('_createBlameCell', () => {
- const mocbBlameCell = document.createElement('span');
- const getBlameStub = sinon.stub(builder, '_getBlameForBaseLine')
- .returns(mocbBlameCell);
- const line = new GrDiffLine(GrDiffLineType.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const result = builder._createBlameCell(line.beforeNumber);
-
- assert.isTrue(getBlameStub.calledWithExactly(3));
- assert.equal(result.getAttribute('data-line-number'), '3');
- assert.equal(result.firstChild, mocbBlameCell);
- });
-
- test('_getBlameForBaseLine', () => {
- const mockCommit = {
- time: 1576105200,
+ test('createBlameCell', () => {
+ const mockBlameInfo = {
+ time: 1576155200,
id: 1234567890,
author: 'Clark Kent',
commit_msg: 'Testing Commit',
ranges: [1],
};
- const blameNode = builder._getBlameForBaseLine(1, mockCommit);
+ const getBlameStub = sinon.stub(builder, 'getBlameCommitForBaseLine')
+ .returns(mockBlameInfo);
+ const line = new GrDiffLine(GrDiffLineType.BOTH);
+ line.beforeNumber = 3;
+ line.afterNumber = 5;
- const authors = blameNode.getElementsByClassName('blameAuthor');
- assert.equal(authors.length, 1);
- assert.equal(authors[0].innerText, ' Clark');
+ const result = builder.createBlameCell(line.beforeNumber);
- const date = (new Date(mockCommit.time * 1000)).toLocaleDateString();
- flush();
- const cards = blameNode.getElementsByClassName('blameHoverCard');
- assert.equal(cards.length, 1);
- assert.equal(cards[0].innerHTML,
- `Commit 1234567890<br>Author: Clark Kent<br>Date: ${date}`
- + '<br><br>Testing Commit'
- );
-
- const url = blameNode.getElementsByClassName('blameDate');
- assert.equal(url[0].getAttribute('href'), '/r/q/1234567890');
+ assert.isTrue(getBlameStub.calledWithExactly(3));
+ assert.equal(result.getAttribute('data-line-number'), '3');
+ expect(result).dom.to.equal(/* HTML */`
+ <span class="gr-diff style-scope">
+ <a class="blameDate gr-diff style-scope" href="/r/q/1234567890">
+ 12/12/2019
+ </a>
+ <span class="blameAuthor gr-diff style-scope">Clark</span>
+ <gr-hovercard class="gr-diff style-scope">
+ <span class="blameHoverCard gr-diff style-scope">
+ Commit 1234567890<br>
+ Author: Clark Kent<br>
+ Date: 12/12/2019<br>
+ <br>
+ Testing Commit
+ </span>
+ </gr-hovercard>
+ </span>
+ `);
});
});
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts
index 92d99e1..3bcec06 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts
@@ -47,13 +47,13 @@
if (this._useNewImageDiffUi) {
this._emitImageViewer(section);
- this._outputEl.appendChild(section);
+ this.outputEl.appendChild(section);
} else {
this._emitImagePair(section);
this._emitImageLabels(section);
- this._outputEl.appendChild(section);
- this._outputEl.appendChild(this._createEndpoint());
+ this.outputEl.appendChild(section);
+ this.outputEl.appendChild(this._createEndpoint());
}
}
@@ -98,7 +98,7 @@
imageViewer.baseUrl = this._getImageSrc(this._baseImage);
imageViewer.revisionUrl = this._getImageSrc(this._revisionImage);
imageViewer.automaticBlink =
- !!this._renderPrefs?.image_diff_prefs?.automatic_blink;
+ !!this.renderPrefs?.image_diff_prefs?.automatic_blink;
td.appendChild(imageViewer);
tr.appendChild(td);
@@ -218,7 +218,7 @@
}
override updateRenderPrefs(renderPrefs: RenderPreferences) {
- const imageViewer = this._outputEl.querySelector(
+ const imageViewer = this.outputEl.querySelector(
'gr-image-viewer'
) as GrImageViewer;
if (this._useNewImageDiffUi && imageViewer) {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
new file mode 100644
index 0000000..298b280
--- /dev/null
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
@@ -0,0 +1,467 @@
+/**
+ * @license
+ * Copyright (C) 2016 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.
+ */
+import {
+ MovedLinkClickedEventDetail,
+ RenderPreferences,
+} from '../../../api/diff';
+import {fire} from '../../../utils/event-util';
+import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
+import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
+import '../gr-context-controls/gr-context-controls';
+import {
+ GrContextControls,
+ GrContextControlsShowConfig,
+} from '../gr-context-controls/gr-context-controls';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
+import {DiffViewMode, Side} from '../../../constants/constants';
+import {DiffLayer} from '../../../types/types';
+import {
+ createBlameElement,
+ createElementDiff,
+ createElementDiffWithText,
+ formatText,
+ getResponsiveMode,
+} from '../gr-diff/gr-diff-utils';
+import {GrDiffBuilder} from './gr-diff-builder';
+import {BlameInfo} from '../../../types/common';
+
+/**
+ * Base class for builders that are creating the DOM elements programmatically
+ * by calling `document.createElement()` and such. We are calling such builders
+ * "legacy", because we want to create (Lit) component based diff elements.
+ *
+ * TODO: Do not subclass `GrDiffBuilder`. Use composition and interfaces.
+ */
+export abstract class GrDiffBuilderLegacy extends GrDiffBuilder {
+ constructor(
+ diff: DiffInfo,
+ prefs: DiffPreferencesInfo,
+ outputEl: HTMLElement,
+ layers: DiffLayer[] = [],
+ renderPrefs?: RenderPreferences
+ ) {
+ super(diff, prefs, outputEl, layers, renderPrefs);
+ }
+
+ override getContentTdByLine(
+ lineNumber: LineNumber,
+ side?: Side,
+ root: Element = this.outputEl
+ ): Element | null {
+ const sideSelector: string = side ? `.${side}` : '';
+ return root.querySelector(
+ `td.lineNum[data-value="${lineNumber}"]${sideSelector} ~ td.content`
+ );
+ }
+
+ override getBlameTdByLine(lineNum: number): Element | undefined {
+ return (
+ this.outputEl.querySelector(`td.blame[data-line-number="${lineNum}"]`) ??
+ undefined
+ );
+ }
+
+ override getContentByLine(
+ lineNumber: LineNumber,
+ side?: Side,
+ root?: HTMLElement
+ ): HTMLElement | null {
+ const td = this.getContentTdByLine(lineNumber, side, root);
+ return td ? td.querySelector('.contentText') : null;
+ }
+
+ override renderContentByRange(
+ start: LineNumber,
+ end: LineNumber,
+ side: Side
+ ) {
+ const lines: GrDiffLine[] = [];
+ const elements: HTMLElement[] = [];
+ let line;
+ let el;
+ this.findLinesByRange(start, end, side, lines, elements);
+ for (let i = 0; i < lines.length; i++) {
+ line = lines[i];
+ el = elements[i];
+ if (!el || !el.parentElement) {
+ // Cannot re-render an element if it does not exist. This can happen
+ // if lines are collapsed and not visible on the page yet.
+ continue;
+ }
+ const lineNumberEl = this.getLineNumberEl(el, side);
+ el.parentElement.replaceChild(
+ this.createTextEl(lineNumberEl, line, side).firstChild!,
+ el
+ );
+ }
+ }
+
+ override renderBlameByRange(blame: BlameInfo, start: number, end: number) {
+ for (let i = start; i <= end; i++) {
+ // TODO(wyatta): this query is expensive, but, when traversing a
+ // range, the lines are consecutive, and given the previous blame
+ // cell, the next one can be reached cheaply.
+ const blameCell = this.getBlameTdByLine(i);
+ if (!blameCell) continue;
+
+ // Remove the element's children (if any).
+ while (blameCell.hasChildNodes()) {
+ blameCell.removeChild(blameCell.lastChild!);
+ }
+ const blameEl = createBlameElement(i, blame);
+ if (blameEl) blameCell.appendChild(blameEl);
+ }
+ }
+
+ /**
+ * Finds the line number element given the content element by walking up the
+ * DOM tree to the diff row and then querying for a .lineNum element on the
+ * requested side.
+ *
+ * TODO(brohlfs): Consolidate this with getLineEl... methods in html file.
+ */
+ private getLineNumberEl(
+ content: HTMLElement,
+ side: Side
+ ): HTMLElement | null {
+ let row: HTMLElement | null = content;
+ while (row && !row.classList.contains('diff-row')) row = row.parentElement;
+ return row ? (row.querySelector('.lineNum.' + side) as HTMLElement) : null;
+ }
+
+ /**
+ * Adds <tr> table rows to a <tbody> section for allowing the user to expand
+ * collapsed of lines. Called by subclasses.
+ */
+ protected createContextControls(
+ section: HTMLElement,
+ group: GrDiffGroup,
+ viewMode: DiffViewMode
+ ) {
+ const leftStart = group.lineRange.left.start_line;
+ const leftEnd = group.lineRange.left.end_line;
+ const firstGroupIsSkipped = !!group.contextGroups[0].skip;
+ const lastGroupIsSkipped =
+ !!group.contextGroups[group.contextGroups.length - 1].skip;
+
+ const containsWholeFile = this.numLinesLeft === leftEnd - leftStart + 1;
+ const showAbove =
+ (leftStart > 1 && !firstGroupIsSkipped) || containsWholeFile;
+ const showBelow = leftEnd < this.numLinesLeft && !lastGroupIsSkipped;
+
+ if (showAbove) {
+ const paddingRow = this.createContextControlPaddingRow(viewMode);
+ paddingRow.classList.add('above');
+ section.appendChild(paddingRow);
+ }
+ section.appendChild(
+ this.createContextControlRow(
+ section,
+ group,
+ showAbove,
+ showBelow,
+ viewMode
+ )
+ );
+ if (showBelow) {
+ const paddingRow = this.createContextControlPaddingRow(viewMode);
+ paddingRow.classList.add('below');
+ section.appendChild(paddingRow);
+ }
+ }
+
+ /**
+ * Creates a context control <tr> table row for with buttons the allow the
+ * user to expand collapsed lines. Buttons extend from the gap created by this
+ * method up or down into the area of code that they affect.
+ */
+ private createContextControlRow(
+ section: HTMLElement,
+ group: GrDiffGroup,
+ showAbove: boolean,
+ showBelow: boolean,
+ viewMode: DiffViewMode
+ ): HTMLElement {
+ const row = createElementDiff('tr', 'dividerRow');
+ let showConfig: GrContextControlsShowConfig;
+ if (showAbove && !showBelow) {
+ showConfig = 'above';
+ } else if (!showAbove && showBelow) {
+ showConfig = 'below';
+ } else {
+ // Note that !showAbove && !showBelow also intentionally creates
+ // "show-both". This means the file is completely collapsed, which is
+ // unusual, but at least happens in one test.
+ showConfig = 'both';
+ }
+ row.classList.add(`show-${showConfig}`);
+
+ row.appendChild(this.createBlameCell(0));
+ if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ row.appendChild(createElementDiff('td'));
+ }
+
+ const cell = createElementDiff('td', 'dividerCell');
+ cell.setAttribute('colspan', '3');
+ row.appendChild(cell);
+
+ const contextControls = createElementDiff(
+ 'gr-context-controls'
+ ) as GrContextControls;
+ contextControls.diff = this._diff;
+ contextControls.renderPreferences = this.renderPrefs;
+ contextControls.section = section;
+ contextControls.group = group;
+ contextControls.showConfig = showConfig;
+ cell.appendChild(contextControls);
+ return row;
+ }
+
+ /**
+ * Creates a table row to serve as padding between code and context controls.
+ * Blame column, line gutters, and content area will continue visually, but
+ * context controls can render over this background to map more clearly to
+ * the area of code they expand.
+ */
+ private createContextControlPaddingRow(viewMode: DiffViewMode) {
+ const row = createElementDiff('tr', 'contextBackground');
+
+ if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ row.classList.add('side-by-side');
+ row.setAttribute('left-type', GrDiffGroupType.CONTEXT_CONTROL);
+ row.setAttribute('right-type', GrDiffGroupType.CONTEXT_CONTROL);
+ } else {
+ row.classList.add('unified');
+ }
+
+ row.appendChild(this.createBlameCell(0));
+ row.appendChild(createElementDiff('td', 'contextLineNum'));
+ if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ row.appendChild(createElementDiff('td'));
+ }
+ row.appendChild(createElementDiff('td', 'contextLineNum'));
+ row.appendChild(createElementDiff('td'));
+
+ return row;
+ }
+
+ protected createLineEl(
+ line: GrDiffLine,
+ number: LineNumber,
+ type: GrDiffLineType,
+ side: Side
+ ) {
+ const td = createElementDiff('td');
+ td.classList.add(side);
+ if (line.type === GrDiffLineType.BLANK) {
+ return td;
+ }
+ if (line.type === GrDiffLineType.BOTH || line.type === type) {
+ td.classList.add('lineNum');
+ td.dataset['value'] = number.toString();
+
+ if (
+ ((this._prefs.show_file_comment_button === false ||
+ this.renderPrefs?.show_file_comment_button === false) &&
+ number === 'FILE') ||
+ number === 'LOST'
+ ) {
+ return td;
+ }
+
+ const button = createElementDiff('button');
+ td.appendChild(button);
+ button.tabIndex = -1;
+ button.classList.add('lineNumButton');
+ button.classList.add(side);
+ button.dataset['value'] = number.toString();
+ button.textContent = number === 'FILE' ? 'File' : number.toString();
+ if (number === 'FILE') {
+ button.setAttribute('aria-label', 'Add file comment');
+ }
+
+ // Add aria-labels for valid line numbers.
+ // For unified diff, this method will be called with number set to 0 for
+ // the empty line number column for added/removed lines. This should not
+ // be announced to the screenreader.
+ if (number > 0) {
+ if (line.type === GrDiffLineType.REMOVE) {
+ button.setAttribute('aria-label', `${number} removed`);
+ } else if (line.type === GrDiffLineType.ADD) {
+ button.setAttribute('aria-label', `${number} added`);
+ }
+ }
+ this.addLineNumberMouseEvents(td, number, side);
+ }
+ return td;
+ }
+
+ private addLineNumberMouseEvents(
+ el: HTMLElement,
+ number: LineNumber,
+ side: Side
+ ) {
+ el.addEventListener('mouseenter', () => {
+ fire(el, 'line-mouse-enter', {lineNum: number, side});
+ });
+ el.addEventListener('mouseleave', () => {
+ fire(el, 'line-mouse-leave', {lineNum: number, side});
+ });
+ }
+
+ protected createTextEl(
+ lineNumberEl: HTMLElement | null,
+ line: GrDiffLine,
+ side?: Side
+ ) {
+ const td = createElementDiff('td');
+ if (line.type !== GrDiffLineType.BLANK) {
+ td.classList.add('content');
+ }
+
+ // If intraline info is not available, the entire line will be
+ // considered as changed and marked as dark red / green color
+ if (!line.hasIntralineInfo) {
+ td.classList.add('no-intraline-info');
+ }
+ td.classList.add(line.type);
+
+ const {beforeNumber, afterNumber} = line;
+ if (beforeNumber !== 'FILE' && beforeNumber !== 'LOST') {
+ const responsiveMode = getResponsiveMode(this._prefs, this.renderPrefs);
+ const contentText = formatText(
+ line.text,
+ responsiveMode,
+ this._prefs.tab_size,
+ this._prefs.line_length
+ );
+
+ if (side) {
+ contentText.setAttribute('data-side', side);
+ const number = side === Side.LEFT ? beforeNumber : afterNumber;
+ this.addLineNumberMouseEvents(td, number, side);
+ }
+
+ if (lineNumberEl && side) {
+ for (const layer of this.layers) {
+ if (typeof layer.annotate === 'function') {
+ layer.annotate(contentText, lineNumberEl, line, side);
+ }
+ }
+ } else {
+ console.error('lineNumberEl or side not set, skipping layer.annotate');
+ }
+
+ td.appendChild(contentText);
+ } else if (line.beforeNumber === 'FILE') td.classList.add('file');
+ else if (line.beforeNumber === 'LOST') td.classList.add('lost');
+
+ return td;
+ }
+
+ private createMovedLineAnchor(line: number, side: Side) {
+ const anchor = createElementDiffWithText('a', `${line}`);
+
+ // href is not actually used but important for Screen Readers
+ anchor.setAttribute('href', `#${line}`);
+ anchor.addEventListener('click', e => {
+ e.preventDefault();
+ anchor.dispatchEvent(
+ new CustomEvent<MovedLinkClickedEventDetail>('moved-link-clicked', {
+ detail: {
+ lineNum: line,
+ side,
+ },
+ composed: true,
+ bubbles: true,
+ })
+ );
+ });
+ return anchor;
+ }
+
+ private createMoveDescriptionDiv(movedIn: boolean, group: GrDiffGroup) {
+ const div = createElementDiff('div');
+ if (group.moveDetails?.range) {
+ const {changed, range} = group.moveDetails;
+ const otherSide = movedIn ? Side.LEFT : Side.RIGHT;
+ const andChangedLabel = changed ? 'and changed ' : '';
+ const direction = movedIn ? 'from' : 'to';
+ const textLabel = `Moved ${andChangedLabel}${direction} lines `;
+ div.appendChild(createElementDiffWithText('span', textLabel));
+ div.appendChild(this.createMovedLineAnchor(range.start, otherSide));
+ div.appendChild(createElementDiffWithText('span', ' - '));
+ div.appendChild(this.createMovedLineAnchor(range.end, otherSide));
+ } else {
+ div.appendChild(
+ createElementDiffWithText('span', movedIn ? 'Moved in' : 'Moved out')
+ );
+ }
+ return div;
+ }
+
+ protected buildMoveControls(group: GrDiffGroup) {
+ const movedIn = group.adds.length > 0;
+ const {numberOfCells, movedOutIndex, movedInIndex, lineNumberCols} =
+ this.getMoveControlsConfig();
+
+ let controlsClass;
+ let descriptionIndex;
+ const descriptionTextDiv = this.createMoveDescriptionDiv(movedIn, group);
+ if (movedIn) {
+ controlsClass = 'movedIn';
+ descriptionIndex = movedInIndex;
+ } else {
+ controlsClass = 'movedOut';
+ descriptionIndex = movedOutIndex;
+ }
+
+ const controls = createElementDiff('tr', `moveControls ${controlsClass}`);
+ const cells = [...Array(numberOfCells).keys()].map(() =>
+ createElementDiff('td')
+ );
+ lineNumberCols.forEach(index => {
+ cells[index].classList.add('moveControlsLineNumCol');
+ });
+
+ const moveRangeHeader = createElementDiff('gr-range-header');
+ moveRangeHeader.setAttribute('icon', 'gr-icons:move-item');
+ moveRangeHeader.appendChild(descriptionTextDiv);
+ cells[descriptionIndex].classList.add('moveHeader');
+ cells[descriptionIndex].appendChild(moveRangeHeader);
+ cells.forEach(c => {
+ controls.appendChild(c);
+ });
+ return controls;
+ }
+
+ /**
+ * Create a blame cell for the given base line. Blame information will be
+ * included in the cell if available.
+ */
+ protected createBlameCell(lineNumber: LineNumber): HTMLTableCellElement {
+ const blameTd = createElementDiff('td', 'blame') as HTMLTableCellElement;
+ blameTd.setAttribute('data-line-number', lineNumber.toString());
+ if (!lineNumber) return blameTd;
+
+ const blameInfo = this.getBlameCommitForBaseLine(lineNumber);
+ if (!blameInfo) return blameTd;
+
+ blameTd.appendChild(createBlameElement(lineNumber, blameInfo));
+ return blameTd;
+ }
+}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
index 5008d10..873e6b7 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
@@ -14,8 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-import {GrDiffBuilder} from './gr-diff-builder';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffLine, LineNumber} from '../gr-diff/gr-diff-line';
@@ -23,8 +21,9 @@
import {DiffLayer} from '../../../types/types';
import {RenderPreferences} from '../../../api/diff';
import {createElementDiff} from '../gr-diff/gr-diff-utils';
+import {GrDiffBuilderLegacy} from './gr-diff-builder-legacy';
-export class GrDiffBuilderSideBySide extends GrDiffBuilder {
+export class GrDiffBuilderSideBySide extends GrDiffBuilderLegacy {
constructor(
diff: DiffInfo,
prefs: DiffPreferencesInfo,
@@ -35,7 +34,7 @@
super(diff, prefs, outputEl, layers, renderPrefs);
}
- _getMoveControlsConfig() {
+ protected override getMoveControlsConfig() {
return {
numberOfCells: 4,
movedOutIndex: 1,
@@ -44,10 +43,10 @@
};
}
- buildSectionElement(group: GrDiffGroup) {
+ protected override buildSectionElement(group: GrDiffGroup) {
const sectionEl = createElementDiff('tbody', 'section');
sectionEl.classList.add(group.type);
- if (this._isTotal(group)) {
+ if (this.isTotal(group)) {
sectionEl.classList.add('total');
}
if (group.dueToRebase) {
@@ -55,24 +54,24 @@
}
if (group.moveDetails) {
sectionEl.classList.add('dueToMove');
- sectionEl.appendChild(this._buildMoveControls(group));
+ sectionEl.appendChild(this.buildMoveControls(group));
}
if (group.ignoredWhitespaceOnly) {
sectionEl.classList.add('ignoredWhitespaceOnly');
}
if (group.type === GrDiffGroupType.CONTEXT_CONTROL) {
- this._createContextControls(sectionEl, group, DiffViewMode.SIDE_BY_SIDE);
+ this.createContextControls(sectionEl, group, DiffViewMode.SIDE_BY_SIDE);
return sectionEl;
}
const pairs = group.getSideBySidePairs();
for (let i = 0; i < pairs.length; i++) {
- sectionEl.appendChild(this._createRow(pairs[i].left, pairs[i].right));
+ sectionEl.appendChild(this.createRow(pairs[i].left, pairs[i].right));
}
return sectionEl;
}
- addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
+ override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
const colgroup = document.createElement('colgroup');
// Add the blame column.
@@ -98,7 +97,7 @@
outputEl.appendChild(colgroup);
}
- _createRow(leftLine: GrDiffLine, rightLine: GrDiffLine) {
+ private createRow(leftLine: GrDiffLine, rightLine: GrDiffLine) {
const row = createElementDiff('tr');
row.classList.add('diff-row', 'side-by-side');
row.setAttribute('left-type', leftLine.type);
@@ -106,25 +105,28 @@
// TabIndex makes screen reader read a row when navigating with j/k
row.tabIndex = -1;
- row.appendChild(this._createBlameCell(leftLine.beforeNumber));
+ row.appendChild(this.createBlameCell(leftLine.beforeNumber));
- this._appendPair(row, leftLine, leftLine.beforeNumber, Side.LEFT);
- this._appendPair(row, rightLine, rightLine.afterNumber, Side.RIGHT);
+ this.appendPair(row, leftLine, leftLine.beforeNumber, Side.LEFT);
+ this.appendPair(row, rightLine, rightLine.afterNumber, Side.RIGHT);
return row;
}
- _appendPair(
+ private appendPair(
row: HTMLElement,
line: GrDiffLine,
lineNumber: LineNumber,
side: Side
) {
- const lineNumberEl = this._createLineEl(line, lineNumber, line.type, side);
+ const lineNumberEl = this.createLineEl(line, lineNumber, line.type, side);
row.appendChild(lineNumberEl);
- row.appendChild(this._createTextEl(lineNumberEl, line, side));
+ row.appendChild(this.createTextEl(lineNumberEl, line, side));
}
- _getNextContentOnSide(content: HTMLElement, side: Side): HTMLElement | null {
+ protected override getNextContentOnSide(
+ content: HTMLElement,
+ side: Side
+ ): HTMLElement | null {
let tr: HTMLElement = content.parentElement!.parentElement!;
while ((tr = tr.nextSibling as HTMLElement)) {
const nextContent = tr.querySelector(
@@ -134,6 +136,4 @@
}
return null;
}
-
- override updateRenderPrefs(_renderPrefs: RenderPreferences) {}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
index 59b9c5f3..3494586 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
@@ -15,15 +15,15 @@
* limitations under the License.
*/
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
-import {GrDiffBuilder} from './gr-diff-builder';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {DiffViewMode, Side} from '../../../constants/constants';
import {DiffLayer} from '../../../types/types';
import {RenderPreferences} from '../../../api/diff';
import {createElementDiff} from '../gr-diff/gr-diff-utils';
+import {GrDiffBuilderLegacy} from './gr-diff-builder-legacy';
-export class GrDiffBuilderUnified extends GrDiffBuilder {
+export class GrDiffBuilderUnified extends GrDiffBuilderLegacy {
constructor(
diff: DiffInfo,
prefs: DiffPreferencesInfo,
@@ -34,7 +34,7 @@
super(diff, prefs, outputEl, layers, renderPrefs);
}
- _getMoveControlsConfig() {
+ protected override getMoveControlsConfig() {
return {
numberOfCells: 3,
movedOutIndex: 2,
@@ -43,10 +43,10 @@
};
}
- buildSectionElement(group: GrDiffGroup): HTMLElement {
+ protected override buildSectionElement(group: GrDiffGroup): HTMLElement {
const sectionEl = createElementDiff('tbody', 'section');
sectionEl.classList.add(group.type);
- if (this._isTotal(group)) {
+ if (this.isTotal(group)) {
sectionEl.classList.add('total');
}
if (group.dueToRebase) {
@@ -54,13 +54,13 @@
}
if (group.moveDetails) {
sectionEl.classList.add('dueToMove');
- sectionEl.appendChild(this._buildMoveControls(group));
+ sectionEl.appendChild(this.buildMoveControls(group));
}
if (group.ignoredWhitespaceOnly) {
sectionEl.classList.add('ignoredWhitespaceOnly');
}
if (group.type === GrDiffGroupType.CONTEXT_CONTROL) {
- this._createContextControls(sectionEl, group, DiffViewMode.UNIFIED);
+ this.createContextControls(sectionEl, group, DiffViewMode.UNIFIED);
return sectionEl;
}
@@ -71,12 +71,12 @@
if (group.ignoredWhitespaceOnly && line.type === GrDiffLineType.REMOVE) {
continue;
}
- sectionEl.appendChild(this._createRow(line));
+ sectionEl.appendChild(this.createRow(line));
}
return sectionEl;
}
- addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
+ override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
const colgroup = document.createElement('colgroup');
// Add the blame column.
@@ -99,20 +99,20 @@
outputEl.appendChild(colgroup);
}
- _createRow(line: GrDiffLine) {
+ protected createRow(line: GrDiffLine) {
const row = createElementDiff('tr', line.type);
row.classList.add('diff-row', 'unified');
// TabIndex makes screen reader read a row when navigating with j/k
row.tabIndex = -1;
- row.appendChild(this._createBlameCell(line.beforeNumber));
- let lineNumberEl = this._createLineEl(
+ row.appendChild(this.createBlameCell(line.beforeNumber));
+ let lineNumberEl = this.createLineEl(
line,
line.beforeNumber,
GrDiffLineType.REMOVE,
Side.LEFT
);
row.appendChild(lineNumberEl);
- lineNumberEl = this._createLineEl(
+ lineNumberEl = this.createLineEl(
line,
line.afterNumber,
GrDiffLineType.ADD,
@@ -126,11 +126,11 @@
if (line.type === GrDiffLineType.REMOVE) {
side = Side.LEFT;
}
- row.appendChild(this._createTextEl(lineNumberEl, line, side));
+ row.appendChild(this.createTextEl(lineNumberEl, line, side));
return row;
}
- _getNextContentOnSide(content: HTMLElement, side: Side): HTMLElement | null {
+ getNextContentOnSide(content: HTMLElement, side: Side): HTMLElement | null {
let tr: HTMLElement = content.parentElement!.parentElement!;
while ((tr = tr.nextSibling as HTMLElement)) {
if (
@@ -143,6 +143,4 @@
}
return null;
}
-
- override updateRenderPrefs(_renderPrefs: RenderPreferences) {}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
index ac109e3..b255c68 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
@@ -17,53 +17,16 @@
import {
ContentLoadNeededEventDetail,
DiffContextExpandedExternalDetail,
- MovedLinkClickedEventDetail,
RenderPreferences,
} from '../../../api/diff';
-import {getBaseUrl} from '../../../utils/url-util';
-import {fire} from '../../../utils/event-util';
import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
import '../gr-context-controls/gr-context-controls';
-import {
- GrContextControls,
- GrContextControlsShowConfig,
-} from '../gr-context-controls/gr-context-controls';
import {BlameInfo} from '../../../types/common';
-import {
- DiffInfo,
- DiffPreferencesInfo,
- DiffResponsiveMode,
-} from '../../../types/diff';
-import {DiffViewMode, Side} from '../../../constants/constants';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
+import {Side} from '../../../constants/constants';
import {DiffLayer} from '../../../types/types';
-import {
- createElementDiff,
- createElementDiffWithText,
-} from '../gr-diff/gr-diff-utils';
-
-/**
- * In JS, unicode code points above 0xFFFF occupy two elements of a string.
- * For example '๐'.length is 2. An occurrence of such a code point is called a
- * surrogate pair.
- *
- * This regex segments a string along tabs ('\t') and surrogate pairs, since
- * these are two cases where '1 char' does not automatically imply '1 column'.
- *
- * TODO: For human languages whose orthographies use combining marks, this
- * approach won't correctly identify the grapheme boundaries. In those cases,
- * a grapheme consists of multiple code points that should count as only one
- * character against the column limit. Getting that correct (if it's desired)
- * is probably beyond the limits of a regex, but there are nonstandard APIs to
- * do this, and proposed (but, as of Nov 2017, unimplemented) standard APIs.
- *
- * Further reading:
- * On Unicode in JS: https://mathiasbynens.be/notes/javascript-unicode
- * Graphemes: http://unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries
- * A proposed JS API: https://github.com/tc39/proposal-intl-segmenter
- */
-const REGEX_TAB_OR_SURROGATE_PAIR = /\t|[\uD800-\uDBFF][\uDC00-\uDFFF]/;
export interface DiffContextExpandedEventDetail
extends DiffContextExpandedExternalDetail {
@@ -79,26 +42,6 @@
}
}
-export function getResponsiveMode(
- prefs: DiffPreferencesInfo,
- renderPrefs?: RenderPreferences
-): DiffResponsiveMode {
- if (renderPrefs?.responsive_mode) {
- return renderPrefs.responsive_mode;
- }
- // Backwards compatibility to the line_wrapping param.
- if (prefs.line_wrapping) {
- return 'FULL_RESPONSIVE';
- }
- return 'NONE';
-}
-
-export function isResponsive(responsiveMode: DiffResponsiveMode) {
- return (
- responsiveMode === 'FULL_RESPONSIVE' || responsiveMode === 'SHRINK_ONLY'
- );
-}
-
/**
* Given that GrDiffBuilder has ~1,000 lines of code, this interface is just
* making refactorings easier by emphasizing what the public facing "contract"
@@ -113,13 +56,14 @@
): GrDiffGroup[];
getIndexOfSection(sectionEl: HTMLElement): number;
addColumns(outputEl: HTMLElement, fontSize: number): void;
+ // TODO: Change `null` to `undefined`.
getContentTdByLine(
lineNumber: LineNumber,
side?: Side,
root?: Element
): Element | null;
- setBlame(blame: BlameInfo[] | null): void;
- updateRenderPrefs(_renderPrefs: RenderPreferences): void;
+ setBlame(blame: BlameInfo[]): void;
+ updateRenderPrefs(renderPrefs: RenderPreferences): void;
}
/**
@@ -130,23 +74,25 @@
* DOM. Callers can use the spliceGroups method to add groups that
* will then be rendered - or remove groups whose sections will then be
* removed from the DOM.
+ *
+ * TODO: Do not subclass `GrDiffBuilder`. Use composition and interfaces.
*/
export abstract class GrDiffBuilder implements DiffBuilder {
- private readonly _diff: DiffInfo;
+ protected readonly _diff: DiffInfo;
- private readonly _numLinesLeft: number;
+ protected readonly numLinesLeft: number;
- private readonly _prefs: DiffPreferencesInfo;
+ protected readonly _prefs: DiffPreferencesInfo;
- protected readonly _renderPrefs?: RenderPreferences;
+ protected readonly renderPrefs?: RenderPreferences;
- protected readonly _outputEl: HTMLElement;
+ protected readonly outputEl: HTMLElement;
protected readonly groups: GrDiffGroup[];
- private blameInfo: BlameInfo[] | null;
+ private blameInfo: BlameInfo[] = [];
- private readonly _layerUpdateListener: (
+ private readonly layerUpdateListener: (
start: LineNumber,
end: LineNumber,
side: Side
@@ -160,17 +106,16 @@
renderPrefs?: RenderPreferences
) {
this._diff = diff;
- this._numLinesLeft = this._diff.content
+ this.numLinesLeft = this._diff.content
? this._diff.content.reduce((sum, chunk) => {
const left = chunk.a || chunk.ab;
return sum + (left?.length || chunk.skip || 0);
}, 0)
: 0;
this._prefs = prefs;
- this._renderPrefs = renderPrefs;
- this._outputEl = outputEl;
+ this.renderPrefs = renderPrefs;
+ this.outputEl = outputEl;
this.groups = [];
- this.blameInfo = null;
if (isNaN(prefs.tab_size) || prefs.tab_size <= 0) {
throw Error('Invalid tab size from preferences.');
@@ -180,14 +125,14 @@
throw Error('Invalid line length from preferences.');
}
- this._layerUpdateListener = (
+ this.layerUpdateListener = (
start: LineNumber,
end: LineNumber,
side: Side
- ) => this._handleLayerUpdate(start, end, side);
+ ) => this.renderContentByRange(start, end, side);
for (const layer of this.layers) {
if (layer.addListener) {
- layer.addListener(this._layerUpdateListener);
+ layer.addListener(this.layerUpdateListener);
}
}
}
@@ -195,24 +140,11 @@
clear() {
for (const layer of this.layers) {
if (layer.removeListener) {
- layer.removeListener(this._layerUpdateListener);
+ layer.removeListener(this.layerUpdateListener);
}
}
}
- // TODO(TS): Convert to enum.
- static readonly GroupType = {
- ADDED: 'b',
- BOTH: 'ab',
- REMOVED: 'a',
- };
-
- // TODO(TS): Convert to enum.
- static readonly Highlights = {
- ADDED: 'edit_b',
- REMOVED: 'edit_a',
- };
-
abstract addColumns(outputEl: HTMLElement, fontSize: number): void;
protected abstract buildSectionElement(group: GrDiffGroup): HTMLElement;
@@ -226,6 +158,7 @@
deleteCount: number,
...addedGroups: GrDiffGroup[]
) {
+ // TODO: Change `null` to `undefined`.
const sectionBeforeWhichToInsert =
start < this.groups.length ? this.groups[start].element ?? null : null;
// Update the groups array
@@ -247,9 +180,10 @@
return deletedGroups;
}
+ // TODO: Change `null` to `undefined`.
private emitGroup(group: GrDiffGroup, beforeSection: HTMLElement | null) {
const element = this.buildSectionElement(group);
- this._outputEl.insertBefore(element, beforeSection);
+ this.outputEl.insertBefore(element, beforeSection);
group.element = element;
}
@@ -271,25 +205,21 @@
.filter(group => group.lines.length > 0);
}
- getContentTdByLine(
+ // TODO: Change `null` to `undefined`.
+ abstract getContentTdByLine(
lineNumber: LineNumber,
side?: Side,
- root: Element = this._outputEl
- ): Element | null {
- const sideSelector: string = side ? `.${side}` : '';
- return root.querySelector(
- `td.lineNum[data-value="${lineNumber}"]${sideSelector} ~ td.content`
- );
- }
+ root?: Element
+ ): Element | null;
- getContentByLine(
+ protected abstract getBlameTdByLine(lineNum: number): Element | undefined;
+
+ // TODO: Change `null` to `undefined`.
+ protected abstract getContentByLine(
lineNumber: LineNumber,
side?: Side,
root?: HTMLElement
- ): HTMLElement | null {
- const td = this.getContentTdByLine(lineNumber, side, root);
- return td ? td.querySelector('.contentText') : null;
- }
+ ): HTMLElement | null;
/**
* Find line elements or line objects by a range of line numbers and a side.
@@ -298,10 +228,14 @@
* @param end The last line number
* @param side The side of the range. Either 'left' or 'right'.
* @param out_lines The output list of line objects. Use null if not desired.
+ * TODO: Change `null` to `undefined` in paramete type. Also: Do we
+ * really need to support null/undefined? Also change to camelCase.
* @param out_elements The output list of line elements. Use null if not
* desired.
+ * TODO: Change `null` to `undefined` in paramete type. Also: Do we
+ * really need to support null/undefined? Also change to camelCase.
*/
- findLinesByRange(
+ protected findLinesByRange(
start: LineNumber,
end: LineNumber,
side: Side,
@@ -329,7 +263,7 @@
}
if (out_elements) {
if (content) {
- content = this._getNextContentOnSide(content, side);
+ content = this.getNextContentOnSide(content, side);
} else {
content = this.getContentByLine(lineNumber, side, group.element);
}
@@ -341,359 +275,25 @@
}
}
- /**
- * Re-renders the DIV.contentText elements for the given side and range of
- * diff content.
- */
- _renderContentByRange(start: LineNumber, end: LineNumber, side: Side) {
- const lines: GrDiffLine[] = [];
- const elements: HTMLElement[] = [];
- let line;
- let el;
- this.findLinesByRange(start, end, side, lines, elements);
- for (let i = 0; i < lines.length; i++) {
- line = lines[i];
- el = elements[i];
- if (!el || !el.parentElement) {
- // Cannot re-render an element if it does not exist. This can happen
- // if lines are collapsed and not visible on the page yet.
- continue;
- }
- const lineNumberEl = this._getLineNumberEl(el, side);
- el.parentElement.replaceChild(
- this._createTextEl(lineNumberEl, line, side).firstChild!,
- el
- );
- }
- }
-
- _createContextControls(
- section: HTMLElement,
- group: GrDiffGroup,
- viewMode: DiffViewMode
- ) {
- const leftStart = group.lineRange.left.start_line;
- const leftEnd = group.lineRange.left.end_line;
- const firstGroupIsSkipped = !!group.contextGroups[0].skip;
- const lastGroupIsSkipped =
- !!group.contextGroups[group.contextGroups.length - 1].skip;
-
- const containsWholeFile = this._numLinesLeft === leftEnd - leftStart + 1;
- const showAbove =
- (leftStart > 1 && !firstGroupIsSkipped) || containsWholeFile;
- const showBelow = leftEnd < this._numLinesLeft && !lastGroupIsSkipped;
-
- if (showAbove) {
- const paddingRow = this._createContextControlPaddingRow(viewMode);
- paddingRow.classList.add('above');
- section.appendChild(paddingRow);
- }
- section.appendChild(
- this._createContextControlRow(
- section,
- group,
- showAbove,
- showBelow,
- viewMode
- )
- );
- if (showBelow) {
- const paddingRow = this._createContextControlPaddingRow(viewMode);
- paddingRow.classList.add('below');
- section.appendChild(paddingRow);
- }
- }
-
- /**
- * Creates context controls. Buttons extend from the gap created by this
- * method up or down into the area of code that they affect.
- */
- _createContextControlRow(
- section: HTMLElement,
- group: GrDiffGroup,
- showAbove: boolean,
- showBelow: boolean,
- viewMode: DiffViewMode
- ): HTMLElement {
- const row = createElementDiff('tr', 'dividerRow');
- let showConfig: GrContextControlsShowConfig;
- if (showAbove && !showBelow) {
- showConfig = 'above';
- } else if (!showAbove && showBelow) {
- showConfig = 'below';
- } else {
- // Note that !showAbove && !showBelow also intentionally creates
- // "show-both". This means the file is completely collapsed, which is
- // unusual, but at least happens in one test.
- showConfig = 'both';
- }
- row.classList.add(`show-${showConfig}`);
-
- row.appendChild(this._createBlameCell(0));
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.appendChild(createElementDiff('td'));
- }
-
- const cell = createElementDiff('td', 'dividerCell');
- cell.setAttribute('colspan', '3');
- row.appendChild(cell);
-
- const contextControls = createElementDiff(
- 'gr-context-controls'
- ) as GrContextControls;
- contextControls.diff = this._diff;
- contextControls.renderPreferences = this._renderPrefs;
- contextControls.section = section;
- contextControls.group = group;
- contextControls.showConfig = showConfig;
- cell.appendChild(contextControls);
- return row;
- }
-
- /**
- * Creates a table row to serve as padding between code and context controls.
- * Blame column, line gutters, and content area will continue visually, but
- * context controls can render over this background to map more clearly to
- * the area of code they expand.
- */
- _createContextControlPaddingRow(viewMode: DiffViewMode) {
- const row = createElementDiff('tr', 'contextBackground');
-
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.classList.add('side-by-side');
- row.setAttribute('left-type', GrDiffGroupType.CONTEXT_CONTROL);
- row.setAttribute('right-type', GrDiffGroupType.CONTEXT_CONTROL);
- } else {
- row.classList.add('unified');
- }
-
- row.appendChild(this._createBlameCell(0));
- row.appendChild(createElementDiff('td', 'contextLineNum'));
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.appendChild(createElementDiff('td'));
- }
- row.appendChild(createElementDiff('td', 'contextLineNum'));
- row.appendChild(createElementDiff('td'));
-
- return row;
- }
-
- _createLineEl(
- line: GrDiffLine,
- number: LineNumber,
- type: GrDiffLineType,
+ protected abstract renderContentByRange(
+ start: LineNumber,
+ end: LineNumber,
side: Side
- ) {
- const td = createElementDiff('td');
- td.classList.add(side);
- if (line.type === GrDiffLineType.BLANK) {
- return td;
- }
- if (line.type === GrDiffLineType.BOTH || line.type === type) {
- td.classList.add('lineNum');
- td.dataset['value'] = number.toString();
+ ): void;
- if (
- ((this._prefs.show_file_comment_button === false ||
- this._renderPrefs?.show_file_comment_button === false) &&
- number === 'FILE') ||
- number === 'LOST'
- ) {
- return td;
- }
-
- const button = createElementDiff('button');
- td.appendChild(button);
- button.tabIndex = -1;
- button.classList.add('lineNumButton');
- button.classList.add(side);
- button.dataset['value'] = number.toString();
- button.textContent = number === 'FILE' ? 'File' : number.toString();
- if (number === 'FILE') {
- button.setAttribute('aria-label', 'Add file comment');
- }
-
- // Add aria-labels for valid line numbers.
- // For unified diff, this method will be called with number set to 0 for
- // the empty line number column for added/removed lines. This should not
- // be announced to the screenreader.
- if (number > 0) {
- if (line.type === GrDiffLineType.REMOVE) {
- button.setAttribute('aria-label', `${number} removed`);
- } else if (line.type === GrDiffLineType.ADD) {
- button.setAttribute('aria-label', `${number} added`);
- }
- }
- this._addLineNumberMouseEvents(td, number, side);
- }
- return td;
- }
-
- _addLineNumberMouseEvents(el: HTMLElement, number: LineNumber, side: Side) {
- el.addEventListener('mouseenter', () => {
- fire(el, 'line-mouse-enter', {lineNum: number, side});
- });
- el.addEventListener('mouseleave', () => {
- fire(el, 'line-mouse-leave', {lineNum: number, side});
- });
- }
-
- _createTextEl(
- lineNumberEl: HTMLElement | null,
- line: GrDiffLine,
- side?: Side
- ) {
- const td = createElementDiff('td');
- if (line.type !== GrDiffLineType.BLANK) {
- td.classList.add('content');
- }
-
- // If intraline info is not available, the entire line will be
- // considered as changed and marked as dark red / green color
- if (!line.hasIntralineInfo) {
- td.classList.add('no-intraline-info');
- }
- td.classList.add(line.type);
-
- const {beforeNumber, afterNumber} = line;
- if (beforeNumber !== 'FILE' && beforeNumber !== 'LOST') {
- const responsiveMode = getResponsiveMode(this._prefs, this._renderPrefs);
- const contentText = this._formatText(
- line.text,
- responsiveMode,
- this._prefs.tab_size,
- this._prefs.line_length
- );
-
- if (side) {
- contentText.setAttribute('data-side', side);
- const number = side === Side.LEFT ? beforeNumber : afterNumber;
- this._addLineNumberMouseEvents(td, number, side);
- }
-
- if (lineNumberEl && side) {
- for (const layer of this.layers) {
- if (typeof layer.annotate === 'function') {
- layer.annotate(contentText, lineNumberEl, line, side);
- }
- }
- } else {
- console.error('lineNumberEl or side not set, skipping layer.annotate');
- }
-
- td.appendChild(contentText);
- } else if (line.beforeNumber === 'FILE') td.classList.add('file');
- else if (line.beforeNumber === 'LOST') td.classList.add('lost');
-
- return td;
- }
-
- private createLineBreak(responsive: boolean) {
- return responsive
- ? createElementDiff('wbr')
- : createElementDiff('span', 'br');
- }
-
- /**
- * Returns a 'div' element containing the supplied |text| as its innerText,
- * with '\t' characters expanded to a width determined by |tabSize|, and the
- * text wrapped at column |lineLimit|, which may be Infinity if no wrapping is
- * desired.
- *
- * @param text The text to be formatted.
- * @param tabSize The width of each tab stop.
- * @param lineLimit The column after which to wrap lines.
- */
- _formatText(
- text: string,
- responsiveMode: DiffResponsiveMode,
- tabSize: number,
- lineLimit: number
- ): HTMLElement {
- const contentText = createElementDiff('div', 'contentText');
- contentText.ariaLabel = text;
- const responsive = isResponsive(responsiveMode);
- let columnPos = 0;
- let textOffset = 0;
- for (const segment of text.split(REGEX_TAB_OR_SURROGATE_PAIR)) {
- if (segment) {
- // |segment| contains only normal characters. If |segment| doesn't fit
- // entirely on the current line, append chunks of |segment| followed by
- // line breaks.
- let rowStart = 0;
- let rowEnd = lineLimit - columnPos;
- while (rowEnd < segment.length) {
- contentText.appendChild(
- document.createTextNode(segment.substring(rowStart, rowEnd))
- );
- contentText.appendChild(this.createLineBreak(responsive));
- columnPos = 0;
- rowStart = rowEnd;
- rowEnd += lineLimit;
- }
- // Append the last part of |segment|, which fits on the current line.
- contentText.appendChild(
- document.createTextNode(segment.substring(rowStart))
- );
- columnPos += segment.length - rowStart;
- textOffset += segment.length;
- }
- if (textOffset < text.length) {
- // Handle the special character at |textOffset|.
- if (text.startsWith('\t', textOffset)) {
- // Append a single '\t' character.
- let effectiveTabSize = tabSize - (columnPos % tabSize);
- if (columnPos + effectiveTabSize > lineLimit) {
- contentText.appendChild(this.createLineBreak(responsive));
- columnPos = 0;
- effectiveTabSize = tabSize;
- }
- contentText.appendChild(this._getTabWrapper(effectiveTabSize));
- columnPos += effectiveTabSize;
- textOffset++;
- } else {
- // Append a single surrogate pair.
- if (columnPos >= lineLimit) {
- contentText.appendChild(this.createLineBreak(responsive));
- columnPos = 0;
- }
- contentText.appendChild(
- document.createTextNode(text.substring(textOffset, textOffset + 2))
- );
- textOffset += 2;
- columnPos += 1;
- }
- }
- }
- return contentText;
- }
-
- /**
- * Returns a <span> element holding a '\t' character, that will visually
- * occupy |tabSize| many columns.
- *
- * @param tabSize The effective size of this tab stop.
- */
- _getTabWrapper(tabSize: number): HTMLElement {
- // Force this to be a number to prevent arbitrary injection.
- const result = createElementDiff('span', 'tab');
- result.setAttribute(
- 'style',
- `tab-size: ${tabSize}; -moz-tab-size: ${tabSize};`
- );
- result.innerText = '\t';
- return result;
- }
-
- _handleLayerUpdate(start: LineNumber, end: LineNumber, side: Side) {
- this._renderContentByRange(start, end, side);
- }
+ protected abstract renderBlameByRange(
+ blame: BlameInfo,
+ start: number,
+ end: number
+ ): void;
/**
* Finds the next DIV.contentText element following the given element, and on
* the same side. Will only search within a group.
+ *
+ * TODO: Change `null` to `undefined`.
*/
- abstract _getNextContentOnSide(
+ protected abstract getNextContentOnSide(
content: HTMLElement,
side: Side
): HTMLElement | null;
@@ -702,7 +302,7 @@
* Gets configuration for creating move controls for chunks marked with
* dueToMove
*/
- abstract _getMoveControlsConfig(): {
+ protected abstract getMoveControlsConfig(): {
numberOfCells: number;
movedOutIndex: number;
movedInIndex: number;
@@ -713,7 +313,7 @@
* Determines whether the given group is either totally an addition or totally
* a removal.
*/
- _isTotal(group: GrDiffGroup): boolean {
+ protected isTotal(group: GrDiffGroup): boolean {
return (
group.type === GrDiffGroupType.DELTA &&
(!group.adds.length || !group.removes.length) &&
@@ -725,117 +325,15 @@
* Set the blame information for the diff. For any already-rendered line,
* re-render its blame cell content.
*/
- setBlame(blame: BlameInfo[] | null) {
+ setBlame(blame: BlameInfo[]) {
this.blameInfo = blame;
- if (!blame) return;
-
- // TODO(wyatta): make this loop asynchronous.
for (const commit of blame) {
for (const range of commit.ranges) {
- for (let i = range.start; i <= range.end; i++) {
- // TODO(wyatta): this query is expensive, but, when traversing a
- // range, the lines are consecutive, and given the previous blame
- // cell, the next one can be reached cheaply.
- const el = this._getBlameByLineNum(i);
- if (!el) {
- continue;
- }
- // Remove the element's children (if any).
- while (el.hasChildNodes()) {
- el.removeChild(el.lastChild!);
- }
- const blame = this._getBlameForBaseLine(i, commit);
- if (blame) el.appendChild(blame);
- }
+ this.renderBlameByRange(commit, range.start, range.end);
}
}
}
- _createMovedLineAnchor(line: number, side: Side) {
- const anchor = createElementDiffWithText('a', `${line}`);
-
- // href is not actually used but important for Screen Readers
- anchor.setAttribute('href', `#${line}`);
- anchor.addEventListener('click', e => {
- e.preventDefault();
- anchor.dispatchEvent(
- new CustomEvent<MovedLinkClickedEventDetail>('moved-link-clicked', {
- detail: {
- lineNum: line,
- side,
- },
- composed: true,
- bubbles: true,
- })
- );
- });
- return anchor;
- }
-
- _createMoveDescriptionDiv(movedIn: boolean, group: GrDiffGroup) {
- const div = createElementDiff('div');
- if (group.moveDetails?.range) {
- const {changed, range} = group.moveDetails;
- const otherSide = movedIn ? Side.LEFT : Side.RIGHT;
- const andChangedLabel = changed ? 'and changed ' : '';
- const direction = movedIn ? 'from' : 'to';
- const textLabel = `Moved ${andChangedLabel}${direction} lines `;
- div.appendChild(createElementDiffWithText('span', textLabel));
- div.appendChild(this._createMovedLineAnchor(range.start, otherSide));
- div.appendChild(createElementDiffWithText('span', ' - '));
- div.appendChild(this._createMovedLineAnchor(range.end, otherSide));
- } else {
- div.appendChild(
- createElementDiffWithText('span', movedIn ? 'Moved in' : 'Moved out')
- );
- }
- return div;
- }
-
- _buildMoveControls(group: GrDiffGroup) {
- const movedIn = group.adds.length > 0;
- const {numberOfCells, movedOutIndex, movedInIndex, lineNumberCols} =
- this._getMoveControlsConfig();
-
- let controlsClass;
- let descriptionIndex;
- const descriptionTextDiv = this._createMoveDescriptionDiv(movedIn, group);
- if (movedIn) {
- controlsClass = 'movedIn';
- descriptionIndex = movedInIndex;
- } else {
- controlsClass = 'movedOut';
- descriptionIndex = movedOutIndex;
- }
-
- const controls = createElementDiff('tr', `moveControls ${controlsClass}`);
- const cells = [...Array(numberOfCells).keys()].map(() =>
- createElementDiff('td')
- );
- lineNumberCols.forEach(index => {
- cells[index].classList.add('moveControlsLineNumCol');
- });
-
- const moveRangeHeader = createElementDiff('gr-range-header');
- moveRangeHeader.setAttribute('icon', 'gr-icons:move-item');
- moveRangeHeader.appendChild(descriptionTextDiv);
- cells[descriptionIndex].classList.add('moveHeader');
- cells[descriptionIndex].appendChild(moveRangeHeader);
- cells.forEach(c => {
- controls.appendChild(c);
- });
- return controls;
- }
-
- /**
- * Find the blame cell for a given line number.
- */
- _getBlameByLineNum(lineNum: number): Element | null {
- return this._outputEl.querySelector(
- `td.blame[data-line-number="${lineNum}"]`
- );
- }
-
/**
* Given a base line number, return the commit containing that line in the
* current set of blame information. If no blame information has been
@@ -843,11 +341,9 @@
*
* @return The commit information.
*/
- _getBlameCommitForBaseLine(lineNum: LineNumber) {
- if (!this.blameInfo) {
- return null;
- }
-
+ protected getBlameCommitForBaseLine(
+ lineNum: LineNumber
+ ): BlameInfo | undefined {
for (const blameCommit of this.blameInfo) {
for (const range of blameCommit.ranges) {
if (range.start <= lineNum && range.end >= lineNum) {
@@ -855,86 +351,12 @@
}
}
}
- return null;
+ return undefined;
}
/**
- * Given the number of a base line, get the content for the blame cell of that
- * line. If there is no blame information for that line, returns null.
- *
- * @param commit Optionally provide the commit object, so that
- * it does not need to be searched.
+ * Only special builders need to implement this. The default is to
+ * just ignore it.
*/
- _getBlameForBaseLine(
- lineNum: LineNumber,
- commit: BlameInfo | null = this._getBlameCommitForBaseLine(lineNum)
- ): HTMLElement | null {
- if (!commit) {
- return null;
- }
-
- const isStartOfRange = commit.ranges.some(r => r.start === lineNum);
-
- const date = new Date(commit.time * 1000).toLocaleDateString();
- const blameNode = createElementDiff(
- 'span',
- isStartOfRange ? 'startOfRange' : ''
- );
-
- const shaNode = createElementDiff('a', 'blameDate');
- shaNode.innerText = `${date}`;
- shaNode.setAttribute('href', `${getBaseUrl()}/q/${commit.id}`);
- blameNode.appendChild(shaNode);
-
- const shortName = commit.author.split(' ')[0];
- const authorNode = createElementDiff('span', 'blameAuthor');
- authorNode.innerText = ` ${shortName}`;
- blameNode.appendChild(authorNode);
-
- const hoverCardFragment = createElementDiff('span', 'blameHoverCard');
- hoverCardFragment.innerText = `Commit ${commit.id}
-Author: ${commit.author}
-Date: ${date}
-
-${commit.commit_msg}`;
- const hovercard = createElementDiff('gr-hovercard');
- hovercard.appendChild(hoverCardFragment);
- blameNode.appendChild(hovercard);
-
- return blameNode;
- }
-
- /**
- * Create a blame cell for the given base line. Blame information will be
- * included in the cell if available.
- */
- _createBlameCell(lineNumber: LineNumber): HTMLTableDataCellElement {
- const blameTd = createElementDiff(
- 'td',
- 'blame'
- ) as HTMLTableDataCellElement;
- blameTd.setAttribute('data-line-number', lineNumber.toString());
- if (lineNumber) {
- const content = this._getBlameForBaseLine(lineNumber);
- if (content) {
- blameTd.appendChild(content);
- }
- }
- return blameTd;
- }
-
- /**
- * Finds the line number element given the content element by walking up the
- * DOM tree to the diff row and then querying for a .lineNum element on the
- * requested side.
- *
- * TODO(brohlfs): Consolidate this with getLineEl... methods in html file.
- */
- _getLineNumberEl(content: HTMLElement, side: Side): HTMLElement | null {
- let row: HTMLElement | null = content;
- while (row && !row.classList.contains('diff-row')) row = row.parentElement;
- return row ? (row.querySelector('.lineNum.' + side) as HTMLElement) : null;
- }
-
- updateRenderPrefs(_renderPrefs: RenderPreferences) {}
+ updateRenderPrefs(_: RenderPreferences) {}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
index 633b5f6..94a7a84 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils.ts
@@ -14,16 +14,63 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-import {CommentRange} from '../../../types/common';
+import {BlameInfo, CommentRange} from '../../../types/common';
import {FILE, LineNumber} from './gr-diff-line';
import {Side} from '../../../constants/constants';
import {DiffInfo} from '../../../types/diff';
+import {
+ DiffPreferencesInfo,
+ DiffResponsiveMode,
+ RenderPreferences,
+} from '../../../api/diff';
+import {getBaseUrl} from '../../../utils/url-util';
+
+/**
+ * In JS, unicode code points above 0xFFFF occupy two elements of a string.
+ * For example '๐'.length is 2. An occurrence of such a code point is called a
+ * surrogate pair.
+ *
+ * This regex segments a string along tabs ('\t') and surrogate pairs, since
+ * these are two cases where '1 char' does not automatically imply '1 column'.
+ *
+ * TODO: For human languages whose orthographies use combining marks, this
+ * approach won't correctly identify the grapheme boundaries. In those cases,
+ * a grapheme consists of multiple code points that should count as only one
+ * character against the column limit. Getting that correct (if it's desired)
+ * is probably beyond the limits of a regex, but there are nonstandard APIs to
+ * do this, and proposed (but, as of Nov 2017, unimplemented) standard APIs.
+ *
+ * Further reading:
+ * On Unicode in JS: https://mathiasbynens.be/notes/javascript-unicode
+ * Graphemes: http://unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries
+ * A proposed JS API: https://github.com/tc39/proposal-intl-segmenter
+ */
+const REGEX_TAB_OR_SURROGATE_PAIR = /\t|[\uD800-\uDBFF][\uDC00-\uDFFF]/;
// If any line of the diff is more than the character limit, then disable
// syntax highlighting for the entire file.
export const SYNTAX_MAX_LINE_LENGTH = 500;
+export function getResponsiveMode(
+ prefs: DiffPreferencesInfo,
+ renderPrefs?: RenderPreferences
+): DiffResponsiveMode {
+ if (renderPrefs?.responsive_mode) {
+ return renderPrefs.responsive_mode;
+ }
+ // Backwards compatibility to the line_wrapping param.
+ if (prefs.line_wrapping) {
+ return 'FULL_RESPONSIVE';
+ }
+ return 'NONE';
+}
+
+export function isResponsive(responsiveMode: DiffResponsiveMode) {
+ return (
+ responsiveMode === 'FULL_RESPONSIVE' || responsiveMode === 'SHRINK_ONLY'
+ );
+}
+
/**
* Compare two ranges. Either argument may be falsy, but will only return
* true if both are falsy or if neither are falsy and have the same position
@@ -186,3 +233,139 @@
element.textContent = textContent;
return element;
}
+
+export function createLineBreak(mode: DiffResponsiveMode) {
+ return isResponsive(mode)
+ ? createElementDiff('wbr')
+ : createElementDiff('span', 'br');
+}
+
+/**
+ * Returns a <span> element holding a '\t' character, that will visually
+ * occupy |tabSize| many columns.
+ *
+ * @param tabSize The effective size of this tab stop.
+ */
+export function createTabWrapper(tabSize: number): HTMLElement {
+ // Force this to be a number to prevent arbitrary injection.
+ const result = createElementDiff('span', 'tab');
+ result.setAttribute(
+ 'style',
+ `tab-size: ${tabSize}; -moz-tab-size: ${tabSize};`
+ );
+ result.innerText = '\t';
+ return result;
+}
+
+/**
+ * Returns a 'div' element containing the supplied |text| as its innerText,
+ * with '\t' characters expanded to a width determined by |tabSize|, and the
+ * text wrapped at column |lineLimit|, which may be Infinity if no wrapping is
+ * desired.
+ *
+ * @param text The text to be formatted.
+ * @param responsiveMode The responsive mode of the diff.
+ * @param tabSize The width of each tab stop.
+ * @param lineLimit The column after which to wrap lines.
+ */
+export function formatText(
+ text: string,
+ responsiveMode: DiffResponsiveMode,
+ tabSize: number,
+ lineLimit: number
+): HTMLElement {
+ const contentText = createElementDiff('div', 'contentText');
+ contentText.ariaLabel = text;
+ let columnPos = 0;
+ let textOffset = 0;
+ for (const segment of text.split(REGEX_TAB_OR_SURROGATE_PAIR)) {
+ if (segment) {
+ // |segment| contains only normal characters. If |segment| doesn't fit
+ // entirely on the current line, append chunks of |segment| followed by
+ // line breaks.
+ let rowStart = 0;
+ let rowEnd = lineLimit - columnPos;
+ while (rowEnd < segment.length) {
+ contentText.appendChild(
+ document.createTextNode(segment.substring(rowStart, rowEnd))
+ );
+ contentText.appendChild(createLineBreak(responsiveMode));
+ columnPos = 0;
+ rowStart = rowEnd;
+ rowEnd += lineLimit;
+ }
+ // Append the last part of |segment|, which fits on the current line.
+ contentText.appendChild(
+ document.createTextNode(segment.substring(rowStart))
+ );
+ columnPos += segment.length - rowStart;
+ textOffset += segment.length;
+ }
+ if (textOffset < text.length) {
+ // Handle the special character at |textOffset|.
+ if (text.startsWith('\t', textOffset)) {
+ // Append a single '\t' character.
+ let effectiveTabSize = tabSize - (columnPos % tabSize);
+ if (columnPos + effectiveTabSize > lineLimit) {
+ contentText.appendChild(createLineBreak(responsiveMode));
+ columnPos = 0;
+ effectiveTabSize = tabSize;
+ }
+ contentText.appendChild(createTabWrapper(effectiveTabSize));
+ columnPos += effectiveTabSize;
+ textOffset++;
+ } else {
+ // Append a single surrogate pair.
+ if (columnPos >= lineLimit) {
+ contentText.appendChild(createLineBreak(responsiveMode));
+ columnPos = 0;
+ }
+ contentText.appendChild(
+ document.createTextNode(text.substring(textOffset, textOffset + 2))
+ );
+ textOffset += 2;
+ columnPos += 1;
+ }
+ }
+ }
+ return contentText;
+}
+
+/**
+ * Given the number of a base line and the BlameInfo create a <span> element
+ * with a hovercard. This is supposed to be put into a <td> cell of the diff.
+ */
+export function createBlameElement(
+ lineNum: LineNumber,
+ commit: BlameInfo
+): HTMLElement {
+ const isStartOfRange = commit.ranges.some(r => r.start === lineNum);
+
+ const date = new Date(commit.time * 1000).toLocaleDateString();
+ const blameNode = createElementDiff(
+ 'span',
+ isStartOfRange ? 'startOfRange' : ''
+ );
+
+ const shaNode = createElementDiff('a', 'blameDate');
+ shaNode.innerText = `${date}`;
+ shaNode.setAttribute('href', `${getBaseUrl()}/q/${commit.id}`);
+ blameNode.appendChild(shaNode);
+
+ const shortName = commit.author.split(' ')[0];
+ const authorNode = createElementDiff('span', 'blameAuthor');
+ authorNode.innerText = ` ${shortName}`;
+ blameNode.appendChild(authorNode);
+
+ const hoverCardFragment = createElementDiff('span', 'blameHoverCard');
+ hoverCardFragment.innerText = `Commit ${commit.id}
+Author: ${commit.author}
+Date: ${date}
+
+${commit.commit_msg}`;
+ const hovercard = createElementDiff('gr-hovercard');
+ hovercard.appendChild(hoverCardFragment);
+ blameNode.appendChild(hovercard);
+
+ return blameNode;
+}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
index 19ed185..600913e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-utils_test.ts
@@ -4,7 +4,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../../test/common-test-setup-karma';
-import {createElementDiff} from './gr-diff-utils';
+import {createElementDiff, formatText, createTabWrapper} from './gr-diff-utils';
+
+const LINE_BREAK_HTML = '<span class="style-scope gr-diff br"></span>';
suite('gr-diff-utils tests', () => {
test('createElementDiff classStr applies all classes', () => {
@@ -13,4 +15,139 @@
assert.isTrue(node.classList.contains('test'));
assert.isTrue(node.classList.contains('classes'));
});
+
+ test('formatText newlines 1', () => {
+ let text = 'abcdef';
+
+ assert.equal(formatText(text, 'NONE', 4, 10).innerHTML, text);
+ text = 'a'.repeat(20);
+ assert.equal(
+ formatText(text, 'NONE', 4, 10).innerHTML,
+ 'a'.repeat(10) + LINE_BREAK_HTML + 'a'.repeat(10)
+ );
+ });
+
+ test('formatText newlines 2', () => {
+ const text = '<span class="thumbsup">๐</span>';
+ assert.equal(
+ formatText(text, 'NONE', 4, 10).innerHTML,
+ '<span clas' +
+ LINE_BREAK_HTML +
+ 's="thumbsu' +
+ LINE_BREAK_HTML +
+ 'p">๐</span' +
+ LINE_BREAK_HTML +
+ '>'
+ );
+ });
+
+ test('formatText newlines 3', () => {
+ const text = '01234\t56789';
+ assert.equal(
+ formatText(text, 'NONE', 4, 10).innerHTML,
+ '01234' + createTabWrapper(3).outerHTML + '56' + LINE_BREAK_HTML + '789'
+ );
+ });
+
+ test('formatText newlines 4', () => {
+ const text = '๐'.repeat(58);
+ assert.equal(
+ formatText(text, 'NONE', 4, 20).innerHTML,
+ '๐'.repeat(20) +
+ LINE_BREAK_HTML +
+ '๐'.repeat(20) +
+ LINE_BREAK_HTML +
+ '๐'.repeat(18)
+ );
+ });
+
+ test('tab wrapper style', () => {
+ const pattern = new RegExp(
+ '^<span class="style-scope gr-diff tab" ' +
+ 'style="((?:-moz-)?tab-size: (\\d+);.?)+">\\t<\\/span>$'
+ );
+
+ for (const size of [1, 3, 8, 55]) {
+ const html = createTabWrapper(size).outerHTML;
+ expect(html).to.match(pattern);
+ assert.equal(html.match(pattern)?.[2], size.toString());
+ }
+ });
+
+ test('tab wrapper insertion', () => {
+ const html = 'abc\tdef';
+ const tabSize = 8;
+ const wrapper = createTabWrapper(tabSize - 3);
+ assert.ok(wrapper);
+ assert.equal(wrapper.innerText, '\t');
+ assert.equal(
+ formatText(html, 'NONE', tabSize, Infinity).innerHTML,
+ 'abc' + wrapper.outerHTML + 'def'
+ );
+ });
+
+ test('escaping HTML', () => {
+ let input = '<script>alert("XSS");<' + '/script>';
+ let expected = '<script>alert("XSS");</script>';
+
+ let result = formatText(
+ input,
+ 'NONE',
+ 1,
+ Number.POSITIVE_INFINITY
+ ).innerHTML;
+ assert.equal(result, expected);
+
+ input = '& < > " \' / `';
+ expected = '& < > " \' / `';
+ result = formatText(input, 'NONE', 1, Number.POSITIVE_INFINITY).innerHTML;
+ assert.equal(result, expected);
+ });
+
+ test('text length with tabs and unicode', () => {
+ function expectTextLength(text: string, tabSize: number, expected: number) {
+ // Formatting to |expected| columns should not introduce line breaks.
+ const result = formatText(text, 'NONE', tabSize, expected);
+ assert.isNotOk(
+ result.querySelector('.contentText > .br'),
+ ' Expected the result of: \n' +
+ ` _formatText(${text}', 'NONE', ${tabSize}, ${expected})\n` +
+ ' to not contain a br. But the actual result HTML was:\n' +
+ ` '${result.innerHTML}'\nwhereupon`
+ );
+
+ // Increasing the line limit should produce the same markup.
+ assert.equal(
+ formatText(text, 'NONE', tabSize, Infinity).innerHTML,
+ result.innerHTML
+ );
+ assert.equal(
+ formatText(text, 'NONE', tabSize, expected + 1).innerHTML,
+ result.innerHTML
+ );
+
+ // Decreasing the line limit should introduce line breaks.
+ if (expected > 0) {
+ const tooSmall = formatText(text, 'NONE', tabSize, expected - 1);
+ assert.isOk(
+ tooSmall.querySelector('.contentText > .br'),
+ ' Expected the result of: \n' +
+ ` _formatText(${text}', ${tabSize}, ${expected - 1})\n` +
+ ' to contain a br. But the actual result HTML was:\n' +
+ ` '${tooSmall.innerHTML}'\nwhereupon`
+ );
+ }
+ }
+ expectTextLength('12345', 4, 5);
+ expectTextLength('\t\t12', 4, 10);
+ expectTextLength('abc๐ข123', 4, 7);
+ expectTextLength('abc\t', 8, 8);
+ expectTextLength('abc\t\t', 10, 20);
+ expectTextLength('', 10, 0);
+ // 17 Thai combining chars.
+ expectTextLength('เธเนเนเนเนเนเนเนเนเนเนเนเนเนเนเนเน', 4, 17);
+ expectTextLength('abc\tde', 10, 12);
+ expectTextLength('abc\tde\t', 10, 20);
+ expectTextLength('\t\t\t\t\t', 20, 100);
+ });
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 0cabf52..863efca 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -37,6 +37,8 @@
isLongCommentRange,
isThreadEl,
rangesEqual,
+ getResponsiveMode,
+ isResponsive,
} from './gr-diff-utils';
import {getHiddenScroll} from '../../../scripts/hiddenscroll';
import {customElement, observe, property} from '@polymer/decorators';
@@ -83,10 +85,6 @@
import {isSafari, toggleClass} from '../../../utils/dom-util';
import {assertIsDefined} from '../../../utils/common-util';
import {debounce, DelayedTask} from '../../../utils/async-util';
-import {
- getResponsiveMode,
- isResponsive,
-} from '../gr-diff-builder/gr-diff-builder';
const NO_NEWLINE_LEFT = 'No newline at end of left file.';
const NO_NEWLINE_RIGHT = 'No newline at end of right file.';
