Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),

Expand Down
37 changes: 30 additions & 7 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -172,7 +174,7 @@ export class Task extends EventEmitter<ClineEvents> {
browserSession: BrowserSession

// Editing
diffViewProvider: DiffViewProvider
editingProvider: IEditingProvider
diffStrategy?: DiffStrategy
diffEnabled: boolean = false
fuzzyMatchThreshold: number
Expand Down Expand Up @@ -260,7 +262,28 @@ export class Task extends EventEmitter<ClineEvents> {
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
Expand Down Expand Up @@ -1066,8 +1089,8 @@ export class Task extends EventEmitter<ClineEvents> {

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)
Expand Down Expand Up @@ -1296,8 +1319,8 @@ export class Task extends EventEmitter<ClineEvents> {
}

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
Expand Down Expand Up @@ -1349,7 +1372,7 @@ export class Task extends EventEmitter<ClineEvents> {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("applyDiffTool experiment routing", () => {
applyDiff: vi.fn(),
getProgressStatus: vi.fn(),
},
diffViewProvider: {
editingProvider: {
reset: vi.fn(),
},
api: {
Expand Down
24 changes: 12 additions & 12 deletions src/core/tools/__tests__/insertContentTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe("insertContentTool", () => {
rooIgnoreController: {
validateAccess: vi.fn().mockReturnValue(true),
},
diffViewProvider: {
editingProvider: {
editType: undefined,
isEditing: false,
originalContent: "",
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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()
})
})
})
46 changes: 23 additions & 23 deletions src/core/tools/__tests__/writeToFileTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe("writeToFileTool", () => {
mockCline.rooIgnoreController = {
validateAccess: vi.fn().mockReturnValue(true),
}
mockCline.diffViewProvider = {
mockCline.editingProvider = {
editType: undefined,
isEditing: false,
originalContent: "",
Expand Down Expand Up @@ -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)
})
})

Expand All @@ -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({})

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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)
})
})

Expand All @@ -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)
})
Expand All @@ -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)
})
})

Expand All @@ -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 })

Expand All @@ -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()
})
})
})
20 changes: 11 additions & 9 deletions src/core/tools/applyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand All @@ -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) {
Expand All @@ -191,21 +193,21 @@ 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)
} else {
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
}
}
Loading
Loading