Fix syntax layer to work with skip chunks and empty ranges
The diff produced for comment context has skip chunks and it may also
produce empty ranges for the last (empty) line, which messes up the
syntax layer and its process/notify/annotate workflow.
Release-Notes: skip
Google-Bug-Id: b/219684547
Change-Id: Ie8425729b7f1af914cc720918a8763b0c409c8d4
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 58926ec..5a92361 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -56,7 +56,7 @@
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {DiffLayer, RenderPreferences} from '../../../api/diff';
import {assertIsDefined} from '../../../utils/common-util';
-import {fire, fireAlert, waitForEventOnce} from '../../../utils/event-util';
+import {fire, fireAlert} from '../../../utils/event-util';
import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
import {anyLineTooLong} from '../../../embed/diff/gr-diff/gr-diff-utils';
@@ -679,9 +679,7 @@
}
if (!anyLineTooLong(diff)) {
- waitForEventOnce(this, 'render').then(() => {
- this.syntaxLayer.process(diff);
- });
+ this.syntaxLayer.process(diff);
}
return diff;
}
diff --git a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
index 92e8f09..dc79e36 100644
--- a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
+++ b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
@@ -177,6 +177,7 @@
for (const range of ranges) {
if (!CLASS_SAFELIST.has(range.className)) continue;
+ if (range.length === 0) continue;
GrAnnotation.annotateElement(
el,
range.start,
@@ -218,14 +219,21 @@
let rightContent = '';
for (const chunk of this.diff.content) {
const a = [...(chunk.a ?? []), ...(chunk.ab ?? [])];
- const b = [...(chunk.b ?? []), ...(chunk.ab ?? [])];
for (const line of a) {
leftContent += line + '\n';
}
+ const b = [...(chunk.b ?? []), ...(chunk.ab ?? [])];
for (const line of b) {
rightContent += line + '\n';
}
+ const skip = chunk.skip ?? 0;
+ if (skip > 0) {
+ leftContent += '\n'.repeat(skip);
+ rightContent += '\n'.repeat(skip);
+ }
}
+ leftContent = leftContent.trimEnd();
+ rightContent = rightContent.trimEnd();
const leftPromise = this.highlight(leftLanguage, leftContent);
const rightPromise = this.highlight(rightLanguage, rightContent);
diff --git a/polygerrit-ui/app/utils/syntax-util.ts b/polygerrit-ui/app/utils/syntax-util.ts
index 2710ead..d154400 100644
--- a/polygerrit-ui/app/utils/syntax-util.ts
+++ b/polygerrit-ui/app/utils/syntax-util.ts
@@ -58,7 +58,6 @@
for (let line of highlightedCode.split('\n')) {
const ranges: SyntaxLayerRange[] = [...carryOverRanges];
carryOverRanges = [];
- rangesPerLine.push({ranges});
// Remove all span tags one after another from left to right.
// For each opening <span ...> push a new (unclosed) range.
@@ -95,6 +94,7 @@
range.length = lineLength - range.start;
}
}
+ rangesPerLine.push({ranges: ranges.filter(r => r.length > 0)});
}
if (carryOverRanges.length > 0) {
throw new Error('unclosed <span>s in highlighted code');
diff --git a/polygerrit-ui/app/utils/syntax-util_test.ts b/polygerrit-ui/app/utils/syntax-util_test.ts
index 226f962..00464df 100644
--- a/polygerrit-ui/app/utils/syntax-util_test.ts
+++ b/polygerrit-ui/app/utils/syntax-util_test.ts
@@ -67,6 +67,19 @@
);
});
+ test('removal of empty spans', async () => {
+ assert.deepEqual(
+ highlightedStringToRanges('asdf<span class="c"></span>asdf'),
+ [{ranges: []}]
+ );
+ assert.deepEqual(
+ highlightedStringToRanges(
+ '<span class="d"></span>\n<span class="d"></span>'
+ ),
+ [{ranges: []}, {ranges: []}]
+ );
+ });
+
test('one line, two spans one after another', async () => {
assert.deepEqual(
highlightedStringToRanges(
