diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index a30550dce1..63b183507b 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -107,6 +107,7 @@ export const globalSettingsSchema = z.object({ rateLimitSeconds: z.number().optional(), diffEnabled: z.boolean().optional(), + fileBasedEditing: z.boolean().optional(), fuzzyMatchThreshold: z.number().optional(), experiments: experimentsSchema.optional(), diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 53b8ef5b87..ad56a322f9 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -54,6 +54,8 @@ import { RepoPerTaskCheckpointService } from "../../services/checkpoints" // integrations import { DiffViewProvider } from "../../integrations/editor/DiffViewProvider" +import { FileWriter } from "../../integrations/editor/FileWriter" +import { IEditingProvider } from "../../integrations/editor/IEditingProvider" import { findToolName, formatContentBlockToMarkdown } from "../../integrations/misc/export-markdown" import { RooTerminalProcess } from "../../integrations/terminal/types" import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" @@ -172,7 +174,7 @@ export class Task extends EventEmitter { browserSession: BrowserSession // Editing - diffViewProvider: DiffViewProvider + editingProvider: IEditingProvider diffStrategy?: DiffStrategy diffEnabled: boolean = false fuzzyMatchThreshold: number @@ -260,7 +262,28 @@ export class Task extends EventEmitter { this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT this.providerRef = new WeakRef(provider) this.globalStoragePath = provider.context.globalStorageUri.fsPath - this.diffViewProvider = new DiffViewProvider(this.cwd) + + // Default to DiffViewProvider initially + this.editingProvider = new DiffViewProvider(this.cwd) + + // Initialize editing provider based on settings + if (provider.getState) { + provider + .getState() + .then((state) => { + const fileBasedEditing = state?.fileBasedEditing ?? false + if (fileBasedEditing) { + this.editingProvider = new FileWriter(this.cwd) + } else { + this.editingProvider = new DiffViewProvider(this.cwd) + } + }) + .catch((error) => { + console.error("Failed to get provider state for editing provider initialization:", error) + // Keep the default DiffViewProvider + }) + } + this.enableCheckpoints = enableCheckpoints this.rootTask = rootTask @@ -1066,8 +1089,8 @@ export class Task extends EventEmitter { try { // If we're not streaming then `abortStream` won't be called - if (this.isStreaming && this.diffViewProvider.isEditing) { - this.diffViewProvider.revertChanges().catch(console.error) + if (this.isStreaming && this.editingProvider.isEditing) { + this.editingProvider.revertChanges().catch(console.error) } } catch (error) { console.error("Error reverting diff changes:", error) @@ -1296,8 +1319,8 @@ export class Task extends EventEmitter { } const abortStream = async (cancelReason: ClineApiReqCancelReason, streamingFailedMessage?: string) => { - if (this.diffViewProvider.isEditing) { - await this.diffViewProvider.revertChanges() // closes diff view + if (this.editingProvider.isEditing) { + await this.editingProvider.revertChanges() // closes diff view } // if last message is a partial we need to update and save it @@ -1349,7 +1372,7 @@ export class Task extends EventEmitter { this.presentAssistantMessageLocked = false this.presentAssistantMessageHasPendingUpdates = false - await this.diffViewProvider.reset() + await this.editingProvider.reset() // Yields only if the first chunk is successful, otherwise will // allow the user to retry the request (most likely due to rate diff --git a/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts b/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts index e763125d4a..16cc3b08e5 100644 --- a/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts @@ -34,7 +34,7 @@ describe("applyDiffTool experiment routing", () => { applyDiff: vi.fn(), getProgressStatus: vi.fn(), }, - diffViewProvider: { + editingProvider: { reset: vi.fn(), }, api: { diff --git a/src/core/tools/__tests__/insertContentTool.spec.ts b/src/core/tools/__tests__/insertContentTool.spec.ts index e23d7aaa33..577a01930e 100644 --- a/src/core/tools/__tests__/insertContentTool.spec.ts +++ b/src/core/tools/__tests__/insertContentTool.spec.ts @@ -82,7 +82,7 @@ describe("insertContentTool", () => { rooIgnoreController: { validateAccess: vi.fn().mockReturnValue(true), }, - diffViewProvider: { + editingProvider: { editType: undefined, isEditing: false, originalContent: "", @@ -179,9 +179,9 @@ describe("insertContentTool", () => { const calledPath = mockedFileExistsAtPath.mock.calls[0][0] expect(toPosix(calledPath)).toContain(testFilePath) expect(mockedFsReadFile).not.toHaveBeenCalled() // Should not read if file doesn't exist - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(contentToInsert, true) - expect(mockCline.diffViewProvider.editType).toBe("create") - expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith(contentToInsert, true) + expect(mockCline.editingProvider.editType).toBe("create") + expect(mockCline.editingProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) }) it("creates a new file and inserts content at line 1 (beginning)", async () => { @@ -195,9 +195,9 @@ describe("insertContentTool", () => { const calledPath = mockedFileExistsAtPath.mock.calls[0][0] expect(toPosix(calledPath)).toContain(testFilePath) expect(mockedFsReadFile).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(contentToInsert, true) - expect(mockCline.diffViewProvider.editType).toBe("create") - expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith(contentToInsert, true) + expect(mockCline.editingProvider.editType).toBe("create") + expect(mockCline.editingProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) }) it("creates an empty new file if content is empty string", async () => { @@ -207,9 +207,9 @@ describe("insertContentTool", () => { const calledPath = mockedFileExistsAtPath.mock.calls[0][0] expect(toPosix(calledPath)).toContain(testFilePath) expect(mockedFsReadFile).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true) - expect(mockCline.diffViewProvider.editType).toBe("create") - expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith("", true) + expect(mockCline.editingProvider.editType).toBe("create") + expect(mockCline.editingProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true) }) it("returns an error when inserting content at an arbitrary line number into a new file", async () => { @@ -226,8 +226,8 @@ describe("insertContentTool", () => { expect(mockCline.consecutiveMistakeCount).toBe(1) expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content") expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file")) - expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled() - expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled() + expect(mockCline.editingProvider.update).not.toHaveBeenCalled() + expect(mockCline.editingProvider.pushToolWriteResult).not.toHaveBeenCalled() }) }) }) diff --git a/src/core/tools/__tests__/writeToFileTool.spec.ts b/src/core/tools/__tests__/writeToFileTool.spec.ts index 1b8582c9cc..bfa90114c5 100644 --- a/src/core/tools/__tests__/writeToFileTool.spec.ts +++ b/src/core/tools/__tests__/writeToFileTool.spec.ts @@ -143,7 +143,7 @@ describe("writeToFileTool", () => { mockCline.rooIgnoreController = { validateAccess: vi.fn().mockReturnValue(true), } - mockCline.diffViewProvider = { + mockCline.editingProvider = { editType: undefined, isEditing: false, originalContent: "", @@ -246,7 +246,7 @@ describe("writeToFileTool", () => { await executeWriteFileTool({}, { accessAllowed: true }) expect(mockCline.rooIgnoreController.validateAccess).toHaveBeenCalledWith(testFilePath) - expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath) + expect(mockCline.editingProvider.open).toHaveBeenCalledWith(testFilePath) }) }) @@ -255,18 +255,18 @@ describe("writeToFileTool", () => { await executeWriteFileTool({}, { fileExists: true }) expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath) - expect(mockCline.diffViewProvider.editType).toBe("modify") + expect(mockCline.editingProvider.editType).toBe("modify") }) it.skipIf(process.platform === "win32")("detects new file and sets editType to create", async () => { await executeWriteFileTool({}, { fileExists: false }) expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath) - expect(mockCline.diffViewProvider.editType).toBe("create") + expect(mockCline.editingProvider.editType).toBe("create") }) it("uses cached editType without filesystem check", async () => { - mockCline.diffViewProvider.editType = "modify" + mockCline.editingProvider.editType = "modify" await executeWriteFileTool({}) @@ -278,13 +278,13 @@ describe("writeToFileTool", () => { it("removes markdown code block markers from content", async () => { await executeWriteFileTool({ content: testContentWithMarkdown }) - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("Line 1\nLine 2", true) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith("Line 1\nLine 2", true) }) it("passes through empty content unchanged", async () => { await executeWriteFileTool({ content: "" }) - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith("", true) }) it("unescapes HTML entities for non-Claude models", async () => { @@ -312,7 +312,7 @@ describe("writeToFileTool", () => { expect(mockedEveryLineHasLineNumbers).toHaveBeenCalledWith(contentWithLineNumbers) expect(mockedStripLineNumbers).toHaveBeenCalledWith(contentWithLineNumbers) - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("line one\nline two", true) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith("line one\nline two", true) }) }) @@ -321,10 +321,10 @@ describe("writeToFileTool", () => { await executeWriteFileTool({}, { fileExists: false }) expect(mockCline.consecutiveMistakeCount).toBe(0) - expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath) - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, true) + expect(mockCline.editingProvider.open).toHaveBeenCalledWith(testFilePath) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith(testContent, true) expect(mockAskApproval).toHaveBeenCalled() - expect(mockCline.diffViewProvider.saveChanges).toHaveBeenCalled() + expect(mockCline.editingProvider.saveChanges).toHaveBeenCalled() expect(mockCline.fileContextTracker.trackFileContext).toHaveBeenCalledWith(testFilePath, "roo_edited") expect(mockCline.didEditFile).toBe(true) }) @@ -349,21 +349,21 @@ describe("writeToFileTool", () => { it("returns early when path is missing in partial block", async () => { await executeWriteFileTool({ path: undefined }, { isPartial: true }) - expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() + expect(mockCline.editingProvider.open).not.toHaveBeenCalled() }) it("returns early when content is undefined in partial block", async () => { await executeWriteFileTool({ content: undefined }, { isPartial: true }) - expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled() + expect(mockCline.editingProvider.open).not.toHaveBeenCalled() }) it("streams content updates during partial execution", async () => { await executeWriteFileTool({}, { isPartial: true }) expect(mockCline.ask).toHaveBeenCalled() - expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath) - expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, false) + expect(mockCline.editingProvider.open).toHaveBeenCalledWith(testFilePath) + expect(mockCline.editingProvider.update).toHaveBeenCalledWith(testContent, false) }) }) @@ -373,19 +373,19 @@ describe("writeToFileTool", () => { await executeWriteFileTool({}) - expect(mockCline.diffViewProvider.revertChanges).toHaveBeenCalled() - expect(mockCline.diffViewProvider.saveChanges).not.toHaveBeenCalled() + expect(mockCline.editingProvider.revertChanges).toHaveBeenCalled() + expect(mockCline.editingProvider.saveChanges).not.toHaveBeenCalled() }) it("reports user edits with diff feedback", async () => { const userEditsValue = "- old line\n+ new line" - mockCline.diffViewProvider.saveChanges.mockResolvedValue({ + mockCline.editingProvider.saveChanges.mockResolvedValue({ newProblemsMessage: " with warnings", userEdits: userEditsValue, finalContent: "modified content", }) // Set the userEdits property on the diffViewProvider mock to simulate user edits - mockCline.diffViewProvider.userEdits = userEditsValue + mockCline.editingProvider.userEdits = userEditsValue await executeWriteFileTool({}, { fileExists: true }) @@ -398,21 +398,21 @@ describe("writeToFileTool", () => { describe("error handling", () => { it("handles general file operation errors", async () => { - mockCline.diffViewProvider.open.mockRejectedValue(new Error("General error")) + mockCline.editingProvider.open.mockRejectedValue(new Error("General error")) await executeWriteFileTool({}) expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error)) - expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() + expect(mockCline.editingProvider.reset).toHaveBeenCalled() }) it("handles partial streaming errors", async () => { - mockCline.diffViewProvider.open.mockRejectedValue(new Error("Open failed")) + mockCline.editingProvider.open.mockRejectedValue(new Error("Open failed")) await executeWriteFileTool({}, { isPartial: true }) expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error)) - expect(mockCline.diffViewProvider.reset).toHaveBeenCalled() + expect(mockCline.editingProvider.reset).toHaveBeenCalled() }) }) }) diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index ad4bb0590f..46028fa025 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -143,10 +143,12 @@ export async function applyDiffToolLegacy( cline.consecutiveMistakeCountForApplyDiff.delete(relPath) // Show diff view before asking for approval - cline.diffViewProvider.editType = "modify" - await cline.diffViewProvider.open(relPath) - await cline.diffViewProvider.update(diffResult.content, true) - cline.diffViewProvider.scrollToFirstDiff() + cline.editingProvider.editType = "modify" + await cline.editingProvider.open(relPath) + await cline.editingProvider.update(diffResult.content, true) + if (cline.editingProvider.scrollToFirstDiff) { + cline.editingProvider.scrollToFirstDiff() + } // Check if file is write-protected const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false @@ -166,7 +168,7 @@ export async function applyDiffToolLegacy( const didApprove = await askApproval("tool", completeMessage, toolProgressStatus, isWriteProtected) if (!didApprove) { - await cline.diffViewProvider.revertChanges() // Cline likely handles closing the diff view + await cline.editingProvider.revertChanges() // Cline likely handles closing the diff view return } @@ -175,7 +177,7 @@ export async function applyDiffToolLegacy( const state = await provider?.getState() const diagnosticsEnabled = state?.diagnosticsEnabled ?? true const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS - await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) + await cline.editingProvider.saveChanges(diagnosticsEnabled, writeDelayMs) // Track file edit operation if (relPath) { @@ -191,7 +193,7 @@ export async function applyDiffToolLegacy( } // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) + const message = await cline.editingProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) if (partFailHint) { pushToolResult(partFailHint + message) @@ -199,13 +201,13 @@ export async function applyDiffToolLegacy( pushToolResult(message) } - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } } catch (error) { await handleError("applying diff", error) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } } diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts index 2b31224400..a80c8bfe41 100644 --- a/src/core/tools/insertContentTool.ts +++ b/src/core/tools/insertContentTool.ts @@ -96,8 +96,8 @@ export async function insertContentTool( cline.consecutiveMistakeCount = 0 - cline.diffViewProvider.editType = fileExists ? "modify" : "create" - cline.diffViewProvider.originalContent = fileContent + cline.editingProvider.editType = fileExists ? "modify" : "create" + cline.editingProvider.originalContent = fileContent const lines = fileExists ? fileContent.split("\n") : [] const updatedContent = insertGroups(lines, [ @@ -108,12 +108,14 @@ export async function insertContentTool( ]).join("\n") // Show changes in diff view - if (!cline.diffViewProvider.isEditing) { + if (!cline.editingProvider.isEditing) { await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {}) // First open with original content - await cline.diffViewProvider.open(relPath) - await cline.diffViewProvider.update(fileContent, false) - cline.diffViewProvider.scrollToFirstDiff() + await cline.editingProvider.open(relPath) + await cline.editingProvider.update(fileContent, false) + if (cline.editingProvider.scrollToFirstDiff) { + cline.editingProvider.scrollToFirstDiff() + } await delay(200) } @@ -135,7 +137,7 @@ export async function insertContentTool( approvalContent = updatedContent } - await cline.diffViewProvider.update(updatedContent, true) + await cline.editingProvider.update(updatedContent, true) const completeMessage = JSON.stringify({ ...sharedMessageProps, @@ -150,7 +152,7 @@ export async function insertContentTool( .then((response) => response.response === "yesButtonClicked") if (!didApprove) { - await cline.diffViewProvider.revertChanges() + await cline.editingProvider.revertChanges() pushToolResult("Changes were rejected by the user.") return } @@ -160,7 +162,7 @@ export async function insertContentTool( const state = await provider?.getState() const diagnosticsEnabled = state?.diagnosticsEnabled ?? true const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS - await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) + await cline.editingProvider.saveChanges(diagnosticsEnabled, writeDelayMs) // Track file edit operation if (relPath) { @@ -170,13 +172,13 @@ export async function insertContentTool( cline.didEditFile = true // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) + const message = await cline.editingProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) pushToolResult(message) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() } catch (error) { handleError("insert content", error) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() } } diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts index b41d409dbb..f6c5c1a9fe 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/src/core/tools/multiApplyDiffTool.ts @@ -508,10 +508,12 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} cline.consecutiveMistakeCountForApplyDiff.delete(relPath) // Show diff view before asking for approval (only for single file or after batch approval) - cline.diffViewProvider.editType = "modify" - await cline.diffViewProvider.open(relPath) - await cline.diffViewProvider.update(originalContent!, true) - cline.diffViewProvider.scrollToFirstDiff() + cline.editingProvider.editType = "modify" + await cline.editingProvider.open(relPath) + await cline.editingProvider.update(originalContent!, true) + if (cline.editingProvider.scrollToFirstDiff) { + cline.editingProvider.scrollToFirstDiff() + } // For batch operations, we've already gotten approval const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false @@ -548,7 +550,7 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} } if (!didApprove) { - await cline.diffViewProvider.revertChanges() + await cline.editingProvider.revertChanges() results.push(`Changes to ${relPath} were not approved by user`) continue } @@ -558,7 +560,7 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} const state = await provider?.getState() const diagnosticsEnabled = state?.diagnosticsEnabled ?? true const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS - await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) + await cline.editingProvider.saveChanges(diagnosticsEnabled, writeDelayMs) // Track file edit operation await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource) @@ -572,7 +574,7 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} } // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) + const message = await cline.editingProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) if (partFailHint) { results.push(partFailHint + "\n" + message) @@ -580,7 +582,7 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} results.push(message) } - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error) updateOperationResult(relPath, { @@ -606,7 +608,7 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""} return } catch (error) { await handleError("applying diff", error) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } } diff --git a/src/core/tools/searchAndReplaceTool.ts b/src/core/tools/searchAndReplaceTool.ts index b6ec3ed39b..8a57e6f824 100644 --- a/src/core/tools/searchAndReplaceTool.ts +++ b/src/core/tools/searchAndReplaceTool.ts @@ -188,27 +188,29 @@ export async function searchAndReplaceTool( } // Initialize diff view - cline.diffViewProvider.editType = "modify" - cline.diffViewProvider.originalContent = fileContent + cline.editingProvider.editType = "modify" + cline.editingProvider.originalContent = fileContent // Generate and validate diff const diff = formatResponse.createPrettyPatch(validRelPath, fileContent, newContent) if (!diff) { pushToolResult(`No changes needed for '${relPath}'`) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } // Show changes in diff view - if (!cline.diffViewProvider.isEditing) { + if (!cline.editingProvider.isEditing) { await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {}) - await cline.diffViewProvider.open(validRelPath) - await cline.diffViewProvider.update(fileContent, false) - cline.diffViewProvider.scrollToFirstDiff() + await cline.editingProvider.open(validRelPath) + await cline.editingProvider.update(fileContent, false) + if (cline.editingProvider.scrollToFirstDiff) { + cline.editingProvider.scrollToFirstDiff() + } await delay(200) } - await cline.diffViewProvider.update(newContent, true) + await cline.editingProvider.update(newContent, true) // Request user approval for changes const completeMessage = JSON.stringify({ @@ -221,9 +223,9 @@ export async function searchAndReplaceTool( .then((response) => response.response === "yesButtonClicked") if (!didApprove) { - await cline.diffViewProvider.revertChanges() + await cline.editingProvider.revertChanges() pushToolResult("Changes were rejected by the user.") - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } @@ -232,7 +234,7 @@ export async function searchAndReplaceTool( const state = await provider?.getState() const diagnosticsEnabled = state?.diagnosticsEnabled ?? true const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS - await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) + await cline.editingProvider.saveChanges(diagnosticsEnabled, writeDelayMs) // Track file edit operation if (relPath) { @@ -242,7 +244,7 @@ export async function searchAndReplaceTool( cline.didEditFile = true // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult( + const message = await cline.editingProvider.pushToolWriteResult( cline, cline.cwd, false, // Always false for search_and_replace @@ -252,10 +254,10 @@ export async function searchAndReplaceTool( // Record successful tool usage and cleanup cline.recordToolUsage("search_and_replace") - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() } catch (error) { handleError("search and replace", error) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() } } diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index fd9d158f3f..fdd1ffa90c 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -37,7 +37,7 @@ export async function writeToFileTool( cline.consecutiveMistakeCount++ cline.recordToolError("write_to_file") pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "path")) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } @@ -45,7 +45,7 @@ export async function writeToFileTool( cline.consecutiveMistakeCount++ cline.recordToolError("write_to_file") pushToolResult(await cline.sayAndCreateMissingParamError("write_to_file", "content")) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } @@ -63,12 +63,12 @@ export async function writeToFileTool( // Check if file exists using cached map or fs.access let fileExists: boolean - if (cline.diffViewProvider.editType !== undefined) { - fileExists = cline.diffViewProvider.editType === "modify" + if (cline.editingProvider.editType !== undefined) { + fileExists = cline.editingProvider.editType === "modify" } else { const absolutePath = path.resolve(cline.cwd, relPath) fileExists = await fileExistsAtPath(absolutePath) - cline.diffViewProvider.editType = fileExists ? "modify" : "create" + cline.editingProvider.editType = fileExists ? "modify" : "create" } // pre-processing newContent for cases where weaker models might add artifacts like markdown codeblock markers (deepseek/llama) or extra escape characters (gemini) @@ -104,13 +104,13 @@ export async function writeToFileTool( await cline.ask("tool", partialMessage, block.partial).catch(() => {}) // update editor - if (!cline.diffViewProvider.isEditing) { + if (!cline.editingProvider.isEditing) { // open the editor and prepare to stream content in - await cline.diffViewProvider.open(relPath) + await cline.editingProvider.open(relPath) } // editor is open, stream content in - await cline.diffViewProvider.update( + await cline.editingProvider.update( everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent, false, ) @@ -143,7 +143,7 @@ export async function writeToFileTool( formatResponse.lineCountTruncationError(actualLineCount, isNewFile, diffStrategyEnabled), ), ) - await cline.diffViewProvider.revertChanges() + await cline.editingProvider.revertChanges() return } @@ -152,25 +152,27 @@ export async function writeToFileTool( // if isEditingFile false, that means we have the full contents of the file already. // it's important to note how cline function works, you can't make the assumption that the block.partial conditional will always be called since it may immediately get complete, non-partial data. So cline part of the logic will always be called. // in other words, you must always repeat the block.partial logic here - if (!cline.diffViewProvider.isEditing) { + if (!cline.editingProvider.isEditing) { // show gui message before showing edit animation const partialMessage = JSON.stringify(sharedMessageProps) await cline.ask("tool", partialMessage, true).catch(() => {}) // sending true for partial even though it's not a partial, cline shows the edit row before the content is streamed into the editor - await cline.diffViewProvider.open(relPath) + await cline.editingProvider.open(relPath) } - await cline.diffViewProvider.update( + await cline.editingProvider.update( everyLineHasLineNumbers(newContent) ? stripLineNumbers(newContent) : newContent, true, ) await delay(300) // wait for diff view to update - cline.diffViewProvider.scrollToFirstDiff() + if (cline.editingProvider.scrollToFirstDiff) { + cline.editingProvider.scrollToFirstDiff() + } // Check for code omissions before proceeding - if (detectCodeOmission(cline.diffViewProvider.originalContent || "", newContent, predictedLineCount)) { + if (detectCodeOmission(cline.editingProvider.originalContent || "", newContent, predictedLineCount)) { if (cline.diffStrategy) { - await cline.diffViewProvider.revertChanges() + await cline.editingProvider.revertChanges() pushToolResult( formatResponse.toolError( @@ -202,14 +204,14 @@ export async function writeToFileTool( ...sharedMessageProps, content: fileExists ? undefined : newContent, diff: fileExists - ? formatResponse.createPrettyPatch(relPath, cline.diffViewProvider.originalContent, newContent) + ? formatResponse.createPrettyPatch(relPath, cline.editingProvider.originalContent, newContent) : undefined, } satisfies ClineSayTool) const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected) if (!didApprove) { - await cline.diffViewProvider.revertChanges() + await cline.editingProvider.revertChanges() return } @@ -218,7 +220,7 @@ export async function writeToFileTool( const state = await provider?.getState() const diagnosticsEnabled = state?.diagnosticsEnabled ?? true const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS - await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) + await cline.editingProvider.saveChanges(diagnosticsEnabled, writeDelayMs) // Track file edit operation if (relPath) { @@ -228,17 +230,17 @@ export async function writeToFileTool( cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request // Get the formatted response message - const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) + const message = await cline.editingProvider.pushToolWriteResult(cline, cline.cwd, !fileExists) pushToolResult(message) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } } catch (error) { await handleError("writing file", error) - await cline.diffViewProvider.reset() + await cline.editingProvider.reset() return } } diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6231f08167..a28993f385 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1440,6 +1440,7 @@ export class ClineProvider alwaysAllowFollowupQuestions, followupAutoApproveTimeoutMs, diagnosticsEnabled, + fileBasedEditing, } = await this.getState() const telemetryKey = process.env.POSTHOG_API_KEY @@ -1561,6 +1562,7 @@ export class ClineProvider alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false, followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000, diagnosticsEnabled: diagnosticsEnabled ?? true, + fileBasedEditing: fileBasedEditing ?? false, } } @@ -1645,6 +1647,7 @@ export class ClineProvider alwaysAllowUpdateTodoList: stateValues.alwaysAllowUpdateTodoList ?? false, followupAutoApproveTimeoutMs: stateValues.followupAutoApproveTimeoutMs ?? 60000, diagnosticsEnabled: stateValues.diagnosticsEnabled ?? true, + fileBasedEditing: stateValues.fileBasedEditing ?? false, allowedMaxRequests: stateValues.allowedMaxRequests, autoCondenseContext: stateValues.autoCondenseContext ?? true, autoCondenseContextPercent: stateValues.autoCondenseContextPercent ?? 100, diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 780d40df89..8c68996bdf 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -932,6 +932,11 @@ export const webviewMessageHandler = async ( await updateGlobalState("diffEnabled", diffEnabled) await provider.postStateToWebview() break + case "fileBasedEditing": + const fileBasedEditing = message.bool ?? false + await updateGlobalState("fileBasedEditing", fileBasedEditing) + await provider.postStateToWebview() + break case "enableCheckpoints": const enableCheckpoints = message.bool ?? true await updateGlobalState("enableCheckpoints", enableCheckpoints) diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index f4133029c9..b0f30a33a8 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -15,12 +15,13 @@ import { Task } from "../../core/task/Task" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { DecorationController } from "./DecorationController" +import { IEditingProvider } from "./IEditingProvider" export const DIFF_VIEW_URI_SCHEME = "cline-diff" export const DIFF_VIEW_LABEL_CHANGES = "Original ↔ Roo's Changes" // TODO: https://github.com/cline/cline/pull/3354 -export class DiffViewProvider { +export class DiffViewProvider implements IEditingProvider { // Properties to store the results of saveChanges newProblemsMessage?: string userEdits?: string @@ -181,7 +182,10 @@ export class DiffViewProvider { } } - async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{ + async saveChanges( + diagnosticsEnabled: boolean = true, + writeDelayMs: number = DEFAULT_WRITE_DELAY_MS, + ): Promise<{ newProblemsMessage: string | undefined userEdits: string | undefined finalContent: string | undefined @@ -216,22 +220,22 @@ export class DiffViewProvider { // and can address them accordingly. If problems don't change immediately after // applying a fix, won't be notified, which is generally fine since the // initial fix is usually correct and it may just take time for linters to catch up. - + let newProblemsMessage = "" - + if (diagnosticsEnabled) { // Add configurable delay to allow linters time to process and clean up issues // like unused imports (especially important for Go and other languages) // Ensure delay is non-negative const safeDelayMs = Math.max(0, writeDelayMs) - + try { await delay(safeDelayMs) } catch (error) { // Log error but continue - delay failure shouldn't break the save operation console.warn(`Failed to apply write delay: ${error}`) } - + const postDiagnostics = vscode.languages.getDiagnostics() const newProblems = await diagnosticsToProblemsString( diff --git a/src/integrations/editor/FileWriter.ts b/src/integrations/editor/FileWriter.ts new file mode 100644 index 0000000000..3ca6430fb8 --- /dev/null +++ b/src/integrations/editor/FileWriter.ts @@ -0,0 +1,183 @@ +import * as path from "path" +import * as fs from "fs/promises" +import { XMLBuilder } from "fast-xml-parser" + +import { IEditingProvider } from "./IEditingProvider" +import { Task } from "../../core/task/Task" +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { createDirectoriesForFile } from "../../utils/fs" +import { getReadablePath } from "../../utils/path" +import { formatResponse } from "../../core/prompts/responses" + +/** + * FileWriter implements direct file system writes without visual feedback. + * This provider bypasses the diff view and writes changes directly to disk. + */ +export class FileWriter implements IEditingProvider { + isEditing = false + editType?: "create" | "modify" + originalContent?: string + + private relPath?: string + private newContent?: string + private createdDirs: string[] = [] + + constructor(private cwd: string) {} + + async open(relPath: string): Promise { + this.relPath = relPath + const absolutePath = path.resolve(this.cwd, relPath) + + try { + // Check if file exists + await fs.access(absolutePath) + this.editType = "modify" + this.originalContent = await fs.readFile(absolutePath, "utf-8") + } catch { + // File doesn't exist + this.editType = "create" + this.originalContent = "" + + // Create necessary directories + this.createdDirs = await createDirectoriesForFile(absolutePath) + } + + this.isEditing = true + } + + async update(content: string, isFinal: boolean): Promise { + if (!this.relPath) { + throw new Error("No file path set for FileWriter") + } + + this.newContent = content + + // For file-based editing, we don't do anything until saveChanges is called + // This maintains compatibility with the streaming interface + } + + async saveChanges( + diagnosticsEnabled: boolean = true, + writeDelayMs: number = 0, + ): Promise<{ + newProblemsMessage: string | undefined + userEdits: string | undefined + finalContent: string | undefined + }> { + if (!this.relPath || !this.newContent) { + return { + newProblemsMessage: undefined, + userEdits: undefined, + finalContent: undefined, + } + } + + const absolutePath = path.resolve(this.cwd, this.relPath) + + // Write the file directly + await fs.writeFile(absolutePath, this.newContent, "utf-8") + + // For file-based editing, we don't check diagnostics or track user edits + // since there's no opportunity for the user to modify the content + return { + newProblemsMessage: undefined, + userEdits: undefined, + finalContent: this.newContent, + } + } + + async pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise { + if (!this.relPath) { + throw new Error("No file path available in FileWriter") + } + + // Create say object for UI feedback (without diff since we're not showing it) + const say: ClineSayTool = { + tool: isNewFile ? "newFileCreated" : "editedExistingFile", + path: getReadablePath(cwd, this.relPath), + } + + // Send the feedback + await task.say("user_feedback_diff", JSON.stringify(say)) + + // Build XML response + const xmlObj = { + file_write_result: { + path: this.relPath, + operation: isNewFile ? "created" : "modified", + notice: { + i: [ + "File has been written directly to disk without visual diff", + "Proceed with the task using these changes as the new baseline.", + ], + }, + }, + } + + const builder = new XMLBuilder({ + format: true, + indentBy: "", + suppressEmptyNode: true, + processEntities: false, + tagValueProcessor: (name, value) => { + if (typeof value === "string") { + return value.replace(/&/g, "&").replace(//g, ">") + } + return value + }, + attributeValueProcessor: (name, value) => { + if (typeof value === "string") { + return value.replace(/&/g, "&").replace(//g, ">") + } + return value + }, + }) + + return builder.build(xmlObj) + } + + async revertChanges(): Promise { + if (!this.relPath) { + return + } + + const absolutePath = path.resolve(this.cwd, this.relPath) + + if (this.editType === "create") { + // Delete the file if it was newly created + try { + await fs.unlink(absolutePath) + } catch { + // File might not exist + } + + // Remove created directories in reverse order + for (let i = this.createdDirs.length - 1; i >= 0; i--) { + try { + await fs.rmdir(this.createdDirs[i]) + } catch { + // Directory might not be empty or already deleted + } + } + } else if (this.editType === "modify" && this.originalContent !== undefined) { + // Restore original content + await fs.writeFile(absolutePath, this.originalContent, "utf-8") + } + + await this.reset() + } + + async reset(): Promise { + this.isEditing = false + this.editType = undefined + this.originalContent = undefined + this.relPath = undefined + this.newContent = undefined + this.createdDirs = [] + } + + // Optional method - not applicable for file-based editing + scrollToFirstDiff(): void { + // No-op for file-based editing + } +} diff --git a/src/integrations/editor/IEditingProvider.ts b/src/integrations/editor/IEditingProvider.ts new file mode 100644 index 0000000000..26154098cb --- /dev/null +++ b/src/integrations/editor/IEditingProvider.ts @@ -0,0 +1,77 @@ +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { Task } from "../../core/task/Task" + +/** + * Interface for file editing providers. + * This abstraction allows switching between different editing strategies: + * - DiffViewProvider: Shows visual diff in editor before applying changes + * - FileWriter: Writes directly to file system without visual feedback + */ +export interface IEditingProvider { + /** + * Whether the provider is currently editing a file + */ + isEditing: boolean + + /** + * The type of edit operation (create or modify) + */ + editType?: "create" | "modify" + + /** + * The original content of the file being edited + */ + originalContent?: string + + /** + * Open a file for editing + * @param relPath Relative path to the file + */ + open(relPath: string): Promise + + /** + * Update the file content + * @param content The new content + * @param isFinal Whether this is the final update + */ + update(content: string, isFinal: boolean): Promise + + /** + * Save the changes to the file + * @param diagnosticsEnabled Whether to check for diagnostics after saving + * @param writeDelayMs Delay in milliseconds before writing + * @returns Object containing diagnostic messages, user edits, and final content + */ + saveChanges( + diagnosticsEnabled?: boolean, + writeDelayMs?: number, + ): Promise<{ + newProblemsMessage: string | undefined + userEdits: string | undefined + finalContent: string | undefined + }> + + /** + * Push the result of a write operation to the task + * @param task The current task + * @param cwd Current working directory + * @param isNewFile Whether this is a new file + * @returns Formatted XML response message + */ + pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise + + /** + * Revert any pending changes + */ + revertChanges(): Promise + + /** + * Reset the provider state + */ + reset(): Promise + + /** + * Scroll to the first difference (only applicable for diff-based providers) + */ + scrollToFirstDiff?(): void +} diff --git a/src/integrations/editor/__tests__/FileWriter.spec.ts b/src/integrations/editor/__tests__/FileWriter.spec.ts new file mode 100644 index 0000000000..db971609b1 --- /dev/null +++ b/src/integrations/editor/__tests__/FileWriter.spec.ts @@ -0,0 +1,278 @@ +import { FileWriter } from "../FileWriter" +import * as fs from "fs/promises" +import * as path from "path" +import { createDirectoriesForFile } from "../../../utils/fs" +import { Task } from "../../../core/task/Task" + +// Mock fs/promises +vi.mock("fs/promises", () => ({ + readFile: vi.fn(), + writeFile: vi.fn(), + access: vi.fn(), + unlink: vi.fn(), + rmdir: vi.fn(), +})) + +// Mock utils +vi.mock("../../../utils/fs", () => ({ + createDirectoriesForFile: vi.fn().mockResolvedValue([]), +})) + +// Mock path +vi.mock("path", () => ({ + resolve: vi.fn((cwd, relPath) => `${cwd}/${relPath}`), + dirname: vi.fn((filePath) => { + const parts = filePath.split("/") + parts.pop() + return parts.join("/") + }), +})) + +// Mock getReadablePath +vi.mock("../../../utils/path", () => ({ + getReadablePath: vi.fn((cwd, relPath) => relPath), +})) + +describe("FileWriter", () => { + let fileWriter: FileWriter + const mockCwd = "/mock/cwd" + + beforeEach(() => { + vi.clearAllMocks() + fileWriter = new FileWriter(mockCwd) + }) + + describe("open method", () => { + it("should set relPath and editType for existing file", async () => { + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.readFile).mockResolvedValue("existing content") + + await fileWriter.open("test.txt") + + expect(fileWriter["relPath"]).toBe("test.txt") + expect(fileWriter["editType"]).toBe("modify") + expect(fileWriter.isEditing).toBe(true) + }) + + it("should set editType to create for new file", async () => { + vi.mocked(fs.access).mockRejectedValue(new Error("File not found")) + + await fileWriter.open("newfile.txt") + + expect(fileWriter["relPath"]).toBe("newfile.txt") + expect(fileWriter["editType"]).toBe("create") + expect(fileWriter.isEditing).toBe(true) + }) + + it("should read file content for existing file", async () => { + const mockContent = "existing content" + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.readFile).mockResolvedValue(mockContent) + + await fileWriter.open("test.txt") + + expect(fs.readFile).toHaveBeenCalledWith(`${mockCwd}/test.txt`, "utf-8") + expect(fileWriter["originalContent"]).toBe(mockContent) + }) + + it("should set empty content for new file", async () => { + vi.mocked(fs.access).mockRejectedValue(new Error("File not found")) + + await fileWriter.open("newfile.txt") + + expect(fileWriter["originalContent"]).toBe("") + expect(fs.readFile).not.toHaveBeenCalled() + }) + + it("should create directories for new file", async () => { + vi.mocked(fs.access).mockRejectedValue(new Error("File not found")) + vi.mocked(createDirectoriesForFile).mockResolvedValue(["/mock/cwd/new", "/mock/cwd/new/dir"]) + + await fileWriter.open("new/dir/file.txt") + + expect(createDirectoriesForFile).toHaveBeenCalledWith(`${mockCwd}/new/dir/file.txt`) + expect(fileWriter["createdDirs"]).toEqual(["/mock/cwd/new", "/mock/cwd/new/dir"]) + }) + }) + + describe("update method", () => { + beforeEach(async () => { + // Setup file writer with a file + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.readFile).mockResolvedValue("original content") + await fileWriter.open("test.txt") + }) + + it("should update newContent", async () => { + await fileWriter.update("new content", false) + + expect(fileWriter["newContent"]).toBe("new content") + }) + + it("should handle multiple updates", async () => { + await fileWriter.update("first content", false) + await fileWriter.update("second content", false) + await fileWriter.update("final content", true) + + expect(fileWriter["newContent"]).toBe("final content") + }) + }) + + describe("saveChanges method", () => { + beforeEach(async () => { + // Setup file writer with a file + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.readFile).mockResolvedValue("original content") + await fileWriter.open("test.txt") + await fileWriter.update("new content", false) + }) + + it("should write content to file", async () => { + vi.mocked(fs.writeFile).mockResolvedValue(undefined) + + const result = await fileWriter.saveChanges() + + expect(fs.writeFile).toHaveBeenCalledWith(`${mockCwd}/test.txt`, "new content", "utf-8") + expect(result.newProblemsMessage).toBeUndefined() + expect(result.userEdits).toBeUndefined() + expect(result.finalContent).toBe("new content") + }) + + it("should handle write errors", async () => { + const error = new Error("Write failed") + vi.mocked(fs.writeFile).mockRejectedValue(error) + + await expect(fileWriter.saveChanges()).rejects.toThrow("Write failed") + }) + + it("should handle saveChanges with parameters", async () => { + vi.mocked(fs.writeFile).mockResolvedValue(undefined) + + const result = await fileWriter.saveChanges(false, 1000) + + expect(fs.writeFile).toHaveBeenCalledWith(`${mockCwd}/test.txt`, "new content", "utf-8") + expect(result.finalContent).toBe("new content") + }) + + it("should return empty result when no content to save", async () => { + const emptyWriter = new FileWriter(mockCwd) + + const result = await emptyWriter.saveChanges() + + expect(result.newProblemsMessage).toBeUndefined() + expect(result.userEdits).toBeUndefined() + expect(result.finalContent).toBeUndefined() + expect(fs.writeFile).not.toHaveBeenCalled() + }) + }) + + describe("revertChanges method", () => { + beforeEach(async () => { + // Setup file writer with a file + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.readFile).mockResolvedValue("original content") + await fileWriter.open("test.txt") + await fileWriter.update("new content", false) + }) + + it("should revert to original content for existing file", async () => { + vi.mocked(fs.writeFile).mockResolvedValue(undefined) + + await fileWriter.revertChanges() + + expect(fs.writeFile).toHaveBeenCalledWith(`${mockCwd}/test.txt`, "original content", "utf-8") + }) + + it("should delete file if it was newly created", async () => { + fileWriter["editType"] = "create" + fileWriter["createdDirs"] = ["/mock/cwd/new", "/mock/cwd/new/dir"] + vi.mocked(fs.unlink).mockResolvedValue(undefined) + vi.mocked(fs.rmdir).mockResolvedValue(undefined) + + await fileWriter.revertChanges() + + expect(fs.unlink).toHaveBeenCalledWith(`${mockCwd}/test.txt`) + expect(fs.rmdir).toHaveBeenCalledWith("/mock/cwd/new/dir") + expect(fs.rmdir).toHaveBeenCalledWith("/mock/cwd/new") + expect(fs.writeFile).not.toHaveBeenCalled() + }) + + it("should handle revert errors", async () => { + const error = new Error("Revert failed") + vi.mocked(fs.writeFile).mockRejectedValue(error) + + await expect(fileWriter.revertChanges()).rejects.toThrow("Revert failed") + }) + }) + + describe("pushToolWriteResult method", () => { + let mockTask: Task + + beforeEach(() => { + mockTask = { + say: vi.fn().mockResolvedValue(undefined), + } as any + }) + + it("should send user feedback and return XML for new file", async () => { + fileWriter["relPath"] = "test.txt" + fileWriter["editType"] = "create" + + const result = await fileWriter.pushToolWriteResult(mockTask, mockCwd, true) + + expect(mockTask.say).toHaveBeenCalledWith("user_feedback_diff", expect.stringContaining("newFileCreated")) + expect(result).toContain("") + expect(result).toContain("test.txt") + expect(result).toContain("created") + }) + + it("should send user feedback and return XML for modified file", async () => { + fileWriter["relPath"] = "test.txt" + fileWriter["editType"] = "modify" + + const result = await fileWriter.pushToolWriteResult(mockTask, mockCwd, false) + + expect(mockTask.say).toHaveBeenCalledWith( + "user_feedback_diff", + expect.stringContaining("editedExistingFile"), + ) + expect(result).toContain("") + expect(result).toContain("test.txt") + expect(result).toContain("modified") + }) + + it("should throw error when no relPath is set", async () => { + await expect(fileWriter.pushToolWriteResult(mockTask, mockCwd, true)).rejects.toThrow( + "No file path available in FileWriter", + ) + }) + }) + + describe("reset method", () => { + it("should reset all state", async () => { + // Setup some state + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.readFile).mockResolvedValue("original content") + await fileWriter.open("test.txt") + await fileWriter.update("new content", false) + + // Reset + await fileWriter.reset() + + // Verify all state is cleared + expect(fileWriter.isEditing).toBe(false) + expect(fileWriter["relPath"]).toBeUndefined() + expect(fileWriter["editType"]).toBeUndefined() + expect(fileWriter["originalContent"]).toBeUndefined() + expect(fileWriter["newContent"]).toBeUndefined() + expect(fileWriter["createdDirs"]).toEqual([]) + }) + }) + + describe("scrollToFirstDiff method", () => { + it("should be a no-op for file-based editing", () => { + // This method should do nothing for FileWriter + expect(() => fileWriter.scrollToFirstDiff()).not.toThrow() + }) + }) +}) diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 4f2aa2da15..b21d837325 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -217,6 +217,7 @@ export type ExtensionState = Pick< | "terminalCompressProgressBar" | "diagnosticsEnabled" | "diffEnabled" + | "fileBasedEditing" | "fuzzyMatchThreshold" // | "experiments" // Optional in GlobalSettings, required here. | "language" @@ -247,6 +248,7 @@ export type ExtensionState = Pick< writeDelayMs: number requestDelaySeconds: number + fileBasedEditing?: boolean enableCheckpoints: boolean maxOpenTabsContext: number // Maximum number of VSCode open tabs to include in context (0-500) maxWorkspaceFiles: number // Maximum number of files to include in current working directory details (0-500) diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 1f56829f7b..ae7b928a6e 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -93,6 +93,7 @@ export interface WebviewMessage { | "ttsSpeed" | "soundVolume" | "diffEnabled" + | "fileBasedEditing" | "enableCheckpoints" | "browserViewportSize" | "screenshotQuality" diff --git a/webview-ui/src/components/settings/FileEditingOptions.tsx b/webview-ui/src/components/settings/FileEditingOptions.tsx new file mode 100644 index 0000000000..a0085dd866 --- /dev/null +++ b/webview-ui/src/components/settings/FileEditingOptions.tsx @@ -0,0 +1,90 @@ +import React from "react" +import { FileText } from "lucide-react" +import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" + +import { useAppTranslation } from "@/i18n/TranslationContext" +import { Input } from "@/components/ui" + +import { SetCachedStateField } from "./types" +import { SectionHeader } from "./SectionHeader" +import { Section } from "./Section" + +interface FileEditingOptionsProps { + diffEnabled: boolean + fileBasedEditing: boolean + writeDelayMs: number + setCachedStateField: SetCachedStateField +} + +export const FileEditingOptions: React.FC = ({ + diffEnabled, + fileBasedEditing, + writeDelayMs, + setCachedStateField, +}) => { + const { t } = useAppTranslation() + + return ( +
+ +
+ +
{t("settings:sections.fileEditing")}
+
+ + +
+
+
+ setCachedStateField("fileBasedEditing", e.target.checked)} + data-testid="file-based-editing-checkbox"> + {t("settings:fileEditing.fileBasedEditingLabel")} + +
+ {t("settings:fileEditing.fileBasedEditingDescription")} +
+
+ +
+ setCachedStateField("diffEnabled", e.target.checked)} + disabled={fileBasedEditing} + data-testid="diff-enabled-checkbox"> + {t("settings:fileEditing.diffEnabledLabel")} + +
+ {t("settings:fileEditing.diffEnabledDescription")} +
+
+ +
+ +
+ { + const value = parseInt(e.target.value, 10) + if (!isNaN(value) && value >= 0) { + setCachedStateField("writeDelayMs", value) + } + }} + className="w-24" + min="0" + step="100" + data-testid="write-delay-input" + /> + ms +
+
+ {t("settings:fileEditing.writeDelayDescription")} +
+
+
+
+
+ ) +} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 517c1c159d..9bc0328634 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -23,6 +23,7 @@ import { Info, MessageSquare, LucideIcon, + FileText, } from "lucide-react" import type { ProviderSettings, ExperimentId } from "@roo-code/types" @@ -65,6 +66,7 @@ import { LanguageSettings } from "./LanguageSettings" import { About } from "./About" import { Section } from "./Section" import PromptsSettings from "./PromptsSettings" +import { FileEditingOptions } from "./FileEditingOptions" import { cn } from "@/lib/utils" export const settingsTabsContainer = "flex flex-1 overflow-hidden [&.narrow_.tab-label]:hidden" @@ -81,6 +83,7 @@ export interface SettingsViewRef { const sectionNames = [ "providers", "autoApprove", + "fileEditing", "browser", "checkpoints", "notifications", @@ -177,6 +180,7 @@ const SettingsView = forwardRef(({ onDone, t alwaysAllowFollowupQuestions, alwaysAllowUpdateTodoList, followupAutoApproveTimeoutMs, + fileBasedEditing, } = cachedState const apiConfiguration = useMemo(() => cachedState.apiConfiguration ?? {}, [cachedState.apiConfiguration]) @@ -294,6 +298,7 @@ const SettingsView = forwardRef(({ onDone, t vscode.postMessage({ type: "ttsSpeed", value: ttsSpeed }) vscode.postMessage({ type: "soundVolume", value: soundVolume }) vscode.postMessage({ type: "diffEnabled", bool: diffEnabled }) + vscode.postMessage({ type: "fileBasedEditing", bool: fileBasedEditing }) vscode.postMessage({ type: "enableCheckpoints", bool: enableCheckpoints }) vscode.postMessage({ type: "browserViewportSize", text: browserViewportSize }) vscode.postMessage({ type: "remoteBrowserHost", text: remoteBrowserHost }) @@ -404,6 +409,7 @@ const SettingsView = forwardRef(({ onDone, t () => [ { id: "providers", icon: Webhook }, { id: "autoApprove", icon: CheckCheck }, + { id: "fileEditing", icon: FileText }, { id: "browser", icon: SquareMousePointer }, { id: "checkpoints", icon: GitBranch }, { id: "notifications", icon: Bell }, @@ -623,6 +629,16 @@ const SettingsView = forwardRef(({ onDone, t /> )} + {/* File Editing Section */} + {activeTab === "fileEditing" && ( + + )} + {/* Browser Section */} {activeTab === "browser" && ( void + fileBasedEditing?: boolean + setFileBasedEditing: (value: boolean) => void } export const ExtensionStateContext = createContext(undefined) @@ -171,6 +173,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode ttsEnabled: false, ttsSpeed: 1.0, diffEnabled: false, + fileBasedEditing: false, enableCheckpoints: true, fuzzyMatchThreshold: 1.0, language: "en", // Default language code @@ -405,6 +408,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode setTtsEnabled: (value) => setState((prevState) => ({ ...prevState, ttsEnabled: value })), setTtsSpeed: (value) => setState((prevState) => ({ ...prevState, ttsSpeed: value })), setDiffEnabled: (value) => setState((prevState) => ({ ...prevState, diffEnabled: value })), + setFileBasedEditing: (value) => setState((prevState) => ({ ...prevState, fileBasedEditing: value })), setEnableCheckpoints: (value) => setState((prevState) => ({ ...prevState, enableCheckpoints: value })), setBrowserViewportSize: (value: string) => setState((prevState) => ({ ...prevState, browserViewportSize: value })), diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index 7e3c2e3fcc..a8d811e93a 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -31,7 +31,8 @@ "prompts": "Prompts", "experimental": "Experimental", "language": "Language", - "about": "About Roo Code" + "about": "About Roo Code", + "fileEditing": "File Editing" }, "prompts": { "description": "Configure support prompts that are used for quick actions like enhancing prompts, explaining code, and fixing issues. These prompts help Roo provide better assistance for common development tasks." @@ -579,6 +580,15 @@ } } }, + "fileEditing": { + "description": "Configure how Roo edits files - either through visual diffs or direct file writes", + "fileBasedEditingLabel": "Enable file-based editing mode", + "fileBasedEditingDescription": "When enabled, Roo will write changes directly to files without showing diffs. This is faster but provides less visibility into changes.", + "diffEnabledLabel": "Show diff view", + "diffEnabledDescription": "When enabled, Roo will show a visual diff of changes before applying them. This is disabled when file-based editing is active.", + "writeDelayLabel": "Write delay", + "writeDelayDescription": "Delay in milliseconds after file writes to allow diagnostics to detect potential problems" + }, "experimental": { "DIFF_STRATEGY_UNIFIED": { "name": "Use experimental unified diff strategy",