fix: preserve viewport when applying diffs to active editor #8114
Conversation
- Check if file is already the active editor before re-opening - Prevents viewport jump to top when applying diffs - Applied fix to saveChanges, revertChanges, and saveDirectly methods - Addresses issue #8112
dosubot
bot
added
size:M
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| const isAlreadyActive = activeEditor && arePathsEqual(activeEditor.document.uri.fsPath, absolutePath) | ||
|
|
||
| if (!isAlreadyActive) { | ||
| // Only show the document if it's not already the active editor |
There was a problem hiding this comment.
Missing test coverage for this viewport preservation logic. The existing tests don't verify the behavior when exists and matches the target file path. Could we add a test case that ensures is NOT called when the file is already active?
| const isAlreadyActive = activeEditor && arePathsEqual(activeEditor.document.uri.fsPath, absolutePath) | ||
|
|
||
| if (!isAlreadyActive) { | ||
| // Only show the document if it's not already the active editor |
There was a problem hiding this comment.
Code duplication detected. This same viewport preservation logic appears in three places (lines 210-217, 416-426, and 679-689). Consider extracting this into a helper method like:
This would make the code more maintainable and ensure consistency across all three methods.
| preserveFocus: true, | ||
| }) | ||
| // Check if the file is already the active editor to avoid viewport jump | ||
| const activeEditor = vscode.window.activeTextEditor |
There was a problem hiding this comment.
Potential race condition here. The document is opened in memory first (line 692), then we check if it's active (line 680). If the file becomes active between these operations, the viewport could still jump. Is this an acceptable edge case, or should we check before line 692?
| } | ||
|
|
||
| await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true }) | ||
| // Check if the file is already the active editor to avoid viewport jump |
There was a problem hiding this comment.
Consider adding a comment explaining why we're avoiding re-opening already active files. Something like:
This would help future maintainers understand the intent behind this check.
hannesrudolph
added
the
Issue/PR - Triage
Summary
This PR fixes the viewport jumping issue when Roo applies diffs to a file that's already open in the editor. Previously, the editor would jump to the top of the file, disrupting the user's workflow. This fix checks if the file is already the active editor before attempting to re-open it, preserving the current viewport position.
Problem
When Roo edits a file you're currently viewing, the editor jumps to the top. This breaks flow and forces constant scrolling, especially in large files.
Solution
vscode.window.showTextDocumentsaveChanges,revertChanges, andsaveDirectlyTesting
Related Issues
Implementation Notes
This implementation follows the maintainer's guidance to avoid re-opening the document when it's already active, rather than trying to capture and restore cursor position (which only exposes selection state, not viewport position).
Important
Fixes viewport jumping issue by checking if file is active before reopening in
DiffViewProvider.ts.DiffViewProvider.ts.vscode.window.showTextDocumentinsaveChanges,revertChanges, andsaveDirectly.This description was created by
for 54c4916. You can customize this summary. It will automatically update as commits are pushed.