Skip to content

Commit c1833d7

Browse files
committed
feat: add diffViewAutoFocus setting to control diff editor focus behavior
- Added diffViewAutoFocus boolean setting to global settings schema - Added UI checkbox in AutoApproveSettings component - Updated ExtensionStateContext to manage the setting state - Modified DiffViewProvider to use the setting for preserveFocus parameter - Updated Task class to pass the setting to DiffViewProvider - Added comprehensive tests for the new functionality - Added i18n translations for the setting Fixes #6010
1 parent 9fce90b commit c1833d7

File tree

11 files changed

+276
-13
lines changed

11 files changed

+276
-13
lines changed

packages/types/src/global-settings.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ export const globalSettingsSchema = z.object({
107107

108108
rateLimitSeconds: z.number().optional(),
109109
diffEnabled: z.boolean().optional(),
110+
diffViewAutoFocus: z.boolean().optional(),
110111
fuzzyMatchThreshold: z.number().optional(),
111112
experiments: experimentsSchema.optional(),
112113

@@ -250,6 +251,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
250251
diagnosticsEnabled: true,
251252

252253
diffEnabled: true,
254+
diffViewAutoFocus: false,
253255
fuzzyMatchThreshold: 1,
254256

255257
enableCheckpoints: false,

src/core/task/Task.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,22 @@ export class Task extends EventEmitter<ClineEvents> {
260260
this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
261261
this.providerRef = new WeakRef(provider)
262262
this.globalStoragePath = provider.context.globalStorageUri.fsPath
263-
this.diffViewProvider = new DiffViewProvider(this.cwd)
263+
264+
// Get diffViewAutoFocus setting from provider state
265+
provider
266+
.getState()
267+
.then((state) => {
268+
const diffViewAutoFocus = (state as any)?.diffViewAutoFocus ?? false
269+
this.diffViewProvider = new DiffViewProvider(this.cwd, diffViewAutoFocus)
270+
})
271+
.catch(() => {
272+
// Fallback if state retrieval fails
273+
this.diffViewProvider = new DiffViewProvider(this.cwd, false)
274+
})
275+
276+
// Create with default for immediate use
277+
this.diffViewProvider = new DiffViewProvider(this.cwd, false)
278+
264279
this.enableCheckpoints = enableCheckpoints
265280

266281
this.rootTask = rootTask

src/core/webview/webviewMessageHandler.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,11 @@ export const webviewMessageHandler = async (
932932
await updateGlobalState("diffEnabled", diffEnabled)
933933
await provider.postStateToWebview()
934934
break
935+
case "diffViewAutoFocus":
936+
const diffViewAutoFocus = message.bool ?? false
937+
await updateGlobalState("diffViewAutoFocus", diffViewAutoFocus)
938+
await provider.postStateToWebview()
939+
break
935940
case "enableCheckpoints":
936941
const enableCheckpoints = message.bool ?? true
937942
await updateGlobalState("enableCheckpoints", enableCheckpoints)

src/integrations/editor/DiffViewProvider.ts

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,14 @@ export class DiffViewProvider {
3636
private activeLineController?: DecorationController
3737
private streamedLines: string[] = []
3838
private preDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = []
39+
private diffViewAutoFocus: boolean
3940

40-
constructor(private cwd: string) {}
41+
constructor(
42+
private cwd: string,
43+
diffViewAutoFocus: boolean = false,
44+
) {
45+
this.diffViewAutoFocus = diffViewAutoFocus
46+
}
4147

4248
async open(relPath: string): Promise<void> {
4349
this.relPath = relPath
@@ -181,7 +187,10 @@ export class DiffViewProvider {
181187
}
182188
}
183189

184-
async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
190+
async saveChanges(
191+
diagnosticsEnabled: boolean = true,
192+
writeDelayMs: number = DEFAULT_WRITE_DELAY_MS,
193+
): Promise<{
185194
newProblemsMessage: string | undefined
186195
userEdits: string | undefined
187196
finalContent: string | undefined
@@ -198,7 +207,10 @@ export class DiffViewProvider {
198207
await updatedDocument.save()
199208
}
200209

201-
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true })
210+
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
211+
preview: false,
212+
preserveFocus: !this.diffViewAutoFocus,
213+
})
202214
await this.closeAllDiffViews()
203215

204216
// Getting diagnostics before and after the file edit is a better approach than
@@ -216,22 +228,22 @@ export class DiffViewProvider {
216228
// and can address them accordingly. If problems don't change immediately after
217229
// applying a fix, won't be notified, which is generally fine since the
218230
// initial fix is usually correct and it may just take time for linters to catch up.
219-
231+
220232
let newProblemsMessage = ""
221-
233+
222234
if (diagnosticsEnabled) {
223235
// Add configurable delay to allow linters time to process and clean up issues
224236
// like unused imports (especially important for Go and other languages)
225237
// Ensure delay is non-negative
226238
const safeDelayMs = Math.max(0, writeDelayMs)
227-
239+
228240
try {
229241
await delay(safeDelayMs)
230242
} catch (error) {
231243
// Log error but continue - delay failure shouldn't break the save operation
232244
console.warn(`Failed to apply write delay: ${error}`)
233245
}
234-
246+
235247
const postDiagnostics = vscode.languages.getDiagnostics()
236248

237249
const newProblems = await diagnosticsToProblemsString(
@@ -390,7 +402,7 @@ export class DiffViewProvider {
390402
if (this.documentWasOpen) {
391403
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
392404
preview: false,
393-
preserveFocus: true,
405+
preserveFocus: !this.diffViewAutoFocus,
394406
})
395407
}
396408

@@ -457,7 +469,9 @@ export class DiffViewProvider {
457469
)
458470

459471
if (diffTab && diffTab.input instanceof vscode.TabInputTextDiff) {
460-
const editor = await vscode.window.showTextDocument(diffTab.input.modified, { preserveFocus: true })
472+
const editor = await vscode.window.showTextDocument(diffTab.input.modified, {
473+
preserveFocus: !this.diffViewAutoFocus,
474+
})
461475
return editor
462476
}
463477

@@ -523,7 +537,11 @@ export class DiffViewProvider {
523537
// Pre-open the file as a text document to ensure it doesn't open in preview mode
524538
// This fixes issues with files that have custom editor associations (like markdown preview)
525539
vscode.window
526-
.showTextDocument(uri, { preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: true })
540+
.showTextDocument(uri, {
541+
preview: false,
542+
viewColumn: vscode.ViewColumn.Active,
543+
preserveFocus: !this.diffViewAutoFocus,
544+
})
527545
.then(() => {
528546
// Execute the diff command after ensuring the file is open as text
529547
return vscode.commands.executeCommand(
@@ -533,7 +551,7 @@ export class DiffViewProvider {
533551
}),
534552
uri,
535553
`${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`,
536-
{ preserveFocus: true },
554+
{ preserveFocus: !this.diffViewAutoFocus },
537555
)
538556
})
539557
.then(

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

Lines changed: 146 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ vi.mock("delay", () => ({
1212
vi.mock("fs/promises", () => ({
1313
readFile: vi.fn().mockResolvedValue("file content"),
1414
writeFile: vi.fn().mockResolvedValue(undefined),
15+
unlink: vi.fn().mockResolvedValue(undefined),
1516
}))
1617

1718
// Mock utils
@@ -110,7 +111,7 @@ describe("DiffViewProvider", () => {
110111
}
111112
vi.mocked(vscode.WorkspaceEdit).mockImplementation(() => mockWorkspaceEdit as any)
112113

113-
diffViewProvider = new DiffViewProvider(mockCwd)
114+
diffViewProvider = new DiffViewProvider(mockCwd, false)
114115
// Mock the necessary properties and methods
115116
;(diffViewProvider as any).relPath = "test.txt"
116117
;(diffViewProvider as any).activeDiffEditor = {
@@ -236,6 +237,63 @@ describe("DiffViewProvider", () => {
236237
)
237238
})
238239

240+
it("should respect diffViewAutoFocus setting when opening diff", async () => {
241+
// Create a new instance with diffViewAutoFocus enabled
242+
const focusEnabledProvider = new DiffViewProvider(mockCwd, true)
243+
244+
// Setup
245+
const mockEditor = {
246+
document: {
247+
uri: { fsPath: `${mockCwd}/test.md` },
248+
getText: vi.fn().mockReturnValue(""),
249+
lineCount: 0,
250+
},
251+
selection: {
252+
active: { line: 0, character: 0 },
253+
anchor: { line: 0, character: 0 },
254+
},
255+
edit: vi.fn().mockResolvedValue(true),
256+
revealRange: vi.fn(),
257+
}
258+
259+
// Mock showTextDocument
260+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
261+
262+
// Mock executeCommand
263+
vi.mocked(vscode.commands.executeCommand).mockResolvedValue(undefined)
264+
265+
// Mock workspace.onDidOpenTextDocument to trigger immediately
266+
vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => {
267+
setTimeout(() => {
268+
callback({ uri: { fsPath: `${mockCwd}/test.md` } } as any)
269+
}, 0)
270+
return { dispose: vi.fn() }
271+
})
272+
273+
// Mock window.visibleTextEditors
274+
vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any]
275+
276+
// Set up for file
277+
;(focusEnabledProvider as any).editType = "modify"
278+
279+
// Execute open
280+
await focusEnabledProvider.open("test.md")
281+
282+
// Verify that preserveFocus is false when diffViewAutoFocus is true
283+
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
284+
expect.objectContaining({ fsPath: `${mockCwd}/test.md` }),
285+
{ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: false },
286+
)
287+
288+
expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
289+
"vscode.diff",
290+
expect.any(Object),
291+
expect.any(Object),
292+
`test.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
293+
{ preserveFocus: false },
294+
)
295+
})
296+
239297
it("should handle showTextDocument failure", async () => {
240298
// Mock showTextDocument to fail
241299
vi.mocked(vscode.window.showTextDocument).mockRejectedValue(new Error("Cannot open file"))
@@ -418,4 +476,91 @@ describe("DiffViewProvider", () => {
418476
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
419477
})
420478
})
479+
480+
describe("diffViewAutoFocus behavior", () => {
481+
it("should use preserveFocus=true when diffViewAutoFocus is false", async () => {
482+
// Default provider has diffViewAutoFocus=false
483+
const mockEditor = {
484+
document: {
485+
uri: { fsPath: `${mockCwd}/test.txt` },
486+
getText: vi.fn().mockReturnValue("content"),
487+
save: vi.fn().mockResolvedValue(undefined),
488+
isDirty: false,
489+
},
490+
}
491+
492+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
493+
;(diffViewProvider as any).activeDiffEditor = mockEditor
494+
;(diffViewProvider as any).relPath = "test.txt"
495+
;(diffViewProvider as any).newContent = "new content"
496+
;(diffViewProvider as any).preDiagnostics = []
497+
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
498+
499+
await diffViewProvider.saveChanges(false)
500+
501+
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(expect.any(Object), {
502+
preview: false,
503+
preserveFocus: true,
504+
})
505+
})
506+
507+
it("should use preserveFocus=false when diffViewAutoFocus is true", async () => {
508+
// Create provider with diffViewAutoFocus=true
509+
const focusProvider = new DiffViewProvider(mockCwd, true)
510+
const mockEditor = {
511+
document: {
512+
uri: { fsPath: `${mockCwd}/test.txt` },
513+
getText: vi.fn().mockReturnValue("content"),
514+
save: vi.fn().mockResolvedValue(undefined),
515+
isDirty: false,
516+
},
517+
}
518+
519+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
520+
;(focusProvider as any).activeDiffEditor = mockEditor
521+
;(focusProvider as any).relPath = "test.txt"
522+
;(focusProvider as any).newContent = "new content"
523+
;(focusProvider as any).preDiagnostics = []
524+
;(focusProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
525+
526+
await focusProvider.saveChanges(false)
527+
528+
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(expect.any(Object), {
529+
preview: false,
530+
preserveFocus: false,
531+
})
532+
})
533+
534+
it("should use preserveFocus=false for revertChanges when diffViewAutoFocus is true", async () => {
535+
// Create provider with diffViewAutoFocus=true
536+
const focusProvider = new DiffViewProvider(mockCwd, true)
537+
const mockEditor = {
538+
document: {
539+
uri: { fsPath: `${mockCwd}/test.txt` },
540+
getText: vi.fn().mockReturnValue("content"),
541+
save: vi.fn().mockResolvedValue(undefined),
542+
isDirty: false,
543+
positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }),
544+
},
545+
edit: vi.fn().mockResolvedValue(true),
546+
}
547+
548+
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
549+
vi.mocked(vscode.workspace.applyEdit).mockResolvedValue(true)
550+
;(focusProvider as any).activeDiffEditor = mockEditor
551+
;(focusProvider as any).relPath = "test.txt"
552+
;(focusProvider as any).originalContent = "original content"
553+
;(focusProvider as any).preDiagnostics = []
554+
;(focusProvider as any).editType = "modify" // Set to modify so it goes through the revert path
555+
;(focusProvider as any).documentWasOpen = true // This needs to be true for showTextDocument to be called
556+
;(focusProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
557+
558+
await focusProvider.revertChanges()
559+
560+
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(expect.any(Object), {
561+
preview: false,
562+
preserveFocus: false,
563+
})
564+
})
565+
})
421566
})

src/shared/WebviewMessage.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ export interface WebviewMessage {
9393
| "ttsSpeed"
9494
| "soundVolume"
9595
| "diffEnabled"
96+
| "diffViewAutoFocus"
9697
| "enableCheckpoints"
9798
| "browserViewportSize"
9899
| "screenshotQuality"

webview-ui/src/components/settings/AutoApproveSettings.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ type AutoApproveSettingsProps = HTMLAttributes<HTMLDivElement> & {
3333
followupAutoApproveTimeoutMs?: number
3434
allowedCommands?: string[]
3535
deniedCommands?: string[]
36+
diffViewAutoFocus?: boolean
3637
setCachedStateField: SetCachedStateField<
3738
| "alwaysAllowReadOnly"
3839
| "alwaysAllowReadOnlyOutsideWorkspace"
@@ -52,6 +53,7 @@ type AutoApproveSettingsProps = HTMLAttributes<HTMLDivElement> & {
5253
| "allowedCommands"
5354
| "deniedCommands"
5455
| "alwaysAllowUpdateTodoList"
56+
| "diffViewAutoFocus"
5557
>
5658
}
5759

@@ -74,6 +76,7 @@ export const AutoApproveSettings = ({
7476
alwaysAllowUpdateTodoList,
7577
allowedCommands,
7678
deniedCommands,
79+
diffViewAutoFocus,
7780
setCachedStateField,
7881
...props
7982
}: AutoApproveSettingsProps) => {
@@ -393,6 +396,25 @@ export const AutoApproveSettings = ({
393396
</div>
394397
</div>
395398
)}
399+
400+
{/* Diff View Settings */}
401+
<div className="mt-6">
402+
<div className="flex items-center gap-4 font-bold mb-3">
403+
<span className="codicon codicon-diff" />
404+
<div>{t("settings:autoApprove.diffView.label")}</div>
405+
</div>
406+
<div>
407+
<VSCodeCheckbox
408+
checked={diffViewAutoFocus}
409+
onChange={(e: any) => setCachedStateField("diffViewAutoFocus", e.target.checked)}
410+
data-testid="diff-view-auto-focus-checkbox">
411+
<span className="font-medium">{t("settings:autoApprove.diffView.autoFocus.label")}</span>
412+
</VSCodeCheckbox>
413+
<div className="text-vscode-descriptionForeground text-sm mt-1">
414+
{t("settings:autoApprove.diffView.autoFocus.description")}
415+
</div>
416+
</div>
417+
</div>
396418
</Section>
397419
</div>
398420
)

0 commit comments

Comments
 (0)