Merge "Fix name of `removeLabel` permission"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index e527633..737e964 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -1168,7 +1168,7 @@
});
test(
- '_prefs.manual_review true means set reviewed is not ' +
+ 'prefs.manual_review true means set reviewed is not ' +
'automatically called',
async () => {
const setReviewedFileStatusStub = sinon
@@ -1204,7 +1204,7 @@
}
);
- test('_prefs.manual_review false means set reviewed is called', async () => {
+ test('prefs.manual_review false means set reviewed is called', async () => {
const setReviewedFileStatusStub = sinon
.stub(changeModel, 'setReviewedFilesStatus')
.callsFake(() => Promise.resolve());
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
index 502c6cf..cc45e1e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
@@ -3,17 +3,13 @@
* Copyright 2017 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {GrDiffBuilderLit} from './gr-diff-builder-lit';
+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 {BinaryDiffBuilder} from './gr-diff-builder';
-export class GrDiffBuilderBinary
- extends GrDiffBuilderLit
- implements BinaryDiffBuilder
-{
+export class GrDiffBuilderBinary extends GrDiffBuilder {
constructor(
diff: DiffInfo,
prefs: DiffPreferencesInfo,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
index 682e747..396e9e2 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -7,15 +7,13 @@
import '../../../elements/shared/gr-hovercard/gr-hovercard';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
import {
- DiffBuilder,
- ImageDiffBuilder,
+ GrDiffBuilder,
DiffContextExpandedEventDetail,
isImageDiffBuilder,
isBinaryDiffBuilder,
} from './gr-diff-builder';
import {GrDiffBuilderImage} from './gr-diff-builder-image';
import {GrDiffBuilderBinary} from './gr-diff-builder-binary';
-import {GrDiffBuilderLit} from './gr-diff-builder-lit';
import {CancelablePromise, makeCancelable} from '../../../utils/async-util';
import {BlameInfo, ImageInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
@@ -117,7 +115,7 @@
layers: DiffLayer[] = [];
// visible for testing
- builder?: DiffBuilder | ImageDiffBuilder;
+ builder?: GrDiffBuilder;
/**
* All layers, both from the outside and the default ones. See `layers` for
@@ -263,31 +261,21 @@
this.layersInternal = layers;
}
- getContentTdByLine(lineNumber: LineNumber, side?: Side, root?: Element) {
- if (!this.builder) return null;
- return this.builder.getContentTdByLine(lineNumber, side, root);
+ getContentTdByLine(lineNumber: LineNumber, side?: Side) {
+ if (!this.builder) return undefined;
+ return this.builder.getContentTdByLine(lineNumber, side);
}
- private getDiffRowByChild(child: Element) {
- while (!child.classList.contains('diff-row') && child.parentElement) {
- child = child.parentElement;
- }
- return child;
- }
-
- getContentTdByLineEl(lineEl?: Element): Element | null {
- if (!lineEl) return null;
+ getContentTdByLineEl(lineEl?: Element): Element | undefined {
+ if (!lineEl) return undefined;
const line = getLineNumber(lineEl);
- if (!line) return null;
+ if (!line) return undefined;
const side = getSideByLineEl(lineEl);
- // Performance optimization because we already have an element in the
- // correct row
- const row = this.getDiffRowByChild(lineEl);
- return this.getContentTdByLine(line, side, row);
+ return this.getContentTdByLine(line, side);
}
getLineElByNumber(lineNumber: LineNumber, side?: Side) {
- if (!this.builder) return null;
+ if (!this.builder) return undefined;
return this.builder.getLineElByNumber(lineNumber, side);
}
@@ -409,7 +397,7 @@
}
// visible for testing
- getDiffBuilder(): DiffBuilder {
+ getDiffBuilder(): GrDiffBuilder {
assertIsDefined(this.diff, 'diff');
assertIsDefined(this.diffElement, 'diff table');
if (isNaN(this.prefs.tab_size) || this.prefs.tab_size <= 0) {
@@ -445,7 +433,7 @@
...this.renderPrefs,
view_mode: DiffViewMode.SIDE_BY_SIDE,
};
- builder = new GrDiffBuilderLit(
+ builder = new GrDiffBuilder(
this.diff,
localPrefs,
this.diffElement,
@@ -457,7 +445,7 @@
...this.renderPrefs,
view_mode: DiffViewMode.UNIFIED,
};
- builder = new GrDiffBuilderLit(
+ builder = new GrDiffBuilder(
this.diff,
localPrefs,
this.diffElement,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
index d2147d1..d776164 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
@@ -26,17 +26,17 @@
import {KeyLocations} from '../gr-diff-processor/gr-diff-processor';
import {fixture, html, assert} from '@open-wc/testing';
import {GrDiffRow} from './gr-diff-row';
-import {GrDiffBuilderLit} from './gr-diff-builder-lit';
+import {GrDiffBuilder} from './gr-diff-builder';
const DEFAULT_PREFS = createDefaultDiffPrefs();
suite('gr-diff-builder tests', () => {
let element: GrDiffBuilderElement;
- let builder: GrDiffBuilderLit;
+ let builder: GrDiffBuilder;
let diffTable: HTMLTableElement;
const setBuilderPrefs = (prefs: Partial<DiffPreferencesInfo>) => {
- builder = new GrDiffBuilderLit(
+ builder = new GrDiffBuilder(
createEmptyDiff(),
{...createDefaultDiffPrefs(), ...prefs},
diffTable
@@ -69,8 +69,8 @@
tab_size: 4,
line_length: 50,
};
- builder = element.getDiffBuilder() as GrDiffBuilderLit;
- assert.equal(builder._prefs.line_length, 50);
+ builder = element.getDiffBuilder();
+ assert.equal(builder.prefs.line_length, 50);
});
test(`line_length ignored for commit msg under ${mode}`, () => {
@@ -82,8 +82,8 @@
tab_size: 4,
line_length: 50,
};
- builder = element.getDiffBuilder() as GrDiffBuilderLit;
- assert.equal(builder._prefs.line_length, 72);
+ builder = element.getDiffBuilder();
+ assert.equal(builder.prefs.line_length, 72);
});
});
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 f0cc69f..3bffe08 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
@@ -7,19 +7,15 @@
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {RenderPreferences, Side} from '../../../api/diff';
import '../gr-diff-image-viewer/gr-image-viewer';
-import {ImageDiffBuilder} from './gr-diff-builder';
import {html, LitElement, nothing} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
-import {GrDiffBuilderLit} from './gr-diff-builder-lit';
+import {GrDiffBuilder} from './gr-diff-builder';
// 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 GrDiffBuilderLit
- implements ImageDiffBuilder
-{
+export class GrDiffBuilderImage extends GrDiffBuilder {
constructor(
diff: DiffInfo,
prefs: DiffPreferencesInfo,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
deleted file mode 100644
index c834afc..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
+++ /dev/null
@@ -1,214 +0,0 @@
-/**
- * @license
- * Copyright 2022 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {DiffViewMode, RenderPreferences} from '../../../api/diff';
-import {LineNumber} from '../gr-diff/gr-diff-line';
-import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {Side} from '../../../constants/constants';
-import {DiffLayer, isDefined} from '../../../types/types';
-import {diffClasses} from '../gr-diff/gr-diff-utils';
-import {GrDiffBuilder} from './gr-diff-builder';
-import {BlameInfo} from '../../../types/common';
-import {html, nothing, render} from 'lit';
-import {GrDiffSection} from './gr-diff-section';
-import '../gr-context-controls/gr-context-controls';
-import './gr-diff-section';
-import {GrDiffRow} from './gr-diff-row';
-
-/**
- * Base class for builders that are creating the diff using Lit elements.
- */
-export class GrDiffBuilderLit extends GrDiffBuilder {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- layers: DiffLayer[] = [],
- renderPrefs?: RenderPreferences
- ) {
- super(diff, prefs, outputEl, layers, renderPrefs);
- }
-
- override getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- _root: Element = this.outputEl
- ): HTMLTableCellElement | null {
- if (!side) return null;
- const row = this.findRow(lineNumber, side);
- return row?.getContentCell(side) ?? null;
- }
-
- override getLineElByNumber(lineNumber: LineNumber, side: Side) {
- const row = this.findRow(lineNumber, side);
- return row?.getLineNumberCell(side) ?? null;
- }
-
- 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();
- }
-
- override getLineNumberRows(): HTMLTableRowElement[] {
- const rows = this.getDiffRows();
- return rows.map(r => r.getTableRow()).filter(isDefined);
- }
-
- override getLineNumEls(side: Side): HTMLTableCellElement[] {
- const rows = this.getDiffRows();
- return rows.map(r => r.getLineNumberCell(side)).filter(isDefined);
- }
-
- override getBlameTdByLine(lineNumber: number): Element | undefined {
- return this.findRow(lineNumber, Side.LEFT)?.getBlameCell();
- }
-
- override getContentByLine(
- lineNumber: LineNumber,
- side?: Side,
- _root?: HTMLElement
- ): HTMLElement | null {
- const cell = this.getContentTdByLine(lineNumber, side);
- return (cell?.firstChild ?? null) as HTMLElement | null;
- }
-
- /** This is used when layers initiate an update. */
- override 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 {
- if (!group) return 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
- );
- }
-
- override renderBlameByRange(
- blameInfo: BlameInfo,
- start: number,
- end: number
- ) {
- for (let lineNumber = start; lineNumber <= end; lineNumber++) {
- const row = this.findRow(lineNumber, Side.LEFT);
- if (!row) continue;
- row.blameInfo = blameInfo;
- }
- }
-
- // TODO: Refactor this such that adding the move controls becomes part of the
- // lit element.
- protected override getMoveControlsConfig() {
- return {
- numberOfCells: 6, // How many cells does the diff table have?
- movedOutIndex: 2, // Index of left content column in diff table.
- movedInIndex: 5, // Index of right content column in diff table.
- lineNumberCols: [0, 3], // Indices of line number columns in diff table.
- signCols: {left: 1, right: 4},
- };
- }
-
- protected override 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: Remove legacy diff builder, then convert <gr-diff> to be fully lit
- // controlled, then this code will become part of the standard `render()` of
- // <gr-diff> as a LitElement.
- const tempEl = document.createElement('div');
- render(section, tempEl);
- const sectionEl = tempEl.firstElementChild as GrDiffSection;
- return sectionEl;
- }
-
- override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
- const colgroup = html`
- <colgroup>
- <col class=${diffClasses('blame')}></col>
- ${this.renderUnifiedColumns(lineNumberWidth)}
- ${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: Remove legacy diff builder, then convert <gr-diff> to be fully lit
- // controlled, then this code will become part of the standard `render()` of
- // <gr-diff> as a LitElement.
- const tempEl = document.createElement('div');
- render(colgroup, tempEl);
- const colgroupEl = tempEl.firstElementChild as HTMLElement;
- outputEl.appendChild(colgroupEl);
- }
-
- private renderUnifiedColumns(lineNumberWidth: number) {
- if (this.renderPrefs?.view_mode !== DiffViewMode.UNIFIED) return nothing;
- return html`
- <col class=${diffClasses()} width=${lineNumberWidth}></col>
- <col class=${diffClasses()} width=${lineNumberWidth}></col>
- <col class=${diffClasses()}></col>
- `;
- }
-
- private renderSideBySideColumns(side: Side, lineNumberWidth: number) {
- if (this.renderPrefs?.view_mode === DiffViewMode.UNIFIED) return nothing;
- return html`
- <col class=${diffClasses(side)} width=${lineNumberWidth}></col>
- <col class=${diffClasses(side, 'sign')}></col>
- <col class=${diffClasses(side)}></col>
- `;
- }
-
- protected override getNextContentOnSide(
- _content: HTMLElement,
- _side: Side
- ): HTMLElement | null {
- // TODO: getNextContentOnSide() is not required by lit based rendering.
- // So let's refactor it to be moved into gr-diff-builder-legacy.
- console.warn('unimplemented method getNextContentOnSide() called');
- return null;
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
index 2581c03..f38ba5c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
@@ -3,19 +3,27 @@
* 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,
RenderPreferences,
} from '../../../api/diff';
-import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
+import {LineNumber} from '../gr-diff/gr-diff-line';
import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {assert} from '../../../utils/common-util';
-import '../gr-context-controls/gr-context-controls';
import {BlameInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {Side} from '../../../constants/constants';
-import {DiffLayer} from '../../../types/types';
+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 {
@@ -32,83 +40,34 @@
}
}
-/**
- * Given that GrDiffBuilder has ~1,000 lines of code, this interface is just
- * making refactorings easier by emphasizing what the public facing "contract"
- * of this class is. There are no plans for adding separate implementations.
- */
-export interface DiffBuilder {
- init(): void;
- cleanup(): void;
- addGroups(groups: readonly GrDiffGroup[]): void;
- clearGroups(): void;
- replaceGroup(
- contextControl: GrDiffGroup,
- groups: readonly GrDiffGroup[]
- ): void;
- findGroup(side: Side, line: LineNumber): GrDiffGroup | undefined;
- addColumns(outputEl: HTMLElement, fontSize: number): void;
- // TODO: Change `null` to `undefined`.
- getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: Element
- ): HTMLTableCellElement | null;
- getLineElByNumber(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | null;
- getLineNumberRows(): HTMLTableRowElement[];
- getLineNumEls(side: Side): HTMLTableCellElement[];
- setBlame(blame: BlameInfo[]): void;
- updateRenderPrefs(renderPrefs: RenderPreferences): void;
+export function isImageDiffBuilder<T extends GrDiffBuilder>(
+ x: T | GrDiffBuilderImage | undefined
+): x is GrDiffBuilderImage {
+ return !!x && !!(x as GrDiffBuilderImage).renderImageDiff;
}
-export interface ImageDiffBuilder extends DiffBuilder {
- renderImageDiff(): void;
-}
-
-export function isImageDiffBuilder(
- x: DiffBuilder | ImageDiffBuilder | undefined
-): x is ImageDiffBuilder {
- return !!x && !!(x as ImageDiffBuilder).renderImageDiff;
-}
-
-export interface BinaryDiffBuilder extends DiffBuilder {
- renderBinaryDiff(): void;
-}
-
-export function isBinaryDiffBuilder(
- x: DiffBuilder | BinaryDiffBuilder | undefined
-): x is BinaryDiffBuilder {
- return !!x && !!(x as BinaryDiffBuilder).renderBinaryDiff;
+export function isBinaryDiffBuilder<T extends GrDiffBuilder>(
+ x: T | GrDiffBuilderBinary | undefined
+): x is GrDiffBuilderBinary {
+ return !!x && !!(x as GrDiffBuilderBinary).renderBinaryDiff;
}
/**
- * Base class for different diff builders, like side-by-side, unified etc.
- *
* 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.
- *
- * TODO: Do not subclass `GrDiffBuilder`. Use composition and interfaces.
*/
-export abstract class GrDiffBuilder implements DiffBuilder {
- protected readonly _diff: DiffInfo;
+export class GrDiffBuilder {
+ private readonly diff: DiffInfo;
- protected readonly numLinesLeft: number;
+ readonly prefs: DiffPreferencesInfo;
- // visible for testing
- readonly _prefs: DiffPreferencesInfo;
+ renderPrefs?: RenderPreferences;
- protected renderPrefs?: RenderPreferences;
+ readonly outputEl: HTMLElement;
- protected readonly outputEl: HTMLElement;
-
- protected groups: GrDiffGroup[];
-
- private blameInfo: BlameInfo[] = [];
+ private groups: GrDiffGroup[];
private readonly layerUpdateListener: (
start: LineNumber,
@@ -123,14 +82,8 @@
readonly layers: DiffLayer[] = [],
renderPrefs?: RenderPreferences
) {
- this._diff = diff;
- this.numLinesLeft = this._diff.content
- ? this._diff.content.reduce((sum, chunk) => {
- const left = chunk.a || chunk.ab;
- return sum + (left?.length || chunk.skip || 0);
- }, 0)
- : 0;
- this._prefs = prefs;
+ this.diff = diff;
+ this.prefs = prefs;
this.renderPrefs = renderPrefs;
this.outputEl = outputEl;
this.groups = [];
@@ -151,6 +104,138 @@
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.
@@ -182,10 +267,6 @@
}
}
- abstract addColumns(outputEl: HTMLElement, fontSize: number): void;
-
- protected abstract buildSectionElement(group: GrDiffGroup): HTMLElement;
-
addGroups(groups: readonly GrDiffGroup[]) {
for (const group of groups) {
this.groups.push(group);
@@ -248,151 +329,19 @@
.filter(group => group.lines.length > 0);
}
- // TODO: Change `null` to `undefined`.
- abstract getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: Element
- ): HTMLTableCellElement | null;
-
- // TODO: Change `null` to `undefined`.
- abstract getLineElByNumber(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | null;
-
- abstract getLineNumberRows(): HTMLTableRowElement[];
-
- abstract getLineNumEls(side: Side): HTMLTableCellElement[];
-
- protected abstract getBlameTdByLine(lineNum: number): Element | undefined;
-
- // TODO: Change `null` to `undefined`.
- protected abstract getContentByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: HTMLElement
- ): HTMLElement | null;
-
- /**
- * Find line elements or line objects by a range of line numbers and a side.
- *
- * @param start The first line number
- * @param end The last line number
- * @param side The side of the range. Either 'left' or 'right'.
- * @param out_lines The output list of line objects.
- * TODO: Change to camelCase.
- * @param out_elements The output list of line elements.
- * TODO: Change to camelCase.
- */
- // visible for testing
- findLinesByRange(
- start: LineNumber,
- end: LineNumber,
- side: Side,
- out_lines: GrDiffLine[],
- out_elements: HTMLElement[]
- ) {
- const groups = this.getGroupsByLineRange(start, end, side);
- for (const group of groups) {
- let content: HTMLElement | null = null;
- for (const line of group.lines) {
- if (
- (side === 'left' && line.type === GrDiffLineType.ADD) ||
- (side === 'right' && line.type === GrDiffLineType.REMOVE)
- ) {
- continue;
- }
- const lineNumber =
- side === 'left' ? line.beforeNumber : line.afterNumber;
- if (lineNumber < start || lineNumber > end) {
- continue;
- }
-
- if (content) {
- content = this.getNextContentOnSide(content, side);
- } else {
- content = this.getContentByLine(lineNumber, side, group.element);
- }
- if (content) {
- // out_lines and out_elements must match. So if we don't have an
- // element to push, then also don't push a line.
- out_lines.push(line);
- out_elements.push(content);
- }
- }
- }
- assert(
- out_lines.length === out_elements.length,
- 'findLinesByRange: lines and elements arrays must have same length'
- );
- }
-
- protected abstract renderContentByRange(
- start: LineNumber,
- end: LineNumber,
- side: Side
- ): void;
-
- protected abstract renderBlameByRange(
- blame: BlameInfo,
- start: number,
- end: number
- ): void;
-
- /**
- * Finds the next DIV.contentText element following the given element, and on
- * the same side. Will only search within a group.
- *
- * TODO: Change `null` to `undefined`.
- */
- protected abstract getNextContentOnSide(
- content: HTMLElement,
- side: Side
- ): HTMLElement | null;
-
- /**
- * Gets configuration for creating move controls for chunks marked with
- * dueToMove
- */
- protected abstract getMoveControlsConfig(): {
- numberOfCells: number;
- movedOutIndex: number;
- movedInIndex: number;
- lineNumberCols: number[];
- signCols?: {left: number; right: number};
- };
-
/**
* Set the blame information for the diff. For any already-rendered line,
* re-render its blame cell content.
*/
setBlame(blame: BlameInfo[]) {
- this.blameInfo = blame;
- for (const commit of blame) {
- for (const range of commit.ranges) {
- this.renderBlameByRange(commit, range.start, range.end);
- }
- }
- }
-
- /**
- * Given a base line number, return the commit containing that line in the
- * current set of blame information. If no blame information has been
- * provided, null is returned.
- *
- * @return The commit information.
- */
- // visible for testing
- getBlameCommitForBaseLine(lineNum: LineNumber): BlameInfo | undefined {
- for (const blameCommit of this.blameInfo) {
- for (const range of blameCommit.ranges) {
- if (range.start <= lineNum && range.end >= lineNum) {
- return blameCommit;
+ 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;
}
}
}
- return undefined;
}
/**
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
index 02f2233..69c0f5c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -44,7 +44,7 @@
* fully blown dependency on GrDiffBuilderElement.
*/
export interface DiffBuilderInterface {
- getContentTdByLineEl(lineEl?: Element): Element | null;
+ getContentTdByLineEl(lineEl?: Element): Element | undefined;
}
/**
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 3b2fa81..2ec0a2e 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
@@ -506,16 +506,8 @@
return Promise.resolve();
}
assertIsDefined(this.element);
- // This is a temporary hack while migration to lit based diff rendering:
- // Elements with 'display: contents;' do not have a height, so they
- // won't work as intended with `untilRendered()`.
- const isLitDiff = this.element.tagName === 'GR-DIFF-SECTION';
- if (isLitDiff) {
- await (this.element as LitElement).updateComplete;
- await untilRendered(this.element.firstElementChild as HTMLElement);
- } else {
- await untilRendered(this.element);
- }
+ await (this.element as LitElement).updateComplete;
+ await untilRendered(this.element.firstElementChild as HTMLElement);
}
/**
