Fix deleted diff syntax highlighting
This commit is contained in:
@@ -1,9 +1,13 @@
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
parseDiff,
|
||||
reconstructNewFile,
|
||||
reconstructOldFile,
|
||||
highlightDiffFromHunks,
|
||||
highlightDiffWithFileContent,
|
||||
type ParsedDiffFile,
|
||||
} from "./diff-highlighter.js";
|
||||
|
||||
@@ -449,3 +453,64 @@ describe("highlightDiffFromHunks", () => {
|
||||
expect(addedLine!.tokens!.some((t) => t.style === "tag")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("highlightDiffWithFileContent", () => {
|
||||
it("uses the old file content to preserve syntax context for removed lines", async () => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "diff-highlight-old-file-"));
|
||||
|
||||
try {
|
||||
const oldFileContent = `/*
|
||||
comment line 1
|
||||
comment line 2
|
||||
comment line 3
|
||||
comment line 4
|
||||
comment line 5
|
||||
comment line 6
|
||||
old comment line
|
||||
comment line 8
|
||||
*/
|
||||
const x = 1;
|
||||
`;
|
||||
const newFileContent = `/*
|
||||
comment line 1
|
||||
comment line 2
|
||||
comment line 3
|
||||
comment line 4
|
||||
comment line 5
|
||||
comment line 6
|
||||
new comment line
|
||||
comment line 8
|
||||
*/
|
||||
const x = 1;
|
||||
`;
|
||||
writeFileSync(join(cwd, "example.ts"), newFileContent, "utf8");
|
||||
|
||||
const diff = `diff --git a/example.ts b/example.ts
|
||||
index 1111111..2222222 100644
|
||||
--- a/example.ts
|
||||
+++ b/example.ts
|
||||
@@ -5,7 +5,7 @@
|
||||
comment line 4
|
||||
comment line 5
|
||||
comment line 6
|
||||
-old comment line
|
||||
+new comment line
|
||||
comment line 8
|
||||
*/
|
||||
const x = 1;
|
||||
`;
|
||||
|
||||
const file = parseDiff(diff)[0];
|
||||
const highlighted = await highlightDiffWithFileContent(file, cwd, {
|
||||
oldFileContent,
|
||||
});
|
||||
const removedLine = highlighted.hunks[0].lines.find((line) => line.type === "remove");
|
||||
const addedLine = highlighted.hunks[0].lines.find((line) => line.type === "add");
|
||||
|
||||
expect(addedLine?.tokens).toEqual([{ text: "new comment line", style: "comment" }]);
|
||||
expect(removedLine?.tokens).toEqual([{ text: "old comment line", style: "comment" }]);
|
||||
} finally {
|
||||
rmSync(cwd, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -26,6 +26,16 @@ export interface ParsedDiffFile {
|
||||
status?: "ok" | "too_large" | "binary";
|
||||
}
|
||||
|
||||
interface HighlightDiffWithFileContentOptions {
|
||||
oldFileContent?: string | null;
|
||||
newFileContent?: string | null;
|
||||
}
|
||||
|
||||
interface ParseAndHighlightDiffOptions {
|
||||
getOldFileContent?: (file: ParsedDiffFile) => Promise<string | null>;
|
||||
getNewFileContent?: (file: ParsedDiffFile) => Promise<string | null>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a unified diff into structured data
|
||||
*/
|
||||
@@ -196,6 +206,37 @@ function buildTokenLookup(
|
||||
return lookup;
|
||||
}
|
||||
|
||||
function buildFullFileTokenLookup(
|
||||
fileContent: string,
|
||||
path: string,
|
||||
): Map<number, HighlightToken[]> {
|
||||
const lookup = new Map<number, HighlightToken[]>();
|
||||
const highlighted = highlightCode(fileContent, path);
|
||||
|
||||
for (let i = 0; i < highlighted.length; i++) {
|
||||
lookup.set(i + 1, highlighted[i]);
|
||||
}
|
||||
|
||||
return lookup;
|
||||
}
|
||||
|
||||
function buildReconstructedTokenLookups(file: ParsedDiffFile): {
|
||||
newTokensByLine: Map<number, HighlightToken[]>;
|
||||
oldTokensByLine: Map<number, HighlightToken[]>;
|
||||
} {
|
||||
const newFileLines = reconstructNewFile(file.hunks);
|
||||
const oldFileLines = reconstructOldFile(file.hunks);
|
||||
const newFileContent = buildFileContent(newFileLines);
|
||||
const oldFileContent = buildFileContent(oldFileLines);
|
||||
const newHighlighted = highlightCode(newFileContent, file.path);
|
||||
const oldHighlighted = highlightCode(oldFileContent, file.path);
|
||||
|
||||
return {
|
||||
newTokensByLine: buildTokenLookup(newFileLines, newHighlighted),
|
||||
oldTokensByLine: buildTokenLookup(oldFileLines, oldHighlighted),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply syntax highlighting to diff hunks using reconstructed file content.
|
||||
* This is the fallback when actual file content is not available.
|
||||
@@ -205,21 +246,7 @@ export function highlightDiffFromHunks(file: ParsedDiffFile): ParsedDiffFile {
|
||||
return file;
|
||||
}
|
||||
|
||||
// Reconstruct both versions from hunks
|
||||
const newFileLines = reconstructNewFile(file.hunks);
|
||||
const oldFileLines = reconstructOldFile(file.hunks);
|
||||
|
||||
// Build complete file content strings for highlighting
|
||||
const newFileContent = buildFileContent(newFileLines);
|
||||
const oldFileContent = buildFileContent(oldFileLines);
|
||||
|
||||
// Highlight both versions
|
||||
const newHighlighted = highlightCode(newFileContent, file.path);
|
||||
const oldHighlighted = highlightCode(oldFileContent, file.path);
|
||||
|
||||
// Build lookup maps: line number -> tokens
|
||||
const newTokensByLine = buildTokenLookup(newFileLines, newHighlighted);
|
||||
const oldTokensByLine = buildTokenLookup(oldFileLines, oldHighlighted);
|
||||
const { newTokensByLine, oldTokensByLine } = buildReconstructedTokenLookups(file);
|
||||
|
||||
return applyTokensToHunks(file, newTokensByLine, oldTokensByLine);
|
||||
}
|
||||
@@ -231,37 +258,34 @@ export function highlightDiffFromHunks(file: ParsedDiffFile): ParsedDiffFile {
|
||||
export async function highlightDiffWithFileContent(
|
||||
file: ParsedDiffFile,
|
||||
cwd: string,
|
||||
options: HighlightDiffWithFileContentOptions = {},
|
||||
): Promise<ParsedDiffFile> {
|
||||
if (!isLanguageSupported(file.path)) {
|
||||
return file;
|
||||
}
|
||||
|
||||
const filePath = resolve(cwd, file.path);
|
||||
const reconstructedTokens = buildReconstructedTokenLookups(file);
|
||||
let newTokensByLine = reconstructedTokens.newTokensByLine;
|
||||
let oldTokensByLine = reconstructedTokens.oldTokensByLine;
|
||||
|
||||
try {
|
||||
// Read the current file content (the "new" version)
|
||||
const fileContent = await readFile(filePath, "utf-8");
|
||||
|
||||
// Highlight the entire file
|
||||
const highlighted = highlightCode(fileContent, file.path);
|
||||
|
||||
// Build lookup: line number (1-indexed) -> tokens
|
||||
const tokensByLine = new Map<number, HighlightToken[]>();
|
||||
for (let i = 0; i < highlighted.length; i++) {
|
||||
tokensByLine.set(i + 1, highlighted[i]);
|
||||
}
|
||||
|
||||
// For removed lines, we need to reconstruct from hunks since they're not in the file
|
||||
const oldFileLines = reconstructOldFile(file.hunks);
|
||||
const oldFileContent = buildFileContent(oldFileLines);
|
||||
const oldHighlighted = highlightCode(oldFileContent, file.path);
|
||||
const oldTokensByLine = buildTokenLookup(oldFileLines, oldHighlighted);
|
||||
|
||||
return applyTokensToHunks(file, tokensByLine, oldTokensByLine);
|
||||
} catch {
|
||||
// If file read fails (deleted file, etc.), fall back to hunk-based highlighting
|
||||
return highlightDiffFromHunks(file);
|
||||
if (typeof options.oldFileContent === "string") {
|
||||
oldTokensByLine = buildFullFileTokenLookup(options.oldFileContent, file.path);
|
||||
}
|
||||
|
||||
if (typeof options.newFileContent === "string") {
|
||||
newTokensByLine = buildFullFileTokenLookup(options.newFileContent, file.path);
|
||||
return applyTokensToHunks(file, newTokensByLine, oldTokensByLine);
|
||||
}
|
||||
|
||||
const filePath = resolve(cwd, file.path);
|
||||
try {
|
||||
const fileContent = await readFile(filePath, "utf-8");
|
||||
newTokensByLine = buildFullFileTokenLookup(fileContent, file.path);
|
||||
} catch {
|
||||
// If file read fails (deleted file, etc.), fall back to reconstructed new-side tokens.
|
||||
}
|
||||
|
||||
return applyTokensToHunks(file, newTokensByLine, oldTokensByLine);
|
||||
}
|
||||
|
||||
function applyTokensToHunks(
|
||||
@@ -308,11 +332,22 @@ function applyTokensToHunks(
|
||||
export async function parseAndHighlightDiff(
|
||||
diffText: string,
|
||||
cwd: string,
|
||||
options: ParseAndHighlightDiffOptions = {},
|
||||
): Promise<ParsedDiffFile[]> {
|
||||
const files = parseDiff(diffText);
|
||||
|
||||
const highlightedFiles = await Promise.all(
|
||||
files.map((file) => highlightDiffWithFileContent(file, cwd)),
|
||||
files.map(async (file) => {
|
||||
const [oldFileContent, newFileContent] = await Promise.all([
|
||||
options.getOldFileContent?.(file),
|
||||
options.getNewFileContent?.(file),
|
||||
]);
|
||||
|
||||
return highlightDiffWithFileContent(file, cwd, {
|
||||
oldFileContent: oldFileContent ?? undefined,
|
||||
newFileContent: newFileContent ?? undefined,
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
return highlightedFiles;
|
||||
|
||||
@@ -81,6 +81,49 @@ describe("checkout git utilities", () => {
|
||||
expect(message).toBe("update file");
|
||||
});
|
||||
|
||||
it("preserves removed-line syntax highlighting with structured diffs", async () => {
|
||||
const originalContent = `/*
|
||||
comment line 1
|
||||
comment line 2
|
||||
comment line 3
|
||||
comment line 4
|
||||
comment line 5
|
||||
comment line 6
|
||||
old comment line
|
||||
comment line 8
|
||||
*/
|
||||
const x = 1;
|
||||
`;
|
||||
const updatedContent = `/*
|
||||
comment line 1
|
||||
comment line 2
|
||||
comment line 3
|
||||
comment line 4
|
||||
comment line 5
|
||||
comment line 6
|
||||
new comment line
|
||||
comment line 8
|
||||
*/
|
||||
const x = 1;
|
||||
`;
|
||||
|
||||
writeFileSync(join(repoDir, "example.ts"), originalContent);
|
||||
execSync("git add example.ts", { cwd: repoDir });
|
||||
execSync("git -c commit.gpgsign=false commit -m 'add multiline comment fixture'", {
|
||||
cwd: repoDir,
|
||||
});
|
||||
|
||||
writeFileSync(join(repoDir, "example.ts"), updatedContent);
|
||||
|
||||
const diff = await getCheckoutDiff(repoDir, { mode: "uncommitted", includeStructured: true });
|
||||
const file = diff.structured?.find((entry) => entry.path === "example.ts");
|
||||
const removedLine = file?.hunks[0]?.lines.find((line) => line.type === "remove");
|
||||
const addedLine = file?.hunks[0]?.lines.find((line) => line.type === "add");
|
||||
|
||||
expect(addedLine?.tokens).toEqual([{ text: "new comment line", style: "comment" }]);
|
||||
expect(removedLine?.tokens).toEqual([{ text: "old comment line", style: "comment" }]);
|
||||
});
|
||||
|
||||
it("returns lightweight checkout status for normal repos", async () => {
|
||||
const status = await getCheckoutStatusLite(repoDir);
|
||||
expect(status.isGit).toBe(true);
|
||||
|
||||
@@ -304,6 +304,23 @@ async function listCheckoutFileChanges(cwd: string, ref: string): Promise<Checko
|
||||
return Array.from(byPath.values());
|
||||
}
|
||||
|
||||
async function readGitFileContentAtRef(
|
||||
cwd: string,
|
||||
ref: string,
|
||||
path: string,
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync("git", ["show", `${ref}:${path}`], {
|
||||
cwd,
|
||||
env: READ_ONLY_GIT_ENV,
|
||||
maxBuffer: SMALL_OUTPUT_MAX_BUFFER,
|
||||
});
|
||||
return stdout;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async function tryResolveMergeBase(cwd: string, baseRef: string): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await execGit(`git merge-base ${baseRef} HEAD`, {
|
||||
@@ -1239,6 +1256,7 @@ export async function getCheckoutDiff(
|
||||
|
||||
const trackedChanges = changes.filter((change) => !change.isUntracked);
|
||||
const untrackedChanges = changes.filter((change) => change.isUntracked === true);
|
||||
const trackedChangeByPath = new Map(trackedChanges.map((change) => [change.path, change]));
|
||||
|
||||
const trackedNumstatByPath =
|
||||
trackedChanges.length > 0
|
||||
@@ -1294,7 +1312,18 @@ export async function getCheckoutDiff(
|
||||
|
||||
if (compare.includeStructured) {
|
||||
const parsedTrackedFiles =
|
||||
trackedDiffText.length > 0 ? await parseAndHighlightDiff(trackedDiffText, cwd) : [];
|
||||
trackedDiffText.length > 0
|
||||
? await parseAndHighlightDiff(trackedDiffText, cwd, {
|
||||
getOldFileContent: async (file) => {
|
||||
const change = trackedChangeByPath.get(file.path);
|
||||
if (!change || change.isNew) {
|
||||
return null;
|
||||
}
|
||||
const refPath = change.oldPath ?? change.path;
|
||||
return readGitFileContentAtRef(cwd, refForDiff, refPath);
|
||||
},
|
||||
})
|
||||
: [];
|
||||
const parsedTrackedByPath = new Map(parsedTrackedFiles.map((file) => [file.path, file]));
|
||||
|
||||
for (const change of trackedChanges) {
|
||||
|
||||
Reference in New Issue
Block a user