Skip to content

Commit a119ca4

Browse files
author
Eric Wheeler
committed
refactor: Add pushToolWriteResult method to DiffViewProvider
Previously, each tool file contained duplicate code for formatting file write responses and conditionally sending user_feedback_diff messages. This led to inconsistent implementations and made changes difficult to maintain. This refactoring centralizes the response formatting and messaging logic in the DiffViewProvider class, which now: - Stores results from saveChanges() in class properties - Only sends user_feedback_diff when user edits exist - Configures XMLBuilder with no indentation for cleaner output Tool files now make a single method call instead of duplicating logic. Fixes: #3647 Signed-off-by: Eric Wheeler <[email protected]>
1 parent d7eec3a commit a119ca4

File tree

5 files changed

+111
-109
lines changed

5 files changed

+111
-109
lines changed

‎src/core/tools/applyDiffTool.ts‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { ToolUse, RemoveClosingTag } from "../../shared/tools"
88
import { formatResponse } from "../prompts/responses"
99
import { AskApproval, HandleError, PushToolResult } from "../../shared/tools"
1010
import { fileExistsAtPath } from "../../utils/fs"
11-
import { addLineNumbers } from "../../integrations/misc/extract-text"
1211
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1312
import { telemetryService } from "../../services/telemetry/TelemetryService"
1413
import { unescapeHtmlEntities } from "../../utils/text-normalization"
@@ -163,7 +162,8 @@ export async function applyDiffTool(
163162
return
164163
}
165164

166-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
165+
// Call saveChanges to update the DiffViewProvider properties
166+
await cline.diffViewProvider.saveChanges()
167167

168168
// Track file edit operation
169169
if (relPath) {
@@ -178,33 +178,13 @@ export async function applyDiffTool(
178178
partFailHint = `But unable to apply all diff parts to file: ${absolutePath}, silently use <read_file> tool to check newest file version and re-apply diffs\n`
179179
}
180180

181-
if (userEdits) {
182-
await cline.say(
183-
"user_feedback_diff",
184-
JSON.stringify({
185-
tool: fileExists ? "editedExistingFile" : "newFileCreated",
186-
path: getReadablePath(cline.cwd, relPath),
187-
diff: userEdits,
188-
} satisfies ClineSayTool),
189-
)
190-
191-
pushToolResult(
192-
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
193-
partFailHint +
194-
`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
195-
`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
196-
finalContent || "",
197-
)}\n</final_file_content>\n\n` +
198-
`Please note:\n` +
199-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
200-
`2. Proceed with the task using this updated file content as the new baseline.\n` +
201-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
202-
`${newProblemsMessage}`,
203-
)
181+
// Get the formatted response message
182+
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
183+
184+
if (partFailHint) {
185+
pushToolResult(partFailHint + message)
204186
} else {
205-
pushToolResult(
206-
`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` + partFailHint,
207-
)
187+
pushToolResult(message)
208188
}
209189

210190
await cline.diffViewProvider.reset()

‎src/core/tools/insertContentTool.ts‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ export async function insertContentTool(
128128
return
129129
}
130130

131-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
131+
// Call saveChanges to update the DiffViewProvider properties
132+
await cline.diffViewProvider.saveChanges()
132133

133134
// Track file edit operation
134135
if (relPath) {
@@ -137,34 +138,14 @@ export async function insertContentTool(
137138

138139
cline.didEditFile = true
139140

140-
if (!userEdits) {
141-
pushToolResult(
142-
`The content was successfully inserted in ${relPath.toPosix()} at line ${lineNumber}.${newProblemsMessage}`,
143-
)
144-
await cline.diffViewProvider.reset()
145-
return
146-
}
147-
148-
await cline.say(
149-
"user_feedback_diff",
150-
JSON.stringify({
151-
tool: "insertContent",
152-
path: getReadablePath(cline.cwd, relPath),
153-
diff: userEdits,
154-
lineNumber: lineNumber,
155-
} satisfies ClineSayTool),
141+
// Get the formatted response message
142+
const message = await cline.diffViewProvider.pushToolWriteResult(
143+
cline,
144+
cline.cwd,
145+
false, // Always false for insert_content
156146
)
157147

158-
pushToolResult(
159-
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
160-
`The updated content has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file:\n\n` +
161-
`<final_file_content path="${relPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n` +
162-
`Please note:\n` +
163-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
164-
`2. Proceed with the task using this updated file content as the new baseline.\n` +
165-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
166-
`${newProblemsMessage}`,
167-
)
148+
pushToolResult(message)
168149

