From bdcee809b93035d4ef3f8ad18ce0add9c4e49350 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 23 Jul 2025 05:55:42 +0000 Subject: [PATCH] feat: add experimental setting to prevent editor focus disruption - Add experimentalPreventFocusDisruption setting to package.json - Update DiffViewProvider to respect the new setting when opening diff views - Add localization entry for the new setting - Add comprehensive tests for the new functionality Fixes #4784 --- src/integrations/editor/DiffViewProvider.ts | 48 ++++- .../editor/__tests__/DiffViewProvider.spec.ts | 195 +++++++++++++++++- src/package.json | 5 + src/package.nls.json | 3 +- 4 files changed, 236 insertions(+), 15 deletions(-) diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index f4133029c9..dbef4539bd 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -13,6 +13,7 @@ import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics" import { ClineSayTool } from "../../shared/ExtensionMessage" import { Task } from "../../core/task/Task" import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" +import { Package } from "../../shared/package" import { DecorationController } from "./DecorationController" @@ -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 @@ -198,7 +202,15 @@ export class DiffViewProvider { await updatedDocument.save() } - await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true }) + // Check if the experimental setting is enabled + const preventFocusDisruption = vscode.workspace + .getConfiguration(Package.name) + .get("experimentalPreventFocusDisruption", false) + + await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { + preview: false, + preserveFocus: preventFocusDisruption, + }) await this.closeAllDiffViews() // Getting diagnostics before and after the file edit is a better approach than @@ -216,22 +228,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( @@ -388,9 +400,14 @@ export class DiffViewProvider { await updatedDocument.save() if (this.documentWasOpen) { + // Check if the experimental setting is enabled + const preventFocusDisruption = vscode.workspace + .getConfiguration(Package.name) + .get("experimentalPreventFocusDisruption", false) + await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, - preserveFocus: true, + preserveFocus: preventFocusDisruption, }) } @@ -444,6 +461,11 @@ export class DiffViewProvider { const uri = vscode.Uri.file(path.resolve(this.cwd, this.relPath)) + // Check if the experimental setting is enabled + const preventFocusDisruption = vscode.workspace + .getConfiguration(Package.name) + .get("experimentalPreventFocusDisruption", false) + // If this diff editor is already open (ie if a previous write file was // interrupted) then we should activate that instead of opening a new // diff. @@ -457,7 +479,9 @@ export class DiffViewProvider { ) if (diffTab && diffTab.input instanceof vscode.TabInputTextDiff) { - const editor = await vscode.window.showTextDocument(diffTab.input.modified, { preserveFocus: true }) + const editor = await vscode.window.showTextDocument(diffTab.input.modified, { + preserveFocus: preventFocusDisruption, + }) return editor } @@ -523,7 +547,11 @@ export class DiffViewProvider { // Pre-open the file as a text document to ensure it doesn't open in preview mode // This fixes issues with files that have custom editor associations (like markdown preview) vscode.window - .showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true }) + .showTextDocument(uri, { + preview: false, + viewColumn: vscode.ViewColumn.Active, + preserveFocus: preventFocusDisruption, + }) .then(() => { // Execute the diff command after ensuring the file is open as text return vscode.commands.executeCommand( @@ -533,7 +561,7 @@ export class DiffViewProvider { }), uri, `${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`, - { preserveFocus: true }, + { preserveFocus: preventFocusDisruption }, ) }) .then( diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts index a4aded95bb..1e35c64b95 100644 --- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts +++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts @@ -34,6 +34,9 @@ vi.mock("vscode", () => ({ fs: { stat: vi.fn(), }, + getConfiguration: vi.fn(() => ({ + get: vi.fn().mockReturnValue(false), // Default value for experimentalPreventFocusDisruption + })), }, window: { createTextEditorDecorationType: vi.fn(), @@ -81,6 +84,7 @@ vi.mock("vscode", () => ({ InCenter: 2, }, TabInputTextDiff: class TabInputTextDiff {}, + TabInputText: class TabInputText {}, Uri: { file: vi.fn((path) => ({ fsPath: path })), parse: vi.fn((uri) => ({ with: vi.fn(() => ({})) })), @@ -188,7 +192,7 @@ describe("DiffViewProvider", () => { // Mock showTextDocument to track when it's called vi.mocked(vscode.window.showTextDocument).mockImplementation(async (uri, options) => { callOrder.push("showTextDocument") - expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true }) + expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: false }) return mockEditor as any }) @@ -220,10 +224,10 @@ describe("DiffViewProvider", () => { // Verify that showTextDocument was called before executeCommand expect(callOrder).toEqual(["showTextDocument", "executeCommand"]) - // Verify that showTextDocument was called with preview: false and preserveFocus: true + // Verify that showTextDocument was called with preview: false and preserveFocus: false (default) expect(vscode.window.showTextDocument).toHaveBeenCalledWith( expect.objectContaining({ fsPath: `${mockCwd}/test.md` }), - { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true }, + { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: false }, ) // Verify that the diff command was executed @@ -232,7 +236,7 @@ describe("DiffViewProvider", () => { expect.any(Object), expect.any(Object), `test.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`, - { preserveFocus: true }, + { preserveFocus: false }, ) }) @@ -418,4 +422,187 @@ describe("DiffViewProvider", () => { expect(vscode.languages.getDiagnostics).toHaveBeenCalled() }) }) + + describe("experimentalPreventFocusDisruption setting", () => { + it("should preserve focus when experimentalPreventFocusDisruption is enabled", async () => { + // Mock the configuration to return true for experimentalPreventFocusDisruption + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockReturnValue(true), + } as any) + + // Setup mock editor + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/test.ts` }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { + active: { line: 0, character: 0 }, + anchor: { line: 0, character: 0 }, + }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + } + + // Mock showTextDocument + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any) + + // Mock workspace.onDidOpenTextDocument + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => { + callback({ uri: { fsPath: `${mockCwd}/test.ts` } } as any) + }, 0) + return { dispose: vi.fn() } + }) + + // Mock window.visibleTextEditors + vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any] + + // Set up for file + ;(diffViewProvider as any).editType = "modify" + + // Execute open + await diffViewProvider.open("test.ts") + + // Verify that showTextDocument was called with preserveFocus: true + expect(vscode.window.showTextDocument).toHaveBeenCalledWith( + expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }), + { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true }, + ) + + // Verify that the diff command was executed with preserveFocus: true + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "vscode.diff", + expect.any(Object), + expect.any(Object), + expect.any(String), + { preserveFocus: true }, + ) + }) + + it("should not preserve focus when experimentalPreventFocusDisruption is disabled", async () => { + // Mock the configuration to return false for experimentalPreventFocusDisruption + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockReturnValue(false), + } as any) + + // Setup mock editor + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/test.ts` }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { + active: { line: 0, character: 0 }, + anchor: { line: 0, character: 0 }, + }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + } + + // Mock showTextDocument + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any) + + // Mock workspace.onDidOpenTextDocument + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => { + callback({ uri: { fsPath: `${mockCwd}/test.ts` } } as any) + }, 0) + return { dispose: vi.fn() } + }) + + // Mock window.visibleTextEditors + vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any] + + // Set up for file + ;(diffViewProvider as any).editType = "modify" + + // Execute open + await diffViewProvider.open("test.ts") + + // Verify that showTextDocument was called with preserveFocus: false + expect(vscode.window.showTextDocument).toHaveBeenCalledWith( + expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }), + { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: false }, + ) + + // Verify that the diff command was executed with preserveFocus: false + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + "vscode.diff", + expect.any(Object), + expect.any(Object), + expect.any(String), + { preserveFocus: false }, + ) + }) + + it("should preserve focus in saveChanges when experimentalPreventFocusDisruption is enabled", async () => { + // Mock the configuration to return true + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockReturnValue(true), + } as any) + + // Setup for saveChanges + ;(diffViewProvider as any).relPath = "test.ts" + ;(diffViewProvider as any).newContent = "new content" + ;(diffViewProvider as any).activeDiffEditor = { + document: { + getText: vi.fn().mockReturnValue("new content"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + }, + } + ;(diffViewProvider as any).preDiagnostics = [] + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + + // Mock vscode functions + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any) + vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([]) + + await diffViewProvider.saveChanges(false) // Disable diagnostics for simplicity + + // Verify showTextDocument was called with preserveFocus: true + expect(vscode.window.showTextDocument).toHaveBeenCalledWith( + expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }), + { preview: false, preserveFocus: true }, + ) + }) + + it("should preserve focus in revertChanges when experimentalPreventFocusDisruption is enabled", async () => { + // Mock the configuration to return true + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn().mockReturnValue(true), + } as any) + + // Setup for revertChanges + ;(diffViewProvider as any).relPath = "test.ts" + ;(diffViewProvider as any).editType = "modify" + ;(diffViewProvider as any).documentWasOpen = true + ;(diffViewProvider as any).originalContent = "original content" + ;(diffViewProvider as any).activeDiffEditor = { + document: { + uri: { fsPath: `${mockCwd}/test.ts` }, + getText: vi.fn().mockReturnValue("modified content"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }), + }, + } + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).reset = vi.fn().mockResolvedValue(undefined) + + // Mock vscode functions + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any) + + await diffViewProvider.revertChanges() + + // Verify showTextDocument was called with preserveFocus: true + expect(vscode.window.showTextDocument).toHaveBeenCalledWith( + expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }), + { preview: false, preserveFocus: true }, + ) + }) + }) }) diff --git a/src/package.json b/src/package.json index cfdec57bde..5f9dc21f45 100644 --- a/src/package.json +++ b/src/package.json @@ -386,6 +386,11 @@ "type": "string", "default": "", "description": "%settings.autoImportSettingsPath.description%" + }, + "roo-cline.experimentalPreventFocusDisruption": { + "type": "boolean", + "default": false, + "description": "%settings.experimentalPreventFocusDisruption.description%" } } } diff --git a/src/package.nls.json b/src/package.nls.json index 1eb294ca44..825e3a91f0 100644 --- a/src/package.nls.json +++ b/src/package.nls.json @@ -36,5 +36,6 @@ "settings.vsCodeLmModelSelector.family.description": "The family of the language model (e.g. gpt-4)", "settings.customStoragePath.description": "Custom storage path. Leave empty to use the default location. Supports absolute paths (e.g. 'D:\\RooCodeStorage')", "settings.enableCodeActions.description": "Enable Roo Code quick fixes", - "settings.autoImportSettingsPath.description": "Path to a RooCode configuration file to automatically import on extension startup. Supports absolute paths and paths relative to the home directory (e.g. '~/Documents/roo-code-settings.json'). Leave empty to disable auto-import." + "settings.autoImportSettingsPath.description": "Path to a RooCode configuration file to automatically import on extension startup. Supports absolute paths and paths relative to the home directory (e.g. '~/Documents/roo-code-settings.json'). Leave empty to disable auto-import.", + "settings.experimentalPreventFocusDisruption.description": "(Experimental) Prevent file edits from stealing focus. When enabled, diff views and file edits will not disrupt your current work. Files will update in the background without forcing you to switch context." }