feat: restore editor scroll position after closing diff view

- Capture visible ranges from diff editor before closing
- Restore scroll position when showing text document
- Applies to both saveChanges() and revertChanges() methods
- Added comprehensive tests for scroll position preservation

Fixes #9282
This commit is contained in:
Roo Code 2025-11-15 15:49:42 +00:00
parent 4e316e0de8
commit 0605187d59
2 changed files with 147 additions and 3 deletions

View file

@ -206,13 +206,24 @@ export class DiffViewProvider {
const updatedDocument = this.activeDiffEditor.document
const editedContent = updatedDocument.getText()
// Capture the visible ranges before closing the diff view to restore scroll position later
const visibleRanges = this.activeDiffEditor.visibleRanges
if (updatedDocument.isDirty) {
await updatedDocument.save()
}
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true })
const editor = await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
preview: false,
preserveFocus: true,
})
await this.closeAllDiffViews()
// Restore the scroll position from the diff view
if (visibleRanges && visibleRanges.length > 0) {
editor.revealRange(visibleRanges[0], vscode.TextEditorRevealType.InCenter)
}
// Getting diagnostics before and after the file edit is a better approach than
// automatically tracking problems in real-time. This method ensures we only
// report new problems that are a direct result of this specific edit.
@ -378,6 +389,9 @@ export class DiffViewProvider {
const updatedDocument = this.activeDiffEditor.document
const absolutePath = path.resolve(this.cwd, this.relPath)
// Capture the visible ranges before closing the diff view to restore scroll position later
const visibleRanges = this.activeDiffEditor.visibleRanges
if (!fileExists) {
if (updatedDocument.isDirty) {
await updatedDocument.save()
@ -408,10 +422,15 @@ export class DiffViewProvider {
await updatedDocument.save()
if (this.documentWasOpen) {
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
const editor = await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
preview: false,
preserveFocus: true,
})
// Restore the scroll position from the diff view
if (visibleRanges && visibleRanges.length > 0) {
editor.revealRange(visibleRanges[0], vscode.TextEditorRevealType.InCenter)
}
}
await this.closeAllDiffViews()

View file

@ -450,11 +450,14 @@ describe("DiffViewProvider", () => {
isDirty: false,
save: vi.fn().mockResolvedValue(undefined),
},
visibleRanges: [{ start: { line: 10 }, end: { line: 20 } }],
}
;(diffViewProvider as any).preDiagnostics = []
// Mock vscode functions
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
vi.mocked(vscode.window.showTextDocument).mockResolvedValue({
revealRange: vi.fn(),
} as any)
vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([])
})
@ -517,4 +520,126 @@ describe("DiffViewProvider", () => {
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
})
})
describe("scroll position preservation", () => {
let mockEditor: any
beforeEach(() => {
// Setup common mocks for scroll position tests
mockEditor = {
revealRange: vi.fn(),
}
;(diffViewProvider as any).relPath = "test.ts"
;(diffViewProvider as any).newContent = "new content"
;(diffViewProvider as any).originalContent = "original content"
;(diffViewProvider as any).documentWasOpen = true
;(diffViewProvider as any).editType = "modify"
;(diffViewProvider as any).activeDiffEditor = {
document: {
getText: vi.fn().mockReturnValue("new content"),
isDirty: false,
save: vi.fn().mockResolvedValue(undefined),
uri: { fsPath: `${mockCwd}/test.ts` },
positionAt: vi.fn((offset) => ({ line: 0, character: offset })),
},
visibleRanges: [{ start: { line: 15 }, end: { line: 25 } }],
}
;(diffViewProvider as any).preDiagnostics = []
// Mock vscode functions
vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor)
vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([])
;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
// Mock WorkspaceEdit
const mockWorkspaceEdit = {
replace: vi.fn(),
}
vi.mocked(vscode.WorkspaceEdit).mockImplementation(() => mockWorkspaceEdit as any)
vi.mocked(vscode.workspace.applyEdit).mockResolvedValue(true)
})
it("should restore scroll position in saveChanges", async () => {
const result = await diffViewProvider.saveChanges(false, 0)
// Verify the editor was shown
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
{ preview: false, preserveFocus: true },
)
// Verify scroll position was restored
expect(mockEditor.revealRange).toHaveBeenCalledWith(
{ start: { line: 15 }, end: { line: 25 } },
vscode.TextEditorRevealType.InCenter,
)
expect(result.newProblemsMessage).toBe("")
})
it("should restore scroll position in revertChanges for existing file", async () => {
const result = await diffViewProvider.revertChanges()
// Verify the editor was shown (since documentWasOpen was true)
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
{ preview: false, preserveFocus: true },
)
// Verify scroll position was restored
expect(mockEditor.revealRange).toHaveBeenCalledWith(
{ start: { line: 15 }, end: { line: 25 } },
vscode.TextEditorRevealType.InCenter,
)
})
it("should handle missing visible ranges gracefully", async () => {
// Remove visible ranges
;(diffViewProvider as any).activeDiffEditor.visibleRanges = undefined
const result = await diffViewProvider.saveChanges(false, 0)
// Verify the editor was shown
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
{ preview: false, preserveFocus: true },
)
// Verify revealRange was NOT called
expect(mockEditor.revealRange).not.toHaveBeenCalled()
expect(result.newProblemsMessage).toBe("")
})
it("should handle empty visible ranges array gracefully", async () => {
// Set empty visible ranges
;(diffViewProvider as any).activeDiffEditor.visibleRanges = []
const result = await diffViewProvider.saveChanges(false, 0)
// Verify the editor was shown
expect(vscode.window.showTextDocument).toHaveBeenCalledWith(
expect.objectContaining({ fsPath: `${mockCwd}/test.ts` }),
{ preview: false, preserveFocus: true },
)
// Verify revealRange was NOT called
expect(mockEditor.revealRange).not.toHaveBeenCalled()
expect(result.newProblemsMessage).toBe("")
})
it("should not restore scroll position if document was not previously open", async () => {
// Set documentWasOpen to false
;(diffViewProvider as any).documentWasOpen = false
await diffViewProvider.revertChanges()
// Verify the editor was NOT shown (since documentWasOpen was false)
expect(vscode.window.showTextDocument).not.toHaveBeenCalled()
// Verify revealRange was NOT called
expect(mockEditor.revealRange).not.toHaveBeenCalled()
})
})
})