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
This commit is contained in:
Roo Code 2025-07-21 13:10:26 +00:00
parent 9fce90be9d
commit c1833d77de
11 changed files with 276 additions and 13 deletions

View file

@ -107,6 +107,7 @@ export const globalSettingsSchema = z.object({
rateLimitSeconds: z.number().optional(),
diffEnabled: z.boolean().optional(),
diffViewAutoFocus: z.boolean().optional(),
fuzzyMatchThreshold: z.number().optional(),
experiments: experimentsSchema.optional(),
@ -250,6 +251,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
diagnosticsEnabled: true,
diffEnabled: true,
diffViewAutoFocus: false,
fuzzyMatchThreshold: 1,
enableCheckpoints: false,

View file

@ -260,7 +260,22 @@ 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)
// Get diffViewAutoFocus setting from provider state
provider
.getState()
.then((state) => {
const diffViewAutoFocus = (state as any)?.diffViewAutoFocus ?? false
this.diffViewProvider = new DiffViewProvider(this.cwd, diffViewAutoFocus)
})
.catch(() => {
// Fallback if state retrieval fails
this.diffViewProvider = new DiffViewProvider(this.cwd, false)
})
// Create with default for immediate use
this.diffViewProvider = new DiffViewProvider(this.cwd, false)
this.enableCheckpoints = enableCheckpoints
this.rootTask = rootTask

View file

@ -932,6 +932,11 @@ export const webviewMessageHandler = async (
await updateGlobalState("diffEnabled", diffEnabled)
await provider.postStateToWebview()
break
case "diffViewAutoFocus":
const diffViewAutoFocus = message.bool ?? false
await updateGlobalState("diffViewAutoFocus", diffViewAutoFocus)
await provider.postStateToWebview()
break
case "enableCheckpoints":
const enableCheckpoints = message.bool ?? true
await updateGlobalState("enableCheckpoints", enableCheckpoints)

View file

@ -36,8 +36,14 @@ export class DiffViewProvider {
private activeLineController?: DecorationController
private streamedLines: string[] = []
private preDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = []
private diffViewAutoFocus: boolean
constructor(private cwd: string) {}
constructor(
private cwd: string,
diffViewAutoFocus: boolean = false,
) {
this.diffViewAutoFocus = diffViewAutoFocus
}
async open(relPath: string): Promise<void> {
this.relPath = relPath
@ -181,7 +187,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 +207,10 @@ export class DiffViewProvider {
await updatedDocument.save()
}
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true })
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
preview: false,
preserveFocus: !this.diffViewAutoFocus,
})
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(
@ -390,7 +402,7 @@ export class DiffViewProvider {
if (this.documentWasOpen) {
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
preview: false,
preserveFocus: true,
preserveFocus: !this.diffViewAutoFocus,
})
}
@ -457,7 +469,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: !this.diffViewAutoFocus,
})
return editor
}
@ -523,7 +537,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: !this.diffViewAutoFocus,
})
.then(() => {
// Execute the diff command after ensuring the file is open as text
return vscode.commands.executeCommand(
@ -533,7 +551,7 @@ export class DiffViewProvider {
}),
uri,
`${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`,
{ preserveFocus: true },
{ preserveFocus: !this.diffViewAutoFocus },
)
})
.then(

View file

@ -12,6 +12,7 @@ vi.mock("delay", () => ({
vi.mock("fs/promises", () => ({
readFile: vi.fn().mockResolvedValue("file content"),
writeFile: vi.fn().mockResolvedValue(undefined),
unlink: vi.fn().mockResolvedValue(undefined),
}))
// Mock utils
@ -110,7 +111,7 @@ describe("DiffViewProvider", () => {
}
vi.mocked(vscode.WorkspaceEdit).mockImplementation(() => mockWorkspaceEdit as any)
diffViewProvider = new DiffViewProvider(mockCwd)
diffViewProvider = new DiffViewProvider(mockCwd, false)
// Mock the necessary properties and methods
;(diffViewProvider as any).relPath = "test.txt"
;(diffViewProvider as any).activeDiffEditor = {
@ -236,6 +237,63 @@ describe("DiffViewProvider", () => {
)
})
it("should respect diffViewAutoFocus setting when opening diff", async () => {
// Create a new instance with diffViewAutoFocus enabled
const focusEnabledProvider = new DiffViewProvider(mockCwd, true)
// Setup
const mockEditor = {
document: {
uri: { fsPath: `${mockCwd}/test.md` },
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 executeCommand
vi.mocked(vscode.commands.executeCommand).mockResolvedValue(undefined)
// Mock workspace.onDidOpenTextDocument to trigger immediately
vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => {
setTimeout(() => {
callback({ uri: { fsPath: `${mockCwd}/test.md` } } as any)
}, 0)
return { dispose: vi.fn() }
})
// Mock window.visibleTextEditors
vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any]
// Set up for file
;(focusEnabledProvider as any).editType = "modify"
// Execute open
await focusEnabledProvider.open("test.md")
// Verify that preserveFocus is false when diffViewAutoFocus is true
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
expect.objectContaining({ fsPath: `${mockCwd}/test.md` }),
{ preview: false, viewColumn: vscode.ViewColumn.Active, preserveFocus: false },
)
expect(vscode.commands.executeCommand).toHaveBeenCalledWith(
"vscode.diff",
expect.any(Object),
expect.any(Object),
`test.md: ${DIFF_VIEW_LABEL_CHANGES} (Editable)`,
{ preserveFocus: false },
)
})
it("should handle showTextDocument failure", async () => {
// Mock showTextDocument to fail
vi.mocked(vscode.window.showTextDocument).mockRejectedValue(new Error("Cannot open file"))
@ -418,4 +476,91 @@ describe("DiffViewProvider", () => {
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
})
})
describe("diffViewAutoFocus behavior", () => {
it("should use preserveFocus=true when diffViewAutoFocus is false", async () => {
// Default provider has diffViewAutoFocus=false
const mockEditor = {
document: {
uri: { fsPath: `${mockCwd}/test.txt` },
getText: vi.fn().mockReturnValue("content"),
save: vi.fn().mockResolvedValue(undefined),
isDirty: false,
},
}
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
;(diffViewProvider as any).activeDiffEditor = mockEditor
;(diffViewProvider as any).relPath = "test.txt"
;(diffViewProvider as any).newContent = "new content"
;(diffViewProvider as any).preDiagnostics = []
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
await diffViewProvider.saveChanges(false)
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(expect.any(Object), {
preview: false,
preserveFocus: true,
})
})
it("should use preserveFocus=false when diffViewAutoFocus is true", async () => {
// Create provider with diffViewAutoFocus=true
const focusProvider = new DiffViewProvider(mockCwd, true)
const mockEditor = {
document: {
uri: { fsPath: `${mockCwd}/test.txt` },
getText: vi.fn().mockReturnValue("content"),
save: vi.fn().mockResolvedValue(undefined),
isDirty: false,
},
}
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
;(focusProvider as any).activeDiffEditor = mockEditor
;(focusProvider as any).relPath = "test.txt"
;(focusProvider as any).newContent = "new content"
;(focusProvider as any).preDiagnostics = []
;(focusProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
await focusProvider.saveChanges(false)
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(expect.any(Object), {
preview: false,
preserveFocus: false,
})
})
it("should use preserveFocus=false for revertChanges when diffViewAutoFocus is true", async () => {
// Create provider with diffViewAutoFocus=true
const focusProvider = new DiffViewProvider(mockCwd, true)
const mockEditor = {
document: {
uri: { fsPath: `${mockCwd}/test.txt` },
getText: vi.fn().mockReturnValue("content"),
save: vi.fn().mockResolvedValue(undefined),
isDirty: false,
positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }),
},
edit: vi.fn().mockResolvedValue(true),
}
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any)
vi.mocked(vscode.workspace.applyEdit).mockResolvedValue(true)
;(focusProvider as any).activeDiffEditor = mockEditor
;(focusProvider as any).relPath = "test.txt"
;(focusProvider as any).originalContent = "original content"
;(focusProvider as any).preDiagnostics = []
;(focusProvider as any).editType = "modify" // Set to modify so it goes through the revert path
;(focusProvider as any).documentWasOpen = true // This needs to be true for showTextDocument to be called
;(focusProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
await focusProvider.revertChanges()
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(expect.any(Object), {
preview: false,
preserveFocus: false,
})
})
})
})

View file

@ -93,6 +93,7 @@ export interface WebviewMessage {
| "ttsSpeed"
| "soundVolume"
| "diffEnabled"
| "diffViewAutoFocus"
| "enableCheckpoints"
| "browserViewportSize"
| "screenshotQuality"

View file

@ -33,6 +33,7 @@ type AutoApproveSettingsProps = HTMLAttributes<HTMLDivElement> & {
followupAutoApproveTimeoutMs?: number
allowedCommands?: string[]
deniedCommands?: string[]
diffViewAutoFocus?: boolean
setCachedStateField: SetCachedStateField<
| "alwaysAllowReadOnly"
| "alwaysAllowReadOnlyOutsideWorkspace"
@ -52,6 +53,7 @@ type AutoApproveSettingsProps = HTMLAttributes<HTMLDivElement> & {
| "allowedCommands"
| "deniedCommands"
| "alwaysAllowUpdateTodoList"
| "diffViewAutoFocus"
>
}
@ -74,6 +76,7 @@ export const AutoApproveSettings = ({
alwaysAllowUpdateTodoList,
allowedCommands,
deniedCommands,
diffViewAutoFocus,
setCachedStateField,
...props
}: AutoApproveSettingsProps) => {
@ -393,6 +396,25 @@ export const AutoApproveSettings = ({
</div>
</div>
)}
{/* Diff View Settings */}
<div className="mt-6">
<div className="flex items-center gap-4 font-bold mb-3">
<span className="codicon codicon-diff" />
<div>{t("settings:autoApprove.diffView.label")}</div>
</div>
<div>
<VSCodeCheckbox
checked={diffViewAutoFocus}
onChange={(e: any) => setCachedStateField("diffViewAutoFocus", e.target.checked)}
data-testid="diff-view-auto-focus-checkbox">
<span className="font-medium">{t("settings:autoApprove.diffView.autoFocus.label")}</span>
</VSCodeCheckbox>
<div className="text-vscode-descriptionForeground text-sm mt-1">
{t("settings:autoApprove.diffView.autoFocus.description")}
</div>
</div>
</div>
</Section>
</div>
)

View file

@ -0,0 +1,42 @@
import { HTMLAttributes } from "react"
import { useAppTranslation } from "@/i18n/TranslationContext"
import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"
import { FileEdit } from "lucide-react"
import { SetCachedStateField } from "./types"
import { SectionHeader } from "./SectionHeader"
import { Section } from "./Section"
type FileEditingSettingsProps = HTMLAttributes<HTMLDivElement> & {
diffViewAutoFocus?: boolean
setCachedStateField: SetCachedStateField<"diffViewAutoFocus">
}
export const FileEditingSettings = ({ diffViewAutoFocus, setCachedStateField, ...props }: FileEditingSettingsProps) => {
const { t } = useAppTranslation()
return (
<div {...props}>
<SectionHeader>
<div className="flex items-center gap-2">
<FileEdit className="w-4" />
<div>{t("settings:sections.fileEditing")}</div>
</div>
</SectionHeader>
<Section>
<div>
<VSCodeCheckbox
checked={diffViewAutoFocus}
onChange={(e: any) => {
setCachedStateField("diffViewAutoFocus", e.target.checked)
}}>
<span className="font-medium">{t("settings:fileEditing.diffViewAutoFocus.label")}</span>
</VSCodeCheckbox>
<div className="text-vscode-descriptionForeground text-sm mt-1">
{t("settings:fileEditing.diffViewAutoFocus.description")}
</div>
</div>
</Section>
</div>
)
}

View file

@ -141,6 +141,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
browserViewportSize,
enableCheckpoints,
diffEnabled,
diffViewAutoFocus,
experiments,
fuzzyMatchThreshold,
maxOpenTabsContext,
@ -294,6 +295,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
vscode.postMessage({ type: "ttsSpeed", value: ttsSpeed })
vscode.postMessage({ type: "soundVolume", value: soundVolume })
vscode.postMessage({ type: "diffEnabled", bool: diffEnabled })
vscode.postMessage({ type: "diffViewAutoFocus", bool: diffViewAutoFocus })
vscode.postMessage({ type: "enableCheckpoints", bool: enableCheckpoints })
vscode.postMessage({ type: "browserViewportSize", text: browserViewportSize })
vscode.postMessage({ type: "remoteBrowserHost", text: remoteBrowserHost })
@ -619,6 +621,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
followupAutoApproveTimeoutMs={followupAutoApproveTimeoutMs}
allowedCommands={allowedCommands}
deniedCommands={deniedCommands}
diffViewAutoFocus={diffViewAutoFocus}
setCachedStateField={setCachedStateField}
/>
)}

View file

@ -25,6 +25,7 @@ import { convertTextMateToHljs } from "@src/utils/textMateToHljs"
export interface ExtensionStateContextType extends ExtensionState {
historyPreviewCollapsed?: boolean // Add the new state property
diffViewAutoFocus?: boolean
didHydrateState: boolean
showWelcome: boolean
theme: any
@ -80,6 +81,7 @@ export interface ExtensionStateContextType extends ExtensionState {
setTtsEnabled: (value: boolean) => void
setTtsSpeed: (value: number) => void
setDiffEnabled: (value: boolean) => void
setDiffViewAutoFocus: (value: boolean) => void
setEnableCheckpoints: (value: boolean) => void
setBrowserViewportSize: (value: string) => void
setFuzzyMatchThreshold: (value: number) => void
@ -405,6 +407,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
setTtsEnabled: (value) => setState((prevState) => ({ ...prevState, ttsEnabled: value })),
setTtsSpeed: (value) => setState((prevState) => ({ ...prevState, ttsSpeed: value })),
setDiffEnabled: (value) => setState((prevState) => ({ ...prevState, diffEnabled: value })),
setDiffViewAutoFocus: (value) => setState((prevState) => ({ ...prevState, diffViewAutoFocus: value })),
setEnableCheckpoints: (value) => setState((prevState) => ({ ...prevState, enableCheckpoints: value })),
setBrowserViewportSize: (value: string) =>
setState((prevState) => ({ ...prevState, browserViewportSize: value })),

View file

@ -190,6 +190,13 @@
"label": "Todo",
"description": "Automatically update the to-do list without requiring approval"
},
"diffView": {
"label": "Diff View",
"autoFocus": {
"label": "Auto-focus diff editors",
"description": "When enabled, diff editors opened by Roo will automatically gain focus. When disabled, diff editors will open in the background without taking focus from your current editor."
}
},
"apiRequestLimit": {
"title": "Max Requests",
"description": "Automatically make this many API requests before asking for approval to continue with the task.",