Skip to content

Commit 80adcd6

Browse files
cm-mm3-supportmeta-codesync[bot]
authored andcommitted
Highlight diff hunks using the file lines above them
Summary: The comparison view tokenizes each hunk on its own, starting from the grammar's initial state. When a hunk starts inside a multi-line construct, such as a template literal, triple-quoted string, or block comment, the closing delimiter is read as an opening one. Everything after it in the hunk then flips: code is colored as a string, and strings as code. The fix fetches the lines of the "before" file above the last hunk, using the same `requestComparisonContextLines` message that expanding context already uses. The worker then tokenizes the unchanged lines between hunks and carries the grammar state into each hunk, on both sides. Lines between hunks are unchanged, so the same lines advance both sides' state. - `tokenizeLinesFromState` in `shared/textmate-lib/tokenize.ts` starts from a given state and returns the state after the last line; `tokenizeLines` calls it with `INITIAL`. - `SplitDiffTable` fetches up to 10,000 context lines when the view provides `fetchAdditionalLines`. Hunks are highlighted without context first, as before, and highlighted again once the lines arrive. Views without `fetchAdditionalLines`, files above the cap, and failed fetches (an error result or a rejected promise) keep the current behavior. - `comparisonContextLines` now echoes the request's `start` and `numLines`, and the client matches on them. Before this, the client matched only on path, so two requests for the same file in flight at once (now more likely, since the view fetches context on open) could both resolve with the first response. Responses from older servers without these fields still match on path alone. Reviewed By: cfirimar Differential Revision: D125599812 fbshipit-source-id: 150b27f7493edbb929c128c51c17a2bed951fa2e
1 parent 97a250e commit 80adcd6

9 files changed

Lines changed: 300 additions & 21 deletions

File tree

‎addons/isl-server/src/ServerToClientAPI.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,13 +645,17 @@ export default class ServerToClientAPI {
645645
type: 'comparisonContextLines',
646646
lines: {value: content.split('\n').slice(start - 1, start - 1 + numLines)},
647647
path: relativePath,
648+
start,
649+
numLines,
648650
}),
649651
)
650652
.catch((error: Error) =>
651653
this.postMessage({
652654
type: 'comparisonContextLines',
653655
lines: {error},
654656
path: relativePath,
657+
start,
658+
numLines,
655659
}),
656660
);
657661
break;

