fix(diff-view): auto-scroll streamed content to bottom#10143
fix(diff-view): auto-scroll streamed content to bottom#10143village-way wants to merge 1 commit into
Conversation
Re-reviewed after new commit (7a42267). Two previously flagged issues are now resolved. One minor issue remains.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| }, | ||
| ) | ||
| // Timeout for slow machines | ||
| setTimeout(() => { |
There was a problem hiding this comment.
In openDiffEditor, the setTimeout timer isn’t cleared when the diff editor becomes active. Consider storing the timer ID and clearing the timeout inside the onDidChangeActiveTextEditor callback to avoid the potential of a late reject being triggered after resolution.
| } | ||
|
|
||
| // Open new diff editor. | ||
| // Open new diff editor (following Cline's simpler approach) |
There was a problem hiding this comment.
The comment currently refers to 'Cline's simpler approach'. For consistency with our product naming guidelines, consider updating this reference to 'Roo Code's simpler approach'.
| // Open new diff editor (following Cline's simpler approach) | |
| // Open new diff editor (following Roo Code's simpler approach) |
This comment was generated because it violated a code review rule: irule_VrRKWqywZ2YV2SOE.
| // Execute the diff command | ||
| vscode.commands.executeCommand( | ||
| "vscode.diff", | ||
| vscode.Uri.from({ | ||
| scheme: DIFF_VIEW_URI_SCHEME, | ||
| path: fileName, | ||
| query: Buffer.from(this.originalContent ?? "").toString("base64"), | ||
| }), | ||
| uri, | ||
| `${fileName}: ${fileExists ? DIFF_VIEW_LABEL_CHANGES : "New File"} (Editable)`, | ||
| { preserveFocus: true }, | ||
| ) |
There was a problem hiding this comment.
The old code pre-opened the file as a text document before executing the diff command. The removed comment explicitly stated this was needed to "fix issues with files that have custom editor associations (like markdown preview)". With this change, markdown files and other files with custom editor associations may not open correctly in the diff view. Consider testing with markdown files to verify this isn't a regression.
Fix it with Roo Code or mention @roomote and request a fix.
| // Timeout for slow machines | ||
| setTimeout(() => { | ||
| disposable.dispose() | ||
| reject(new Error("Failed to open diff editor, please try again...")) | ||
| }, 10_000) |
There was a problem hiding this comment.
The old implementation had explicit error handling for executeCommand failures that provided a meaningful error message (e.g., "Failed to execute diff command for : "). Now if the diff command fails, the promise will silently hang until the 10-second timeout with the generic message "Failed to open diff editor, please try again...", losing the specific error information that could help with debugging.
Fix it with Roo Code or mention @roomote and request a fix.
7639d07 to
1233c6b
Compare
Ensure the diff editor scrolls to the latest line when streaming long content and simplify diff view editor detection.
1233c6b to
7a42267
Compare
Ensure the diff editor scrolls to the latest line when streaming long content and simplify diff view editor detection.
Related GitHub Issue
Closes: #10142
Roo Code Task Context (Optional)
Description
This PR fixes an issue where the diff view does not automatically scroll to the bottom when streaming long content. The previous implementation had a condition that checked if the target line was within the visible range before scrolling, which prevented auto-scrolling when content extended beyond the viewport.
Key changes:
Simplified scroll logic: Removed the visibility range check that was preventing auto-scroll. Now the editor always scrolls to the current line being written, ensuring users can see the latest content as it streams.
Improved diff editor opening: Simplified the diff editor opening logic by using
onDidChangeActiveTextEditorevent listener, following a simpler and more reliable approach similar to Cline's implementation. This ensures the correct editor (right-side of diff view) is captured.Enhanced scroll method: Improved
scrollEditorToLinemethod with:: void)Added scroll animation helper: Added a
scrollAnimationmethod (for future use) that provides smooth scrolling between two lines, which can be helpful when tracking large content changes.Design choices:
Test Procedure
Manual testing steps:
Test with long content streaming:
Test with file modifications:
Test diff editor opening:
Test edge cases:
Expected results:
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
Important
Improves auto-scroll in diff view and simplifies diff editor detection in
DiffViewProvider.ts.update()inDiffViewProvider.tsto ensure auto-scroll to the latest line when streaming content.openDiffEditor()usingonDidChangeActiveTextEditorfor reliable editor detection.scrollAnimation()inDiffViewProvider.tsfor smooth scrolling between lines.scrollEditorToLine()with early return guard and return type annotation.openDiffEditor()for cleaner code.This description was created by
for 0bdfaf3. You can customize this summary. It will automatically update as commits are pushed.