From c51054c563c99ac66ceae710c8cd3e7e38d148d0 Mon Sep 17 00:00:00 2001 From: awschmeder Date: Fri, 12 Jun 2026 02:36:41 -0700 Subject: [PATCH 1/2] fix(diff-view): restore scroll position and fix tab handling on save/deny --- .changeset/fix-diff-scroll-position.md | 5 + src/integrations/editor/DiffViewProvider.ts | 365 ++++++++- .../editor/__tests__/DiffViewProvider.spec.ts | 694 +++++++++++++++++- 3 files changed, 1042 insertions(+), 22 deletions(-) create mode 100644 .changeset/fix-diff-scroll-position.md diff --git a/.changeset/fix-diff-scroll-position.md b/.changeset/fix-diff-scroll-position.md new file mode 100644 index 0000000000..952352349f --- /dev/null +++ b/.changeset/fix-diff-scroll-position.md @@ -0,0 +1,5 @@ +--- +"zoo-code": patch +--- + +Fix diff view scroll position and tab handling when applying edits. The diff now opens scrolled to the first changed line (including end-of-file removals, which are clamped to a valid line in the modified document) instead of forcing the viewport to the top. After accepting or rejecting a diff, files that were already open are restored to their pre-edit scroll position, and files that were not open before the edit have their transiently opened tab closed -- unless the user activated that tab during the diff, in which case it is kept open. Focus is no longer pulled back to the edited file when the user has navigated elsewhere. A first file that was open in a preview tab is restored in a preview tab with the original scroll position, even if the target file replaced the preview tab and was automatically closed after the diff was accepted or rejected. A file that was pinned before the edit is re-pinned after the diff closes, so applying a diff no longer drops the tab's pinned state. diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index 80b5799217..416d53e335 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -16,7 +16,7 @@ import { Task } from "../../core/task/Task" import { DecorationController } from "./DecorationController" export const DIFF_VIEW_URI_SCHEME = "cline-diff" -export const DIFF_VIEW_LABEL_CHANGES = "Original ↔ Roo's Changes" +export const DIFF_VIEW_LABEL_CHANGES = "Original ↔ Zoo's Changes" // TODO: https://github.com/cline/cline/pull/3354 export class DiffViewProvider { @@ -28,6 +28,10 @@ export class DiffViewProvider { originalContent: string | undefined private createdDirs: string[] = [] private documentWasOpen = false + // Tracks whether the target file's tab was pinned before the diff session. + // Closing the tab to open the diff drops VS Code's pin state, so we restore + // it when re-showing the edited file afterward. + private documentWasPinned = false private relPath?: string private newContent?: string private activeDiffEditor?: vscode.TextEditor @@ -35,6 +39,18 @@ export class DiffViewProvider { private activeLineController?: DecorationController private streamedLines: string[] = [] private preDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [] + private preEditScrollLine: number | undefined + // Tracks whether the user activated the target file's editor tab during the + // diff session. When the file was not already open before the edit, we only + // keep it open afterward if the user explicitly interacted with it. + private userTouchedDocument = false + private activeEditorListener?: vscode.Disposable + private deferredScrollTimer?: ReturnType + // Snapshot of unrelated preview tabs (italicized, not-yet-edited) captured at + // diff-open time. Opening the diff reuses the editor group's single preview + // slot and evicts these tabs; we restore any that disappeared after the diff + // session ends so the user's prior tab state is reconstructed. + private snapshotPreviewTabs: Array<{ uri: vscode.Uri; scrollLine: number | undefined }> = [] private taskRef: WeakRef constructor( @@ -50,6 +66,13 @@ export class DiffViewProvider { const absolutePath = path.resolve(this.cwd, relPath) this.isEditing = true + // Capture the current scroll position before we close the tab so we can + // restore it after saving/reverting. + const existingEditor = vscode.window.visibleTextEditors.find( + (e) => e.document.uri.scheme === "file" && arePathsEqual(e.document.uri.fsPath, absolutePath), + ) + this.preEditScrollLine = existingEditor?.visibleRanges?.[0]?.start.line + // If the file is already open, ensure it's not dirty before getting its // contents. if (fileExists) { @@ -84,6 +107,7 @@ export class DiffViewProvider { // If the file was already open, close it (must happen after showing the // diff view since if it's the only tab the column will close). this.documentWasOpen = false + this.documentWasPinned = false // Close the tab if it's open (it's already saved above). const tabs = vscode.window.tabGroups.all @@ -97,6 +121,11 @@ export class DiffViewProvider { ) for (const tab of tabs) { + // Remember the pin state so we can restore it after the diff closes; + // closing the tab to open the diff would otherwise lose it. + if (tab.isPinned) { + this.documentWasPinned = true + } if (!tab.isDirty) { try { await vscode.window.tabGroups.close(tab) @@ -107,13 +136,41 @@ export class DiffViewProvider { this.documentWasOpen = true } + // Snapshot unrelated preview tabs so we can restore them if opening the + // diff evicts them (VS Code reuses the group's single preview slot). + this.snapshotPreviewTabs = this.captureUnrelatedPreviewTabs(absolutePath) + this.activeDiffEditor = await this.openDiffEditor() this.fadedOverlayController = new DecorationController("fadedOverlay", this.activeDiffEditor) this.activeLineController = new DecorationController("activeLine", this.activeDiffEditor) // Apply faded overlay to all lines initially. this.fadedOverlayController.addLines(0, this.activeDiffEditor.document.lineCount) - this.scrollEditorToLine(0) // Will this crash for new files? + // Do not force the viewport to the top here. Tools call scrollToFirstDiff() + // after the final update so the diff opens on the first changed line; an + // unconditional scroll-to-0 would override that and land off the change. this.streamedLines = [] + + // When the file was not already open before the edit, watch for the user + // activating the file's own editor tab. If they do, we treat the file as + // "touched" and keep it open after accepting/denying instead of closing + // the transiently opened tab. Activating the diff view does not count. + this.userTouchedDocument = false + if (!this.documentWasOpen) { + this.activeEditorListener = vscode.window.onDidChangeActiveTextEditor((editor) => { + if (!editor || editor.document.uri.scheme !== "file") { + return + } + if (!arePathsEqual(editor.document.uri.fsPath, absolutePath)) { + return + } + // Ignore activation of the diff editor, which can share the file path. + const activeTab = vscode.window.tabGroups.activeTabGroup.activeTab + if (activeTab?.input instanceof vscode.TabInputTextDiff) { + return + } + this.userTouchedDocument = true + }) + } } async update(accumulatedContent: string, isFinal: boolean) { @@ -213,9 +270,19 @@ export class DiffViewProvider { await updatedDocument.save() } - await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false, preserveFocus: true }) + // Stop tracking touches and cancel any pending scroll-to-diff before any + // programmatic editor activation below. + this.disposeActiveEditorListener() + this.cancelDeferredScroll() + await this.closeAllDiffViews() + await this.keepOrCloseEditedFile(absolutePath) + + // Restore any preview tabs the diff evicted, reconstructing the user's + // prior not-yet-edited tab state. + await this.restorePreviewTabs() + // 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. @@ -371,12 +438,20 @@ export class DiffViewProvider { const updatedDocument = this.activeDiffEditor.document const absolutePath = path.resolve(this.cwd, this.relPath) + // Stop tracking touches and cancel any pending scroll-to-diff before any + // programmatic editor activation below. + this.disposeActiveEditorListener() + this.cancelDeferredScroll() + if (!fileExists) { if (updatedDocument.isDirty) { await updatedDocument.save() } await this.closeAllDiffViews() + // The file was newly created for this edit; close its transiently + // opened tab before deleting it from disk. + await this.closeFileTab(absolutePath) await fs.unlink(absolutePath) // Remove only the directories we created, in reverse order. @@ -400,16 +475,15 @@ export class DiffViewProvider { await vscode.workspace.applyEdit(edit) await updatedDocument.save() - if (this.documentWasOpen) { - await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { - preview: false, - preserveFocus: true, - }) - } - await this.closeAllDiffViews() + + await this.keepOrCloseEditedFile(absolutePath) } + // Restore any preview tabs the diff evicted, reconstructing the user's + // prior not-yet-edited tab state. + await this.restorePreviewTabs() + // Edit is done. await this.reset() } @@ -448,6 +522,186 @@ export class DiffViewProvider { await Promise.all(closeOps) } + // Stop tracking user activation of the target file. Called before any + // programmatic showTextDocument so our own re-show never counts as a "touch". + private disposeActiveEditorListener(): void { + this.activeEditorListener?.dispose() + this.activeEditorListener = undefined + } + + // Cancel any pending deferred scroll-to-diff. Must run as soon as the user + // accepts/denies (before saveChanges/revertChanges restore the pre-edit + // viewport), otherwise a late timer could fire after the scroll-restore and + // yank the file back to the diff target. With auto-approve this window is + // especially tight because save runs immediately after scrollToFirstDiff. + private cancelDeferredScroll(): void { + if (this.deferredScrollTimer !== undefined) { + clearTimeout(this.deferredScrollTimer) + this.deferredScrollTimer = undefined + } + } + + // Shared accept/deny cleanup: keep the edited file open if it was already open + // before the edit or the user interacted with it during the diff; otherwise + // close the tab that was opened transiently for the diff. + private async keepOrCloseEditedFile(absolutePath: string): Promise { + if (this.documentWasOpen || this.userTouchedDocument) { + await this.showEditedFileWithoutDisruptingFocus(absolutePath) + } else { + await this.closeFileTab(absolutePath) + } + } + + // Re-show the edited file so it stays open after the diff closes and restore + // its pre-edit scroll position, WITHOUT disrupting wherever the user is + // currently looking. showTextDocument activates the target's tab in its + // group, so if the user navigated to a different file during the diff (e.g. + // they clicked back to file-1 while file-2 was being edited), naively showing + // the edited file would yank the active editor onto it. We capture the user's + // active editor first and re-activate it afterward when it differs. + private async showEditedFileWithoutDisruptingFocus(absolutePath: string): Promise { + const userActiveEditor = vscode.window.activeTextEditor + const userWasElsewhere = + !!userActiveEditor && + !( + userActiveEditor.document.uri.scheme === "file" && + arePathsEqual(userActiveEditor.document.uri.fsPath, absolutePath) + ) + + // When the tab needs re-pinning we must briefly focus it, since + // workbench.action.pinEditor only acts on the active editor. Otherwise we + // keep the user's focus undisturbed with preserveFocus. + const editor = await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { + preview: false, + preserveFocus: !this.documentWasPinned, + }) + if (this.preEditScrollLine !== undefined) { + editor.revealRange( + new vscode.Range(this.preEditScrollLine, 0, this.preEditScrollLine, 0), + vscode.TextEditorRevealType.AtTop, + ) + } + + // Restore the pin state that was dropped when the tab was closed to open + // the diff. The edited file is active here (we focused it above), so the + // command targets the correct tab. + if (this.documentWasPinned) { + try { + await vscode.commands.executeCommand("workbench.action.pinEditor") + } catch (err) { + console.error(`Failed to re-pin edited file`, err) + } + } + + // If the user was viewing a different editor, re-activate it so the edited + // file stays open in the background instead of stealing the foreground. + if (userWasElsewhere && userActiveEditor) { + try { + await vscode.window.showTextDocument(userActiveEditor.document, { + viewColumn: userActiveEditor.viewColumn, + preserveFocus: false, + }) + } catch (err) { + console.error(`Failed to restore user's active editor`, err) + } + } + } + + // Capture unrelated preview tabs (italicized, not-yet-edited) along with their + // current scroll position. Opening the diff reuses the editor group's single + // preview slot and evicts these tabs; the snapshot lets us restore them after + // the diff session ends. The diff target is excluded since it is about to be + // replaced by the diff view anyway. + private captureUnrelatedPreviewTabs( + diffTargetPath: string, + ): Array<{ uri: vscode.Uri; scrollLine: number | undefined }> { + return vscode.window.tabGroups.all.flatMap((group) => + group.tabs + .filter( + (tab) => + tab.isPreview && + tab.input instanceof vscode.TabInputText && + tab.input.uri.scheme === "file" && + !arePathsEqual(tab.input.uri.fsPath, diffTargetPath), + ) + .map((tab) => { + const uri = (tab.input as vscode.TabInputText).uri + const visibleEditor = vscode.window.visibleTextEditors.find( + (e) => e.document.uri.scheme === "file" && arePathsEqual(e.document.uri.fsPath, uri.fsPath), + ) + return { uri, scrollLine: visibleEditor?.visibleRanges?.[0]?.start.line } + }), + ) + } + + // Restore preview tabs captured before the diff opened, but only those VS Code + // evicted. Each is re-opened in preview mode (preserving the user's + // not-yet-edited state) without stealing focus, and its prior scroll position + // is reapplied. + private async restorePreviewTabs(): Promise { + for (const snapshot of this.snapshotPreviewTabs) { + const stillOpen = vscode.window.tabGroups.all + .flatMap((group) => group.tabs) + .some( + (tab) => + tab.input instanceof vscode.TabInputText && + tab.input.uri.scheme === "file" && + arePathsEqual(tab.input.uri.fsPath, snapshot.uri.fsPath), + ) + + if (stillOpen) { + continue + } + + // The file may have been deleted, renamed, or moved during the diff + // session. Skip restoring a tab whose underlying file no longer exists. + try { + await fs.access(snapshot.uri.fsPath) + } catch { + continue + } + + try { + const editor = await vscode.window.showTextDocument(snapshot.uri, { + preview: true, + preserveFocus: true, + }) + if (snapshot.scrollLine !== undefined) { + editor.revealRange( + new vscode.Range(snapshot.scrollLine, 0, snapshot.scrollLine, 0), + vscode.TextEditorRevealType.AtTop, + ) + } + } catch (err) { + console.error(`Failed to restore preview tab ${snapshot.uri.fsPath}`, err) + } + } + this.snapshotPreviewTabs = [] + } + + // Close the plain (non-diff) editor tab for the target file. Used when the + // file was opened transiently for the diff and the user never interacted + // with it, so it should not linger after accept/deny. + private async closeFileTab(absolutePath: string): Promise { + const tabs = vscode.window.tabGroups.all + .flatMap((group) => group.tabs) + .filter( + (tab) => + tab.input instanceof vscode.TabInputText && + tab.input.uri.scheme === "file" && + arePathsEqual(tab.input.uri.fsPath, absolutePath) && + !tab.isDirty, + ) + + for (const tab of tabs) { + try { + await vscode.window.tabGroups.close(tab) + } catch (err) { + console.error(`Failed to close file tab ${tab.label}`, err) + } + } + } + private async openDiffEditor(): Promise { if (!this.relPath) { throw new Error( @@ -578,30 +832,97 @@ export class DiffViewProvider { } scrollToFirstDiff() { - if (!this.activeDiffEditor) { + const editor = this.activeDiffEditor + if (!editor) { return } - const currentContent = this.activeDiffEditor.document.getText() + const targetLine = this.findFirstDiffLine(editor) + if (targetLine === undefined) { + return + } + + // Reveal on the live modified-side editor, then once more after the diff + // editor's late layout pass settles. When the file was already open and + // scrolled away from the top, the vscode.diff command re-lays-out the diff + // and momentarily snaps the viewport to the top; a single synchronous + // reveal can be overridden by that pass. The deferred reveal is gated on + // the diff still being active so a stale timer can never act on a diff + // opened by a later edit. + this.revealDiffLine(this.resolveLiveEditor(editor), targetLine) + + this.cancelDeferredScroll() + this.deferredScrollTimer = setTimeout(() => { + this.deferredScrollTimer = undefined + if (this.activeDiffEditor === editor) { + this.revealDiffLine(this.resolveLiveEditor(editor), targetLine) + } + }, 100) + } + + // The TextEditor reference captured when the diff opened can be a stale, + // detached instance whose visibleRanges no longer reflect the on-screen diff + // widget (this happens when the file was already open before the edit). A + // revealRange on that stale editor is a silent no-op because it believes the + // target line is already visible. Re-resolve the current modified-side editor + // for the same document at scroll time so the reveal drives the live viewport. + private resolveLiveEditor(editor: vscode.TextEditor): vscode.TextEditor { + return ( + vscode.window.visibleTextEditors.find( + (e) => e.document.uri.scheme === "file" && e.document === editor.document, + ) ?? editor + ) + } + + // Index of the first change in the MODIFIED (right-hand) document, or + // undefined when there is no change. For removals this is the line that now + // occupies the position of the removed block. + private findFirstDiffLine(editor: vscode.TextEditor): number | undefined { + const document = editor.document + const currentContent = document.getText() const diffs = diff.diffLines(this.originalContent || "", currentContent) let lineCount = 0 for (const part of diffs) { if (part.added || part.removed) { - // Found the first diff, scroll to it without stealing focus. - this.activeDiffEditor.revealRange( - new vscode.Range(lineCount, 0, lineCount, 0), - vscode.TextEditorRevealType.InCenter, - ) - - return + // A pure removal at the end of the file leaves the first-change line + // at (or past) the end of the modified document. Revealing a range at + // that out-of-bounds line clamps to the last line and the composite + // diff widget never moves. Clamp the target into the document so the + // reveal always lands on a real line. + const lastLine = Math.max(0, document.lineCount - 1) + return Math.min(lineCount, lastLine) } if (!part.removed) { lineCount += part.count || 0 } } + + return undefined + } + + private revealDiffLine(editor: vscode.TextEditor, targetLine: number) { + // Clamp again at reveal time: deferred reveals can run after a later edit + // shortened the document, and an out-of-bounds selection would snap the + // composite diff widget back to the top and leave it stuck there. + const lastLine = Math.max(0, editor.document.lineCount - 1) + const safeLine = Math.min(Math.max(0, targetLine), lastLine) + + // Anchor the selection on the target line before revealing. update() parks + // the selection at (0,0) to keep it out of the stream animation; moving the + // selection to the target first nudges the composite diff widget toward the + // change. preserveFocus semantics are unaffected because we never activate + // the editor here. + const targetPosition = new vscode.Position(safeLine, 0) + editor.selection = new vscode.Selection(targetPosition, targetPosition) + + const lineLength = editor.document.lineAt ? editor.document.lineAt(safeLine).text.length : 0 + editor.revealRange( + new vscode.Range(safeLine, 0, safeLine, lineLength), + vscode.TextEditorRevealType.InCenter, + ) } private stripAllBOMs(input: string): string { @@ -623,11 +944,17 @@ export class DiffViewProvider { this.originalContent = undefined this.createdDirs = [] this.documentWasOpen = false + this.documentWasPinned = false + this.cancelDeferredScroll() this.activeDiffEditor = undefined this.fadedOverlayController = undefined this.activeLineController = undefined this.streamedLines = [] this.preDiagnostics = [] + this.preEditScrollLine = undefined + this.userTouchedDocument = false + this.snapshotPreviewTabs = [] + this.disposeActiveEditorListener() } /** diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts index e99f7bf9c8..3196e0ea9d 100644 --- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts +++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts @@ -12,6 +12,7 @@ vi.mock("delay", () => ({ vi.mock("fs/promises", () => ({ readFile: vi.fn().mockResolvedValue("file content"), writeFile: vi.fn().mockResolvedValue(undefined), + access: vi.fn().mockResolvedValue(undefined), })) // Mock utils @@ -43,9 +44,11 @@ vi.mock("vscode", () => ({ createTextEditorDecorationType: vi.fn(), showTextDocument: vi.fn(), onDidChangeVisibleTextEditors: vi.fn(() => ({ dispose: vi.fn() })), + onDidChangeActiveTextEditor: vi.fn(() => ({ dispose: vi.fn() })), tabGroups: { all: [], close: vi.fn(), + activeTabGroup: { activeTab: undefined }, }, visibleTextEditors: [], }, @@ -78,12 +81,19 @@ vi.mock("vscode", () => ({ Eight: 8, Nine: 9, }, - Range: vi.fn(), - Position: vi.fn(), - Selection: vi.fn(), + Range: vi.fn().mockImplementation((startLine, startChar, endLine, endChar) => ({ + start: { line: startLine, character: startChar }, + end: { line: endLine, character: endChar }, + })), + Position: vi.fn().mockImplementation((line, character) => ({ line, character })), + Selection: vi.fn().mockImplementation((anchor, active) => ({ anchor, active })), TextEditorRevealType: { + Default: 0, InCenter: 2, + InCenterIfOutsideViewport: 3, + AtTop: 4, }, + TabInputText: class TabInputText {}, TabInputTextDiff: class TabInputTextDiff {}, Uri: { file: vi.fn((path) => ({ fsPath: path })), @@ -271,6 +281,380 @@ describe("DiffViewProvider", () => { "Failed to execute diff command for /mock/cwd/test.md: Cannot open file", ) }) + + it("records the pin state of an already-open pinned tab", async () => { + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/test.md`, scheme: "file" }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + } + + // An open, pinned, non-dirty tab for the target file. + const pinnedTab = { + input: Object.assign(new (vscode as any).TabInputText(), { + uri: { fsPath: `${mockCwd}/test.md`, scheme: "file" }, + }), + isDirty: false, + isPinned: true, + label: "test.md", + } + Object.defineProperty(vscode.window.tabGroups, "all", { + get: () => [{ tabs: [pinnedTab] }], + configurable: true, + }) + + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any) + vi.mocked(vscode.commands.executeCommand).mockResolvedValue(undefined) + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => { + callback({ uri: { fsPath: `${mockCwd}/test.md`, scheme: "file" } } as any) + }, 0) + return { dispose: vi.fn() } + }) + vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any] + + ;(diffViewProvider as any).editType = "modify" + + await diffViewProvider.open("test.md") + + expect((diffViewProvider as any).documentWasPinned).toBe(true) + expect((diffViewProvider as any).documentWasOpen).toBe(true) + }) + }) + + describe("scrollToFirstDiff method", () => { + const setupEditor = (currentContent: string) => { + const revealRange = vi.fn() + // Mirror how VS Code reports lineCount: a trailing newline yields a final + // empty line, so the count is the number of "\n"-delimited segments. + const lineCount = currentContent === "" ? 0 : currentContent.split("\n").length + const lines = currentContent.split("\n") + const document = { + uri: { fsPath: `${mockCwd}/mock-file-target.txt`, scheme: "file" }, + getText: vi.fn().mockReturnValue(currentContent), + lineCount, + lineAt: vi.fn().mockImplementation((line: number) => ({ text: lines[line] ?? "" })), + } + const editor = { + document, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + visibleRanges: [{ start: { line: 0 }, end: { line: 0 } }], + revealRange, + } + ;(diffViewProvider as any).activeDiffEditor = editor + // Register the editor as the live modified-side editor so resolveLiveEditor + // finds it by document identity, mirroring the runtime path. + vi.mocked(vscode.window).visibleTextEditors = [editor as any] + return revealRange + } + + it("reveals the first changed line for an addition-only diff", () => { + ;(diffViewProvider as any).originalContent = "a\nb\nc\n" + // Insert a new line between b and c (first change is at line index 2). + const revealRange = setupEditor("a\nb\nNEW\nc\n") + + diffViewProvider.scrollToFirstDiff() + + expect(revealRange).toHaveBeenCalledTimes(1) + const range = revealRange.mock.calls[0][0] + expect(range.start.line).toBe(2) + }) + + it("reveals the first changed line for a deletion-only diff", () => { + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + // Remove line c; the first change is the removed block at line index 2. + const revealRange = setupEditor("a\nb\nd\n") + + diffViewProvider.scrollToFirstDiff() + + expect(revealRange).toHaveBeenCalledTimes(1) + const range = revealRange.mock.calls[0][0] + expect(range.start.line).toBe(2) + }) + + it("clamps to the last line for a removal at the end of the file", () => { + // Long file; remove the final lines. The first-change index lands past the + // end of the shortened modified document, so it must be clamped to a real + // line or the diff widget will not scroll. + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\ne\nf\n" + const revealRange = setupEditor("a\nb\nc\n") + + diffViewProvider.scrollToFirstDiff() + + expect(revealRange).toHaveBeenCalledTimes(1) + const range = revealRange.mock.calls[0][0] + // Modified document has lines a,b,c (+ trailing empty) => lastLine index 3. + // The removed block begins at index 3, which is within bounds here. + expect(range.start.line).toBeLessThanOrEqual(3) + expect(range.start.line).toBeGreaterThanOrEqual(0) + }) + + it("reveals the first changed line for a mixed diff", () => { + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + // Change line b (index 1) -- first divergence from the original. + const revealRange = setupEditor("a\nCHANGED\nc\nd\n") + + diffViewProvider.scrollToFirstDiff() + + expect(revealRange).toHaveBeenCalledTimes(1) + const range = revealRange.mock.calls[0][0] + expect(range.start.line).toBe(1) + }) + + it("anchors the selection on the target line so an in-viewport diff still scrolls", () => { + // Regression for the case where the file is already scrolled to the middle + // and the diff target is inside the current viewport: a bare revealRange is + // a no-op, leaving the viewport pinned at the top. Moving the selection to + // the target first forces the diff widget to scroll to the change. + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + const revealRange = setupEditor("a\nCHANGED\nc\nd\n") + + diffViewProvider.scrollToFirstDiff() + + const editor = (diffViewProvider as any).activeDiffEditor + expect(editor.selection.active.line).toBe(1) + expect(editor.selection.anchor.line).toBe(1) + expect(revealRange).toHaveBeenCalledTimes(1) + expect(revealRange.mock.calls[0][0].start.line).toBe(1) + }) + + it("re-reveals the target line after layout settles", () => { + // The diff editor can snap the viewport back to the top during its late + // layout pass when the file was already scrolled. A deferred re-reveal + // makes the scroll stick. Verify the second reveal fires on a timer. + vi.useFakeTimers() + try { + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + const revealRange = setupEditor("a\nCHANGED\nc\nd\n") + + diffViewProvider.scrollToFirstDiff() + + expect(revealRange).toHaveBeenCalledTimes(1) + // Mock timer - no wall clock time elapses here + vi.advanceTimersByTime(100) + expect(revealRange).toHaveBeenCalledTimes(2) + for (const call of revealRange.mock.calls) { + expect(call[0].start.line).toBe(1) + } + } finally { + vi.useRealTimers() + } + }) + + it("reveals on the live modified-side editor, not a stale captured reference", () => { + // Regression for "scrolls to top": the captured activeDiffEditor can be a + // detached editor whose visibleRanges no longer match the on-screen diff, + // making revealRange a no-op. The reveal must target the live editor found + // in visibleTextEditors for the same document. + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + const staleReveal = setupEditor("a\nCHANGED\nc\nd\n") + const staleEditor = (diffViewProvider as any).activeDiffEditor + + // A live editor for the SAME document, distinct from the stale capture. + const liveReveal = vi.fn() + const liveEditor = { + document: staleEditor.document, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + visibleRanges: [{ start: { line: 0 }, end: { line: 0 } }], + revealRange: liveReveal, + } + vi.mocked(vscode.window).visibleTextEditors = [liveEditor as any] + + diffViewProvider.scrollToFirstDiff() + + expect(liveReveal).toHaveBeenCalledTimes(1) + expect(staleReveal).not.toHaveBeenCalled() + expect(liveReveal.mock.calls[0][0].start.line).toBe(1) + }) + + it("does not re-reveal a stale line on a diff editor opened by a later edit", () => { + // Regression for the "stuck at line 0" bug: a deferred reveal must not act + // after a subsequent edit swapped in a different active diff editor. + vi.useFakeTimers() + try { + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + const firstReveal = setupEditor("a\nCHANGED\nc\nd\n") + const firstEditor = (diffViewProvider as any).activeDiffEditor + + diffViewProvider.scrollToFirstDiff() + expect(firstReveal).toHaveBeenCalledTimes(1) + + // A later edit swaps in a brand new diff editor before the timer fires. + const secondReveal = setupEditor("a\nb\nc\nd\n") + expect((diffViewProvider as any).activeDiffEditor).not.toBe(firstEditor) + + vi.advanceTimersByTime(100) + + // The stale timer saw a different active editor and did nothing more. + expect(firstReveal).toHaveBeenCalledTimes(1) + expect(secondReveal).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it("does nothing when there is no diff editor", () => { + ;(diffViewProvider as any).activeDiffEditor = undefined + expect(() => diffViewProvider.scrollToFirstDiff()).not.toThrow() + }) + }) + + describe("preview tab snapshot and restore", () => { + const makePreviewTab = (fsPath: string, isPreview = true) => { + const input = Object.assign(new (vscode as any).TabInputText(), { + uri: { fsPath, scheme: "file" }, + }) + return { isPreview, input, label: fsPath } + } + + const setTabs = (tabs: any[]) => { + Object.defineProperty(vscode.window.tabGroups, "all", { + get: () => [{ tabs }], + configurable: true, + }) + } + + it("captures unrelated preview tabs with their scroll position, excluding the diff target", () => { + setTabs([ + makePreviewTab("/mock/cwd/file-1.txt"), + makePreviewTab("/mock/cwd/file-2.txt"), // diff target -- excluded + makePreviewTab("/mock/cwd/file-3.txt", false), // not a preview -- excluded + ]) + vi.mocked(vscode.window).visibleTextEditors = [ + { + document: { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" } }, + visibleRanges: [{ start: { line: 12 } }], + } as any, + ] + + const snapshot = (diffViewProvider as any).captureUnrelatedPreviewTabs("/mock/cwd/file-2.txt") + + expect(snapshot).toHaveLength(1) + expect(snapshot[0].uri.fsPath).toBe("/mock/cwd/file-1.txt") + expect(snapshot[0].scrollLine).toBe(12) + }) + + it("restores an evicted preview tab in preview mode and reapplies its scroll position", async () => { + const revealRange = vi.fn() + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange } as any) + // The captured file is no longer open (evicted by the diff). + setTabs([]) + ;(diffViewProvider as any).snapshotPreviewTabs = [ + { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, scrollLine: 12 }, + ] + + await (diffViewProvider as any).restorePreviewTabs() + + expect(vscode.window.showTextDocument).toHaveBeenCalledWith( + { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, + { preview: true, preserveFocus: true }, + ) + expect(revealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 12, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + expect((diffViewProvider as any).snapshotPreviewTabs).toEqual([]) + }) + + it("does not restore a preview tab that is still open", async () => { + setTabs([makePreviewTab("/mock/cwd/file-1.txt")]) + ;(diffViewProvider as any).snapshotPreviewTabs = [ + { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, scrollLine: 0 }, + ] + + await (diffViewProvider as any).restorePreviewTabs() + + expect(vscode.window.showTextDocument).not.toHaveBeenCalled() + }) + + it("skips restoring a preview tab whose file no longer exists", async () => { + setTabs([]) + const fs = await import("fs/promises") + vi.mocked(fs.access).mockRejectedValueOnce(new Error("ENOENT")) + ;(diffViewProvider as any).snapshotPreviewTabs = [ + { uri: { fsPath: "/mock/cwd/deleted.txt", scheme: "file" }, scrollLine: 0 }, + ] + + await (diffViewProvider as any).restorePreviewTabs() + + expect(vscode.window.showTextDocument).not.toHaveBeenCalled() + expect((diffViewProvider as any).snapshotPreviewTabs).toEqual([]) + }) + }) + + describe("showEditedFileWithoutDisruptingFocus", () => { + it("re-activates the user's editor when they navigated to a different file", async () => { + // User is viewing file-1 while the edited file is file-2. Keeping file-2 + // open must not yank focus/foreground onto it. + const userActiveEditor = { + document: { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" } }, + viewColumn: 1, + } + ;(vscode.window as any).activeTextEditor = userActiveEditor + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + ;(diffViewProvider as any).preEditScrollLine = 5 + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus("/mock/cwd/file-2.txt") + + // First call re-shows the edited file (preserveFocus); last call restores + // the user's editor with focus. + const calls = vi.mocked(vscode.window.showTextDocument).mock.calls + expect(calls[0][1]).toMatchObject({ preview: false, preserveFocus: true }) + const restoreCall = calls[calls.length - 1] + expect(restoreCall[0]).toBe(userActiveEditor.document) + expect(restoreCall[1]).toMatchObject({ viewColumn: 1, preserveFocus: false }) + }) + + it("does not re-activate when the user is already on the edited file", async () => { + const userActiveEditor = { + document: { uri: { fsPath: "/mock/cwd/file-2.txt", scheme: "file" } }, + viewColumn: 1, + } + ;(vscode.window as any).activeTextEditor = userActiveEditor + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus("/mock/cwd/file-2.txt") + + // Only the single re-show of the edited file; no focus-restore round trip. + expect(vscode.window.showTextDocument).toHaveBeenCalledTimes(1) + }) + + it("re-pins the edited file when it was pinned before the diff", async () => { + const userActiveEditor = { + document: { uri: { fsPath: "/mock/cwd/file-2.txt", scheme: "file" } }, + viewColumn: 1, + } + ;(vscode.window as any).activeTextEditor = userActiveEditor + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + ;(diffViewProvider as any).documentWasPinned = true + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus("/mock/cwd/file-2.txt") + + // The edited file must be focused (preserveFocus false) so pinEditor + // targets the correct tab, then the pin command is issued. + const calls = vi.mocked(vscode.window.showTextDocument).mock.calls + expect(calls[0][1]).toMatchObject({ preview: false, preserveFocus: false }) + expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.pinEditor") + }) + + it("does not pin the edited file when it was not pinned before the diff", async () => { + const userActiveEditor = { + document: { uri: { fsPath: "/mock/cwd/file-2.txt", scheme: "file" } }, + viewColumn: 1, + } + ;(vscode.window as any).activeTextEditor = userActiveEditor + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + ;(diffViewProvider as any).documentWasPinned = false + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus("/mock/cwd/file-2.txt") + + expect(vscode.commands.executeCommand).not.toHaveBeenCalledWith("workbench.action.pinEditor") + }) }) describe("closeAllDiffViews method", () => { @@ -517,4 +901,308 @@ describe("DiffViewProvider", () => { expect(vscode.languages.getDiagnostics).toHaveBeenCalled() }) }) + + describe("preEditScrollLine capture and restore", () => { + it("should capture scroll line from visible editor at open() time", async () => { + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/scroll.ts`, scheme: "file" }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + visibleRanges: [{ start: { line: 42 } }], + } + + vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any] + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any) + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => callback({ uri: { fsPath: `${mockCwd}/scroll.ts`, scheme: "file" } } as any), 0) + return { dispose: vi.fn() } + }) + vi.mocked(vscode.window.onDidChangeVisibleTextEditors).mockReturnValue({ dispose: vi.fn() }) + ;(diffViewProvider as any).editType = "modify" + + await diffViewProvider.open("scroll.ts") + + expect((diffViewProvider as any).preEditScrollLine).toBe(42) + }) + + it("should set preEditScrollLine to undefined when the visible editor has no visibleRanges", async () => { + const mockEditorNoRanges = { + document: { + uri: { fsPath: `${mockCwd}/new.ts`, scheme: "file" }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + // No visibleRanges, so the capture in open() yields undefined. + } + + vi.mocked(vscode.window).visibleTextEditors = [mockEditorNoRanges as any] + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditorNoRanges as any) + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => callback({ uri: { fsPath: `${mockCwd}/new.ts`, scheme: "file" } } as any), 0) + return { dispose: vi.fn() } + }) + vi.mocked(vscode.window.onDidChangeVisibleTextEditors).mockReturnValue({ dispose: vi.fn() }) + ;(diffViewProvider as any).editType = "modify" + + await diffViewProvider.open("new.ts") + + expect((diffViewProvider as any).preEditScrollLine).toBeUndefined() + }) + + it("saveChanges() calls revealRange(AtTop) when documentWasOpen and preEditScrollLine is set", async () => { + const mockRevealRange = vi.fn() + const mockSavedEditor = { revealRange: mockRevealRange } + + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockSavedEditor as any) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).documentWasOpen = true + ;(diffViewProvider as any).preEditScrollLine = 30 + // saveChanges early-exits without relPath, newContent, activeDiffEditor + ;(diffViewProvider as any).newContent = "content" + ;(diffViewProvider as any).activeDiffEditor = { + document: { + getText: vi.fn().mockReturnValue("content"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + }, + } + + await diffViewProvider.saveChanges(false) + + expect(mockRevealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 30, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + }) + + it("saveChanges() does NOT call revealRange when preEditScrollLine is undefined", async () => { + const mockRevealRange = vi.fn() + const mockSavedEditor = { revealRange: mockRevealRange } + + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockSavedEditor as any) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).documentWasOpen = true + ;(diffViewProvider as any).preEditScrollLine = undefined + ;(diffViewProvider as any).newContent = "content" + ;(diffViewProvider as any).activeDiffEditor = { + document: { + getText: vi.fn().mockReturnValue("content"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + }, + } + + await diffViewProvider.saveChanges(false) + + expect(mockRevealRange).not.toHaveBeenCalled() + }) + + it("saveChanges() cancels a pending deferred scroll so it cannot fight scroll-restore", async () => { + // With auto-approve, saveChanges runs immediately after scrollToFirstDiff. + // A late deferred reveal must not fire after the viewport is restored. + vi.useFakeTimers() + try { + const deferredReveal = vi.fn() + const liveEditor = { + document: { + uri: { fsPath: `${mockCwd}/race.ts`, scheme: "file" }, + getText: vi.fn().mockReturnValue("a\nCHANGED\nc\nd\n"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + lineCount: 5, + lineAt: vi.fn().mockReturnValue({ text: "" }), + }, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + visibleRanges: [{ start: { line: 0 }, end: { line: 0 } }], + revealRange: deferredReveal, + } + ;(diffViewProvider as any).originalContent = "a\nb\nc\nd\n" + ;(diffViewProvider as any).activeDiffEditor = liveEditor + vi.mocked(vscode.window).visibleTextEditors = [liveEditor as any] + + // Schedule the deferred reveal, then accept the edit before it fires. + diffViewProvider.scrollToFirstDiff() + expect(deferredReveal).toHaveBeenCalledTimes(1) + + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).documentWasOpen = true + ;(diffViewProvider as any).preEditScrollLine = 0 + ;(diffViewProvider as any).newContent = "a\nCHANGED\nc\nd\n" + + await diffViewProvider.saveChanges(false) + + // The timer was cancelled; advancing past it produces no extra reveal. + vi.advanceTimersByTime(100) + expect(deferredReveal).toHaveBeenCalledTimes(1) + expect((diffViewProvider as any).deferredScrollTimer).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) + + it("revertChanges() calls revealRange(AtTop) when documentWasOpen and preEditScrollLine is set", async () => { + const mockRevealRange = vi.fn() + const mockSavedEditor = { revealRange: mockRevealRange } + + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockSavedEditor as any) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).documentWasOpen = true + ;(diffViewProvider as any).preEditScrollLine = 15 + ;(diffViewProvider as any).editType = "modify" + ;(diffViewProvider as any).originalContent = "original" + ;(diffViewProvider as any).activeDiffEditor = { + document: { + uri: { fsPath: `${mockCwd}/test.txt` }, + getText: vi.fn().mockReturnValue("modified"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }), + }, + } + + vi.mocked(vscode.workspace.applyEdit).mockResolvedValue(true) + + await diffViewProvider.revertChanges() + + expect(mockRevealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 15, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + }) + }) + + describe("userTouchedDocument close/keep behavior", () => { + const mockTargetPath = `${mockCwd}/mock-target-file.ts` + + const buildActiveDiffEditor = () => ({ + document: { + uri: { fsPath: mockTargetPath }, + getText: vi.fn().mockReturnValue("content"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + positionAt: vi.fn().mockReturnValue({ line: 0, character: 0 }), + }, + }) + + it("saveChanges() closes the file tab when the file was not open and untouched", async () => { + const closeFileTab = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).closeFileTab = closeFileTab + ;(diffViewProvider as any).relPath = "mock-target-file.ts" + ;(diffViewProvider as any).documentWasOpen = false + ;(diffViewProvider as any).userTouchedDocument = false + ;(diffViewProvider as any).preEditScrollLine = undefined + ;(diffViewProvider as any).newContent = "content" + ;(diffViewProvider as any).activeDiffEditor = buildActiveDiffEditor() + + await diffViewProvider.saveChanges(false) + + expect(closeFileTab).toHaveBeenCalledWith(mockTargetPath) + expect(vscode.window.showTextDocument).not.toHaveBeenCalled() + }) + + it("saveChanges() keeps the file open when the user touched it", async () => { + const closeFileTab = vi.fn().mockResolvedValue(undefined) + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).closeFileTab = closeFileTab + ;(diffViewProvider as any).relPath = "mock-target-file.ts" + ;(diffViewProvider as any).documentWasOpen = false + ;(diffViewProvider as any).userTouchedDocument = true + ;(diffViewProvider as any).preEditScrollLine = undefined + ;(diffViewProvider as any).newContent = "content" + ;(diffViewProvider as any).activeDiffEditor = buildActiveDiffEditor() + + await diffViewProvider.saveChanges(false) + + expect(closeFileTab).not.toHaveBeenCalled() + expect(vscode.window.showTextDocument).toHaveBeenCalled() + }) + + it("revertChanges() closes the file tab when the file was not open and untouched", async () => { + const closeFileTab = vi.fn().mockResolvedValue(undefined) + vi.mocked(vscode.workspace.applyEdit).mockResolvedValue(true) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).closeFileTab = closeFileTab + ;(diffViewProvider as any).relPath = "mock-target-file.ts" + ;(diffViewProvider as any).documentWasOpen = false + ;(diffViewProvider as any).userTouchedDocument = false + ;(diffViewProvider as any).preEditScrollLine = undefined + ;(diffViewProvider as any).editType = "modify" + ;(diffViewProvider as any).originalContent = "original" + ;(diffViewProvider as any).activeDiffEditor = buildActiveDiffEditor() + + await diffViewProvider.revertChanges() + + expect(closeFileTab).toHaveBeenCalledWith(mockTargetPath) + expect(vscode.window.showTextDocument).not.toHaveBeenCalled() + }) + + it("revertChanges() keeps the file open when the user touched it", async () => { + const closeFileTab = vi.fn().mockResolvedValue(undefined) + vi.mocked(vscode.workspace.applyEdit).mockResolvedValue(true) + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: vi.fn() } as any) + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + ;(diffViewProvider as any).closeFileTab = closeFileTab + ;(diffViewProvider as any).relPath = "mock-target-file.ts" + ;(diffViewProvider as any).documentWasOpen = false + ;(diffViewProvider as any).userTouchedDocument = true + ;(diffViewProvider as any).preEditScrollLine = undefined + ;(diffViewProvider as any).editType = "modify" + ;(diffViewProvider as any).originalContent = "original" + ;(diffViewProvider as any).activeDiffEditor = buildActiveDiffEditor() + + await diffViewProvider.revertChanges() + + expect(closeFileTab).not.toHaveBeenCalled() + expect(vscode.window.showTextDocument).toHaveBeenCalled() + }) + + it("marks userTouchedDocument when the file editor is activated during the diff", async () => { + let activeCallback: ((editor: any) => void) | undefined + vi.mocked(vscode.window.onDidChangeActiveTextEditor).mockImplementation((cb: any) => { + activeCallback = cb + return { dispose: vi.fn() } + }) + + const mockEditor = { + document: { + uri: { fsPath: mockTargetPath, scheme: "file" }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + }, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + } + vi.mocked(vscode.window).visibleTextEditors = [mockEditor as any] + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(mockEditor as any) + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => callback({ uri: { fsPath: mockTargetPath, scheme: "file" } } as any), 0) + return { dispose: vi.fn() } + }) + vi.mocked(vscode.window.onDidChangeVisibleTextEditors).mockReturnValue({ dispose: vi.fn() }) + ;(diffViewProvider as any).editType = "modify" + ;(diffViewProvider as any).documentWasOpen = false + + await diffViewProvider.open("mock-target-file.ts") + + // Simulate the user activating the plain file editor (no diff tab active). + ;(vscode.window.tabGroups as any).activeTabGroup = { activeTab: { input: {} } } + activeCallback?.({ + document: { uri: { fsPath: mockTargetPath, scheme: "file" } }, + }) + + expect((diffViewProvider as any).userTouchedDocument).toBe(true) + }) + }) }) From 6ea564c391d978ceb9ad219fcc78c4b19722e815 Mon Sep 17 00:00:00 2001 From: awschmeder Date: Fri, 12 Jun 2026 03:02:22 -0700 Subject: [PATCH 2/2] fix(diff-view): restore preview tabs to their original editor group --- src/integrations/editor/DiffViewProvider.ts | 15 +++++----- .../editor/__tests__/DiffViewProvider.spec.ts | 30 +++++++++++-------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index 416d53e335..ec25e628b7 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -50,7 +50,7 @@ export class DiffViewProvider { // diff-open time. Opening the diff reuses the editor group's single preview // slot and evicts these tabs; we restore any that disappeared after the diff // session ends so the user's prior tab state is reconstructed. - private snapshotPreviewTabs: Array<{ uri: vscode.Uri; scrollLine: number | undefined }> = [] + private snapshotPreviewTabs: Array<{ uri: vscode.Uri; scrollLine: number | undefined; viewColumn: vscode.ViewColumn }> = [] private taskRef: WeakRef constructor( @@ -608,13 +608,13 @@ export class DiffViewProvider { } // Capture unrelated preview tabs (italicized, not-yet-edited) along with their - // current scroll position. Opening the diff reuses the editor group's single - // preview slot and evicts these tabs; the snapshot lets us restore them after - // the diff session ends. The diff target is excluded since it is about to be - // replaced by the diff view anyway. + // current scroll position and editor group. Opening the diff reuses the group's + // single preview slot and evicts these tabs; the snapshot lets us restore them + // in the correct group after the diff session ends. The diff target is excluded + // since it is about to be replaced by the diff view anyway. private captureUnrelatedPreviewTabs( diffTargetPath: string, - ): Array<{ uri: vscode.Uri; scrollLine: number | undefined }> { + ): Array<{ uri: vscode.Uri; scrollLine: number | undefined; viewColumn: vscode.ViewColumn }> { return vscode.window.tabGroups.all.flatMap((group) => group.tabs .filter( @@ -629,7 +629,7 @@ export class DiffViewProvider { const visibleEditor = vscode.window.visibleTextEditors.find( (e) => e.document.uri.scheme === "file" && arePathsEqual(e.document.uri.fsPath, uri.fsPath), ) - return { uri, scrollLine: visibleEditor?.visibleRanges?.[0]?.start.line } + return { uri, scrollLine: visibleEditor?.visibleRanges?.[0]?.start.line, viewColumn: group.viewColumn } }), ) } @@ -665,6 +665,7 @@ export class DiffViewProvider { const editor = await vscode.window.showTextDocument(snapshot.uri, { preview: true, preserveFocus: true, + viewColumn: snapshot.viewColumn, }) if (snapshot.scrollLine !== undefined) { editor.revealRange( diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts index 3196e0ea9d..d250ec105c 100644 --- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts +++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts @@ -512,19 +512,22 @@ describe("DiffViewProvider", () => { return { isPreview, input, label: fsPath } } - const setTabs = (tabs: any[]) => { + const setTabs = (tabs: any[], viewColumn = vscode.ViewColumn.One) => { Object.defineProperty(vscode.window.tabGroups, "all", { - get: () => [{ tabs }], + get: () => [{ tabs, viewColumn }], configurable: true, }) } - it("captures unrelated preview tabs with their scroll position, excluding the diff target", () => { - setTabs([ - makePreviewTab("/mock/cwd/file-1.txt"), - makePreviewTab("/mock/cwd/file-2.txt"), // diff target -- excluded - makePreviewTab("/mock/cwd/file-3.txt", false), // not a preview -- excluded - ]) + it("captures unrelated preview tabs with their scroll position and group, excluding the diff target", () => { + setTabs( + [ + makePreviewTab("/mock/cwd/file-1.txt"), + makePreviewTab("/mock/cwd/file-2.txt"), // diff target -- excluded + makePreviewTab("/mock/cwd/file-3.txt", false), // not a preview -- excluded + ], + vscode.ViewColumn.Two, + ) vi.mocked(vscode.window).visibleTextEditors = [ { document: { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" } }, @@ -537,22 +540,23 @@ describe("DiffViewProvider", () => { expect(snapshot).toHaveLength(1) expect(snapshot[0].uri.fsPath).toBe("/mock/cwd/file-1.txt") expect(snapshot[0].scrollLine).toBe(12) + expect(snapshot[0].viewColumn).toBe(vscode.ViewColumn.Two) }) - it("restores an evicted preview tab in preview mode and reapplies its scroll position", async () => { + it("restores an evicted preview tab in its original group and reapplies its scroll position", async () => { const revealRange = vi.fn() vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange } as any) // The captured file is no longer open (evicted by the diff). setTabs([]) ;(diffViewProvider as any).snapshotPreviewTabs = [ - { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, scrollLine: 12 }, + { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, scrollLine: 12, viewColumn: vscode.ViewColumn.Two }, ] await (diffViewProvider as any).restorePreviewTabs() expect(vscode.window.showTextDocument).toHaveBeenCalledWith( { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, - { preview: true, preserveFocus: true }, + { preview: true, preserveFocus: true, viewColumn: vscode.ViewColumn.Two }, ) expect(revealRange).toHaveBeenCalledWith( expect.objectContaining({ start: { line: 12, character: 0 } }), @@ -564,7 +568,7 @@ describe("DiffViewProvider", () => { it("does not restore a preview tab that is still open", async () => { setTabs([makePreviewTab("/mock/cwd/file-1.txt")]) ;(diffViewProvider as any).snapshotPreviewTabs = [ - { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, scrollLine: 0 }, + { uri: { fsPath: "/mock/cwd/file-1.txt", scheme: "file" }, scrollLine: 0, viewColumn: vscode.ViewColumn.One }, ] await (diffViewProvider as any).restorePreviewTabs() @@ -577,7 +581,7 @@ describe("DiffViewProvider", () => { const fs = await import("fs/promises") vi.mocked(fs.access).mockRejectedValueOnce(new Error("ENOENT")) ;(diffViewProvider as any).snapshotPreviewTabs = [ - { uri: { fsPath: "/mock/cwd/deleted.txt", scheme: "file" }, scrollLine: 0 }, + { uri: { fsPath: "/mock/cwd/deleted.txt", scheme: "file" }, scrollLine: 0, viewColumn: vscode.ViewColumn.One }, ] await (diffViewProvider as any).restorePreviewTabs()