diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0db139ff05..12fdfe601f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1020,6 +1020,9 @@ importers: debounce: specifier: ^2.1.1 version: 2.2.0 + diff: + specifier: ^5.2.0 + version: 5.2.0 fast-deep-equal: specifier: ^3.1.3 version: 3.1.3 @@ -1153,6 +1156,9 @@ importers: '@testing-library/user-event': specifier: ^14.6.1 version: 14.6.1(@testing-library/dom@10.4.0) + '@types/diff': + specifier: ^5.2.1 + version: 5.2.3 '@types/jest': specifier: ^29.0.0 version: 29.5.14 @@ -14020,7 +14026,7 @@ snapshots: sirv: 3.0.1 tinyglobby: 0.2.14 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.2.1)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@20.17.57)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) '@vitest/utils@3.2.4': dependencies: diff --git a/src/core/diff/stats.ts b/src/core/diff/stats.ts new file mode 100644 index 0000000000..b842f5c04e --- /dev/null +++ b/src/core/diff/stats.ts @@ -0,0 +1,71 @@ +import { parsePatch, createTwoFilesPatch } from "diff" + +/** + * Diff utilities for backend (extension) use. + * Source of truth for diff normalization and stats. + */ + +export interface DiffStats { + added: number + removed: number +} + +/** + * Remove non-semantic diff noise like "No newline at end of file" + */ +export function sanitizeUnifiedDiff(diff: string): string { + if (!diff) return diff + return diff.replace(/\r\n/g, "\n").replace(/(^|\n)[ \t]*(?:\\ )?No newline at end of file[ \t]*(?=\n|$)/gi, "$1") +} + +/** + * Compute +/− counts from a unified diff (ignores headers/hunk lines) + */ +export function computeUnifiedDiffStats(diff?: string): DiffStats | null { + if (!diff) return null + + try { + const patches = parsePatch(diff) + if (!patches || patches.length === 0) return null + + let added = 0 + let removed = 0 + + for (const p of patches) { + for (const h of (p as any).hunks ?? []) { + for (const l of h.lines ?? []) { + const ch = (l as string)[0] + if (ch === "+") added++ + else if (ch === "-") removed++ + } + } + } + + if (added > 0 || removed > 0) return { added, removed } + return { added: 0, removed: 0 } + } catch { + // If parsing fails for any reason, signal no stats + return null + } +} + +/** + * Compute diff stats from any supported diff format (unified or search-replace) + * Tries unified diff format first, then falls back to search-replace format + */ +export function computeDiffStats(diff?: string): DiffStats | null { + if (!diff) return null + return computeUnifiedDiffStats(diff) +} + +/** + * Build a unified diff for a brand new file (all content lines are additions). + * Trailing newline is ignored for line counting and emission. + */ +export function convertNewFileToUnifiedDiff(content: string, filePath?: string): string { + const newFileName = filePath || "file" + // Normalize EOLs; rely on library for unified patch formatting + const normalized = (content || "").replace(/\r\n/g, "\n") + // Old file is empty (/dev/null), new file has content; zero context to show all lines as additions + return createTwoFilesPatch("/dev/null", newFileName, "", normalized, undefined, undefined, { context: 0 }) +} diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index 2f3ea87d4c..21703684b8 100644 --- a/src/core/prompts/responses.ts +++ b/src/core/prompts/responses.ts @@ -177,7 +177,9 @@ Otherwise, if you have not completed the task and do not need additional informa createPrettyPatch: (filename = "file", oldStr?: string, newStr?: string) => { // strings cannot be undefined or diff throws exception - const patch = diff.createPatch(filename.toPosix(), oldStr || "", newStr || "") + const patch = diff.createPatch(filename.toPosix(), oldStr || "", newStr || "", undefined, undefined, { + context: 3, + }) const lines = patch.split("\n") const prettyPatchLines = lines.slice(4) return prettyPatchLines.join("\n") diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index dcdd134624..1077b7bf39 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -13,6 +13,7 @@ import { fileExistsAtPath } from "../../utils/fs" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { unescapeHtmlEntities } from "../../utils/text-normalization" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" +import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" export async function applyDiffToolLegacy( cline: Task, @@ -140,6 +141,11 @@ export async function applyDiffToolLegacy( cline.consecutiveMistakeCount = 0 cline.consecutiveMistakeCountForApplyDiff.delete(relPath) + // Generate backend-unified diff for display in chat/webview + const unifiedPatchRaw = formatResponse.createPrettyPatch(relPath, originalContent, diffResult.content) + const unifiedPatch = sanitizeUnifiedDiff(unifiedPatchRaw) + const diffStats = computeDiffStats(unifiedPatch) || undefined + // Check if preventFocusDisruption experiment is enabled const provider = cline.providerRef.deref() const state = await provider?.getState() @@ -158,6 +164,8 @@ export async function applyDiffToolLegacy( const completeMessage = JSON.stringify({ ...sharedMessageProps, diff: diffContent, + content: unifiedPatch, + diffStats, isProtected: isWriteProtected, } satisfies ClineSayTool) @@ -194,6 +202,8 @@ export async function applyDiffToolLegacy( const completeMessage = JSON.stringify({ ...sharedMessageProps, diff: diffContent, + content: unifiedPatch, + diffStats, isProtected: isWriteProtected, } satisfies ClineSayTool) diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts index e7d3a06ab9..38ca309a3b 100644 --- a/src/core/tools/insertContentTool.ts +++ b/src/core/tools/insertContentTool.ts @@ -12,6 +12,7 @@ import { fileExistsAtPath } from "../../utils/fs" import { insertGroups } from "../diff/insert-groups" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" +import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" export async function insertContentTool( cline: Task, @@ -101,7 +102,7 @@ export async function insertContentTool( cline.diffViewProvider.originalContent = fileContent const lines = fileExists ? fileContent.split("\n") : [] - const updatedContent = insertGroups(lines, [ + let updatedContent = insertGroups(lines, [ { index: lineNumber - 1, elements: content.split("\n"), @@ -118,31 +119,31 @@ export async function insertContentTool( EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION, ) - // For consistency with writeToFileTool, handle new files differently - let diff: string | undefined - let approvalContent: string | undefined - + // Build unified diff for display (normalize EOLs only for diff generation) + let unified: string if (fileExists) { - // For existing files, generate diff and check for changes - diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent) - if (!diff) { + const oldForDiff = fileContent.replace(/\r\n/g, "\n") + const newForDiff = updatedContent.replace(/\r\n/g, "\n") + unified = formatResponse.createPrettyPatch(relPath, oldForDiff, newForDiff) + if (!unified) { pushToolResult(`No changes needed for '${relPath}'`) return } - approvalContent = undefined } else { - // For new files, skip diff generation and provide full content - diff = undefined - approvalContent = updatedContent + const newForDiff = updatedContent.replace(/\r\n/g, "\n") + unified = convertNewFileToUnifiedDiff(newForDiff, relPath) } + unified = sanitizeUnifiedDiff(unified) + const diffStats = computeDiffStats(unified) || undefined // Prepare the approval message (same for both flows) const completeMessage = JSON.stringify({ ...sharedMessageProps, - diff, - content: approvalContent, + // Send unified diff as content for render-only webview + content: unified, lineNumber: lineNumber, isProtected: isWriteProtected, + diffStats, } satisfies ClineSayTool) // Show diff view if focus disruption prevention is disabled diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts index a30778c5af..08bce08ede 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/src/core/tools/multiApplyDiffTool.ts @@ -15,6 +15,7 @@ import { unescapeHtmlEntities } from "../../utils/text-normalization" import { parseXmlForDiff } from "../../utils/xml" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" import { applyDiffToolLegacy } from "./applyDiffTool" +import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" interface DiffOperation { path: string @@ -282,31 +283,70 @@ Original error: ${errorMessage}` (opResult) => cline.rooProtectedController?.isWriteProtected(opResult.path) || false, ) - // Prepare batch diff data - const batchDiffs = operationsToApprove.map((opResult) => { + // Stream batch diffs progressively for better UX + const batchDiffs: Array<{ + path: string + changeCount: number + key: string + content: string + diffStats?: { added: number; removed: number } + diffs?: Array<{ content: string; startLine?: number }> + }> = [] + + for (const opResult of operationsToApprove) { const readablePath = getReadablePath(cline.cwd, opResult.path) const changeCount = opResult.diffItems?.length || 0 const changeText = changeCount === 1 ? "1 change" : `${changeCount} changes` - return { + let unified = "" + try { + const original = await fs.readFile(opResult.absolutePath!, "utf-8") + const processed = !cline.api.getModel().id.includes("claude") + ? (opResult.diffItems || []).map((item) => ({ + ...item, + content: item.content ? unescapeHtmlEntities(item.content) : item.content, + })) + : opResult.diffItems || [] + + const applyRes = + (await cline.diffStrategy?.applyDiff(original, processed)) ?? ({ success: false } as any) + const newContent = applyRes.success && applyRes.content ? applyRes.content : original + unified = formatResponse.createPrettyPatch(opResult.path, original, newContent) + } catch { + unified = "" + } + + const unifiedSanitized = sanitizeUnifiedDiff(unified) + const stats = computeDiffStats(unifiedSanitized) || undefined + batchDiffs.push({ path: readablePath, changeCount, key: `${readablePath} (${changeText})`, - content: opResult.path, // Full relative path + content: unifiedSanitized, + diffStats: stats, diffs: opResult.diffItems?.map((item) => ({ content: item.content, startLine: item.startLine, })), - } - }) + }) + + // Send a partial update after each file preview is ready + const partialMessage = JSON.stringify({ + tool: "appliedDiff", + batchDiffs, + isProtected: hasProtectedFiles, + } satisfies ClineSayTool) + await cline.ask("tool", partialMessage, true).catch(() => {}) + } + // Final approval message (non-partial) const completeMessage = JSON.stringify({ tool: "appliedDiff", batchDiffs, isProtected: hasProtectedFiles, } satisfies ClineSayTool) - const { response, text, images } = await cline.ask("tool", completeMessage, hasProtectedFiles) + const { response, text, images } = await cline.ask("tool", completeMessage, false) // Process batch response if (response === "yesButtonClicked") { @@ -418,6 +458,7 @@ Original error: ${errorMessage}` try { let originalContent: string | null = await fs.readFile(absolutePath, "utf-8") + let beforeContent: string | null = originalContent let successCount = 0 let formattedError = "" @@ -540,9 +581,13 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} if (operationsToApprove.length === 1) { // Prepare common data for single file operation const diffContents = diffItems.map((item) => item.content).join("\n\n") + const unifiedPatchRaw = formatResponse.createPrettyPatch(relPath, beforeContent!, originalContent!) + const unifiedPatch = sanitizeUnifiedDiff(unifiedPatchRaw) const operationMessage = JSON.stringify({ ...sharedMessageProps, diff: diffContents, + content: unifiedPatch, + diffStats: computeDiffStats(unifiedPatch) || undefined, } satisfies ClineSayTool) let toolProgressStatus diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index 5abd96a20a..b8e6da0caa 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -16,6 +16,7 @@ import { detectCodeOmission } from "../../integrations/editor/detect-omission" import { unescapeHtmlEntities } from "../../utils/text-normalization" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" +import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" export async function writeToFileTool( cline: Task, @@ -173,6 +174,15 @@ export async function writeToFileTool( if (isPreventFocusDisruptionEnabled) { // Direct file write without diff view + // Set up diffViewProvider properties needed for diff generation and saveDirectly + cline.diffViewProvider.editType = fileExists ? "modify" : "create" + if (fileExists) { + const absolutePath = path.resolve(cline.cwd, relPath) + cline.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8") + } else { + cline.diffViewProvider.originalContent = "" + } + // Check for code omissions before proceeding if (detectCodeOmission(cline.diffViewProvider.originalContent || "", newContent, predictedLineCount)) { if (cline.diffStrategy) { @@ -202,9 +212,15 @@ export async function writeToFileTool( } } + // Build unified diff for both existing and new files + let unified = fileExists + ? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent) + : convertNewFileToUnifiedDiff(newContent, relPath) + unified = sanitizeUnifiedDiff(unified) const completeMessage = JSON.stringify({ ...sharedMessageProps, - content: newContent, + content: unified, + diffStats: computeDiffStats(unified) || undefined, } satisfies ClineSayTool) const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected) @@ -213,15 +229,6 @@ export async function writeToFileTool( return } - // Set up diffViewProvider properties needed for saveDirectly - cline.diffViewProvider.editType = fileExists ? "modify" : "create" - if (fileExists) { - const absolutePath = path.resolve(cline.cwd, relPath) - cline.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8") - } else { - cline.diffViewProvider.originalContent = "" - } - // Save directly without showing diff view or opening the file await cline.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs) } else { @@ -275,12 +282,15 @@ export async function writeToFileTool( } } + // Build unified diff for both existing and new files + let unified = fileExists + ? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent) + : convertNewFileToUnifiedDiff(newContent, relPath) + unified = sanitizeUnifiedDiff(unified) const completeMessage = JSON.stringify({ ...sharedMessageProps, - content: fileExists ? undefined : newContent, - diff: fileExists - ? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent) - : undefined, + content: unified, + diffStats: computeDiffStats(unified) || undefined, } satisfies ClineSayTool) const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected) diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 7d2759c919..c3926d5073 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -386,6 +386,8 @@ export interface ClineSayTool { path?: string diff?: string content?: string + // Unified diff statistics computed by the extension + diffStats?: { added: number; removed: number } regex?: string filePattern?: string mode?: string @@ -407,6 +409,8 @@ export interface ClineSayTool { changeCount: number key: string content: string + // Per-file unified diff statistics computed by the extension + diffStats?: { added: number; removed: number } diffs?: Array<{ content: string startLine?: number diff --git a/webview-ui/package.json b/webview-ui/package.json index 9fda22097c..a2d35432a4 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -41,6 +41,7 @@ "cmdk": "^1.0.0", "date-fns": "^4.1.0", "debounce": "^2.1.1", + "diff": "^5.2.0", "fast-deep-equal": "^3.1.3", "fzf": "^0.5.2", "hast-util-to-jsx-runtime": "^2.3.6", @@ -87,6 +88,7 @@ "@testing-library/jest-dom": "^6.6.3", "@testing-library/react": "^16.2.0", "@testing-library/user-event": "^14.6.1", + "@types/diff": "^5.2.1", "@types/jest": "^29.0.0", "@types/katex": "^0.16.7", "@types/node": "20.x", diff --git a/webview-ui/src/components/chat/BatchDiffApproval.tsx b/webview-ui/src/components/chat/BatchDiffApproval.tsx index 24ad8d489d..a88914cd88 100644 --- a/webview-ui/src/components/chat/BatchDiffApproval.tsx +++ b/webview-ui/src/components/chat/BatchDiffApproval.tsx @@ -6,6 +6,7 @@ interface FileDiff { changeCount: number key: string content: string + diffStats?: { added: number; removed: number } diffs?: Array<{ content: string startLine?: number @@ -35,17 +36,18 @@ export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProp
{files.map((file) => { - // Combine all diffs into a single diff string for this file - const combinedDiff = file.diffs?.map((diff) => diff.content).join("\n\n") || file.content + // Use backend-provided unified diff only. Stats also provided by backend. + const unified = file.content || "" return (
handleToggleExpand(file.path)} + diffStats={file.diffStats ?? undefined} />
) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 4299240a54..23df05cc62 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -15,7 +15,6 @@ import { useExtensionState } from "@src/context/ExtensionStateContext" import { findMatchingResourceOrTemplate } from "@src/utils/mcp" import { vscode } from "@src/utils/vscode" import { formatPathTooltip } from "@src/utils/formatPathTooltip" -import { getLanguageFromPath } from "@src/utils/getLanguageFromPath" import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock" import UpdateTodoListToolBlock from "./UpdateTodoListToolBlock" @@ -336,6 +335,12 @@ export const ChatRowContent = ({ [message.ask, message.text], ) + // Unified diff content (provided by backend when relevant) + const unifiedDiff = useMemo(() => { + if (!tool) return undefined + return (tool.content ?? tool.diff) as string | undefined + }, [tool]) + const followUpData = useMemo(() => { if (message.type === "ask" && message.ask === "followup" && !message.partial) { return safeJsonParse(message.text) @@ -350,7 +355,7 @@ export const ChatRowContent = ({ style={{ color: "var(--vscode-foreground)", marginBottom: "-1.5px" }}> ) - switch (tool.tool) { + switch (tool.tool as string) { case "editedExistingFile": case "appliedDiff": // Check if this is a batch diff request @@ -391,12 +396,13 @@ export const ChatRowContent = ({
@@ -428,12 +434,47 @@ export const ChatRowContent = ({
+
+ + ) + case "searchAndReplace": + return ( + <> +
+ {tool.isProtected ? ( + + ) : ( + toolIcon("replace") + )} + + {tool.isProtected && message.type === "ask" + ? t("chat:fileOperations.wantsToEditProtected") + : message.type === "ask" + ? t("chat:fileOperations.wantsToSearchReplace") + : t("chat:fileOperations.didSearchReplace")} + +
+
+
@@ -465,7 +506,7 @@ export const ChatRowContent = ({ return ( { if (typeof vscode !== "undefined" && vscode?.postMessage) { vscode.postMessage({ type: "updateTodoList", payload: { todos: updatedTodos } }) @@ -496,12 +537,13 @@ export const ChatRowContent = ({
vscode.postMessage({ type: "openFile", text: "./" + tool.path })} + diffStats={tool.diffStats} />
diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 929fa9427a..b9e2323cbb 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -465,7 +465,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - // Reset UI states + // Reset UI states only when task changes setExpandedRows({}) everVisibleMessagesTsRef.current.clear() // Clear for new task setCurrentFollowUpTs(null) // Clear follow-up answered state for new task diff --git a/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx new file mode 100644 index 0000000000..61a6633f86 --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx @@ -0,0 +1,139 @@ +import React from "react" +import { render, screen } from "@/utils/test-utils" +import { QueryClient, QueryClientProvider } from "@tanstack/react-query" +import { ExtensionStateContextProvider } from "@src/context/ExtensionStateContext" +import { ChatRowContent } from "../ChatRow" + +// Mock i18n +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => { + const map: Record = { + "chat:fileOperations.wantsToEdit": "Roo wants to edit this file", + } + return map[key] || key + }, + }), + Trans: ({ children }: { children?: React.ReactNode }) => <>{children}, + initReactI18next: { type: "3rdParty", init: () => {} }, +})) + +// Mock CodeBlock (avoid ESM/highlighter costs) +vi.mock("@src/components/common/CodeBlock", () => ({ + default: () => null, +})) + +const queryClient = new QueryClient() + +function renderChatRow(message: any, isExpanded = false) { + return render( + + + {}} + onSuggestionClick={() => {}} + onBatchFileResponse={() => {}} + onFollowUpUnmount={() => {}} + isFollowUpAnswered={false} + /> + + , + ) +} + +describe("ChatRow - inline diff stats and actions", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("shows + and - counts for editedExistingFile ask", () => { + const diff = "@@ -1,1 +1,1 @@\n-old\n+new\n" + const message: any = { + type: "ask", + ask: "tool", + ts: Date.now(), + partial: false, + text: JSON.stringify({ + tool: "editedExistingFile", + path: "src/file.ts", + diff, + diffStats: { added: 1, removed: 1 }, + }), + } + + renderChatRow(message, false) + + // Plus/minus counts + expect(screen.getByText("+1")).toBeInTheDocument() + expect(screen.getByText("-1")).toBeInTheDocument() + }) + + it("derives counts from searchAndReplace diff", () => { + const diff = "-a\n-b\n+c\n" + const message: any = { + type: "ask", + ask: "tool", + ts: Date.now(), + partial: false, + text: JSON.stringify({ + tool: "searchAndReplace", + path: "src/file.ts", + diff, + diffStats: { added: 1, removed: 2 }, + }), + } + + renderChatRow(message) + + expect(screen.getByText("+1")).toBeInTheDocument() + expect(screen.getByText("-2")).toBeInTheDocument() + }) + + it("counts only added lines for newFileCreated (ignores diff headers)", () => { + const content = "a\nb\nc" + const message: any = { + type: "ask", + ask: "tool", + ts: Date.now(), + partial: false, + text: JSON.stringify({ + tool: "newFileCreated", + path: "src/new-file.ts", + content, + diffStats: { added: 3, removed: 0 }, + }), + } + + renderChatRow(message) + + // Should only count the three content lines as additions + expect(screen.getByText("+3")).toBeInTheDocument() + expect(screen.getByText("-0")).toBeInTheDocument() + }) + + it("counts only added lines for newFileCreated with trailing newline", () => { + const content = "a\nb\nc\n" + const message: any = { + type: "ask", + ask: "tool", + ts: Date.now(), + partial: false, + text: JSON.stringify({ + tool: "newFileCreated", + path: "src/new-file.ts", + content, + diffStats: { added: 3, removed: 0 }, + }), + } + + renderChatRow(message) + + // Trailing newline should not increase the added count + expect(screen.getByText("+3")).toBeInTheDocument() + expect(screen.getByText("-0")).toBeInTheDocument() + }) +}) diff --git a/webview-ui/src/components/common/CodeAccordian.tsx b/webview-ui/src/components/common/CodeAccordian.tsx index a86f9c3221..0d15bdb7db 100644 --- a/webview-ui/src/components/common/CodeAccordian.tsx +++ b/webview-ui/src/components/common/CodeAccordian.tsx @@ -7,6 +7,7 @@ import { formatPathTooltip } from "@src/utils/formatPathTooltip" import { ToolUseBlock, ToolUseBlockHeader } from "./ToolUseBlock" import CodeBlock from "./CodeBlock" import { PathTooltip } from "../ui/PathTooltip" +import DiffView from "./DiffView" interface CodeAccordianProps { path?: string @@ -19,6 +20,8 @@ interface CodeAccordianProps { onToggleExpand: () => void header?: string onJumpToFile?: () => void + // New props for diff stats + diffStats?: { added: number; removed: number } } const CodeAccordian = ({ @@ -32,11 +35,20 @@ const CodeAccordian = ({ onToggleExpand, header, onJumpToFile, + diffStats, }: CodeAccordianProps) => { const inferredLanguage = useMemo(() => language ?? (path ? getLanguageFromPath(path) : "txt"), [path, language]) const source = useMemo(() => code.trim(), [code]) const hasHeader = Boolean(path || isFeedback || header) + // Use provided diff stats only (render-only) + const derivedStats = useMemo(() => { + if (diffStats && (diffStats.added > 0 || diffStats.removed > 0)) return diffStats + return null + }, [diffStats]) + + const hasValidStats = Boolean(derivedStats && (derivedStats.added > 0 || derivedStats.removed > 0)) + return ( {hasHeader && ( @@ -67,13 +79,24 @@ const CodeAccordian = ({ )}
- {progressStatus && progressStatus.text && ( - <> - {progressStatus.icon && } - - {progressStatus.text} - - + {/* Prefer diff stats over generic progress indicator if available */} + {hasValidStats ? ( +
+ +{derivedStats!.added} + -{derivedStats!.removed} +
+ ) : ( + progressStatus && + progressStatus.text && ( + <> + {progressStatus.icon && ( + + )} + + {progressStatus.text} + + + ) )} {onJumpToFile && path && ( )} {(!hasHeader || isExpanded) && ( -
- +
+ {inferredLanguage === "diff" ? ( + + ) : ( + + )}
)} diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx new file mode 100644 index 0000000000..dc1c3c40c2 --- /dev/null +++ b/webview-ui/src/components/common/DiffView.tsx @@ -0,0 +1,247 @@ +import { memo, useMemo, useEffect, useState } from "react" +import { parseUnifiedDiff, type DiffLine } from "@src/utils/parseUnifiedDiff" +import { normalizeLanguage } from "@src/utils/highlighter" +import { getLanguageFromPath } from "@src/utils/getLanguageFromPath" +import { highlightHunks } from "@src/utils/highlightDiff" + +interface DiffViewProps { + source: string + filePath?: string +} + +// Interface for hunk data +interface Hunk { + lines: DiffLine[] + oldText: string + newText: string + highlightedOldLines?: React.ReactNode[] + highlightedNewLines?: React.ReactNode[] +} + +/** + * DiffView component renders unified diffs with side-by-side line numbers + * matching VSCode's diff editor style + */ +const DiffView = memo(({ source, filePath }: DiffViewProps) => { + // Determine language from file path + const normalizedLang = useMemo(() => normalizeLanguage(getLanguageFromPath(filePath || "") || "txt"), [filePath]) + + const isLightTheme = useMemo(() => { + if (typeof document === "undefined") return false + const cls = document.body.className + return /\bvscode-light\b|\bvscode-high-contrast-light\b/i.test(cls) + }, []) + + // Disable syntax highlighting for large diffs (performance optimization) + const shouldHighlight = useMemo(() => { + const lineCount = source.split("\n").length + return lineCount <= 1000 // Only highlight diffs with <= 1000 lines + }, [source]) + + // Parse diff and group into hunks + const diffLines = useMemo(() => parseUnifiedDiff(source, filePath), [source, filePath]) + + const hunks = useMemo(() => { + const result: Hunk[] = [] + let currentHunk: DiffLine[] = [] + + for (const line of diffLines) { + if (line.type === "gap") { + // Finish current hunk if it has content + if (currentHunk.length > 0) { + const oldLines: string[] = [] + const newLines: string[] = [] + + for (const hunkLine of currentHunk) { + if (hunkLine.type === "deletion" || hunkLine.type === "context") { + oldLines.push(hunkLine.content) + } + if (hunkLine.type === "addition" || hunkLine.type === "context") { + newLines.push(hunkLine.content) + } + } + + result.push({ + lines: [...currentHunk], + oldText: oldLines.join("\n"), + newText: newLines.join("\n"), + }) + } + + // Start new hunk with the gap + currentHunk = [line] + } else { + currentHunk.push(line) + } + } + + // Add the last hunk if it has content + if (currentHunk.length > 0 && currentHunk.some((line) => line.type !== "gap")) { + const oldLines: string[] = [] + const newLines: string[] = [] + + for (const hunkLine of currentHunk) { + if (hunkLine.type === "deletion" || hunkLine.type === "context") { + oldLines.push(hunkLine.content) + } + if (hunkLine.type === "addition" || hunkLine.type === "context") { + newLines.push(hunkLine.content) + } + } + + result.push({ + lines: [...currentHunk], + oldText: oldLines.join("\n"), + newText: newLines.join("\n"), + }) + } + + return result + }, [diffLines]) + + // State for the processed hunks with highlighting + const [processedHunks, setProcessedHunks] = useState(hunks) + + // Effect to handle async highlighting + useEffect(() => { + if (!shouldHighlight) { + setProcessedHunks(hunks) + return + } + + const processHunks = async () => { + const processed: Hunk[] = [] + + for (let i = 0; i < hunks.length; i++) { + const hunk = hunks[i] + try { + const highlighted = await highlightHunks( + hunk.oldText, + hunk.newText, + normalizedLang, + isLightTheme ? "light" : "dark", + i, + filePath, + ) + processed.push({ + ...hunk, + highlightedOldLines: highlighted.oldLines, + highlightedNewLines: highlighted.newLines, + }) + } catch { + // Fall back to unhighlighted on error + processed.push(hunk) + } + } + + setProcessedHunks(processed) + } + + processHunks() + }, [hunks, shouldHighlight, normalizedLang, isLightTheme, filePath]) + + // Render helper that uses precomputed highlighting + const renderContent = (line: DiffLine, hunk: Hunk, lineIndexInHunk: number): React.ReactNode => { + if (!shouldHighlight || !hunk.highlightedOldLines || !hunk.highlightedNewLines) { + return line.content + } + + // Find the line index within the old/new text for this hunk + const hunkLinesBeforeThis = hunk.lines.slice(0, lineIndexInHunk).filter((l) => l.type !== "gap") + + if (line.type === "deletion") { + // Count deletions and context lines before this line + const oldLineIndex = hunkLinesBeforeThis.filter((l) => l.type === "deletion" || l.type === "context").length + return hunk.highlightedOldLines[oldLineIndex] || line.content + } else if (line.type === "addition") { + // Count additions and context lines before this line + const newLineIndex = hunkLinesBeforeThis.filter((l) => l.type === "addition" || l.type === "context").length + return hunk.highlightedNewLines[newLineIndex] || line.content + } else if (line.type === "context") { + // For context lines, prefer new-side highlighting, fall back to old-side + const newLineIndex = hunkLinesBeforeThis.filter((l) => l.type === "addition" || l.type === "context").length + const oldLineIndex = hunkLinesBeforeThis.filter((l) => l.type === "deletion" || l.type === "context").length + return hunk.highlightedNewLines[newLineIndex] || hunk.highlightedOldLines[oldLineIndex] || line.content + } + + return line.content + } + + return ( +
+
+ + + {processedHunks.flatMap((hunk, hunkIndex) => + hunk.lines.map((line, lineIndex) => { + const globalIndex = `${hunkIndex}-${lineIndex}` + + // Render compact separator between hunks + if (line.type === "gap") { + return ( + + + + ) + } + + // Use VSCode's built-in diff editor color variables as classes for gutters + const gutterBgClass = + line.type === "addition" + ? "bg-[var(--vscode-diffEditor-insertedTextBackground)]" + : line.type === "deletion" + ? "bg-[var(--vscode-diffEditor-removedTextBackground)]" + : "bg-[var(--vscode-editorGroup-border)]" + + const contentBgClass = + line.type === "addition" + ? "diff-content-inserted" + : line.type === "deletion" + ? "diff-content-removed" + : "diff-content-context" + + const sign = line.type === "addition" ? "+" : line.type === "deletion" ? "-" : "" + + return ( + + {/* Old line number */} + + {/* New line number */} + + {/* Narrow colored gutter */} + + {/* Code content (no +/- prefix here) */} + + + ) + }), + )} + +
+ + + {/* +/- column (empty for gap) */} + + + {`${line.hiddenCount ?? 0} hidden lines`} +
+ {line.oldLineNum || ""} + + {line.newLineNum || ""} + + {/* +/- fixed column to prevent wrapping into it */} + + {sign} + + {renderContent(line, hunk, lineIndex)} +
+
+
+ ) +}) + +export default DiffView diff --git a/webview-ui/src/index.css b/webview-ui/src/index.css index 6f23892ced..eacc86f371 100644 --- a/webview-ui/src/index.css +++ b/webview-ui/src/index.css @@ -124,6 +124,7 @@ --color-vscode-titleBar-inactiveForeground: var(--vscode-titleBar-inactiveForeground); --color-vscode-charts-green: var(--vscode-charts-green); + --color-vscode-charts-red: var(--vscode-charts-red); --color-vscode-charts-yellow: var(--vscode-charts-yellow); --color-vscode-inputValidation-infoForeground: var(--vscode-inputValidation-infoForeground); @@ -490,3 +491,28 @@ input[cmdk-input]:focus { transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1); transition-duration: 150ms; } + +/* DiffView code font: use VS Code editor font and enable ligatures */ +.diff-view, +.diff-view pre, +.diff-view code, +.diff-view .hljs { + font-family: + var(--vscode-editor-font-family), ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", + "Courier New", monospace; + font-variant-ligatures: contextual; + font-feature-settings: + "calt" 1, + "liga" 1; +} + +/* DiffView background tints via CSS classes instead of inline styles */ +.diff-content-inserted { + background-color: color-mix(in srgb, var(--vscode-diffEditor-insertedTextBackground) 70%, transparent); +} +.diff-content-removed { + background-color: color-mix(in srgb, var(--vscode-diffEditor-removedTextBackground) 70%, transparent); +} +.diff-content-context { + background-color: color-mix(in srgb, var(--vscode-editorGroup-border) 100%, transparent); +} diff --git a/webview-ui/src/utils/__tests__/highlightDiff.spec.ts b/webview-ui/src/utils/__tests__/highlightDiff.spec.ts new file mode 100644 index 0000000000..46aa2231ef --- /dev/null +++ b/webview-ui/src/utils/__tests__/highlightDiff.spec.ts @@ -0,0 +1,223 @@ +import { highlightHunks } from "../highlightDiff" +import { getHighlighter } from "../highlighter" + +// Mock the highlighter +vi.mock("../highlighter", () => ({ + getHighlighter: vi.fn(), +})) + +// Mock hast-util-to-jsx-runtime +vi.mock("hast-util-to-jsx-runtime", () => ({ + toJsxRuntime: vi.fn((node, _options) => { + // Simple mock that returns a string representation + if (node.children) { + return node.children + .map((child: any) => { + if (child.type === "text") { + return child.value + } + return `${child.value || ""}` + }) + .join("") + } + return node.value || "highlighted-content" + }), +})) + +const mockHighlighter = { + codeToHast: vi.fn((text: string, options: any) => ({ + children: [ + { + children: [ + { + tagName: "code", + properties: { class: `hljs language-${options.lang}` }, + children: text.split("\n").map((line) => ({ + tagName: "span", + properties: { className: ["line"] }, + children: [{ type: "text", value: `highlighted(${line})` }], + })), + }, + ], + }, + ], + })), +} + +beforeEach(() => { + vi.clearAllMocks() + ;(getHighlighter as any).mockResolvedValue(mockHighlighter) +}) + +describe("highlightHunks", () => { + it("should highlight simple old and new text", async () => { + const result = await highlightHunks( + "const x = 1\nconsole.log(x)", + "const x = 2\nconsole.log(x)", + "javascript", + "light", + ) + + expect(result.oldLines).toHaveLength(2) + expect(result.newLines).toHaveLength(2) + expect(getHighlighter).toHaveBeenCalledWith("javascript") + expect(mockHighlighter.codeToHast).toHaveBeenCalledTimes(2) + }) + + it("should handle empty text", async () => { + const result = await highlightHunks("", "", "javascript", "light") + + expect(result.oldLines).toEqual([""]) + expect(result.newLines).toEqual([""]) + }) + + it("should handle single-line text", async () => { + const result = await highlightHunks("const x = 1", "const x = 2", "javascript", "dark") + + expect(result.oldLines).toHaveLength(1) + expect(result.newLines).toHaveLength(1) + expect(mockHighlighter.codeToHast).toHaveBeenCalledWith( + "const x = 1", + expect.objectContaining({ + lang: "javascript", + theme: "github-dark", + }), + ) + }) + + it("should handle multi-line text with different lengths", async () => { + const oldText = "line1\nline2\nline3" + const newText = "line1\nmodified line2" + + const result = await highlightHunks(oldText, newText, "txt", "light") + + expect(result.oldLines).toHaveLength(3) + expect(result.newLines).toHaveLength(2) + }) + + it("should map light theme to github-light", async () => { + await highlightHunks("test", "test", "javascript", "light") + + expect(mockHighlighter.codeToHast).toHaveBeenCalledWith( + "test", + expect.objectContaining({ + theme: "github-light", + }), + ) + }) + + it("should map dark theme to github-dark", async () => { + await highlightHunks("test", "test", "javascript", "dark") + + expect(mockHighlighter.codeToHast).toHaveBeenCalledWith( + "test", + expect.objectContaining({ + theme: "github-dark", + }), + ) + }) + + it("should use correct transformers", async () => { + await highlightHunks("test", "test", "javascript", "light") + + expect(mockHighlighter.codeToHast).toHaveBeenCalledWith( + "test", + expect.objectContaining({ + transformers: expect.arrayContaining([ + expect.objectContaining({ + pre: expect.any(Function), + code: expect.any(Function), + }), + ]), + }), + ) + }) + + it("should handle highlighting errors gracefully", async () => { + mockHighlighter.codeToHast.mockImplementation(() => { + throw new Error("Highlighting failed") + }) + + const result = await highlightHunks("const x = 1", "const x = 2", "javascript", "light") + + // Should fall back to plain text + expect(result.oldLines).toEqual(["const x = 1"]) + expect(result.newLines).toEqual(["const x = 2"]) + }) + + it("should handle getHighlighter rejection", async () => { + ;(getHighlighter as any).mockRejectedValueOnce(new Error("Highlighter failed")) + + const result = await highlightHunks("const x = 1", "const x = 2", "javascript", "light") + + // Should fall back to plain text + expect(result.oldLines).toEqual(["const x = 1"]) + expect(result.newLines).toEqual(["const x = 2"]) + }) + + it("should handle text with trailing newlines", async () => { + const result = await highlightHunks("line1\nline2\n", "line1\nline2\n", "txt", "light") + + expect(result.oldLines).toHaveLength(3) // Including empty line from trailing newline + expect(result.newLines).toHaveLength(3) + // The empty line at the end is preserved as-is (performance optimization) + expect(result.oldLines[2]).toBe("") + expect(result.newLines[2]).toBe("") + }) + + it("should preserve whitespace-only lines", async () => { + const result = await highlightHunks("line1\n \nline3", "line1\n\t\nline3", "txt", "light") + + expect(result.oldLines).toHaveLength(3) + expect(result.newLines).toHaveLength(3) + // Whitespace-only lines are preserved as-is (performance optimization) + expect(result.oldLines[1]).toBe(" ") + expect(result.newLines[1]).toBe("\t") + }) +}) + +describe("integration scenarios", () => { + it("should handle typical single hunk scenario", async () => { + const oldText = "function hello() {\n console.log('old')\n}" + const newText = "function hello() {\n console.log('new')\n}" + + const result = await highlightHunks(oldText, newText, "javascript", "light") + + expect(result.oldLines).toHaveLength(3) + expect(result.newLines).toHaveLength(3) + // Each line should be processed by the highlighter + result.oldLines.forEach((line) => { + expect(typeof line === "string" || typeof line === "object").toBe(true) + }) + }) + + it("should handle addition-only hunk", async () => { + const oldText = "" + const newText = "// New comment\nconst x = 1" + + const result = await highlightHunks(oldText, newText, "javascript", "light") + + expect(result.oldLines).toEqual([""]) + expect(result.newLines).toHaveLength(2) + }) + + it("should handle deletion-only hunk", async () => { + const oldText = "// Deleted comment\nconst x = 1" + const newText = "" + + const result = await highlightHunks(oldText, newText, "javascript", "light") + + expect(result.oldLines).toHaveLength(2) + expect(result.newLines).toEqual([""]) + }) + + it("should handle context with mixed changes", async () => { + const oldText = "line1\nold line\nline3\nold line2" + const newText = "line1\nnew line\nline3\nnew line2" + + const result = await highlightHunks(oldText, newText, "txt", "light") + + expect(result.oldLines).toHaveLength(4) + expect(result.newLines).toHaveLength(4) + }) +}) diff --git a/webview-ui/src/utils/highlightDiff.ts b/webview-ui/src/utils/highlightDiff.ts new file mode 100644 index 0000000000..a59745a8cb --- /dev/null +++ b/webview-ui/src/utils/highlightDiff.ts @@ -0,0 +1,131 @@ +import { ReactNode } from "react" +import { getHighlighter } from "./highlighter" +import { toJsxRuntime } from "hast-util-to-jsx-runtime" +import { Fragment, jsx, jsxs } from "react/jsx-runtime" + +/** + * Highlight two pieces of code (old and new) in a single pass and return + * arrays of ReactNode representing each line + */ +export async function highlightHunks( + oldText: string, + newText: string, + lang: string, + theme: "light" | "dark", + _hunkIndex = 0, + _filePath?: string, +): Promise<{ oldLines: ReactNode[]; newLines: ReactNode[] }> { + try { + const highlighter = await getHighlighter(lang) + const shikiTheme = theme === "light" ? "github-light" : "github-dark" + + // Helper to highlight text and extract lines + const highlightAndExtractLines = (text: string): ReactNode[] => { + const textLines = text.split("\n") + + if (!text.trim()) { + return textLines.map((line) => line || "") + } + + try { + // Use Shiki's line transformer to get per-line highlighting + const hast: any = highlighter.codeToHast(text, { + lang, + theme: shikiTheme, + transformers: [ + { + pre(node: any) { + node.properties.style = "padding:0;margin:0;background:none;" + return node + }, + code(node: any) { + node.properties.class = `hljs language-${lang}` + return node + }, + line(node: any, line: number) { + // Add a line marker to help with extraction + node.properties["data-line"] = line + return node + }, + }, + ], + }) + + // Extract the element's children (which should be line elements) + const codeEl = hast?.children?.[0]?.children?.[0] + if (!codeEl || !codeEl.children) { + return textLines.map((line) => line || "") + } + + // Convert each line element to a ReactNode + const highlightedLines: ReactNode[] = [] + + for (const lineNode of codeEl.children) { + if (lineNode.tagName === "span" && lineNode.properties?.className?.includes("line")) { + // This is a line span from Shiki + const reactNode = toJsxRuntime( + { type: "element", tagName: "span", properties: {}, children: lineNode.children || [] }, + { Fragment, jsx, jsxs }, + ) + highlightedLines.push(reactNode) + } + } + + // If we didn't get the expected structure, fall back to simple approach + if (highlightedLines.length !== textLines.length) { + // For each line, highlight it individually (fallback) + return textLines.map((line) => { + if (!line.trim()) return line + + try { + const lineHast: any = highlighter.codeToHast(line, { + lang, + theme: shikiTheme, + transformers: [ + { + pre(node: any) { + node.properties.style = "padding:0;margin:0;background:none;" + return node + }, + code(node: any) { + node.properties.class = `hljs language-${lang}` + return node + }, + }, + ], + }) + + const lineCodeEl = lineHast?.children?.[0]?.children?.[0] + if (!lineCodeEl || !lineCodeEl.children) { + return line + } + + return toJsxRuntime( + { type: "element", tagName: "span", properties: {}, children: lineCodeEl.children }, + { Fragment, jsx, jsxs }, + ) + } catch { + return line + } + }) + } + + return highlightedLines + } catch { + return textLines.map((line) => line || "") + } + } + + // Process both old and new text + const oldLines = highlightAndExtractLines(oldText) + const newLines = highlightAndExtractLines(newText) + + return { oldLines, newLines } + } catch { + // Fallback to plain text on any error + return { + oldLines: oldText.split("\n").map((line) => line || ""), + newLines: newText.split("\n").map((line) => line || ""), + } + } +} diff --git a/webview-ui/src/utils/parseUnifiedDiff.ts b/webview-ui/src/utils/parseUnifiedDiff.ts new file mode 100644 index 0000000000..bed84c4ca9 --- /dev/null +++ b/webview-ui/src/utils/parseUnifiedDiff.ts @@ -0,0 +1,96 @@ +import { parsePatch } from "diff" + +export interface DiffLine { + oldLineNum: number | null + newLineNum: number | null + type: "context" | "addition" | "deletion" | "gap" + content: string + hiddenCount?: number +} + +/** + * Parse a unified diff string into a flat list of renderable lines with + * line numbers, addition/deletion/context flags, and compact "gap" separators + * between hunks. + */ +export function parseUnifiedDiff(source: string, filePath?: string): DiffLine[] { + if (!source) return [] + + try { + const patches = parsePatch(source) + if (!patches || patches.length === 0) return [] + + const patch = filePath + ? (patches.find((p) => + [p.newFileName, p.oldFileName].some( + (n) => typeof n === "string" && (n === filePath || (n as string).endsWith("/" + filePath)), + ), + ) ?? patches[0]) + : patches[0] + + if (!patch) return [] + + const lines: DiffLine[] = [] + let prevHunk: any = null + for (const hunk of (patch as any).hunks || []) { + // Insert a compact "hidden lines" separator between hunks + if (prevHunk) { + const gapNew = hunk.newStart - (prevHunk.newStart + prevHunk.newLines) + const gapOld = hunk.oldStart - (prevHunk.oldStart + prevHunk.oldLines) + const hidden = Math.max(gapNew, gapOld) + if (hidden > 0) { + lines.push({ + oldLineNum: null, + newLineNum: null, + type: "gap", + content: "", + hiddenCount: hidden, + }) + } + } + + let oldLine = hunk.oldStart + let newLine = hunk.newStart + + for (const raw of hunk.lines || []) { + const firstChar = (raw as string)[0] + const content = (raw as string).slice(1) + + if (firstChar === "-") { + lines.push({ + oldLineNum: oldLine, + newLineNum: null, + type: "deletion", + content, + }) + oldLine++ + } else if (firstChar === "+") { + lines.push({ + oldLineNum: null, + newLineNum: newLine, + type: "addition", + content, + }) + newLine++ + } else { + // Context line + lines.push({ + oldLineNum: oldLine, + newLineNum: newLine, + type: "context", + content, + }) + oldLine++ + newLine++ + } + } + + prevHunk = hunk + } + + return lines + } catch { + // swallow parse errors and render nothing rather than breaking the UI + return [] + } +}