Skip to content

Commit e4a3e79

Browse files
committed
feat: add preventFocusDisruption setting to prevent editor focus stealing
- Add new configuration property roo-cline.preventFocusDisruption (default: false) - Update DiffViewProvider to check setting before showing documents or focusing - Preserve focus when opening diff views if setting is enabled - Add comprehensive tests for the new functionality - Add localization entry for the setting description Fixes #4784
1 parent 2eb586b commit e4a3e79

File tree

4 files changed

+213
-14
lines changed

4 files changed

+213
-14
lines changed

src/integrations/editor/DiffViewProvider.ts

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
1313
import { ClineSayTool } from "../../shared/ExtensionMessage"
1414
import { Task } from "../../core/task/Task"
1515
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
16+
import { Package } from "../../shared/package"
1617

1718
import { DecorationController } from "./DecorationController"
1819

@@ -181,7 +182,10 @@ export class DiffViewProvider {
181182
}
182183
}
183184

184-
async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
185+
async saveChanges(
186+
diagnosticsEnabled: boolean = true,
187+
writeDelayMs: number = DEFAULT_WRITE_DELAY_MS,
188+
): Promise<{
185189
newProblemsMessage: string | undefined
186190
userEdits: string | undefined
187191
finalContent: string | undefined
@@ -198,7 +202,16 @@ export class DiffViewProvider {
198202
await updatedDocument.save()
199203
}
200204

201-
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true })
205+
// Check if focus disruption prevention is enabled
206+
const preventFocusDisruption = vscode.workspace
207+
.getConfiguration(Package.name)
208+
.get<boolean>("preventFocusDisruption", false)
209+
210+
if (!preventFocusDisruption) {
211+
// Original behavior: show the document
212+
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true })
213+
}
214+
202215
await this.closeAllDiffViews()
203216

