Skip to content

Commit 0fd7023

Browse files
feat(chat): Improve diff appearance in main chat view (#8932)
Co-authored-by: daniel-lxs <[email protected]>
1 parent d631aa6 commit 0fd7023

19 files changed

+1141
-57
lines changed

pnpm-lock.yaml

Lines changed: 7 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/diff/stats.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { parsePatch, createTwoFilesPatch } from "diff"
2+
3+
/**
4+
* Diff utilities for backend (extension) use.
5+
* Source of truth for diff normalization and stats.
6+
*/
7+
8+
export interface DiffStats {
9+
added: number
10+
removed: number
11+
}
12+
13+
/**
14+
* Remove non-semantic diff noise like "No newline at end of file"
15+
*/
16+
export function sanitizeUnifiedDiff(diff: string): string {
17+
if (!diff) return diff
18+
return diff.replace(/\r\n/g, "\n").replace(/(^|\n)[ \t]*(?:\\ )?No newline at end of file[ \t]*(?=\n|$)/gi, "$1")
19+
}
20+
21+
/**
22+
* Compute +/− counts from a unified diff (ignores headers/hunk lines)
23+
*/
24+
export function computeUnifiedDiffStats(diff?: string): DiffStats | null {
25+
if (!diff) return null
26+
27+
try {
28+
const patches = parsePatch(diff)
29+
if (!patches || patches.length === 0) return null
30+
31+
let added = 0
32+
let removed = 0
33+
34+
for (const p of patches) {
35+
for (const h of (p as any).hunks ?? []) {
36+
for (const l of h.lines ?? []) {
37+
const ch = (l as string)[0]
38+
if (ch === "+") added++
39+
else if (ch === "-") removed++
40+
}
41+
}
42+
}
43+
44+
if (added > 0 || removed > 0) return { added, removed }
45+
return { added: 0, removed: 0 }
46+
} catch {
47+
// If parsing fails for any reason, signal no stats
48+
return null
49+
}
50+
}
51+
52+
/**
53+
* Compute diff stats from any supported diff format (unified or search-replace)
54+
* Tries unified diff format first, then falls back to search-replace format
55+
*/
56+
export function computeDiffStats(diff?: string): DiffStats | null {
57+
if (!diff) return null
58+
return computeUnifiedDiffStats(diff)
59+
}
60+
61+
/**
62+
* Build a unified diff for a brand new file (all content lines are additions).
63+
* Trailing newline is ignored for line counting and emission.
64+
*/
65+
export function convertNewFileToUnifiedDiff(content: string, filePath?: string): string {
66+
const newFileName = filePath || "file"
67+
// Normalize EOLs; rely on library for unified patch formatting
68+
const normalized = (content || "").replace(/\r\n/g, "\n")
69+
// Old file is empty (/dev/null), new file has content; zero context to show all lines as additions
70+
return createTwoFilesPatch("/dev/null", newFileName, "", normalized, undefined, undefined, { context: 0 })
71+
}

src/core/prompts/responses.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,9 @@ Otherwise, if you have not completed the task and do not need additional informa
177177

178178
createPrettyPatch: (filename = "file", oldStr?: string, newStr?: string) => {
179179
// strings cannot be undefined or diff throws exception
180-
const patch = diff.createPatch(filename.toPosix(), oldStr || "", newStr || "")
180+
const patch = diff.createPatch(filename.toPosix(), oldStr || "", newStr || "", undefined, undefined, {
181+
context: 3,
182+
})
181183
const lines = patch.split("\n")
182184
const prettyPatchLines = lines.slice(4)
183185
return prettyPatchLines.join("\n")

src/core/tools/applyDiffTool.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { fileExistsAtPath } from "../../utils/fs"
1313
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1414
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1515
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
16+
import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
1617

1718
export async function applyDiffToolLegacy(
1819
cline: Task,
@@ -140,6 +141,11 @@ export async function applyDiffToolLegacy(
140141
cline.consecutiveMistakeCount = 0
141142
cline.consecutiveMistakeCountForApplyDiff.delete(relPath)
142143

144+
// Generate backend-unified diff for display in chat/webview
145+
const unifiedPatchRaw = formatResponse.createPrettyPatch(relPath, originalContent, diffResult.content)
146+
const unifiedPatch = sanitizeUnifiedDiff(unifiedPatchRaw)
147+
const diffStats = computeDiffStats(unifiedPatch) || undefined
148+
143149
// Check if preventFocusDisruption experiment is enabled
144150
const provider = cline.providerRef.deref()
145151
const state = await provider?.getState()
@@ -158,6 +164,8 @@ export async function applyDiffToolLegacy(
158164
const completeMessage = JSON.stringify({
159165
...sharedMessageProps,
160166
diff: diffContent,
167+
content: unifiedPatch,
168+
diffStats,
161169
isProtected: isWriteProtected,
162170
} satisfies ClineSayTool)
163171

@@ -194,6 +202,8 @@ export async function applyDiffToolLegacy(
194202
const completeMessage = JSON.stringify({
195203
...sharedMessageProps,
196204
diff: diffContent,
205+
content: unifiedPatch,
206+
diffStats,
197207
isProtected: isWriteProtected,
198208
} satisfies ClineSayTool)
199209

src/core/tools/insertContentTool.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { fileExistsAtPath } from "../../utils/fs"
1212
import { insertGroups } from "../diff/insert-groups"
1313
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
1414
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
15+
import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
1516

1617
export async function insertContentTool(
1718
cline: Task,
@@ -101,7 +102,7 @@ export async function insertContentTool(
101102
cline.diffViewProvider.originalContent = fileContent
102103
const lines = fileExists ? fileContent.split("\n") : []
103104

104-
const updatedContent = insertGroups(lines, [
105+
let updatedContent = insertGroups(lines, [
105106
{
106107
index: lineNumber - 1,
107108
elements: content.split("\n"),
@@ -118,31 +119,31 @@ export async function insertContentTool(
118119
EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION,
119120
)
120121

121-
// For consistency with writeToFileTool, handle new files differently
122-
let diff: string | undefined
123-
let approvalContent: string | undefined
124-
122+
// Build unified diff for display (normalize EOLs only for diff generation)
123+
let unified: string
125124
if (fileExists) {
126-
// For existing files, generate diff and check for changes
127-
diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent)
128-
if (!diff) {
125+
const oldForDiff = fileContent.replace(/\r\n/g, "\n")
126+
const newForDiff = updatedContent.replace(/\r\n/g, "\n")
127+
unified = formatResponse.createPrettyPatch(relPath, oldForDiff, newForDiff)
128+
if (!unified) {
129129
pushToolResult(`No changes needed for '${relPath}'`)
130130
return
131131
}
132-
approvalContent = undefined
133132
} else {
134-
// For new files, skip diff generation and provide full content
135-
diff = undefined
136-
approvalContent = updatedContent
133+
const newForDiff = updatedContent.replace(/\r\n/g, "\n")
134+
unified = convertNewFileToUnifiedDiff(newForDiff, relPath)
137135
}
136+
unified = sanitizeUnifiedDiff(unified)
137+
const diffStats = computeDiffStats(unified) || undefined
138138

139139
// Prepare the approval message (same for both flows)
140140
const completeMessage = JSON.stringify({
141141
...sharedMessageProps,
142-
diff,
143-
content: approvalContent,
142+
// Send unified diff as content for render-only webview
143+
content: unified,
144144
lineNumber: lineNumber,
145145
isProtected: isWriteProtected,
146+
diffStats,
146147
} satisfies ClineSayTool)
147148

148149
// Show diff view if focus disruption prevention is disabled

src/core/tools/multiApplyDiffTool.ts

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { unescapeHtmlEntities } from "../../utils/text-normalization"
1515
import { parseXmlForDiff } from "../../utils/xml"
1616
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
1717
import { applyDiffToolLegacy } from "./applyDiffTool"
18+
import { computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
1819

1920
interface DiffOperation {
2021
path: string
@@ -282,31 +283,70 @@ Original error: ${errorMessage}`
282283
(opResult) => cline.rooProtectedController?.isWriteProtected(opResult.path) || false,
283284
)
284285

285-
// Prepare batch diff data
286-
const batchDiffs = operationsToApprove.map((opResult) => {
286+
// Stream batch diffs progressively for better UX
287+
const batchDiffs: Array<{
288+
path: string
289+
changeCount: number
290+
key: string
291+
content: string
292+
diffStats?: { added: number; removed: number }
293+
diffs?: Array<{ content: string; startLine?: number }>
294+
}> = []
295+
296+
for (const opResult of operationsToApprove) {
287297
const readablePath = getReadablePath(cline.cwd, opResult.path)
288298
const changeCount = opResult.diffItems?.length || 0
289299
const changeText = changeCount === 1 ? "1 change" : `${changeCount} changes`
290300

291-
return {
301+
let unified = ""
302+
try {
303+
const original = await fs.readFile(opResult.absolutePath!, "utf-8")
304+
const processed = !cline.api.getModel().id.includes("claude")
305+
? (opResult.diffItems || []).map((item) => ({
306+
...item,
307+
content: item.content ? unescapeHtmlEntities(item.content) : item.content,
308+
}))
309+
: opResult.diffItems || []
310+
311+
const applyRes =
312+
(await cline.diffStrategy?.applyDiff(original, processed)) ?? ({ success: false } as any)
313+
const newContent = applyRes.success && applyRes.content ? applyRes.content : original
314+
unified = formatResponse.createPrettyPatch(opResult.path, original, newContent)
315+
} catch {
316+
unified = ""
317+
}
318+
319+
const unifiedSanitized = sanitizeUnifiedDiff(unified)
320+
const stats = computeDiffStats(unifiedSanitized) || undefined
321+
batchDiffs.push({
292322
path: readablePath,
293323
changeCount,
294324
key: `${readablePath} (${changeText})`,
295-
content: opResult.path, // Full relative path
325+
content: unifiedSanitized,
326+
diffStats: stats,
296327
diffs: opResult.diffItems?.map((item) => ({
297328
content: item.content,
298329
startLine: item.startLine,
299330
})),
300-
}
301-
})
331+
})
332+
333+
// Send a partial update after each file preview is ready
334+
const partialMessage = JSON.stringify({
335+
tool: "appliedDiff",
336+
batchDiffs,
337+
isProtected: hasProtectedFiles,
338+
} satisfies ClineSayTool)
339+
await cline.ask("tool", partialMessage, true).catch(() => {})
340+
}
302341

342+
// Final approval message (non-partial)
303343
const completeMessage = JSON.stringify({
304344
tool: "appliedDiff",
305345
batchDiffs,
306346
isProtected: hasProtectedFiles,
307347
} satisfies ClineSayTool)
308348

309-
const { response, text, images } = await cline.ask("tool", completeMessage, hasProtectedFiles)
349+
const { response, text, images } = await cline.ask("tool", completeMessage, false)
310350

311351
// Process batch response
312352
if (response === "yesButtonClicked") {
@@ -418,6 +458,7 @@ Original error: ${errorMessage}`
418458

419459
try {
420460
let originalContent: string | null = await fs.readFile(absolutePath, "utf-8")
461+
let beforeContent: string | null = originalContent
421462
let successCount = 0
422463
let formattedError = ""
423464

@@ -540,9 +581,13 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
540581
if (operationsToApprove.length === 1) {
541582
// Prepare common data for single file operation
542583
const diffContents = diffItems.map((item) => item.content).join("\n\n")
584+
const unifiedPatchRaw = formatResponse.createPrettyPatch(relPath, beforeContent!, originalContent!)
585+
const unifiedPatch = sanitizeUnifiedDiff(unifiedPatchRaw)
543586
const operationMessage = JSON.stringify({
544587
...sharedMessageProps,
545588
diff: diffContents,
589+
content: unifiedPatch,
590+
diffStats: computeDiffStats(unifiedPatch) || undefined,
546591
} satisfies ClineSayTool)
547592

548593
let toolProgressStatus

src/core/tools/writeToFileTool.ts

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { detectCodeOmission } from "../../integrations/editor/detect-omission"
1616
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1717
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
1818
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
19+
import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats"
1920

2021
export async function writeToFileTool(
2122
cline: Task,
@@ -173,6 +174,15 @@ export async function writeToFileTool(
173174

174175
if (isPreventFocusDisruptionEnabled) {
175176
// Direct file write without diff view
177+
// Set up diffViewProvider properties needed for diff generation and saveDirectly
178+
cline.diffViewProvider.editType = fileExists ? "modify" : "create"
179+
if (fileExists) {
180+
const absolutePath = path.resolve(cline.cwd, relPath)
181+
cline.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8")
182+
} else {
183+
cline.diffViewProvider.originalContent = ""
184+
}
185+
176186
// Check for code omissions before proceeding
177187
if (detectCodeOmission(cline.diffViewProvider.originalContent || "", newContent, predictedLineCount)) {
178188
if (cline.diffStrategy) {
@@ -202,9 +212,15 @@ export async function writeToFileTool(
202212
}
203213
}
204214

215+
// Build unified diff for both existing and new files
216+
let unified = fileExists
217+
? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent)
218+
: convertNewFileToUnifiedDiff(newContent, relPath)
219+
unified = sanitizeUnifiedDiff(unified)
205220
const completeMessage = JSON.stringify({
206221
...sharedMessageProps,
207-
content: newContent,
222+
content: unified,
223+
diffStats: computeDiffStats(unified) || undefined,
208224
} satisfies ClineSayTool)
209225

210226
const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected)
@@ -213,15 +229,6 @@ export async function writeToFileTool(
213229
return
214230
}
215231

216-
// Set up diffViewProvider properties needed for saveDirectly
217-
cline.diffViewProvider.editType = fileExists ? "modify" : "create"
218-
if (fileExists) {
219-
const absolutePath = path.resolve(cline.cwd, relPath)
220-
cline.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8")
221-
} else {
222-
cline.diffViewProvider.originalContent = ""
223-
}
224-
225232
// Save directly without showing diff view or opening the file
226233
await cline.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs)
227234
} else {
@@ -275,12 +282,15 @@ export async function writeToFileTool(
275282
}
276283
}
277284

285+
// Build unified diff for both existing and new files
286+
let unified = fileExists
287+
? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent)
288+
: convertNewFileToUnifiedDiff(newContent, relPath)
289+
unified = sanitizeUnifiedDiff(unified)
278290
const completeMessage = JSON.stringify({
279291
...sharedMessageProps,
280-
content: fileExists ? undefined : newContent,
281-
diff: fileExists
282-
? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent)
283-
: undefined,
292+
content: unified,
293+
diffStats: computeDiffStats(unified) || undefined,
284294
} satisfies ClineSayTool)
285295

286296
const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected)

0 commit comments

Comments
 (0)