Skip to content

Commit b38c905

Browse files
daniel-lxsdaniel-lxs
committed
refactor: reduce code duplication in file editing tools
- Consolidated approval flow logic in searchAndReplaceTool, insertContentTool, and multiApplyDiffTool - Moved common code outside conditional blocks - Reduced code duplication while maintaining the same functionality - Makes the code more maintainable and easier to understand
1 parent 8ae4296 commit b38c905

File tree

3 files changed

+94
-132
lines changed

3 files changed

+94
-132
lines changed

‎src/core/tools/insertContentTool.ts‎

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -136,52 +136,42 @@ export async function insertContentTool(
136136
approvalContent = updatedContent
137137
}
138138

139-
if (isPreventFocusDisruptionEnabled) {
140-
// Direct file write without diff view
141-
const completeMessage = JSON.stringify({
142-
...sharedMessageProps,
143-
diff,
144-
content: approvalContent,
145-
lineNumber: lineNumber,
146-
isProtected: isWriteProtected,
147-
} satisfies ClineSayTool)
148-
149-
const didApprove = await cline
150-
.ask("tool", completeMessage, isWriteProtected)
151-
.then((response) => response.response === "yesButtonClicked")
152-
153-
if (!didApprove) {
154-
pushToolResult("Changes were rejected by the user.")
155-
return
156-
}
157-
158-
// Save directly without showing diff view or opening the file
159-
await cline.diffViewProvider.saveDirectly(relPath, updatedContent, false, diagnosticsEnabled, writeDelayMs)
160-
} else {
161-
// Original behavior with diff view
162-
// Show diff view BEFORE asking for approval
139+
// Prepare the approval message (same for both flows)
140+
const completeMessage = JSON.stringify({
141+
...sharedMessageProps,
142+
diff,
143+
content: approvalContent,
144+
lineNumber: lineNumber,
145+
isProtected: isWriteProtected,
146+
} satisfies ClineSayTool)
147+
148+
// Show diff view if focus disruption prevention is disabled
149+
if (!isPreventFocusDisruptionEnabled) {
163150
await cline.diffViewProvider.open(relPath)
164151
await cline.diffViewProvider.update(updatedContent, true)
165152
cline.diffViewProvider.scrollToFirstDiff()
153+
}
166154

167-
const completeMessage = JSON.stringify({
168-
...sharedMessageProps,
169-
diff,
170-
content: approvalContent,
171-
lineNumber: lineNumber,
172-
isProtected: isWriteProtected,
173-
} satisfies ClineSayTool)
174-
175-
const didApprove = await cline
176-
.ask("tool", completeMessage, isWriteProtected)
177-
.then((response) => response.response === "yesButtonClicked")
155+
// Ask for approval (same for both flows)
156+
const didApprove = await cline
157+
.ask("tool", completeMessage, isWriteProtected)
158+
.then((response) => response.response === "yesButtonClicked")
178159

179-
if (!didApprove) {
160+
if (!didApprove) {
161+
// Revert changes if diff view was shown
162+
if (!isPreventFocusDisruptionEnabled) {
180163
await cline.diffViewProvider.revertChanges()
181-
pushToolResult("Changes were rejected by the user.")
182-
return
183164
}
165+
pushToolResult("Changes were rejected by the user.")
166+
await cline.diffViewProvider.reset()
167+
return
168+
}
184169

170+
// Save the changes
171+
if (isPreventFocusDisruptionEnabled) {
172+
// Direct file write without diff view or opening the file
173+
await cline.diffViewProvider.saveDirectly(relPath, updatedContent, false, diagnosticsEnabled, writeDelayMs)
174+
} else {
185175
// Call saveChanges to update the DiffViewProvider properties
186176
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
187177
}

‎src/core/tools/multiApplyDiffTool.ts‎

Lines changed: 42 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -528,38 +528,53 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
528528
// If single file, handle based on PREVENT_FOCUS_DISRUPTION setting
529529
let didApprove = true
530530
if (operationsToApprove.length === 1) {
531-
if (isPreventFocusDisruptionEnabled) {
532-
// Direct approval without showing diff view
533-
const diffContents = diffItems.map((item) => item.content).join("\n\n")
534-
const operationMessage = JSON.stringify({
535-
...sharedMessageProps,
536-
diff: diffContents,
537-
} satisfies ClineSayTool)
538-
539-
let toolProgressStatus
540-
541-
if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) {
542-
toolProgressStatus = cline.diffStrategy.getProgressStatus(
543-
{
544-
...block,
545-
params: { ...block.params, diff: diffContents },
546-
},
547-
{ success: true },
548-
)
549-
}
531+
// Prepare common data for single file operation
532+
const diffContents = diffItems.map((item) => item.content).join("\n\n")
533+
const operationMessage = JSON.stringify({
534+
...sharedMessageProps,
535+
diff: diffContents,
536+
} satisfies ClineSayTool)
537+
538+
let toolProgressStatus
539+
if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) {
540+
toolProgressStatus = cline.diffStrategy.getProgressStatus(
541+
{
542+
...block,
543+
params: { ...block.params, diff: diffContents },
544+
},
545+
{ success: true },
546+
)
547+
}
548+
549+
// Set up diff view
550+
cline.diffViewProvider.editType = "modify"
551+
552+
// Show diff view if focus disruption prevention is disabled
553+
if (!isPreventFocusDisruptionEnabled) {
554+
await cline.diffViewProvider.open(relPath)
555+
await cline.diffViewProvider.update(originalContent!, true)
556+
cline.diffViewProvider.scrollToFirstDiff()
557+
} else {
558+
// For direct save, we still need to set originalContent
559+
cline.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8")
560+
}
550561

551-
// Check if file is write-protected
552-
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
553-
didApprove = await askApproval("tool", operationMessage, toolProgressStatus, isWriteProtected)
562+
// Ask for approval (same for both flows)
563+
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
564+
didApprove = await askApproval("tool", operationMessage, toolProgressStatus, isWriteProtected)
554565

555-
if (!didApprove) {
556-
results.push(`Changes to ${relPath} were not approved by user`)
557-
continue
566+
if (!didApprove) {
567+
// Revert changes if diff view was shown
568+
if (!isPreventFocusDisruptionEnabled) {
569+
await cline.diffViewProvider.revertChanges()
558570
}
571+
results.push(`Changes to ${relPath} were not approved by user`)
572+
continue
573+
}
559574

575+
// Save the changes
576+
if (isPreventFocusDisruptionEnabled) {
560577
// Direct file write without diff view or opening the file
561-
cline.diffViewProvider.editType = "modify"
562-
cline.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8")
563578
await cline.diffViewProvider.saveDirectly(
564579
relPath,
565580
originalContent!,
@@ -568,40 +583,6 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
568583
writeDelayMs,
569584
)
570585
} else {
571-
// Show diff view BEFORE asking for approval
572-
cline.diffViewProvider.editType = "modify"
573-
await cline.diffViewProvider.open(relPath)
574-
await cline.diffViewProvider.update(originalContent!, true)
575-
cline.diffViewProvider.scrollToFirstDiff()
576-
577-
const diffContents = diffItems.map((item) => item.content).join("\n\n")
578-
const operationMessage = JSON.stringify({
579-
...sharedMessageProps,
580-
diff: diffContents,
581-
} satisfies ClineSayTool)
582-
583-
let toolProgressStatus
584-
585-
if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) {
586-
toolProgressStatus = cline.diffStrategy.getProgressStatus(
587-
{
588-
...block,
589-
params: { ...block.params, diff: diffContents },
590-
},
591-
{ success: true },
592-
)
593-
}
594-
595-
// Check if file is write-protected
596-
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
597-
didApprove = await askApproval("tool", operationMessage, toolProgressStatus, isWriteProtected)
598-
599-
if (!didApprove) {
600-
await cline.diffViewProvider.revertChanges()
601-
results.push(`Changes to ${relPath} were not approved by user`)
602-
continue
603-
}
604-
605586
// Call saveChanges to update the DiffViewProvider properties
606587
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
607588
}

‎src/core/tools/searchAndReplaceTool.ts‎

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -210,47 +210,38 @@ export async function searchAndReplaceTool(
210210
EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION,
211211
)
212212

213-
if (isPreventFocusDisruptionEnabled) {
214-
// Direct approval without showing diff view
215-
const completeMessage = JSON.stringify({
216-
...sharedMessageProps,
217-
diff,
218-
isProtected: isWriteProtected,
219-
} satisfies ClineSayTool)
220-
const didApprove = await cline
221-
.ask("tool", completeMessage, isWriteProtected)
222-
.then((response) => response.response === "yesButtonClicked")
223-
224-
if (!didApprove) {
225-
pushToolResult("Changes were rejected by the user.")
226-
await cline.diffViewProvider.reset()
227-
return
228-
}
229-
230-
// Direct file write without diff view or opening the file
231-
await cline.diffViewProvider.saveDirectly(validRelPath, newContent, false, diagnosticsEnabled, writeDelayMs)
232-
} else {
233-
// Show diff view BEFORE asking for approval
213+
const completeMessage = JSON.stringify({
214+
...sharedMessageProps,
215+
diff,
216+
isProtected: isWriteProtected,
217+
} satisfies ClineSayTool)
218+
219+
// Show diff view if focus disruption prevention is disabled
220+
if (!isPreventFocusDisruptionEnabled) {
234221
await cline.diffViewProvider.open(validRelPath)
235222
await cline.diffViewProvider.update(newContent, true)
236223
cline.diffViewProvider.scrollToFirstDiff()
224+
}
237225

238-
const completeMessage = JSON.stringify({
239-
...sharedMessageProps,
240-
diff,
241-
isProtected: isWriteProtected,
242-
} satisfies ClineSayTool)
243-
const didApprove = await cline
244-
.ask("tool", completeMessage, isWriteProtected)
245-
.then((response) => response.response === "yesButtonClicked")
226+
const didApprove = await cline
227+
.ask("tool", completeMessage, isWriteProtected)
228+
.then((response) => response.response === "yesButtonClicked")
246229

247-
if (!didApprove) {
230+
if (!didApprove) {
231+
// Revert changes if diff view was shown
232+
if (!isPreventFocusDisruptionEnabled) {
248233
await cline.diffViewProvider.revertChanges()
249-
pushToolResult("Changes were rejected by the user.")
250-
await cline.diffViewProvider.reset()
251-
return
252234
}
235+
pushToolResult("Changes were rejected by the user.")
236+
await cline.diffViewProvider.reset()
237+
return
238+
}
253239

240+
// Save the changes
241+
if (isPreventFocusDisruptionEnabled) {
242+
// Direct file write without diff view or opening the file
243+
await cline.diffViewProvider.saveDirectly(validRelPath, newContent, false, diagnosticsEnabled, writeDelayMs)
244+
} else {
254245
// Call saveChanges to update the DiffViewProvider properties
255246
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
256247
}

0 commit comments

Comments
 (0)