Add outline information to gr-diff
Implement preliminary block expansion behavior based on the
right side of the syntax tree.
Refator: Split the creation of different button groups (different types)
in different methods.
Change-Id: Iabdc469748b86a1333e57bf2c0ebe5c88f802871
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index a10cb14..396fd8e 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -53,6 +53,35 @@
}
/**
+ * Represents a syntax block in a code (e.g. method, function, class, if-else).
+ */
+export interface SyntaxBlock {
+ /** Name of the block (e.g. name of the method/class)*/
+ name: string;
+ /** Where does this block syntatically starts and ends (line number and column).*/
+ range: {
+ /** first line of the block (1-based inclusive). */
+ start_line: number;
+ /**
+ * column of the range start inside the first line (e.g. "{" character ending a function/method)
+ * (1-based inclusive).
+ */
+ start_column: number;
+ /**
+ * last line of the block (1-based inclusive).
+ */
+ end_line: number;
+ /**
+ * column of the block end inside the end line (e.g. "}" character ending a function/method)
+ * (1-based inclusive).
+ */
+ end_column: number;
+ };
+ /** Sub-blocks of the current syntax block (e.g. methods of a class) */
+ children: SyntaxBlock[];
+}
+
+/**
* The DiffFileMetaInfo entity contains meta information about a file diff.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-file-meta-info
*/
@@ -65,6 +94,12 @@
lines: number;
// TODO: Not documented.
language?: string;
+ /**
+ * The first level of syntax blocks tree (outline) within the current file.
+ * It contains an hierarchical structure where each block contains its
+ * sub-blocks (children).
+ */
+ syntax_tree?: SyntaxBlock[];
}
export declare type ChangeType =
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
index 7c01a95..5ba3606 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
@@ -97,8 +97,13 @@
return section;
}
+ let diffInfo;
+ let renderPrefs;
+
setup(() => {
- builder = new GrDiffBuilder({content: []}, prefs, null, []);
+ diffInfo = {content: []};
+ renderPrefs = {};
+ builder = new GrDiffBuilder(diffInfo, prefs, null, [], renderPrefs);
});
test('no +10 buttons for 10 or less lines', () => {
@@ -149,6 +154,70 @@
assert.include([...buttons[0].classList.values()], 'aboveButton');
assert.include([...buttons[1].classList.values()], 'aboveButton');
});
+
+ suite('with block expansion', () => {
+ setup(() => {
+ builder._numLinesLeft = 50;
+ renderPrefs.use_block_expansion = true;
+ diffInfo.meta_b = {
+ syntax_tree: [],
+ };
+ });
+
+ test('context control with block expansion at the top', () => {
+ const section = createContextSectionForGroups({offset: 0, count: 20});
+
+ const fullExpansionButtons = section
+ .querySelectorAll('.fullExpansion gr-button');
+ const partialExpansionButtons = section
+ .querySelectorAll('.partialExpansion gr-button');
+ const blockExpansionButtons = section
+ .querySelectorAll('.blockExpansion gr-button');
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons[0].textContent, '+Block');
+ assert.include([...blockExpansionButtons[0].classList.values()],
+ 'belowButton');
+ });
+
+ test('context control in the middle', () => {
+ const section = createContextSectionForGroups({offset: 10, count: 20});
+
+ const fullExpansionButtons = section
+ .querySelectorAll('.fullExpansion gr-button');
+ const partialExpansionButtons = section
+ .querySelectorAll('.partialExpansion gr-button');
+ const blockExpansionButtons = section
+ .querySelectorAll('.blockExpansion gr-button');
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 2);
+ assert.equal(blockExpansionButtons.length, 2);
+ assert.equal(blockExpansionButtons[0].textContent, '+Block');
+ assert.equal(blockExpansionButtons[1].textContent, '+Block');
+ assert.include([...blockExpansionButtons[0].classList.values()],
+ 'aboveButton');
+ assert.include([...blockExpansionButtons[1].classList.values()],
+ 'belowButton');
+ });
+
+ test('context control at the bottom', () => {
+ const section = createContextSectionForGroups({offset: 30, count: 20});
+
+ const fullExpansionButtons = section
+ .querySelectorAll('.fullExpansion gr-button');
+ const partialExpansionButtons = section
+ .querySelectorAll('.partialExpansion gr-button');
+ const blockExpansionButtons = section
+ .querySelectorAll('.blockExpansion gr-button');
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons[0].textContent, '+Block');
+ assert.include([...blockExpansionButtons[0].classList.values()],
+ 'aboveButton');
+ });
+ });
});
test('newlines 1', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index c8b69f0..d5e6ecd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -20,6 +20,7 @@
DiffContextExpandedExternalDetail,
MovedLinkClickedEventDetail,
RenderPreferences,
+ SyntaxBlock,
} from '../../../api/diff';
import {getBaseUrl} from '../../../utils/url-util';
import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
@@ -73,6 +74,19 @@
}
}
+function findMostNestedContainingBlock(
+ lineNum: number,
+ blocks?: SyntaxBlock[]
+): SyntaxBlock | undefined {
+ const containingBlock = blocks?.find(
+ ({range}) => range.start_line < lineNum && range.end_line > lineNum
+ );
+ const containingChildBlock = containingBlock
+ ? findMostNestedContainingBlock(lineNum, containingBlock?.children)
+ : undefined;
+ return containingChildBlock || containingBlock;
+}
+
export abstract class GrDiffBuilder {
private readonly _diff: DiffInfo;
@@ -306,6 +320,7 @@
);
}
+ // TODO(renanoliveira): Move context controls to polymer component (or at least a separate class).
_createContextControls(
section: HTMLElement,
contextGroups: GrDiffGroup[],
@@ -314,9 +329,9 @@
const leftStart = contextGroups[0].lineRange.left.start_line;
const leftEnd =
contextGroups[contextGroups.length - 1].lineRange.left.end_line;
- const numLines = leftEnd - leftStart + 1;
-
- if (numLines === 0) console.error('context group without lines');
+ const rightStart = contextGroups[0].lineRange.right.start_line;
+ const rightEnd =
+ contextGroups[contextGroups.length - 1].lineRange.right.end_line;
const firstGroupIsSkipped = !!contextGroups[0].skip;
const lastGroupIsSkipped = !!contextGroups[contextGroups.length - 1].skip;
@@ -335,7 +350,8 @@
contextGroups,
showAbove,
showBelow,
- numLines
+ rightStart,
+ rightEnd
)
);
if (showBelow) {
@@ -354,8 +370,12 @@
contextGroups: GrDiffGroup[],
showAbove: boolean,
showBelow: boolean,
- numLines: number
+ rightStart: number,
+ rightEnd: number
): HTMLElement {
+ const numLines = rightEnd - rightStart + 1;
+ if (numLines === 0) console.error('context group without lines');
+
const row = this._createElement('tr', 'contextDivider');
if (!(showAbove && showBelow)) {
row.classList.add('collapsed');
@@ -364,13 +384,55 @@
const element = this._createElement('td', 'dividerCell');
row.appendChild(element);
- const showAllContainer = this._createElement('div', 'aboveBelowButtons');
+ const showAllContainer = this._createExpandAllButtonContainer(
+ section,
+ contextGroups,
+ showAbove,
+ showBelow,
+ numLines
+ );
element.appendChild(showAllContainer);
+ const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
+ if (showPartialLinks) {
+ const partialExpansionContainer = this._createPartialExpansionButtons(
+ section,
+ contextGroups,
+ showAbove,
+ showBelow,
+ numLines
+ );
+ if (partialExpansionContainer) {
+ element.appendChild(partialExpansionContainer);
+ }
+ const blockExpansionContainer = this._createBlockExpansionButtons(
+ section,
+ contextGroups,
+ showAbove,
+ showBelow,
+ rightStart,
+ rightEnd,
+ numLines
+ );
+ if (blockExpansionContainer) {
+ element.appendChild(blockExpansionContainer);
+ }
+ }
+ return row;
+ }
+
+ private _createExpandAllButtonContainer(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ showAbove: boolean,
+ showBelow: boolean,
+ numLines: number
+ ) {
const showAllButton = this._createContextButton(
ContextButtonType.ALL,
section,
contextGroups,
+ numLines,
numLines
);
showAllButton.classList.add(
@@ -380,61 +442,131 @@
? 'aboveButton'
: 'belowButton'
);
+ const showAllContainer = this._createElement(
+ 'div',
+ 'aboveBelowButtons fullExpansion'
+ );
showAllContainer.appendChild(showAllButton);
+ return showAllContainer;
+ }
- const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
- if (showPartialLinks) {
- const container = this._createElement('div', 'aboveBelowButtons');
- if (showAbove) {
- container.appendChild(
- this._createContextButton(
- ContextButtonType.ABOVE,
- section,
- contextGroups,
- numLines
- )
- );
- }
- if (showBelow) {
- container.appendChild(
- this._createContextButton(
- ContextButtonType.BELOW,
- section,
- contextGroups,
- numLines
- )
- );
- }
- element.appendChild(container);
- if (this._renderPrefs?.use_block_expansion) {
- const blockExpansionContainer = this._createElement(
- 'div',
- 'aboveBelowButtons'
- );
- if (showAbove) {
- blockExpansionContainer.appendChild(
- this._createContextButton(
- ContextButtonType.BLOCK_ABOVE,
- section,
- contextGroups,
- numLines
- )
- );
- }
- if (showBelow) {
- blockExpansionContainer.appendChild(
- this._createContextButton(
- ContextButtonType.BLOCK_BELOW,
- section,
- contextGroups,
- numLines
- )
- );
- }
- element.appendChild(blockExpansionContainer);
+ private _createPartialExpansionButtons(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ showAbove: boolean,
+ showBelow: boolean,
+ numLines: number
+ ) {
+ let aboveButton;
+ let belowButton;
+ if (showAbove) {
+ aboveButton = this._createContextButton(
+ ContextButtonType.ABOVE,
+ section,
+ contextGroups,
+ numLines,
+ PARTIAL_CONTEXT_AMOUNT
+ );
+ }
+ if (showBelow) {
+ belowButton = this._createContextButton(
+ ContextButtonType.BELOW,
+ section,
+ contextGroups,
+ numLines,
+ PARTIAL_CONTEXT_AMOUNT
+ );
+ }
+ if (aboveButton || belowButton) {
+ const partialExpansionContainer = this._createElement(
+ 'div',
+ 'aboveBelowButtons partialExpansion'
+ );
+ aboveButton && partialExpansionContainer.appendChild(aboveButton);
+ belowButton && partialExpansionContainer.appendChild(belowButton);
+ return partialExpansionContainer;
+ }
+ return undefined;
+ }
+
+ private _createBlockExpansionButtons(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ showAbove: boolean,
+ showBelow: boolean,
+ rightStart: number,
+ rightEnd: number,
+ numLines: number
+ ) {
+ if (!this._renderPrefs?.use_block_expansion) {
+ return undefined;
+ }
+ let aboveBlockButton;
+ let belowBlockButton;
+ const rightSyntaxTree = this._diff.meta_b.syntax_tree;
+ if (showAbove) {
+ aboveBlockButton = this._createBlockButton(
+ section,
+ contextGroups,
+ ContextButtonType.BLOCK_ABOVE,
+ numLines,
+ rightStart - 1,
+ rightSyntaxTree
+ );
+ }
+ if (showBelow) {
+ belowBlockButton = this._createBlockButton(
+ section,
+ contextGroups,
+ ContextButtonType.BLOCK_BELOW,
+ numLines,
+ rightEnd + 1,
+ rightSyntaxTree
+ );
+ }
+ if (aboveBlockButton || belowBlockButton) {
+ const blockExpansionContainer = this._createElement(
+ 'div',
+ 'blockExpansion aboveBelowButtons'
+ );
+ aboveBlockButton && blockExpansionContainer.appendChild(aboveBlockButton);
+ belowBlockButton && blockExpansionContainer.appendChild(belowBlockButton);
+ return blockExpansionContainer;
+ }
+ return undefined;
+ }
+
+ private _createBlockButton(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ buttonType: ContextButtonType,
+ numLines: number,
+ referenceLine: number,
+ syntaxTree?: SyntaxBlock[]
+ ) {
+ const containingBlock = findMostNestedContainingBlock(
+ referenceLine,
+ syntaxTree
+ );
+ let linesToExpand = numLines;
+ if (containingBlock) {
+ const {range} = containingBlock;
+ const targetLine =
+ buttonType === ContextButtonType.BLOCK_ABOVE
+ ? range.end_line
+ : range.start_line;
+ const distanceToTargetLine = Math.abs(targetLine - referenceLine);
+ if (distanceToTargetLine < numLines) {
+ linesToExpand = distanceToTargetLine;
}
}
- return row;
+ return this._createContextButton(
+ buttonType,
+ section,
+ contextGroups,
+ numLines,
+ linesToExpand
+ );
}
/**
@@ -469,10 +601,9 @@
type: ContextButtonType,
section: HTMLElement,
contextGroups: GrDiffGroup[],
- numLines: number
+ numLines: number,
+ linesToExpand: number
) {
- const linesToExpand =
- type === ContextButtonType.ALL ? numLines : PARTIAL_CONTEXT_AMOUNT;
const button = this._createElement('gr-button', 'showContext');
button.classList.add('contextControlButton');
button.setAttribute('link', 'true');