204217
// Getting diagnostics before and after the file edit is a better approach than
@@ -216,22 +229,22 @@ export class DiffViewProvider {
216229
// and can address them accordingly. If problems don't change immediately after
217230
// applying a fix, won't be notified, which is generally fine since the
218231
// initial fix is usually correct and it may just take time for linters to catch up.
219-
232+
220233
let newProblemsMessage = ""
221-
234+
222235
if (diagnosticsEnabled) {
223236
// Add configurable delay to allow linters time to process and clean up issues
224237
// like unused imports (especially important for Go and other languages)
225238
// Ensure delay is non-negative
226239
const safeDelayMs = Math.max(0, writeDelayMs)
227-
240+
228241
try {
229242
await delay(safeDelayMs)
230243
} catch (error) {
231244
// Log error but continue - delay failure shouldn't break the save operation
232245
console.warn(`Failed to apply write delay: ${error}`)
233246
}
234-
247+
235248
const postDiagnostics = vscode.languages.getDiagnostics()
236249

237250
const newProblems = await diagnosticsToProblemsString(
@@ -387,7 +400,12 @@ export class DiffViewProvider {
387400
await vscode.workspace.applyEdit(edit)
388401
await updatedDocument.save()
389402

390-
if (this.documentWasOpen) {
403+
// Check if focus disruption prevention is enabled
404+
const preventFocusDisruption = vscode.workspace
405+
.getConfiguration(Package.name)
406+
.get<boolean>("preventFocusDisruption", false)
407+
408+
if (this.documentWasOpen && !preventFocusDisruption) {
391409
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
392410
preview: false,
393411
preserveFocus: true,
@@ -520,10 +538,19 @@ export class DiffViewProvider {
520538
}),
521539
)
522540

541+
// Check if focus disruption prevention is enabled
542+
const preventFocusDisruption = vscode.workspace
543+
.getConfiguration(Package.name)
544+
.get<boolean>("preventFocusDisruption", false)
545+
523546
// Pre-open the file as a text document to ensure it doesn't open in preview mode
524547
// This fixes issues with files that have custom editor associations (like markdown preview)
525548
vscode.window
526-
.showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true })
549+
.showTextDocument(uri, {
550+
preview: false,
551+
viewColumn: vscode.ViewColumn.Active,
552+
preserveFocus: preventFocusDisruption, // Preserve focus if setting is enabled
553+
})
527554
.then(() => {
528555
// Execute the diff command after ensuring the file is open as text
529556
return vscode.commands.executeCommand(
@@ -533,7 +560,7 @@ export class DiffViewProvider {
533560
}),
534561
uri,
535562
`${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`,
536-
{ preserveFocus: true },
563+
{ preserveFocus: preventFocusDisruption }, // Preserve focus if setting is enabled
537564
)
538565
})
539566
.then(

src/integrations/editor/__tests__/DiffViewProvider.spec.ts

Lines changed: 170 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ vi.mock("vscode", () => ({
3434
fs: {
3535
stat: vi.fn(),
3636
},
37+
getConfiguration: vi.fn(() => ({
38+
get: vi.fn().mockReturnValue(false), // Default to false for preventFocusDisruption
39+
})),
3740
},
3841
window: {
3942
createTextEditorDecorationType: vi.fn(),
@@ -81,6 +84,7 @@ vi.mock("vscode", () => ({
8184
InCenter: 2,
8285
},
8386
TabInputTextDiff: class TabInputTextDiff {},
87+
TabInputText: class TabInputText {},
8488
Uri: {
8589
file: vi.fn((path) => ({ fsPath: path })),
8690
parse: vi.fn((uri) => ({ with: vi.fn(() => ({})) })),
@@ -188,7 +192,9 @@ describe("DiffViewProvider", () => {
188192
// Mock showTextDocument to track when it's called
189193
vi.mocked(vscode.window.showTextDocument).mockImplementation(async (uri, options) => {
190194
callOrder.push("showTextDocument")
191-
expect(options).toEqual({ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true })
195+
// Don't check preserveFocus here as it depends on the configuration
196+
expect(options?.preview).toBe(false)
197+
expect(options?.viewColumn).toBe(vscode.ViewColumn.Active)
192198
return mockEditor as any
193199
})
194200

@@ -220,10 +226,13 @@ describe("DiffViewProvider", () => {
220226
// Verify that showTextDocument was called before executeCommand
221227
expect(callOrder).toEqual(["showTextDocument", "executeCommand"])
222228

223-
// Verify that showTextDocument was called with preview: false and preserveFocus: true
229+
// Verify that showTextDocument was called with preview: false
224230
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
225231
expect.objectContaining({ fsPath: `${mockCwd}/test.md` }),
226-
{ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true },
232+
expect.objectContaining({
233+
preview: false,
234+
viewColumn: vscode.ViewColumn.Active,
235+
}),
227236
)
228237

229238
// Verify that the diff command was executed
@@ -232,7 +241,7 @@ describe("DiffViewProvider", () => {
232241
expect.any(Object),
233242
expect.any(Object),
234243
`test.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
235-
{ preserveFocus: true },
244+
expect.objectContaining({ preserveFocus: expect.any(Boolean) }),
236245
)
237246
})
238247

@@ -418,4 +427,161 @@ describe("DiffViewProvider", () => {
418427
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
419428
})
420429
})
430+
431+
describe("preventFocusDisruption setting", () => {
432+
beforeEach(() => {
433+
// Setup common mocks
434+
;(diffViewProvider as any).relPath = "test.ts"
435+
;(diffViewProvider as any).newContent = "new content"
436+
;(diffViewProvider as any).activeDiffEditor = {
437+
document: {
438+
getText: vi.fn().mockReturnValue("new content"),
439+
isDirty: false,
440+
save: vi.fn().mockResolvedValue(undefined),
441+
uri: { fsPath: `${mockCwd}/test.ts` },
442+
},
443+
}
444+
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
445+
})
446+
447+
it("should show document when preventFocusDisruption is false", async () => {
448+
// Mock configuration to return false
449+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
450+
get: vi.fn().mockReturnValue(false),
451+
} as any)
452+
453+
await diffViewProvider.saveChanges(false)
454+
455+
// Verify showTextDocument was called
456+
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
457+
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
458+
{ preview: false, preserveFocus: true },
459+
)
460+
})
461+
462+
it("should not show document when preventFocusDisruption is true", async () => {
463+
// Mock configuration to return true
464+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
465+
get: vi.fn().mockReturnValue(true),
466+
} as any)
467+
468+
await diffViewProvider.saveChanges(false)
469+
470+
// Verify showTextDocument was NOT called
471+
expect(vscode.window.showTextDocument).not.toHaveBeenCalled()
472+
})
473+
474+
it("should preserve focus in diff view when preventFocusDisruption is true", async () => {
475+
// Mock configuration to return true
476+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
477+
get: vi.fn().mockReturnValue(true),
478+
} as any)
479+
480+
// Track the order of calls
481+
const callOrder: string[] = []
482+
483+
// Mock showTextDocument
484+
vi.mocked(vscode.window.showTextDocument).mockImplementation(async (uri, options) => {
485+
callOrder.push("showTextDocument")
486+
expect(options?.preserveFocus).toBe(true) // Should preserve focus
487+
return {} as any
488+
})
489+
490+
// Mock executeCommand
491+
vi.mocked(vscode.commands.executeCommand).mockImplementation(async (command, ...args) => {
492+
callOrder.push("executeCommand")
493+
if (command === "vscode.diff") {
494+
// Check that preserveFocus is true in the options
495+
const options = args[3]
496+
expect(options?.preserveFocus).toBe(true)
497+
}
498+
return undefined
499+
})
500+
501+
// Mock workspace.onDidOpenTextDocument to trigger immediately
502+
vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => {
503+
setTimeout(() => {
504+
callback({ uri: { fsPath: `${mockCwd}/test.ts` } } as any)
505+
}, 0)
506+
return { dispose: vi.fn() }
507+
})
508+
509+
// Mock window.visibleTextEditors
510+
const mockEditor = {
511+
document: {
512+
uri: { fsPath: `${mockCwd}/test.ts` },
513+
getText: vi.fn().mockReturnValue(""),
514+
lineCount: 0,
515+
},
516+
selection: {
517+
active: { line: 0, character: 0 },
518+
anchor: { line: 0, character: 0 },
519+
},
520+
edit: vi.fn().mockResolvedValue(true),
521+
revealRange: vi.fn(),
522+
}
523+
vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any]
524+
;(diffViewProvider as any).editType = "modify"
525+
await diffViewProvider.open("test.ts")
526+
527+
// Verify preserveFocus was set to true in both calls
528+
expect(callOrder).toContain("showTextDocument")
529+
expect(callOrder).toContain("executeCommand")
530+
})
531+
532+
it("should not restore document view on revert when preventFocusDisruption is true", async () => {
533+
// Mock configuration to return true
534+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
535+
get: vi.fn().mockReturnValue(true),
536+
} as any)
537+
538+
// Setup for revertChanges
539+
;(diffViewProvider as any).editType = "modify"
540+
;(diffViewProvider as any).documentWasOpen = true
541+
;(diffViewProvider as any).originalContent = "original content"
542+
;(diffViewProvider as any).activeDiffEditor = {
543+
document: {
544+
uri: { fsPath: `${mockCwd}/test.ts` },
545+
isDirty: false,
546+
save: vi.fn().mockResolvedValue(undefined),
547+
getText: vi.fn().mockReturnValue("modified content"),
548+
positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }),
549+
},
550+
}
551+
552+
await diffViewProvider.revertChanges()
553+
554+
// Verify showTextDocument was NOT called even though documentWasOpen was true
555+
expect(vscode.window.showTextDocument).not.toHaveBeenCalled()
556+
})
557+
558+
it("should restore document view on revert when preventFocusDisruption is false", async () => {
559+
// Mock configuration to return false
560+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
561+
get: vi.fn().mockReturnValue(false),
562+
} as any)
563+
564+
// Setup for revertChanges
565+
;(diffViewProvider as any).editType = "modify"
566+
;(diffViewProvider as any).documentWasOpen = true
567+
;(diffViewProvider as any).originalContent = "original content"
568+
;(diffViewProvider as any).activeDiffEditor = {
569+
document: {
570+
uri: { fsPath: `${mockCwd}/test.ts` },
571+
isDirty: false,
572+
save: vi.fn().mockResolvedValue(undefined),
573+
getText: vi.fn().mockReturnValue("modified content"),
574+
positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }),
575+
},
576+
}
577+
578+
await diffViewProvider.revertChanges()
579+
580+
// Verify showTextDocument was called since documentWasOpen was true
581+
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
582+
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
583+
{ preview: false, preserveFocus: true },
584+
)
585+
})
586+
})
421587
})

src/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,11 @@
386386
"type": "string",
387387
"default": "",
388388
"description": "%settings.autoImportSettingsPath.description%"
389+
},
390+
"roo-cline.preventFocusDisruption": {
391+
"type": "boolean",
392+
"default": false,
393+
"description": "%settings.preventFocusDisruption.description%"
389394
}
390395
}
391396
}

src/package.nls.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,6 @@
3636
"settings.vsCodeLmModelSelector.family.description": "The family of the language model (e.g. gpt-4)",
3737
"settings.customStoragePath.description": "Custom storage path. Leave empty to use the default location. Supports absolute paths (e.g. 'D:\\RooCodeStorage')",
3838
"settings.enableCodeActions.description": "Enable Roo Code quick fixes",
39-
"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."
39+
"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.",
40+
"settings.preventFocusDisruption.description": "Prevent file edits from stealing focus. When enabled, file changes happen in the background without disrupting your current work. Diff views will not automatically open or take focus."
4041
}

0 commit comments

Comments
 (0)