Merge "PerformanceMetrics: Log per request latency for configured operations"
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index a9b82d6..a510e25 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -93,10 +93,6 @@
** `plugin`:
The name of the plugin that performed the operation.
-Performance metrics can be enabled via the
-link:config.gerrit.html#tracing.exportPerformanceMetrics[`tracing.exportPerformanceMetrics`]
-setting.
-
=== Pushes
* `receivecommits/changes`: histogram of number of changes processed in a single
diff --git a/java/com/google/gerrit/acceptance/AssertUtil.java b/java/com/google/gerrit/acceptance/AssertUtil.java
index a1d3e79..f72c6d3 100644
--- a/java/com/google/gerrit/acceptance/AssertUtil.java
+++ b/java/com/google/gerrit/acceptance/AssertUtil.java
@@ -25,9 +25,9 @@
public class AssertUtil {
public static <T> void assertPrefs(T actual, T expected, String... fieldsToExclude)
throws IllegalArgumentException, IllegalAccessException {
- Set<String> exludedFields = new HashSet<>(Arrays.asList(fieldsToExclude));
+ Set<String> excludedFields = new HashSet<>(Arrays.asList(fieldsToExclude));
for (Field field : actual.getClass().getDeclaredFields()) {
- if (exludedFields.contains(field.getName()) || skipField(field)) {
+ if (excludedFields.contains(field.getName()) || skipField(field)) {
continue;
}
Object actualVal = field.get(actual);
diff --git a/java/com/google/gerrit/server/cancellation/RequestCancelledException.java b/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
index dc01567..9663427 100644
--- a/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
+++ b/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
@@ -15,7 +15,9 @@
package com.google.gerrit.server.cancellation;
import com.google.common.base.Throwables;
+import com.google.common.collect.Streams;
import com.google.gerrit.common.Nullable;
+import java.util.Arrays;
import java.util.Optional;
import org.apache.commons.text.WordUtils;
@@ -28,7 +30,9 @@
* {@link RequestCancelledException} is returned. If not, {@link Optional#empty()} is returned.
*/
public static Optional<RequestCancelledException> getFromCausalChain(Throwable e) {
- return Throwables.getCausalChain(e).stream()
+ return Streams.concat(
+ Throwables.getCausalChain(e).stream(),
+ Throwables.getCausalChain(e).stream().flatMap(t -> Arrays.stream(t.getSuppressed())))
.filter(RequestCancelledException.class::isInstance)
.map(RequestCancelledException.class::cast)
.findFirst();
diff --git a/java/com/google/gerrit/server/config/CachedPreferences.java b/java/com/google/gerrit/server/config/CachedPreferences.java
index 6e957e6..3b013c0 100644
--- a/java/com/google/gerrit/server/config/CachedPreferences.java
+++ b/java/com/google/gerrit/server/config/CachedPreferences.java
@@ -47,6 +47,7 @@
public Optional<CachedPreferencesProto> nonEmptyConfig() {
return config().equals(EMPTY.config()) ? Optional.empty() : Optional.of(config());
}
+
/** Returns a cache-able representation of the preferences proto. */
public static CachedPreferences fromUserPreferencesProto(UserPreferences proto) {
return fromCachedPreferencesProto(
@@ -125,9 +126,8 @@
CachedPreferencesProto userPreferencesProto = userPreferences.config();
switch (userPreferencesProto.getPreferencesCase()) {
case USER_PREFERENCES:
- PreferencesT pref =
- preferencesParser.fromUserPreferences(userPreferencesProto.getUserPreferences());
- return preferencesParser.parse(pref, configOrNull(defaultPreferences));
+ return preferencesParser.fromUserPreferences(
+ userPreferencesProto.getUserPreferences(), configOrNull(defaultPreferences));
case LEGACY_GIT_CONFIG:
return preferencesParser.parse(
userPreferences.asConfig(), configOrNull(defaultPreferences), null);
diff --git a/java/com/google/gerrit/server/config/PreferencesParserUtil.java b/java/com/google/gerrit/server/config/PreferencesParserUtil.java
index ecb0868..3e39e25 100644
--- a/java/com/google/gerrit/server/config/PreferencesParserUtil.java
+++ b/java/com/google/gerrit/server/config/PreferencesParserUtil.java
@@ -320,13 +320,12 @@
}
/** Provides methods for parsing user configs */
- public interface PreferencesParser<T> {
+ interface PreferencesParser<T> {
T parse(Config cfg, @Nullable Config defaultConfig, @Nullable T input)
throws ConfigInvalidException;
- T parse(T cfg, @Nullable Config defaultConfig) throws ConfigInvalidException;
-
- T fromUserPreferences(UserPreferences userPreferences);
+ T fromUserPreferences(UserPreferences userPreferences, @Nullable Config defaultCfg)
+ throws ConfigInvalidException;
T getJavaDefaults();
}
@@ -346,14 +345,10 @@
}
@Override
- public GeneralPreferencesInfo parse(GeneralPreferencesInfo cfg, @Nullable Config defaultCfg)
- throws ConfigInvalidException {
- return PreferencesParserUtil.parseGeneralPreferences(cfg, defaultCfg);
- }
-
- @Override
- public GeneralPreferencesInfo fromUserPreferences(UserPreferences p) {
- return GENERAL_PREFERENCES_INFO_CONVERTER.fromProto(p.getGeneralPreferencesInfo());
+ public GeneralPreferencesInfo fromUserPreferences(
+ UserPreferences p, @Nullable Config defaultCfg) throws ConfigInvalidException {
+ return PreferencesParserUtil.parseGeneralPreferences(
+ GENERAL_PREFERENCES_INFO_CONVERTER.fromProto(p.getGeneralPreferencesInfo()), defaultCfg);
}
@Override
@@ -376,14 +371,10 @@
}
@Override
- public EditPreferencesInfo parse(EditPreferencesInfo cfg, @Nullable Config defaultCfg)
+ public EditPreferencesInfo fromUserPreferences(UserPreferences p, @Nullable Config defaultCfg)
throws ConfigInvalidException {
- return PreferencesParserUtil.parseEditPreferences(cfg, defaultCfg);
- }
-
- @Override
- public EditPreferencesInfo fromUserPreferences(UserPreferences p) {
- return EDIT_PREFERENCES_INFO_CONVERTER.fromProto(p.getEditPreferencesInfo());
+ return PreferencesParserUtil.parseEditPreferences(
+ EDIT_PREFERENCES_INFO_CONVERTER.fromProto(p.getEditPreferencesInfo()), defaultCfg);
}
@Override
@@ -406,14 +397,10 @@
}
@Override
- public DiffPreferencesInfo parse(DiffPreferencesInfo cfg, @Nullable Config defaultCfg)
+ public DiffPreferencesInfo fromUserPreferences(UserPreferences p, @Nullable Config defaultCfg)
throws ConfigInvalidException {
- return PreferencesParserUtil.parseDiffPreferences(cfg, defaultCfg);
- }
-
- @Override
- public DiffPreferencesInfo fromUserPreferences(UserPreferences p) {
- return DIFF_PREFERENCES_INFO_CONVERTER.fromProto(p.getDiffPreferencesInfo());
+ return PreferencesParserUtil.parseDiffPreferences(
+ DIFF_PREFERENCES_INFO_CONVERTER.fromProto(p.getDiffPreferencesInfo()), defaultCfg);
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
index 6802333..74829a3 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/EditPreferencesIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.api.accounts;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.AssertUtil.assertPrefs;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
@@ -60,7 +61,7 @@
EditPreferencesInfo info = gApi.accounts().id(admin.id().get()).setEditPreferences(out);
- assertEditPreferences(info, out);
+ assertPrefs(info, out);
// Partially filled input record
EditPreferencesInfo in = new EditPreferencesInfo();
@@ -69,24 +70,6 @@
info = gApi.accounts().id(admin.id().get()).setEditPreferences(in);
out.tabSize = in.tabSize;
- assertEditPreferences(info, out);
- }
-
- private void assertEditPreferences(EditPreferencesInfo out, EditPreferencesInfo in)
- throws Exception {
- assertThat(out.lineLength).isEqualTo(in.lineLength);
- assertThat(out.indentUnit).isEqualTo(in.indentUnit);
- assertThat(out.tabSize).isEqualTo(in.tabSize);
- assertThat(out.cursorBlinkRate).isEqualTo(in.cursorBlinkRate);
- assertThat(out.hideTopMenu).isEqualTo(in.hideTopMenu);
- assertThat(out.showTabs).isNull();
- assertThat(out.showWhitespaceErrors).isEqualTo(in.showWhitespaceErrors);
- assertThat(out.syntaxHighlighting).isNull();
- assertThat(out.hideLineNumbers).isEqualTo(in.hideLineNumbers);
- assertThat(out.matchBrackets).isNull();
- assertThat(out.lineWrapping).isEqualTo(in.lineWrapping);
- assertThat(out.indentWithTabs).isEqualTo(in.indentWithTabs);
- assertThat(out.autoCloseBrackets).isEqualTo(in.autoCloseBrackets);
- assertThat(out.showBase).isEqualTo(in.showBase);
+ assertPrefs(info, out);
}
}
diff --git a/javatests/com/google/gerrit/server/config/CachedPreferencesTest.java b/javatests/com/google/gerrit/server/config/CachedPreferencesTest.java
index b149d09..131fb23 100644
--- a/javatests/com/google/gerrit/server/config/CachedPreferencesTest.java
+++ b/javatests/com/google/gerrit/server/config/CachedPreferencesTest.java
@@ -151,6 +151,58 @@
}
@Test
+ public void bothPreferencesTypes_getGeneralPreferencesAreEqual() throws Exception {
+ UserPreferences originalProto =
+ UserPreferences.newBuilder()
+ .setGeneralPreferencesInfo(
+ UserPreferences.GeneralPreferencesInfo.newBuilder().setChangesPerPage(19))
+ .build();
+ Config originalCfg = new Config();
+ originalCfg.fromText("[general]\n\tchangesPerPage = 19");
+
+ CachedPreferences protoPref = CachedPreferences.fromUserPreferencesProto(originalProto);
+ GeneralPreferencesInfo protoGeneral = CachedPreferences.general(Optional.empty(), protoPref);
+ CachedPreferences cfgPref = CachedPreferences.fromLegacyConfig(originalCfg);
+ GeneralPreferencesInfo cfgGeneral = CachedPreferences.general(Optional.empty(), cfgPref);
+
+ assertThat(protoGeneral).isEqualTo(cfgGeneral);
+ }
+
+ @Test
+ public void bothPreferencesTypes_getDiffPreferencesAreEqual() throws Exception {
+ UserPreferences originalProto =
+ UserPreferences.newBuilder()
+ .setDiffPreferencesInfo(UserPreferences.DiffPreferencesInfo.newBuilder().setContext(23))
+ .build();
+ Config originalCfg = new Config();
+ originalCfg.fromText("[diff]\n\tcontext = 23");
+
+ CachedPreferences protoPref = CachedPreferences.fromUserPreferencesProto(originalProto);
+ DiffPreferencesInfo protoDiff = CachedPreferences.diff(Optional.empty(), protoPref);
+ CachedPreferences cfgPref = CachedPreferences.fromLegacyConfig(originalCfg);
+ DiffPreferencesInfo cfgDiff = CachedPreferences.diff(Optional.empty(), cfgPref);
+
+ assertThat(protoDiff).isEqualTo(cfgDiff);
+ }
+
+ @Test
+ public void bothPreferencesTypes_getEditPreferencesAreEqual() throws Exception {
+ UserPreferences originalProto =
+ UserPreferences.newBuilder()
+ .setEditPreferencesInfo(UserPreferences.EditPreferencesInfo.newBuilder().setTabSize(27))
+ .build();
+ Config originalCfg = new Config();
+ originalCfg.fromText("[edit]\n\ttabSize = 27");
+
+ CachedPreferences protoPref = CachedPreferences.fromUserPreferencesProto(originalProto);
+ EditPreferencesInfo protoEdit = CachedPreferences.edit(Optional.empty(), protoPref);
+ CachedPreferences cfgPref = CachedPreferences.fromLegacyConfig(originalCfg);
+ EditPreferencesInfo cfgEdit = CachedPreferences.edit(Optional.empty(), cfgPref);
+
+ assertThat(protoEdit).isEqualTo(cfgEdit);
+ }
+
+ @Test
public void defaultPreferences_acceptingGitConfig() throws Exception {
Config cfg = new Config();
cfg.fromText("[general]\n\tchangesPerPage = 19");
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 984391d..e2a1d63 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -277,7 +277,8 @@
return html`<section id="relatedChanges">
<gr-related-collapse
- title="Relation chain"
+ .name=${'Relation chain'}
+ title="parent changes are ordered after child changes"
class=${classMap({first: isFirst})}
.length=${this.relatedChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.RELATED_CHANGES)}
@@ -342,7 +343,8 @@
);
return html`<section id="submittedTogether">
<gr-related-collapse
- title="Submitted together"
+ .name=${'Submitted together'}
+ title="parent changes are ordered after child changes"
class=${classMap({first: isFirst})}
.length=${submittedTogetherChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.SUBMITTED_TOGETHER)}
@@ -405,7 +407,7 @@
);
return html`<section id="sameTopic">
<gr-related-collapse
- title="Same topic"
+ .name=${'Same topic'}
class=${classMap({first: isFirst})}
.length=${this.sameTopicChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.SAME_TOPIC)}
@@ -445,7 +447,7 @@
);
return html`<section id="mergeConflicts">
<gr-related-collapse
- title="Merge conflicts"
+ .name=${'Merge conflicts'}
class=${classMap({first: isFirst})}
.length=${this.conflictingChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.MERGE_CONFLICTS)}
@@ -490,7 +492,7 @@
);
return html`<section id="cherryPicks">
<gr-related-collapse
- title="Cherry picks"
+ .name=${'Cherry picks'}
class=${classMap({first: isFirst})}
.length=${this.cherryPickChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.CHERRY_PICKS)}
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index 2e33333..e8bf442 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -200,7 +200,10 @@
<gr-endpoint-param name="change"> </gr-endpoint-param>
<gr-endpoint-slot name="top"> </gr-endpoint-slot>
<section id="relatedChanges">
- <gr-related-collapse class="first" title="Relation chain">
+ <gr-related-collapse
+ class="first"
+ title="parent changes are ordered after child changes"
+ >
<div class="relatedChangeLine show-when-collapsed">
<span class="marker space"> </span>
<gr-related-change
@@ -213,7 +216,9 @@
</gr-related-collapse>
</section>
<section id="submittedTogether">
- <gr-related-collapse title="Submitted together">
+ <gr-related-collapse
+ title="parent changes are ordered after child changes"
+ >
<div class="relatedChangeLine selected show-when-collapsed">
<span
aria-label="Arrow marking current change"
@@ -236,7 +241,7 @@
<div class="note" hidden="">(+ )</div>
</section>
<section id="cherryPicks">
- <gr-related-collapse title="Cherry picks">
+ <gr-related-collapse>
<div class="relatedChangeLine show-when-collapsed">
<span class="marker space"> </span>
<gr-related-change show-change-status="">
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-collapse.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-collapse.ts
index 30d2282..8c2f459 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-collapse.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-collapse.ts
@@ -18,7 +18,7 @@
@customElement('gr-related-collapse')
export class GrRelatedCollapse extends LitElement {
@property()
- override title = '';
+ name = '';
@property({type: Boolean})
showAll = false;
@@ -64,7 +64,7 @@
}
override render() {
- const title = html`<h3 class="title heading-3">${this.title}</h3>`;
+ const title = html`<h3 class="title heading-3">${this.name}</h3>`;
const collapsible = this.length > this.numChangesWhenCollapsed;
this.collapsed = !this.showAll && collapsible;
@@ -88,7 +88,7 @@
e.stopPropagation();
this.showAll = !this.showAll;
this.reporting.reportInteraction(Interaction.TOGGLE_SHOW_ALL_BUTTON, {
- sectionName: this.title,
+ sectionName: this.name,
toState: this.showAll ? 'Show all' : 'Show less',
});
}
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 65fdb93..6cff5ab 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -489,7 +489,7 @@
return html`
<!-- The is for being able to shrink a tiny amount without
the text itself getting shrunk with an ellipsis. -->
- <div class="summary" @click=${this.toggleExpanded} title=${text}>
+ <div class="summary" @click=${this.toggleExpandedClick} title=${text}>
${text}
</div>
`;
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts b/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
index a349b58..22c2940 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results_test.ts
@@ -123,6 +123,22 @@
`
);
});
+
+ test('click summary, toggle expand', async () => {
+ element.isExpandable = true;
+ await element.updateComplete;
+ assert.isFalse(element.isExpanded);
+
+ const summaryDiv: HTMLElement =
+ element.shadowRoot!.querySelector('.summary')!;
+ summaryDiv.click();
+ await element.updateComplete;
+ assert.isTrue(element.isExpanded);
+
+ summaryDiv.click();
+ await element.updateComplete;
+ assert.isFalse(element.isExpanded);
+ });
});
suite('gr-checks-results test', () => {
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 1561dad..d138414 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
@@ -80,10 +80,14 @@
private groups: GrDiffGroup[] = [];
+ // visible for testing
+ diffRangesToFocus?: DiffRangesToFocus;
+
constructor(options: ProcessingOptions) {
this.context = options.context;
this.keyLocations = options.keyLocations ?? {left: {}, right: {}};
this.isBinary = options.isBinary ?? false;
+ this.diffRangesToFocus = options.diffRangesToFocus;
}
/**
@@ -127,7 +131,7 @@
processNext(state: State, chunks: DiffContent[]) {
const firstUncollapsibleChunkIndex = this.firstUncollapsibleChunkIndex(
chunks,
- state.chunkIndex
+ state
);
if (firstUncollapsibleChunkIndex === state.chunkIndex) {
const chunk = chunks[state.chunkIndex];
@@ -162,19 +166,67 @@
return chunk.ab || chunk.b || [];
}
- private firstUncollapsibleChunkIndex(chunks: DiffContent[], offset: number) {
- let chunkIndex = offset;
+ private firstUncollapsibleChunkIndex(chunks: DiffContent[], state: State) {
+ let chunkIndex = state.chunkIndex;
+ let offsetLeft = state.lineNums.left;
+ let offsetRight = state.lineNums.right;
while (
chunkIndex < chunks.length &&
- this.isCollapsibleChunk(chunks[chunkIndex])
+ this.isCollapsibleChunk(chunks[chunkIndex], offsetLeft, offsetRight)
) {
+ offsetLeft += this.chunkLength(chunks[chunkIndex], Side.LEFT);
+ offsetRight += this.chunkLength(chunks[chunkIndex], Side.RIGHT);
chunkIndex++;
}
return chunkIndex;
}
- private isCollapsibleChunk(chunk: DiffContent) {
- return (chunk.ab || chunk.common || chunk.skip) && !chunk.keyLocation;
+ private isCollapsibleChunk(
+ chunk: DiffContent,
+ offsetLeft: number,
+ offsetRight: number
+ ) {
+ return (
+ (chunk.ab ||
+ chunk.common ||
+ chunk.skip ||
+ this.isChunkOutsideOfFocusRange(chunk, offsetLeft, offsetRight)) &&
+ !chunk.keyLocation
+ );
+ }
+
+ private isChunkOutsideOfFocusRange(
+ chunk: DiffContent,
+ offsetLeft: number,
+ offsetRight: number
+ ) {
+ if (!this.diffRangesToFocus) {
+ return false;
+ }
+ const leftLineCount = this.linesLeft(chunk).length;
+ const rightLineCount = this.linesRight(chunk).length;
+ const hasLeftSideOverlap = this.diffRangesToFocus.left.some(range =>
+ this.hasAnyOverlap(
+ {start: offsetLeft, end: offsetLeft + leftLineCount},
+ range
+ )
+ );
+ const hasRightSideOverlap = this.diffRangesToFocus.right.some(range =>
+ this.hasAnyOverlap(
+ {start: offsetRight, end: offsetRight + rightLineCount},
+ range
+ )
+ );
+ return !hasLeftSideOverlap && !hasRightSideOverlap;
+ }
+
+ private hasAnyOverlap(
+ firstRange: {start: number; end: number},
+ secondRange: {start: number; end: number}
+ ) {
+ const startOverlap = Math.max(firstRange.start, secondRange.start);
+ const endOverlap = Math.min(firstRange.end, secondRange.end);
+ return startOverlap <= endOverlap;
}
/**
@@ -196,8 +248,12 @@
state.chunkIndex,
firstUncollapsibleChunkIndex
);
- const lineCount = collapsibleChunks.reduce(
- (sum, chunk) => sum + this.commonChunkLength(chunk),
+ const leftLineCount = collapsibleChunks.reduce(
+ (sum, chunk) => sum + this.chunkLength(chunk, Side.LEFT),
+ 0
+ );
+ const rightLineCount = collapsibleChunks.reduce(
+ (sum, chunk) => sum + this.chunkLength(chunk, Side.RIGHT),
0
);
@@ -208,25 +264,50 @@
);
const hasSkippedGroup = !!groups.find(g => g.skip);
- if (this.context !== FULL_CONTEXT || hasSkippedGroup) {
+ const hasNonCommonDeltaGroup = !!groups.find(
+ g => g.type === GrDiffGroupType.DELTA && !g.ignoredWhitespaceOnly
+ );
+ if (
+ this.context !== FULL_CONTEXT ||
+ hasSkippedGroup ||
+ hasNonCommonDeltaGroup
+ ) {
const contextNumLines = this.context > 0 ? this.context : 0;
const hiddenStart = state.chunkIndex === 0 ? 0 : contextNumLines;
- const hiddenEnd =
- lineCount -
+ const hiddenEndLeft =
+ leftLineCount -
(firstUncollapsibleChunkIndex === chunks.length ? 0 : this.context);
- groups = hideInContextControl(groups, hiddenStart, hiddenEnd);
+ const hiddenEndRight =
+ rightLineCount -
+ (firstUncollapsibleChunkIndex === chunks.length ? 0 : this.context);
+ groups = hideInContextControl(
+ groups,
+ hiddenStart,
+ hiddenEndLeft,
+ hiddenEndRight
+ );
}
return {
lineDelta: {
- left: lineCount,
- right: lineCount,
+ left: leftLineCount,
+ right: rightLineCount,
},
groups,
newChunkIndex: firstUncollapsibleChunkIndex,
};
}
+ private chunkLength(chunk: DiffContent, side: Side) {
+ if (chunk.skip || chunk.common || chunk.ab) {
+ return this.commonChunkLength(chunk);
+ } else if (side === Side.LEFT) {
+ return this.linesLeft(chunk).length;
+ } else {
+ return this.linesRight(chunk).length;
+ }
+ }
+
private commonChunkLength(chunk: DiffContent) {
if (chunk.skip) {
return chunk.skip;
@@ -248,9 +329,8 @@
): GrDiffGroup[] {
return chunks.map(chunk => {
const group = this.chunkToGroup(chunk, offsetLeft, offsetRight);
- const chunkLength = this.commonChunkLength(chunk);
- offsetLeft += chunkLength;
- offsetRight += chunkLength;
+ offsetLeft += this.chunkLength(chunk, Side.LEFT);
+ offsetRight += this.chunkLength(chunk, Side.RIGHT);
return group;
});
}
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 f6b9737..3f01096 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
@@ -933,6 +933,128 @@
);
});
});
+
+ suite('with diffRangesToFocus', () => {
+ let state: State;
+ let chunks: DiffContent[];
+
+ setup(() => {
+ state = {
+ lineNums: {left: 0, right: 0},
+ chunkIndex: 0,
+ };
+ processor.context = 3;
+ processor.diffRangesToFocus = {
+ left: [{start: 6, end: 7}],
+ right: [{start: 6, end: 6}],
+ };
+ chunks = [
+ {
+ ab: Array.from<string>({length: 5}).fill(
+ 'all work and no play make jill a dull boy'
+ ),
+ },
+ {
+ a: ['Old ', ' Change!'],
+ b: ['New Change'],
+ },
+ {
+ ab: Array.from<string>({length: 5}).fill(
+ 'all work and no play make jack a dull boy'
+ ),
+ },
+ {
+ a: ['Old ', ' Change!', '1'],
+ b: ['New Change', '2'],
+ },
+ ];
+ });
+
+ test('focussed group is not collapsed in context control group', () => {
+ const result = processor.processNext(state, chunks);
+
+ // This should consider second delta group as focussed and not collapse it.
+ // This result is first chunk itself.
+ assert.equal(result.groups.length, 1);
+ assert.equal(result.groups[0].type, GrDiffGroupType.BOTH);
+ assert.equal(result.groups[0].lines.length, 5);
+ });
+
+ test('collapsing delta group at end in context control group', () => {
+ state = {
+ lineNums: {left: 7, right: 6},
+ chunkIndex: 2,
+ };
+ const result = processor.processNext(state, [
+ ...chunks,
+ {
+ ab: Array.from<string>({length: 5}).fill(
+ 'all work and no play make jack a dull boy'
+ ),
+ },
+ ]);
+
+ // The first chunk is split into two groups:
+ // 1) A common group which is rendered before contextControl group
+ // 2) Second group is a context control which contains split from 4th chunk
+ // and the delta group and the last unchanged group.
+ assert.equal(result.groups.length, 2);
+ assert.equal(result.groups[0].type, GrDiffGroupType.BOTH);
+ assert.equal(result.groups[0].lines.length, 3);
+ assert.equal(result.groups[1].type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.equal(result.groups[1].contextGroups.length, 3);
+ assert.equal(result.groups[1].contextGroups[0].lines.length, 2);
+ assert.equal(
+ result.groups[1].contextGroups[1].type,
+ GrDiffGroupType.DELTA
+ );
+ assert.equal(
+ result.groups[1].contextGroups[2].type,
+ GrDiffGroupType.BOTH
+ );
+ });
+
+ test('collapsing delta group in middle in context control group', () => {
+ state = {
+ lineNums: {left: 7, right: 6},
+ chunkIndex: 2,
+ };
+ const result = processor.processNext(state, chunks);
+
+ // The first chunk is split into two groups:
+ // 1) A common group which is rendered before contextControl group
+ // 2) Second group is a context control which contains split from 4th chunk
+ // and the delta group.
+ assert.equal(result.groups.length, 2);
+ assert.equal(result.groups[0].type, GrDiffGroupType.BOTH);
+ assert.equal(result.groups[0].lines.length, 3);
+ assert.equal(result.groups[1].type, GrDiffGroupType.CONTEXT_CONTROL);
+ assert.equal(result.groups[1].contextGroups.length, 2);
+ assert.equal(result.groups[1].contextGroups[0].lines.length, 2);
+ assert.equal(
+ result.groups[1].contextGroups[1].type,
+ GrDiffGroupType.DELTA
+ );
+ });
+
+ test('do not collapse if there are not enough context lines', () => {
+ processor.context = 10;
+ state = {
+ lineNums: {left: 7, right: 6},
+ chunkIndex: 2,
+ };
+ const result = processor.processNext(state, chunks);
+
+ // The first chunk is split into two groups:
+ // 1) A common group which is rendered before contextControl group
+ // 2) Second group is a context control which contains split from 4th chunk
+ // and the delta group.
+ assert.equal(result.groups.length, 2);
+ assert.equal(result.groups[0].type, GrDiffGroupType.BOTH);
+ assert.equal(result.groups[0].lines.length, 5);
+ assert.equal(result.groups[1].type, GrDiffGroupType.DELTA);
+ });
+ });
});
suite('gr-diff-processor helpers', () => {
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 c7e9f44..e7df002 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
@@ -42,40 +42,56 @@
* @param hiddenStart The first element to be hidden, as a
* non-negative line number offset relative to the first group's start
* line, left and right respectively.
- * @param hiddenEnd The first visible element after the hidden range,
- * as a non-negative line number offset relative to the first group's
- * start line, left and right respectively.
+ * @param hiddenEndLeft The first visible element after the hidden range,
+ * as a non-negative line number offset for left side relative to the first
+ * group's start line.
+ * @param hiddenEndRight The first visible element after the hidden range,
+ * as a non-negative line number offset for right side relative to the first
+ * group's start line. If not provided hiddenEndLeft will be used.
*/
export function hideInContextControl(
groups: readonly GrDiffGroup[],
hiddenStart: number,
- hiddenEnd: number
+ hiddenEndLeft: number,
+ hiddenEndRight?: number
): GrDiffGroup[] {
if (groups.length === 0) return [];
// Clamp hiddenStart and hiddenEnd - inspired by e.g. substring
hiddenStart = Math.max(hiddenStart, 0);
- hiddenEnd = Math.max(hiddenEnd, hiddenStart);
+ hiddenEndLeft = Math.max(hiddenEndLeft, hiddenStart);
+ hiddenEndRight = Math.max(hiddenEndRight ?? hiddenEndLeft, hiddenStart);
let before: GrDiffGroup[] = [];
let hidden = groups;
let after: readonly GrDiffGroup[] = [];
- const numHidden = hiddenEnd - hiddenStart;
+ const numHiddenLeft = hiddenEndLeft - hiddenStart;
+ const numHiddenRight = hiddenEndRight - hiddenStart;
// Showing a context control row for less than 4 lines does not make much,
// because then that row would consume as much space as the collapsed code.
- if (numHidden > 3) {
+ if (numHiddenLeft > 3 && numHiddenRight > 3) {
if (hiddenStart) {
- [before, hidden] = splitCommonGroups(hidden, hiddenStart);
+ [before, hidden] = splitCommonGroups(hidden, hiddenStart, hiddenStart);
}
- if (hiddenEnd) {
- let beforeLength = 0;
+ if (hiddenEndLeft && hiddenEndRight) {
+ let beforeLengthLeft = 0;
+ let beforeLengthRight = 0;
if (before.length > 0) {
- const beforeStart = before[0].lineRange.left.start_line;
- const beforeEnd = before[before.length - 1].lineRange.left.end_line;
- beforeLength = beforeEnd - beforeStart + 1;
+ const beforeStartLeft = before[0].lineRange.left.start_line;
+ const beforeEndLeft = before[before.length - 1].lineRange.left.end_line;
+ beforeLengthLeft = beforeEndLeft - beforeStartLeft + 1;
+
+ const beforeStartRight = before[0].lineRange.right.start_line;
+ const beforeEndRight =
+ before[before.length - 1].lineRange.right.end_line;
+ beforeLengthRight = beforeEndRight - beforeStartRight + 1;
}
- [hidden, after] = splitCommonGroups(hidden, hiddenEnd - beforeLength);
+ [hidden, after] = splitCommonGroups(
+ hidden,
+ hiddenEndLeft - beforeLengthLeft,
+ hiddenEndRight - beforeLengthRight
+ );
}
} else {
[hidden, after] = [[], hidden];
@@ -168,18 +184,21 @@
* two groups, which will be put into the first and second list. Groups with
* type DELTA which are not common will not be split.
*
- * @param split A line number offset relative to the first group's
- * start line at which the groups should be split.
+ * @param splitLeft A line number offset for left side relative to the first
+ * group's start line at which the groups should be split.
+ * @param splitRight A line number offset for right side relative to the first
+ * group's start line at which the groups should be split.
* @return The outer array has 2 elements, the
* list of groups before and the list of groups after the split.
*/
function splitCommonGroups(
groups: readonly GrDiffGroup[],
- split: number
+ splitLeft: number,
+ splitRight: number
): GrDiffGroup[][] {
if (groups.length === 0) return [[], []];
- const leftSplit = groups[0].lineRange.left.start_line + split;
- const rightSplit = groups[0].lineRange.right.start_line + split;
+ const leftSplit = groups[0].lineRange.left.start_line + splitLeft;
+ const rightSplit = groups[0].lineRange.right.start_line + splitRight;
let isSplitDone = false;
const beforeGroups = [];
const afterGroups = [];
