Replace diff-builder rendering by plain Lit rendering
This is a vast simplification of how GrDiff is being rendered. There is
no need to explicitly request rendering of the diff. It will just happen
as part of Lit's usual rendering process.
This also means that lots of `init()` and `cleanup()` methods are
obsolete.
With this change the notion of a "diff builder" is gone. Apart from
merging the GrDiffBuilder into GrDiff we are also mergin the image diff
builder and the binary diff builder into GrDiff, see the new
`renderImageDiff()` and `renderBinaryDiff()` methods.
We are adding a `getUpdateComplete` pattern to gr-diff and
gr-diff-section that is already used elsewhere: It allows parents to
`await el.updateComplete` making sure that all children are also
already rendered. This allows us to remove `waitUntilRendered()` from
gr-diff-group.
Release-Notes: skip
Google-Bug-Id: b/280018960
Change-Id: I8dd98ab9ea35dbed6eb05a43071f9cf67594d013
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 33c8c22..ff46351 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -99,6 +99,7 @@
import {subscribe} from '../../lit/subscription-controller';
import {userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {keyed} from 'lit/directives/keyed.js';
const EMPTY_BLAME = 'No blame information for this diff.';
@@ -337,6 +338,12 @@
private checksSubscription?: Subscription;
+ /**
+ * This key is used for the `keyed()` directive when rendering `gr-diff` and
+ * can thus be used to trigger re-construction of `gr-diff`.
+ */
+ private grDiffKey = 0;
+
constructor() {
super();
this.syntaxLayer = new GrSyntaxLayerWorker(
@@ -493,30 +500,33 @@
KnownExperimentId.NEW_IMAGE_DIFF_UI
);
- return html` <gr-diff
- id="diff"
- ?hidden=${this.hidden}
- .noAutoRender=${this.noAutoRender}
- .path=${this.path}
- .prefs=${this.prefs}
- .isImageDiff=${this.isImageDiff}
- .noRenderOnPrefsChange=${this.noRenderOnPrefsChange}
- .renderPrefs=${this.renderPrefs}
- .lineWrapping=${this.lineWrapping}
- .viewMode=${this.viewMode}
- .lineOfInterest=${this.lineOfInterest}
- .loggedIn=${this.loggedIn}
- .errorMessage=${this.errorMessage}
- .baseImage=${this.baseImage}
- .revisionImage=${this.revisionImage}
- .coverageRanges=${this.coverageRanges}
- .blame=${this.blame}
- .layers=${this.layers}
- .diff=${this.diff}
- .showNewlineWarningLeft=${showNewlineWarningLeft}
- .showNewlineWarningRight=${showNewlineWarningRight}
- .useNewImageDiffUi=${useNewImageDiffUi}
- ></gr-diff>`;
+ return keyed(
+ this.grDiffKey,
+ html`<gr-diff
+ id="diff"
+ ?hidden=${this.hidden}
+ .noAutoRender=${this.noAutoRender}
+ .path=${this.path}
+ .prefs=${this.prefs}
+ .isImageDiff=${this.isImageDiff}
+ .noRenderOnPrefsChange=${this.noRenderOnPrefsChange}
+ .renderPrefs=${this.renderPrefs}
+ .lineWrapping=${this.lineWrapping}
+ .viewMode=${this.viewMode}
+ .lineOfInterest=${this.lineOfInterest}
+ .loggedIn=${this.loggedIn}
+ .errorMessage=${this.errorMessage}
+ .baseImage=${this.baseImage}
+ .revisionImage=${this.revisionImage}
+ .coverageRanges=${this.coverageRanges}
+ .blame=${this.blame}
+ .layers=${this.layers}
+ .diff=${this.diff}
+ .showNewlineWarningLeft=${showNewlineWarningLeft}
+ .showNewlineWarningRight=${showNewlineWarningRight}
+ .useNewImageDiffUi=${useNewImageDiffUi}
+ ></gr-diff>`
+ );
}
async initLayers() {
@@ -566,6 +576,7 @@
async reloadInternal(shouldReportMetric?: boolean) {
this.reporting.time(Timing.DIFF_TOTAL);
this.reporting.time(Timing.DIFF_LOAD);
+ this.grDiffKey++;
// TODO: Find better names for these 3 clear/cancel methods. Ideally the
// <gr-diff-host> should not re-used at all for another diff rendering pass.
this.clear();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index e0d5f1d..c4f4d1a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -60,6 +60,7 @@
CommentsModel,
commentsModelToken,
} from '../../../models/comments/comments-model';
+import {isNewDiff} from '../../../embed/diff/gr-diff/gr-diff-utils';
suite('gr-diff-host tests', () => {
let element: GrDiffHost;
@@ -153,6 +154,7 @@
});
test('reload() cancels before network resolves', async () => {
+ if (isNewDiff()) return;
assertIsDefined(element.diffElement);
const cancelStub = sinon.stub(element.diffElement, 'cancel');
diff --git a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts
index 36e1f1a..e7ae8a4 100644
--- a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts
+++ b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts
@@ -12,6 +12,9 @@
import {getShowConfig} from './gr-context-controls';
import {ifDefined} from 'lit/directives/if-defined.js';
import {when} from 'lit/directives/when.js';
+import {subscribe} from '../../../elements/lit/subscription-controller';
+import {resolve} from '../../../models/dependency';
+import {diffModelToken} from '../gr-diff-model/gr-diff-model';
export class GrContextControlsSection extends LitElement {
/** Should context controls be rendered for expanding above the section? */
@@ -38,6 +41,19 @@
@state()
addTableWrapperForTesting = false;
+ @state() viewMode: DiffViewMode = DiffViewMode.SIDE_BY_SIDE;
+
+ private readonly getDiffModel = resolve(this, diffModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getDiffModel().viewMode$,
+ viewMode => (this.viewMode = viewMode)
+ );
+ }
+
/**
* The browser API for handling selection does not (yet) work for selection
* across multiple shadow DOM elements. So we are rendering gr-diff components
@@ -82,7 +98,7 @@
}
private isSideBySide() {
- return this.renderPrefs?.view_mode !== DiffViewMode.UNIFIED;
+ return this.viewMode !== DiffViewMode.UNIFIED;
}
private createContextControlRow() {
diff --git a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
index 6a557fc..d936126 100644
--- a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
@@ -3,7 +3,12 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
+import {
+ DIProviderElement,
+ wrapInProvider,
+} from '../../../models/di-provider-element';
import '../../../test/common-test-setup';
+import {DiffModel, diffModelToken} from '../gr-diff-model/gr-diff-model';
import './gr-context-controls-section';
import {GrContextControlsSection} from './gr-context-controls-section';
import {fixture, html, assert} from '@open-wc/testing';
@@ -12,9 +17,17 @@
let element: GrContextControlsSection;
setup(async () => {
- element = await fixture<GrContextControlsSection>(
- html`<gr-context-controls-section></gr-context-controls-section>`
- );
+ const diffModel = new DiffModel();
+ element = (
+ await fixture<DIProviderElement>(
+ wrapInProvider(
+ html`<gr-context-controls-section></gr-context-controls-section>`,
+ diffModelToken,
+ diffModel
+ )
+ )
+ ).querySelector<GrContextControlsSection>('gr-context-controls-section')!;
+
element.addTableWrapperForTesting = true;
await element.updateComplete;
});
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
deleted file mode 100644
index 7ace605..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ /dev/null
@@ -1,43 +0,0 @@
-/**
- * @license
- * Copyright 2017 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {GrDiffBuilder} from './gr-diff-builder';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {createElementDiff} from '../gr-diff/gr-diff-utils';
-import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {html, render} from 'lit';
-import {FILE} from '../../../api/diff';
-
-export class GrDiffBuilderBinary extends GrDiffBuilder {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement
- ) {
- super(diff, prefs, outputEl);
- }
-
- override buildSectionElement(group: GrDiffGroup): HTMLElement {
- const section = createElementDiff('tbody', 'binary-diff');
- // Do not create a diff row for LOST.
- if (group.lines[0].beforeNumber !== FILE) return section;
- return super.buildSectionElement(group);
- }
-
- public renderBinaryDiff() {
- render(
- html`
- <tbody class="gr-diff binary-diff">
- <tr class="gr-diff">
- <td colspan="5" class="gr-diff">
- <span>Difference in binary files</span>
- </td>
- </tr>
- </tbody>
- `,
- this.outputEl
- );
- }
-}
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 eeb07d8..fd3b975 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
@@ -4,83 +4,16 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {ImageInfo} from '../../../types/common';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {FILE, RenderPreferences, Side} from '../../../api/diff';
+import {Side} from '../../../api/diff';
import '../gr-diff-image-viewer/gr-image-viewer';
import {html, LitElement, nothing} from 'lit';
import {property, query, state} from 'lit/decorators.js';
-import {GrDiffBuilder} from './gr-diff-builder';
-import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {isNewDiff, createElementDiff} from '../gr-diff/gr-diff-utils';
+import {isNewDiff} from '../gr-diff/gr-diff-utils';
// MIME types for images we allow showing. Do not include SVG, it can contain
// arbitrary JavaScript.
const IMAGE_MIME_PATTERN = /^image\/(bmp|gif|x-icon|jpeg|jpg|png|tiff|webp)$/;
-export class GrDiffBuilderImage extends GrDiffBuilder {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- private readonly baseImage: ImageInfo | null,
- private readonly revisionImage: ImageInfo | null,
- renderPrefs?: RenderPreferences,
- private readonly useNewImageDiffUi: boolean = false
- ) {
- super(diff, prefs, outputEl, [], renderPrefs);
- }
-
- override buildSectionElement(group: GrDiffGroup): HTMLElement {
- const section = createElementDiff('tbody');
- // Do not create a diff row for LOST.
- if (group.lines[0].beforeNumber !== FILE) return section;
- return super.buildSectionElement(group);
- }
-
- public renderImageDiff() {
- const imageDiff = this.useNewImageDiffUi
- ? this.createImageDiffNew()
- : this.createImageDiffOld();
- this.outputEl.appendChild(imageDiff);
- }
-
- private createImageDiffNew() {
- // TODO(newdiff-cleanup): Remove cast when newdiff migration is complete.
- const imageDiff = document.createElement(
- 'gr-diff-image-new'
- ) as GrDiffImageNew;
- imageDiff.automaticBlink = this.autoBlink();
- imageDiff.baseImage = this.baseImage ?? undefined;
- imageDiff.revisionImage = this.revisionImage ?? undefined;
- return imageDiff;
- }
-
- private createImageDiffOld() {
- // TODO(newdiff-cleanup): Remove cast when newdiff migration is complete.
- const imageDiff = document.createElement(
- 'gr-diff-image-old'
- ) as GrDiffImageOld;
- imageDiff.baseImage = this.baseImage ?? undefined;
- imageDiff.revisionImage = this.revisionImage ?? undefined;
- return imageDiff;
- }
-
- private autoBlink(): boolean {
- return !!this.renderPrefs?.image_diff_prefs?.automatic_blink;
- }
-
- override updateRenderPrefs(renderPrefs: RenderPreferences) {
- this.renderPrefs = renderPrefs;
-
- // We have to update `imageDiff.automaticBlink` manually, because `this` is
- // not a LitElement.
- const imageDiff = this.outputEl.querySelector(
- 'gr-diff-image-new'
- ) as GrDiffImageNew;
- if (imageDiff) imageDiff.automaticBlink = this.autoBlink();
- }
-}
-
class GrDiffImageNew extends LitElement {
@property() baseImage?: ImageInfo;
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
deleted file mode 100644
index bcc54d4..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
+++ /dev/null
@@ -1,352 +0,0 @@
-/**
- * @license
- * Copyright 2016 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import './gr-diff-section';
-import '../gr-context-controls/gr-context-controls';
-import {
- ContentLoadNeededEventDetail,
- DiffContextExpandedExternalDetail,
- DiffViewMode,
- LineNumber,
- RenderPreferences,
-} from '../../../api/diff';
-import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {BlameInfo} from '../../../types/common';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {Side} from '../../../constants/constants';
-import {DiffLayer, isDefined} from '../../../types/types';
-import {GrDiffRow} from './gr-diff-row';
-import {GrDiffSection} from './gr-diff-section';
-import {html, render} from 'lit';
-import {diffClasses} from '../gr-diff/gr-diff-utils';
-import {when} from 'lit/directives/when.js';
-import {GrDiffBuilderImage} from './gr-diff-builder-image';
-import {GrDiffBuilderBinary} from './gr-diff-builder-binary';
-
-export interface DiffContextExpandedEventDetail
- extends DiffContextExpandedExternalDetail {
- /** The context control group that should be replaced by `groups`. */
- contextGroup: GrDiffGroup;
- groups: GrDiffGroup[];
-}
-
-declare global {
- interface HTMLElementEventMap {
- 'diff-context-expanded-internal-new': CustomEvent<DiffContextExpandedEventDetail>;
- 'diff-context-expanded': CustomEvent<DiffContextExpandedExternalDetail>;
- 'content-load-needed': CustomEvent<ContentLoadNeededEventDetail>;
- }
-}
-
-export function isImageDiffBuilder<T extends GrDiffBuilder>(
- x: T | GrDiffBuilderImage | undefined
-): x is GrDiffBuilderImage {
- return !!x && !!(x as GrDiffBuilderImage).renderImageDiff;
-}
-
-export function isBinaryDiffBuilder<T extends GrDiffBuilder>(
- x: T | GrDiffBuilderBinary | undefined
-): x is GrDiffBuilderBinary {
- return !!x && !!(x as GrDiffBuilderBinary).renderBinaryDiff;
-}
-
-/**
- * The builder takes GrDiffGroups, and builds the corresponding DOM elements,
- * called sections. Only the builder should add or remove sections from the
- * DOM. Callers can use the ...group() methods to modify groups and thus cause
- * rendering changes.
- */
-export class GrDiffBuilder {
- private readonly diff: DiffInfo;
-
- readonly prefs: DiffPreferencesInfo;
-
- renderPrefs?: RenderPreferences;
-
- readonly outputEl: HTMLElement;
-
- private groups: GrDiffGroup[];
-
- private readonly layerUpdateListener: (
- start: LineNumber,
- end: LineNumber,
- side: Side
- ) => void;
-
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- readonly layers: DiffLayer[] = [],
- renderPrefs?: RenderPreferences
- ) {
- this.diff = diff;
- this.prefs = prefs;
- this.renderPrefs = renderPrefs;
- this.outputEl = outputEl;
- this.groups = [];
-
- if (isNaN(prefs.tab_size) || prefs.tab_size <= 0) {
- throw Error('Invalid tab size from preferences.');
- }
-
- if (isNaN(prefs.line_length) || prefs.line_length <= 0) {
- throw Error('Invalid line length from preferences.');
- }
-
- this.layerUpdateListener = (
- start: LineNumber,
- end: LineNumber,
- side: Side
- ) => this.renderContentByRange(start, end, side);
- this.init();
- }
-
- getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | undefined {
- if (!side) return undefined;
- const row = this.findRow(lineNumber, side);
- return row?.getContentCell(side);
- }
-
- getLineElByNumber(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | undefined {
- if (!side) return undefined;
- const row = this.findRow(lineNumber, side);
- return row?.getLineNumberCell(side);
- }
-
- private findRow(lineNumber?: LineNumber, side?: Side): GrDiffRow | undefined {
- if (!side || !lineNumber) return undefined;
- const group = this.findGroup(side, lineNumber);
- if (!group) return undefined;
- const section = this.findSection(group);
- if (!section) return undefined;
- return section.findRow(side, lineNumber);
- }
-
- private getDiffRows() {
- const sections = [
- ...this.outputEl.querySelectorAll<GrDiffSection>('gr-diff-section'),
- ];
- return sections.map(s => s.getDiffRows()).flat();
- }
-
- getLineNumberRows(): HTMLTableRowElement[] {
- const rows = this.getDiffRows();
- return rows.map(r => r.getTableRow()).filter(isDefined);
- }
-
- getLineNumEls(side: Side): HTMLTableCellElement[] {
- const rows = this.getDiffRows();
- return rows.map(r => r.getLineNumberCell(side)).filter(isDefined);
- }
-
- /** This is used when layers initiate an update. */
- renderContentByRange(start: LineNumber, end: LineNumber, side: Side) {
- const groups = this.getGroupsByLineRange(start, end, side);
- for (const group of groups) {
- const section = this.findSection(group);
- for (const row of section?.getDiffRows() ?? []) {
- row.requestUpdate();
- }
- }
- }
-
- private findSection(group: GrDiffGroup): GrDiffSection | undefined {
- const leftClass = `left-${group.startLine(Side.LEFT)}`;
- const rightClass = `right-${group.startLine(Side.RIGHT)}`;
- return (
- this.outputEl.querySelector<GrDiffSection>(
- `gr-diff-section.${leftClass}.${rightClass}`
- ) ?? undefined
- );
- }
-
- buildSectionElement(group: GrDiffGroup): HTMLElement {
- const leftCl = `left-${group.startLine(Side.LEFT)}`;
- const rightCl = `right-${group.startLine(Side.RIGHT)}`;
- const section = html`
- <gr-diff-section
- class="${leftCl} ${rightCl}"
- .group=${group}
- .diff=${this.diff}
- .layers=${this.layers}
- .diffPrefs=${this.prefs}
- .renderPrefs=${this.renderPrefs}
- ></gr-diff-section>
- `;
- // When using Lit's `render()` method it wants to be in full control of the
- // element that it renders into, so we let it render into a temp element.
- // Rendering into the diff table directly would interfere with
- // `clearDiffContent()`for example.
- // TODO: Convert <gr-diff> to be fully lit controlled and incorporate this
- // method into Lit's `render()` cycle.
- const tempEl = document.createElement('div');
- render(section, tempEl);
- const sectionEl = tempEl.firstElementChild as GrDiffSection;
- return sectionEl;
- }
-
- addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
- const colgroup = html`
- <colgroup>
- <col class=${diffClasses('blame')}></col>
- ${when(
- this.renderPrefs?.view_mode === DiffViewMode.UNIFIED,
- () => html` ${this.renderUnifiedColumns(lineNumberWidth)} `,
- () => html`
- ${this.renderSideBySideColumns(Side.LEFT, lineNumberWidth)}
- ${this.renderSideBySideColumns(Side.RIGHT, lineNumberWidth)}
- `
- )}
- </colgroup>
- `;
- // When using Lit's `render()` method it wants to be in full control of the
- // element that it renders into, so we let it render into a temp element.
- // Rendering into the diff table directly would interfere with
- // `clearDiffContent()`for example.
- // TODO: Convert <gr-diff> to be fully lit controlled and incorporate this
- // method into Lit's `render()` cycle.
- const tempEl = document.createElement('div');
- render(colgroup, tempEl);
- const colgroupEl = tempEl.firstElementChild as HTMLElement;
- outputEl.appendChild(colgroupEl);
- }
-
- private renderUnifiedColumns(lineNumberWidth: number) {
- return html`
- <col class=${diffClasses()} width=${lineNumberWidth}></col>
- <col class=${diffClasses()} width=${lineNumberWidth}></col>
- <col class=${diffClasses()}></col>
- `;
- }
-
- private renderSideBySideColumns(side: Side, lineNumberWidth: number) {
- return html`
- <col class=${diffClasses(side)} width=${lineNumberWidth}></col>
- <col class=${diffClasses(side, 'sign')}></col>
- <col class=${diffClasses(side)}></col>
- `;
- }
-
- /**
- * This is meant to be called when the gr-diff component re-connects, or when
- * the diff is (re-)rendered.
- *
- * Make sure that this method is symmetric with cleanup(), which is called
- * when gr-diff disconnects.
- */
- init() {
- this.cleanup();
- for (const layer of this.layers) {
- if (layer.addListener) {
- layer.addListener(this.layerUpdateListener);
- }
- }
- }
-
- /**
- * This is meant to be called when the gr-diff component disconnects, or when
- * the diff is (re-)rendered.
- *
- * Make sure that this method is symmetric with init(), which is called when
- * gr-diff re-connects.
- */
- cleanup() {
- for (const layer of this.layers) {
- if (layer.removeListener) {
- layer.removeListener(this.layerUpdateListener);
- }
- }
- }
-
- addGroups(groups: readonly GrDiffGroup[]) {
- for (const group of groups) {
- this.groups.push(group);
- this.emitGroup(group);
- }
- }
-
- clearGroups() {
- for (const deletedGroup of this.groups) {
- deletedGroup.element?.remove();
- }
- this.groups = [];
- }
-
- replaceGroup(contextControl: GrDiffGroup, groups: readonly GrDiffGroup[]) {
- const i = this.groups.indexOf(contextControl);
- if (i === -1) throw new Error('cannot find context control group');
-
- const contextControlSection = this.groups[i].element;
- if (!contextControlSection) throw new Error('diff group element not set');
-
- this.groups.splice(i, 1, ...groups);
- for (const group of groups) {
- this.emitGroup(group, contextControlSection);
- }
- if (contextControlSection) contextControlSection.remove();
- }
-
- findGroup(side: Side, line: LineNumber) {
- return this.groups.find(group => group.containsLine(side, line));
- }
-
- private emitGroup(group: GrDiffGroup, beforeSection?: HTMLElement) {
- const element = this.buildSectionElement(group);
- this.outputEl.insertBefore(element, beforeSection ?? null);
- group.element = element;
- }
-
- // visible for testing
- getGroupsByLineRange(
- startLine: LineNumber,
- endLine: LineNumber,
- side: Side
- ): GrDiffGroup[] {
- const startIndex = this.groups.findIndex(group =>
- group.containsLine(side, startLine)
- );
- if (startIndex === -1) return [];
- let endIndex = this.groups.findIndex(group =>
- group.containsLine(side, endLine)
- );
- // Not all groups may have been processed yet (i.e. this.groups is still
- // incomplete). In that case let's just return *all* groups until the end
- // of the array.
- if (endIndex === -1) endIndex = this.groups.length - 1;
- // The filter preserves the legacy behavior to only return non-context
- // groups
- return this.groups
- .slice(startIndex, endIndex + 1)
- .filter(group => group.lines.length > 0);
- }
-
- /**
- * Set the blame information for the diff. For any already-rendered line,
- * re-render its blame cell content.
- */
- setBlame(blame: BlameInfo[]) {
- for (const blameInfo of blame) {
- for (const range of blameInfo.ranges) {
- for (let line = range.start; line <= range.end; line++) {
- const row = this.findRow(line, Side.LEFT);
- if (row) row.blameInfo = blameInfo;
- }
- }
- }
- }
-
- /**
- * Only special builders need to implement this. The default is to
- * just ignore it.
- */
- updateRenderPrefs(_: RenderPreferences) {}
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
index 51024da..bfd6a0d 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
@@ -21,6 +21,7 @@
import {fire} from '../../../utils/event-util';
import {getBaseUrl} from '../../../utils/url-util';
import './gr-diff-text';
+import '../../../elements/shared/gr-hovercard/gr-hovercard';
import {GrDiffLine} from '../gr-diff/gr-diff-line';
import {diffClasses, isNewDiff, isResponsive} from '../gr-diff/gr-diff-utils';
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
index e02d62b..aad7928 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {html, LitElement} from 'lit';
-import {property, state} from 'lit/decorators.js';
+import {property, queryAll, state} from 'lit/decorators.js';
import {
DiffInfo,
DiffLayer,
@@ -28,8 +28,14 @@
import {when} from 'lit/directives/when.js';
import {fire} from '../../../utils/event-util';
import {countLines} from '../../../utils/diff-util';
+import {resolve} from '../../../models/dependency';
+import {diffModelToken} from '../gr-diff-model/gr-diff-model';
+import {subscribe} from '../../../elements/lit/subscription-controller';
export class GrDiffSection extends LitElement {
+ @queryAll('gr-diff-row')
+ diffRows?: NodeListOf<GrDiffRow>;
+
@property({type: Object})
group?: GrDiffGroup;
@@ -45,6 +51,9 @@
@property({type: Object})
layers: DiffLayer[] = [];
+ @state()
+ lineLength = 100;
+
/**
* Semantic DOM diff testing does not work with just table fragments, so when
* running such tests the render() method has to wrap the DOM in a proper
@@ -53,6 +62,24 @@
@state()
addTableWrapperForTesting = false;
+ @state() viewMode: DiffViewMode = DiffViewMode.SIDE_BY_SIDE;
+
+ private readonly getDiffModel = resolve(this, diffModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getDiffModel().lineLength$,
+ lineLength => (this.lineLength = lineLength)
+ );
+ subscribe(
+ this,
+ () => this.getDiffModel().viewMode$,
+ viewMode => (this.viewMode = viewMode)
+ );
+ }
+
/**
* The browser API for handling selection does not (yet) work for selection
* across multiple shadow DOM elements. So we are rendering gr-diff components
@@ -64,6 +91,13 @@
return this;
}
+ protected override async getUpdateComplete(): Promise<boolean> {
+ const result = await super.getUpdateComplete();
+ const rows = [...(this.diffRows ?? [])];
+ await Promise.all(rows.map(row => row.updateComplete));
+ return result;
+ }
+
override render() {
if (!this.group) return;
const extras: string[] = [];
@@ -84,11 +118,11 @@
<tbody class=${diffClasses(...extras)}>
${this.renderContextControls()} ${this.renderMoveControls()}
${pairs.map(pair => {
- const leftCl = `left-${pair.left.lineNumber(Side.LEFT)}`;
- const rightCl = `right-${pair.right.lineNumber(Side.RIGHT)}`;
+ const leftClass = `left-${pair.left.lineNumber(Side.LEFT)}`;
+ const rightClass = `right-${pair.right.lineNumber(Side.RIGHT)}`;
return html`
<gr-diff-row
- class="${leftCl} ${rightCl}"
+ class="${leftClass} ${rightClass}"
.left=${pair.left}
.right=${pair.right}
.layers=${this.layers}
@@ -112,7 +146,7 @@
}
private isUnifiedDiff() {
- return this.renderPrefs?.view_mode === DiffViewMode.UNIFIED;
+ return this.viewMode === DiffViewMode.UNIFIED;
}
getLinePairs() {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
index 381f9b2..d23c9c5 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
@@ -73,10 +73,7 @@
});
test('unified', async () => {
- element.renderPrefs = {
- ...element.renderPrefs,
- view_mode: DiffViewMode.UNIFIED,
- };
+ element.viewMode = DiffViewMode.UNIFIED;
const row = await waitQueryAndAssert(element, 'tr.moveControls');
// Semantic dom diff has a problem with just comparing table rows or
// cells directly. So as a workaround put the row into an empty test
@@ -162,7 +159,7 @@
data-side="left"
id="left-content-1"
>
- <gr-diff-text> </gr-diff-text>
+ <gr-diff-text>asdf</gr-diff-text>
</div>
<div class="thread-group" data-side="left">
<slot name="left-1"> </slot>
@@ -186,7 +183,7 @@
data-side="right"
id="right-content-1"
>
- <gr-diff-text> </gr-diff-text>
+ <gr-diff-text>asdf </gr-diff-text>
</div>
<div class="thread-group" data-side="right">
<slot name="right-1"> </slot>
@@ -219,7 +216,7 @@
data-side="left"
id="left-content-1"
>
- <gr-diff-text> </gr-diff-text>
+ <gr-diff-text> qwer</gr-diff-text>
</div>
<div class="thread-group" data-side="left">
<slot name="left-1"> </slot>
@@ -243,7 +240,7 @@
data-side="right"
id="right-content-1"
>
- <gr-diff-text> </gr-diff-text>
+ <gr-diff-text>qwer </gr-diff-text>
</div>
<div class="thread-group" data-side="right">
<slot name="right-1"> </slot>
@@ -276,7 +273,7 @@
data-side="left"
id="left-content-1"
>
- <gr-diff-text> </gr-diff-text>
+ <gr-diff-text>zxcv </gr-diff-text>
</div>
<div class="thread-group" data-side="left">
<slot name="left-1"> </slot>
@@ -300,7 +297,7 @@
data-side="right"
id="right-content-1"
>
- <gr-diff-text> </gr-diff-text>
+ <gr-diff-text>zxcv </gr-diff-text>
</div>
<div class="thread-group" data-side="right">
<slot name="right-1"> </slot>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
index 61f8551..e5aaafd 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
@@ -229,7 +229,7 @@
suite('unified diff', () => {
setup(async () => {
diffElement.viewMode = DiffViewMode.UNIFIED;
- await waitForEventOnce(diffElement, 'render');
+ await diffElement.updateComplete;
cursor.reInitCursor();
});
@@ -457,9 +457,15 @@
.callsFake(() => {
scrollBehaviorDuringMove = cursor.cursorManager.scrollMode;
});
- diffElement.diff = createDiff();
- await diffElement.updateComplete;
- await waitForEventOnce(diffElement, 'render');
+ cursor.dispose();
+ const diff = createDiff();
+ diff.content.push({ab: ['one more line']});
+ diffElement.diff = diff;
+ diffElement.prefs = createDefaultDiffPrefs();
+ await Promise.all([
+ diffElement.updateComplete,
+ waitForEventOnce(diffElement, 'render'),
+ ]);
cursor.reInitCursor();
assert.isFalse(moveToNumStub.called);
assert.isTrue(moveToChunkStub.called);
@@ -478,9 +484,10 @@
cursor.initialLineNumber = 10;
cursor.side = Side.RIGHT;
- diffElement.diff = createDiff();
- await diffElement.updateComplete;
- await waitForEventOnce(diffElement, 'render');
+ cursor.dispose();
+ const diff = createDiff();
+ diff.content.push({ab: ['one more line']});
+ diffElement.diff = diff;
cursor.reInitCursor();
assert.isFalse(moveToChunkStub.called);
assert.isTrue(moveToNumStub.called);
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts b/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
index 252f7cb..8dae154 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-model/gr-diff-model.ts
@@ -4,10 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {Observable, combineLatest, from} from 'rxjs';
-import {switchMap, withLatestFrom} from 'rxjs/operators';
+import {debounceTime, filter, switchMap, withLatestFrom} from 'rxjs/operators';
import {
DiffInfo,
DiffPreferencesInfo,
+ DiffViewMode,
DisplayLine,
RenderPreferences,
} from '../../../api/diff';
@@ -20,16 +21,18 @@
KeyLocations,
computeContext,
computeKeyLocations,
+ computeLineLength,
} from '../gr-diff/gr-diff-utils';
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {
GrDiffProcessor,
ProcessingOptions,
} from '../gr-diff-processor/gr-diff-processor';
-import {GrDiffGroup} from '../gr-diff/gr-diff-group';
+import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
+import {assert} from '../../../utils/common-util';
export interface DiffState {
- diff: DiffInfo;
+ diff?: DiffInfo;
path?: string;
renderPrefs: RenderPreferences;
diffPrefs: DiffPreferencesInfo;
@@ -45,8 +48,8 @@
export class DiffModel extends Model<DiffState> {
readonly diff$: Observable<DiffInfo> = select(
- this.state$,
- diffState => diffState.diff
+ this.state$.pipe(filter(state => state.diff !== undefined)),
+ diffState => diffState.diff!
);
readonly path$: Observable<string | undefined> = select(
@@ -59,6 +62,11 @@
diffState => diffState.renderPrefs
);
+ readonly viewMode$: Observable<DiffViewMode> = select(
+ this.renderPrefs$,
+ renderPrefs => renderPrefs.view_mode ?? DiffViewMode.SIDE_BY_SIDE
+ );
+
readonly diffPrefs$: Observable<DiffPreferencesInfo> = select(
this.state$,
diffState => diffState.diffPrefs
@@ -77,6 +85,15 @@
diffState => diffState.isImageDiff
);
+ readonly groups$: Observable<GrDiffGroup[]> = select(
+ this.state$,
+ diffState => diffState.groups ?? []
+ );
+
+ readonly lineLength$: Observable<number> = select(this.state$, state =>
+ computeLineLength(state.diffPrefs, state.path)
+ );
+
readonly keyLocations$: Observable<KeyLocations> = select(
this.state$,
diffState =>
@@ -85,7 +102,6 @@
constructor() {
super({
- diff: {content: [], change_type: 'MODIFIED', intraline_status: 'OK'},
diffPrefs: createDefaultDiffPrefs(),
renderPrefs: {},
comments: [],
@@ -105,6 +121,7 @@
])
.pipe(
withLatestFrom(this.keyLocations$),
+ debounceTime(1),
switchMap(
([[diff, context, renderPrefs, isImageDiff], keyLocations]) => {
const options: ProcessingOptions = {
@@ -112,10 +129,10 @@
keyLocations,
isBinary: !!(isImageDiff || diff.binary),
};
- const processor = new GrDiffProcessor(undefined, options);
if (renderPrefs?.num_lines_rendered_at_once) {
options.asyncThreshold = renderPrefs.num_lines_rendered_at_once;
}
+ const processor = new GrDiffProcessor(options);
return from(processor.process(diff.content));
}
)
@@ -124,4 +141,20 @@
this.updateState({groups});
});
}
+
+ /**
+ * Replace a context control group with some expanded groups. Happens when the
+ * user clicks "+10" or something similar.
+ */
+ replaceGroup(group: GrDiffGroup, newGroups: readonly GrDiffGroup[]) {
+ assert(
+ group.type === GrDiffGroupType.CONTEXT_CONTROL,
+ 'gr-diff can only replace context control groups'
+ );
+ const groups = [...this.getState().groups];
+ const i = groups.indexOf(group);
+ if (i === -1) throw new Error('cannot find context control group');
+ groups.splice(i, 1, ...newGroups);
+ this.updateState({groups});
+ }
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts
index 70fe338..98c0ab7 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor.ts
@@ -14,7 +14,7 @@
import {debounce, DelayedTask} from '../../../utils/async-util';
import {assert} from '../../../utils/common-util';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
-import {FILE, GrDiffLineType, LineNumber} from '../../../api/diff';
+import {GrDiffLineType, LineNumber} from '../../../api/diff';
import {FULL_CONTEXT, KeyLocations} from '../gr-diff/gr-diff-utils';
// visible for testing
@@ -105,11 +105,9 @@
private resetIsScrollingTask?: DelayedTask;
- constructor(
- private consumer: GroupConsumer | undefined,
- options: ProcessingOptions
- ) {
- this.consumer = consumer;
+ private readonly groups: GrDiffGroup[] = [];
+
+ constructor(options: ProcessingOptions) {
this.context = options.context;
this.asyncThreshold = options.asyncThreshold ?? 64;
this.keyLocations = options.keyLocations ?? {left: {}, right: {}};
@@ -137,22 +135,19 @@
window.addEventListener('scroll', this.handleWindowScroll);
- this.consumer?.clearGroups();
- const groups = [this.makeGroup('LOST'), this.makeGroup(FILE)];
- this.consumer?.addGroup(groups[0]);
- this.consumer?.addGroup(groups[1]);
+ this.groups.push(this.makeGroup('LOST'));
+ this.groups.push(this.makeGroup('FILE'));
- if (this.isBinary) return groups;
+ if (this.isBinary) return this.groups;
try {
await this.processChunks(chunks);
} finally {
this.finish();
}
- return groups;
+ return this.groups;
}
finish() {
- this.consumer = undefined;
window.removeEventListener('scroll', this.handleWindowScroll);
}
@@ -186,7 +181,7 @@
const stateUpdate = this.processNext(state, chunks);
for (const group of stateUpdate.groups) {
- this.consumer?.addGroup(group);
+ this.groups.push(group);
currentBatch += group.lines.length;
}
state.lineNums.left += stateUpdate.lineDelta.left;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
index 49dab4f..3485fe4 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-processor/gr-diff-processor_test.ts
@@ -7,12 +7,7 @@
import './gr-diff-processor';
import {GrDiffLine} from '../gr-diff/gr-diff-line';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {
- GrDiffProcessor,
- GroupConsumer,
- ProcessingOptions,
- State,
-} from './gr-diff-processor';
+import {GrDiffProcessor, ProcessingOptions, State} from './gr-diff-processor';
import {DiffContent} from '../../../types/diff';
import {assert} from '@open-wc/testing';
import {FILE, GrDiffLineType} from '../../../api/diff';
@@ -30,26 +25,16 @@
let options: ProcessingOptions = {
context: 4,
};
- let groups: GrDiffGroup[];
- const consumer: GroupConsumer = {
- addGroup(group: GrDiffGroup) {
- groups.push(group);
- },
- clearGroups() {
- groups = [];
- },
- };
setup(() => {});
suite('not logged in', () => {
setup(() => {
- groups = [];
options = {context: 4};
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
});
- test('process loaded content', () => {
+ test('process loaded content', async () => {
const content: DiffContent[] = [
{
ab: ['<!DOCTYPE html>', '<meta charset="utf-8">'],
@@ -67,82 +52,80 @@
},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
- assert.equal(groups.length, 4);
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
+ assert.equal(groups.length, 4);
- let group = groups[0];
- assert.equal(group.type, GrDiffGroupType.BOTH);
- assert.equal(group.lines.length, 1);
- assert.equal(group.lines[0].text, '');
- assert.equal(group.lines[0].beforeNumber, FILE);
- assert.equal(group.lines[0].afterNumber, FILE);
+ let group = groups[0];
+ assert.equal(group.type, GrDiffGroupType.BOTH);
+ assert.equal(group.lines.length, 1);
+ assert.equal(group.lines[0].text, '');
+ assert.equal(group.lines[0].beforeNumber, FILE);
+ assert.equal(group.lines[0].afterNumber, FILE);
- group = groups[1];
- assert.equal(group.type, GrDiffGroupType.BOTH);
- assert.equal(group.lines.length, 2);
+ group = groups[1];
+ assert.equal(group.type, GrDiffGroupType.BOTH);
+ assert.equal(group.lines.length, 2);
- function beforeNumberFn(l: GrDiffLine) {
- return l.beforeNumber;
- }
- function afterNumberFn(l: GrDiffLine) {
- return l.afterNumber;
- }
- function textFn(l: GrDiffLine) {
- return l.text;
- }
+ function beforeNumberFn(l: GrDiffLine) {
+ return l.beforeNumber;
+ }
+ function afterNumberFn(l: GrDiffLine) {
+ return l.afterNumber;
+ }
+ function textFn(l: GrDiffLine) {
+ return l.text;
+ }
- assert.deepEqual(group.lines.map(beforeNumberFn), [1, 2]);
- assert.deepEqual(group.lines.map(afterNumberFn), [1, 2]);
- assert.deepEqual(group.lines.map(textFn), [
- '<!DOCTYPE html>',
- '<meta charset="utf-8">',
- ]);
+ assert.deepEqual(group.lines.map(beforeNumberFn), [1, 2]);
+ assert.deepEqual(group.lines.map(afterNumberFn), [1, 2]);
+ assert.deepEqual(group.lines.map(textFn), [
+ '<!DOCTYPE html>',
+ '<meta charset="utf-8">',
+ ]);
- group = groups[2];
- assert.equal(group.type, GrDiffGroupType.DELTA);
- assert.equal(group.lines.length, 3);
- assert.equal(group.adds.length, 1);
- assert.equal(group.removes.length, 2);
- assert.deepEqual(group.removes.map(beforeNumberFn), [3, 4]);
- assert.deepEqual(group.adds.map(afterNumberFn), [3]);
- assert.deepEqual(group.removes.map(textFn), [
- ' Welcome ',
- ' to the wooorld of tomorrow!',
- ]);
- assert.deepEqual(group.adds.map(textFn), [' Hello, world!']);
+ group = groups[2];
+ assert.equal(group.type, GrDiffGroupType.DELTA);
+ assert.equal(group.lines.length, 3);
+ assert.equal(group.adds.length, 1);
+ assert.equal(group.removes.length, 2);
+ assert.deepEqual(group.removes.map(beforeNumberFn), [3, 4]);
+ assert.deepEqual(group.adds.map(afterNumberFn), [3]);
+ assert.deepEqual(group.removes.map(textFn), [
+ ' Welcome ',
+ ' to the wooorld of tomorrow!',
+ ]);
+ assert.deepEqual(group.adds.map(textFn), [' Hello, world!']);
- group = groups[3];
- assert.equal(group.type, GrDiffGroupType.BOTH);
- assert.equal(group.lines.length, 3);
- assert.deepEqual(group.lines.map(beforeNumberFn), [5, 6, 7]);
- assert.deepEqual(group.lines.map(afterNumberFn), [4, 5, 6]);
- assert.deepEqual(group.lines.map(textFn), [
- 'Leela: This is the only place the ship can’t hear us, so ',
- 'everyone pretend to shower.',
- 'Fry: Same as every day. Got it.',
- ]);
- });
+ group = groups[3];
+ assert.equal(group.type, GrDiffGroupType.BOTH);
+ assert.equal(group.lines.length, 3);
+ assert.deepEqual(group.lines.map(beforeNumberFn), [5, 6, 7]);
+ assert.deepEqual(group.lines.map(afterNumberFn), [4, 5, 6]);
+ assert.deepEqual(group.lines.map(textFn), [
+ 'Leela: This is the only place the ship can’t hear us, so ',
+ 'everyone pretend to shower.',
+ 'Fry: Same as every day. Got it.',
+ ]);
});
- test('first group is for file', () => {
+ test('first group is for file', async () => {
const content = [{b: ['foo']}];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- assert.equal(groups[0].type, GrDiffGroupType.BOTH);
- assert.equal(groups[0].lines.length, 1);
- assert.equal(groups[0].lines[0].text, '');
- assert.equal(groups[0].lines[0].beforeNumber, FILE);
- assert.equal(groups[0].lines[0].afterNumber, FILE);
- });
+ assert.equal(groups[0].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[0].lines.length, 1);
+ assert.equal(groups[0].lines[0].text, '');
+ assert.equal(groups[0].lines[0].beforeNumber, FILE);
+ assert.equal(groups[0].lines[0].afterNumber, FILE);
});
- suite('context groups', () => {
- test('at the beginning, larger than context', () => {
+ suite('context groups', async () => {
+ test('at the beginning, larger than context', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{
ab: Array.from<string>({length: 100}).fill(
@@ -152,28 +135,27 @@
{a: ['all work and no play make andybons a dull boy']},
];
- return processor.process(content).then(() => {
- // group[0] is the LOST group
- // group[1] is the FILE group
+ const groups = await processor.process(content);
+ // group[0] is the LOST group
+ // group[1] is the FILE group
- assert.equal(groups[2].type, GrDiffGroupType.CONTEXT_CONTROL);
- assert.instanceOf(groups[2].contextGroups[0], GrDiffGroup);
- assert.equal(groups[2].contextGroups[0].lines.length, 90);
- for (const l of groups[2].contextGroups[0].lines) {
- assert.equal(l.text, 'all work and no play make jack a dull boy');
- }
+ assert.equal(groups[2].type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.instanceOf(groups[2].contextGroups[0], GrDiffGroup);
+ assert.equal(groups[2].contextGroups[0].lines.length, 90);
+ for (const l of groups[2].contextGroups[0].lines) {
+ assert.equal(l.text, 'all work and no play make jack a dull boy');
+ }
- assert.equal(groups[3].type, GrDiffGroupType.BOTH);
- assert.equal(groups[3].lines.length, 10);
- for (const l of groups[3].lines) {
- assert.equal(l.text, 'all work and no play make jack a dull boy');
- }
- });
+ assert.equal(groups[3].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[3].lines.length, 10);
+ for (const l of groups[3].lines) {
+ assert.equal(l.text, 'all work and no play make jack a dull boy');
+ }
});
test('at the beginning with skip chunks', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{
ab: Array.from<string>({length: 20}).fill(
@@ -185,8 +167,7 @@
{a: ['some other content']},
];
- await processor.process(content);
-
+ const groups = await processor.process(content);
groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
@@ -224,9 +205,9 @@
}
});
- test('at the beginning, smaller than context', () => {
+ test('at the beginning, smaller than context', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{
ab: Array.from<string>({length: 5}).fill(
@@ -236,22 +217,21 @@
{a: ['all work and no play make andybons a dull boy']},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- // group[0] is the file group
+ // group[0] is the file group
- assert.equal(groups[1].type, GrDiffGroupType.BOTH);
- assert.equal(groups[1].lines.length, 5);
- for (const l of groups[1].lines) {
- assert.equal(l.text, 'all work and no play make jack a dull boy');
- }
- });
+ assert.equal(groups[1].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[1].lines.length, 5);
+ for (const l of groups[1].lines) {
+ assert.equal(l.text, 'all work and no play make jack a dull boy');
+ }
});
- test('at the end, larger than context', () => {
+ test('at the end, larger than context', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
@@ -261,28 +241,27 @@
},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- // group[0] is the file group
- // group[1] is the "a" group
+ // group[0] is the file group
+ // group[1] is the "a" group
- assert.equal(groups[2].type, GrDiffGroupType.BOTH);
- assert.equal(groups[2].lines.length, 10);
- for (const l of groups[2].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
+ assert.equal(groups[2].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[2].lines.length, 10);
+ for (const l of groups[2].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
- assert.equal(groups[3].type, GrDiffGroupType.CONTEXT_CONTROL);
- assert.instanceOf(groups[3].contextGroups[0], GrDiffGroup);
- assert.equal(groups[3].contextGroups[0].lines.length, 90);
- for (const l of groups[3].contextGroups[0].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- });
+ assert.equal(groups[3].type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.instanceOf(groups[3].contextGroups[0], GrDiffGroup);
+ assert.equal(groups[3].contextGroups[0].lines.length, 90);
+ for (const l of groups[3].contextGroups[0].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
});
- test('at the end, smaller than context', () => {
+ test('at the end, smaller than context', async () => {
options.context = 10;
const content = [
{a: ['all work and no play make andybons a dull boy']},
@@ -293,23 +272,22 @@
},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- // group[0] is the file group
- // group[1] is the "a" group
+ // group[0] is the file group
+ // group[1] is the "a" group
- assert.equal(groups[2].type, GrDiffGroupType.BOTH);
- assert.equal(groups[2].lines.length, 5);
- for (const l of groups[2].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- });
+ assert.equal(groups[2].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[2].lines.length, 5);
+ for (const l of groups[2].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
});
- test('for interleaved ab and common: true chunks', () => {
+ test('for interleaved ab and common: true chunks', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
@@ -347,85 +325,75 @@
},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- // group[0] is the file group
- // group[1] is the "a" group
+ // group[0] is the file group
+ // group[1] is the "a" group
- // The first three interleaved chunks are completely shown because
- // they are part of the context (3 * 3 <= 10)
+ // The first three interleaved chunks are completely shown because
+ // they are part of the context (3 * 3 <= 10)
- assert.equal(groups[2].type, GrDiffGroupType.BOTH);
- assert.equal(groups[2].lines.length, 3);
- for (const l of groups[2].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
+ assert.equal(groups[2].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[2].lines.length, 3);
+ for (const l of groups[2].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
- assert.equal(groups[3].type, GrDiffGroupType.DELTA);
- assert.equal(groups[3].lines.length, 6);
- assert.equal(groups[3].adds.length, 3);
- assert.equal(groups[3].removes.length, 3);
- for (const l of groups[3].removes) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- for (const l of groups[3].adds) {
- assert.equal(
- l.text,
- ' all work and no play make jill a dull girl'
- );
- }
+ assert.equal(groups[3].type, GrDiffGroupType.DELTA);
+ assert.equal(groups[3].lines.length, 6);
+ assert.equal(groups[3].adds.length, 3);
+ assert.equal(groups[3].removes.length, 3);
+ for (const l of groups[3].removes) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
+ for (const l of groups[3].adds) {
+ assert.equal(l.text, ' all work and no play make jill a dull girl');
+ }
- assert.equal(groups[4].type, GrDiffGroupType.BOTH);
- assert.equal(groups[4].lines.length, 3);
- for (const l of groups[4].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
+ assert.equal(groups[4].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[4].lines.length, 3);
+ for (const l of groups[4].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
- // The next chunk is partially shown, so it results in two groups
+ // The next chunk is partially shown, so it results in two groups
- assert.equal(groups[5].type, GrDiffGroupType.DELTA);
- assert.equal(groups[5].lines.length, 2);
- assert.equal(groups[5].adds.length, 1);
- assert.equal(groups[5].removes.length, 1);
- for (const l of groups[5].removes) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- for (const l of groups[5].adds) {
- assert.equal(
- l.text,
- ' all work and no play make jill a dull girl'
- );
- }
+ assert.equal(groups[5].type, GrDiffGroupType.DELTA);
+ assert.equal(groups[5].lines.length, 2);
+ assert.equal(groups[5].adds.length, 1);
+ assert.equal(groups[5].removes.length, 1);
+ for (const l of groups[5].removes) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
+ for (const l of groups[5].adds) {
+ assert.equal(l.text, ' all work and no play make jill a dull girl');
+ }
- assert.equal(groups[6].type, GrDiffGroupType.CONTEXT_CONTROL);
- assert.equal(groups[6].contextGroups.length, 2);
+ assert.equal(groups[6].type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.equal(groups[6].contextGroups.length, 2);
- assert.equal(groups[6].contextGroups[0].lines.length, 4);
- assert.equal(groups[6].contextGroups[0].removes.length, 2);
- assert.equal(groups[6].contextGroups[0].adds.length, 2);
- for (const l of groups[6].contextGroups[0].removes) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- for (const l of groups[6].contextGroups[0].adds) {
- assert.equal(
- l.text,
- ' all work and no play make jill a dull girl'
- );
- }
+ assert.equal(groups[6].contextGroups[0].lines.length, 4);
+ assert.equal(groups[6].contextGroups[0].removes.length, 2);
+ assert.equal(groups[6].contextGroups[0].adds.length, 2);
+ for (const l of groups[6].contextGroups[0].removes) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
+ for (const l of groups[6].contextGroups[0].adds) {
+ assert.equal(l.text, ' all work and no play make jill a dull girl');
+ }
- // The final chunk is completely hidden
- assert.equal(groups[6].contextGroups[1].type, GrDiffGroupType.BOTH);
- assert.equal(groups[6].contextGroups[1].lines.length, 3);
- for (const l of groups[6].contextGroups[1].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- });
+ // The final chunk is completely hidden
+ assert.equal(groups[6].contextGroups[1].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[6].contextGroups[1].lines.length, 3);
+ for (const l of groups[6].contextGroups[1].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
});
- test('in the middle, larger than context', () => {
+ test('in the middle, larger than context', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
@@ -436,36 +404,35 @@
{a: ['all work and no play make andybons a dull boy']},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- // group[0] is the file group
- // group[1] is the "a" group
+ // group[0] is the file group
+ // group[1] is the "a" group
- assert.equal(groups[2].type, GrDiffGroupType.BOTH);
- assert.equal(groups[2].lines.length, 10);
- for (const l of groups[2].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
+ assert.equal(groups[2].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[2].lines.length, 10);
+ for (const l of groups[2].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
- assert.equal(groups[3].type, GrDiffGroupType.CONTEXT_CONTROL);
- assert.instanceOf(groups[3].contextGroups[0], GrDiffGroup);
- assert.equal(groups[3].contextGroups[0].lines.length, 80);
- for (const l of groups[3].contextGroups[0].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
+ assert.equal(groups[3].type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.instanceOf(groups[3].contextGroups[0], GrDiffGroup);
+ assert.equal(groups[3].contextGroups[0].lines.length, 80);
+ for (const l of groups[3].contextGroups[0].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
- assert.equal(groups[4].type, GrDiffGroupType.BOTH);
- assert.equal(groups[4].lines.length, 10);
- for (const l of groups[4].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- });
+ assert.equal(groups[4].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[4].lines.length, 10);
+ for (const l of groups[4].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
});
- test('in the middle, smaller than context', () => {
+ test('in the middle, smaller than context', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
@@ -476,24 +443,23 @@
{a: ['all work and no play make andybons a dull boy']},
];
- return processor.process(content).then(() => {
- groups.shift(); // remove portedThreadsWithoutRangeGroup
+ const groups = await processor.process(content);
+ groups.shift(); // remove portedThreadsWithoutRangeGroup
- // group[0] is the file group
- // group[1] is the "a" group
+ // group[0] is the file group
+ // group[1] is the "a" group
- assert.equal(groups[2].type, GrDiffGroupType.BOTH);
- assert.equal(groups[2].lines.length, 5);
- for (const l of groups[2].lines) {
- assert.equal(l.text, 'all work and no play make jill a dull girl');
- }
- });
+ assert.equal(groups[2].type, GrDiffGroupType.BOTH);
+ assert.equal(groups[2].lines.length, 5);
+ for (const l of groups[2].lines) {
+ assert.equal(l.text, 'all work and no play make jill a dull girl');
+ }
});
});
test('in the middle with skip chunks', async () => {
options.context = 10;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{a: ['all work and no play make andybons a dull boy']},
{
@@ -510,8 +476,7 @@
{a: ['all work and no play make andybons a dull boy']},
];
- await processor.process(content);
-
+ const groups = await processor.process(content);
groups.shift(); // remove portedThreadsWithoutRangeGroup
// group[0] is the file group
@@ -547,7 +512,7 @@
test('works with skip === 0', async () => {
options.context = 3;
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{
skip: 0,
@@ -571,7 +536,7 @@
left: {1: true},
right: {10: true},
};
- processor = new GrDiffProcessor(consumer, options);
+ processor = new GrDiffProcessor(options);
const content = [
{
@@ -802,27 +767,20 @@
]);
});
- test('isScrolling paused', () => {
+ test('isScrolling paused', async () => {
const content = Array(200).fill({ab: ['', '']});
processor.isScrolling = true;
- processor.process(content);
- // Just the FILE and LOST groups.
- assert.equal(groups.length, 2);
- });
-
- test('isScrolling unpaused', () => {
- const content = Array(200).fill({ab: ['', '']});
+ const promise = processor.process(content);
processor.isScrolling = false;
- processor.process(content);
- // More groups have been processed. How many does not matter here.
+ const groups = await promise;
assert.isAtLeast(groups.length, 3);
});
- test('image diffs', () => {
+ test('image diffs', async () => {
const content = Array(200).fill({ab: ['', '']});
options.isBinary = true;
- processor = new GrDiffProcessor(consumer, options);
- processor.process(content);
+ processor = new GrDiffProcessor(options);
+ const groups = await processor.process(content);
assert.equal(groups.length, 2);
// Image diffs don't process content, just the 'FILE' line.
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
index f216e04..9cc6a90 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
@@ -123,6 +123,7 @@
test('asks for text for left side Elements', () => {
const getSelectedTextStub = sinon.stub(element, 'getSelectedText');
emulateCopyOn(diffTable.querySelector('div.contentText'));
+ assert.isTrue(getSelectedTextStub.called);
assert.deepEqual([Side.LEFT], getSelectedTextStub.lastCall.args);
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
index 771e298..f406215 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
@@ -6,9 +6,7 @@
import {BLANK_LINE, GrDiffLine} from './gr-diff-line';
import {GrDiffLineType, LineNumber, LineRange, Side} from '../../../api/diff';
import {assertIsDefined, assert} from '../../../utils/common-util';
-import {untilRendered} from '../../../utils/dom-util';
import {isDefined} from '../../../types/types';
-import {LitElement} from 'lit';
export enum GrDiffGroupType {
/** Unchanged context. */
@@ -318,11 +316,6 @@
*/
readonly keyLocation: boolean = false;
- /**
- * Once rendered the diff builder sets this to the diff section element.
- */
- element?: HTMLElement;
-
readonly lines: GrDiffLine[] = [];
readonly adds: GrDiffLine[] = [];
@@ -490,22 +483,6 @@
}
}
- async waitUntilRendered() {
- const lineNumber = this.lines[0]?.beforeNumber;
- // The LOST or FILE lines may be hidden and thus never resolve an
- // untilRendered() promise.
- if (
- this.skip !== undefined ||
- typeof lineNumber !== 'number' ||
- this.type === GrDiffGroupType.CONTEXT_CONTROL
- ) {
- return Promise.resolve();
- }
- assertIsDefined(this.element);
- await (this.element as LitElement).updateComplete;
- await untilRendered(this.element.firstElementChild as HTMLElement);
- }
-
/**
* Determines whether the group is either totally an addition or totally
* a removal.
@@ -517,4 +494,10 @@
!(!this.adds.length && !this.removes.length)
);
}
+
+ id() {
+ return `${this.type} ${this.startLine(Side.LEFT)} ${this.startLine(
+ Side.RIGHT
+ )}`;
+ }
}
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 0b688a6..cfb64b9 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
@@ -4,8 +4,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {BlameInfo, CommentRange} from '../../../types/common';
-import {Side} from '../../../constants/constants';
+import {Side, SpecialFilePath} from '../../../constants/constants';
import {
+ DiffContextExpandedExternalDetail,
DiffPreferencesInfo,
DiffResponsiveMode,
DisplayLine,
@@ -15,6 +16,7 @@
RenderPreferences,
} from '../../../api/diff';
import {getBaseUrl} from '../../../utils/url-util';
+import {GrDiffGroup} from './gr-diff-group';
/**
* In JS, unicode code points above 0xFFFF occupy two elements of a string.
@@ -227,6 +229,20 @@
return defaultContext;
}
+export function computeLineLength(
+ prefs: DiffPreferencesInfo,
+ path: string | undefined
+): number {
+ if (path === SpecialFilePath.COMMIT_MESSAGE) {
+ return 72;
+ }
+ const lineLength = prefs.line_length;
+ if (Number.isInteger(lineLength) && lineLength > 0) {
+ return lineLength;
+ }
+ return 100;
+}
+
export function computeKeyLocations(
lineOfInterest: DisplayLine | undefined,
comments: GrDiffCommentThread[]
@@ -474,3 +490,11 @@
return blameNode;
}
+
+export interface DiffContextExpandedEventDetail
+ extends DiffContextExpandedExternalDetail {
+ /** The context control group that should be replaced by `groups`. */
+ contextGroup: GrDiffGroup;
+ groups: GrDiffGroup[];
+ numLines: number;
+}
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 28c6c08..639b1ac 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
@@ -18,8 +18,10 @@
computeContext,
FULL_CONTEXT,
FullContext,
+ computeLineLength,
} from './gr-diff-utils';
import {FILE, LOST, Side} from '../../../api/diff';
+import {createDefaultDiffPrefs} from '../../../constants/constants';
const LINE_BREAK_HTML = '<span class="gr-diff br"></span>';
@@ -207,6 +209,35 @@
});
});
+ suite('computeLineLength', () => {
+ test('computeLineLength(1, ...)', () => {
+ assert.equal(
+ computeLineLength(
+ {...createDefaultDiffPrefs(), line_length: 1},
+ 'a.txt'
+ ),
+ 1
+ );
+ assert.equal(
+ computeLineLength(
+ {...createDefaultDiffPrefs(), line_length: 1},
+ undefined
+ ),
+ 1
+ );
+ });
+
+ test('computeLineLength(1, "/COMMIT_MSG")', () => {
+ assert.equal(
+ computeLineLength(
+ {...createDefaultDiffPrefs(), line_length: 1},
+ '/COMMIT_MSG'
+ ),
+ 72
+ );
+ });
+ });
+
suite('key locations', () => {
test('lineOfInterest is a key location', () => {
const lineOfInterest = {lineNum: 789, side: Side.LEFT};
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 54d578a..cc49b61 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -11,6 +11,9 @@
import '../gr-syntax-themes/gr-syntax-theme';
import '../gr-ranged-comment-themes/gr-ranged-comment-theme';
import '../gr-ranged-comment-hint/gr-ranged-comment-hint';
+import '../gr-diff-builder/gr-diff-builder-image';
+import '../gr-diff-builder/gr-diff-section';
+import '../gr-diff-builder/gr-diff-row';
import {
getLine,
getLineElByChild,
@@ -27,8 +30,9 @@
getSideByLineEl,
compareComments,
toCommentThreadModel,
- KeyLocations,
FullContext,
+ diffClasses,
+ DiffContextExpandedEventDetail,
} from '../gr-diff/gr-diff-utils';
import {BlameInfo, CommentRange, ImageInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
@@ -41,11 +45,11 @@
CommentRangeLayer,
GrRangedCommentLayer,
} from '../gr-ranged-comment-layer/gr-ranged-comment-layer';
-import {DiffViewMode, Side} from '../../../constants/constants';
import {
- GrDiffProcessor,
- ProcessingOptions,
-} from '../gr-diff-processor/gr-diff-processor';
+ DiffViewMode,
+ Side,
+ createDefaultDiffPrefs,
+} from '../../../constants/constants';
import {fire, fireAlert} from '../../../utils/event-util';
import {MovedLinkClickedEvent, ValueChangedEvent} from '../../../types/events';
import {getContentEditableRange} from '../../../utils/safari-selection-util';
@@ -56,16 +60,12 @@
DisplayLine,
LineNumber,
LOST,
+ ContentLoadNeededEventDetail,
} from '../../../api/diff';
import {isHtmlElement, isSafari, toggleClass} from '../../../utils/dom-util';
import {assertIsDefined} from '../../../utils/common-util';
-import {
- debounceP,
- DelayedPromise,
- DELAYED_CANCELLATION,
-} from '../../../utils/async-util';
import {GrDiffSelection} from '../gr-diff-selection/gr-diff-selection';
-import {property, query, state} from 'lit/decorators.js';
+import {property, query, queryAll, state} from 'lit/decorators.js';
import {sharedStyles} from '../../../styles/shared-styles';
import {html, LitElement, nothing, PropertyValues} from 'lit';
import {when} from 'lit/directives/when.js';
@@ -79,14 +79,6 @@
import {grDiffStyles} from './gr-diff-styles';
import {getDiffLength} from '../../../utils/diff-util';
import {GrCoverageLayer} from '../gr-coverage-layer/gr-coverage-layer';
-import {
- GrDiffBuilder,
- isImageDiffBuilder,
- isBinaryDiffBuilder,
- DiffContextExpandedEventDetail,
-} from '../gr-diff-builder/gr-diff-builder';
-import {GrDiffBuilderBinary} from '../gr-diff-builder/gr-diff-builder-binary';
-import {GrDiffBuilderImage} from '../gr-diff-builder/gr-diff-builder-image';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
import {
GrDiffGroup,
@@ -95,6 +87,9 @@
} from './gr-diff-group';
import {GrDiffLine} from './gr-diff-line';
import {subscribe} from '../../../elements/lit/subscription-controller';
+import {GrDiffSection} from '../gr-diff-builder/gr-diff-section';
+import {GrDiffRow} from '../gr-diff-builder/gr-diff-row';
+import {repeat} from 'lit/directives/repeat.js';
const TRAILING_WHITESPACE_PATTERN = /\s+$/;
@@ -151,6 +146,9 @@
@query('#diffTable')
diffTable?: HTMLTableElement;
+ @queryAll('gr-diff-section')
+ diffSections?: NodeListOf<GrDiffSection>;
+
@property({type: Boolean})
noAutoRender = false;
@@ -233,10 +231,6 @@
@property({type: Boolean})
override isContentEditable = isSafari();
- // Private but used in tests.
- @state()
- showWarning?: boolean;
-
@property({type: String})
errorMessage: string | null = null;
@@ -256,18 +250,9 @@
@state()
diffLength?: number;
- /**
- * Observes comment nodes added or removed at any point.
- * Can be used to unregister upon detachment.
- */
+ /** Observes comment nodes added or removed at any point. */
private nodeObserver?: MutationObserver;
- @property({type: Array})
- layers?: DiffLayer[];
-
- // Private but used in tests.
- renderDiffTableTask?: DelayedPromise<void>;
-
// Private but used in tests.
diffSelection = new GrDiffSelection();
@@ -276,44 +261,39 @@
private diffModel = new DiffModel();
- // visible for testing
- builder?: GrDiffBuilder;
+ /**
+ * Just the layers that are passed in from the outside. See `layersAll`
+ * for an array of all layers.
+ */
+ @property({type: Array})
+ layers: DiffLayer[] = [];
/**
- * All layers, both from the outside and the default ones. See `layers` for
- * the property that can be set from the outside.
+ * Just the internal default layers. See `layers` for the property that can
+ * be set from the outside.
*/
- // visible for testing
- layersInternal: DiffLayer[] = [];
+ @state() layersInternal: DiffLayer[] = [];
- // visible for testing
- showTabs?: boolean;
-
- // visible for testing
- showTrailingWhitespace?: boolean;
+ /**
+ * All layers, just combines `layers` and `layersInternal`.
+ */
+ @state() layersAll: DiffLayer[] = [];
private coverageLayerLeft = new GrCoverageLayer(Side.LEFT);
private coverageLayerRight = new GrCoverageLayer(Side.RIGHT);
- private rangeLayer?: GrRangedCommentLayer;
+ private rangeLayer = new GrRangedCommentLayer();
- // TODO: Remove. Let the model instantiate the processor.
- // visible for testing
- processor?: GrDiffProcessor;
+ @state() groups: GrDiffGroup[] = [];
- /**
- * Groups are mostly just passed on to the diff builder (this.builder). But
- * we also keep track of them here for being able to fire a `render-content`
- * event when .element of each group has rendered.
- */
- private groups: GrDiffGroup[] = [];
+ @state() private context = 3;
- // TODO: Can be removed when GrDiffProcessor is not instantiated anymore.
- private keyLocations: KeyLocations = {left: {}, right: {}};
-
- // TODO: Can be removed when GrDiffProcessor is not instantiated anymore.
- private context = 3;
+ private readonly layerUpdateListener: (
+ start: LineNumber,
+ end: LineNumber,
+ side: Side
+ ) => void;
static override get styles() {
return [
@@ -330,23 +310,32 @@
provide(this, diffModelToken, () => this.diffModel);
subscribe(
this,
- () => this.diffModel.keyLocations$,
- keyLocations => (this.keyLocations = keyLocations)
+ () => this.diffModel.context$,
+ context => (this.context = context)
);
subscribe(
this,
- () => this.diffModel.context$,
- context => (this.context = context)
+ () => this.diffModel.groups$,
+ groups => (this.groups = groups)
);
this.addEventListener(
'create-range-comment',
(e: CustomEvent<CreateRangeCommentEventDetail>) =>
this.handleCreateRangeComment(e)
);
- this.addEventListener('render-content', () => this.handleRenderContent());
this.addEventListener('moved-link-clicked', (e: MovedLinkClickedEvent) => {
this.dispatchSelectedLine(e.detail.lineNum, e.detail.side);
});
+ this.addEventListener(
+ 'diff-context-expanded-internal-new',
+ this.onDiffContextExpanded
+ );
+ this.layerUpdateListener = (
+ start: LineNumber,
+ end: LineNumber,
+ side: Side
+ ) => this.requestRowUpdates(start, end, side);
+ this.layersInternalInit();
}
override connectedCallback() {
@@ -360,15 +349,12 @@
if (this.diffTable) {
this.highlights.init(this.diffTable, this);
}
- this.diffBuilderInit();
}
override disconnectedCallback() {
this.removeSelectionListeners();
- this.renderDiffTableTask?.cancel();
this.diffSelection.cleanup();
this.highlights.cleanup();
- this.diffBuilderCleanup();
super.disconnectedCallback();
}
@@ -377,14 +363,19 @@
changedProperties.has('diff') ||
changedProperties.has('path') ||
changedProperties.has('renderPrefs') ||
+ changedProperties.has('viewMode') ||
changedProperties.has('prefs') ||
changedProperties.has('lineOfInterest')
) {
if (this.diff && this.prefs) {
+ const renderPrefs = {...(this.renderPrefs ?? {})};
+ if (renderPrefs.view_mode === undefined) {
+ renderPrefs.view_mode = this.viewMode;
+ }
this.diffModel.updateState({
diff: this.diff,
path: this.path,
- renderPrefs: this.renderPrefs ?? {},
+ renderPrefs,
diffPrefs: this.prefs,
lineOfInterest: this.lineOfInterest,
isImageDiff: this.isImageDiff,
@@ -400,6 +391,9 @@
) {
this.prefsChanged();
}
+ if (changedProperties.has('layers')) {
+ this.layersChanged();
+ }
if (changedProperties.has('blame')) {
this.blameChanged();
}
@@ -421,18 +415,37 @@
}
}
- protected override updated(changedProperties: PropertyValues<this>): void {
+ private async fireRenderContent() {
+ await this.updateComplete;
+ this.loading = false;
+ this.observeNodes();
+ // TODO: Retire one of these two events.
+ fire(this, 'render-content', {});
+ fire(this, 'render', {});
+ }
+
+ protected override async getUpdateComplete(): Promise<boolean> {
+ const result = await super.getUpdateComplete();
+ const sections = [...(this.diffSections ?? [])];
+ await Promise.all(sections.map(section => section.updateComplete));
+ return result;
+ }
+
+ protected override updated(changedProperties: PropertyValues<this>) {
if (changedProperties.has('diff')) {
- // diffChanged relies on diffTable ahving been rendered.
+ // diffChanged relies on diffTable having been rendered.
this.diffChanged();
}
+ if (changedProperties.has('groups')) {
+ if (this.groups?.length > 0) this.fireRenderContent();
+ }
}
override render() {
+ fire(this.diffTable, 'render-start', {});
return html`
${this.renderHeader()} ${this.renderContainer()}
${this.renderNewlineWarning()} ${this.renderLoadingError()}
- ${this.renderSizeWarning()}
`;
}
@@ -459,7 +472,19 @@
id="diffTable"
class=${this.diffTableClass}
?contenteditable=${this.isContentEditable}
- ></table>
+ >
+ ${this.renderColumns()}
+ ${when(!this.showWarning(), () =>
+ repeat(
+ this.groups,
+ group => group.id(),
+ group => this.renderSectionElement(group)
+ )
+ )}
+ ${when(this.diff?.binary, () =>
+ this.isImageDiff ? this.renderImageDiff() : this.renderBinaryDiff()
+ )}
+ </table>
${when(
this.showNoChangeMessage(),
() => html`
@@ -469,6 +494,7 @@
</div>
`
)}
+ ${when(this.showWarning(), () => this.renderSizeWarning())}
</div>
`;
}
@@ -485,7 +511,7 @@
}
private renderSizeWarning() {
- if (!this.showWarning) return nothing;
+ if (!this.showWarning()) return nothing;
// TODO: Update comment about 'Whole file' as it's not in settings.
return html`
<div id="sizeWarning">
@@ -596,7 +622,7 @@
});
}
- this.updateCommentRanges(this.commentRanges);
+ this.rangeLayer?.updateRanges(this.commentRanges);
}
// Dispatch events that are handled by the gr-diff-highlight.
@@ -612,11 +638,8 @@
});
}
- /** Cancel any remaining diff builder rendering work. */
- cancel() {
- this.diffBuilderCleanup();
- this.renderDiffTableTask?.cancel();
- }
+ /** TODO: Can be removed when diff-old is gone. */
+ cancel() {}
getCursorStops(): Array<HTMLElement | AbortStop> {
if (this.hidden && this.noAutoRender) return [];
@@ -641,7 +664,7 @@
}
private blameChanged() {
- this.setBlame(this.blame);
+ this.setBlame(this.blame ?? []);
if (this.blame) {
this.classList.add('showBlame');
} else {
@@ -759,22 +782,20 @@
this.unhideLine(lineNum, this.lineOfInterest.side);
}
- private cleanup() {
- this.cancel();
- this.blame = null;
- this.diffModel.updateState({showFullContext: FullContext.UNDECIDED});
- this.showWarning = false;
- this.clearDiffContent();
- }
-
private prefsChanged() {
if (!this.prefs) return;
this.blame = null;
this.updatePreferenceStyles();
- if (this.diff && !this.noRenderOnPrefsChange) {
- this.debounceRenderDiffTable();
+ if (!Number.isInteger(this.prefs.tab_size) || this.prefs.tab_size <= 0) {
+ this.handlePreferenceError('tab size');
+ }
+ if (
+ !Number.isInteger(this.prefs.line_length) ||
+ this.prefs.line_length <= 0
+ ) {
+ this.handlePreferenceError('diff width');
}
}
@@ -854,15 +875,12 @@
if (this.prefs) {
this.updatePreferenceStyles();
}
- this.updateRenderPrefs(this.renderPrefs);
}
private diffChanged() {
this.loading = true;
- this.cleanup();
if (this.diff) {
this.diffLength = this.getDiffLength(this.diff);
- this.debounceRenderDiffTable();
assertIsDefined(this.diffTable, 'diffTable');
this.diffSelection.init(this.diff, this.diffTable);
this.highlights.init(this.diffTable, this);
@@ -874,73 +892,23 @@
return getDiffLength(diff);
}
- /**
- * When called multiple times from the same task, will call
- * _renderDiffTable only once, in the next task (scheduled via `setTimeout`).
- *
- * This should be used instead of calling _renderDiffTable directly to
- * render the diff in response to an input change, because there may be
- * multiple inputs changing in the same microtask, but we only want to
- * render once.
- */
- private debounceRenderDiffTable() {
- // at this point gr-diff might be considered as rendered from the outside
- // (client), although it was not actually rendered. Clients need to know
- // when it is safe to perform operations like cursor moves, for example,
- // and if changing an input actually requires a reload of the diff table.
- // Since `fire` is synchronous it allows clients to be aware when an
- // async render is needed and that they can wait for a further `render`
- // event to actually take further action.
- fire(this, 'render-required', {});
- this.renderDiffTableTask = debounceP(
- this.renderDiffTableTask,
- async () => await this.renderDiffTable()
- );
- this.renderDiffTableTask.catch((e: unknown) => {
- if (e === DELAYED_CANCELLATION) return;
- throw e;
- });
- }
-
- // Private but used in tests.
- async renderDiffTable() {
- this.unobserveNodes();
- if (!this.diff || !this.prefs) {
- fire(this, 'render', {});
- return;
- }
- if (
- this.prefs.context === FULL_CONTEXT &&
+ private showWarning() {
+ return (
+ this.prefs?.context === FULL_CONTEXT &&
this.diffModel.getState().showFullContext === FullContext.UNDECIDED &&
this.diffLength &&
this.diffLength >= LARGE_DIFF_THRESHOLD_LINES
- ) {
- this.showWarning = true;
- fire(this, 'render', {});
- return;
- }
-
- this.showWarning = false;
-
- this.updateCommentRanges(this.commentRanges);
- this.updateCoverageRanges(this.coverageRanges);
- await this.legacyRender();
- }
-
- private handleRenderContent() {
- this.querySelectorAll('gr-ranged-comment-hint').forEach(element =>
- element.remove()
);
- this.loading = false;
- this.observeNodes();
- // We are just converting 'render-content' into 'render' here. Maybe we
- // should retire the 'render' event in favor of 'render-content'?
- fire(this, 'render', {});
}
+ /**
+ * This must be called once, but only after diff lines are rendered. Otherwise
+ * `processNodes()` will fail to lookup the HTML elements that it wants to
+ * manipulate.
+ */
private observeNodes() {
+ if (this.nodeObserver) return;
// First stop observing old nodes.
- this.unobserveNodes();
// Then introduce a Mutation observer that watches for children being added
// to gr-diff. If those children are `isThreadEl`, namely then they are
// processed.
@@ -1019,19 +987,6 @@
}
}
- private unobserveNodes() {
- if (this.nodeObserver) {
- this.nodeObserver.disconnect();
- this.nodeObserver = undefined;
- }
- // You only stop observing for comment thread elements when the diff is
- // completely rendered from scratch. And then comment thread elements
- // will be (re-)added *after* rendering is done. That is also when we
- // re-start observing. So it is appropriate to thoroughly clean up
- // everything that the observer is managing.
- this.commentRanges = [];
- }
-
private insertPortedCommentsWithoutRangeMessage(lostCell: Element) {
const existingMessage = lostCell.querySelector('div.lost-message');
if (existingMessage) return;
@@ -1047,11 +1002,8 @@
lostCell.insertBefore(div, lostCell.firstChild);
}
- clearDiffContent() {
- this.unobserveNodes();
- if (!this.diffTable) return;
- this.diffTable.innerHTML = '';
- }
+ /** TODO: Can be removed when diff-old is gone. */
+ clearDiffContent() {}
// Private but used in tests.
computeDiffHeaderItems() {
@@ -1071,13 +1023,10 @@
private handleFullBypass() {
this.diffModel.updateState({showFullContext: FullContext.YES});
- this.debounceRenderDiffTable();
}
private collapseContext() {
this.diffModel.updateState({showFullContext: FullContext.NO});
- // Uses the default context amount if the preference is for the entire file.
- this.debounceRenderDiffTable();
}
// TODO: Migrate callers to just update prefs.context.
@@ -1103,72 +1052,49 @@
return messages.join(' \u2014 '); // \u2014 - '—'
}
- private updateCommentRanges(ranges: CommentRangeLayer[]) {
- this.rangeLayer?.updateRanges(ranges);
- }
-
private updateCoverageRanges(rs: CoverageRange[]) {
this.coverageLayerLeft.setRanges(rs.filter(r => r?.side === Side.LEFT));
this.coverageLayerRight.setRanges(rs.filter(r => r?.side === Side.RIGHT));
}
- legacyRender(): Promise<void> {
- assertIsDefined(this.diff, 'diff');
- assertIsDefined(this.diffTable, 'diff table');
- assertIsDefined(this.prefs, 'prefs');
-
- // Setting up annotation layers must happen after plugins are
- // installed, and |render| satisfies the requirement, however,
- // |attached| doesn't because in the diff view page, the element is
- // attached before plugins are installed.
- this.setupAnnotationLayers();
-
- this.showTabs = this.prefs.show_tabs;
- this.showTrailingWhitespace = this.prefs.show_whitespace_errors;
-
- this.diffBuilderCleanup();
- this.builder = this.getDiffBuilder();
- this.diffBuilderInit();
-
- this.diffTable.innerHTML = '';
- this.builder.addColumns(this.diffTable, getLineNumberCellWidth(this.prefs));
-
- const options: ProcessingOptions = {
- context: this.context,
- keyLocations: this.keyLocations,
- isBinary: !!(this.isImageDiff || this.diff.binary),
- };
- if (this.renderPrefs?.num_lines_rendered_at_once) {
- options.asyncThreshold = this.renderPrefs.num_lines_rendered_at_once;
- }
- this.processor = new GrDiffProcessor(this, options);
-
- fire(this.diffTable, 'render-start', {});
- return (
- this.processor
- .process(this.diff.content)
- .then(async () => {
- if (isImageDiffBuilder(this.builder)) {
- this.builder.renderImageDiff();
- } else if (isBinaryDiffBuilder(this.builder)) {
- this.builder.renderBinaryDiff();
- }
- await this.untilGroupsRendered();
- fire(this.diffTable, 'render-content', {});
- })
- // Mocha testing does not like uncaught rejections, so we catch
- // the cancels which are expected and should not throw errors in
- // tests.
- .catch(e => {
- if (!e.isCanceled) return Promise.reject(e);
- return;
- })
+ public renderImageDiff() {
+ return when(
+ this.useNewImageDiffUi,
+ () => this.renderImageDiffNew(),
+ () => this.renderImageDiffOld()
);
}
- // visible for testing
- async untilGroupsRendered(groups: readonly GrDiffGroup[] = this.groups) {
- return Promise.all(groups.map(g => g.waitUntilRendered()));
+ private renderImageDiffNew() {
+ const autoBlink = !!this.renderPrefs?.image_diff_prefs?.automatic_blink;
+ return html`
+ <gr-diff-image-new
+ .automaticBlink=${autoBlink}
+ .baseImage=${this.baseImage ?? undefined}
+ .revisionImage=${this.revisionImage ?? undefined}
+ ></gr-diff-image-new>
+ `;
+ }
+
+ private renderImageDiffOld() {
+ return html`
+ <gr-diff-image-old
+ .baseImage=${this.baseImage ?? undefined}
+ .revisionImage=${this.revisionImage ?? undefined}
+ ></gr-diff-image-old>
+ `;
+ }
+
+ public renderBinaryDiff() {
+ return html`
+ <tbody class="gr-diff binary-diff">
+ <tr class="gr-diff">
+ <td colspan="5" class="gr-diff">
+ <span>Difference in binary files</span>
+ </td>
+ </tr>
+ </tbody>
+ `;
}
private onDiffContextExpanded = (
@@ -1176,14 +1102,19 @@
) => {
// Don't stop propagation. The host may listen for reporting or
// resizing.
- this.replaceGroup(e.detail.contextGroup, e.detail.groups);
+ this.diffModel.replaceGroup(e.detail.contextGroup, e.detail.groups);
};
- // visible for testing
- setupAnnotationLayers() {
- this.rangeLayer = new GrRangedCommentLayer();
+ private layersChanged() {
+ this.layersAll = [...this.layersInternal, ...this.layers];
+ for (const layer of this.layersAll) {
+ layer.removeListener?.(this.layerUpdateListener);
+ layer.addListener?.(this.layerUpdateListener);
+ }
+ }
- const layers: DiffLayer[] = [
+ private layersInternalInit() {
+ this.layersInternal = [
this.createTrailingWhitespaceLayer(),
this.createIntralineLayer(),
this.createTabIndicatorLayer(),
@@ -1192,16 +1123,7 @@
this.coverageLayerLeft,
this.coverageLayerRight,
];
-
- if (this.layers) {
- layers.push(...this.layers);
- }
- this.layersInternal = layers;
- }
-
- getContentTdByLine(lineNumber: LineNumber, side?: Side) {
- if (!this.builder) return undefined;
- return this.builder.getContentTdByLine(lineNumber, side);
+ this.layersChanged();
}
getContentTdByLineEl(lineEl?: Element): Element | undefined {
@@ -1212,21 +1134,6 @@
return this.getContentTdByLine(line, side);
}
- getLineElByNumber(lineNumber: LineNumber, side?: Side) {
- if (!this.builder) return undefined;
- return this.builder.getLineElByNumber(lineNumber, side);
- }
-
- getLineNumberRows() {
- if (!this.builder) return [];
- return this.builder.getLineNumberRows();
- }
-
- getLineNumEls(side: Side) {
- if (!this.builder) return [];
- return this.builder.getLineNumEls(side);
- }
-
/**
* When the line is hidden behind a context expander, expand it.
*
@@ -1237,8 +1144,7 @@
*/
unhideLine(lineNum: number, side: Side) {
assertIsDefined(this.prefs, 'prefs');
- if (!this.builder) return;
- const group = this.builder.findGroup(side, lineNum);
+ const group = this.findGroup(side, lineNum);
// Cannot unhide a line that is not part of the diff.
if (!group) return;
// If it's already visible, great!
@@ -1249,7 +1155,7 @@
const groups = hideInContextControl(
group.contextGroups,
0,
- lineOffset - 1 - this.prefs.context
+ lineOffset - 1 - this.context
);
// If there is a context group, it will be the first group because we
// start hiding from 0 offset
@@ -1259,68 +1165,14 @@
newGroups.push(
...hideInContextControl(
groups,
- lineOffset + 1 + this.prefs.context,
+ lineOffset + 1 + this.context,
// Both ends inclusive, so difference is the offset of the last line.
// But we need to pass the first line not to hide, which is the element
// after.
lineRange.end_line - lineRange.start_line + 1
)
);
- this.replaceGroup(group, newGroups);
- }
-
- /**
- * Replace the group of a context control section by rendering the provided
- * groups instead. This happens in response to expanding a context control
- * group.
- *
- * @param contextGroup The context control group to replace
- * @param newGroups The groups that are replacing the context control group
- */
- private replaceGroup(
- contextGroup: GrDiffGroup,
- newGroups: readonly GrDiffGroup[]
- ) {
- if (!this.builder) return;
- fire(this.diffTable, 'render-start', {});
- this.builder.replaceGroup(contextGroup, newGroups);
- this.groups = this.groups.filter(g => g !== contextGroup);
- this.groups.push(...newGroups);
- this.untilGroupsRendered(newGroups).then(() => {
- fire(this.diffTable, 'render-content', {});
- });
- }
-
- /**
- * This is meant to be called when the gr-diff component re-connects, or when
- * the diff is (re-)rendered.
- *
- * Make sure that this method is symmetric with cleanup(), which is called
- * when gr-diff disconnects.
- */
- private diffBuilderInit() {
- this.cleanup();
- this.diffTable?.addEventListener(
- 'diff-context-expanded-internal-new',
- this.onDiffContextExpanded
- );
- this.builder?.init();
- }
-
- /**
- * This is meant to be called when the gr-diff component disconnects, or when
- * the diff is (re-)rendered.
- *
- * Make sure that this method is symmetric with init(), which is called when
- * gr-diff re-connects.
- */
- private diffBuilderCleanup() {
- this.processor?.cancel();
- this.builder?.cleanup();
- this.diffTable?.removeEventListener(
- 'diff-context-expanded-internal-new',
- this.onDiffContextExpanded
- );
+ this.diffModel.replaceGroup(group, newGroups);
}
// visible for testing
@@ -1328,95 +1180,11 @@
const message =
`The value of the '${pref}' user preference is ` +
'invalid. Fix in diff preferences';
- assertIsDefined(this.diffTable, 'diff table');
- fireAlert(this.diffTable, message);
+ fireAlert(this, message);
throw Error(`Invalid preference value: ${pref}`);
}
// visible for testing
- getDiffBuilder(): GrDiffBuilder {
- assertIsDefined(this.diff, 'diff');
- assertIsDefined(this.diffTable, 'diff table');
- assertIsDefined(this.prefs, 'prefs');
- if (isNaN(this.prefs.tab_size) || this.prefs.tab_size <= 0) {
- this.handlePreferenceError('tab size');
- }
-
- if (isNaN(this.prefs.line_length) || this.prefs.line_length <= 0) {
- this.handlePreferenceError('diff width');
- }
-
- const localPrefs = {...this.prefs};
- if (this.path === COMMIT_MSG_PATH) {
- // override line_length for commit msg the same way as
- // in gr-diff
- localPrefs.line_length = COMMIT_MSG_LINE_LENGTH;
- }
-
- let builder = null;
- if (this.isImageDiff) {
- builder = new GrDiffBuilderImage(
- this.diff,
- localPrefs,
- this.diffTable,
- this.baseImage ?? null,
- this.revisionImage ?? null,
- this.renderPrefs,
- this.useNewImageDiffUi
- );
- } else if (this.diff.binary) {
- return new GrDiffBuilderBinary(this.diff, localPrefs, this.diffTable);
- } else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
- this.renderPrefs = {
- ...this.renderPrefs,
- view_mode: DiffViewMode.SIDE_BY_SIDE,
- };
- builder = new GrDiffBuilder(
- this.diff,
- localPrefs,
- this.diffTable,
- this.layersInternal,
- this.renderPrefs
- );
- } else if (this.viewMode === DiffViewMode.UNIFIED) {
- this.renderPrefs = {
- ...this.renderPrefs,
- view_mode: DiffViewMode.UNIFIED,
- };
- builder = new GrDiffBuilder(
- this.diff,
- localPrefs,
- this.diffTable,
- this.layersInternal,
- this.renderPrefs
- );
- }
- if (!builder) {
- throw Error(`Unsupported diff view mode: ${this.viewMode}`);
- }
- return builder;
- }
-
- /**
- * Called when the processor starts converting the diff information from the
- * server into chunks.
- */
- clearGroups() {
- if (!this.builder) return;
- this.groups = [];
- this.builder.clearGroups();
- }
-
- /**
- * Called when the processor is done converting a chunk of the diff.
- */
- addGroup(group: GrDiffGroup) {
- if (!this.builder) return;
- this.builder.addGroups([group]);
- this.groups.push(group);
- }
-
- // visible for testing
createIntralineLayer(): DiffLayer {
return {
// Take a DIV.contentText element and a line object with intraline
@@ -1451,15 +1219,10 @@
// visible for testing
createTabIndicatorLayer(): DiffLayer {
- const show = () => this.showTabs;
+ const show = () => this.prefs?.show_tabs;
return {
annotate(contentEl: HTMLElement, _: HTMLElement, line: GrDiffLine) {
- // If visible tabs are disabled, do nothing.
- if (!show()) {
- return;
- }
-
- // Find and annotate the locations of tabs.
+ if (!show()) return;
annotateSymbols(contentEl, line, '\t', 'tab-indicator');
},
};
@@ -1483,14 +1246,10 @@
// visible for testing
createTrailingWhitespaceLayer(): DiffLayer {
- const show = () => this.showTrailingWhitespace;
-
+ const show = () => this.prefs?.show_whitespace_errors;
return {
annotate(contentEl: HTMLElement, _: HTMLElement, line: GrDiffLine) {
- if (!show()) {
- return;
- }
-
+ if (!show()) return;
const match = line.text.match(TRAILING_WHITESPACE_PATTERN);
if (match) {
// Normalize string positions in case there is unicode before or
@@ -1510,13 +1269,167 @@
};
}
- setBlame(blame: BlameInfo[] | null) {
- if (!this.builder) return;
- this.builder.setBlame(blame ?? []);
+ getContentTdByLine(
+ lineNumber: LineNumber,
+ side?: Side
+ ): HTMLTableCellElement | undefined {
+ if (!side) return undefined;
+ const row = this.findRow(side, lineNumber);
+ return row?.getContentCell(side);
}
- updateRenderPrefs(renderPrefs: RenderPreferences) {
- this.builder?.updateRenderPrefs(renderPrefs);
+ getLineElByNumber(
+ lineNumber: LineNumber,
+ side?: Side
+ ): HTMLTableCellElement | undefined {
+ if (!side) return undefined;
+ const row = this.findRow(side, lineNumber);
+ return row?.getLineNumberCell(side);
+ }
+
+ private findRow(side: Side, lineNumber: LineNumber): GrDiffRow | undefined {
+ const group = this.findGroup(side, lineNumber);
+ if (!group) return undefined;
+ const section = this.findSection(group);
+ if (!section) return undefined;
+ return section.findRow(side, lineNumber);
+ }
+
+ private getDiffRows() {
+ assertIsDefined(this.diffTable, 'diffTable');
+ const sections = [
+ ...this.diffTable.querySelectorAll<GrDiffSection>('gr-diff-section'),
+ ];
+ return sections.map(s => s.getDiffRows()).flat();
+ }
+
+ getLineNumberRows(): HTMLTableRowElement[] {
+ const rows = this.getDiffRows();
+ return rows.map(r => r.getTableRow()).filter(isDefined);
+ }
+
+ getLineNumEls(side: Side): HTMLTableCellElement[] {
+ const rows = this.getDiffRows();
+ return rows.map(r => r.getLineNumberCell(side)).filter(isDefined);
+ }
+
+ /** This is used when layers initiate an update. */
+ private requestRowUpdates(start: LineNumber, end: LineNumber, side: Side) {
+ const groups = this.getGroupsByLineRange(start, end, side);
+ for (const group of groups) {
+ const section = this.findSection(group);
+ for (const row of section?.getDiffRows() ?? []) {
+ row.requestUpdate();
+ }
+ }
+ }
+
+ private findSection(group: GrDiffGroup): GrDiffSection | undefined {
+ assertIsDefined(this.diffTable, 'diffTable');
+ const leftClass = `left-${group.startLine(Side.LEFT)}`;
+ const rightClass = `right-${group.startLine(Side.RIGHT)}`;
+ return (
+ this.diffTable.querySelector<GrDiffSection>(
+ `gr-diff-section.${leftClass}.${rightClass}`
+ ) ?? undefined
+ );
+ }
+
+ renderSectionElement(group: GrDiffGroup) {
+ const leftClass = `left-${group.startLine(Side.LEFT)}`;
+ const rightClass = `right-${group.startLine(Side.RIGHT)}`;
+ if (this.diff?.binary && group.startLine(Side.LEFT) === LOST) {
+ return nothing;
+ }
+ return html`
+ <gr-diff-section
+ class="${leftClass} ${rightClass}"
+ .group=${group}
+ .diff=${this.diff}
+ .layers=${this.layersAll}
+ .diffPrefs=${this.prefs}
+ .renderPrefs=${this.renderPrefs}
+ ></gr-diff-section>
+ `;
+ }
+
+ renderColumns() {
+ const lineNumberWidth = getLineNumberCellWidth(
+ this.prefs ?? createDefaultDiffPrefs()
+ );
+ return html`
+ <colgroup>
+ <col class=${diffClasses('blame')}></col>
+ ${when(
+ (this.renderPrefs?.view_mode ?? this.viewMode) ===
+ DiffViewMode.UNIFIED,
+ () => html` ${this.renderUnifiedColumns(lineNumberWidth)} `,
+ () => html`
+ ${this.renderSideBySideColumns(Side.LEFT, lineNumberWidth)}
+ ${this.renderSideBySideColumns(Side.RIGHT, lineNumberWidth)}
+ `
+ )}
+ </colgroup>
+ `;
+ }
+
+ private renderUnifiedColumns(lineNumberWidth: number) {
+ return html`
+ <col class=${diffClasses()} width=${lineNumberWidth}></col>
+ <col class=${diffClasses()} width=${lineNumberWidth}></col>
+ <col class=${diffClasses()}></col>
+ `;
+ }
+
+ private renderSideBySideColumns(side: Side, lineNumberWidth: number) {
+ return html`
+ <col class=${diffClasses(side)} width=${lineNumberWidth}></col>
+ <col class=${diffClasses(side, 'sign')}></col>
+ <col class=${diffClasses(side)}></col>
+ `;
+ }
+
+ findGroup(side: Side, line: LineNumber) {
+ return this.groups.find(group => group.containsLine(side, line));
+ }
+
+ // visible for testing
+ getGroupsByLineRange(
+ startLine: LineNumber,
+ endLine: LineNumber,
+ side: Side
+ ): GrDiffGroup[] {
+ const startIndex = this.groups.findIndex(group =>
+ group.containsLine(side, startLine)
+ );
+ if (startIndex === -1) return [];
+ let endIndex = this.groups.findIndex(group =>
+ group.containsLine(side, endLine)
+ );
+ // Not all groups may have been processed yet (i.e. this.groups is still
+ // incomplete). In that case let's just return *all* groups until the end
+ // of the array.
+ if (endIndex === -1) endIndex = this.groups.length - 1;
+ // The filter preserves the legacy behavior to only return non-context
+ // groups
+ return this.groups
+ .slice(startIndex, endIndex + 1)
+ .filter(group => group.lines.length > 0);
+ }
+
+ /**
+ * Set the blame information for the diff. For any already-rendered line,
+ * re-render its blame cell content.
+ */
+ setBlame(blame: BlameInfo[]) {
+ for (const blameInfo of blame) {
+ for (const range of blameInfo.ranges) {
+ for (let line = range.start; line <= range.end; line++) {
+ const row = this.findRow(Side.LEFT, line);
+ if (row) row.blameInfo = blameInfo;
+ }
+ }
+ }
}
}
@@ -1577,5 +1490,7 @@
* renders and for partial rerenders.
*/
'render-content': CustomEvent<{}>;
+ 'diff-context-expanded-internal-new': CustomEvent<DiffContextExpandedEventDetail>;
+ 'content-load-needed': CustomEvent<ContentLoadNeededEventDetail>;
}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index 5603edb..8806dbf 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -30,7 +30,6 @@
queryAndAssert,
stubBaseUrl,
stubRestApi,
- waitEventLoop,
waitQueryAndAssert,
waitUntil,
} from '../../../test/test-utils';
@@ -39,10 +38,8 @@
import {GrDiff} from './gr-diff';
import {ImageInfo} from '../../../types/common';
import {GrRangedCommentHint} from '../gr-ranged-comment-hint/gr-ranged-comment-hint';
-import {assertIsDefined} from '../../../utils/common-util';
import {fixture, html, assert} from '@open-wc/testing';
import {createDefaultDiffPrefs} from '../../../constants/constants';
-import {GrDiffBuilder} from '../gr-diff-builder/gr-diff-builder';
import {GrDiffRow} from '../gr-diff-builder/gr-diff-row';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
import {GrDiffLine} from './gr-diff-line';
@@ -77,7 +74,17 @@
element,
/* HTML */ `
<div class="diffContainer sideBySide">
- <table id="diffTable"></table>
+ <table id="diffTable">
+ <colgroup>
+ <col class="blame gr-diff" />
+ <col class="gr-diff left" width="48" />
+ <col class="gr-diff left sign" />
+ <col class="gr-diff left" />
+ <col class="gr-diff right" width="48" />
+ <col class="gr-diff right sign" />
+ <col class="gr-diff right" />
+ </colgroup>
+ </table>
</div>
`
);
@@ -3163,7 +3170,6 @@
<col class="gr-diff right sign" />
<col class="gr-diff right" />
</colgroup>
- <tbody class="binary-diff gr-diff"></tbody>
<tbody class="both gr-diff section">
<tr
aria-labelledby="left-button-FILE left-content-FILE right-button-FILE right-content-FILE"
@@ -3305,13 +3311,13 @@
<td class="blank gr-diff left lineNum"></td>
<td class="gr-diff left">
<label class="gr-diff">
- <span class="gr-diff label"> image/bmp </span>
+ <span class="gr-diff label"> 1×1 image/bmp </span>
</label>
</td>
<td class="blank gr-diff lineNum right"></td>
<td class="gr-diff right">
<label class="gr-diff">
- <span class="gr-diff label"> image/bmp </span>
+ <span class="gr-diff label"> 1×1 image/bmp </span>
</label>
</td>
</tr>
@@ -3369,7 +3375,7 @@
<label class="gr-diff">
<span class="gr-diff name"> carrot.jpg </span>
<br class="gr-diff" />
- <span class="gr-diff label"> image/bmp </span>
+ <span class="gr-diff label"> 1×1 image/bmp </span>
</label>
`
);
@@ -3379,7 +3385,7 @@
<label class="gr-diff">
<span class="gr-diff name"> carrot2.jpg </span>
<br class="gr-diff" />
- <span class="gr-diff label"> image/bmp </span>
+ <span class="gr-diff label"> 1×1 image/bmp </span>
</label>
`
);
@@ -3538,7 +3544,6 @@
ignore_whitespace: 'IGNORE_NONE',
};
await element.updateComplete;
- element.renderDiffTable();
}
test('returns [] when hidden and noAutoRender', async () => {
@@ -3554,6 +3559,7 @@
test('returns one stop per line and one for the file row', async () => {
await setupDiff();
element.loading = false;
+ await waitUntil(() => element.groups.length > 2);
await element.updateComplete;
const ROWS = 48;
const FILE_ROW = 1;
@@ -3567,10 +3573,12 @@
test('returns an additional AbortStop when still loading', async () => {
await setupDiff();
element.loading = true;
+ await waitUntil(() => element.groups.length > 2);
await element.updateComplete;
const ROWS = 48;
const FILE_ROW = 1;
const LOST_ROW = 1;
+ element.loading = true;
const actual = element.getCursorStops();
assert.equal(actual.length, ROWS + FILE_ROW + LOST_ROW + 1);
assert.isTrue(actual[actual.length - 1] instanceof AbortStop);
@@ -3694,67 +3702,6 @@
assert.isEmpty(element.querySelectorAll('gr-ranged-comment-hint'));
});
-
- suite('change in preferences', () => {
- setup(async () => {
- element.diff = {
- meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
- meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 560},
- diff_header: [],
- intraline_status: 'OK',
- change_type: 'MODIFIED',
- content: [{skip: 66}],
- };
- await element.updateComplete;
- await element.renderDiffTableTask?.flush();
- });
-
- test('change in preferences re-renders diff', async () => {
- const stub = sinon.stub(element, 'renderDiffTable');
- element.prefs = {
- ...MINIMAL_PREFS,
- };
- await element.updateComplete;
- await element.renderDiffTableTask?.flush();
- assert.isTrue(stub.called);
- });
-
- test('adding/removing property in preferences re-renders diff', async () => {
- const stub = sinon.stub(element, 'renderDiffTable');
- const newPrefs1: DiffPreferencesInfo = {
- ...MINIMAL_PREFS,
- line_wrapping: true,
- };
- element.prefs = newPrefs1;
- await element.updateComplete;
- await element.renderDiffTableTask?.flush();
- assert.isTrue(stub.called);
- stub.reset();
-
- const newPrefs2 = {...newPrefs1};
- delete newPrefs2.line_wrapping;
- element.prefs = newPrefs2;
- await element.updateComplete;
- await element.renderDiffTableTask?.flush();
- assert.isTrue(stub.called);
- });
-
- test(
- 'change in preferences does not re-renders diff with ' +
- 'noRenderOnPrefsChange',
- async () => {
- const stub = sinon.stub(element, 'renderDiffTable');
- element.noRenderOnPrefsChange = true;
- element.prefs = {
- ...MINIMAL_PREFS,
- context: 12,
- };
- await element.updateComplete;
- await element.renderDiffTableTask?.flush();
- assert.isFalse(stub.called);
- }
- );
- });
});
suite('diff header', () => {
@@ -3808,7 +3755,7 @@
element.classList.add('showBlame');
element.blame = null;
await element.updateComplete;
- assert.isTrue(setBlameSpy.calledWithExactly(null));
+ assert.isTrue(setBlameSpy.calledWithExactly([]));
assert.isFalse(element.classList.contains('showBlame'));
});
@@ -3916,36 +3863,10 @@
content,
binary,
};
+ await waitUntil(() => element.groups.length > 1);
await element.updateComplete;
- await element.renderDiffTableTask;
};
- test('clear diff table content as soon as diff changes', async () => {
- const content = [
- {
- a: ['all work and no play make andybons a dull boy'],
- },
- {
- b: ['Non eram nescius, Brute, cum, quae summis ingeniis '],
- },
- ];
- function diffTableHasContent() {
- assertIsDefined(element.diffTable);
- return element.diffTable.innerText.includes(content[0].a?.[0] ?? '');
- }
- await setupSampleDiff({content});
- await waitUntil(diffTableHasContent);
- element.diff = {...element.diff!};
- await element.updateComplete;
- // immediately cleaned up
- assertIsDefined(element.diffTable);
- assert.equal(element.diffTable.innerHTML, '');
- element.renderDiffTable();
- await element.updateComplete;
- // rendered again
- await waitUntil(diffTableHasContent);
- });
-
suite('selection test', () => {
test('user-select set correctly on side-by-side view', async () => {
const content = [
@@ -3961,8 +3882,9 @@
},
];
await setupSampleDiff({content});
- await waitEventLoop();
+ // We are selecting "Non eram nescius..." on the left side.
+ // The default is `selected-right`, so we will have to click.
const diffLine = queryAll<HTMLElement>(element, '.contentText')[2];
assert.equal(getComputedStyle(diffLine).userSelect, 'none');
mouseDown(diffLine);
@@ -3982,10 +3904,12 @@
],
},
];
- await setupSampleDiff({content});
element.viewMode = DiffViewMode.UNIFIED;
- await element.updateComplete;
- const diffLine = queryAll<HTMLElement>(element, '.contentText')[2];
+ await setupSampleDiff({content});
+
+ // We are selecting "all work and no play..." on the left side.
+ // The default is `selected-right`, so we will have to click.
+ const diffLine = queryAll<HTMLElement>(element, '.contentText')[0];
assert.equal(getComputedStyle(diffLine).userSelect, 'none');
mouseDown(diffLine);
assert.equal(getComputedStyle(diffLine).userSelect, 'text');
@@ -4057,16 +3981,6 @@
suite('former gr-diff-builder tests', () => {
let element: GrDiff;
- let builder: GrDiffBuilder;
- let diffTable: HTMLTableElement;
-
- const setBuilderPrefs = (prefs: Partial<DiffPreferencesInfo>) => {
- builder = new GrDiffBuilder(
- createEmptyDiff(),
- {...createDefaultDiffPrefs(), ...prefs},
- diffTable
- );
- };
const line = (text: string) => {
const line = new GrDiffLine(GrDiffLineType.BOTH);
@@ -4081,51 +3995,6 @@
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
stubRestApi('getProjectConfig').returns(Promise.resolve(createConfig()));
stubBaseUrl('/r');
- setBuilderPrefs({});
- });
-
- [DiffViewMode.UNIFIED, DiffViewMode.SIDE_BY_SIDE].forEach(mode => {
- test(`line_length used for regular files under ${mode}`, () => {
- element.path = '/a.txt';
- element.viewMode = mode;
- element.diff = createEmptyDiff();
- element.prefs = {
- ...createDefaultDiffPrefs(),
- tab_size: 4,
- line_length: 50,
- };
- builder = element.getDiffBuilder();
- assert.equal(builder.prefs.line_length, 50);
- });
-
- test(`line_length ignored for commit msg under ${mode}`, () => {
- element.path = '/COMMIT_MSG';
- element.viewMode = mode;
- element.diff = createEmptyDiff();
- element.prefs = {
- ...createDefaultDiffPrefs(),
- tab_size: 4,
- line_length: 50,
- };
- builder = element.getDiffBuilder();
- assert.equal(builder.prefs.line_length, 72);
- });
- });
-
- test('_handlePreferenceError throws with invalid preference', () => {
- element.prefs = {...createDefaultDiffPrefs(), tab_size: 0};
- assert.throws(() => element.getDiffBuilder());
- });
-
- test('_handlePreferenceError triggers alert and javascript error', () => {
- const errorStub = sinon.stub();
- element.diffTable!.addEventListener('show-alert', errorStub);
- assert.throws(() => element.handlePreferenceError('tab size'));
- assert.equal(
- errorStub.lastCall.args[0].detail.message,
- "The value of the 'tab size' user preference is invalid. " +
- 'Fix in diff preferences'
- );
});
suite('intraline differences', () => {
@@ -4276,7 +4145,7 @@
const lineNumberEl = document.createElement('td');
setup(() => {
- element.showTabs = true;
+ element.prefs = {...DEFAULT_PREFS, show_tabs: true};
layer = element.createTabIndicatorLayer();
});
@@ -4320,7 +4189,7 @@
});
test('does not annotate when disabled', () => {
- element.showTabs = false;
+ element.prefs = {...DEFAULT_PREFS, show_tabs: false};
const str = '\tlorem upsum';
const l = line(str);
@@ -4375,44 +4244,15 @@
});
});
- suite('layers', () => {
- let initialLayersCount = 0;
- let withLayerCount = 0;
- setup(() => {
- const layers: DiffLayer[] = [];
- element.layers = layers;
- element.showTrailingWhitespace = true;
- element.setupAnnotationLayers();
- initialLayersCount = element.layersInternal.length;
- });
-
- test('no layers', () => {
- element.setupAnnotationLayers();
- assert.equal(element.layersInternal.length, initialLayersCount);
- });
-
- suite('with layers', () => {
- const layers: DiffLayer[] = [{annotate: () => {}}, {annotate: () => {}}];
- setup(() => {
- element.layers = layers;
- element.showTrailingWhitespace = true;
- element.setupAnnotationLayers();
- withLayerCount = element.layersInternal.length;
- });
- test('with layers', () => {
- element.setupAnnotationLayers();
- assert.equal(element.layersInternal.length, withLayerCount);
- assert.equal(initialLayersCount + layers.length, withLayerCount);
- });
- });
- });
-
suite('trailing whitespace', () => {
let layer: DiffLayer;
const lineNumberEl = document.createElement('td');
setup(() => {
- element.showTrailingWhitespace = true;
+ element.prefs = {
+ ...createDefaultDiffPrefs(),
+ show_whitespace_errors: true,
+ };
layer = element.createTrailingWhitespaceLayer();
});
@@ -4483,7 +4323,10 @@
});
test('does not annotate when disabled', () => {
- element.showTrailingWhitespace = false;
+ element.prefs = {
+ ...createDefaultDiffPrefs(),
+ show_whitespace_errors: false,
+ };
const str = 'lorem upsum\t \t ';
const l = line(str);
const el = document.createElement('div');
@@ -4520,29 +4363,47 @@
test('text', async () => {
element.diff = {...createEmptyDiff(), content};
- await waitForEventOnce(element.diffTable!, 'render-content');
- assert.equal(querySelectorAll(element.diffTable!, 'tbody')?.length, 4);
+ await waitUntil(() => element.groups.length > 2);
+ await element.updateComplete;
+ const bodies = [...(querySelectorAll(element.diffTable!, 'tbody') ?? [])];
+ assert.equal(bodies.length, 4);
+ assert.isTrue(bodies[0].innerHTML.includes('LOST'));
+ assert.isTrue(bodies[1].innerHTML.includes('FILE'));
+ assert.isTrue(bodies[2].innerHTML.includes('andybons a dull boy'));
+ assert.isTrue(bodies[3].innerHTML.includes('Non eram nescius'));
});
test('image', async () => {
element.diff = {...createEmptyDiff(), content, binary: true};
element.isImageDiff = true;
- await waitForEventOnce(element.diffTable!, 'render-content');
- assert.equal(querySelectorAll(element.diffTable!, 'tbody')?.length, 4);
+ await element.updateComplete;
+ const body = queryAndAssert(element, 'tbody.image-diff');
+ assert.lightDom.equal(
+ body,
+ /* HTML */ `
+ <label class="gr-diff">
+ <span class="gr-diff label"> No image </span>
+ </label>
+ <label class="gr-diff">
+ <span class="gr-diff label"> No image </span>
+ </label>
+ `
+ );
});
test('binary', async () => {
element.diff = {...createEmptyDiff(), content, binary: true};
- await waitForEventOnce(element.diffTable!, 'render-content');
- assert.equal(querySelectorAll(element.diffTable!, 'tbody')?.length, 3);
+ await element.updateComplete;
+ const body = queryAndAssert(element, 'tbody.binary-diff');
+ assert.lightDom.equal(
+ body,
+ /* HTML */ '<span>Difference in binary files</span>'
+ );
});
});
suite('context hiding and expanding', () => {
- let dispatchStub: sinon.SinonStub;
-
setup(async () => {
- dispatchStub = sinon.stub(element.diffTable!, 'dispatchEvent');
element.diff = {
...createEmptyDiff(),
content: [
@@ -4552,15 +4413,12 @@
],
};
element.viewMode = DiffViewMode.SIDE_BY_SIDE;
-
element.prefs = {
...DEFAULT_PREFS,
context: 1,
};
+ await waitUntil(() => element.groups.length > 2);
await element.updateComplete;
- element.legacyRender();
- // Make sure all listeners are installed.
- await element.untilGroupsRendered();
});
test('hides lines behind two context controls', () => {
@@ -4617,7 +4475,6 @@
});
test('unhideLine shows the line with context', async () => {
- dispatchStub.reset();
element.unhideLine(4, Side.LEFT);
await waitUntil(() => {
@@ -4642,10 +4499,6 @@
assert.include(diffRows[8].textContent, 'before');
assert.include(diffRows[8].textContent, 'after');
assert.include(diffRows[9].textContent, 'unchanged 11');
-
- await element.untilGroupsRendered();
- const firedEventTypes = dispatchStub.getCalls().map(c => c.args[0].type);
- assert.include(firedEventTypes, 'render-content');
});
});
});
diff --git a/polygerrit-ui/app/test/test-app-context-init.ts b/polygerrit-ui/app/test/test-app-context-init.ts
index 026e3b5..73ee6aa 100644
--- a/polygerrit-ui/app/test/test-app-context-init.ts
+++ b/polygerrit-ui/app/test/test-app-context-init.ts
@@ -18,6 +18,10 @@
import {DependencyToken} from '../models/dependency';
import {storageServiceToken} from '../services/storage/gr-storage_impl';
import {highlightServiceToken} from '../services/highlight/highlight-service';
+import {
+ diffModelToken,
+ DiffModel,
+} from '../embed/diff/gr-diff-model/gr-diff-model';
export function createTestAppContext(): AppContext & Finalizable {
const appRegistry: Registry<AppContext> = {
@@ -49,5 +53,6 @@
highlightServiceToken,
() => new MockHighlightService(appContext.reportingService)
);
+ dependencies.set(diffModelToken, () => new DiffModel());
return dependencies;
}
