From db288a67a6e8fd30f9e7b71b8cc67307d0340257 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 10:02:20 -0600 Subject: [PATCH 01/22] =?UTF-8?q?feat(chat):=20improve=20diff=20appearance?= =?UTF-8?q?=20in=20main=20chat=20view=20-=20Add=20DiffView=20with=20line?= =?UTF-8?q?=20numbers,=20+/-=20column,=20wrapping=20-=20Show=20inline=20+/?= =?UTF-8?q?=E2=88=92=20counts=20in=20headers=20-=20Convert=20SEARCH/REPLAC?= =?UTF-8?q?E=20to=20unified=20diffs=20-=20Remove=20Undo=20mechanism=20(UI/?= =?UTF-8?q?backend)=20-=20Update=20tests=20and=20deps=20(diff,=20@types/di?= =?UTF-8?q?ff)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pnpm-lock.yaml | 8 +- webview-ui/package.json | 2 + .../src/components/chat/BatchDiffApproval.tsx | 74 +++++- webview-ui/src/components/chat/ChatRow.tsx | 210 +++++++++++++++++- webview-ui/src/components/chat/ChatView.tsx | 2 +- .../__tests__/ChatRow.diff-actions.spec.tsx | 93 ++++++++ .../src/components/common/CodeAccordian.tsx | 103 ++++++++- webview-ui/src/components/common/DiffView.tsx | 181 +++++++++++++++ 8 files changed, 657 insertions(+), 16 deletions(-) create mode 100644 webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx create mode 100644 webview-ui/src/components/common/DiffView.tsx diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0db139ff05a..12fdfe601fc 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/webview-ui/package.json b/webview-ui/package.json index 9fda22097c9..a2d35432a45 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 24ad8d489d9..5338017af39 100644 --- a/webview-ui/src/components/chat/BatchDiffApproval.tsx +++ b/webview-ui/src/components/chat/BatchDiffApproval.tsx @@ -1,4 +1,5 @@ import React, { memo, useState } from "react" +import { structuredPatch } from "diff" import CodeAccordian from "../common/CodeAccordian" interface FileDiff { @@ -17,6 +18,63 @@ interface BatchDiffApprovalProps { ts: number } +/** + * Converts Roo's SEARCH/REPLACE format to unified diff format for better readability + */ +function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { + const blockRegex = + /<<<<<>>>>>> REPLACE)/gim + + let hasBlocks = false + let combinedDiff = "" + const fileName = filePath || "file" + + let match: RegExpExecArray | null + while ((match = blockRegex.exec(content)) !== null) { + hasBlocks = true + const searchContent = (match[1] ?? "").replace(/\n$/, "") // Remove trailing newline + const replaceContent = (match[2] ?? "").replace(/\n$/, "") + + // Use the diff library to create a proper unified diff + const patch = structuredPatch(fileName, fileName, searchContent, replaceContent, "", "", { context: 3 }) + + // Convert to unified diff format + if (patch.hunks.length > 0) { + for (const hunk of patch.hunks) { + combinedDiff += `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@\n` + combinedDiff += hunk.lines.join("\n") + "\n" + } + } + } + + return hasBlocks ? combinedDiff : content +} + +function computeDiffStats(diff?: string): { added: number; removed: number } | null { + if (!diff) return null + + let added = 0 + let removed = 0 + let sawPlusMinus = false + + for (const line of diff.split("\n")) { + if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue + if (line.startsWith("+")) { + added++ + sawPlusMinus = true + } else if (line.startsWith("-")) { + removed++ + sawPlusMinus = true + } + } + + if (sawPlusMinus && (added > 0 || removed > 0)) { + return { added, removed } + } + + return null +} + export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProps) => { const [expandedFiles, setExpandedFiles] = useState>({}) @@ -36,16 +94,28 @@ 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 + const rawCombinedDiff = file.diffs?.map((diff) => diff.content).join("\n\n") || file.content + + // Remove CDATA markers + const withoutCData = rawCombinedDiff.replace(//g, "") + + // Convert SEARCH/REPLACE to unified diff if needed + const cleanDiff = /<<<<<< handleToggleExpand(file.path)} + diffStats={stats ?? undefined} />
) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 4299240a549..c26c9613ec6 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -3,6 +3,7 @@ import { useSize } from "react-use" import { useTranslation, Trans } from "react-i18next" import deepEqual from "fast-deep-equal" import { VSCodeBadge } from "@vscode/webview-ui-toolkit/react" +import { structuredPatch } from "diff" import type { ClineMessage, FollowUpData, SuggestionItem } from "@roo-code/types" import { Mode } from "@roo/modes" @@ -15,7 +16,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" @@ -117,6 +117,116 @@ const ChatRow = memo( export default ChatRow +function computeDiffStats(diff?: string): { added: number; removed: number } | null { + if (!diff) return null + + // Strategy 1: Unified diff (+/- lines) + let added = 0 + let removed = 0 + let sawPlusMinus = false + for (const line of diff.split("\n")) { + if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue + if (line.startsWith("+")) { + added++ + sawPlusMinus = true + } else if (line.startsWith("-")) { + removed++ + sawPlusMinus = true + } + } + if (sawPlusMinus) { + if (added === 0 && removed === 0) return null + return { added, removed } + } + + // Strategy 2: Roo multi-search-replace blocks + // Count lines in SEARCH vs REPLACE sections across all blocks + // Matches optional metadata lines and optional '-------' line + const blockRegex = + /<<<<<>>>>>> REPLACE)/gim + + let hasBlocks = false + added = 0 + removed = 0 + + const asLines = (s: string) => { + // Normalize Windows newlines and trim trailing newline so counts reflect real lines + const norm = s.replace(/\r\n/g, "\n") + if (norm === "") return 0 + // Split, drop potential trailing empty caused by final newline + const parts = norm.split("\n") + return parts[parts.length - 1] === "" ? parts.length - 1 : parts.length + } + + let match: RegExpExecArray | null + while ((match = blockRegex.exec(diff)) !== null) { + hasBlocks = true + const searchContent = match[1] ?? "" + const replaceContent = match[2] ?? "" + const searchCount = asLines(searchContent) + const replaceCount = asLines(replaceContent) + if (replaceCount > searchCount) added += replaceCount - searchCount + else if (searchCount > replaceCount) removed += searchCount - replaceCount + } + + if (hasBlocks) { + if (added === 0 && removed === 0) return null + return { added, removed } + } + + return null +} + +/** + * Converts new file content to unified diff format (all lines as additions) + */ +function convertNewFileToUnifiedDiff(content: string, filePath?: string): string { + const fileName = filePath || "file" + const lines = content.split("\n") + + let diff = `--- /dev/null\n` + diff += `+++ ${fileName}\n` + diff += `@@ -0,0 +1,${lines.length} @@\n` + + for (const line of lines) { + diff += `+${line}\n` + } + + return diff +} + +/** + * Converts Roo's SEARCH/REPLACE format to unified diff format for better readability + */ +function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { + const blockRegex = + /<<<<<>>>>>> REPLACE)/gim + + let hasBlocks = false + let combinedDiff = "" + const fileName = filePath || "file" + + let match: RegExpExecArray | null + while ((match = blockRegex.exec(content)) !== null) { + hasBlocks = true + const searchContent = (match[1] ?? "").replace(/\n$/, "") // Remove trailing newline + const replaceContent = (match[2] ?? "").replace(/\n$/, "") + + // Use the diff library to create a proper unified diff + const patch = structuredPatch(fileName, fileName, searchContent, replaceContent, "", "", { context: 3 }) + + // Convert to unified diff format + if (patch.hunks.length > 0) { + for (const hunk of patch.hunks) { + combinedDiff += `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@\n` + combinedDiff += hunk.lines.join("\n") + "\n" + } + } + } + + return hasBlocks ? combinedDiff : content +} + export const ChatRowContent = ({ message, lastModifiedMessage, @@ -336,6 +446,59 @@ export const ChatRowContent = ({ [message.ask, message.text], ) + // Inline diff stats for edit/apply_diff/insert/search-replace/newFile asks + const diffTextForStats = useMemo(() => { + if (!tool) return "" + let content = "" + switch (tool.tool) { + case "editedExistingFile": + case "appliedDiff": + content = (tool.content ?? tool.diff) || "" + break + case "insertContent": + case "searchAndReplace": + content = tool.diff || "" + break + case "newFileCreated": + // For new files, convert to unified diff format + const newFileContent = tool.content || "" + content = convertNewFileToUnifiedDiff(newFileContent, tool.path) + break + default: + return "" + } + // Strip CDATA markers for proper parsing + return content.replace(//g, "") + }, [tool]) + + const diffStatsForInline = useMemo(() => { + if (tool?.tool === "newFileCreated") { + // For new files, count all lines as additions + const content = diffTextForStats + if (!content) return null + const lines = content.split("\n").length + return { added: lines, removed: 0 } + } + return computeDiffStats(diffTextForStats) + }, [diffTextForStats, tool]) + + // Clean diff content for display (remove CDATA markers and convert to unified diff) + const cleanDiffContent = useMemo(() => { + if (!tool) return undefined + const raw = (tool as any).content ?? (tool as any).diff + if (!raw) return undefined + + // Remove CDATA markers + const withoutCData = raw.replace(//g, "") + + // Check if it's SEARCH/REPLACE format and convert to unified diff + if (/<<<<<< { if (message.type === "ask" && message.ask === "followup" && !message.partial) { return safeJsonParse(message.text) @@ -391,12 +554,13 @@ export const ChatRowContent = ({
@@ -428,12 +592,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")} + +
+
+
@@ -496,12 +695,13 @@ export const ChatRowContent = ({
vscode.postMessage({ type: "openFile", text: "./" + tool.path })} + diffStats={diffStatsForInline ?? undefined} />
diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 929fa9427aa..b9e2323cbb3 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 00000000000..9445c83f21c --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx @@ -0,0 +1,93 @@ +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, + }), + } + + 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, + }), + } + + renderChatRow(message) + + expect(screen.getByText("+1")).toBeInTheDocument() + expect(screen.getByText("-2")).toBeInTheDocument() + }) +}) diff --git a/webview-ui/src/components/common/CodeAccordian.tsx b/webview-ui/src/components/common/CodeAccordian.tsx index a86f9c3221b..7ce4214d097 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,63 @@ interface CodeAccordianProps { onToggleExpand: () => void header?: string onJumpToFile?: () => void + // New props for diff stats + diffStats?: { added: number; removed: number } +} + +// Fallback computation of + / - counts from code (supports both unified diff and Roo's multi-search-replace blocks) +function computeDiffStatsFromCode(diff?: string): { added: number; removed: number } | null { + if (!diff) return null + + // Strategy 1: unified diff markers + let added = 0 + let removed = 0 + let sawPlusMinus = false + for (const line of diff.split("\n")) { + if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue + if (line.startsWith("+")) { + added++ + sawPlusMinus = true + } else if (line.startsWith("-")) { + removed++ + sawPlusMinus = true + } + } + if (sawPlusMinus) { + if (added === 0 && removed === 0) return null + return { added, removed } + } + + // Strategy 2: Roo multi-search-replace blocks + const blockRegex = + /<<<<<>>>>>> REPLACE)/gim + + const asLines = (s: string) => { + const norm = (s || "").replace(/\r\n/g, "\n") + if (!norm) return 0 + const parts = norm.split("\n") + return parts[parts.length - 1] === "" ? parts.length - 1 : parts.length + } + + let hasBlocks = false + added = 0 + removed = 0 + + let match: RegExpExecArray | null + while ((match = blockRegex.exec(diff)) !== null) { + hasBlocks = true + const searchCount = asLines(match[1] ?? "") + const replaceCount = asLines(match[2] ?? "") + if (replaceCount > searchCount) added += replaceCount - searchCount + else if (searchCount > replaceCount) removed += searchCount - replaceCount + } + + if (hasBlocks) { + if (added === 0 && removed === 0) return null + return { added, removed } + } + + return null } const CodeAccordian = ({ @@ -32,11 +90,23 @@ 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) + // Derive diff stats from code when not provided + const derivedStats = useMemo(() => { + if (diffStats && (diffStats.added > 0 || diffStats.removed > 0)) return diffStats + if ((language || inferredLanguage) && (language || inferredLanguage) === "diff") { + return computeDiffStatsFromCode(source || code || "") + } + return null + }, [diffStats, language, inferredLanguage, source, code]) + + const hasValidStats = Boolean(derivedStats && (derivedStats.added > 0 || derivedStats.removed > 0)) + return ( {hasHeader && ( @@ -67,13 +137,28 @@ 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 && ( - + {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 00000000000..e9faa5b4167 --- /dev/null +++ b/webview-ui/src/components/common/DiffView.tsx @@ -0,0 +1,181 @@ +import { memo, useMemo } from "react" +import { parsePatch } from "diff" + +interface DiffViewProps { + source: string + filePath?: string +} + +interface DiffLine { + oldLineNum: number | null + newLineNum: number | null + type: "context" | "addition" | "deletion" + content: string +} + +/** + * DiffView component renders unified diffs with side-by-side line numbers + * matching VSCode's diff editor style + */ +const DiffView = memo(({ source }: DiffViewProps) => { + // Parse diff and extract line information + const diffLines = useMemo(() => { + if (!source) return [] + + try { + const patches = parsePatch(source) + if (!patches || patches.length === 0) return [] + + const lines: DiffLine[] = [] + const patch = patches[0] + + for (const hunk of patch.hunks) { + let oldLine = hunk.oldStart + let newLine = hunk.newStart + + for (const line of hunk.lines) { + const firstChar = line[0] + const content = line.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++ + } + } + } + + return lines + } catch (error) { + console.error("[DiffView] Failed to parse diff:", error) + return [] + } + }, [source]) + + return ( +
+
+ + + {diffLines.map((line, idx) => { + const bgColor = + line.type === "addition" + ? "var(--vscode-diffEditor-insertedTextBackground)" + : line.type === "deletion" + ? "var(--vscode-diffEditor-removedTextBackground)" + : "transparent" + + const lineColor = + line.type === "addition" + ? "var(--vscode-gitDecoration-addedResourceForeground)" + : line.type === "deletion" + ? "var(--vscode-gitDecoration-deletedResourceForeground)" + : "var(--vscode-editorLineNumber-foreground)" + + return ( + + {/* Old line number */} + + {/* New line number */} + + {/* +/- indicator */} + + {/* Code content */} + + + ) + })} + +
+ {line.oldLineNum || ""} + + {line.newLineNum || ""} + + {line.type === "addition" ? "+" : line.type === "deletion" ? "−" : ""} + + {line.content} +
+
+
+ ) +}) + +export default DiffView From b09b3ffee811ceb1e36e2e3b5f85f962a89c39cc Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 10:10:23 -0600 Subject: [PATCH 02/22] ui(chat): constrain expanded diff height with scroll (max-h 300px) to match existing patterns --- webview-ui/src/components/common/CodeAccordian.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/common/CodeAccordian.tsx b/webview-ui/src/components/common/CodeAccordian.tsx index 7ce4214d097..fa5d2cd53ef 100644 --- a/webview-ui/src/components/common/CodeAccordian.tsx +++ b/webview-ui/src/components/common/CodeAccordian.tsx @@ -178,7 +178,7 @@ const CodeAccordian = ({ )} {(!hasHeader || isExpanded) && ( -
+
{inferredLanguage === "diff" ? ( ) : ( From d689b6e8aec6a0861f8716e52b7e93452d9c0372 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 10:28:42 -0600 Subject: [PATCH 03/22] fix(chat): rely on computeDiffStats for new file unified diffs; remove special-case counting and add test (addresses roomote review)\n\n- Remove special-case line counting for newFileCreated in [diffStatsForInline](webview-ui/src/components/chat/ChatRow.tsx:472)\n- Always use [computeDiffStats()](webview-ui/src/components/chat/ChatRow.tsx:118)\n- Add test to assert +N/-0 for new files in [ChatRow.diff-actions.spec.tsx](webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx) --- webview-ui/src/components/chat/ChatRow.tsx | 9 +------- .../__tests__/ChatRow.diff-actions.spec.tsx | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index c26c9613ec6..58641e48061 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -472,15 +472,8 @@ export const ChatRowContent = ({ }, [tool]) const diffStatsForInline = useMemo(() => { - if (tool?.tool === "newFileCreated") { - // For new files, count all lines as additions - const content = diffTextForStats - if (!content) return null - const lines = content.split("\n").length - return { added: lines, removed: 0 } - } return computeDiffStats(diffTextForStats) - }, [diffTextForStats, tool]) + }, [diffTextForStats]) // Clean diff content for display (remove CDATA markers and convert to unified diff) const cleanDiffContent = useMemo(() => { 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 index 9445c83f21c..36a364c2ddc 100644 --- a/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx @@ -90,4 +90,25 @@ describe("ChatRow - inline diff stats and actions", () => { 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, + }), + } + + renderChatRow(message) + + // Should only count the three content lines as additions + expect(screen.getByText("+3")).toBeInTheDocument() + expect(screen.getByText("-0")).toBeInTheDocument() + }) }) From 0118ead54b47198851bfc0b04a8e24d23627f0c0 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 10:32:59 -0600 Subject: [PATCH 04/22] ui(diff): VSCode-style highlighting in [DiffView](webview-ui/src/components/common/DiffView.tsx)\n\n- Tint only content and +/- gutter; keep line-number columns neutral\n- Match look of VSCode unified diff blocks (clear red/green bands)\n --- webview-ui/src/components/common/DiffView.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index e9faa5b4167..6a37d467e4d 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -92,12 +92,15 @@ const DiffView = memo(({ source }: DiffViewProps) => { }}> {diffLines.map((line, idx) => { - const bgColor = + // Backgrounds: tint only the content and +/- gutter, not the line-number columns + const contentBg = line.type === "addition" ? "var(--vscode-diffEditor-insertedTextBackground)" : line.type === "deletion" ? "var(--vscode-diffEditor-removedTextBackground)" : "transparent" + // Use same tint for the +/- gutter for a cohesive band effect + const gutterBg = contentBg const lineColor = line.type === "addition" @@ -107,7 +110,7 @@ const DiffView = memo(({ source }: DiffViewProps) => { : "var(--vscode-editorLineNumber-foreground)" return ( - + {/* Old line number */} { style={{ width: "20px", textAlign: "center", + backgroundColor: gutterBg, color: line.type === "addition" ? "var(--vscode-gitDecoration-addedResourceForeground)" @@ -165,6 +169,7 @@ const DiffView = memo(({ source }: DiffViewProps) => { fontFamily: "var(--vscode-editor-font-family)", color: "var(--vscode-editor-foreground)", width: "100%", + backgroundColor: contentBg, }}> {line.content} From 3ea5a3445f7dffb7c2fe7fa8856145fdd7335744 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 11:50:03 -0600 Subject: [PATCH 05/22] feat(diff): enhance DiffView with syntax highlighting and improved styling - Integrate highlighter for code syntax based on file language - Adjust background colors for additions and deletions to match VSCode styles - Refactor rendering logic to include inline highlighting and gutter styles --- webview-ui/src/components/common/DiffView.tsx | 125 +++++++++++++----- 1 file changed, 93 insertions(+), 32 deletions(-) diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index 6a37d467e4d..6b775177564 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -1,5 +1,9 @@ -import { memo, useMemo } from "react" +import { memo, useMemo, useEffect, useState } from "react" import { parsePatch } from "diff" +import { toJsxRuntime } from "hast-util-to-jsx-runtime" +import { Fragment, jsx, jsxs } from "react/jsx-runtime" +import { getHighlighter, normalizeLanguage } from "@src/utils/highlighter" +import { getLanguageFromPath } from "@src/utils/getLanguageFromPath" interface DiffViewProps { source: string @@ -17,7 +21,62 @@ interface DiffLine { * DiffView component renders unified diffs with side-by-side line numbers * matching VSCode's diff editor style */ -const DiffView = memo(({ source }: DiffViewProps) => { +const DiffView = memo(({ source, filePath }: DiffViewProps) => { + // Determine language from file path and prepare highlighter + const normalizedLang = useMemo(() => normalizeLanguage(getLanguageFromPath(filePath || "") || "txt"), [filePath]) + const [highlighter, setHighlighter] = useState(null) + const isLightTheme = useMemo( + () => typeof document !== "undefined" && document.body.className.toLowerCase().includes("light"), + [], + ) + + useEffect(() => { + let mounted = true + getHighlighter(normalizedLang) + .then((h) => { + if (mounted) setHighlighter(h) + }) + .catch(() => { + // fall back to plain text if highlighting fails + }) + return () => { + mounted = false + } + }, [normalizedLang]) + + const renderHighlighted = (code: string): React.ReactNode => { + if (!highlighter) return code + try { + const hast: any = highlighter.codeToHast(code, { + lang: normalizedLang, + theme: isLightTheme ? "github-light" : "github-dark", + transformers: [ + { + pre(node: any) { + node.properties.style = "padding:0;margin:0;background:none;" + return node + }, + code(node: any) { + node.properties.class = `hljs language-${normalizedLang}` + return node + }, + }, + ], + }) + + // Extract just the children to render inline inside our table cell + const codeEl = hast?.children?.[0]?.children?.[0] + const inlineRoot = + codeEl && codeEl.children + ? { type: "element", tagName: "span", properties: {}, children: codeEl.children } + : { type: "element", tagName: "span", properties: {}, children: hast.children || [] } + + return toJsxRuntime(inlineRoot as any, { Fragment, jsx, jsxs }) + } catch { + return code + } + } + // Parse diff and extract line information const diffLines = useMemo(() => { if (!source) return [] @@ -92,22 +151,29 @@ const DiffView = memo(({ source }: DiffViewProps) => { }}> {diffLines.map((line, idx) => { - // Backgrounds: tint only the content and +/- gutter, not the line-number columns - const contentBg = + // Use VSCode's built-in diff editor color variables with 50% opacity + const gutterBg = line.type === "addition" ? "var(--vscode-diffEditor-insertedTextBackground)" : line.type === "deletion" ? "var(--vscode-diffEditor-removedTextBackground)" - : "transparent" - // Use same tint for the +/- gutter for a cohesive band effect - const gutterBg = contentBg + : "var(--vscode-editorGroup-border)" - const lineColor = + const contentBgStyles = line.type === "addition" - ? "var(--vscode-gitDecoration-addedResourceForeground)" + ? { + backgroundColor: + "color-mix(in srgb, var(--vscode-diffEditor-insertedTextBackground) 70%, transparent)", + } : line.type === "deletion" - ? "var(--vscode-gitDecoration-deletedResourceForeground)" - : "var(--vscode-editorLineNumber-foreground)" + ? { + backgroundColor: + "color-mix(in srgb, var(--vscode-diffEditor-removedTextBackground) 70%, transparent)", + } + : { + backgroundColor: + "color-mix(in srgb, var(--vscode-editorGroup-border) 100%, transparent)", + } return ( @@ -118,11 +184,10 @@ const DiffView = memo(({ source }: DiffViewProps) => { textAlign: "right", paddingRight: "12px", paddingLeft: "8px", - color: lineColor, - opacity: 0.5, userSelect: "none", verticalAlign: "top", whiteSpace: "nowrap", + backgroundColor: gutterBg, }}> {line.oldLineNum || ""} @@ -132,33 +197,22 @@ const DiffView = memo(({ source }: DiffViewProps) => { width: "45px", textAlign: "right", paddingRight: "12px", - color: lineColor, - opacity: 0.5, userSelect: "none", verticalAlign: "top", whiteSpace: "nowrap", + backgroundColor: gutterBg, }}> {line.newLineNum || ""} - {/* +/- indicator */} + {/* Narrow colored gutter (no +/- glyph) */} - {line.type === "addition" ? "+" : line.type === "deletion" ? "−" : ""} - - {/* Code content */} + }} + /> + {/* Code content (includes +/- prefix inside the code cell) */} { fontFamily: "var(--vscode-editor-font-family)", color: "var(--vscode-editor-foreground)", width: "100%", - backgroundColor: contentBg, + ...contentBgStyles, }}> - {line.content} + + {line.type === "addition" ? "+ " : line.type === "deletion" ? "- " : ""} + + {renderHighlighted(line.content)} ) From 1a75e1f4dc9fda94c53e91ace3c9bb4c57e95879 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 14:10:00 -0600 Subject: [PATCH 06/22] feat(webview-ui): normalize diffs to unified format and enhance DiffView rendering Add diffUtils with unified diff normalization: - extractUnifiedDiff, convertSearchReplaceToUnifiedDiff, convertNewFileToUnifiedDiff BatchDiffApproval: - normalize tool output to unified diff via extractUnifiedDiff - compute unified diff stats - remove CDATA handling and legacy conversion paths DiffView: - add compact gap rows between hunks - dedicated +/- column - improved gutter/background and layout tweaks ChatRow: - integrate unified diff normalization pipeline --- .../src/components/chat/BatchDiffApproval.tsx | 78 +++------- webview-ui/src/components/chat/ChatRow.tsx | 84 +++------- webview-ui/src/components/common/DiffView.tsx | 120 ++++++++++++-- webview-ui/src/utils/diffUtils.ts | 146 ++++++++++++++++++ 4 files changed, 290 insertions(+), 138 deletions(-) create mode 100644 webview-ui/src/utils/diffUtils.ts diff --git a/webview-ui/src/components/chat/BatchDiffApproval.tsx b/webview-ui/src/components/chat/BatchDiffApproval.tsx index 5338017af39..815b648dffa 100644 --- a/webview-ui/src/components/chat/BatchDiffApproval.tsx +++ b/webview-ui/src/components/chat/BatchDiffApproval.tsx @@ -1,6 +1,6 @@ import React, { memo, useState } from "react" -import { structuredPatch } from "diff" import CodeAccordian from "../common/CodeAccordian" +import { extractUnifiedDiff } from "../../utils/diffUtils" interface FileDiff { path: string @@ -18,63 +18,27 @@ interface BatchDiffApprovalProps { ts: number } -/** - * Converts Roo's SEARCH/REPLACE format to unified diff format for better readability - */ -function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { - const blockRegex = - /<<<<<>>>>>> REPLACE)/gim - - let hasBlocks = false - let combinedDiff = "" - const fileName = filePath || "file" - - let match: RegExpExecArray | null - while ((match = blockRegex.exec(content)) !== null) { - hasBlocks = true - const searchContent = (match[1] ?? "").replace(/\n$/, "") // Remove trailing newline - const replaceContent = (match[2] ?? "").replace(/\n$/, "") - - // Use the diff library to create a proper unified diff - const patch = structuredPatch(fileName, fileName, searchContent, replaceContent, "", "", { context: 3 }) - - // Convert to unified diff format - if (patch.hunks.length > 0) { - for (const hunk of patch.hunks) { - combinedDiff += `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@\n` - combinedDiff += hunk.lines.join("\n") + "\n" - } - } - } - - return hasBlocks ? combinedDiff : content -} - -function computeDiffStats(diff?: string): { added: number; removed: number } | null { +/** Compute +/− from a unified diff (ignores headers/hunk lines) */ +function computeUnifiedStats(diff?: string): { added: number; removed: number } | null { if (!diff) return null - let added = 0 let removed = 0 - let sawPlusMinus = false - + let saw = false for (const line of diff.split("\n")) { if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue if (line.startsWith("+")) { added++ - sawPlusMinus = true + saw = true } else if (line.startsWith("-")) { removed++ - sawPlusMinus = true + saw = true } } - - if (sawPlusMinus && (added > 0 || removed > 0)) { - return { added, removed } - } - - return null + return saw && (added > 0 || removed > 0) ? { added, removed } : null } +/* keep placeholder (legacy) – replaced by computeUnifiedStats after normalization */ + export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProps) => { const [expandedFiles, setExpandedFiles] = useState>({}) @@ -93,25 +57,21 @@ export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProp
{files.map((file) => { - // Combine all diffs into a single diff string for this file - const rawCombinedDiff = file.diffs?.map((diff) => diff.content).join("\n\n") || file.content - - // Remove CDATA markers - const withoutCData = rawCombinedDiff.replace(//g, "") - - // Convert SEARCH/REPLACE to unified diff if needed - const cleanDiff = /<<<<<< d.content).join("\n\n") || file.content + const unified = extractUnifiedDiff({ + toolName: "appliedDiff", + path: file.path, + diff: rawCombined, + content: undefined, + }) + const stats = computeUnifiedStats(unified) return (
handleToggleExpand(file.path)} diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 58641e48061..184225072b8 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -3,7 +3,6 @@ import { useSize } from "react-use" import { useTranslation, Trans } from "react-i18next" import deepEqual from "fast-deep-equal" import { VSCodeBadge } from "@vscode/webview-ui-toolkit/react" -import { structuredPatch } from "diff" import type { ClineMessage, FollowUpData, SuggestionItem } from "@roo-code/types" import { Mode } from "@roo/modes" @@ -25,6 +24,7 @@ import { ReasoningBlock } from "./ReasoningBlock" import Thumbnails from "../common/Thumbnails" import ImageBlock from "../common/ImageBlock" import ErrorRow from "./ErrorRow" +import { extractUnifiedDiff } from "../../utils/diffUtils" import McpResourceRow from "../mcp/McpResourceRow" @@ -195,38 +195,6 @@ function convertNewFileToUnifiedDiff(content: string, filePath?: string): string return diff } -/** - * Converts Roo's SEARCH/REPLACE format to unified diff format for better readability - */ -function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { - const blockRegex = - /<<<<<>>>>>> REPLACE)/gim - - let hasBlocks = false - let combinedDiff = "" - const fileName = filePath || "file" - - let match: RegExpExecArray | null - while ((match = blockRegex.exec(content)) !== null) { - hasBlocks = true - const searchContent = (match[1] ?? "").replace(/\n$/, "") // Remove trailing newline - const replaceContent = (match[2] ?? "").replace(/\n$/, "") - - // Use the diff library to create a proper unified diff - const patch = structuredPatch(fileName, fileName, searchContent, replaceContent, "", "", { context: 3 }) - - // Convert to unified diff format - if (patch.hunks.length > 0) { - for (const hunk of patch.hunks) { - combinedDiff += `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@\n` - combinedDiff += hunk.lines.join("\n") + "\n" - } - } - } - - return hasBlocks ? combinedDiff : content -} - export const ChatRowContent = ({ message, lastModifiedMessage, @@ -449,47 +417,31 @@ export const ChatRowContent = ({ // Inline diff stats for edit/apply_diff/insert/search-replace/newFile asks const diffTextForStats = useMemo(() => { if (!tool) return "" - let content = "" - switch (tool.tool) { - case "editedExistingFile": - case "appliedDiff": - content = (tool.content ?? tool.diff) || "" - break - case "insertContent": - case "searchAndReplace": - content = tool.diff || "" - break - case "newFileCreated": - // For new files, convert to unified diff format - const newFileContent = tool.content || "" - content = convertNewFileToUnifiedDiff(newFileContent, tool.path) - break - default: - return "" - } - // Strip CDATA markers for proper parsing - return content.replace(//g, "") + // Normalize to unified diff using frontend-only capture/surmise helper + return ( + extractUnifiedDiff({ + toolName: tool.tool as string, + path: tool.path, + diff: (tool as any).diff, + content: (tool as any).content, + }) || "" + ) }, [tool]) const diffStatsForInline = useMemo(() => { return computeDiffStats(diffTextForStats) }, [diffTextForStats]) - // Clean diff content for display (remove CDATA markers and convert to unified diff) + // Clean diff content for display (normalize to unified diff) const cleanDiffContent = useMemo(() => { if (!tool) return undefined - const raw = (tool as any).content ?? (tool as any).diff - if (!raw) return undefined - - // Remove CDATA markers - const withoutCData = raw.replace(//g, "") - - // Check if it's SEARCH/REPLACE format and convert to unified diff - if (/<<<<<< { diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index 6b775177564..e461efb4206 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -13,8 +13,9 @@ interface DiffViewProps { interface DiffLine { oldLineNum: number | null newLineNum: number | null - type: "context" | "addition" | "deletion" + type: "context" | "addition" | "deletion" | "gap" content: string + hiddenCount?: number } /** @@ -88,7 +89,24 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { const lines: DiffLine[] = [] const patch = patches[0] + let prevHunk: any = null for (const hunk of patch.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 @@ -124,6 +142,8 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { newLine++ } } + + prevHunk = hunk } return lines @@ -151,6 +171,70 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { }}> {diffLines.map((line, idx) => { + // Render compact separator between hunks + if (line.type === "gap") { + const gapBg = "color-mix(in srgb, var(--vscode-editorGroup-border) 100%, transparent)" + return ( + + + + + {/* +/- column (empty for gap) */} + + + {`${line.hiddenCount ?? 0} hidden lines`} + + + ) + } + // Use VSCode's built-in diff editor color variables with 50% opacity const gutterBg = line.type === "addition" @@ -175,6 +259,8 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { "color-mix(in srgb, var(--vscode-editorGroup-border) 100%, transparent)", } + const sign = line.type === "addition" ? "+" : line.type === "deletion" ? "-" : "" + return ( {/* Old line number */} @@ -182,8 +268,8 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { style={{ width: "45px", textAlign: "right", - paddingRight: "12px", - paddingLeft: "8px", + paddingRight: "4px", + paddingLeft: "4px", userSelect: "none", verticalAlign: "top", whiteSpace: "nowrap", @@ -196,7 +282,7 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { style={{ width: "45px", textAlign: "right", - paddingRight: "12px", + paddingRight: "4px", userSelect: "none", verticalAlign: "top", whiteSpace: "nowrap", @@ -204,7 +290,7 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { }}> {line.newLineNum || ""} - {/* Narrow colored gutter (no +/- glyph) */} + {/* Narrow colored gutter */} { verticalAlign: "top", }} /> - {/* Code content (includes +/- prefix inside the code cell) */} + {/* +/- fixed column to prevent wrapping into it */} + + {sign} + + {/* Code content (no +/- prefix here) */} { width: "100%", ...contentBgStyles, }}> - - {line.type === "addition" ? "+ " : line.type === "deletion" ? "- " : ""} - {renderHighlighted(line.content)} diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts new file mode 100644 index 00000000000..3db32775aa2 --- /dev/null +++ b/webview-ui/src/utils/diffUtils.ts @@ -0,0 +1,146 @@ +/** + * Frontend-only normalization helper. + * - If a unified diff already exists, return it. + * - If a Roo SEARCH/REPLACE block is provided, convert to unified diff. + * - If it's a new file with raw content, synthesize a unified diff with all lines as additions. + * - Otherwise, pass through raw content (DiffView will no-op if not unified). + */ +export function extractUnifiedDiff(params: { + toolName?: string + path?: string + diff?: string + content?: string +}): string { + const filePath = params.path || "file" + let raw = (params.diff ?? params.content ?? "") || "" + + if (!raw) return "" + + raw = stripCData(raw) + + // Explicit new file: build a unified diff from raw content + if ((params.toolName || "").toLowerCase() === "newfilecreated") { + return convertNewFileToUnifiedDiff(raw, filePath) + } + + // SEARCH/REPLACE blocks → unified + if (isSearchReplace(raw)) { + return convertSearchReplaceToUnifiedDiff(raw, filePath) + } + + // Already unified? + if (isUnifiedDiff(raw)) { + return raw + } + + // Fallback: return as-is (non-unified content) + return raw +} + +/** Detects unified diff by presence of headers/hunks */ +function isUnifiedDiff(s: string): boolean { + const hasHunk = /(^|\n)@@\s+-[0-9,]+\s+\+[0-9,]+\s+@@/.test(s) + const hasHeaders = /(^|\n)---\s|\n\+\+\+\s/.test(s) + return hasHunk || hasHeaders +} + +/** Detects Roo SEARCH/REPLACE multi-block format */ +function isSearchReplace(s: string): boolean { + return ( + /(^|\n)<<<<<<< ?SEARCH|(^|\n)<<<<<</.test(s) || /(^|\n)<<<<<<< ?SEARCH/.test(s) + ) +} + +/** Remove CDATA markers and any HTML-encoded variants */ +function stripCData(s: string): string { + return ( + s + // HTML-encoded CDATA open + .replace(//g, "") + ) +} + +/** + * Convert Roo SEARCH/REPLACE blocks into unified diff using the diff library. + * Matches optional metadata lines and optional '-------' separator. + */ +export function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { + // Backend-compatible regex: captures :start_line: and :end_line:, optional '-------', and SEARCH/REPLACE bodies + const blockRegex = + /(?:^|\n)(??\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?>>>>>> REPLACE)(?=\n|$)/g + + const fileName = filePath || "file" + let hasBlocks = false + let headerEmitted = false + let unified = "" + + // Helper to normalize EOLs and get stable line arrays without trailing empty caused by final newline + const toStableLines = (s: string): string[] => { + const norm = s.replace(/\r\n/g, "\n") + if (norm === "") return [] + const parts = norm.split("\n") + return parts[parts.length - 1] === "" ? parts.slice(0, -1) : parts + } + + let match: RegExpExecArray | null + while ((match = blockRegex.exec(content)) !== null) { + hasBlocks = true + + // 1: full start_line line, 2: start_line number, 3: full end_line line, 4: end_line number + const startLine = match[2] ? parseInt(match[2], 10) : 1 + const endLine = match[4] ? parseInt(match[4], 10) : undefined + + // 6: SEARCH body, 7: REPLACE body + const searchBody = match[6] ?? "" + const replaceBody = match[7] ?? "" + + const searchLines = toStableLines(searchBody) + const replaceLines = toStableLines(replaceBody) + + // Old/new hunk metadata. If end_line is present, prefer it for oldLines; otherwise count SEARCH lines. + const oldStart = startLine + const oldLines = endLine !== undefined ? Math.max(0, endLine - startLine + 1) : searchLines.length + const newStart = startLine + const newLines = replaceLines.length + + // Emit file headers once so parsePatch can recognize a complete unified diff + if (!headerEmitted) { + unified += `--- a/${fileName}\n` + unified += `+++ b/${fileName}\n` + headerEmitted = true + } + + // Hunk header + unified += `@@ -${oldStart},${oldLines} +${newStart},${newLines} @@\n` + + // We don't have surrounding context here; emit deletions then additions to visualize the change + for (const line of searchLines) unified += `-${line}\n` + for (const line of replaceLines) unified += `+${line}\n` + } + + return hasBlocks ? unified : content +} + +/** Build a unified diff for a brand new file (all content lines are additions) */ +export function convertNewFileToUnifiedDiff(content: string, filePath?: string): string { + const fileName = filePath || "file" + // Normalize EOLs to keep counts consistent + const normalized = content.replace(/\r\n/g, "\n") + const lines = normalized.split("\n") + + let diff = `--- /dev/null\n` + diff += `+++ ${fileName}\n` + diff += `@@ -0,0 +1,${normalized === "" ? 0 : lines.length} @@\n` + + for (const line of lines) { + // Preserve final newline behavior: if content ended with newline, split will produce trailing "" + // which is still okay to emit as "+", it represents a blank line. + diff += `+${line}\n` + } + + return diff +} From 2e666b32adf80f2b3eca3ad07da7f8ab2c479391 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 14:24:24 -0600 Subject: [PATCH 07/22] fix(diff): update regex in convertSearchReplaceToUnifiedDiff for improved compatibility --- webview-ui/src/utils/diffUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts index 3db32775aa2..307a1eba056 100644 --- a/webview-ui/src/utils/diffUtils.ts +++ b/webview-ui/src/utils/diffUtils.ts @@ -71,7 +71,7 @@ function stripCData(s: string): string { export function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { // Backend-compatible regex: captures :start_line: and :end_line:, optional '-------', and SEARCH/REPLACE bodies const blockRegex = - /(?:^|\n)(??\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?>>>>>> REPLACE)(?=\n|$)/g + /(?:^|\n)(??\s*\n((?::start_line:\s*(\d+)\s*\n))?((?::end_line:\s*(\d+)\s*\n))?((?>>>>>> REPLACE)(?=\n|$)/g const fileName = filePath || "file" let hasBlocks = false From 3a69facd0757f3148bea2799964cce2f054d045a Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 14:33:51 -0600 Subject: [PATCH 08/22] fix(webview-ui): use theme foreground for +/- indicators in DiffView to support light/dark themes --- webview-ui/src/components/common/DiffView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index e461efb4206..f69b35264e4 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -308,7 +308,7 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { paddingLeft: "4px", paddingRight: "4px", backgroundColor: gutterBg, - color: "#ffffff", + color: "var(--vscode-editor-foreground)", fontFamily: "var(--vscode-editor-font-family)", }}> {sign} From 40f94350dc501d5c0a3e155c6aa02056119f58bf Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 14:39:21 -0600 Subject: [PATCH 09/22] fix(webview-ui): resolve CodeQL warning in stripCData by handling HTML-encoded CDATA markers and removing no-op replacement --- webview-ui/src/utils/diffUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts index 307a1eba056..8a128a13ba8 100644 --- a/webview-ui/src/utils/diffUtils.ts +++ b/webview-ui/src/utils/diffUtils.ts @@ -55,11 +55,11 @@ function isSearchReplace(s: string): boolean { function stripCData(s: string): string { return ( s - // HTML-encoded CDATA open + // HTML-encoded CDATA open -> raw, then strip raw .replace(/ raw, then strip raw + .replace(/\]\]>/g, "]]>") .replace(/\]\]>/g, "") ) } From 80dd8fc69634f6ab61a30f2ca923636b0c7e64db Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 14:53:09 -0600 Subject: [PATCH 10/22] fix(webview-ui): stripCData removes raw and HTML-encoded CDATA markers without no-op replacements (resolves CodeQL warning) --- webview-ui/src/utils/diffUtils.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts index 8a128a13ba8..203fa974053 100644 --- a/webview-ui/src/utils/diffUtils.ts +++ b/webview-ui/src/utils/diffUtils.ts @@ -55,11 +55,11 @@ function isSearchReplace(s: string): boolean { function stripCData(s: string): string { return ( s - // HTML-encoded CDATA open -> raw, then strip raw - .replace(/ raw, then strip raw - .replace(/\]\]>/g, "]]>") + // Remove HTML-encoded and raw CDATA close + .replace(/\]\]>/gi, "") .replace(/\]\]>/g, "") ) } From 43b76c928fb7ebb72c84502af005c1e37eb40e1a Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 15:02:22 -0600 Subject: [PATCH 11/22] fix(webview-ui): correct new-file diff line counting by ignoring trailing newline in unified diff; add test for ChatRow --- .../__tests__/ChatRow.diff-actions.spec.tsx | 21 +++++++++++++++++++ webview-ui/src/utils/diffUtils.ts | 14 +++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) 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 index 36a364c2ddc..5aaa69dad2c 100644 --- a/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx @@ -111,4 +111,25 @@ describe("ChatRow - inline diff stats and actions", () => { 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, + }), + } + + 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/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts index 203fa974053..51a933c5681 100644 --- a/webview-ui/src/utils/diffUtils.ts +++ b/webview-ui/src/utils/diffUtils.ts @@ -125,20 +125,22 @@ export function convertSearchReplaceToUnifiedDiff(content: string, filePath?: st return hasBlocks ? unified : content } -/** Build a unified diff for a brand new file (all content lines are additions) */ +/** 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 fileName = filePath || "file" // Normalize EOLs to keep counts consistent const normalized = content.replace(/\r\n/g, "\n") - const lines = normalized.split("\n") + const parts = normalized.split("\n") + // Drop trailing empty item produced by a final newline so we count only real content lines + const contentLines = parts[parts.length - 1] === "" ? parts.slice(0, -1) : parts let diff = `--- /dev/null\n` diff += `+++ ${fileName}\n` - diff += `@@ -0,0 +1,${normalized === "" ? 0 : lines.length} @@\n` + diff += `@@ -0,0 +1,${contentLines.length} @@\n` - for (const line of lines) { - // Preserve final newline behavior: if content ended with newline, split will produce trailing "" - // which is still okay to emit as "+", it represents a blank line. + for (const line of contentLines) { diff += `+${line}\n` } From e4f186601fe56c709282704934843d621c9915bb Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 30 Oct 2025 15:46:34 -0600 Subject: [PATCH 12/22] webview-ui: remove redundant CDATA replacements; improve unified diff header detection; reuse extractUnifiedDiff for new-file rendering --- webview-ui/src/components/chat/ChatRow.tsx | 20 +------------------- webview-ui/src/utils/diffUtils.ts | 7 ++----- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 184225072b8..f20d9aa190e 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -177,24 +177,6 @@ function computeDiffStats(diff?: string): { added: number; removed: number } | n return null } -/** - * Converts new file content to unified diff format (all lines as additions) - */ -function convertNewFileToUnifiedDiff(content: string, filePath?: string): string { - const fileName = filePath || "file" - const lines = content.split("\n") - - let diff = `--- /dev/null\n` - diff += `+++ ${fileName}\n` - diff += `@@ -0,0 +1,${lines.length} @@\n` - - for (const line of lines) { - diff += `+${line}\n` - } - - return diff -} - export const ChatRowContent = ({ message, lastModifiedMessage, @@ -640,7 +622,7 @@ export const ChatRowContent = ({
/gi, "") - .replace(/\]\]>/g, "") ) } From 223e04d0ee32f88b85c40ce17a0f6bf233b6de6b Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 3 Nov 2025 16:29:20 -0700 Subject: [PATCH 13/22] apply_diff: stream unified diffs to UI during batch preview; include unified patch in single-file approval; remove client-side fallback normalization for appliedDiff; align gap-row styling to editor background --- src/core/tools/applyDiffTool.ts | 5 ++ src/core/tools/multiApplyDiffTool.ts | 52 ++++++++++++++++--- .../src/components/chat/BatchDiffApproval.tsx | 11 +--- webview-ui/src/components/chat/ChatRow.tsx | 17 ++++-- webview-ui/src/components/common/DiffView.tsx | 7 ++- 5 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index dcdd1346240..daef418962e 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -140,6 +140,9 @@ export async function applyDiffToolLegacy( cline.consecutiveMistakeCount = 0 cline.consecutiveMistakeCountForApplyDiff.delete(relPath) + // Generate backend-unified diff for display in chat/webview + const unifiedPatch = formatResponse.createPrettyPatch(relPath, originalContent, diffResult.content) + // Check if preventFocusDisruption experiment is enabled const provider = cline.providerRef.deref() const state = await provider?.getState() @@ -158,6 +161,7 @@ export async function applyDiffToolLegacy( const completeMessage = JSON.stringify({ ...sharedMessageProps, diff: diffContent, + content: unifiedPatch, isProtected: isWriteProtected, } satisfies ClineSayTool) @@ -194,6 +198,7 @@ export async function applyDiffToolLegacy( const completeMessage = JSON.stringify({ ...sharedMessageProps, diff: diffContent, + content: unifiedPatch, isProtected: isWriteProtected, } satisfies ClineSayTool) diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts index a30778c5af0..5c6f7bd1d60 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/src/core/tools/multiApplyDiffTool.ts @@ -282,31 +282,66 @@ 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 + 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 = "" + } + + batchDiffs.push({ path: readablePath, changeCount, key: `${readablePath} (${changeText})`, - content: opResult.path, // Full relative path + content: unified, 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 +453,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 +576,11 @@ ${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 unifiedPatch = formatResponse.createPrettyPatch(relPath, beforeContent!, originalContent!) const operationMessage = JSON.stringify({ ...sharedMessageProps, diff: diffContents, + content: unifiedPatch, } satisfies ClineSayTool) let toolProgressStatus diff --git a/webview-ui/src/components/chat/BatchDiffApproval.tsx b/webview-ui/src/components/chat/BatchDiffApproval.tsx index 815b648dffa..5e99b15c0bf 100644 --- a/webview-ui/src/components/chat/BatchDiffApproval.tsx +++ b/webview-ui/src/components/chat/BatchDiffApproval.tsx @@ -1,6 +1,5 @@ import React, { memo, useState } from "react" import CodeAccordian from "../common/CodeAccordian" -import { extractUnifiedDiff } from "../../utils/diffUtils" interface FileDiff { path: string @@ -57,14 +56,8 @@ export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProp
{files.map((file) => { - // Normalize to unified diff and compute stats - const rawCombined = file.diffs?.map((d) => d.content).join("\n\n") || file.content - const unified = extractUnifiedDiff({ - toolName: "appliedDiff", - path: file.path, - diff: rawCombined, - content: undefined, - }) + // Use backend-provided unified diff only. No client-side fallback for apply_diff batches. + const unified = file.content || "" const stats = computeUnifiedStats(unified) return ( diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index f20d9aa190e..e78c926717c 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -399,13 +399,16 @@ export const ChatRowContent = ({ // Inline diff stats for edit/apply_diff/insert/search-replace/newFile asks const diffTextForStats = useMemo(() => { if (!tool) return "" - // Normalize to unified diff using frontend-only capture/surmise helper + // For appliedDiff, backend provides unified diff; do not fallback/normalize + if ((tool as any).tool === "appliedDiff") { + return ((tool as any).content as string) || "" + } return ( extractUnifiedDiff({ toolName: tool.tool as string, path: tool.path, - diff: (tool as any).diff, - content: (tool as any).content, + diff: (tool as any).content, + content: (tool as any).diff, }) || "" ) }, [tool]) @@ -417,11 +420,15 @@ export const ChatRowContent = ({ // Clean diff content for display (normalize to unified diff) const cleanDiffContent = useMemo(() => { if (!tool) return undefined + // For appliedDiff, show backend's unified diff directly + if ((tool as any).tool === "appliedDiff") { + return ((tool as any).content as string) || undefined + } const unified = extractUnifiedDiff({ toolName: tool.tool as string, path: tool.path, - diff: (tool as any).diff, - content: (tool as any).content, + diff: (tool as any).content, + content: (tool as any).diff, }) return unified || undefined }, [tool]) diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index f69b35264e4..0da6fad65cd 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -173,7 +173,8 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { {diffLines.map((line, idx) => { // Render compact separator between hunks if (line.type === "gap") { - const gapBg = "color-mix(in srgb, var(--vscode-editorGroup-border) 100%, transparent)" + // Match the header/container background tone + const gapBg = "var(--vscode-editor-background)" return ( { /> From 6be5714fd96d48136f2fdd41325fd194bfc4d064 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 3 Nov 2025 18:48:24 -0700 Subject: [PATCH 14/22] refactor(diffUtils): remove SEARCH/REPLACE handling and normalize diff content processing --- webview-ui/src/components/chat/ChatRow.tsx | 12 +--- webview-ui/src/utils/diffUtils.ts | 78 +--------------------- 2 files changed, 5 insertions(+), 85 deletions(-) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index e78c926717c..8d46e918f5c 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -399,15 +399,11 @@ export const ChatRowContent = ({ // Inline diff stats for edit/apply_diff/insert/search-replace/newFile asks const diffTextForStats = useMemo(() => { if (!tool) return "" - // For appliedDiff, backend provides unified diff; do not fallback/normalize - if ((tool as any).tool === "appliedDiff") { - return ((tool as any).content as string) || "" - } return ( extractUnifiedDiff({ toolName: tool.tool as string, path: tool.path, - diff: (tool as any).content, + diff: (tool as any).content ?? (tool as any).diff, content: (tool as any).diff, }) || "" ) @@ -420,14 +416,10 @@ export const ChatRowContent = ({ // Clean diff content for display (normalize to unified diff) const cleanDiffContent = useMemo(() => { if (!tool) return undefined - // For appliedDiff, show backend's unified diff directly - if ((tool as any).tool === "appliedDiff") { - return ((tool as any).content as string) || undefined - } const unified = extractUnifiedDiff({ toolName: tool.tool as string, path: tool.path, - diff: (tool as any).content, + diff: (tool as any).content ?? (tool as any).diff, content: (tool as any).diff, }) return unified || undefined diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts index df8b22db216..261acf59496 100644 --- a/webview-ui/src/utils/diffUtils.ts +++ b/webview-ui/src/utils/diffUtils.ts @@ -1,7 +1,6 @@ /** * Frontend-only normalization helper. * - If a unified diff already exists, return it. - * - If a Roo SEARCH/REPLACE block is provided, convert to unified diff. * - If it's a new file with raw content, synthesize a unified diff with all lines as additions. * - Otherwise, pass through raw content (DiffView will no-op if not unified). */ @@ -17,17 +16,14 @@ export function extractUnifiedDiff(params: { if (!raw) return "" raw = stripCData(raw) + // Remove diff noise lines like "\ No newline at end of file" + raw = raw.replace(/(^|\n)[ \t]*(?:\\ )?No newline at end of file[ \t]*(?=\n|$)/gi, "$1") // Explicit new file: build a unified diff from raw content if ((params.toolName || "").toLowerCase() === "newfilecreated") { return convertNewFileToUnifiedDiff(raw, filePath) } - // SEARCH/REPLACE blocks → unified - if (isSearchReplace(raw)) { - return convertSearchReplaceToUnifiedDiff(raw, filePath) - } - // Already unified? if (isUnifiedDiff(raw)) { return raw @@ -44,13 +40,6 @@ function isUnifiedDiff(s: string): boolean { return hasHunk || hasHeaders } -/** Detects Roo SEARCH/REPLACE multi-block format */ -function isSearchReplace(s: string): boolean { - return ( - /(^|\n)<<<<<<< ?SEARCH|(^|\n)<<<<<</.test(s) || /(^|\n)<<<<<<< ?SEARCH/.test(s) - ) -} - /** Remove CDATA markers and any HTML-encoded variants */ function stripCData(s: string): string { return ( @@ -61,71 +50,10 @@ function stripCData(s: string): string { ) } -/** - * Convert Roo SEARCH/REPLACE blocks into unified diff using the diff library. - * Matches optional metadata lines and optional '-------' separator. - */ -export function convertSearchReplaceToUnifiedDiff(content: string, filePath?: string): string { - // Backend-compatible regex: captures :start_line: and :end_line:, optional '-------', and SEARCH/REPLACE bodies - const blockRegex = - /(?:^|\n)(??\s*\n((?::start_line:\s*(\d+)\s*\n))?((?::end_line:\s*(\d+)\s*\n))?((?>>>>>> REPLACE)(?=\n|$)/g - - const fileName = filePath || "file" - let hasBlocks = false - let headerEmitted = false - let unified = "" - - // Helper to normalize EOLs and get stable line arrays without trailing empty caused by final newline - const toStableLines = (s: string): string[] => { - const norm = s.replace(/\r\n/g, "\n") - if (norm === "") return [] - const parts = norm.split("\n") - return parts[parts.length - 1] === "" ? parts.slice(0, -1) : parts - } - - let match: RegExpExecArray | null - while ((match = blockRegex.exec(content)) !== null) { - hasBlocks = true - - // 1: full start_line line, 2: start_line number, 3: full end_line line, 4: end_line number - const startLine = match[2] ? parseInt(match[2], 10) : 1 - const endLine = match[4] ? parseInt(match[4], 10) : undefined - - // 6: SEARCH body, 7: REPLACE body - const searchBody = match[6] ?? "" - const replaceBody = match[7] ?? "" - - const searchLines = toStableLines(searchBody) - const replaceLines = toStableLines(replaceBody) - - // Old/new hunk metadata. If end_line is present, prefer it for oldLines; otherwise count SEARCH lines. - const oldStart = startLine - const oldLines = endLine !== undefined ? Math.max(0, endLine - startLine + 1) : searchLines.length - const newStart = startLine - const newLines = replaceLines.length - - // Emit file headers once so parsePatch can recognize a complete unified diff - if (!headerEmitted) { - unified += `--- a/${fileName}\n` - unified += `+++ b/${fileName}\n` - headerEmitted = true - } - - // Hunk header - unified += `@@ -${oldStart},${oldLines} +${newStart},${newLines} @@\n` - - // We don't have surrounding context here; emit deletions then additions to visualize the change - for (const line of searchLines) unified += `-${line}\n` - for (const line of replaceLines) unified += `+${line}\n` - } - - return hasBlocks ? unified : content -} - /** 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 { +function convertNewFileToUnifiedDiff(content: string, filePath?: string): string { const fileName = filePath || "file" // Normalize EOLs to keep counts consistent const normalized = content.replace(/\r\n/g, "\n") From 55973b52d24d163dca9857d8fe0fbbccba16651c Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Mon, 3 Nov 2025 20:30:19 -0700 Subject: [PATCH 15/22] fix(webview): accept 'searchAndReplace' in ChatRow switch via string discriminant --- webview-ui/src/components/chat/ChatRow.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 8d46e918f5c..3a96db3586b 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -439,7 +439,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 From 266650e6e06454ee6df426036ec5d60d9574859e Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Tue, 4 Nov 2025 00:20:14 -0700 Subject: [PATCH 16/22] fix(webview-ui): Address PR review findings for diff view improvements - Fix stripCData() to properly handle HTML-encoded CDATA markers - Fix convertNewFileToUnifiedDiff() to emit valid hunks for empty files - Improve DiffView patch selection to match by filePath - Add performance optimization to disable highlighting for large diffs (>1000 lines) - Improve theme detection using proper VS Code theme classes - Extract duplicate diff stats logic into shared utility module --- .../src/components/chat/BatchDiffApproval.tsx | 24 +---- webview-ui/src/components/chat/ChatRow.tsx | 61 +----------- .../src/components/common/CodeAccordian.tsx | 58 +----------- webview-ui/src/components/common/DiffView.tsx | 29 ++++-- webview-ui/src/utils/diffStats.ts | 92 +++++++++++++++++++ webview-ui/src/utils/diffUtils.ts | 8 +- 6 files changed, 125 insertions(+), 147 deletions(-) create mode 100644 webview-ui/src/utils/diffStats.ts diff --git a/webview-ui/src/components/chat/BatchDiffApproval.tsx b/webview-ui/src/components/chat/BatchDiffApproval.tsx index 5e99b15c0bf..162ccba8f35 100644 --- a/webview-ui/src/components/chat/BatchDiffApproval.tsx +++ b/webview-ui/src/components/chat/BatchDiffApproval.tsx @@ -1,5 +1,6 @@ import React, { memo, useState } from "react" import CodeAccordian from "../common/CodeAccordian" +import { computeUnifiedDiffStats } from "../../utils/diffStats" interface FileDiff { path: string @@ -17,27 +18,6 @@ interface BatchDiffApprovalProps { ts: number } -/** Compute +/− from a unified diff (ignores headers/hunk lines) */ -function computeUnifiedStats(diff?: string): { added: number; removed: number } | null { - if (!diff) return null - let added = 0 - let removed = 0 - let saw = false - for (const line of diff.split("\n")) { - if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue - if (line.startsWith("+")) { - added++ - saw = true - } else if (line.startsWith("-")) { - removed++ - saw = true - } - } - return saw && (added > 0 || removed > 0) ? { added, removed } : null -} - -/* keep placeholder (legacy) – replaced by computeUnifiedStats after normalization */ - export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProps) => { const [expandedFiles, setExpandedFiles] = useState>({}) @@ -58,7 +38,7 @@ export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProp {files.map((file) => { // Use backend-provided unified diff only. No client-side fallback for apply_diff batches. const unified = file.content || "" - const stats = computeUnifiedStats(unified) + const stats = computeUnifiedDiffStats(unified) return (
diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 3a96db3586b..d8e4cc868bd 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -25,6 +25,7 @@ import Thumbnails from "../common/Thumbnails" import ImageBlock from "../common/ImageBlock" import ErrorRow from "./ErrorRow" import { extractUnifiedDiff } from "../../utils/diffUtils" +import { computeDiffStats } from "../../utils/diffStats" import McpResourceRow from "../mcp/McpResourceRow" @@ -117,66 +118,6 @@ const ChatRow = memo( export default ChatRow -function computeDiffStats(diff?: string): { added: number; removed: number } | null { - if (!diff) return null - - // Strategy 1: Unified diff (+/- lines) - let added = 0 - let removed = 0 - let sawPlusMinus = false - for (const line of diff.split("\n")) { - if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue - if (line.startsWith("+")) { - added++ - sawPlusMinus = true - } else if (line.startsWith("-")) { - removed++ - sawPlusMinus = true - } - } - if (sawPlusMinus) { - if (added === 0 && removed === 0) return null - return { added, removed } - } - - // Strategy 2: Roo multi-search-replace blocks - // Count lines in SEARCH vs REPLACE sections across all blocks - // Matches optional metadata lines and optional '-------' line - const blockRegex = - /<<<<<>>>>>> REPLACE)/gim - - let hasBlocks = false - added = 0 - removed = 0 - - const asLines = (s: string) => { - // Normalize Windows newlines and trim trailing newline so counts reflect real lines - const norm = s.replace(/\r\n/g, "\n") - if (norm === "") return 0 - // Split, drop potential trailing empty caused by final newline - const parts = norm.split("\n") - return parts[parts.length - 1] === "" ? parts.length - 1 : parts.length - } - - let match: RegExpExecArray | null - while ((match = blockRegex.exec(diff)) !== null) { - hasBlocks = true - const searchContent = match[1] ?? "" - const replaceContent = match[2] ?? "" - const searchCount = asLines(searchContent) - const replaceCount = asLines(replaceContent) - if (replaceCount > searchCount) added += replaceCount - searchCount - else if (searchCount > replaceCount) removed += searchCount - replaceCount - } - - if (hasBlocks) { - if (added === 0 && removed === 0) return null - return { added, removed } - } - - return null -} - export const ChatRowContent = ({ message, lastModifiedMessage, diff --git a/webview-ui/src/components/common/CodeAccordian.tsx b/webview-ui/src/components/common/CodeAccordian.tsx index fa5d2cd53ef..b3b9d59814d 100644 --- a/webview-ui/src/components/common/CodeAccordian.tsx +++ b/webview-ui/src/components/common/CodeAccordian.tsx @@ -8,6 +8,7 @@ import { ToolUseBlock, ToolUseBlockHeader } from "./ToolUseBlock" import CodeBlock from "./CodeBlock" import { PathTooltip } from "../ui/PathTooltip" import DiffView from "./DiffView" +import { computeDiffStats } from "../../utils/diffStats" interface CodeAccordianProps { path?: string @@ -24,61 +25,6 @@ interface CodeAccordianProps { diffStats?: { added: number; removed: number } } -// Fallback computation of + / - counts from code (supports both unified diff and Roo's multi-search-replace blocks) -function computeDiffStatsFromCode(diff?: string): { added: number; removed: number } | null { - if (!diff) return null - - // Strategy 1: unified diff markers - let added = 0 - let removed = 0 - let sawPlusMinus = false - for (const line of diff.split("\n")) { - if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue - if (line.startsWith("+")) { - added++ - sawPlusMinus = true - } else if (line.startsWith("-")) { - removed++ - sawPlusMinus = true - } - } - if (sawPlusMinus) { - if (added === 0 && removed === 0) return null - return { added, removed } - } - - // Strategy 2: Roo multi-search-replace blocks - const blockRegex = - /<<<<<>>>>>> REPLACE)/gim - - const asLines = (s: string) => { - const norm = (s || "").replace(/\r\n/g, "\n") - if (!norm) return 0 - const parts = norm.split("\n") - return parts[parts.length - 1] === "" ? parts.length - 1 : parts.length - } - - let hasBlocks = false - added = 0 - removed = 0 - - let match: RegExpExecArray | null - while ((match = blockRegex.exec(diff)) !== null) { - hasBlocks = true - const searchCount = asLines(match[1] ?? "") - const replaceCount = asLines(match[2] ?? "") - if (replaceCount > searchCount) added += replaceCount - searchCount - else if (searchCount > replaceCount) removed += searchCount - replaceCount - } - - if (hasBlocks) { - if (added === 0 && removed === 0) return null - return { added, removed } - } - - return null -} - const CodeAccordian = ({ path, code = "", @@ -100,7 +46,7 @@ const CodeAccordian = ({ const derivedStats = useMemo(() => { if (diffStats && (diffStats.added > 0 || diffStats.removed > 0)) return diffStats if ((language || inferredLanguage) && (language || inferredLanguage) === "diff") { - return computeDiffStatsFromCode(source || code || "") + return computeDiffStats(source || code || "") } return null }, [diffStats, language, inferredLanguage, source, code]) diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index 0da6fad65cd..1473d5d4461 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -26,10 +26,11 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { // Determine language from file path and prepare highlighter const normalizedLang = useMemo(() => normalizeLanguage(getLanguageFromPath(filePath || "") || "txt"), [filePath]) const [highlighter, setHighlighter] = useState(null) - const isLightTheme = useMemo( - () => typeof document !== "undefined" && document.body.className.toLowerCase().includes("light"), - [], - ) + 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) + }, []) useEffect(() => { let mounted = true @@ -45,8 +46,14 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { } }, [normalizedLang]) + // 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]) + const renderHighlighted = (code: string): React.ReactNode => { - if (!highlighter) return code + if (!highlighter || !shouldHighlight) return code try { const hast: any = highlighter.codeToHast(code, { lang: normalizedLang, @@ -87,7 +94,15 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { if (!patches || patches.length === 0) return [] const lines: DiffLine[] = [] - const patch = patches[0] + const patch = filePath + ? (patches.find((p) => + [p.newFileName, p.oldFileName].some( + (n) => typeof n === "string" && (n === filePath || n?.endsWith("/" + filePath)), + ), + ) ?? patches[0]) + : patches[0] + + if (!patch) return [] let prevHunk: any = null for (const hunk of patch.hunks) { @@ -151,7 +166,7 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { console.error("[DiffView] Failed to parse diff:", error) return [] } - }, [source]) + }, [source, filePath]) return (
0 || removed > 0)) { + return { added, removed } + } + + return null +} + +/** + * Compute +/− counts from Roo's multi-search-replace block format + */ +export function computeSearchReplaceDiffStats(diff?: string): { added: number; removed: number } | null { + if (!diff) return null + + // Matches optional metadata lines and optional '-------' line + const blockRegex = + /<<<<<>>>>>> REPLACE)/gim + + const asLines = (s: string) => { + // Normalize Windows newlines and trim trailing newline so counts reflect real lines + const norm = (s || "").replace(/\r\n/g, "\n") + if (!norm) return 0 + const parts = norm.split("\n") + return parts[parts.length - 1] === "" ? parts.length - 1 : parts.length + } + + let hasBlocks = false + let added = 0 + let removed = 0 + + let match: RegExpExecArray | null + while ((match = blockRegex.exec(diff)) !== null) { + hasBlocks = true + const searchContent = match[1] ?? "" + const replaceContent = match[2] ?? "" + const searchCount = asLines(searchContent) + const replaceCount = asLines(replaceContent) + + if (replaceCount > searchCount) { + added += replaceCount - searchCount + } else if (searchCount > replaceCount) { + removed += searchCount - replaceCount + } + } + + if (hasBlocks && (added > 0 || removed > 0)) { + return { added, removed } + } + + 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): { added: number; removed: number } | null { + if (!diff) return null + + // Try unified diff format first + const unifiedStats = computeUnifiedDiffStats(diff) + if (unifiedStats) return unifiedStats + + // Fall back to search-replace format + return computeSearchReplaceDiffStats(diff) +} diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts index 261acf59496..4eeb235c398 100644 --- a/webview-ui/src/utils/diffUtils.ts +++ b/webview-ui/src/utils/diffUtils.ts @@ -44,7 +44,10 @@ function isUnifiedDiff(s: string): boolean { function stripCData(s: string): string { return ( s - // Remove HTML-encoded and raw CDATA open/close (case-insensitive covers both) + // First, normalize HTML-encoded CDATA markers to raw + .replace(/<!\[CDATA\[/gi, "") + // Then strip raw markers .replace(//gi, "") ) @@ -61,9 +64,10 @@ function convertNewFileToUnifiedDiff(content: string, filePath?: string): string // Drop trailing empty item produced by a final newline so we count only real content lines const contentLines = parts[parts.length - 1] === "" ? parts.slice(0, -1) : parts + const count = contentLines.length let diff = `--- /dev/null\n` diff += `+++ ${fileName}\n` - diff += `@@ -0,0 +1,${contentLines.length} @@\n` + diff += `@@ -0,0 +${count ? 1 : 0},${count} @@\n` for (const line of contentLines) { diff += `+${line}\n` From 504d12cd5e4e8f03b176ea41694ede7b6c1bcb54 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Tue, 4 Nov 2025 00:23:35 -0700 Subject: [PATCH 17/22] fix(tools): Generate unified diff for write_to_file in background editing mode When PREVENT_FOCUS_DISRUPTION experiment is enabled, write_to_file was not generating a unified diff for existing files, causing the diff not to display in the chat view. Now generates the same diff format as non-background mode for consistency. --- src/core/tools/writeToFileTool.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index 5abd96a20af..bda35fe055c 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -204,7 +204,10 @@ export async function writeToFileTool( const completeMessage = JSON.stringify({ ...sharedMessageProps, - content: newContent, + content: fileExists ? undefined : newContent, + diff: fileExists + ? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent) + : undefined, } satisfies ClineSayTool) const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected) From 7ed323863304e44c648c66f819639ac15000a5df Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Tue, 4 Nov 2025 00:37:18 -0700 Subject: [PATCH 18/22] fix(tools): Load original content before generating diff in background mode Moved originalContent loading before diff generation to ensure proper diff comparison. Previously, the diff was being generated with empty originalContent, causing all lines to appear as additions. --- src/core/tools/writeToFileTool.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index bda35fe055c..c43c95b70b7 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -173,6 +173,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) { @@ -216,15 +225,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 { From 35c1f038bdbc2eb4c387d9f9686728153bde4c79 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Wed, 5 Nov 2025 20:15:08 -0700 Subject: [PATCH 19/22] Moved business logic out of webview and cleaned up --- src/core/diff/stats.ts | 71 ++++++++++++++ src/core/tools/applyDiffTool.ts | 7 +- src/core/tools/insertContentTool.ts | 29 +++--- src/core/tools/multiApplyDiffTool.ts | 11 ++- src/core/tools/writeToFileTool.ts | 23 +++-- src/shared/ExtensionMessage.ts | 4 + .../src/components/chat/BatchDiffApproval.tsx | 7 +- webview-ui/src/components/chat/ChatRow.tsx | 47 +++------- .../__tests__/ChatRow.diff-actions.spec.tsx | 4 + .../src/components/common/CodeAccordian.tsx | 8 +- webview-ui/src/utils/diffStats.ts | 92 ------------------- webview-ui/src/utils/diffUtils.ts | 77 ---------------- 12 files changed, 140 insertions(+), 240 deletions(-) create mode 100644 src/core/diff/stats.ts delete mode 100644 webview-ui/src/utils/diffStats.ts delete mode 100644 webview-ui/src/utils/diffUtils.ts diff --git a/src/core/diff/stats.ts b/src/core/diff/stats.ts new file mode 100644 index 00000000000..b842f5c04e8 --- /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/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index daef418962e..1077b7bf390 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, @@ -141,7 +142,9 @@ export async function applyDiffToolLegacy( cline.consecutiveMistakeCountForApplyDiff.delete(relPath) // Generate backend-unified diff for display in chat/webview - const unifiedPatch = formatResponse.createPrettyPatch(relPath, originalContent, diffResult.content) + 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() @@ -162,6 +165,7 @@ export async function applyDiffToolLegacy( ...sharedMessageProps, diff: diffContent, content: unifiedPatch, + diffStats, isProtected: isWriteProtected, } satisfies ClineSayTool) @@ -199,6 +203,7 @@ export async function applyDiffToolLegacy( ...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 e7d3a06ab92..38ca309a3b3 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 5c6f7bd1d60..08bce08ede1 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 @@ -288,6 +289,7 @@ Original error: ${errorMessage}` changeCount: number key: string content: string + diffStats?: { added: number; removed: number } diffs?: Array<{ content: string; startLine?: number }> }> = [] @@ -314,11 +316,14 @@ Original error: ${errorMessage}` unified = "" } + const unifiedSanitized = sanitizeUnifiedDiff(unified) + const stats = computeDiffStats(unifiedSanitized) || undefined batchDiffs.push({ path: readablePath, changeCount, key: `${readablePath} (${changeText})`, - content: unified, + content: unifiedSanitized, + diffStats: stats, diffs: opResult.diffItems?.map((item) => ({ content: item.content, startLine: item.startLine, @@ -576,11 +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 unifiedPatch = formatResponse.createPrettyPatch(relPath, beforeContent!, originalContent!) + 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 c43c95b70b7..b8e6da0caa2 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, @@ -211,12 +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: 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) @@ -278,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 7d2759c9190..c3926d5073e 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/src/components/chat/BatchDiffApproval.tsx b/webview-ui/src/components/chat/BatchDiffApproval.tsx index 162ccba8f35..a88914cd88a 100644 --- a/webview-ui/src/components/chat/BatchDiffApproval.tsx +++ b/webview-ui/src/components/chat/BatchDiffApproval.tsx @@ -1,12 +1,12 @@ import React, { memo, useState } from "react" import CodeAccordian from "../common/CodeAccordian" -import { computeUnifiedDiffStats } from "../../utils/diffStats" interface FileDiff { path: string changeCount: number key: string content: string + diffStats?: { added: number; removed: number } diffs?: Array<{ content: string startLine?: number @@ -36,9 +36,8 @@ export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProp
{files.map((file) => { - // Use backend-provided unified diff only. No client-side fallback for apply_diff batches. + // Use backend-provided unified diff only. Stats also provided by backend. const unified = file.content || "" - const stats = computeUnifiedDiffStats(unified) return (
@@ -48,7 +47,7 @@ export const BatchDiffApproval = memo(({ files = [], ts }: BatchDiffApprovalProp language="diff" isExpanded={expandedFiles[file.path] || false} onToggleExpand={() => handleToggleExpand(file.path)} - diffStats={stats ?? undefined} + diffStats={file.diffStats ?? undefined} />
) diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index d8e4cc868bd..be51501a4a7 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -24,8 +24,6 @@ import { ReasoningBlock } from "./ReasoningBlock" import Thumbnails from "../common/Thumbnails" import ImageBlock from "../common/ImageBlock" import ErrorRow from "./ErrorRow" -import { extractUnifiedDiff } from "../../utils/diffUtils" -import { computeDiffStats } from "../../utils/diffStats" import McpResourceRow from "../mcp/McpResourceRow" @@ -337,33 +335,10 @@ export const ChatRowContent = ({ [message.ask, message.text], ) - // Inline diff stats for edit/apply_diff/insert/search-replace/newFile asks - const diffTextForStats = useMemo(() => { - if (!tool) return "" - return ( - extractUnifiedDiff({ - toolName: tool.tool as string, - path: tool.path, - diff: (tool as any).content ?? (tool as any).diff, - content: (tool as any).diff, - }) || "" - ) - }, [tool]) - - const diffStatsForInline = useMemo(() => { - return computeDiffStats(diffTextForStats) - }, [diffTextForStats]) - - // Clean diff content for display (normalize to unified diff) - const cleanDiffContent = useMemo(() => { + // Unified diff content (provided by backend when relevant) + const unifiedDiff = useMemo(() => { if (!tool) return undefined - const unified = extractUnifiedDiff({ - toolName: tool.tool as string, - path: tool.path, - diff: (tool as any).content ?? (tool as any).diff, - content: (tool as any).diff, - }) - return unified || undefined + return ((tool as any).content ?? (tool as any).diff) as string | undefined }, [tool]) const followUpData = useMemo(() => { @@ -421,13 +396,13 @@ export const ChatRowContent = ({
@@ -459,13 +434,13 @@ export const ChatRowContent = ({
@@ -493,13 +468,13 @@ export const ChatRowContent = ({
@@ -562,13 +537,13 @@ export const ChatRowContent = ({
vscode.postMessage({ type: "openFile", text: "./" + tool.path })} - diffStats={diffStatsForInline ?? undefined} + diffStats={(tool as any).diffStats ?? undefined} />
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 index 5aaa69dad2c..61a6633f866 100644 --- a/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatRow.diff-actions.spec.tsx @@ -61,6 +61,7 @@ describe("ChatRow - inline diff stats and actions", () => { tool: "editedExistingFile", path: "src/file.ts", diff, + diffStats: { added: 1, removed: 1 }, }), } @@ -82,6 +83,7 @@ describe("ChatRow - inline diff stats and actions", () => { tool: "searchAndReplace", path: "src/file.ts", diff, + diffStats: { added: 1, removed: 2 }, }), } @@ -102,6 +104,7 @@ describe("ChatRow - inline diff stats and actions", () => { tool: "newFileCreated", path: "src/new-file.ts", content, + diffStats: { added: 3, removed: 0 }, }), } @@ -123,6 +126,7 @@ describe("ChatRow - inline diff stats and actions", () => { tool: "newFileCreated", path: "src/new-file.ts", content, + diffStats: { added: 3, removed: 0 }, }), } diff --git a/webview-ui/src/components/common/CodeAccordian.tsx b/webview-ui/src/components/common/CodeAccordian.tsx index b3b9d59814d..ebc4dd09c30 100644 --- a/webview-ui/src/components/common/CodeAccordian.tsx +++ b/webview-ui/src/components/common/CodeAccordian.tsx @@ -8,7 +8,6 @@ import { ToolUseBlock, ToolUseBlockHeader } from "./ToolUseBlock" import CodeBlock from "./CodeBlock" import { PathTooltip } from "../ui/PathTooltip" import DiffView from "./DiffView" -import { computeDiffStats } from "../../utils/diffStats" interface CodeAccordianProps { path?: string @@ -42,14 +41,11 @@ const CodeAccordian = ({ const source = useMemo(() => code.trim(), [code]) const hasHeader = Boolean(path || isFeedback || header) - // Derive diff stats from code when not provided + // Use provided diff stats only (render-only) const derivedStats = useMemo(() => { if (diffStats && (diffStats.added > 0 || diffStats.removed > 0)) return diffStats - if ((language || inferredLanguage) && (language || inferredLanguage) === "diff") { - return computeDiffStats(source || code || "") - } return null - }, [diffStats, language, inferredLanguage, source, code]) + }, [diffStats]) const hasValidStats = Boolean(derivedStats && (derivedStats.added > 0 || derivedStats.removed > 0)) diff --git a/webview-ui/src/utils/diffStats.ts b/webview-ui/src/utils/diffStats.ts deleted file mode 100644 index f16eb352d7c..00000000000 --- a/webview-ui/src/utils/diffStats.ts +++ /dev/null @@ -1,92 +0,0 @@ -/** - * Shared utility for computing diff statistics from various diff formats - */ - -/** - * Compute +/− counts from a unified diff (ignores headers/hunk lines) - */ -export function computeUnifiedDiffStats(diff?: string): { added: number; removed: number } | null { - if (!diff) return null - - let added = 0 - let removed = 0 - let sawPlusMinus = false - - for (const line of diff.split("\n")) { - // Skip unified diff headers and hunk markers - if (line.startsWith("+++ ") || line.startsWith("--- ") || line.startsWith("@@")) continue - - if (line.startsWith("+")) { - added++ - sawPlusMinus = true - } else if (line.startsWith("-")) { - removed++ - sawPlusMinus = true - } - } - - if (sawPlusMinus && (added > 0 || removed > 0)) { - return { added, removed } - } - - return null -} - -/** - * Compute +/− counts from Roo's multi-search-replace block format - */ -export function computeSearchReplaceDiffStats(diff?: string): { added: number; removed: number } | null { - if (!diff) return null - - // Matches optional metadata lines and optional '-------' line - const blockRegex = - /<<<<<>>>>>> REPLACE)/gim - - const asLines = (s: string) => { - // Normalize Windows newlines and trim trailing newline so counts reflect real lines - const norm = (s || "").replace(/\r\n/g, "\n") - if (!norm) return 0 - const parts = norm.split("\n") - return parts[parts.length - 1] === "" ? parts.length - 1 : parts.length - } - - let hasBlocks = false - let added = 0 - let removed = 0 - - let match: RegExpExecArray | null - while ((match = blockRegex.exec(diff)) !== null) { - hasBlocks = true - const searchContent = match[1] ?? "" - const replaceContent = match[2] ?? "" - const searchCount = asLines(searchContent) - const replaceCount = asLines(replaceContent) - - if (replaceCount > searchCount) { - added += replaceCount - searchCount - } else if (searchCount > replaceCount) { - removed += searchCount - replaceCount - } - } - - if (hasBlocks && (added > 0 || removed > 0)) { - return { added, removed } - } - - 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): { added: number; removed: number } | null { - if (!diff) return null - - // Try unified diff format first - const unifiedStats = computeUnifiedDiffStats(diff) - if (unifiedStats) return unifiedStats - - // Fall back to search-replace format - return computeSearchReplaceDiffStats(diff) -} diff --git a/webview-ui/src/utils/diffUtils.ts b/webview-ui/src/utils/diffUtils.ts deleted file mode 100644 index 4eeb235c398..00000000000 --- a/webview-ui/src/utils/diffUtils.ts +++ /dev/null @@ -1,77 +0,0 @@ -/** - * Frontend-only normalization helper. - * - If a unified diff already exists, return it. - * - If it's a new file with raw content, synthesize a unified diff with all lines as additions. - * - Otherwise, pass through raw content (DiffView will no-op if not unified). - */ -export function extractUnifiedDiff(params: { - toolName?: string - path?: string - diff?: string - content?: string -}): string { - const filePath = params.path || "file" - let raw = (params.diff ?? params.content ?? "") || "" - - if (!raw) return "" - - raw = stripCData(raw) - // Remove diff noise lines like "\ No newline at end of file" - raw = raw.replace(/(^|\n)[ \t]*(?:\\ )?No newline at end of file[ \t]*(?=\n|$)/gi, "$1") - - // Explicit new file: build a unified diff from raw content - if ((params.toolName || "").toLowerCase() === "newfilecreated") { - return convertNewFileToUnifiedDiff(raw, filePath) - } - - // Already unified? - if (isUnifiedDiff(raw)) { - return raw - } - - // Fallback: return as-is (non-unified content) - return raw -} - -/** Detects unified diff by presence of headers/hunks */ -function isUnifiedDiff(s: string): boolean { - const hasHunk = /(^|\n)@@\s+-[0-9,]+\s+\+[0-9,]+\s+@@/.test(s) - const hasHeaders = /(^|\n)---\s|(^|\n)\+\+\+\s/.test(s) - return hasHunk || hasHeaders -} - -/** Remove CDATA markers and any HTML-encoded variants */ -function stripCData(s: string): string { - return ( - s - // First, normalize HTML-encoded CDATA markers to raw - .replace(/<!\[CDATA\[/gi, "") - // Then strip raw markers - .replace(//gi, "") - ) -} - -/** Build a unified diff for a brand new file (all content lines are additions). - * Trailing newline is ignored for line counting and emission. - */ -function convertNewFileToUnifiedDiff(content: string, filePath?: string): string { - const fileName = filePath || "file" - // Normalize EOLs to keep counts consistent - const normalized = content.replace(/\r\n/g, "\n") - const parts = normalized.split("\n") - // Drop trailing empty item produced by a final newline so we count only real content lines - const contentLines = parts[parts.length - 1] === "" ? parts.slice(0, -1) : parts - - const count = contentLines.length - let diff = `--- /dev/null\n` - diff += `+++ ${fileName}\n` - diff += `@@ -0,0 +${count ? 1 : 0},${count} @@\n` - - for (const line of contentLines) { - diff += `+${line}\n` - } - - return diff -} From 02e87798aec5b10fa0b681dd612e23007aaa4239 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Wed, 5 Nov 2025 20:27:36 -0700 Subject: [PATCH 20/22] change display to match checkpoint view diff before and after lines. --- src/core/prompts/responses.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index 2f3ea87d4c6..21703684b8b 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") From 1ff1032c090fc95f03452d4730b21a1d70768971 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Wed, 5 Nov 2025 21:03:30 -0700 Subject: [PATCH 21/22] Cleanup and formatting --- webview-ui/src/components/common/DiffView.tsx | 250 ++---------------- webview-ui/src/index.css | 25 ++ webview-ui/src/utils/parseUnifiedDiff.ts | 96 +++++++ 3 files changed, 147 insertions(+), 224 deletions(-) create mode 100644 webview-ui/src/utils/parseUnifiedDiff.ts diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index 1473d5d4461..d4cf326f5b6 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -1,5 +1,5 @@ import { memo, useMemo, useEffect, useState } from "react" -import { parsePatch } from "diff" +import { parseUnifiedDiff } from "@src/utils/parseUnifiedDiff" import { toJsxRuntime } from "hast-util-to-jsx-runtime" import { Fragment, jsx, jsxs } from "react/jsx-runtime" import { getHighlighter, normalizeLanguage } from "@src/utils/highlighter" @@ -10,14 +10,6 @@ interface DiffViewProps { filePath?: string } -interface DiffLine { - oldLineNum: number | null - newLineNum: number | null - type: "context" | "addition" | "deletion" | "gap" - content: string - hiddenCount?: number -} - /** * DiffView component renders unified diffs with side-by-side line numbers * matching VSCode's diff editor style @@ -85,193 +77,46 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { } } - // Parse diff and extract line information - const diffLines = useMemo(() => { - if (!source) return [] - - try { - const patches = parsePatch(source) - if (!patches || patches.length === 0) return [] - - const lines: DiffLine[] = [] - const patch = filePath - ? (patches.find((p) => - [p.newFileName, p.oldFileName].some( - (n) => typeof n === "string" && (n === filePath || n?.endsWith("/" + filePath)), - ), - ) ?? patches[0]) - : patches[0] - - if (!patch) return [] - - let prevHunk: any = null - for (const hunk of patch.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 line of hunk.lines) { - const firstChar = line[0] - const content = line.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 (error) { - console.error("[DiffView] Failed to parse diff:", error) - return [] - } - }, [source, filePath]) + // Parse diff server-provided unified patch into renderable lines + const diffLines = useMemo(() => parseUnifiedDiff(source, filePath), [source, filePath]) return ( -
-
- +
+
+
{diffLines.map((line, idx) => { // Render compact separator between hunks if (line.type === "gap") { - // Match the header/container background tone - const gapBg = "var(--vscode-editor-background)" + // Compact separator between hunks return ( - ) } - // Use VSCode's built-in diff editor color variables with 50% opacity - const gutterBg = + // Use VSCode's built-in diff editor color variables as classes for gutters + const gutterBgClass = line.type === "addition" - ? "var(--vscode-diffEditor-insertedTextBackground)" + ? "bg-[var(--vscode-diffEditor-insertedTextBackground)]" : line.type === "deletion" - ? "var(--vscode-diffEditor-removedTextBackground)" - : "var(--vscode-editorGroup-border)" + ? "bg-[var(--vscode-diffEditor-removedTextBackground)]" + : "bg-[var(--vscode-editorGroup-border)]" - const contentBgStyles = + const contentBgClass = line.type === "addition" - ? { - backgroundColor: - "color-mix(in srgb, var(--vscode-diffEditor-insertedTextBackground) 70%, transparent)", - } + ? "diff-content-inserted" : line.type === "deletion" - ? { - backgroundColor: - "color-mix(in srgb, var(--vscode-diffEditor-removedTextBackground) 70%, transparent)", - } - : { - backgroundColor: - "color-mix(in srgb, var(--vscode-editorGroup-border) 100%, transparent)", - } + ? "diff-content-removed" + : "diff-content-context" const sign = line.type === "addition" ? "+" : line.type === "deletion" ? "-" : "" @@ -279,67 +124,24 @@ const DiffView = memo(({ source, filePath }: DiffViewProps) => { {/* Old line number */} {/* New line number */} {/* Narrow colored gutter */} - {/* Code content (no +/- prefix here) */} diff --git a/webview-ui/src/index.css b/webview-ui/src/index.css index 6f23892ced3..6355ded21be 100644 --- a/webview-ui/src/index.css +++ b/webview-ui/src/index.css @@ -490,3 +490,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/parseUnifiedDiff.ts b/webview-ui/src/utils/parseUnifiedDiff.ts new file mode 100644 index 00000000000..bed84c4ca9f --- /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 [] + } +} From 3adfd464d69472e00106d6cffa335fe6eb1b161e Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Thu, 6 Nov 2025 13:34:52 -0500 Subject: [PATCH 22/22] webview-ui: per-hunk diff highlighting (no cache); replace inline counter colors with VS Code theme classes; use typed diffStats in ChatRow --- webview-ui/src/components/chat/ChatRow.tsx | 12 +- .../src/components/common/CodeAccordian.tsx | 8 +- webview-ui/src/components/common/DiffView.tsx | 306 +++++++++++------- webview-ui/src/index.css | 1 + .../src/utils/__tests__/highlightDiff.spec.ts | 223 +++++++++++++ webview-ui/src/utils/highlightDiff.ts | 131 ++++++++ 6 files changed, 561 insertions(+), 120 deletions(-) create mode 100644 webview-ui/src/utils/__tests__/highlightDiff.spec.ts create mode 100644 webview-ui/src/utils/highlightDiff.ts diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index be51501a4a7..23df05cc626 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -338,7 +338,7 @@ export const ChatRowContent = ({ // Unified diff content (provided by backend when relevant) const unifiedDiff = useMemo(() => { if (!tool) return undefined - return ((tool as any).content ?? (tool as any).diff) as string | undefined + return (tool.content ?? tool.diff) as string | undefined }, [tool]) const followUpData = useMemo(() => { @@ -402,7 +402,7 @@ export const ChatRowContent = ({ isLoading={message.partial} isExpanded={isExpanded} onToggleExpand={handleToggleExpand} - diffStats={(tool as any).diffStats ?? undefined} + diffStats={tool.diffStats} /> @@ -440,7 +440,7 @@ export const ChatRowContent = ({ isLoading={message.partial} isExpanded={isExpanded} onToggleExpand={handleToggleExpand} - diffStats={(tool as any).diffStats ?? undefined} + diffStats={tool.diffStats} /> @@ -474,7 +474,7 @@ export const ChatRowContent = ({ isLoading={message.partial} isExpanded={isExpanded} onToggleExpand={handleToggleExpand} - diffStats={(tool as any).diffStats ?? undefined} + diffStats={tool.diffStats} /> @@ -506,7 +506,7 @@ export const ChatRowContent = ({ return ( { if (typeof vscode !== "undefined" && vscode?.postMessage) { vscode.postMessage({ type: "updateTodoList", payload: { todos: updatedTodos } }) @@ -543,7 +543,7 @@ export const ChatRowContent = ({ isExpanded={isExpanded} onToggleExpand={handleToggleExpand} onJumpToFile={() => vscode.postMessage({ type: "openFile", text: "./" + tool.path })} - diffStats={(tool as any).diffStats ?? undefined} + diffStats={tool.diffStats} /> diff --git a/webview-ui/src/components/common/CodeAccordian.tsx b/webview-ui/src/components/common/CodeAccordian.tsx index ebc4dd09c30..0d15bdb7dbc 100644 --- a/webview-ui/src/components/common/CodeAccordian.tsx +++ b/webview-ui/src/components/common/CodeAccordian.tsx @@ -82,12 +82,8 @@ const CodeAccordian = ({ {/* Prefer diff stats over generic progress indicator if available */} {hasValidStats ? (
- - +{derivedStats!.added} - - - -{derivedStats!.removed} - + +{derivedStats!.added} + -{derivedStats!.removed}
) : ( progressStatus && diff --git a/webview-ui/src/components/common/DiffView.tsx b/webview-ui/src/components/common/DiffView.tsx index d4cf326f5b6..dc1c3c40c22 100644 --- a/webview-ui/src/components/common/DiffView.tsx +++ b/webview-ui/src/components/common/DiffView.tsx @@ -1,152 +1,242 @@ import { memo, useMemo, useEffect, useState } from "react" -import { parseUnifiedDiff } from "@src/utils/parseUnifiedDiff" -import { toJsxRuntime } from "hast-util-to-jsx-runtime" -import { Fragment, jsx, jsxs } from "react/jsx-runtime" -import { getHighlighter, normalizeLanguage } from "@src/utils/highlighter" +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 and prepare highlighter + // Determine language from file path const normalizedLang = useMemo(() => normalizeLanguage(getLanguageFromPath(filePath || "") || "txt"), [filePath]) - const [highlighter, setHighlighter] = useState(null) + 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) }, []) - useEffect(() => { - let mounted = true - getHighlighter(normalizedLang) - .then((h) => { - if (mounted) setHighlighter(h) - }) - .catch(() => { - // fall back to plain text if highlighting fails - }) - return () => { - mounted = false - } - }, [normalizedLang]) - // 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]) - const renderHighlighted = (code: string): React.ReactNode => { - if (!highlighter || !shouldHighlight) return code - try { - const hast: any = highlighter.codeToHast(code, { - lang: normalizedLang, - theme: isLightTheme ? "github-light" : "github-dark", - transformers: [ - { - pre(node: any) { - node.properties.style = "padding:0;margin:0;background:none;" - return node - }, - code(node: any) { - node.properties.class = `hljs language-${normalizedLang}` - return node - }, - }, - ], + // 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) - // Extract just the children to render inline inside our table cell - const codeEl = hast?.children?.[0]?.children?.[0] - const inlineRoot = - codeEl && codeEl.children - ? { type: "element", tagName: "span", properties: {}, children: codeEl.children } - : { type: "element", tagName: "span", properties: {}, children: hast.children || [] } + // Effect to handle async highlighting + useEffect(() => { + if (!shouldHighlight) { + setProcessedHunks(hunks) + return + } + + const processHunks = async () => { + const processed: Hunk[] = [] - return toJsxRuntime(inlineRoot as any, { Fragment, jsx, jsxs }) - } catch { - return code + 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) } - } - // Parse diff server-provided unified patch into renderable lines - const diffLines = useMemo(() => parseUnifiedDiff(source, filePath), [source, filePath]) + 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 (
- - + + + {/* +/- column (empty for gap) */} - - + + {`${line.hiddenCount ?? 0} hidden lines`}
+ className={`w-[45px] text-right pr-1 pl-1 select-none align-top whitespace-nowrap ${gutterBgClass}`}> {line.oldLineNum || ""} + className={`w-[45px] text-right pr-1 select-none align-top whitespace-nowrap ${gutterBgClass}`}> {line.newLineNum || ""} + {/* +/- fixed column to prevent wrapping into it */} + className={`w-[16px] text-center select-none whitespace-nowrap px-1 ${gutterBgClass}`}> {sign} + className={`pl-1 pr-3 whitespace-pre-wrap break-words w-full ${contentBgClass}`}> {renderHighlighted(line.content)}
- {diffLines.map((line, idx) => { - // Render compact separator between hunks - if (line.type === "gap") { - // Compact separator between hunks + {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) */} + ) - } - - // 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`} +
- - - {/* +/- 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)}
- {line.oldLineNum || ""} - - {line.newLineNum || ""} - - {/* +/- fixed column to prevent wrapping into it */} - - {sign} - - {renderHighlighted(line.content)} -
diff --git a/webview-ui/src/index.css b/webview-ui/src/index.css index 6355ded21be..eacc86f371f 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); 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 00000000000..46aa2231efe --- /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 00000000000..a59745a8cbb --- /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 || ""), + } + } +}