Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),

Expand Down Expand Up @@ -250,6 +251,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
diagnosticsEnabled: true,

diffEnabled: true,
diffViewAutoFocus: false,
fuzzyMatchThreshold: 1,

enableCheckpoints: false,
Expand Down
17 changes: 16 additions & 1 deletion src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions src/core/task/__tests__/Task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ vi.mock("vscode", () => {
stat: vi.fn().mockResolvedValue({ type: 1 }), // FileType.File = 1
},
onDidSaveTextDocument: vi.fn(() => mockDisposable),
onDidChangeWorkspaceFolders: vi.fn(() => mockDisposable),
getConfiguration: vi.fn(() => ({ get: (key: string, defaultValue: any) => defaultValue })),
},
env: {
Expand All @@ -127,6 +128,11 @@ vi.mock("vscode", () => {
from: vi.fn(),
},
TabInputText: vi.fn(),
Uri: {
file: vi.fn((path: string) => ({ fsPath: path, scheme: "file", path })),
parse: vi.fn((str: string) => ({ fsPath: str, scheme: "file", path: str })),
},
RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ base, pattern })),
}
})

Expand Down Expand Up @@ -1386,6 +1392,9 @@ describe("Cline", () => {
})

it("should not create diff strategy when enableDiff is false", async () => {
// Ensure getState returns a valid response
mockProvider.getState.mockResolvedValue({})

const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
Expand Down
5 changes: 5 additions & 0 deletions src/core/webview/webviewMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
40 changes: 29 additions & 11 deletions src/integrations/editor/DiffViewProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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,
})
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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(
Expand All @@ -533,7 +551,7 @@ export class DiffViewProvider {
}),
uri,
`${fileName}: ${fileExists ? `${DIFF_VIEW_LABEL_CHANGES}` : "New File"} (Editable)`,
{ preserveFocus: true },
{ preserveFocus: !this.diffViewAutoFocus },
)
})
.then(
Expand Down
147 changes: 146 additions & 1 deletion src/integrations/editor/__tests__/DiffViewProvider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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,
})
})
})
})
1 change: 1 addition & 0 deletions src/shared/WebviewMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export interface WebviewMessage {
| "ttsSpeed"
| "soundVolume"
| "diffEnabled"
| "diffViewAutoFocus"
| "enableCheckpoints"
| "browserViewportSize"
| "screenshotQuality"
Expand Down
Loading