Feature/2122 editor focus

.changeset/large-snakes-suffer.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Added an editable setting to allow users to preserve focus on the tab they are in while roo code is opening new tabs to do what it has to do.

packages/types/src/global-settings.ts

@@ -84,8 +84,16 @@ export const globalSettingsSchema = z.object({
 	terminalCompressProgressBar: z.boolean().optional(),
 
 	rateLimitSeconds: z.number().optional(),
+
 	diffEnabled: z.boolean().optional(),
+	diffViewAutoFocus: z.boolean().optional(),
+	autoCloseRooTabs: z.boolean().optional(),
+	autoCloseAllRooTabs: z.boolean().optional(),
+	fileBasedEditing: z.boolean().optional(),
+	openTabsInCorrectGroup: z.boolean().optional(),
+	openTabsAtEndOfList: z.boolean().optional(),
 	fuzzyMatchThreshold: z.number().optional(),
+
 	experiments: experimentsSchema.optional(),
 
 	codebaseIndexModels: codebaseIndexModelsSchema.optional(),
@@ -220,6 +228,9 @@ export const EVALS_SETTINGS: RooCodeSettings = {
 	terminalShellIntegrationDisabled: true,
 
 	diffEnabled: true,
+	fileBasedEditing: false,
+	openTabsInCorrectGroup: false,
+	openTabsAtEndOfList: false,
 	fuzzyMatchThreshold: 1,
 
 	enableCheckpoints: false,

packages/types/src/provider-settings.ts

@@ -55,8 +55,6 @@ export type ProviderSettingsEntry = z.infer<typeof providerSettingsEntrySchema>
 
 const baseProviderSettingsSchema = z.object({
 	includeMaxTokens: z.boolean().optional(),
-	diffEnabled: z.boolean().optional(),
-	fuzzyMatchThreshold: z.number().optional(),
 	modelTemperature: z.number().nullish(),
 	rateLimitSeconds: z.number().optional(),
 

src/core/config/ProviderSettingsManager.ts

@@ -123,12 +123,6 @@ export class ProviderSettingsManager {
 					isDirty = true
 				}
 
-				if (!providerProfiles.migrations.diffSettingsMigrated) {
-					await this.migrateDiffSettings(providerProfiles)
-					providerProfiles.migrations.diffSettingsMigrated = true
-					isDirty = true
-				}
-
 				if (!providerProfiles.migrations.openAiHeadersMigrated) {
 					await this.migrateOpenAiHeaders(providerProfiles)
 					providerProfiles.migrations.openAiHeadersMigrated = true
@@ -169,41 +163,6 @@ export class ProviderSettingsManager {
 		}
 	}
 
-	private async migrateDiffSettings(providerProfiles: ProviderProfiles) {
-		try {
-			let diffEnabled: boolean | undefined
-			let fuzzyMatchThreshold: number | undefined
-
-			try {
-				diffEnabled = await this.context.globalState.get<boolean>("diffEnabled")
-				fuzzyMatchThreshold = await this.context.globalState.get<number>("fuzzyMatchThreshold")
-			} catch (error) {
-				console.error("[MigrateDiffSettings] Error getting global diff settings:", error)
-			}
-
-			if (diffEnabled === undefined) {
-				// Failed to get the existing value, use the default.
-				diffEnabled = true
-			}
-
-			if (fuzzyMatchThreshold === undefined) {
-				// Failed to get the existing value, use the default.
-				fuzzyMatchThreshold = 1.0
-			}
-
-			for (const [_name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
-				if (apiConfig.diffEnabled === undefined) {
-					apiConfig.diffEnabled = diffEnabled
-				}
-				if (apiConfig.fuzzyMatchThreshold === undefined) {
-					apiConfig.fuzzyMatchThreshold = fuzzyMatchThreshold
-				}
-			}
-		} catch (error) {
-			console.error(`[MigrateDiffSettings] Failed to migrate diff settings:`, error)
-		}
-	}
-
 	private async migrateOpenAiHeaders(providerProfiles: ProviderProfiles) {
 		try {
 			for (const [_name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {

src/core/task/Task.ts

@@ -48,7 +48,8 @@ import { McpServerManager } from "../../services/mcp/McpServerManager"
 import { RepoPerTaskCheckpointService } from "../../services/checkpoints"
 
 // integrations
-import { DiffViewProvider } from "../../integrations/editor/DiffViewProvider"
+import { IEditingProvider } from "../../integrations/editor/IEditingProvider"
+import { EditingProviderFactory } from "../../integrations/editor/EditingProviderFactory"
 import { findToolName, formatContentBlockToMarkdown } from "../../integrations/misc/export-markdown"
 import { RooTerminalProcess } from "../../integrations/terminal/types"
 import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
@@ -165,7 +166,7 @@ export class Task extends EventEmitter<ClineEvents> {
 	browserSession: BrowserSession
 
 	// Editing
-	diffViewProvider: DiffViewProvider
+	editingProvider: IEditingProvider
 	diffStrategy?: DiffStrategy
 	diffEnabled: boolean = false
 	fuzzyMatchThreshold: number
@@ -253,7 +254,7 @@ export class Task extends EventEmitter<ClineEvents> {
 		this.consecutiveMistakeLimit = consecutiveMistakeLimit
 		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
@@ -754,6 +755,7 @@ export class Task extends EventEmitter<ClineEvents> {
 
 	public async resumePausedTask(lastMessage: string) {
 		// Release this Cline instance from paused state.
+		this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd)
 		this.isPaused = false
 		this.emit("taskUnpaused")
 
@@ -1058,8 +1060,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)
@@ -1113,6 +1115,9 @@ export class Task extends EventEmitter<ClineEvents> {
 	private async initiateTaskLoop(userContent: Anthropic.Messages.ContentBlockParam[]): Promise<void> {
 		// Kicks off the checkpoints initialization process in the background.
 		getCheckpointService(this)
+		// Lets track if the user is interacting with the editor after we start our task loop.
+		this.editingProvider.initialize()
+		this.editingProvider.disableAutoFocusAfterUserInteraction?.()
 
 		let nextUserContent = userContent
 		let includeFileDetails = true
@@ -1153,6 +1158,8 @@ export class Task extends EventEmitter<ClineEvents> {
 			throw new Error(`[RooCode#recursivelyMakeRooRequests] task ${this.taskId}.${this.instanceId} aborted`)
 		}
 
+		this.editingProvider = EditingProviderFactory.resetAndCreateNewEditingProvider(this.cwd, this.editingProvider)
+
 		if (this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
 			const { response, text, images } = await this.ask(
 				"mistake_limit_reached",
@@ -1280,8 +1287,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
@@ -1333,7 +1340,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
@@ -1716,7 +1723,9 @@ export class Task extends EventEmitter<ClineEvents> {
 
 			const contextWindow = modelInfo.contextWindow
 
-			const currentProfileId = state?.listApiConfigMeta.find((profile) => profile.name === state?.currentApiConfigName)?.id ?? "default";
+			const currentProfileId =
+				state?.listApiConfigMeta.find((profile) => profile.name === state?.currentApiConfigName)?.id ??
+				"default"
 
 			const truncateResult = await truncateConversationIfNeeded({
 				messages: this.apiConversationHistory,

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: {

src/core/tools/__tests__/writeToFileTool.spec.ts

@@ -92,6 +92,8 @@ vi.mock("../../ignore/RooIgnoreController", () => ({
 	},
 }))
 
+const MOCK_VIEW_COLUMN = 1
+
 describe("writeToFileTool", () => {
 	// Test data
 	const testFilePath = "test/file.txt"
@@ -112,7 +114,6 @@ describe("writeToFileTool", () => {
 	const mockCline: any = {}
 	let mockAskApproval: ReturnType<typeof vi.fn>
 	let mockHandleError: ReturnType<typeof vi.fn>
-	let mockPushToolResult: ReturnType<typeof vi.fn>
 	let mockRemoveClosingTag: ReturnType<typeof vi.fn>
 	let toolResult: ToolResponse | undefined
 
@@ -135,7 +136,7 @@ describe("writeToFileTool", () => {
 		mockCline.rooIgnoreController = {
 			validateAccess: vi.fn().mockReturnValue(true),
 		}
-		mockCline.diffViewProvider = {
+		mockCline.editingProvider = {
 			editType: undefined,
 			isEditing: false,
 			originalContent: "",
@@ -168,6 +169,7 @@ describe("writeToFileTool", () => {
 				}
 				return "Tool result message"
 			}),
+			resetWithListeners: vi.fn().mockResolvedValue(undefined),
 		}
 		mockCline.api = {
 			getModel: vi.fn().mockReturnValue({ id: "claude-3" }),
@@ -179,6 +181,11 @@ describe("writeToFileTool", () => {
 		mockCline.ask = vi.fn().mockResolvedValue(undefined)
 		mockCline.recordToolError = vi.fn()
 		mockCline.sayAndCreateMissingParamError = vi.fn().mockResolvedValue("Missing param error")
+		mockCline.providerRef = {
+			deref: vi.fn().mockReturnValue({
+				getViewColumn: vi.fn().mockReturnValue(MOCK_VIEW_COLUMN),
+			}),
+		}
 
 		mockAskApproval = vi.fn().mockResolvedValue(true)
 		mockHandleError = vi.fn().mockResolvedValue(undefined)
@@ -238,7 +245,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, MOCK_VIEW_COLUMN)
 		})
 	})
 
@@ -247,18 +254,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({})
 
@@ -270,13 +277,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 () => {
@@ -304,7 +311,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)
 		})
 	})
 
@@ -313,10 +320,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, MOCK_VIEW_COLUMN)
+			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)
 		})
@@ -341,21 +348,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, MOCK_VIEW_COLUMN)
+			expect(mockCline.editingProvider.update).toHaveBeenCalledWith(testContent, false)
 		})
 	})
 
@@ -365,19 +372,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",
 			})
 			// Manually set the property on the mock instance because the original saveChanges is not called
-			mockCline.diffViewProvider.userEdits = userEditsValue
+			mockCline.editingProvider.userEdits = userEditsValue
 
 			await executeWriteFileTool({}, { fileExists: true })
 
@@ -390,21 +397,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.resetWithListeners).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.resetWithListeners).toHaveBeenCalled()
 		})
 	})
 })