diff --git a/.changeset/fix-diff-scroll-position.md b/.changeset/fix-diff-scroll-position.md new file mode 100644 index 0000000000..ca6cc61382 --- /dev/null +++ b/.changeset/fix-diff-scroll-position.md @@ -0,0 +1,7 @@ +--- +"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. + +When a user clicks or edits inside the diff pane, the target file's tab is kept open after saving -- even if it was not previously open. If the user only scrolls in the diff pane, the existing close behavior is preserved. When the target file is re-revealed after a diff, it scrolls to the position most recently viewed by the user: if the user last scrolled in the diff pane, the target file mirrors that scroll position; if the user last scrolled in the target file's own editor, that position is used instead; otherwise the pre-edit scroll position is restored. diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index 80b5799217..98f46f7171 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,38 @@ 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 + // Tracks whether the user clicked or edited inside the diff editor itself. + // A selection-change event fires on click/keyboard/edit but NOT on + // scroll, so this reliably distinguishes interaction from passive viewing. + // When true and the user saves (accepts) the diff, the target file tab is + // kept open even if it was not open before the edit began. + private userTouchedDiffEditor = false + // Tracks the most recent scroll position seen in the diff editor. Updated + // continuously so that when the user accepts the diff, the target file can + // be revealed at the same line they were viewing in the diff. + private diffScrollLine: number | undefined + // Tracks the most recent scroll position seen in the target file's own + // editor during the diff session. If the user opens the target file's tab + // and scrolls there, that position overrides the diff scroll line. + private targetFileScrollLine: number | undefined + // Records which editor (diff or the target file itself) the user scrolled + // most recently. The most-recently-scrolled source wins when choosing the + // restore scroll line. + private lastScrolledSource: "diff" | "targetFile" | undefined + private activeEditorListener?: vscode.Disposable + private diffEditorSelectionListener?: vscode.Disposable + private diffScrollListener?: 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; viewColumn: vscode.ViewColumn }> = [] private taskRef: WeakRef constructor( @@ -50,6 +86,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 +127,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 +141,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 +156,84 @@ 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 + }) + } + + // Track whether the user clicks or edits inside the diff editor. + // onDidChangeTextEditorSelection fires on click/keyboard/edit but NOT on + // scroll. We additionally filter to Mouse and Keyboard kinds to exclude + // programmatic selection changes (e.g. the selection anchor set by + // revealDiffLine / scrollToFirstDiff), which VS Code reports with kind + // undefined / Command and must not count as user interaction. + this.userTouchedDiffEditor = false + this.diffEditorSelectionListener = vscode.window.onDidChangeTextEditorSelection((event) => { + if ( + this.activeDiffEditor && + event.textEditor.document === this.activeDiffEditor.document && + (event.kind === vscode.TextEditorSelectionChangeKind.Mouse || + event.kind === vscode.TextEditorSelectionChangeKind.Keyboard) + ) { + this.userTouchedDiffEditor = true + } + }) + + // Track scroll position in both the diff editor and the target file's own + // editor. Whichever the user scrolled most recently determines the line we + // reveal when we re-open the target file after the diff closes. + this.diffScrollLine = undefined + this.targetFileScrollLine = undefined + this.lastScrolledSource = undefined + this.diffScrollListener = vscode.window.onDidChangeTextEditorVisibleRanges((event) => { + if (this.activeDiffEditor && event.textEditor.document === this.activeDiffEditor.document) { + const line = event.visibleRanges[0]?.start.line + if (line !== undefined) { + this.diffScrollLine = line + this.lastScrolledSource = "diff" + } + } else if ( + event.textEditor.document.uri.scheme === "file" && + arePathsEqual(event.textEditor.document.uri.fsPath, absolutePath) + ) { + const line = event.visibleRanges[0]?.start.line + if (line !== undefined) { + this.targetFileScrollLine = line + this.lastScrolledSource = "targetFile" + } + } + }) } async update(accumulatedContent: string, isFinal: boolean) { @@ -213,9 +333,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, this.userTouchedDiffEditor) + + // 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 +501,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 +538,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 +585,203 @@ 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 + this.diffEditorSelectionListener?.dispose() + this.diffEditorSelectionListener = undefined + this.diffScrollListener?.dispose() + this.diffScrollListener = 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. + // Pass keepIfTouchedDiff=true (from saveChanges) to also keep the file when + // the user clicked or edited inside the diff editor itself. + private async keepOrCloseEditedFile(absolutePath: string, keepIfTouchedDiff = false): Promise { + if (this.documentWasOpen || this.userTouchedDocument || keepIfTouchedDiff) { + 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, + }) + // Determine the scroll line to restore. Prefer the most-recently-scrolled + // source: if the user scrolled in the diff, reveal that line; if they + // scrolled in the target file's own editor, use that position; otherwise + // fall back to the scroll position the file had before the edit began. + const restoreScrollLine = + this.lastScrolledSource === "targetFile" + ? this.targetFileScrollLine + : this.lastScrolledSource === "diff" + ? this.diffScrollLine + : this.preEditScrollLine + if (restoreScrollLine !== undefined) { + editor.revealRange( + new vscode.Range(restoreScrollLine, 0, restoreScrollLine, 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 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; viewColumn: vscode.ViewColumn }> { + 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, viewColumn: group.viewColumn } + }), + ) + } + + // 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, + viewColumn: snapshot.viewColumn, + }) + 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 +912,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 +1024,21 @@ 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.diffScrollLine = undefined + this.targetFileScrollLine = undefined + this.lastScrolledSource = undefined + this.userTouchedDocument = false + this.userTouchedDiffEditor = 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..8c04eb6c6f 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,13 @@ vi.mock("vscode", () => ({ createTextEditorDecorationType: vi.fn(), showTextDocument: vi.fn(), onDidChangeVisibleTextEditors: vi.fn(() => ({ dispose: vi.fn() })), + onDidChangeActiveTextEditor: vi.fn(() => ({ dispose: vi.fn() })), + onDidChangeTextEditorSelection: vi.fn(() => ({ dispose: vi.fn() })), + onDidChangeTextEditorVisibleRanges: vi.fn(() => ({ dispose: vi.fn() })), tabGroups: { all: [], close: vi.fn(), + activeTabGroup: { activeTab: undefined }, }, visibleTextEditors: [], }, @@ -78,12 +83,24 @@ 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, + }, + TextEditorSelectionChangeKind: { + Keyboard: 1, + Mouse: 2, + Command: 3, }, + TabInputText: class TabInputText {}, TabInputTextDiff: class TabInputTextDiff {}, Uri: { file: vi.fn((path) => ({ fsPath: path })), @@ -271,6 +288,384 @@ 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[], viewColumn = vscode.ViewColumn.One) => { + Object.defineProperty(vscode.window.tabGroups, "all", { + get: () => [{ tabs, viewColumn }], + configurable: true, + }) + } + + 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" } }, + 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) + expect(snapshot[0].viewColumn).toBe(vscode.ViewColumn.Two) + }) + + 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, 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, viewColumn: vscode.ViewColumn.Two }, + ) + 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, viewColumn: vscode.ViewColumn.One }, + ] + + 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, viewColumn: vscode.ViewColumn.One }, + ] + + 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 +912,679 @@ 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) + }) + }) + + describe("userTouchedDiffEditor keep/close 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() keeps the file open when the user clicked inside the diff editor", 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 = false + // The user clicked in the diff editor -- the flag is true. + ;(diffViewProvider as any).userTouchedDiffEditor = 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("saveChanges() closes the file tab when the user only scrolled (not clicked) in the diff", 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 + // The user only scrolled -- the flag stays false. + ;(diffViewProvider as any).userTouchedDiffEditor = 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("open() registers onDidChangeTextEditorSelection and sets userTouchedDiffEditor on event", async () => { + let selectionCallback: ((event: any) => void) | undefined + vi.mocked(vscode.window.onDidChangeTextEditorSelection).mockImplementation((cb: any) => { + selectionCallback = cb + return { dispose: vi.fn() } + }) + + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/sel.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(), + } + 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}/sel.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("sel.ts") + + expect((diffViewProvider as any).userTouchedDiffEditor).toBe(false) + + // Simulate a Mouse selection-change event on the captured diff editor. + const activeDiffEditor = (diffViewProvider as any).activeDiffEditor + selectionCallback?.({ + textEditor: activeDiffEditor, + kind: vscode.TextEditorSelectionChangeKind.Mouse, + }) + + expect((diffViewProvider as any).userTouchedDiffEditor).toBe(true) + }) + + it("open() does NOT set userTouchedDiffEditor for programmatic selection changes (kind=Command or undefined)", async () => { + let selectionCallback: ((event: any) => void) | undefined + vi.mocked(vscode.window.onDidChangeTextEditorSelection).mockImplementation((cb: any) => { + selectionCallback = cb + return { dispose: vi.fn() } + }) + + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/prog.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(), + } + 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}/prog.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("prog.ts") + + const activeDiffEditor = (diffViewProvider as any).activeDiffEditor + + // Programmatic change via editor.selection= (kind undefined) -- e.g. revealDiffLine + selectionCallback?.({ textEditor: activeDiffEditor, kind: undefined }) + expect((diffViewProvider as any).userTouchedDiffEditor).toBe(false) + + // Programmatic change via command (kind=Command) + selectionCallback?.({ + textEditor: activeDiffEditor, + kind: vscode.TextEditorSelectionChangeKind.Command, + }) + expect((diffViewProvider as any).userTouchedDiffEditor).toBe(false) + }) + + it("open() sets userTouchedDiffEditor when event comes from a fresh editor instance wrapping the same document (stale ref fix)", async () => { + let selectionCallback: ((event: any) => void) | undefined + vi.mocked(vscode.window.onDidChangeTextEditorSelection).mockImplementation((cb: any) => { + selectionCallback = cb + return { dispose: vi.fn() } + }) + + const sharedDocument = { + uri: { fsPath: `${mockCwd}/stale.ts`, scheme: "file" }, + getText: vi.fn().mockReturnValue(""), + lineCount: 0, + } + const originalEditor = { + document: sharedDocument, + selection: { active: { line: 0, character: 0 }, anchor: { line: 0, character: 0 } }, + edit: vi.fn().mockResolvedValue(true), + revealRange: vi.fn(), + } + vi.mocked(vscode.window).visibleTextEditors = [originalEditor as any] + vi.mocked(vscode.window.showTextDocument).mockResolvedValue(originalEditor as any) + vi.mocked(vscode.workspace.onDidOpenTextDocument).mockImplementation((callback) => { + setTimeout(() => callback({ uri: { fsPath: `${mockCwd}/stale.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("stale.ts") + + // Simulate VS Code giving us a NEW editor object that wraps the same + // document -- this is the real-world stale reference scenario. + const freshEditorSameDoc = { document: sharedDocument } + selectionCallback?.({ + textEditor: freshEditorSameDoc, + kind: vscode.TextEditorSelectionChangeKind.Mouse, + }) + + expect((diffViewProvider as any).userTouchedDiffEditor).toBe(true) + }) + + it("open() ignores selection events on editors other than the diff editor", async () => { + let selectionCallback: ((event: any) => void) | undefined + vi.mocked(vscode.window.onDidChangeTextEditorSelection).mockImplementation((cb: any) => { + selectionCallback = cb + return { dispose: vi.fn() } + }) + + const mockEditor = { + document: { + uri: { fsPath: `${mockCwd}/other.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(), + } + 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}/other.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("other.ts") + + // Fire the event with a completely different editor object. + const unrelatedEditor = { document: { uri: { fsPath: "/some/other/file.ts", scheme: "file" } } } + selectionCallback?.({ textEditor: unrelatedEditor }) + + expect((diffViewProvider as any).userTouchedDiffEditor).toBe(false) + }) + }) + + describe("scroll position precedence in showEditedFileWithoutDisruptingFocus", () => { + const setupForScroll = ( + lastScrolledSource: "diff" | "targetFile" | undefined, + diffScrollLine: number | undefined, + targetFileScrollLine: number | undefined, + preEditScrollLine: number | undefined, + ) => { + const mockRevealRange = vi.fn() + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({ revealRange: mockRevealRange } as any) + const userActiveEditor = { + document: { uri: { fsPath: `${mockCwd}/target.ts`, scheme: "file" } }, + viewColumn: 1, + } + ;(vscode.window as any).activeTextEditor = userActiveEditor + ;(diffViewProvider as any).lastScrolledSource = lastScrolledSource + ;(diffViewProvider as any).diffScrollLine = diffScrollLine + ;(diffViewProvider as any).targetFileScrollLine = targetFileScrollLine + ;(diffViewProvider as any).preEditScrollLine = preEditScrollLine + ;(diffViewProvider as any).documentWasPinned = false + return mockRevealRange + } + + it("uses diffScrollLine when lastScrolledSource is 'diff'", async () => { + const mockRevealRange = setupForScroll("diff", 20, 5, 0) + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus(`${mockCwd}/target.ts`) + + expect(mockRevealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 20, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + }) + + it("uses targetFileScrollLine when lastScrolledSource is 'targetFile'", async () => { + const mockRevealRange = setupForScroll("targetFile", 20, 8, 0) + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus(`${mockCwd}/target.ts`) + + expect(mockRevealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 8, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + }) + + it("falls back to preEditScrollLine when lastScrolledSource is undefined", async () => { + const mockRevealRange = setupForScroll(undefined, 20, 8, 3) + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus(`${mockCwd}/target.ts`) + + expect(mockRevealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 3, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + }) + + it("does not call revealRange when all scroll sources are undefined", async () => { + const mockRevealRange = setupForScroll(undefined, undefined, undefined, undefined) + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus(`${mockCwd}/target.ts`) + + expect(mockRevealRange).not.toHaveBeenCalled() + }) + + it("targetFile scroll overrides diff scroll regardless of their values", async () => { + // lastScrolledSource=targetFile means the user scrolled there AFTER the diff, + // so targetFileScrollLine must win even when diffScrollLine is higher. + const mockRevealRange = setupForScroll("targetFile", 100, 2, 0) + + await (diffViewProvider as any).showEditedFileWithoutDisruptingFocus(`${mockCwd}/target.ts`) + + expect(mockRevealRange).toHaveBeenCalledWith( + expect.objectContaining({ start: { line: 2, character: 0 } }), + vscode.TextEditorRevealType.AtTop, + ) + }) + }) + + describe("diffScrollListener wiring in open()", () => { + const openWithScrollListener = async (relPath: string) => { + let scrollCallback: ((event: any) => void) | undefined + vi.mocked(vscode.window.onDidChangeTextEditorVisibleRanges).mockImplementation((cb: any) => { + scrollCallback = cb + return { dispose: vi.fn() } + }) + + const fsPath = `${mockCwd}/${relPath}` + const mockEditor = { + document: { + uri: { fsPath, 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, 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(relPath) + return { scrollCallback: scrollCallback!, activeDiffEditor: (diffViewProvider as any).activeDiffEditor, fsPath } + } + + it("records diffScrollLine and sets lastScrolledSource to 'diff' on diff editor scroll", async () => { + const { scrollCallback, activeDiffEditor } = await openWithScrollListener("scroll-diff.ts") + + scrollCallback({ textEditor: activeDiffEditor, visibleRanges: [{ start: { line: 42 } }] }) + + expect((diffViewProvider as any).diffScrollLine).toBe(42) + expect((diffViewProvider as any).lastScrolledSource).toBe("diff") + }) + + it("records targetFileScrollLine and sets lastScrolledSource to 'targetFile' on target file scroll", async () => { + const { scrollCallback, fsPath } = await openWithScrollListener("scroll-target.ts") + + const targetFileEditor = { + document: { uri: { fsPath, scheme: "file" } }, + } + scrollCallback({ textEditor: targetFileEditor, visibleRanges: [{ start: { line: 17 } }] }) + + expect((diffViewProvider as any).targetFileScrollLine).toBe(17) + expect((diffViewProvider as any).lastScrolledSource).toBe("targetFile") + }) + + it("ignores scroll events from unrelated editors", async () => { + const { scrollCallback } = await openWithScrollListener("scroll-unrelated.ts") + + const unrelatedEditor = { + document: { uri: { fsPath: "/some/other/file.ts", scheme: "file" } }, + } + scrollCallback({ textEditor: unrelatedEditor, visibleRanges: [{ start: { line: 99 } }] }) + + expect((diffViewProvider as any).diffScrollLine).toBeUndefined() + expect((diffViewProvider as any).targetFileScrollLine).toBeUndefined() + expect((diffViewProvider as any).lastScrolledSource).toBeUndefined() + }) + + it("lastScrolledSource reflects the most recent scroll between diff and target file", async () => { + const { scrollCallback, activeDiffEditor, fsPath } = await openWithScrollListener("scroll-recency.ts") + + // First scroll in diff. + scrollCallback({ textEditor: activeDiffEditor, visibleRanges: [{ start: { line: 10 } }] }) + expect((diffViewProvider as any).lastScrolledSource).toBe("diff") + + // Then scroll in the target file -- this must win. + const targetFileEditor = { document: { uri: { fsPath, scheme: "file" } } } + scrollCallback({ textEditor: targetFileEditor, visibleRanges: [{ start: { line: 5 } }] }) + expect((diffViewProvider as any).lastScrolledSource).toBe("targetFile") + + // Back to diff again. + scrollCallback({ textEditor: activeDiffEditor, visibleRanges: [{ start: { line: 20 } }] }) + expect((diffViewProvider as any).lastScrolledSource).toBe("diff") + }) + }) })