169150
await cline.diffViewProvider.reset()
170151
} catch (error) {

‎src/core/tools/searchAndReplaceTool.ts‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ export async function searchAndReplaceTool(
211211
return
212212
}
213213

214-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
214+
// Call saveChanges to update the DiffViewProvider properties
215+
await cline.diffViewProvider.saveChanges()
215216

216217
// Track file edit operation
217218
if (relPath) {
@@ -220,34 +221,14 @@ export async function searchAndReplaceTool(
220221

221222
cline.didEditFile = true
222223

223-
if (!userEdits) {
224-
pushToolResult(`The content was successfully replaced in ${relPath}.${newProblemsMessage}`)
225-
await cline.diffViewProvider.reset()
226-
return
227-
}
228-
229-
await cline.say(
230-
"user_feedback_diff",
231-
JSON.stringify({
232-
tool: "appliedDiff",
233-
path: getReadablePath(cline.cwd, relPath),
234-
diff: userEdits,
235-
} satisfies ClineSayTool),
224+
// Get the formatted response message
225+
const message = await cline.diffViewProvider.pushToolWriteResult(
226+
cline,
227+
cline.cwd,
228+
false, // Always false for search_and_replace
236229
)
237230

238-
// Format and send response with user's updates
239-
const resultMessage = [
240-
`The user made the following updates to your content:\n\n${userEdits}\n\n`,
241-
`The updated content has been successfully saved to ${validRelPath.toPosix()}. Here is the full, updated content of the file:\n\n`,
242-
`<final_file_content path="${validRelPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n`,
243-
`Please note:\n`,
244-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n`,
245-
`2. Proceed with the task using the updated file content as the new baseline.\n`,
246-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.`,
247-
newProblemsMessage,
248-
].join("")
249-
250-
pushToolResult(resultMessage)
231+
pushToolResult(message)
251232

252233
// Record successful tool usage and cleanup
253234
cline.recordToolUsage("search_and_replace")

‎src/core/tools/writeToFileTool.ts‎

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { formatResponse } from "../prompts/responses"
88
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
99
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1010
import { fileExistsAtPath } from "../../utils/fs"
11-
import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
11+
import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
1212
import { getReadablePath } from "../../utils/path"
1313
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
1414
import { detectCodeOmission } from "../../integrations/editor/detect-omission"
@@ -208,7 +208,8 @@ export async function writeToFileTool(
208208
return
209209
}
210210

211-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
211+
// Call saveChanges to update the DiffViewProvider properties
212+
await cline.diffViewProvider.saveChanges()
212213

213214
// Track file edit operation
214215
if (relPath) {
@@ -217,31 +218,10 @@ export async function writeToFileTool(
217218

218219
cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request
219220

220-
if (userEdits) {
221-
await cline.say(
222-
"user_feedback_diff",
223-
JSON.stringify({
224-
tool: fileExists ? "editedExistingFile" : "newFileCreated",
225-
path: getReadablePath(cline.cwd, relPath),
226-
diff: userEdits,
227-
} satisfies ClineSayTool),
228-
)
221+
// Get the formatted response message
222+
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
229223

230-
pushToolResult(
231-
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
232-
`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
233-
`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
234-
finalContent || "",
235-
)}\n</final_file_content>\n\n` +
236-
`Please note:\n` +
237-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
238-
`2. Proceed with the task using this updated file content as the new baseline.\n` +
239-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
240-
`${newProblemsMessage}`,
241-
)
242-
} else {
243-
pushToolResult(`The content was successfully saved to ${relPath.toPosix()}.${newProblemsMessage}`)
244-
}
224+
pushToolResult(message)
245225

246226
await cline.diffViewProvider.reset()
247227

‎src/integrations/editor/DiffViewProvider.ts‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,24 @@ import * as path from "path"
33
import * as fs from "fs/promises"
44
import * as diff from "diff"
55
import stripBom from "strip-bom"
6+
import { XMLBuilder } from "fast-xml-parser"
67

78
import { createDirectoriesForFile } from "../../utils/fs"
8-
import { arePathsEqual } from "../../utils/path"
9+
import { arePathsEqual, getReadablePath } from "../../utils/path"
910
import { formatResponse } from "../../core/prompts/responses"
1011
import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
12+
import { ClineSayTool } from "../../shared/ExtensionMessage"
13+
import { Task } from "../../core/task/Task"
1114

1215
import { DecorationController } from "./DecorationController"
1316

1417
export const DIFF_VIEW_URI_SCHEME = "cline-diff"
1518

1619
// TODO: https://github.com/cline/cline/pull/3354
1720
export class DiffViewProvider {
21+
// Properties to store the results of saveChanges
22+
newProblemsMessage?: string
23+
userEdits?: string
1824
editType?: "create" | "modify"
1925
isEditing = false
2026
originalContent: string | undefined
@@ -235,13 +241,87 @@ export class DiffViewProvider {
235241
normalizedEditedContent,
236242
)
237243

244+
// Store the results as class properties for formatFileWriteResponse to use
245+
this.newProblemsMessage = newProblemsMessage
246+
this.userEdits = userEdits
247+
238248
return { newProblemsMessage, userEdits, finalContent: normalizedEditedContent }
239249
} else {
240250
// No changes to Roo's edits.
251+
// Store the results as class properties for formatFileWriteResponse to use
252+
this.newProblemsMessage = newProblemsMessage
253+
this.userEdits = undefined
254+
241255
return { newProblemsMessage, userEdits: undefined, finalContent: normalizedEditedContent }
242256
}
243257
}
244258

259+
/**
260+
* Formats a standardized XML response for file write operations
261+
*
262+
* @param cwd Current working directory for path resolution
263+
* @param isNewFile Whether this is a new file or an existing file being modified
264+
* @returns Formatted message and say object for UI feedback
265+
*/
266+
async pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise<string> {
267+
if (!this.relPath) {
268+
throw new Error("No file path available in DiffViewProvider")
269+
}
270+
271+
// Only send user_feedback_diff if userEdits exists
272+
if (this.userEdits) {
273+
// Create say object for UI feedback
274+
const say: ClineSayTool = {
275+
tool: isNewFile ? "newFileCreated" : "editedExistingFile",
276+
path: getReadablePath(cwd, this.relPath),
277+
diff: this.userEdits,
278+
}
279+
280+
// Send the user feedback
281+
await task.say("user_feedback_diff", JSON.stringify(say))
282+
}
283+
284+
// Build XML response
285+
const xmlObj = {
286+
file_write_result: {
287+
path: this.relPath,
288+
operation: isNewFile ? "created" : "modified",
289+
user_edits: this.userEdits ? this.userEdits : undefined,
290+
problems: this.newProblemsMessage || undefined,
291+
notice: {
292+
i: [
293+
"You do not need to re-read the file, as you have seen all changes in the diff above.",
294+
"Proceed with the task using these changes as the new baseline.",
295+
"If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.",
296+
],
297+
},
298+
},
299+
}
300+
301+
const builder = new XMLBuilder({
302+
format: true,
303+
indentBy: "",
304+
suppressEmptyNode: true,
305+
processEntities: false,
306+
tagValueProcessor: (name, value) => {
307+
if (typeof value === "string") {
308+
// Only escape <, >, and & characters
309+
return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
310+
}
311+
return value
312+
},
313+
attributeValueProcessor: (name, value) => {
314+
if (typeof value === "string") {
315+
// Only escape <, >, and & characters
316+
return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
317+
}
318+
return value
319+
},
320+
})
321+
322+
return builder.build(xmlObj)
323+
}
324+
245325
async revertChanges(): Promise<void> {
246326
if (!this.relPath || !this.activeDiffEditor) {
247327
return

0 commit comments

Comments
 (0)