diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index a30550dce1..3dae25e83f 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -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, diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 53b8ef5b87..78f63f3278 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -260,7 +260,22 @@ export class Task extends EventEmitter { 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 diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 780d40df89..0f32b3867a 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -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) diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index f4133029c9..36f842c461 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -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 { 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( diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts index a4aded95bb..7a653f351e 100644 --- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts +++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts @@ -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, + }) + }) + }) }) diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 1f56829f7b..d334bf6b46 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -93,6 +93,7 @@ export interface WebviewMessage { | "ttsSpeed" | "soundVolume" | "diffEnabled" + | "diffViewAutoFocus" | "enableCheckpoints" | "browserViewportSize" | "screenshotQuality" diff --git a/webview-ui/src/components/settings/AutoApproveSettings.tsx b/webview-ui/src/components/settings/AutoApproveSettings.tsx index e1d3c52cb9..d6df303582 100644 --- a/webview-ui/src/components/settings/AutoApproveSettings.tsx +++ b/webview-ui/src/components/settings/AutoApproveSettings.tsx @@ -33,6 +33,7 @@ type AutoApproveSettingsProps = HTMLAttributes & { followupAutoApproveTimeoutMs?: number allowedCommands?: string[] deniedCommands?: string[] + diffViewAutoFocus?: boolean setCachedStateField: SetCachedStateField< | "alwaysAllowReadOnly" | "alwaysAllowReadOnlyOutsideWorkspace" @@ -52,6 +53,7 @@ type AutoApproveSettingsProps = HTMLAttributes & { | "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 = ({ )} + + {/* Diff View Settings */} +
+
+ +
{t("settings:autoApprove.diffView.label")}
+
+
+ setCachedStateField("diffViewAutoFocus", e.target.checked)} + data-testid="diff-view-auto-focus-checkbox"> + {t("settings:autoApprove.diffView.autoFocus.label")} + +
+ {t("settings:autoApprove.diffView.autoFocus.description")} +
+
+
) diff --git a/webview-ui/src/components/settings/FileEditingSettings.tsx b/webview-ui/src/components/settings/FileEditingSettings.tsx new file mode 100644 index 0000000000..c89c8918f5 --- /dev/null +++ b/webview-ui/src/components/settings/FileEditingSettings.tsx @@ -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 & { + diffViewAutoFocus?: boolean + setCachedStateField: SetCachedStateField<"diffViewAutoFocus"> +} + +export const FileEditingSettings = ({ diffViewAutoFocus, setCachedStateField, ...props }: FileEditingSettingsProps) => { + const { t } = useAppTranslation() + return ( +
+ +
+ +
{t("settings:sections.fileEditing")}
+
+ + +
+
+ { + setCachedStateField("diffViewAutoFocus", e.target.checked) + }}> + {t("settings:fileEditing.diffViewAutoFocus.label")} + +
+ {t("settings:fileEditing.diffViewAutoFocus.description")} +
+
+
+
+ ) +} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 517c1c159d..99df83c8c3 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -141,6 +141,7 @@ const SettingsView = forwardRef(({ onDone, t browserViewportSize, enableCheckpoints, diffEnabled, + diffViewAutoFocus, experiments, fuzzyMatchThreshold, maxOpenTabsContext, @@ -294,6 +295,7 @@ const SettingsView = forwardRef(({ 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(({ onDone, t followupAutoApproveTimeoutMs={followupAutoApproveTimeoutMs} allowedCommands={allowedCommands} deniedCommands={deniedCommands} + diffViewAutoFocus={diffViewAutoFocus} setCachedStateField={setCachedStateField} /> )} diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index c970733fba..1e2604f013 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -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 })), diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index 7e3c2e3fcc..2ea6bce413 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -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.",