diff --git a/packages/server/src/server/utils/diff-highlighter.test.ts b/packages/server/src/server/utils/diff-highlighter.test.ts index 9bf93bdf1..25f4ac751 100644 --- a/packages/server/src/server/utils/diff-highlighter.test.ts +++ b/packages/server/src/server/utils/diff-highlighter.test.ts @@ -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 }); + } + }); +}); diff --git a/packages/server/src/server/utils/diff-highlighter.ts b/packages/server/src/server/utils/diff-highlighter.ts index 840be6e15..47dcf713f 100644 --- a/packages/server/src/server/utils/diff-highlighter.ts +++ b/packages/server/src/server/utils/diff-highlighter.ts @@ -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; + getNewFileContent?: (file: ParsedDiffFile) => Promise; +} + /** * Parse a unified diff into structured data */ @@ -196,6 +206,37 @@ function buildTokenLookup( return lookup; } +function buildFullFileTokenLookup( + fileContent: string, + path: string, +): Map { + const lookup = new Map(); + 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; + oldTokensByLine: Map; +} { + 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 { 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(); - 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 { 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; diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index ee8ca99e3..af7fabd80 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -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); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 37d1c13db..14b382afb 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -304,6 +304,23 @@ async function listCheckoutFileChanges(cwd: string, ref: string): Promise { + 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 { 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) {