From d38d7ae42a84e6c7c76014aacec6da65fc832077 Mon Sep 17 00:00:00 2001 From: Felix Anhalt Date: Sun, 20 Jul 2025 23:49:16 +0200 Subject: [PATCH] feat: implement file-based editing option and update related settings --- packages/types/src/global-settings.ts | 2 + src/core/task/Task.ts | 18 +- .../applyDiffTool.experiment.spec.ts | 2 +- .../tools/__tests__/insertContentTool.spec.ts | 24 +- .../tools/__tests__/writeToFileTool.spec.ts | 48 +-- src/core/tools/applyDiffTool.ts | 18 +- src/core/tools/insertContentTool.ts | 24 +- src/core/tools/multiApplyDiffTool.ts | 18 +- src/core/tools/searchAndReplaceTool.ts | 28 +- src/core/tools/writeToFileTool.ts | 42 +-- src/core/webview/ClineProvider.ts | 3 + src/core/webview/webviewMessageHandler.ts | 10 + src/integrations/editor/DiffViewProvider.ts | 16 +- .../editor/EditingProviderFactory.ts | 42 +++ src/integrations/editor/FileWriter.ts | 336 ++++++++++++++++++ src/integrations/editor/IEditingProvider.ts | 65 ++++ .../__tests__/EditingProviderFactory.spec.ts | 90 +++++ .../editor/__tests__/FileWriter.spec.ts | 207 +++++++++++ src/package.json | 5 + src/shared/ExtensionMessage.ts | 1 + src/shared/WebviewMessage.ts | 1 + .../src/components/settings/ApiOptions.tsx | 6 - .../settings/DiffSettingsControl.tsx | 68 ---- .../settings/FileEditingOptions.tsx | 189 ++++++++++ .../src/components/settings/SettingsView.tsx | 25 ++ .../settings/__tests__/ApiOptions.spec.tsx | 12 +- .../__tests__/FileEditingOptions.spec.tsx | 134 +++++++ webview-ui/src/i18n/locales/ca/settings.json | 18 +- webview-ui/src/i18n/locales/de/settings.json | 18 +- webview-ui/src/i18n/locales/en/settings.json | 18 +- webview-ui/src/i18n/locales/es/settings.json | 19 +- webview-ui/src/i18n/locales/fr/settings.json | 18 +- webview-ui/src/i18n/locales/hi/settings.json | 18 +- webview-ui/src/i18n/locales/id/settings.json | 18 +- webview-ui/src/i18n/locales/it/settings.json | 18 +- webview-ui/src/i18n/locales/ja/settings.json | 18 +- webview-ui/src/i18n/locales/ko/settings.json | 18 +- webview-ui/src/i18n/locales/nl/settings.json | 18 +- webview-ui/src/i18n/locales/pl/settings.json | 18 +- .../src/i18n/locales/pt-BR/settings.json | 18 +- webview-ui/src/i18n/locales/ru/settings.json | 18 +- webview-ui/src/i18n/locales/tr/settings.json | 18 +- webview-ui/src/i18n/locales/vi/settings.json | 18 +- .../src/i18n/locales/zh-CN/settings.json | 18 +- .../src/i18n/locales/zh-TW/settings.json | 18 +- 45 files changed, 1543 insertions(+), 216 deletions(-) create mode 100644 src/integrations/editor/EditingProviderFactory.ts create mode 100644 src/integrations/editor/FileWriter.ts create mode 100644 src/integrations/editor/IEditingProvider.ts create mode 100644 src/integrations/editor/__tests__/EditingProviderFactory.spec.ts create mode 100644 src/integrations/editor/__tests__/FileWriter.spec.ts delete mode 100644 webview-ui/src/components/settings/DiffSettingsControl.tsx create mode 100644 webview-ui/src/components/settings/FileEditingOptions.tsx create mode 100644 webview-ui/src/components/settings/__tests__/FileEditingOptions.spec.tsx diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index a30550dce1..345c8cf3ba 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(), @@ -250,6 +251,7 @@ export const EVALS_SETTINGS: RooCodeSettings = { diagnosticsEnabled: true, diffEnabled: true, + fileBasedEditing: false, fuzzyMatchThreshold: 1, enableCheckpoints: false, diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 53b8ef5b87..c4ce4625cf 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -91,6 +91,8 @@ import { ApiMessage } from "../task-persistence/apiMessages" import { getMessagesSinceLastSummary, summarizeConversation } from "../condense" import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning" import { restoreTodoListForTask } from "../tools/updateTodoListTool" +import { IEditingProvider } from "../../integrations/editor/IEditingProvider" +import { EditingProviderFactory } from "../../integrations/editor/EditingProviderFactory" // Constants const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes @@ -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,7 @@ 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) + this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd) this.enableCheckpoints = enableCheckpoints this.rootTask = rootTask @@ -762,6 +764,7 @@ export class Task extends EventEmitter { public async resumePausedTask(lastMessage: string) { // Release this Cline instance from paused state. + this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd) this.isPaused = false this.emit("taskUnpaused") @@ -1066,8 +1069,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) @@ -1160,6 +1163,7 @@ export class Task extends EventEmitter { if (this.abort) { throw new Error(`[RooCode#recursivelyMakeRooRequests] task ${this.taskId}.${this.instanceId} aborted`) } + this.editingProvider = EditingProviderFactory.resetAndCreateNewEditingProvider(this.cwd, this.editingProvider) if (this.consecutiveMistakeLimit > 0 && this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) { const { response, text, images } = await this.ask( @@ -1296,8 +1300,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 +1353,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..58be8fd46d 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 + // Set the userEdits property on the editingProvider mock to simulate user edits + 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..252ad38fe6 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -143,10 +143,10 @@ 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) + cline.editingProvider.scrollToFirstDiff() // Check if file is write-protected const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false @@ -166,7 +166,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 +175,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 +191,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 +199,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..06b8daeddc 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,12 @@ 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) + cline.editingProvider.scrollToFirstDiff() await delay(200) } @@ -135,7 +135,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 +150,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 +160,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 +170,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..943fd942a6 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/src/core/tools/multiApplyDiffTool.ts @@ -508,10 +508,10 @@ ${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) + cline.editingProvider.scrollToFirstDiff() // For batch operations, we've already gotten approval const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false @@ -548,7 +548,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 +558,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 +572,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 +580,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 +606,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..24e9eb880f 100644 --- a/src/core/tools/searchAndReplaceTool.ts +++ b/src/core/tools/searchAndReplaceTool.ts @@ -188,27 +188,27 @@ 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) + 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 +221,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 +232,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 +242,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 +252,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..8b3f6daf23 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,25 @@ 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() + 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 +202,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 +218,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 +228,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..65acefd537 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1384,6 +1384,7 @@ export class ClineProvider ttsEnabled, ttsSpeed, diffEnabled, + fileBasedEditing, enableCheckpoints, taskHistory, soundVolume, @@ -1482,6 +1483,7 @@ export class ClineProvider ttsEnabled: ttsEnabled ?? false, ttsSpeed: ttsSpeed ?? 1.0, diffEnabled: diffEnabled ?? true, + fileBasedEditing: fileBasedEditing ?? false, enableCheckpoints: enableCheckpoints ?? true, shouldShowAnnouncement: telemetrySetting !== "unset" && lastShownAnnouncementId !== this.latestAnnouncementId, @@ -1655,6 +1657,7 @@ export class ClineProvider ttsEnabled: stateValues.ttsEnabled ?? false, ttsSpeed: stateValues.ttsSpeed ?? 1.0, diffEnabled: stateValues.diffEnabled ?? true, + fileBasedEditing: stateValues.fileBasedEditing ?? false, enableCheckpoints: stateValues.enableCheckpoints ?? true, soundVolume: stateValues.soundVolume, browserViewportSize: stateValues.browserViewportSize ?? "900x600", diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 780d40df89..9699f8fd2e 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -932,6 +932,16 @@ export const webviewMessageHandler = async ( await updateGlobalState("diffEnabled", diffEnabled) await provider.postStateToWebview() break + case "fileBasedEditing": + const fileBasedEditing = message.bool ?? false + await provider.context.globalState.update("fileBasedEditing", fileBasedEditing) + // Also update workspace settings + await vscode.workspace + .getConfiguration("roo-cline") + .update("fileBasedEditing", fileBasedEditing, vscode.ConfigurationTarget.Global) + 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/EditingProviderFactory.ts b/src/integrations/editor/EditingProviderFactory.ts new file mode 100644 index 0000000000..9ac1cd099b --- /dev/null +++ b/src/integrations/editor/EditingProviderFactory.ts @@ -0,0 +1,42 @@ +import * as vscode from "vscode" +import { DiffViewProvider } from "./DiffViewProvider" +import { FileWriter } from "./FileWriter" +import { IEditingProvider } from "./IEditingProvider" + +/** + * Factory for creating the appropriate editing provider based on user settings + */ +export class EditingProviderFactory { + /** + * Creates an editing provider based on current VSCode settings + * @param cwd The current working directory + * @returns The appropriate editing provider (DiffViewProvider or FileWriter) + */ + static createEditingProvider(cwd: string): IEditingProvider { + const config = vscode.workspace.getConfiguration("roo-cline") + const fileBasedEditing = config.get("fileBasedEditing", false) + + if (fileBasedEditing) { + return new FileWriter(cwd) + } else { + return new DiffViewProvider(cwd) + } + } + + static resetAndCreateNewEditingProvider(cwd: string, editingProvider: IEditingProvider): IEditingProvider { + // Reset the current editing provider + editingProvider.reset() + + // Create a new instance of the appropriate provider + return this.createEditingProvider(cwd) + } + + /** + * Checks if file-based editing is currently enabled + * @returns True if file-based editing is enabled, false otherwise + */ + static isFileBasedEditingEnabled(): boolean { + const config = vscode.workspace.getConfiguration("roo-cline") + return config.get("fileBasedEditing", false) + } +} diff --git a/src/integrations/editor/FileWriter.ts b/src/integrations/editor/FileWriter.ts new file mode 100644 index 0000000000..6958e5a2a7 --- /dev/null +++ b/src/integrations/editor/FileWriter.ts @@ -0,0 +1,336 @@ +import * as vscode from "vscode" +import * as path from "path" +import * as fs from "fs/promises" +import stripBom from "strip-bom" +import { XMLBuilder } from "fast-xml-parser" + +import { createDirectoriesForFile } from "../../utils/fs" +import { getReadablePath } from "../../utils/path" +import { formatResponse } from "../../core/prompts/responses" +import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics" +import { ClineSayTool } from "../../shared/ExtensionMessage" +import { Task } from "../../core/task/Task" +import { IEditingProvider } from "./IEditingProvider" +import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" +import delay from "delay" + +interface FileWriterSettings { + fileBasedEditing: boolean +} + +/** + * FileWriter provides direct file-system editing without diff views. + * It mirrors the API of DiffViewProvider for seamless integration. + */ +export class FileWriter implements IEditingProvider { + // Properties to store the results of saveChanges + newProblemsMessage?: string + userEdits?: string + editType?: "create" | "modify" + isEditing = false + originalContent: string | undefined + private createdDirs: string[] = [] + private relPath?: string + private newContent?: string + private preDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [] + + constructor(private cwd: string) {} + + /** + * Reads file writing settings from VSCode configuration + */ + private async _readFileWriterSettings(): Promise { + const config = vscode.workspace.getConfiguration("roo-cline") + const fileBasedEditing = config.get("fileBasedEditing", false) + + return { + fileBasedEditing, + } + } + + /** + * Prepares for editing the given relative path file + * @param relPath The relative file path to prepare for editing + */ + async open(relPath: string): Promise { + this.relPath = relPath + const absolutePath = path.resolve(this.cwd, relPath) + this.isEditing = true + + // Get diagnostics before editing the file + this.preDiagnostics = vscode.languages.getDiagnostics() + + // Check if file exists to set edit type + try { + this.originalContent = await fs.readFile(absolutePath, "utf-8") + this.editType = "modify" + } catch (error) { + this.originalContent = "" + this.editType = "create" + } + + // For new files, create any necessary directories + if (this.editType === "create") { + this.createdDirs = await createDirectoriesForFile(absolutePath) + } + } + + /** + * Updates the file content (writes directly to file system) + * @param accumulatedContent The content to write + * @param isFinal Whether this is the final update + */ + async update(accumulatedContent: string, isFinal: boolean): Promise { + if (!this.relPath) { + throw new Error("Required values not set") + } + + this.newContent = accumulatedContent + const absolutePath = path.resolve(this.cwd, this.relPath) + + if (isFinal) { + // Preserve empty last line if original content had one + const hasEmptyLastLine = this.originalContent?.endsWith("\n") + + if (hasEmptyLastLine && !accumulatedContent.endsWith("\n")) { + accumulatedContent += "\n" + } + + // Write the final content directly to file + await fs.writeFile(absolutePath, this.stripAllBOMs(accumulatedContent), "utf-8") + } + } + + /** + * Finalizes the file changes and returns diagnostics information + * @param diagnosticsEnabled Whether to enable diagnostics (default: true) + * @param writeDelayMs Delay in milliseconds before writing changes (default: 1000) + */ + async saveChanges( + diagnosticsEnabled: boolean = true, + writeDelayMs: number = DEFAULT_WRITE_DELAY_MS, + ): 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) + + // Read the actual file content to check if it matches what we wrote + const finalContent = await fs.readFile(absolutePath, "utf-8") + + // Getting diagnostics before and after the file edit is a better approach than + // automatically tracking problems in real-time. This method ensures we only + // report new problems that are a direct result of this specific edit. + // Since these are new problems resulting from Roo's edit, we know they're + // directly related to the work he's doing. This eliminates the risk of Roo + // going off-task or getting distracted by unrelated issues, which was a problem + // with the previous auto-debug approach. Some users' machines may be slow to + // update diagnostics, so this approach provides a good balance between automation + // and avoiding potential issues where Roo might get stuck in loops due to + // outdated problem information. If no new problems show up by the time the user + // accepts the changes, they can always debug later using the '@problems' mention. + // This way, Roo only becomes aware of new problems resulting from his edits + // 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( + getNewDiagnostics(this.preDiagnostics, postDiagnostics), + [ + vscode.DiagnosticSeverity.Error, // only including errors since warnings can be distracting (if user wants to fix warnings they can use the @problems mention) + ], + this.cwd, + ) // Will be empty string if no errors. + + newProblemsMessage = + newProblems.length > 0 ? `\n\nNew problems detected after saving the file:\n${newProblems}` : "" + } + + // In file-based editing, there should be no user edits since we write directly + // But we check if the final content differs from what we intended to write + const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, "\n") + const normalizedFinalContent = finalContent.replace(/\r\n|\n/g, "\n").trimEnd() + + if (normalizedFinalContent !== normalizedNewContent) { + // This shouldn't happen in normal file-based editing, but handle it just in case + const userEdits = formatResponse.createPrettyPatch( + this.relPath.toPosix(), + normalizedNewContent, + normalizedFinalContent, + ) + + this.newProblemsMessage = newProblemsMessage + this.userEdits = userEdits + + return { newProblemsMessage, userEdits, finalContent: normalizedFinalContent } + } else { + this.newProblemsMessage = newProblemsMessage + this.userEdits = undefined + + return { newProblemsMessage, userEdits: undefined, finalContent: normalizedFinalContent } + } + } + + /** + * Formats a standardized XML response for file write operations + * @param task The current task context for sending user feedback + * @param cwd Current working directory for path resolution + * @param isNewFile Whether this is a new file or an existing file being modified + * @returns Formatted message and say object for UI feedback + */ + async pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise { + if (!this.relPath) { + throw new Error("No file path available in FileWriter") + } + + // Only send user_feedback_diff if userEdits exists (shouldn't happen in file-based editing) + if (this.userEdits) { + // Create say object for UI feedback + const say: ClineSayTool = { + tool: isNewFile ? "newFileCreated" : "editedExistingFile", + path: getReadablePath(cwd, this.relPath), + diff: this.userEdits, + } + + // Send the user 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", + user_edits: this.userEdits ? this.userEdits : undefined, + problems: this.newProblemsMessage || undefined, + notice: { + i: [ + "You do not need to re-read the file, as you have seen all changes", + "Proceed with the task using these changes as the new baseline.", + ...(this.userEdits + ? [ + "If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.", + ] + : []), + ], + }, + }, + } + + const builder = new XMLBuilder({ + format: true, + indentBy: "", + suppressEmptyNode: true, + processEntities: false, + tagValueProcessor: (name, value) => { + if (typeof value === "string") { + // Only escape <, >, and & characters + return value.replace(/&/g, "&").replace(//g, ">") + } + return value + }, + attributeValueProcessor: (name, value) => { + if (typeof value === "string") { + // Only escape <, >, and & characters + return value.replace(/&/g, "&").replace(//g, ">") + } + return value + }, + }) + + return builder.build(xmlObj) + } + + /** + * Reverts changes (deletes new files, restores original content for modified files) + */ + async revertChanges(): Promise { + if (!this.relPath) { + return + } + + const fileExists = this.editType === "modify" + const absolutePath = path.resolve(this.cwd, this.relPath) + + if (!fileExists) { + // Delete the newly created file + try { + await fs.unlink(absolutePath) + } catch (error) { + // File might not exist, ignore error + } + + // Remove only the directories we created, in reverse order + for (let i = this.createdDirs.length - 1; i >= 0; i--) { + try { + await fs.rmdir(this.createdDirs[i]) + } catch (error) { + // Directory might not be empty or not exist, ignore error + } + } + } else { + // Restore original content + await fs.writeFile(absolutePath, this.stripAllBOMs(this.originalContent ?? ""), "utf-8") + } + + // Reset state + this.reset() + } + + /** + * Strips all BOM characters from input string + */ + private stripAllBOMs(input: string): string { + let result = input + let previous + + do { + previous = result + result = stripBom(result) + } while (result !== previous) + + return result + } + + /** + * Resets the FileWriter state + */ + async reset(): Promise { + this.editType = undefined + this.isEditing = false + this.originalContent = undefined + this.newContent = undefined + this.createdDirs = [] + this.relPath = undefined + this.preDiagnostics = [] + this.newProblemsMessage = undefined + this.userEdits = undefined + } + + async scrollToFirstDiff(): Promise { + // No-op for FileWriter, as it doesn't handle diffs + return + } +} diff --git a/src/integrations/editor/IEditingProvider.ts b/src/integrations/editor/IEditingProvider.ts new file mode 100644 index 0000000000..17a56d9a8f --- /dev/null +++ b/src/integrations/editor/IEditingProvider.ts @@ -0,0 +1,65 @@ +import { Task } from "../../core/task/Task" + +/** + * Interface for editing providers (DiffViewProvider, FileWriter, etc.) + * This allows tools to work with different editing strategies seamlessly + */ +export interface IEditingProvider { + // Properties to store the results of saveChanges + newProblemsMessage?: string + userEdits?: string + editType?: "create" | "modify" + isEditing: boolean + originalContent: string | undefined + + /** + * Prepares for editing the given relative path file + * @param relPath The relative file path to open/prepare for editing + */ + open(relPath: string): Promise + + /** + * Updates the content being edited + * @param content The content to apply + * @param isFinal Whether this is the final update + */ + update(content: string, isFinal: boolean): Promise + + /** + * Finalizes the changes and returns diagnostics information + * @param diagnosticsEnabled Whether to enable diagnostics (default: true) + * @param writeDelayMs Delay in milliseconds before writing changes (default: 1000) + */ + saveChanges( + diagnosticsEnabled: boolean, + writeDelayMs: number, + ): Promise<{ + newProblemsMessage: string | undefined + userEdits: string | undefined + finalContent: string | undefined + }> + + /** + * Formats a standardized XML response for file write operations + * @param task The current task context for sending user feedback + * @param cwd Current working directory for path resolution + * @param isNewFile Whether this is a new file or an existing file being modified + * @returns Formatted XML response message + */ + pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise + + /** + * Reverts changes (cancels the editing operation) + */ + revertChanges(): Promise + + /** + * Resets the provider state + */ + reset(): Promise + + /** + * Scrolls to first diff (diff providers only, no-op for file providers) + */ + scrollToFirstDiff(): void +} diff --git a/src/integrations/editor/__tests__/EditingProviderFactory.spec.ts b/src/integrations/editor/__tests__/EditingProviderFactory.spec.ts new file mode 100644 index 0000000000..00e9dcb15b --- /dev/null +++ b/src/integrations/editor/__tests__/EditingProviderFactory.spec.ts @@ -0,0 +1,90 @@ +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest" +import * as vscode from "vscode" + +import { EditingProviderFactory } from "../EditingProviderFactory" +import { DiffViewProvider } from "../DiffViewProvider" +import { FileWriter } from "../FileWriter" + +// Mock VSCode API +const mockGet = vi.fn() +vi.mock("vscode", () => ({ + workspace: { + getConfiguration: vi.fn(() => ({ + get: mockGet, + })), + }, +})) + +// Mock the providers +vi.mock("../DiffViewProvider", () => ({ + DiffViewProvider: vi.fn(), +})) + +vi.mock("../FileWriter", () => ({ + FileWriter: vi.fn(), +})) + +describe("EditingProviderFactory", () => { + const mockCwd = "/test/cwd" + + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe("createEditingProvider", () => { + it("should create FileWriter when fileBasedEditing is enabled", () => { + mockGet.mockReturnValue(true) + + const provider = EditingProviderFactory.createEditingProvider(mockCwd) + + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline") + expect(mockGet).toHaveBeenCalledWith("fileBasedEditing", false) + expect(FileWriter).toHaveBeenCalledWith(mockCwd) + expect(DiffViewProvider).not.toHaveBeenCalled() + }) + + it("should create DiffViewProvider when fileBasedEditing is disabled", () => { + mockGet.mockReturnValue(false) + + const provider = EditingProviderFactory.createEditingProvider(mockCwd) + + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline") + expect(mockGet).toHaveBeenCalledWith("fileBasedEditing", false) + expect(DiffViewProvider).toHaveBeenCalledWith(mockCwd) + expect(FileWriter).not.toHaveBeenCalled() + }) + + it("should create DiffViewProvider when fileBasedEditing is undefined", () => { + mockGet.mockReturnValue(undefined) + + const provider = EditingProviderFactory.createEditingProvider(mockCwd) + + expect(DiffViewProvider).toHaveBeenCalledWith(mockCwd) + expect(FileWriter).not.toHaveBeenCalled() + }) + }) + + describe("isFileBasedEditingEnabled", () => { + it("should return true when fileBasedEditing is enabled", () => { + mockGet.mockReturnValue(true) + + const result = EditingProviderFactory.isFileBasedEditingEnabled() + + expect(result).toBe(true) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline") + expect(mockGet).toHaveBeenCalledWith("fileBasedEditing", false) + }) + + it("should return false when fileBasedEditing is disabled", () => { + mockGet.mockReturnValue(false) + + const result = EditingProviderFactory.isFileBasedEditingEnabled() + + expect(result).toBe(false) + }) + }) +}) diff --git a/src/integrations/editor/__tests__/FileWriter.spec.ts b/src/integrations/editor/__tests__/FileWriter.spec.ts new file mode 100644 index 0000000000..fc539cae99 --- /dev/null +++ b/src/integrations/editor/__tests__/FileWriter.spec.ts @@ -0,0 +1,207 @@ +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest" +import * as vscode from "vscode" +import * as fs from "fs/promises" +import * as path from "path" + +import { FileWriter } from "../FileWriter" +import { Task } from "../../../core/task/Task" + +// Mock VSCode API +vi.mock("vscode", () => ({ + workspace: { + getConfiguration: vi.fn(() => ({ + get: vi.fn(), + })), + }, + languages: { + getDiagnostics: vi.fn(() => []), + }, + DiagnosticSeverity: { + Error: 0, + }, +})) + +// Mock fs module +vi.mock("fs/promises", () => ({ + readFile: vi.fn(), + writeFile: vi.fn(), + unlink: vi.fn(), + rmdir: vi.fn(), +})) + +// Mock other dependencies +vi.mock("../../../utils/fs", () => ({ + createDirectoriesForFile: vi.fn(() => Promise.resolve([])), +})) + +vi.mock("../../diagnostics", () => ({ + diagnosticsToProblemsString: vi.fn(() => Promise.resolve("")), + getNewDiagnostics: vi.fn(() => []), +})) + +vi.mock("../../../utils/path", () => ({ + getReadablePath: vi.fn((cwd, relPath) => relPath), +})) + +vi.mock("../../../core/prompts/responses", () => ({ + formatResponse: { + createPrettyPatch: vi.fn(() => "mock-diff"), + }, +})) + +vi.mock("strip-bom", () => ({ + default: vi.fn((content) => content), +})) + +describe("FileWriter", () => { + let fileWriter: FileWriter + const mockCwd = "/test/cwd" + const mockTask = {} as Task + + beforeEach(() => { + vi.clearAllMocks() + fileWriter = new FileWriter(mockCwd) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe("open", () => { + it("should open an existing file for modification", async () => { + const mockContent = "existing content" + vi.mocked(fs.readFile).mockResolvedValue(mockContent) + + await fileWriter.open("test.txt") + + expect(fileWriter.editType).toBe("modify") + expect(fileWriter.originalContent).toBe(mockContent) + expect(fileWriter.isEditing).toBe(true) + }) + + it("should open a new file for creation", async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error("File not found")) + + await fileWriter.open("new-file.txt") + + expect(fileWriter.editType).toBe("create") + expect(fileWriter.originalContent).toBe("") + expect(fileWriter.isEditing).toBe(true) + }) + + it("should handle viewColumn parameter (ignored)", async () => { + vi.mocked(fs.readFile).mockResolvedValue("content") + + await expect(fileWriter.open("test.txt")).resolves.toBeUndefined() + }) + }) + + describe("update", () => { + beforeEach(async () => { + vi.mocked(fs.readFile).mockResolvedValue("original content") + await fileWriter.open("test.txt") + }) + + it("should write final content to file", async () => { + const content = "new content" + + await fileWriter.update(content, true) + + expect(fs.writeFile).toHaveBeenCalledWith(path.resolve(mockCwd, "test.txt"), content, "utf-8") + }) + + it("should preserve empty last line if original content had one", async () => { + fileWriter.originalContent = "content\n" + const content = "new content" + + await fileWriter.update(content, true) + + expect(fs.writeFile).toHaveBeenCalledWith(path.resolve(mockCwd, "test.txt"), "new content\n", "utf-8") + }) + + it("should not write to file if not final", async () => { + await fileWriter.update("content", false) + + expect(fs.writeFile).not.toHaveBeenCalled() + }) + }) + + describe("saveChanges", () => { + beforeEach(async () => { + vi.mocked(fs.readFile).mockResolvedValue("original content") + await fileWriter.open("test.txt") + await fileWriter.update("new content", true) + }) + + it("should return save results", async () => { + vi.mocked(fs.readFile).mockResolvedValue("new content") + + const result = await fileWriter.saveChanges() + + expect(result).toEqual({ + newProblemsMessage: "", + userEdits: undefined, + finalContent: "new content", + }) + }) + }) + + describe("pushToolWriteResult", () => { + beforeEach(async () => { + vi.mocked(fs.readFile).mockResolvedValue("content") + await fileWriter.open("test.txt") + }) + + it("should return formatted XML response", async () => { + const result = await fileWriter.pushToolWriteResult(mockTask, mockCwd, false) + + expect(result).toContain("") + expect(result).toContain("test.txt") + expect(result).toContain("modified") + }) + + it("should handle new file creation", async () => { + const result = await fileWriter.pushToolWriteResult(mockTask, mockCwd, true) + + expect(result).toContain("created") + }) + }) + + describe("revertChanges", () => { + it("should delete new file and directories", async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error("File not found")) + await fileWriter.open("new-file.txt") + + await fileWriter.revertChanges() + + expect(fs.unlink).toHaveBeenCalled() + }) + + it("should restore original content for existing file", async () => { + const originalContent = "original content" + vi.mocked(fs.readFile).mockResolvedValue(originalContent) + await fileWriter.open("existing-file.txt") + + await fileWriter.revertChanges() + + expect(fs.writeFile).toHaveBeenCalledWith( + path.resolve(mockCwd, "existing-file.txt"), + originalContent, + "utf-8", + ) + }) + }) + + describe("reset", () => { + it("should reset all state", async () => { + vi.mocked(fs.readFile).mockResolvedValue("content") + await fileWriter.open("test.txt") + + await fileWriter.reset() + + expect(fileWriter.editType).toBeUndefined() + expect(fileWriter.isEditing).toBe(false) + expect(fileWriter.originalContent).toBeUndefined() + }) + }) +}) diff --git a/src/package.json b/src/package.json index 5e3cd3bc53..1e6d494bfd 100644 --- a/src/package.json +++ b/src/package.json @@ -386,6 +386,11 @@ "type": "string", "default": "", "description": "%settings.autoImportSettingsPath.description%" + }, + "roo-cline.fileBasedEditing": { + "type": "boolean", + "default": false, + "description": "%roo-cline.fileBasedEditing.description%" } } } diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 4f2aa2da15..0ee6b6a014 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" 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/ApiOptions.tsx b/webview-ui/src/components/settings/ApiOptions.tsx index 06994b16b9..f3c9ba6681 100644 --- a/webview-ui/src/components/settings/ApiOptions.tsx +++ b/webview-ui/src/components/settings/ApiOptions.tsx @@ -77,7 +77,6 @@ import { inputEventTransform, noTransform } from "./transforms" import { ModelInfoView } from "./ModelInfoView" import { ApiErrorMessage } from "./ApiErrorMessage" import { ThinkingBudget } from "./ThinkingBudget" -import { DiffSettingsControl } from "./DiffSettingsControl" import { TemperatureControl } from "./TemperatureControl" import { RateLimitSecondsControl } from "./RateLimitSecondsControl" import { ConsecutiveMistakeLimitControl } from "./ConsecutiveMistakeLimitControl" @@ -564,11 +563,6 @@ const ApiOptions = ({ {t("settings:advancedSettings.title")} - setApiConfigurationField(field, value)} - /> void -} - -export const DiffSettingsControl: React.FC = ({ - diffEnabled = true, - fuzzyMatchThreshold = 1.0, - onChange, -}) => { - const { t } = useAppTranslation() - - const handleDiffEnabledChange = useCallback( - (e: any) => { - onChange("diffEnabled", e.target.checked) - }, - [onChange], - ) - - const handleThresholdChange = useCallback( - (newValue: number[]) => { - onChange("fuzzyMatchThreshold", newValue[0]) - }, - [onChange], - ) - - return ( -
-
- - {t("settings:advanced.diff.label")} - -
- {t("settings:advanced.diff.description")} -
-
- - {diffEnabled && ( -
-
- -
- - {Math.round(fuzzyMatchThreshold * 100)}% -
-
- {t("settings:advanced.diff.matchPrecision.description")} -
-
-
- )} -
- ) -} diff --git a/webview-ui/src/components/settings/FileEditingOptions.tsx b/webview-ui/src/components/settings/FileEditingOptions.tsx new file mode 100644 index 0000000000..edc8fc11fd --- /dev/null +++ b/webview-ui/src/components/settings/FileEditingOptions.tsx @@ -0,0 +1,189 @@ +import React, { useCallback } from "react" +import { Slider } from "@/components/ui" +import { useAppTranslation } from "@/i18n/TranslationContext" +import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" +import { SectionHeader } from "@/components/settings/SectionHeader" +import { Section } from "@/components/settings/Section" +import { FileDiff, Settings2 } from "lucide-react" + +type FileEditingOptionsField = "diffEnabled" | "fuzzyMatchThreshold" | "fileBasedEditing" + +interface FileEditingOptionsProps { + diffEnabled?: boolean + fuzzyMatchThreshold?: number + fileBasedEditing?: boolean + onChange: (field: FileEditingOptionsField, value: any) => void +} + +interface DiffPrecisionMatchControlProps { + fuzzyMatchThreshold: number + disabled: boolean + onValueChange: (newValue: number[]) => void +} + +interface FileBasedEditingControlProps { + fileBasedEditing: boolean + onChange: (e: any) => void +} + +/** + * Control for diff precision match threshold + */ +const DiffPrecisionMatchControl: React.FC = ({ + fuzzyMatchThreshold, + disabled, + onValueChange, +}) => { + const { t } = useAppTranslation() + return ( +
+
+ +
+ + {Math.round(fuzzyMatchThreshold * 100)}% +
+
+ {t("settings:advanced.diff.matchPrecision.description")} +
+
+
+ ) +} + +/** + * Control for enabling file-based editing mode + */ +const FileBasedEditingControl: React.FC = ({ fileBasedEditing, onChange }) => { + const { t } = useAppTranslation() + return ( +
+ + {t("settings:advanced.fileEditing.fileBasedEditing.label")} + +
+ {t("settings:advanced.fileEditing.fileBasedEditing.description")} +
+
+ ) +} + +/** + * File editing options control component with mutual exclusivity logic + */ +export const FileEditingOptions: React.FC = ({ + diffEnabled = true, + fuzzyMatchThreshold = 1.0, + fileBasedEditing = false, + onChange, +}) => { + const { t } = useAppTranslation() + + // When file-based editing is enabled, diff settings should be disabled + const isDiffDisabled = fileBasedEditing + + const resetAllButFileBasedEditing = useCallback(() => { + onChange("diffEnabled", false) + onChange("fileBasedEditing", true) + }, [onChange]) + + const handleDiffEnabledChange = useCallback( + (e: any) => { + onChange("diffEnabled", e.target.checked) + // if diffEnabled is checked, uncheck fileBasedEditing + if (e.target.checked) { + onChange("fileBasedEditing", false) + } + }, + [onChange], + ) + + const handleThresholdChange = useCallback( + (newValue: number[]) => { + onChange("fuzzyMatchThreshold", newValue[0]) + }, + [onChange], + ) + + const handleFileBasedEditingChange = useCallback( + (e: any) => { + if (e.target.checked) { + // if we enable file-based editing, we reset all other settings + resetAllButFileBasedEditing() + } else { + // if we disable file-based editing, we reset only the file-based editing setting and set diffEnabled to true + onChange("fileBasedEditing", false) + } + }, + [onChange, resetAllButFileBasedEditing], + ) + + return ( +
+ {/* File editing options section */} + +
+ +
{t("settings:sections.editingType")}
+
+ + +
+
+ + {t("settings:advanced.diff.label")} + +
+ {t("settings:advanced.diff.description")} +
+
+
+ +
+
+ + {/* Diff settings section */} + +
+ +
{t("settings:sections.diffSettings")}
+
+ + +
+ {/* Diff settings section */} +
+ {diffEnabled && !isDiffDisabled && ( + <> + + + )} + {isDiffDisabled && ( +
+ {t("settings:advanced.fileEditing.exclusivityNotice")} +
+ )} + {!diffEnabled && ( +
+ {t("settings:advanced.diff.disabledNotice")} +
+ )} +
+
+
+ ) +} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 517c1c159d..fd25279d94 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -18,6 +18,7 @@ import { Database, SquareTerminal, FlaskConical, + Pencil, AlertTriangle, Globe, Info, @@ -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" @@ -80,6 +82,7 @@ export interface SettingsViewRef { const sectionNames = [ "providers", + "fileEditing", "autoApprove", "browser", "checkpoints", @@ -141,6 +144,7 @@ const SettingsView = forwardRef(({ onDone, t browserViewportSize, enableCheckpoints, diffEnabled, + fileBasedEditing, experiments, fuzzyMatchThreshold, maxOpenTabsContext, @@ -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 }) @@ -403,6 +408,7 @@ const SettingsView = forwardRef(({ onDone, t const sections: { id: SectionName; icon: LucideIcon }[] = useMemo( () => [ { id: "providers", icon: Webhook }, + { id: "fileEditing", icon: Pencil }, { id: "autoApprove", icon: CheckCheck }, { id: "browser", icon: SquareMousePointer }, { id: "checkpoints", icon: GitBranch }, @@ -598,6 +604,25 @@ const SettingsView = forwardRef(({ onDone, t )} + {/* File Editing Section */} + {activeTab === "fileEditing" && ( +
+ +
+ +
{t("settings:sections.fileEditing")}
+
+ + + +
+ )} + {/* Auto-Approve Section */} {activeTab === "autoApprove" && ( ({ })) // Mock DiffSettingsControl for tests -vi.mock("../DiffSettingsControl", () => ({ +vi.mock("../FileEditingOptions", () => ({ DiffSettingsControl: ({ diffEnabled, fuzzyMatchThreshold, onChange }: any) => (