fix: preserve cursor position when applying diffs

- Check if file is already active editor before calling showTextDocument
- Skip showTextDocument call when file is already open to preserve viewport
- Add tests for the new behavior to ensure viewport preservation
- Fixes #6853
This commit is contained in:
Roo Code 2025-08-14 17:51:15 +00:00
parent 2a105a5881
commit 6181b0c6eb
2 changed files with 85 additions and 6 deletions

View file

@ -207,7 +207,13 @@ export class DiffViewProvider {
await updatedDocument.save()
}
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true })
// Only show the document if it's not already the active editor
// This preserves the viewport position when the file is already open
const activeEditor = vscode.window.activeTextEditor
const fileUri = vscode.Uri.file(absolutePath)
if (!activeEditor || !arePathsEqual(activeEditor.document.uri.fsPath, absolutePath)) {
await vscode.window.showTextDocument(fileUri, { preview: false, preserveFocus: true })
}
await this.closeAllDiffViews()
// Getting diagnostics before and after the file edit is a better approach than
@ -661,11 +667,17 @@ export class DiffViewProvider {
// Open the document to ensure diagnostics are loaded
// When openFile is false (PREVENT_FOCUS_DISRUPTION enabled), we only open in memory
if (openFile) {
// Show the document in the editor
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
preview: false,
preserveFocus: true,
})
// Only show the document if it's not already the active editor
// This preserves the viewport position when the file is already open
const activeEditor = vscode.window.activeTextEditor
const fileUri = vscode.Uri.file(absolutePath)
if (!activeEditor || !arePathsEqual(activeEditor.document.uri.fsPath, absolutePath)) {
// Show the document in the editor
await vscode.window.showTextDocument(fileUri, {
preview: false,
preserveFocus: true,
})
}
} else {
// Just open the document in memory to trigger diagnostics without showing it
const doc = await vscode.workspace.openTextDocument(vscode.Uri.file(absolutePath))

View file

@ -362,6 +362,8 @@ describe("DiffViewProvider", () => {
// Mock vscode functions
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([])
// Mock activeTextEditor as null by default
vi.mocked(vscode.window).activeTextEditor = null as any
})
it("should write content directly to file without opening diff view", async () => {
@ -390,6 +392,26 @@ describe("DiffViewProvider", () => {
expect(result.finalContent).toBe("new content")
})
it("should not call showTextDocument when file is already active editor", async () => {
// Mock active editor with the same file
vi.mocked(vscode.window).activeTextEditor = {
document: {
uri: { fsPath: `${mockCwd}/test.ts` },
},
} as any
vi.mocked(vscode.window.showTextDocument).mockClear()
await diffViewProvider.saveDirectly("test.ts", "new content", true, true, 1000)
// Verify file was written
const fs = await import("fs/promises")
expect(fs.writeFile).toHaveBeenCalledWith(`${mockCwd}/test.ts`, "new content", "utf-8")
// Verify showTextDocument was NOT called since file is already active
expect(vscode.window.showTextDocument).not.toHaveBeenCalled()
})
it("should not open file when openWithoutFocus is false", async () => {
await diffViewProvider.saveDirectly("test.ts", "new content", false, true, 1000)
@ -456,6 +478,8 @@ describe("DiffViewProvider", () => {
// Mock vscode functions
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([])
// Mock activeTextEditor as null by default
vi.mocked(vscode.window).activeTextEditor = null as any
})
it("should apply diagnostic delay when diagnosticsEnabled is true", async () => {
@ -516,5 +540,48 @@ describe("DiffViewProvider", () => {
expect(mockDelay).toHaveBeenCalledWith(5000)
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
})
it("should not call showTextDocument when file is already active editor", async () => {
// Mock active editor with the same file
vi.mocked(vscode.window).activeTextEditor = {
document: {
uri: { fsPath: `${mockCwd}/test.ts` },
},
} as any
// Mock closeAllDiffViews
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
vi.mocked(vscode.window.showTextDocument).mockClear()
await diffViewProvider.saveChanges(true, 1000)
// Verify showTextDocument was NOT called since file is already active
expect(vscode.window.showTextDocument).not.toHaveBeenCalled()
// Verify closeAllDiffViews was still called
expect((diffViewProvider as any).closeAllDiffViews).toHaveBeenCalled()
})
it("should call showTextDocument when active editor is different file", async () => {
// Mock active editor with a different file
vi.mocked(vscode.window).activeTextEditor = {
document: {
uri: { fsPath: `${mockCwd}/different.ts` },
},
} as any
// Mock closeAllDiffViews
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
vi.mocked(vscode.window.showTextDocument).mockClear()
await diffViewProvider.saveChanges(true, 1000)
// Verify showTextDocument WAS called since active editor is different file
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
{ preview: false, preserveFocus: true },
)
})
})
})