‎addons/isl/src/ComparisonView/ComparisonView.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,10 @@ function ComparisonViewFile({
628628

629629
const result = await serverAPI.nextMessageMatching(
630630
'comparisonContextLines',
631-
msg => msg.path === id.path,
631+
msg =>
632+
msg.path === id.path &&
633+
// Older servers don't say which lines they sent.
634+
(msg.start == null || (msg.start === start && msg.numLines === numLines)),
632635
);
633636

634637
return result.lines;

‎addons/isl/src/ComparisonView/SplitDiffView/SplitDiffHunk.tsx‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {useTableColumnSelection} from './copyFromSelectedColumn';
2626
import {useTokenizedContents, useTokenizedHunks} from './syntaxHighlighting';
2727

2828
const MAX_INPUT_LENGTH_FOR_INTRALINE_DIFF = 300;
29+
const MAX_CONTEXT_LINES_FOR_SYNTAX_HIGHLIGHTING = 10000;
2930

3031
export type SplitDiffTableProps = {
3132
ctx: Context;
@@ -49,7 +50,13 @@ export const SplitDiffTable = React.memo(
4950
);
5051

5152
const t = ctx.t ?? (s => s);
52-
const tokenization = useTokenizedHunks(patch.newFileName ?? '', patch.hunks, ctx.useThemeHook);
53+
const contextLinesBefore = useContextLinesBeforeHunks(ctx, patch.hunks);
54+
const tokenization = useTokenizedHunks(
55+
patch.newFileName ?? '',
56+
patch.hunks,
57+
ctx.useThemeHook,
58+
contextLinesBefore,
59+
);
5360

5461
const {className: tableSelectionClassName, ...tableSelectionProps} = useTableColumnSelection();
5562

@@ -587,3 +594,50 @@ function useFetchLines(ctx: Context, numLines: number, start: number) {
587594

588595
return fetchedLines;
589596
}
597+
598+
/**
599+
* Fetches the lines of the "before" file above the last hunk, so syntax highlighting can
600+
* start each hunk in the right state, for example inside a multi-line string or comment.
601+
*/
602+
function useContextLinesBeforeHunks(
603+
ctx: Context,
604+
hunks: ParsedDiff['hunks'],
605+
): Array<string> | undefined {
606+
const [fetched, setFetched] = useState<{key: string; lines: Array<string>} | undefined>(
607+
undefined,
608+
);
609+
610+
const lastHunk = hunks.at(-1);
611+
// A hunk that only adds lines comes after line `oldStart`, so that line is context too.
612+
const numLines =
613+
lastHunk == null ? 0 : lastHunk.oldLines === 0 ? lastHunk.oldStart : lastHunk.oldStart - 1;
614+
const shouldFetch = numLines > 0 && numLines <= MAX_CONTEXT_LINES_FOR_SYNTAX_HIGHLIGHTING;
615+
616+
// Use-case controlled key that allows invalidating the fetched lines.
617+
const invalidationKey = ctx.useComparisonInvalidationKeyHook?.();
618+
619+
const comparisonKey = comparisonStringKey(ctx.id.comparison);
620+
// Lines fetched for an earlier comparison or file must not be used for this one.
621+
const key = `${invalidationKey}:${comparisonKey}:${ctx.id.path}:${numLines}`;
622+
useEffect(() => {
623+
if (!shouldFetch) {
624+
return;
625+
}
626+
let isCancelled = false;
627+
ctx
628+
.fetchAdditionalLines?.(ctx.id, 1, numLines)
629+
.then(result => {
630+
if (!isCancelled && result.value != null) {
631+
setFetched({key, lines: result.value});
632+
}
633+
})
634+
// Without the lines, hunks keep their per-hunk highlighting.
635+
.catch(() => undefined);
636+
return () => {
637+
isCancelled = true;
638+
};
639+
// eslint-disable-next-line react-hooks/exhaustive-deps
640+
}, [key, shouldFetch]);
641+
642+
return shouldFetch && fetched?.key === key ? fetched.lines : undefined;
643+
}

‎addons/isl/src/ComparisonView/SplitDiffView/syntaxHighlighting.tsx‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,14 @@ function getWorker(): Promise<WorkerApi<SyntaxWorkerRequest, SyntaxWorkerRespons
110110
* Given a set of hunks from a diff view,
111111
* asynchronously provide syntax highlighting by tokenizing.
112112
*
113-
* Note that we reconstruct the file contents from the diff,
114-
* so syntax highlighting can be inaccurate since it's missing full context.
113+
* Note that we reconstruct the file contents from the diff. Without `contextLinesBefore`,
114+
* syntax highlighting can be inaccurate since it's missing the lines above each hunk.
115115
*/
116116
export function useTokenizedHunks(
117117
path: string,
118118
hunks: ParsedDiff['hunks'],
119119
useThemeHook: () => ThemeColor,
120+
contextLinesBefore?: Array<string>,
120121
): TokenizedDiffHunks | undefined {
121122
const theme = useThemeHook();
122123

@@ -125,14 +126,16 @@ export function useTokenizedHunks(
125126
useEffect(() => {
126127
const token = newTrackedCancellationToken();
127128
getWorker().then(worker =>
128-
worker.request({type: 'tokenizeHunks', theme, path, hunks}, token).then(result => {
129-
if (!token.isCancelled) {
130-
setTokenized(result.result);
131-
}
132-
}),
129+
worker
130+
.request({type: 'tokenizeHunks', theme, path, hunks, contextLinesBefore}, token)
131+
.then(result => {
132+
if (!token.isCancelled) {
133+
setTokenized(result.result);
134+
}
135+
}),
133136
);
134137
return () => token.cancel();
135-
}, [theme, path, hunks]);
138+
}, [theme, path, hunks, contextLinesBefore]);
136139
return tokenized;
137140
}
138141

‎addons/isl/src/ComparisonView/SplitDiffView/syntaxHighlightingTypes.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ export type SyntaxWorkerRequest =
4242
theme: ThemeColor;
4343
path: string;
4444
hunks: ParsedDiff['hunks'];
45+
/**
46+
* Lines of the "before" file, starting at line 1, that come before the last hunk.
47+
* Lets each hunk start from the grammar state the lines above it leave, for example inside a multi-line string.
48+
*/
49+
contextLinesBefore?: Array<string>;
4550
};
4651

4752
export type SyntaxWorkerResponse =

‎addons/isl/src/ComparisonView/SplitDiffView/syntaxHighlightingWorker.ts‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8+
import type {ParsedDiff} from 'shared/patch/types';
89
import type {IGrammar} from 'vscode-textmate';
910
import type {ThemeColor} from '../../theme';
1011
import type {
@@ -16,8 +17,9 @@ import type {
1617

1718
import {CancellationToken} from 'shared/CancellationToken';
1819
import FilepathClassifier from 'shared/textmate-lib/FilepathClassifier';
19-
import {tokenizeLines} from 'shared/textmate-lib/tokenize';
20+
import {tokenizeLines, tokenizeLinesFromState} from 'shared/textmate-lib/tokenize';
2021
import {loadWASM} from 'vscode-oniguruma';
22+
import {INITIAL} from 'vscode-textmate';
2123
import {grammars, languages} from '../../generated/textmate/TextMateGrammarManifest';
2224
import {getGrammar, getGrammarStore} from './grammar';
2325

@@ -151,7 +153,7 @@ export function handleMessage(
151153
case 'tokenizeHunks': {
152154
workQueue.push(async () => {
153155
const grammar = await loadGrammar(data.theme, data.path, postMessage);
154-
const result = tokenizeHunks(grammar, data.hunks, token);
156+
const result = tokenizeHunks(grammar, data.hunks, data.contextLinesBefore, token);
155157
postMessage({type: data.type, id: data.id, result});
156158
cancellationTokenForId.delete(data.id);
157159
});
@@ -172,7 +174,8 @@ if (typeof self.document === 'undefined') {
172174

173175
function tokenizeHunks(
174176
grammar: IGrammar | undefined,
175-
hunks: Array<{lines: Array<string>}>,
177+
hunks: ParsedDiff['hunks'],
178+
contextLinesBefore: Array<string> | undefined,
176179
cancellationToken: CancellationToken,
177180
): TokenizedDiffHunks | undefined {
178181
if (grammar == null) {
@@ -184,11 +187,32 @@ function tokenizeHunks(
184187
return undefined;
185188
}
186189

187-
const tokenizedPatches: TokenizedDiffHunks = hunks
188-
.map(hunk => recoverFileContentsFromPatchLines(hunk.lines))
189-
.map(([before, after]) => [tokenizeLines(before, grammar), tokenizeLines(after, grammar)]);
190+
if (contextLinesBefore == null) {
191+
return hunks
192+
.map(hunk => recoverFileContentsFromPatchLines(hunk.lines))
193+
.map(([before, after]) => [tokenizeLines(before, grammar), tokenizeLines(after, grammar)]);
194+
}
190195

191-
return tokenizedPatches;
196+
let beforeState = INITIAL;
197+
let afterState = INITIAL;
198+
// 1-indexed line number in the "before" file of the first line not yet tokenized.
199+
let nextLine = 1;
200+
return hunks.map(hunk => {
201+
// A hunk that only adds lines comes after line `oldStart`, rather than starting at it.
202+
const hunkStart = hunk.oldLines === 0 ? hunk.oldStart + 1 : hunk.oldStart;
203+
// Lines between hunks are unchanged, so they are the same on both sides.
204+
const unchangedLines = contextLinesBefore.slice(nextLine - 1, hunkStart - 1);
205+
beforeState = tokenizeLinesFromState(unchangedLines, grammar, beforeState).state;
206+
afterState = tokenizeLinesFromState(unchangedLines, grammar, afterState).state;
207+
nextLine = hunkStart + hunk.oldLines;
208+
209+
const [before, after] = recoverFileContentsFromPatchLines(hunk.lines);
210+
const tokenizedBefore = tokenizeLinesFromState(before, grammar, beforeState);
211+
const tokenizedAfter = tokenizeLinesFromState(after, grammar, afterState);
212+
beforeState = tokenizedBefore.state;
213+
afterState = tokenizedAfter.state;
214+
return [tokenizedBefore.tokens, tokenizedAfter.tokens];
215+
});
192216
}
193217

194218
function tokenizeContent(

0 commit comments

Comments
 (0)