Skip to content

fix: preserve cursor position when applying diffs (#6853) #7105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
roomote wants to merge 1 commit into from
Closed

fix: preserve cursor position when applying diffs (#6853) #7105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 18 additions & 6 deletions src/integrations/editor/DiffViewProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix preserving the viewport by conditionally calling showTextDocument only if the file isn’t already active. Consider extracting this repeated check (using vscode.window.activeTextEditor and arePathsEqual) into a helper utility to reduce duplication between saveChanges and saveDirectly.

This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.

// 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
Expand Down Expand Up @@ -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))
Expand Down
67 changes: 67 additions & 0 deletions src/integrations/editor/__tests__/DiffViewProvider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 },
)
})
})
})
Loading