Feature/autoscroll to user edits Closes #7996 #7998
Conversation
dosubot
bot
added
size:M
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| * @param relPath Relative path to the file | ||
| * @param userEdits The diff content showing user edits | ||
| */ | ||
| static async openAndScrollToEdits(cwd: string, relPath: string, userEdits: string): Promise<void> { |
There was a problem hiding this comment.
These new methods need test coverage. The existing test file at src/integrations/editor/__tests__/EditorUtils.spec.ts doesn't include tests for openAndScrollToEdits() and extractFirstChangedLineFromDiff(). Could you add tests to ensure these methods work correctly with various diff formats and edge cases?
| viewColumn: vscode.ViewColumn.Active, | ||
| }) | ||
| } catch (error) { | ||
| console.warn(`Failed to open and scroll to edits for ${relPath}:`, error) |
There was a problem hiding this comment.
Is the silent failure intentional here? Users won't know if the scroll operation fails. Would it be better to provide some user feedback, perhaps through a notification or status bar message?
| // Show the document with selection at the first changed line | ||
| const selection = | ||
| firstChangedLine !== null | ||
| ? new vscode.Selection(Math.max(firstChangedLine - 1, 0), 0, Math.max(firstChangedLine - 1, 0), 0) |
There was a problem hiding this comment.
Could we add validation to ensure the extracted line number is within the document bounds? If a diff has an invalid line number (e.g., line 1000 in a 100-line file), this could cause issues when trying to scroll to it.
| ? new vscode.Selection(Math.max(firstChangedLine - 1, 0), 0, Math.max(firstChangedLine - 1, 0), 0) | |
| const firstChangedLine = this.extractFirstChangedLineFromDiff(userEdits) | |
| if (!firstChangedLine) { | |
| console.warn(`No changes found in user edits for ${relPath}`) | |
| return | |
| } | |
| const document = await vscode.workspace.openTextDocument(fileUri) | |
| // Validate line number is within document bounds | |
| const validLine = Math.min(Math.max(firstChangedLine - 1, 0), document.lineCount - 1) | |
| // Show the document with selection at the first changed line | |
| const selection = new vscode.Selection(validLine, 0, validLine, 0) |
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
The JSDoc could be more detailed about the expected diff format. Could you specify that this expects a unified diff format and what happens if no valid hunk headers are found?
| await task.say("user_feedback_diff", JSON.stringify(say)) | ||
|
|
||
| // Open and focus the file, scrolling to the first edited section | ||
| await EditorUtils.openAndScrollToEdits(cwd, this.relPath, this.userEdits) |
There was a problem hiding this comment.
Would it make sense to add a configuration option to enable/disable the auto-scroll behavior? Some users might prefer not to have their view automatically changed after saving edits.
There was a problem hiding this comment.
I don't think it will be an issue. Reason being is that diff edits are not super common but when they do happen the user has already interrupted their workflow
|
I'm not sure why this is failing. Looks like a timeout idk. Please advise. core/prompts/tests/custom-system-prompt.spec.ts > File-Based Custom System Prompt > should use default generation when no file-based system prompt is found |
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
|
After review and testing, we've determined that while the auto-scrolling feature is a nice quality-of-life improvement, it adds unnecessary complexity to the codebase for a relatively minor benefit. The current behavior where users can manually scroll to their edits is sufficient. Thank you for the contribution and the effort put into this implementation! |
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
3 participants
Related GitHub Issue
Closes #7996
Description
This PR provides a straightforward way to focus the file and scroll to the user edits (if any). It does this by adding 2 new static class methods in EditorUtils. There is adequate error handling such that if there are errors (I could not get any errors to happen in testing) it is minimally impactful
Test Procedure
Pre-Submission Checklist
Screenshots / Videos
RooScrollToEdits.mov
Documentation Updates
Additional Notes
n/a
Get in Touch
jevons
Important
Adds feature to auto-scroll to user edits in a file using new methods in
EditorUtilsand integrates it intoDiffViewProvider.openAndScrollToEdits()andextractFirstChangedLineFromDiff()methods inEditorUtilsto open a file and scroll to the first user edit.openAndScrollToEdits()inDiffViewProviderto focus and scroll to user edits after saving changes.extractFirstChangedLineFromDiff()or ifopenAndScrollToEdits()fails.This description was created by
for 7fe2585. You can customize this summary. It will automatically update as commits are pushed.