-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Feature/autoscroll to user edits Closes #7996 #7998
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -207,4 +207,62 @@ export class EditorUtils { | |||||||||||||||||||||||||||
| return null | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Opens a file and scrolls to the first edited section when user edits are detected | ||||||||||||||||||||||||||||
| * @param cwd Current working directory | ||||||||||||||||||||||||||||
| * @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> { | ||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These new methods need test coverage. The existing test file at |
||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||
| const absolutePath = path.resolve(cwd, relPath) | ||||||||||||||||||||||||||||
| const fileUri = vscode.Uri.file(absolutePath) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // 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) | ||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
|
||||||||||||||||||||||||||||
| : undefined | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| await vscode.window.showTextDocument(document, { | ||||||||||||||||||||||||||||
| preview: false, | ||||||||||||||||||||||||||||
| selection, | ||||||||||||||||||||||||||||
| viewColumn: vscode.ViewColumn.Active, | ||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||
| console.warn(`Failed to open and scroll to edits for ${relPath}:`, error) | ||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||||||||||||||||||||||||||||
| * Extracts the first line number where changes occur from a unified diff | ||||||||||||||||||||||||||||
| * @param diffContent The unified diff content | ||||||||||||||||||||||||||||
| * @returns The first line number where changes occur, or null if no changes found | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| static extractFirstChangedLineFromDiff(diffContent: string): number | null { | ||||||||||||||||||||||||||||
| if (!diffContent || diffContent.trim() === "") { | ||||||||||||||||||||||||||||
| return null | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const lines = diffContent.split("\n") | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| for (const line of lines) { | ||||||||||||||||||||||||||||
| // Look for hunk headers like "@@ -1,4 +1,5 @@" | ||||||||||||||||||||||||||||
| const hunkMatch = line.match(/^@@\s+-(\d+)(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s+@@/) | ||||||||||||||||||||||||||||
| if (hunkMatch) { | ||||||||||||||||||||||||||||
| // Return the line number from the new file (the + side) | ||||||||||||||||||||||||||||
| return parseInt(hunkMatch[2], 10) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return null | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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