From 5c3feea2a8b115271e03a249e49bba22e896d683 Mon Sep 17 00:00:00 2001 From: Mosab Anini <50235797+manini-1@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:01:50 +0200 Subject: [PATCH] =?UTF-8?q?fix(vcs):=20honour=20the=20modal=20commit=20int?= =?UTF-8?q?erface=20for=20Commit=20File=E2=80=A6=20and=20diffs=20(#2158)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Use non-modal commit interface unticked, parts of the IDE still assumed the Commit tool window: Commit File only set the inclusion and focused a message field that mode does not have, and diffs opened as editor tabs. Commit File and Commit Changelist now open the Commit dialog with those changes included, and user-opened VCS diffs (Local Changes, log, Shelf, compare actions) open in a front window whose close shortcut stays inside it. With the setting ticked, the default, behaviour is unchanged. Co-authored-by: manini-1 <50235797+manini-1@users.noreply.github.com> --- src/features/app/__tests__/windows.test.ts | 2 +- src/features/app/windows.ts | 4 +- src/features/vcs/CommitView.tsx | 50 ++++---- src/features/vcs/ComparisonTree.tsx | 4 +- src/features/vcs/DiffEditor.tsx | 9 +- src/features/vcs/DirectoryCompare.tsx | 9 +- src/features/vcs/GitLog.tsx | 13 +- src/features/vcs/MergeEditor.tsx | 4 +- src/features/vcs/__tests__/changes.test.ts | 31 +++++ src/features/vcs/__tests__/commitMode.test.ts | 49 ++++++++ .../vcs/__tests__/compareFlow.test.ts | 2 +- .../vcs/__tests__/diffDialog.test.tsx | 82 ++++++++++-- .../vcs/__tests__/directoryCompare.test.tsx | 1 + .../__tests__/directoryCompareActions.test.ts | 2 +- .../vcs/__tests__/folderCompare.test.tsx | 15 +++ .../vcs/__tests__/localChanges.test.tsx | 108 ++++++++++++++-- .../vcs/__tests__/modalDiffWindow.test.ts | 70 +++++++++++ src/features/vcs/__tests__/shelf.test.tsx | 1 + .../vcs/__tests__/show-diff-chain.test.ts | 117 +++++++++++++++++- src/features/vcs/actions.tsx | 64 ++++++++-- src/features/vcs/changelistsUi.tsx | 13 +- src/features/vcs/changes.ts | 13 ++ src/features/vcs/commitStore.ts | 4 + src/features/vcs/compare.tsx | 12 +- src/features/vcs/comparePaths.ts | 4 +- src/features/vcs/diffDialog.tsx | 102 +++++++++------ src/features/vcs/diffInput.ts | 37 +++++- src/features/vcs/logChanges.ts | 48 ++++--- src/features/vcs/operations.tsx | 7 +- src/features/vcs/shelf.tsx | 7 +- 30 files changed, 732 insertions(+), 152 deletions(-) create mode 100644 src/features/vcs/__tests__/modalDiffWindow.test.ts diff --git a/src/features/app/__tests__/windows.test.ts b/src/features/app/__tests__/windows.test.ts index fe15480d7..d8d65a161 100644 --- a/src/features/app/__tests__/windows.test.ts +++ b/src/features/app/__tests__/windows.test.ts @@ -47,7 +47,7 @@ vi.mock("@/platform/project", async (orig) => { return { ...real, openProject: (p: string) => openProject(p) }; }); const diffs: unknown[] = []; -vi.mock("@/features/vcs/diffInput", () => ({ openDiff: (data: unknown) => void diffs.push(data) })); +vi.mock("@/features/vcs/diffInput", () => ({ openVcsDiff: (data: unknown) => void diffs.push(data) })); const { openProjectInWindow, diff --git a/src/features/app/windows.ts b/src/features/app/windows.ts index 4afd1feb5..871ca060f 100644 --- a/src/features/app/windows.ts +++ b/src/features/app/windows.ts @@ -242,11 +242,11 @@ export async function consumeLaunch(): Promise { async function openCliDiff(diff: { left: string; right: string }): Promise { const s = useProject.getState(); if (!s.project && !s.lightEdit) await openLightEdit(diff.right).catch(() => {}); - const [{ openDiff }, { compareFilesData }] = await Promise.all([ + const [{ openVcsDiff }, { compareFilesData }] = await Promise.all([ import("@/features/vcs/diffInput"), import("@/features/vcs/compareData"), ]); - await openDiff(compareFilesData(diff.left, diff.right)); + await openVcsDiff(compareFilesData(diff.left, diff.right)); } /** Closes the project tabs on `side` of the `arg` tab (default: the current one), keeping that tab. */ diff --git a/src/features/vcs/CommitView.tsx b/src/features/vcs/CommitView.tsx index 156c8124b..64214b4bf 100644 --- a/src/features/vcs/CommitView.tsx +++ b/src/features/vcs/CommitView.tsx @@ -3,7 +3,7 @@ * parts also make the Git window's Local Changes / Stash tabs and the Commit dialog. */ import { useWorkflowSettings } from "@/platform/workflowSettings"; -import { formatBinding, isMacHost, macStroke, normalizeBinding, strokeOf } from "@/platform/keymap"; +import { formatBinding, isMacHost, macStroke, normalizeBinding, shortcutLabel, strokeOf } from "@/platform/keymap"; import { useCallback, useEffect, @@ -50,12 +50,10 @@ import { buildChangeTree, checkOf, filesOf, - groupFileNodes, groupOf, ignoreTargets, isPartiallyStaged, planCommit, - stagingSide, toggledCheck, type ChangeNode, } from "./changes"; @@ -79,8 +77,15 @@ import { import { pushAfterCommit } from "./commitUi"; import { rebaseAfterFixup } from "./rebase"; import { stashTabOf, stashTabs, type ContentTab } from "./gitTabs"; -import { DIFF_KIND, diffInput, openDiff, withChain, workingDiffData } from "./diffInput"; -import { openDiffDialog } from "./diffDialog"; +import { + DIFF_KIND, + changeNodeDiff, + changeNodeDiffData, + diffInput, + openVcsDiff, + vcsDiffInFront, + workingDiffData, +} from "./diffInput"; import { DiffEditor } from "./DiffEditor"; import { KIND_COLOR, KIND_LABEL, fileCount, formatTime, stashLabel, stripSlash } from "./model"; import { call, commands } from "@/platform/ipc"; @@ -196,16 +201,13 @@ const bottomLeft = (el: HTMLElement) => { /** * Opens a file's diff; F7 continues through the other files of its group, in tree - * order. `dialog` opens it in a front dialog: an editor tab would sit behind the - * modal Commit dialog, invisible until it closes (#1863). + * order. Where depends on the commit interface setting (openVcsDiff): a preview editor + * tab, or under the modal interface the front diff window, as an editor tab would sit + * behind the modal Commit dialog (#1863). */ -function openNode(n: ChangeNode, roots: readonly ChangeNode[], mode: "commit" | "local" | "dialog") { +function openNode(n: ChangeNode, roots: readonly ChangeNode[]) { if (n.type !== "file" || !n.path) return; - const nodes = groupFileNodes(roots, n); - const items = nodes.map((f) => workingDiffData(f.path!, undefined, stagingSide(f.group, f.files[0]!))); - const data = withChain(items, Math.max(0, nodes.indexOf(n))); - if (mode === "dialog") openDiffDialog(data); - else void openDiff(data, { preview: true }); + void openVcsDiff(changeNodeDiffData(roots, n), { preview: true }); } /** Context menu of a change list's header: the list's own actions, then the bulk ones on its files. */ @@ -272,7 +274,7 @@ export function copyChangePaths(nodes: ChangeNode[], kind: "absolute" | "name") export function nodeMenu( nodes: ChangeNode[], repos: readonly string[], - opts?: { mode?: "commit" | "local" | "dialog"; roots?: readonly ChangeNode[] }, + opts?: { roots?: readonly ChangeNode[] }, ): MenuItem[] { const files = filesOf(nodes); const deletable = deletableFiles(nodes); @@ -302,10 +304,14 @@ export function nodeMenu( run: () => commitFiles(paths), }, { kind: "separator" }, - // In `dialog` mode an editor tab would sit behind the modal Commit dialog (#1863): - // open the diff in a front dialog, like double-click does. - opts?.mode === "dialog" && opts.roots && first - ? { label: "Show Diff", icon: "diff", run: () => openNode(first, opts.roots!, "dialog") } + // Under the modal interface the node itself, like double-click, in the front window. + vcsDiffInFront() && opts?.roots && first + ? { + label: "Show Diff", + icon: "diff", + shortcut: shortcutLabel("vcs.showDiff"), + run: () => openNode(first, opts.roots!), + } : { kind: "action", action: "vcs.showDiff", arg: first?.path }, { label: "Jump to Source", @@ -558,8 +564,7 @@ export function ChangesTree({ mode }: { mode: "commit" | "local" | "dialog" }) { if (input.key === followedDiff.current) return; const node = [...byKey.values()].find((node) => { if (node.type !== "file" || node.path !== input.path) return false; - const side = stagingSide(node.group, node.files[0]!); - return diffInput(workingDiffData(node.path!, undefined, side)).key === input.key; + return diffInput(changeNodeDiff(node)).key === input.key; }); if (node) { followedDiff.current = input.key; @@ -726,10 +731,11 @@ export function ChangesTree({ mode }: { mode: "commit" | "local" | "dialog" }) { const nodes = [...sel].map((k) => byKey.get(k)).filter((n): n is ChangeNode => !!n); useCommit.setState({ selection: filesOf(nodes).map((f) => f.path), + selectionKeys: [...sel], selectedList: nodes.find((n) => n.changelist)?.changelist ?? null, }); }} - onActivate={(n) => openNode(n, roots, mode)} + onActivate={(n) => openNode(n, roots)} onContextMenu={(node, sel, e: MouseEvent) => { if (!node) return; if (node.type === "group" && node.changelist) { @@ -737,7 +743,7 @@ export function ChangesTree({ mode }: { mode: "commit" | "local" | "dialog" }) { return; } const nodes = [...sel].map((k) => byKey.get(k)).filter((n): n is ChangeNode => !!n); - showContextMenu(e, nodeMenu(nodes.length ? nodes : [node], repos, { mode, roots })); + showContextMenu(e, nodeMenu(nodes.length ? nodes : [node], repos, { roots })); }} aria-label="Changes" empty={ diff --git a/src/features/vcs/ComparisonTree.tsx b/src/features/vcs/ComparisonTree.tsx index ec52938df..7b9d1d1bf 100644 --- a/src/features/vcs/ComparisonTree.tsx +++ b/src/features/vcs/ComparisonTree.tsx @@ -1,6 +1,6 @@ import { useState, type ReactNode } from "react"; import { Icon, Tree, fileIconName, lazyComponent } from "@/ui"; -import { diffInput, openDiff, type DiffData } from "./diffInput"; +import { diffInput, openVcsDiff, type DiffData } from "./diffInput"; import { dirKeys, type FileTreeNode } from "./fileTree"; import styles from "./common.module.css"; @@ -53,7 +53,7 @@ export function ComparisonTree({ onSelect(entry); const diff = diffFor(entry); if (openOnActivate && diff) { - void openDiff(diff); + void openVcsDiff(diff); onOpen?.(); } }} diff --git a/src/features/vcs/DiffEditor.tsx b/src/features/vcs/DiffEditor.tsx index 61b966e6e..c5ddb9cef 100644 --- a/src/features/vcs/DiffEditor.tsx +++ b/src/features/vcs/DiffEditor.tsx @@ -218,6 +218,7 @@ export function DiffEditor({ input, toolbarActions, onNeighbour, + onJumpToSource, }: { input: EditorInput; toolbarActions?: ReactNode; @@ -227,6 +228,8 @@ export function DiffEditor({ * own content, as a modal dialog hides the editor area. */ onNeighbour?: (data: DiffData) => void; + /** Called as Jump to Source opens the file in an editor: a modal host closes, or the editor stays hidden behind it. */ + onJumpToSource?: () => void; }) { const data = input.data as DiffData; const { path, left, right } = data; @@ -735,8 +738,10 @@ export function DiffEditor({ const own = onA ? fileA : fileB; const file = own ?? fileB ?? fileA ?? worktreeSource; if (!file) return; - const open = (line: number, col: number) => + const open = (line: number, col: number) => { + onJumpToSource?.(); void editorService().openFile(file, { selection: { line, col }, focus: true }); + }; if (!v) return open(0, 0); const head = v.state.selection.main.head; const at = v.state.doc.lineAt(head); @@ -750,7 +755,7 @@ export function DiffEditor({ const line = mapLine(Chunk.build(from, to), at.number - 1, from, to); open(line, to.line(line + 1).text === at.text ? head - at.from : 0); }); - }, [left, right, path, worktreeSource]); + }, [left, right, path, worktreeSource, onJumpToSource]); const currentChunk = (): { chunk: Chunk; index: number } | undefined => { const v = viewsRef.current?.b; diff --git a/src/features/vcs/DirectoryCompare.tsx b/src/features/vcs/DirectoryCompare.tsx index b70e2acad..bb509ee07 100644 --- a/src/features/vcs/DirectoryCompare.tsx +++ b/src/features/vcs/DirectoryCompare.tsx @@ -9,7 +9,7 @@ import { upsertTask } from "@/workbench/tasks"; import { ComparisonTree } from "./ComparisonTree"; import { buildFileTree } from "./fileTree"; import { confirmDialog, openDialog } from "./dialogs"; -import { openDiff, type DiffData, type DiffSide } from "./diffInput"; +import { openVcsDiff, vcsDiffInFront, type DiffData, type DiffSide } from "./diffInput"; import { compareFilesData } from "./compareData"; import styles from "./common.module.css"; @@ -200,8 +200,9 @@ export function DirectoryCompareDialog({ left, right, onClose }: { left: string; disabled: !diff || busy || operating, onClick: () => { if (diff) { - void openDiff(diff); - onClose(); + void openVcsDiff(diff); + // A diff window opens on top of this list (#1863); an editor tab would sit behind it. + if (!vcsDiffInFront()) onClose(); } }, }, @@ -278,7 +279,7 @@ export function DirectoryCompareDialog({ left, right, onClose }: { left: string; onSelect={setCurrent} diffFor={(e) => directoryDiff(left, right, e)} openOnActivate={!busy && !operating} - onOpen={onClose} + onOpen={() => !vcsDiffInFront() && onClose()} ariaLabel="Compared files" renderDetails={(n) => { const entry = n.file ?? n.directory; diff --git a/src/features/vcs/GitLog.tsx b/src/features/vcs/GitLog.tsx index e896d8914..a57db1c84 100644 --- a/src/features/vcs/GitLog.tsx +++ b/src/features/vcs/GitLog.tsx @@ -100,7 +100,7 @@ import { } from "./branches"; import { chooseDateRange, chooseMany } from "./dialogs"; import { buildFileTree, dirKeys, type FileTreeNode } from "./fileTree"; -import { DIFF_KIND, diffInput } from "./diffInput"; +import { DIFF_KIND, diffInput, vcsDiffInFront } from "./diffInput"; import { absolutePath, filesUnder, @@ -2413,7 +2413,11 @@ export function CommitDetails({ commits }: { commits: Commit[] }) { const fromParents = useLogOptions((s) => s.changesFromParents); const onlyAffected = useLogOptions((s) => s.showOnlyAffected); const preview = useLogOptions((s) => s.diffPreview); - const diffLocation = useLogOptions((s) => s.diffLocation); + const savedLocation = useLogOptions((s) => s.diffLocation); + // The modal commit interface keeps diffs out of editor tabs (#1863): the "Editor" + // preview shows beside the changes instead. + const modal = useWorkflowSettings((s) => !s.vcs.commit.nonModal); + const diffLocation = modal && savedLocation === "editor" ? "right" : savedLocation; const showLineStats = useLogOptions((s) => s.showLineStats); const filters = useGitLog((s) => s.filters); const filesKey = JSON.stringify([idsKey, fromParents, parentIndex, commitRoot, showLineStats]); @@ -2525,7 +2529,8 @@ export function CommitDetails({ commits }: { commits: Commit[] }) { const previewSelection = (sel: ReadonlySet) => { clearTimeout(previewTimer.current); if (syncingDiff.current) return; - if (!logOptions().diffPreview || logOptions().diffLocation !== "editor" || sel.size !== 1) return; + if (!logOptions().diffPreview || logOptions().diffLocation !== "editor" || vcsDiffInFront() || sel.size !== 1) + return; const file = fileNodeAt(tree, [...sel][0]!)?.file; // A preview that cannot open (no editor yet, the file went away) is just skipped. if (file) @@ -2596,7 +2601,7 @@ export function CommitDetails({ commits }: { commits: Commit[] }) { label: "Diff Preview Location", submenu: (["editor", "right", "bottom"] as const).map((location) => ({ label: location === "editor" ? "Editor" : location === "right" ? "Right" : "Bottom", - checked: diffLocation === location, + checked: savedLocation === location, run: () => updateLogOptions({ diffLocation: location }), })), }, diff --git a/src/features/vcs/MergeEditor.tsx b/src/features/vcs/MergeEditor.tsx index 385628c85..010b0f9ef 100644 --- a/src/features/vcs/MergeEditor.tsx +++ b/src/features/vcs/MergeEditor.tsx @@ -30,7 +30,7 @@ import { type MergeControls, } from "./mergeGutters"; import { COMPARISONS, showCompareMenu, showResultMenu, showSettingsMenu, showSideMenu } from "./mergeMenus"; -import { openDiff } from "./diffInput"; +import { openVcsDiff } from "./diffInput"; import { clearFocusedMergeEditor, refreshMergeActions, @@ -511,7 +511,7 @@ export function MergeEditor({ input }: { input: EditorInput }) { theirs: { text: state.theirs, label: txt(titles.theirs) }, }; const [a, b] = COMPARISONS[id as keyof typeof COMPARISONS]; - await openDiff({ path, left: { type: "text", ...versions[a] }, right: { type: "text", ...versions[b] } }); + await openVcsDiff({ path, left: { type: "text", ...versions[a] }, right: { type: "text", ...versions[b] } }); } } }; diff --git a/src/features/vcs/__tests__/changes.test.ts b/src/features/vcs/__tests__/changes.test.ts index 0ecf92ffc..b7a610b26 100644 --- a/src/features/vcs/__tests__/changes.test.ts +++ b/src/features/vcs/__tests__/changes.test.ts @@ -5,6 +5,7 @@ import { buildChangeTree, checkOf, commitRepos, + changeFileNodes, ignoreTargets, initialCheck, hunkIncluded, @@ -266,3 +267,33 @@ describe("commit inclusion", () => { expect(toggledHunk("hunks", new Set(["gone", "a"]), all, "b").excluded).toEqual(new Set(["a", "b"])); }); }); + +describe("changeFileNodes", () => { + const file: ChangeNode = { + key: "/p/src/a.ts", + type: "file", + label: "a.ts", + group: "changes", + path: "/p/src/a.ts", + files: [], + }; + const tree: ChangeNode[] = [ + { key: "group:changes", type: "group", label: "Changes", group: "changes", children: [file], files: [] }, + ]; + + it("lists the file nodes through nested groups, in tree order", () => { + expect(changeFileNodes(tree)).toEqual([file]); + }); + + it("lists a partially staged file under both staging groups", () => { + const fs = { + path: "/p/a.ts", + relPath: "a.ts", + staged: "modified", + unstaged: "modified", + renamedFrom: null, + } as const; + const nodes = changeFileNodes(buildChangeTree([fs], "/p", false, undefined, ["/p"], true)); + expect(nodes.map((n) => n.group)).toEqual(["staged", "unstaged"]); + }); +}); diff --git a/src/features/vcs/__tests__/commitMode.test.ts b/src/features/vcs/__tests__/commitMode.test.ts index f128c4ef0..7d255677e 100644 --- a/src/features/vcs/__tests__/commitMode.test.ts +++ b/src/features/vcs/__tests__/commitMode.test.ts @@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const vcsLog = vi.fn(); const fsReadText = vi.fn(); +const showCommitDialog = vi.hoisted(() => vi.fn()); +vi.mock("../CommitDialog", () => ({ showCommitDialog })); const vcsConfigGet = vi.fn(async (_repo: unknown, _key: string) => ({ status: "ok", data: null as string | null })); vi.mock("@/platform/ipc", async (importOriginal) => { @@ -17,6 +19,10 @@ vi.mock("@/platform/ipc", async (importOriginal) => { }; }); +import type { FileStatus, Settings } from "@/bindings"; +import { useProject } from "@/platform/project"; +import { commitChangelist, commitFiles } from "../changelistsUi"; +import { initialChangelists, useChangelists } from "../changelists"; import { gitTabs, shownTab, stashTabOf, stashTabs } from "../gitTabs"; import { pushDialogNeeded } from "../commitUi"; import { @@ -139,6 +145,49 @@ describe("commit template", () => { }); }); +describe("Commit File… and the modal commit interface", () => { + const status: FileStatus[] = [ + { path: "/p/src/a.ts", relPath: "src/a.ts", staged: null, unstaged: "modified", renamedFrom: null }, + { path: "/p/src/b.ts", relPath: "src/b.ts", staged: null, unstaged: "modified", renamedFrom: null }, + ]; + + beforeEach(() => { + showCommitDialog.mockClear(); + useProject.setState({ settings: null }); + useVcs.setState({ available: true, root: "/p", repo: "/p", roots: ["/p"], status }); + useCommit.setState({ checks: new Map(), message: "" }); + }); + + it("opens the Commit dialog with just the file included", async () => { + useProject.setState({ settings: { vcs: { commit: { nonModal: false } } } as Settings }); + commitFiles(["/p/src/a.ts"]); + await vi.waitFor(() => expect(showCommitDialog).toHaveBeenCalled()); + expect(useCommit.getState().checks.get("/p/src/a.ts")).toBe("all"); + expect(useCommit.getState().checks.get("/p/src/b.ts")).toBe("none"); + }); + + it("keeps the message in the Commit tool window without the modal interface", () => { + const tick = useCommit.getState().focusTick; + commitFiles(["/p/src/a.ts"]); + expect(showCommitDialog).not.toHaveBeenCalled(); + expect(useCommit.getState().focusTick).toBe(tick + 1); + expect(useCommit.getState().checks.get("/p/src/a.ts")).toBe("all"); + expect(useCommit.getState().checks.get("/p/src/b.ts")).toBe("none"); + }); + + it("opens the Commit dialog for Commit… on a change list too", async () => { + useProject.setState({ settings: { vcs: { commit: { nonModal: false } } } as Settings }); + const lists = initialChangelists(); + lists.lists.push({ id: "other", name: "Other", comment: "" }); + lists.files = { "src/a.ts": "other" }; + useChangelists.setState({ state: lists }); + commitChangelist("other"); + await vi.waitFor(() => expect(showCommitDialog).toHaveBeenCalled()); + expect(useCommit.getState().checks.get("/p/src/a.ts")).toBe("all"); + expect(useCommit.getState().checks.get("/p/src/b.ts")).toBe("none"); + }); +}); + describe("log tabs of their own", () => { beforeEach(() => { vcsLog.mockReset().mockResolvedValue({ status: "ok", data: [] }); diff --git a/src/features/vcs/__tests__/compareFlow.test.ts b/src/features/vcs/__tests__/compareFlow.test.ts index 50e84ad20..d4ceefecc 100644 --- a/src/features/vcs/__tests__/compareFlow.test.ts +++ b/src/features/vcs/__tests__/compareFlow.test.ts @@ -6,7 +6,7 @@ vi.mock("@/platform/ipc", async (original) => { return { ...real, commands: { ...real.commands, vcsCompareScope: mocks.scope } }; }); vi.mock("../RevisionChooser", () => ({ chooseRepositoryRevision: mocks.choose })); -vi.mock("../diffInput", () => ({ openDiff: mocks.open })); +vi.mock("../diffInput", () => ({ openDiff: mocks.open, openVcsDiff: mocks.open })); vi.mock("../operations", () => ({ showPathComparison: mocks.show })); vi.mock("@/workbench/contributions", () => ({ notifications: () => ({ notify: mocks.notify }) })); import { useProject } from "@/platform/project"; diff --git a/src/features/vcs/__tests__/diffDialog.test.tsx b/src/features/vcs/__tests__/diffDialog.test.tsx index a7ef66887..8b8b65eaa 100644 --- a/src/features/vcs/__tests__/diffDialog.test.tsx +++ b/src/features/vcs/__tests__/diffDialog.test.tsx @@ -8,22 +8,22 @@ vi.mock("@/platform/ipc", async (importOriginal) => { const editor = vi.hoisted(() => vi.fn()); vi.mock("../DiffEditor", () => ({ - DiffEditor: (props: { input: { data: { path: string; embedded?: boolean } } }) => { + DiffEditor: (props: { input: { data: { path: string; embedded?: boolean } }; onJumpToSource?: () => void }) => { editor(props); return
; }, })); -const openDiff = vi.hoisted(() => vi.fn()); -vi.mock("../diffInput", async (original) => ({ - ...(await original()), - openDiff, -})); - +import type { Settings } from "@/bindings"; +import { useProject } from "@/platform/project"; +import { registerAction } from "@/platform/actions"; +import { installKeyboardDispatcher, useKeymap } from "@/platform/keymap"; +import { provideEditorService } from "@/workbench/contributions"; import { ChangesTree, nodeMenu } from "../CommitView"; import type { ChangeNode } from "../changes"; import { VcsDialogHost } from "../dialogs"; import { useVcs } from "../store"; +import { openDiffDialog } from "../diffDialog"; const modified = (path: string) => ({ path, @@ -33,6 +33,10 @@ const modified = (path: string) => ({ renamedFrom: null, }); +/** Editor tabs: none may open behind the modal Commit dialog. */ +const openDiff = vi.fn(async () => undefined); +provideEditorService({ openInput: openDiff } as never); + /** Activates (double-clicks) the file row the way a user does. */ function activateFile(name: string) { const row = screen.getByText(name).closest('[role="treeitem"]')!; @@ -44,8 +48,8 @@ const diffDialogs = async () => (await screen.findAllByRole("dialog", { name: /\ describe("Commit dialog diff window (#1863)", () => { beforeEach(() => { - openDiff.mockClear(); - editor.mockClear(); + // The Commit dialog exists with "Use non-modal commit interface" unticked. + useProject.setState({ settings: { vcs: { commit: { nonModal: false } } } as Settings }); // The dialog stack is module state: close whatever a previous test left open. for (const close of screen.queryAllByRole("button", { name: "Close" })) act(() => fireEvent.click(close)); useVcs.setState({ available: true, root: "/p", repo: "/p", roots: ["/p"], status: [modified("/p/a.ts")] }); @@ -59,8 +63,8 @@ describe("Commit dialog diff window (#1863)", () => { , ); activateFile("a.ts"); - expect(openDiff).not.toHaveBeenCalled(); await diffDialogs(); + expect(openDiff).not.toHaveBeenCalled(); expect(screen.getByTestId("diff-view")).toHaveAttribute("data-path", "/p/a.ts"); expect(editor.mock.calls.at(-1)?.[0].input.data.embedded).toBe(true); }); @@ -85,13 +89,13 @@ describe("Commit dialog diff window (#1863)", () => { render(); const f = modified("/p/a.ts"); const node: ChangeNode = { key: f.path, type: "file", label: "a.ts", group: "changes", files: [f], path: f.path }; - const item = nodeMenu([node], ["/p"], { mode: "dialog", roots: [node] }).find( + const item = nodeMenu([node], ["/p"], { roots: [node] }).find( (i) => i.kind !== "separator" && "label" in i && i.label === "Show Diff", ); expect(item).toMatchObject({ label: "Show Diff" }); act(() => (item as { run: () => void }).run()); - expect(openDiff).not.toHaveBeenCalled(); await diffDialogs(); + expect(openDiff).not.toHaveBeenCalled(); expect(screen.getByTestId("diff-view")).toHaveAttribute("data-path", "/p/a.ts"); }); @@ -111,4 +115,58 @@ describe("Commit dialog diff window (#1863)", () => { expect(screen.getByTestId("diff-view")).toHaveAttribute("data-path", "/p/b.ts"); await screen.findByRole("dialog", { name: "b.ts (HEAD ↔ Your version)" }); }); + + it("closes on Jump to Source so the editor it opens is not hidden behind it", async () => { + render( + <> + + + , + ); + activateFile("a.ts"); + await diffDialogs(); + act(() => editor.mock.calls.at(-1)?.[0].onJumpToSource()); + expect(screen.queryByRole("dialog", { name: /\(HEAD ↔ Your version\)/ })).toBeNull(); + expect(screen.getByRole("tree", { name: "Changes" })).toBeTruthy(); + }); + + it.each(["body", "footer", "titlebar"])("owns the close shortcut with %s focus", (location) => { + const closeTab = vi.fn(); + const unregister = registerAction({ id: "editor.closeTab", title: "Close Tab", run: closeTab }); + const keymap = useKeymap.getState(); + useKeymap.setState({ overrides: { "editor.closeTab": ["Meta+W"], "toolwindow.closeContent": ["Meta+W"] } }); + const uninstall = installKeyboardDispatcher(window); + render(); + let close: () => void; + try { + act(() => { + close = openDiffDialog({ path: "/p/a.ts", left: { type: "rev", rev: "HEAD" }, right: { type: "worktree" } }); + }); + const dialog = screen.getByRole("dialog", { name: /\(HEAD ↔ Your version\)/ }); + const target = dialog.querySelector( + location === "body" + ? "[data-testid='diff-view']" + : location === "footer" + ? "[data-dialog-footer] button" + : "[data-dialog-titlebar] button", + )!; + fireEvent.keyDown(target, { key: "w", code: "KeyW", metaKey: true }); + expect(closeTab).not.toHaveBeenCalled(); + expect(screen.queryByRole("dialog", { name: /\(HEAD ↔ Your version\)/ })).toBeNull(); + } finally { + act(() => close?.()); + uninstall(); + unregister(); + useKeymap.setState(keymap); + } + }); +}); + +it("opens an editor tab instead once the non-modal interface is chosen", async () => { + useProject.setState({ settings: null }); + useVcs.setState({ available: true, root: "/p", repo: "/p", roots: ["/p"], status: [modified("/p/a.ts")] }); + render(); + activateFile("a.ts"); + await vi.waitFor(() => expect(openDiff).toHaveBeenCalledTimes(1)); + expect(screen.queryByRole("dialog", { name: /\(HEAD ↔ Your version\)/ })).toBeNull(); }); diff --git a/src/features/vcs/__tests__/directoryCompare.test.tsx b/src/features/vcs/__tests__/directoryCompare.test.tsx index d4c069d10..411f898fd 100644 --- a/src/features/vcs/__tests__/directoryCompare.test.tsx +++ b/src/features/vcs/__tests__/directoryCompare.test.tsx @@ -36,6 +36,7 @@ vi.mock("../dialogs", () => ({ confirmDialog: mocks.confirm, openDialog: vi.fn() vi.mock("../diffInput", async (original) => ({ ...(await original()), openDiff: mocks.open, + openVcsDiff: mocks.open, })); import { DirectoryCompareDialog, directoryDiff } from "../DirectoryCompare"; diff --git a/src/features/vcs/__tests__/directoryCompareActions.test.ts b/src/features/vcs/__tests__/directoryCompareActions.test.ts index 4a0d5cbae..5eccd6ce9 100644 --- a/src/features/vcs/__tests__/directoryCompareActions.test.ts +++ b/src/features/vcs/__tests__/directoryCompareActions.test.ts @@ -10,7 +10,7 @@ const mocks = vi.hoisted(() => ({ })); vi.mock("../DirectoryCompare", () => ({ showDirectoryComparison: mocks.show })); vi.mock("../dialogs", () => ({ promptText: mocks.prompt })); -vi.mock("../diffInput", () => ({ openDiff: mocks.openDiff })); +vi.mock("../diffInput", () => ({ openDiff: mocks.openDiff, openVcsDiff: mocks.openDiff })); vi.mock("@tauri-apps/plugin-dialog", () => ({ open: mocks.pick })); vi.mock("@/platform/remote", () => ({ isRemoteWindow: mocks.remote })); vi.mock("@/platform/ipc", async (original) => ({ diff --git a/src/features/vcs/__tests__/folderCompare.test.tsx b/src/features/vcs/__tests__/folderCompare.test.tsx index 58d703ced..db3f4a03f 100644 --- a/src/features/vcs/__tests__/folderCompare.test.tsx +++ b/src/features/vcs/__tests__/folderCompare.test.tsx @@ -22,6 +22,7 @@ vi.mock("@/platform/ipc", async (original) => { }, }; }); +vi.mock("../diffDialog", () => ({ openDiffDialog: mocks.open })); vi.mock("../DiffEditor", () => ({ DiffEditor: ({ input }: { input: unknown }) =>
{JSON.stringify(input)}
, })); @@ -151,6 +152,20 @@ describe("directory comparison", () => { expect(screen.getByTestId("diff")).toHaveTextContent("/workspace/client/src/app.ts"); }); + it("keeps the list open under the diff window of the modal commit interface", async () => { + mocks.compare.mockResolvedValue({ status: "ok", data: result }); + useProject.setState({ settings: { vcs: { commit: { nonModal: false } } } as never }); + const onClose = vi.fn(); + render(); + fireEvent.mouseDown(await screen.findByText("app.ts"), { button: 0 }); + fireEvent.click(await screen.findByRole("button", { name: "Show Diff" })); + await waitFor(() => + expect(mocks.open).toHaveBeenCalledWith(expect.objectContaining({ right: { type: "worktree" } })), + ); + expect(onClose).not.toHaveBeenCalled(); + useProject.setState({ settings: null }); + }); + it("cancels the backend request when a running comparison is closed", async () => { mocks.compare.mockReturnValue(new Promise(() => {})); const view = render( diff --git a/src/features/vcs/__tests__/localChanges.test.tsx b/src/features/vcs/__tests__/localChanges.test.tsx index 23b2f8a52..22bc895fc 100644 --- a/src/features/vcs/__tests__/localChanges.test.tsx +++ b/src/features/vcs/__tests__/localChanges.test.tsx @@ -12,21 +12,30 @@ vi.mock("../DiffEditor", () => ({ ), })); -const openDiff = vi.hoisted(() => vi.fn()); -vi.mock("../diffInput", async (original) => ({ - ...(await original()), - openDiff, -})); +const openDiffDialog = vi.hoisted(() => vi.fn()); +vi.mock("../diffDialog", () => ({ openDiffDialog })); -import { LocalChangesView } from "../CommitView"; +import { ChangesTree, LocalChangesView, nodeMenu } from "../CommitView"; +import type { ChangeNode } from "../changes"; import { setDiffPreview, useCommit } from "../commitStore"; import { useVcs } from "../store"; import { useToolWindowLayout } from "@/workbench/toolwindows/store"; +import type { FileStatus, Settings } from "@/bindings"; +import { useProject } from "@/platform/project"; +import { provideEditorService } from "@/workbench/contributions"; + +/** Editor tabs the diffs open in (openDiff → the editor service). */ +const openInput = vi.fn(async () => undefined); +provideEditorService({ openInput } as never); +/** Settings › Version Control › Commit: "Use non-modal commit interface" unticked. */ +const modal = (on: boolean) => + useProject.setState({ settings: on ? ({ vcs: { commit: { nonModal: false } } } as Settings) : null }); +const tabPaths = () => openInput.mock.calls.map((c) => (c as unknown as [{ data: { path: string } }])[0].data.path); describe("Local Changes diff preview (#710)", () => { beforeEach(() => { localStorage.clear(); - openDiff.mockClear(); + modal(true); useCommit.setState({ diffPreview: true, selection: [] }); useVcs.setState({ root: "/p", repo: "/p", roots: ["/p"], status: [] }); useToolWindowLayout.setState({ placements: {} }); @@ -68,7 +77,7 @@ describe("Local Changes diff preview (#710)", () => { expect(screen.queryByTestId("diff-preview")).toBeNull(); }); - it("previews keyboard selection and opens the full diff on Enter and double click", () => { + it("previews keyboard selection and opens the diff window on Enter and double click", async () => { useVcs.setState({ status: ["a.ts", "b.ts"].map((name) => ({ path: `/p/${name}`, @@ -87,11 +96,16 @@ describe("Local Changes diff preview (#710)", () => { fireEvent.keyDown(tree, { key: "ArrowDown" }); expect(screen.getByTestId("diff-preview")).toHaveAttribute("data-path", "/p/b.ts"); expect(tree).toHaveFocus(); - expect(openDiff).not.toHaveBeenCalled(); + expect(openInput).not.toHaveBeenCalled(); fireEvent.keyDown(tree, { key: "Enter" }); - expect(openDiff).toHaveBeenLastCalledWith(expect.objectContaining({ path: "/p/b.ts" }), { preview: true }); + await vi.waitFor(() => + expect(openDiffDialog).toHaveBeenLastCalledWith(expect.objectContaining({ path: "/p/b.ts" })), + ); fireEvent.doubleClick(screen.getByText("a.ts")); - expect(openDiff).toHaveBeenLastCalledWith(expect.objectContaining({ path: "/p/a.ts" }), { preview: true }); + await vi.waitFor(() => + expect(openDiffDialog).toHaveBeenLastCalledWith(expect.objectContaining({ path: "/p/a.ts" })), + ); + expect(openInput).not.toHaveBeenCalled(); }); it("turns off via the View Options toggle, persists, and keeps the tree alone", () => { @@ -128,3 +142,75 @@ describe("Local Changes diff preview (#710)", () => { expect(screen.getByTestId("diff-preview")).toHaveAttribute("data-path", "/p/b.ts"); }); }); + +describe("Changes tree diffs follow the commit interface setting (#1863)", () => { + const file: FileStatus = { + path: "/p/src/a.ts", + relPath: "src/a.ts", + staged: null, + unstaged: "modified", + renamedFrom: null, + }; + const node: ChangeNode = { + key: file.path, + type: "file", + label: "src/a.ts", + group: "changes", + files: [file], + path: file.path, + }; + + beforeEach(() => { + localStorage.clear(); + modal(true); + useCommit.setState({ diffPreview: false, selection: [], checks: new Map() }); + useVcs.setState({ available: true, root: "/p", repo: "/p", roots: ["/p"], status: [file] }); + useToolWindowLayout.setState({ placements: {} }); + }); + + const activate = async () => { + const row = (await screen.findByText("a.ts")).closest('[role="treeitem"]')!; + act(() => fireEvent.doubleClick(row)); + }; + + it("opens the diff of a double-clicked file in front with the modal interface", async () => { + render(); + await activate(); + await vi.waitFor(() => expect(openDiffDialog).toHaveBeenCalledTimes(1)); + expect(openDiffDialog.mock.calls[0]?.[0]).toMatchObject({ path: "/p/src/a.ts" }); + expect(openInput).not.toHaveBeenCalled(); + }); + + it("opens an editor tab with the non-modal interface", async () => { + modal(false); + render(); + await activate(); + await vi.waitFor(() => expect(tabPaths()).toEqual(["/p/src/a.ts"])); + expect(openInput.mock.calls[0]).toEqual([expect.anything(), { preview: true, focus: true }]); + expect(openDiffDialog).not.toHaveBeenCalled(); + }); + + it("goes by the setting, not by which tree shows the file", async () => { + modal(false); + render(); + await activate(); + await vi.waitFor(() => expect(tabPaths()).toEqual(["/p/src/a.ts"])); + expect(openDiffDialog).not.toHaveBeenCalled(); + }); + + it("opens the Show Diff menu item in front with the modal interface", async () => { + const menu = nodeMenu([node], ["/p"], { roots: [node] }); + const show = menu.find((m) => m.kind !== "separator" && "label" in m && m.label === "Show Diff"); + expect(show && "run" in show).toBe(true); + (show as { run: () => void }).run(); + await vi.waitFor(() => + expect(openDiffDialog).toHaveBeenCalledWith(expect.objectContaining({ path: "/p/src/a.ts" })), + ); + }); + + it("keeps the Show Diff action item with the non-modal interface", () => { + modal(false); + const menu = nodeMenu([node], ["/p"], { roots: [node] }); + expect(menu).toContainEqual({ kind: "action", action: "vcs.showDiff", arg: "/p/src/a.ts" }); + }); +}); diff --git a/src/features/vcs/__tests__/modalDiffWindow.test.ts b/src/features/vcs/__tests__/modalDiffWindow.test.ts new file mode 100644 index 000000000..2153fa3f4 --- /dev/null +++ b/src/features/vcs/__tests__/modalDiffWindow.test.ts @@ -0,0 +1,70 @@ +import { afterAll, beforeAll, beforeEach, expect, it, vi } from "vitest"; +import type { Settings } from "@/bindings"; +import { getAction } from "@/platform/actions"; +import { useProject } from "@/platform/project"; +import { provideEditorService } from "@/workbench/contributions"; +import { registerCompareActions } from "../compare"; +import { showChangesDiff, type LogChange } from "../logChanges"; + +const front = vi.hoisted(() => ({ diff: vi.fn(), merge: vi.fn() })); +vi.mock("../diffDialog", () => ({ openDiffDialog: front.diff, openMergeDiffDialog: front.merge })); + +/** Editor tabs the diffs open in (openDiff → the editor service). */ +const openInput = vi.fn(async () => undefined); +provideEditorService({ openInput } as never); +/** Settings › Version Control › Commit: "Use non-modal commit interface" unticked. */ +const modal = (on: boolean) => + useProject.setState({ settings: on ? ({ vcs: { commit: { nonModal: false } } } as Settings) : null }); + +let unregister: () => void; +beforeAll(() => (unregister = registerCompareActions())); +afterAll(() => unregister()); +beforeEach(() => modal(false)); + +const texts = { path: "/p/a.ts", left: { text: "a", label: "Expected" }, right: { text: "b", label: "Actual" } }; +const merge = { + path: "a.ts", + repo: "/p", + oldPath: null, + kind: "modified", + before: "p1", + after: "m", + statsFrom: "p1", + statsTo: "m", + mergeParents: [ + { rev: "p1", path: "a.ts" }, + { rev: "p2", path: "a.ts" }, + ], + diff: { path: "/p/a.ts", left: { type: "rev", rev: "p1" }, right: { type: "rev", rev: "m" } }, +} as LogChange; + +it("opens Compare actions in the front window under the modal interface", async () => { + modal(true); + await getAction("diff.compareTexts")!.run({ arg: texts, ctx: {} }); + expect(front.diff).toHaveBeenCalledWith(expect.objectContaining({ path: "/p/a.ts" })); + expect(openInput).not.toHaveBeenCalled(); +}); + +it("keeps Compare actions in editor tabs with the non-modal interface", async () => { + await getAction("diff.compareTexts")!.run({ arg: texts, ctx: {} }); + expect(openInput).toHaveBeenCalledTimes(1); + expect(front.diff).not.toHaveBeenCalled(); +}); + +it("opens a merge commit's file in the front window under the modal interface", async () => { + modal(true); + await showChangesDiff([merge]); + expect(front.merge).toHaveBeenCalledWith(expect.objectContaining({ kind: "git-merge-diff" })); + expect(openInput).not.toHaveBeenCalled(); +}); + +it("keeps a merge commit's file in an editor tab with the non-modal interface, and previews too", async () => { + await showChangesDiff([merge]); + expect(openInput).toHaveBeenLastCalledWith(expect.objectContaining({ kind: "git-merge-diff" }), { + preview: false, + focus: true, + }); + modal(true); + await showChangesDiff([merge], true); + expect(front.merge).not.toHaveBeenCalled(); +}); diff --git a/src/features/vcs/__tests__/shelf.test.tsx b/src/features/vcs/__tests__/shelf.test.tsx index 3d9623644..330a749b5 100644 --- a/src/features/vcs/__tests__/shelf.test.tsx +++ b/src/features/vcs/__tests__/shelf.test.tsx @@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => ({ openDiff: vi.fn(), execute: vi.fn(), ipc: vi.f vi.mock("../diffInput", async (original) => ({ ...(await original()), openDiff: mocks.openDiff, + openVcsDiff: mocks.openDiff, })); vi.mock("@/platform/actions", async (original) => ({ ...(await original()), diff --git a/src/features/vcs/__tests__/show-diff-chain.test.ts b/src/features/vcs/__tests__/show-diff-chain.test.ts index b917a11e6..76d13a7ef 100644 --- a/src/features/vcs/__tests__/show-diff-chain.test.ts +++ b/src/features/vcs/__tests__/show-diff-chain.test.ts @@ -1,14 +1,18 @@ -import { afterEach, beforeEach, expect, it, vi } from "vitest"; -import type { FileStatus } from "@/bindings"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { FileStatus, Settings } from "@/bindings"; import { getAction } from "@/platform/actions"; import { setContext } from "@/platform/context"; +import { useProject } from "@/platform/project"; import { registerVcsActions } from "../actions"; import { useVcs } from "../store"; import { useCommit } from "../commitStore"; import { initialChangelists, useChangelists } from "../changelists"; -import { chainNeighbour } from "../diffInput"; +import { useGitLog } from "../logStore"; +import { compareChangeWithLocal } from "../logChanges"; +import { chainNeighbour, openVcsDiff } from "../diffInput"; const opened = vi.hoisted(() => vi.fn()); +const diffWindow = vi.hoisted(() => vi.fn()); const project = vi.hoisted(() => ({ paths: [] as string[] })); vi.mock("@/workbench/contributions", async (original) => ({ ...(await original()), @@ -16,6 +20,7 @@ vi.mock("@/workbench/contributions", async (original) => ({ projectViewSelection: () => project.paths, })); vi.mock("../comparePaths", () => ({ showDirectoryWorkingDiff: async () => false })); +vi.mock("../diffDialog", () => ({ openDiffDialog: diffWindow })); const status = (name: string): FileStatus => ({ path: `/repo/${name}`, @@ -27,6 +32,9 @@ const status = (name: string): FileStatus => ({ const vcs = useVcs.getState(); const commit = useCommit.getState(); const lists = useChangelists.getState(); +const gitTab = useGitLog.getState().tab; +const settings = useProject.getState().settings; +const modal = () => useProject.setState({ settings: { vcs: { commit: { nonModal: false } } } as Settings }); let unregister: () => void; beforeEach(() => { @@ -49,6 +57,8 @@ afterEach(() => { useVcs.setState(vcs); useCommit.setState(commit); useChangelists.setState(lists); + useGitLog.setState({ tab: gitTab }); + useProject.setState({ settings }); }); it("keeps sibling files in tree order when Show Diff opens a changed file", async () => { @@ -112,3 +122,104 @@ it("shows a Project view selection outside the changes tree", async () => { await getAction("vcs.showDiff")!.run({ arg: undefined, ctx: { vcsAvailable: true } }); expect(opened.mock.calls[0]![0].data.path).toBe("/repo/z.ts"); }); + +it("opens the front diff window from the modal Local Changes selection (⌘D)", async () => { + modal(); + setContext("activeToolWindow", "git"); + useGitLog.setState({ tab: "changes" }); + useCommit.setState({ selection: ["/repo/b.ts"] }); + await getAction("vcs.showDiff")!.run({ arg: undefined, ctx: { vcsAvailable: true } }); + expect(diffWindow).toHaveBeenCalledTimes(1); + const data = diffWindow.mock.calls[0]![0]; + expect(data.path).toBe("/repo/b.ts"); + expect(data.chain.map((d: { path: string }) => d.path)).toEqual(["/repo/a.ts", "/repo/b.ts", "/repo/z.ts"]); + expect(opened).not.toHaveBeenCalled(); +}); + +it("keeps the editor diff outside the modal interface with the Commit window focused", async () => { + useProject.setState({ settings: null }); + setContext("activeToolWindow", "commit"); + useCommit.setState({ selection: ["/repo/b.ts"] }); + await getAction("vcs.showDiff")!.run({ arg: undefined, ctx: { vcsAvailable: true } }); + expect(diffWindow).not.toHaveBeenCalled(); + expect(opened).toHaveBeenCalled(); +}); + +it("openVcsDiff opens the front window under the modal interface", async () => { + modal(); + await openVcsDiff({ path: "/repo/b.ts", left: { type: "rev", rev: "HEAD" }, right: { type: "worktree" } }); + expect(diffWindow).toHaveBeenCalledTimes(1); + expect(diffWindow.mock.calls[0]![0]).toMatchObject({ path: "/repo/b.ts" }); + expect(opened).not.toHaveBeenCalled(); +}); + +it("openVcsDiff keeps editor tabs without the modal interface", async () => { + useProject.setState({ settings: null }); + await openVcsDiff({ path: "/repo/b.ts", left: { type: "rev", rev: "HEAD" }, right: { type: "worktree" } }); + expect(diffWindow).not.toHaveBeenCalled(); + expect(opened).toHaveBeenCalled(); +}); + +it("Compare with Local opens the front window under the modal interface", async () => { + modal(); + await compareChangeWithLocal({ + path: "b.ts", + repo: "/repo", + oldPath: null, + kind: "modified", + before: null, + after: "abc123", + statsFrom: "p", + statsTo: "c", + diff: { path: "/repo/b.ts", left: { type: "rev", rev: "p" }, right: { type: "rev", rev: "c" } }, + }); + expect(diffWindow).toHaveBeenCalledTimes(1); + expect(diffWindow.mock.calls[0]![0]).toMatchObject({ path: "/repo/b.ts" }); +}); + +it("keeps the hunk's line when Show Diff opens the front window (gutter popup)", async () => { + modal(); + await getAction("vcs.showDiff")!.run({ arg: { path: "/repo/b.ts", line: 42 }, ctx: { vcsAvailable: true } }); + expect(diffWindow).toHaveBeenCalledTimes(1); + expect(diffWindow.mock.calls[0]![0]).toMatchObject({ path: "/repo/b.ts", line: 42 }); +}); + +describe("a partially staged file under the staging groups", () => { + const partial: FileStatus = { ...status("b.ts"), staged: "modified" }; + beforeEach(() => { + modal(); + useVcs.setState({ status: [partial], byPath: new Map([[partial.path, partial]]) }); + useCommit.setState({ groupByStaging: true }); + }); + + it("opens its whole working diff for a path, which does not tell the side", async () => { + await getAction("vcs.showDiff")!.run({ arg: "/repo/b.ts", ctx: { vcsAvailable: true } }); + expect(diffWindow.mock.calls[0]![0]).toMatchObject({ + left: { type: "rev", rev: "HEAD" }, + right: { type: "worktree" }, + }); + }); + + it("opens the side selected in the changes tree", async () => { + setContext("activeToolWindow", "git"); + useGitLog.setState({ tab: "changes" }); + useCommit.setState({ selection: ["/repo/b.ts"], selectionKeys: ["unstaged:/repo/b.ts"] }); + await getAction("vcs.showDiff")!.run({ arg: undefined, ctx: { vcsAvailable: true } }); + expect(diffWindow.mock.calls[0]![0]).toMatchObject({ + left: { type: "rev", rev: ":0" }, + right: { type: "worktree" }, + }); + }); +}); + +it("chains the selected files in the front window", async () => { + modal(); + setContext("activeToolWindow", "git"); + useGitLog.setState({ tab: "changes" }); + useCommit.setState({ selection: ["/repo/z.ts", "/repo/other.ts"], selectionKeys: [] }); + await getAction("vcs.showDiff")!.run({ arg: undefined, ctx: { vcsAvailable: true } }); + expect(diffWindow).toHaveBeenCalledTimes(1); + const data = diffWindow.mock.calls[0]![0]; + expect(data.path).toBe("/repo/z.ts"); + expect(data.chain.map((d: { path: string }) => d.path)).toEqual(["/repo/z.ts", "/repo/other.ts"]); +}); diff --git a/src/features/vcs/actions.tsx b/src/features/vcs/actions.tsx index a1ca25733..8bb7b59c2 100644 --- a/src/features/vcs/actions.tsx +++ b/src/features/vcs/actions.tsx @@ -27,8 +27,16 @@ import { import { setAmend, useCommit } from "./commitStore"; import { gotoConflict, resolveAtCaret, showConflictsDialog } from "./conflictsUi"; import { focusedDiffNavigator } from "./diffNavigator"; -import { openDiff, withChain, workingDiffData } from "./diffInput"; -import { buildChangeTree, changeGroupPaths } from "./changes"; +import { + changeNodeDiff, + changeNodeDiffData, + openDiff, + openVcsDiff, + withChain, + workingDiffData, + type DiffData, +} from "./diffInput"; +import { buildChangeTree, changeFileNodes, changeGroupPaths, type ChangeNode } from "./changes"; import { useChangelists } from "./changelists"; import { openDialog } from "./dialogs"; import { gotoChange, rollbackSelectedLines } from "./gutter"; @@ -75,6 +83,34 @@ export const commitWindowFocused = () => { return active === "commit" || (active === "git" && useGitLog.getState().tab === "changes"); }; +/** A changed file's working diff, with the other files of its group (in tree order) as the F7 chain. */ +function groupDiffData(tree: readonly ChangeNode[], path: string): DiffData { + const siblings = changeGroupPaths(tree, path); + return withChain( + siblings.map((p) => workingDiffData(p)), + Math.max(0, siblings.indexOf(path)), + ); +} + +/** + * Show Diff's files as tree nodes where the side of the index is known: the changes + * tree's selected file nodes, else the nodes of `paths` the tree lists once. A + * partially staged file listed under both staging groups stays a path: its whole + * working diff. + */ +function showDiffTargets(tree: readonly ChangeNode[], paths: readonly string[], fromTree: boolean) { + const files = changeFileNodes(tree); + if (fromTree) { + const keys = new Set(useCommit.getState().selectionKeys); + const selected = files.filter((n) => keys.has(n.key)); + if (selected.length && selected.length === keys.size) return selected; + } + return paths.map((path): ChangeNode | string => { + const nodes = files.filter((n) => n.path === path); + return nodes.length === 1 ? nodes[0]! : path; + }); +} + /** The action's own paths, or the changes tree's selection while the tree has focus. */ function changedPaths(arg: unknown): string[] { const explicit = argPaths(arg); @@ -354,13 +390,25 @@ export function registerVcsActions(): () => void { state.roots, useCommit.getState().groupByStaging, ); + // The modal interface has one front diff window (#1863): a file with its group + // as the F7 chain, or several selected files as the chain. + if (isModalCommit()) { + const targets = showDiffTargets(tree, paths, !argPaths(arg)?.length && commitWindowFocused()); + const [first] = targets; + const data = + targets.length > 1 + ? withChain( + targets.map((t) => (typeof t === "string" ? workingDiffData(t) : changeNodeDiff(t))), + 0, + ) + : typeof first === "string" + ? groupDiffData(tree, first) + : changeNodeDiffData(tree, first!); + await openVcsDiff({ ...data, line }); + return; + } for (const path of paths.slice(0, 5)) { - const siblings = changeGroupPaths(tree, path); - const items = siblings.map((path) => workingDiffData(path)); - await openDiff( - { ...withChain(items, Math.max(0, siblings.indexOf(path))), line }, - { preview: paths.length === 1 }, - ); + await openDiff({ ...groupDiffData(tree, path), line }, { preview: paths.length === 1 }); } }, }, diff --git a/src/features/vcs/changelistsUi.tsx b/src/features/vcs/changelistsUi.tsx index 2f4021233..935fbd08c 100644 --- a/src/features/vcs/changelistsUi.tsx +++ b/src/features/vcs/changelistsUi.tsx @@ -17,6 +17,7 @@ import { } from "./changelists"; import { groupOf } from "./changes"; import { useCommit } from "./commitStore"; +import { isModalCommit, openCommit } from "./commitUi"; import { confirmDialog, openDialog } from "./dialogs"; import { fileCount, stripSlash } from "./model"; import { useVcs } from "./store"; @@ -178,20 +179,28 @@ export function setActiveChangelist(id: string) { updateChangelists((s) => setActiveList(s, id)); } -/** Commit… on a change list: include exactly its changes and focus the message. */ +/** + * Commit… on a change list: include exactly its changes and focus the message — under + * the modal interface it is in the Commit dialog, which this opens. + */ export function commitChangelist(id: string) { const { status, root } = useVcs.getState(); const checks = checksForList(status, root ?? "", useChangelists.getState().state, id); useCommit.setState((s) => ({ checks, tab: "changes", focusTick: s.focusTick + 1 })); + if (isModalCommit()) openCommit(); } -/** Commit File…: include exactly these changes (absolute paths) and focus the message. */ +/** + * Commit File…: include exactly these changes (absolute paths) and focus the message — + * under the modal interface it is in the Commit dialog, which this opens. + */ export function commitFiles(paths: readonly string[]) { const chosen = new Set(paths.map(stripSlash)); const checks = new Map(); for (const f of useVcs.getState().status) if (groupOf(f) === "changes") checks.set(f.path, chosen.has(stripSlash(f.path)) ? "all" : "none"); useCommit.setState((s) => ({ checks, tab: "changes", focusTick: s.focusTick + 1 })); + if (isModalCommit()) openCommit(); } /** Moves files (absolute paths) to a list. */ diff --git a/src/features/vcs/changes.ts b/src/features/vcs/changes.ts index 87e5c93b5..3bbfaa40b 100644 --- a/src/features/vcs/changes.ts +++ b/src/features/vcs/changes.ts @@ -311,6 +311,19 @@ export function changeGroupPaths(roots: readonly ChangeNode[], path: string): st return paths; } +/** Every file node of the tree, in tree order (a partially staged file is listed under both staging groups). */ +export function changeFileNodes(roots: readonly ChangeNode[]): ChangeNode[] { + const out: ChangeNode[] = []; + const walk = (nodes: readonly ChangeNode[]) => { + for (const node of nodes) { + if (node.type === "file" && node.path) out.push(node); + else walk(node.children ?? []); + } + }; + walk(roots); + return out; +} + /** File nodes, in tree order, of the group a file node belongs to. */ export function groupFileNodes(roots: readonly ChangeNode[], node: ChangeNode): ChangeNode[] { const group = roots.find((r) => r.group === node.group && r.files.some((f) => stripSlash(f.path) === node.path)); diff --git a/src/features/vcs/commitStore.ts b/src/features/vcs/commitStore.ts index b8b98a0e1..80f98eeac 100644 --- a/src/features/vcs/commitStore.ts +++ b/src/features/vcs/commitStore.ts @@ -27,6 +27,8 @@ interface CommitState { excludedHunks: Map>; /** Absolute paths selected in the changes tree. */ selection: string[]; + /** Keys of the selected tree nodes: which staging side of a partially staged file. */ + selectionKeys: string[]; /** Bumped to focus the message field. */ focusTick: number; /** Bumped to open the commit message history popup. */ @@ -57,6 +59,7 @@ export const useCommit = create(() => ({ checks: new Map(), excludedHunks: new Map(), selection: [], + selectionKeys: [], focusTick: 0, historyTick: 0, checkProblems: [], @@ -280,6 +283,7 @@ export function installPreparedMessage(): () => void { checks: new Map(), excludedHunks: new Map(), selection: [], + selectionKeys: [], tab: "changes", selectedList: null, selectedShelf: null, diff --git a/src/features/vcs/compare.tsx b/src/features/vcs/compare.tsx index 1437abdbe..1d9daffa9 100644 --- a/src/features/vcs/compare.tsx +++ b/src/features/vcs/compare.tsx @@ -11,7 +11,7 @@ import { promptText } from "./dialogs"; import { focusedDiffNavigator } from "./diffNavigator"; import { compareFilesData, displayPath } from "./compareData"; export { compareFilesData, displayPath } from "./compareData"; -import { openDiff, type DiffData } from "./diffInput"; +import { openDiff, openVcsDiff, type DiffData } from "./diffInput"; import { baseName } from "./model"; import { tx } from "@/platform/i18n"; @@ -238,9 +238,9 @@ export function registerCompareActions(): () => void { if (!other) return; if (other.type === "file") { if (other.path === path) return; - await openDiff(compareFilesData(path, other.path)); + await openVcsDiff(compareFilesData(path, other.path)); } else { - await openDiff({ + await openVcsDiff({ path, left: { type: "text", text: other.text, label: other.label }, right: { type: "worktree", label: displayPath(path) }, @@ -271,7 +271,7 @@ export function registerCompareActions(): () => void { run: async ({ arg }) => { const paths = argPaths(arg).filter((p) => !p.endsWith("/")); if (paths.length !== 2) return info("Select two files to compare"); - await openDiff(compareFilesData(paths[0]!, paths[1]!)); + await openVcsDiff(compareFilesData(paths[0]!, paths[1]!)); }, }, { @@ -292,7 +292,7 @@ export function registerCompareActions(): () => void { const ed = activeEditor(); const sel = ed && ed.ctx.path === path ? ed.view.state.selection.main : null; const selection = sel && !sel.empty ? ed!.view.state.sliceDoc(sel.from, sel.to) : null; - await openDiff(clipboardDiffData(path, clipboard, selection)); + await openVcsDiff(clipboardDiffData(path, clipboard, selection)); }, }, { @@ -304,7 +304,7 @@ export function registerCompareActions(): () => void { run: async ({ arg }) => { const a = parseTextsArg(arg); if (!a) return; - await openDiff({ + await openVcsDiff({ path: a.path, title: a.title, line: a.line, diff --git a/src/features/vcs/comparePaths.ts b/src/features/vcs/comparePaths.ts index 2ebd9a9bc..d8921e9dc 100644 --- a/src/features/vcs/comparePaths.ts +++ b/src/features/vcs/comparePaths.ts @@ -2,7 +2,7 @@ import { call, commands } from "@/platform/ipc"; import { notifications } from "@/workbench/contributions"; import { tx } from "@/platform/i18n"; import { chooseRepositoryRevision } from "./RevisionChooser"; -import { openDiff } from "./diffInput"; +import { openVcsDiff } from "./diffInput"; import { showPathComparison } from "./operations"; import { displayPath } from "./compare"; @@ -25,7 +25,7 @@ export async function comparePathsWithRevision(paths: string[], revision?: strin }); if (!choice) return; if (scope.singleFile && !revision) { - await openDiff({ + await openVcsDiff({ path: paths[0]!, left: { type: "rev", rev: choice.rev, label: choice.rev }, right: { type: "worktree" }, diff --git a/src/features/vcs/diffDialog.tsx b/src/features/vcs/diffDialog.tsx index 73056a84a..e7aeed4a4 100644 --- a/src/features/vcs/diffDialog.tsx +++ b/src/features/vcs/diffDialog.tsx @@ -1,43 +1,33 @@ /** - * The diff "window" of the modal Commit dialog (#1863): activating a changed file - * there opens the diff in front of the dialog, not in an editor tab behind it. - * A `Dialog` stacked on `openDialog`'s stack keeps the Commit dialog (and its - * message, selection and options) mounted underneath; Esc / the close button / - * the close-tab chord return to it. F7 / Shift+F7 and the diff navigation actions - * move along the file's change chain by swapping this dialog's content. + * The diff "window" of the modal commit interface (#1863): diffs open in front, not in + * an editor tab that the modal Commit dialog would hide. A `Dialog` stacked on + * `openDialog`'s stack keeps whatever is underneath (the Commit dialog and its message, + * selection and options, a comparison list) mounted; Esc / the close button / the + * close-tab chord return to it. F7 / Shift+F7 and the diff navigation actions move + * along the file's change chain by swapping this dialog's content. */ -import { useState, type KeyboardEvent } from "react"; -import { bindingsFor, normalizeBinding, strokeOf } from "@/platform/keymap"; +import { useEffect, useRef, useState, type ReactNode } from "react"; +import { bindingsFor, normalizeBinding, registerKeyClaim } from "@/platform/keymap"; import { Dialog } from "@/ui"; +import type { EditorInput } from "@/workbench/contributions"; import { DiffEditor } from "./DiffEditor"; import { diffInput, diffTitle, type DiffData } from "./diffInput"; import { openDialog } from "./dialogs"; - -/** Whether `e` is one of the strokes bound to `actionId`. */ -function isStrokeOf(e: KeyboardEvent, actionId: string): boolean { - const stroke = strokeOf(e.nativeEvent); - return !!stroke && bindingsFor(actionId).some((b) => normalizeBinding(b) === stroke); -} +import { GitMergeDiff } from "./GitMergeDiff"; /** The open diff dialog's close function (one diff window at a time, as in IDEA). */ let shown: (() => void) | null = null; -/** - * Opens `data` in a front dialog; returns its close function. A second activation - * replaces the open diff window instead of stacking another on top. - */ -export function openDiffDialog(data: DiffData): () => void { +/** Shows `render`'s window, replacing the open diff window instead of stacking another on top. */ +function showFront(render: (onClose: () => void) => ReactNode): () => void { shown?.(); shown = null; - const close = openDialog((c) => ( - { - if (shown === close) shown = null; - c(); - }} - /> - )); + const close = openDialog((c) => + render(() => { + if (shown === close) shown = null; + c(); + }), + ); shown = close; return () => { if (shown === close) shown = null; @@ -45,13 +35,40 @@ export function openDiffDialog(data: DiffData): () => void { }; } -function DiffDialog({ data: initial, onClose }: { data: DiffData; onClose: () => void }) { - const [data, setData] = useState(initial); - const input = diffInput({ ...data, embedded: true }); +/** Opens `data` in a front dialog; returns its close function. */ +export function openDiffDialog(data: DiffData): () => void { + return showFront((onClose) => ); +} + +/** Opens a merge commit's file (parent ↔ merge result ↔ parent, a `git-merge-diff` input) in front. */ +export function openMergeDiffDialog(input: EditorInput): () => void { + const title = (input.data as { path: string }).path; + return showFront((onClose) => ( + + + + )); +} + +function FrontWindow({ title, onClose, children }: { title: string; onClose: () => void; children: ReactNode }) { + const body = useRef(null); + useEffect( + () => + registerKeyClaim((e, stroke) => { + const dialog = body.current?.closest('[role="dialog"]'); + if (!dialog || !(e.target instanceof Element) || e.target.closest('[role="dialog"]') !== dialog) return false; + if (!bindingsFor("toolwindow.closeContent").some((b) => normalizeBinding(b) === stroke)) return false; + e.preventDefault(); + e.stopPropagation(); + onClose(); + return true; + }), + [onClose], + ); return ( buttons={[{ label: "Close", role: "cancel" }]} >
{ - // The "close tab" chord would otherwise close an editor tab behind the modal. - if (isStrokeOf(e, "toolwindow.closeContent")) { - e.preventDefault(); - e.stopPropagation(); - onClose(); - } - }} > - + {children}
); } + +function DiffDialog({ data: initial, onClose }: { data: DiffData; onClose: () => void }) { + const [data, setData] = useState(initial); + const input = diffInput({ ...data, embedded: true }); + return ( + + + + ); +} diff --git a/src/features/vcs/diffInput.ts b/src/features/vcs/diffInput.ts index 05567bd55..503b004e4 100644 --- a/src/features/vcs/diffInput.ts +++ b/src/features/vcs/diffInput.ts @@ -1,10 +1,11 @@ /** Diff editor inputs and helpers to open them. */ import type { ChangedFile, RevisionChange } from "@/bindings"; import { call, commands, type LineEnding } from "@/platform/ipc"; +import { workflowSettings } from "@/platform/workflowSettings"; import { editorService, type EditorInput } from "@/workbench/contributions"; import { baseName, effectiveKind } from "./model"; import { readWorkingText } from "./ops"; -import { renamedFromPath } from "./changes"; +import { groupFileNodes, renamedFromPath, stagingSide, type ChangeNode } from "./changes"; import { currentRepoRoot, statusOf } from "./store"; import { repositoryPath } from "./repositoryPaths"; @@ -133,6 +134,40 @@ export async function openDiff(data: DiffData, opts: { preview?: boolean; focus? await editorService().openInput(diffInput(data), { preview: opts.preview, focus: opts.focus ?? true }); } +/** + * Whether openVcsDiff shows diffs in the front window: the modal commit interface + * (Settings › Version Control › Commit). Same test as commitUi.isModalCommit, which + * this module cannot import (commitUi → remoteUi → … → diffInput is a cycle). + */ +export const vcsDiffInFront = () => !workflowSettings().vcs.commit.nonModal; + +/** + * Diffs the user opens (changed files, a log entry's file, Show Diff, Shelf, compare + * with a revision, Compare actions): the modal commit interface shows them in the front + * window (#1863), the non-modal one keeps editor tabs. Passive previews, diffs embedded + * in dedicated surfaces (the comparison window, merge editors) and pull request files + * (`vcs.openDiff`, with review comments) call openDiff directly. + */ +export async function openVcsDiff(data: DiffData, opts: { preview?: boolean; focus?: boolean } = {}) { + if (!vcsDiffInFront()) return openDiff(data, opts); + const { openDiffDialog } = await import("./diffDialog"); + openDiffDialog(data); +} + +/** A changes-tree file's working diff, on its side of the index under a "by staging state" group. */ +export function changeNodeDiff(node: ChangeNode, line?: number): DiffData { + return workingDiffData(node.path!, line, stagingSide(node.group, node.files[0]!)); +} + +/** A changes-tree file's diff, with the files of its group (in tree order) as the F7 chain. */ +export function changeNodeDiffData(roots: readonly ChangeNode[], node: ChangeNode): DiffData { + const nodes = groupFileNodes(roots, node); + return withChain( + nodes.map((n) => changeNodeDiff(n)), + Math.max(0, nodes.indexOf(node)), + ); +} + /** Default local-changes diff for a working file: HEAD (or empty) vs the working copy. */ export function workingDiffData(path: string, line?: number, side?: "staged" | "unstaged" | null): DiffData { if (side === "staged") { diff --git a/src/features/vcs/logChanges.ts b/src/features/vcs/logChanges.ts index c54879f5f..2b710c889 100644 --- a/src/features/vcs/logChanges.ts +++ b/src/features/vcs/logChanges.ts @@ -8,7 +8,15 @@ import type { ChangedFile, Commit, FileLineStats, RevisionChange } from "@/bindi import { call, commands } from "@/platform/ipc"; import { editorService } from "@/workbench/contributions"; import { tx } from "@/platform/i18n"; -import { commitFileDiffData, openDiff, revisionChangeDiffData, withChain, type DiffData } from "./diffInput"; +import { + commitFileDiffData, + openDiff, + openVcsDiff, + revisionChangeDiffData, + vcsDiffInFront, + withChain, + type DiffData, +} from "./diffInput"; import type { FileTreeNode } from "./fileTree"; import { baseName } from "./model"; import { readWorkingText, runOp, saveAll, writeFileText } from "./ops"; @@ -218,30 +226,30 @@ export async function showChangesDiff(changes: readonly LogChange[], preview = f const first = changes[0]; if (!first) return; if (first.mergeParents?.length === 2 && first.after) { - await editorService().openInput( - { - kind: "git-merge-diff", - key: `git-merge-diff:${first.repo}:${first.after}:${first.path}`, - data: { - repo: first.repo ?? currentRepoRoot(), - revision: first.after, - path: first.path, - parents: first.mergeParents, - }, + const input = { + kind: "git-merge-diff", + key: `git-merge-diff:${first.repo}:${first.after}:${first.path}`, + data: { + repo: first.repo ?? currentRepoRoot(), + revision: first.after, + path: first.path, + parents: first.mergeParents, }, - { preview, focus: !preview }, - ); + }; + if (!preview && vcsDiffInFront()) (await import("./diffDialog")).openMergeDiffDialog(input); + else await editorService().openInput(input, { preview, focus: !preview }); return; } const all = useLogChanges.getState().all; const chain = all.includes(first) ? all : [...changes]; - await openDiff( - withChain( - chain.map((c) => c.diff), - Math.max(0, chain.indexOf(first)), - ), - preview ? { preview: true, focus: false } : {}, + const data = withChain( + chain.map((c) => c.diff), + Math.max(0, chain.indexOf(first)), ); + // Selection preview stays a preview tab even under the modal interface; an explicit + // diff opens where the commit interface puts it. + if (preview) await openDiff(data, { preview: true, focus: false }); + else await openVcsDiff(data); } /** Compare with Local (`side` after) / Compare Before with Local: the revision ↔ the working file. */ @@ -250,7 +258,7 @@ export async function compareChangeWithLocal(change: LogChange, side: "after" | const rev = side === "after" ? change.after : change.before; if (!rev) return; const revPath = side === "before" && change.oldPath ? absolutePath(change.oldPath, change.repo) : undefined; - await openDiff({ + await openVcsDiff({ path: absolutePath(change.path, change.repo), left: { type: "rev", rev, path: revPath, label: rev.slice(0, 8) }, right: { type: "worktree" }, diff --git a/src/features/vcs/operations.tsx b/src/features/vcs/operations.tsx index 54a2bcab3..3e9f11700 100644 --- a/src/features/vcs/operations.tsx +++ b/src/features/vcs/operations.tsx @@ -11,7 +11,7 @@ import { call, commands, isTauri, nextRequestId } from "@/platform/ipc"; import { Button, Checkbox, Dialog, Icon, Input, Spinner, T } from "@/ui"; import { notifications } from "@/workbench/contributions"; import { windowBackgroundHex } from "@/workbench/theme"; -import { diffInput, openDiff, type DiffData, type DiffSide } from "./diffInput"; +import { diffInput, openVcsDiff, vcsDiffInFront, type DiffData, type DiffSide } from "./diffInput"; import { openDialog, promptText } from "./dialogs"; import { KIND_COLOR, fileCount } from "./model"; import { runGitOperation, runOp } from "./ops"; @@ -672,8 +672,9 @@ export function ChangesBetweenDialog({ if (!current) return; if (onShowDiff) onShowDiff(diffFor(current)); else { - void openDiff(diffFor(current)); - onClose(); + void openVcsDiff(diffFor(current)); + // A diff window opens on top of this list (#1863); an editor tab would sit behind it. + if (!vcsDiffInFront()) onClose(); } }; const contents = ( diff --git a/src/features/vcs/shelf.tsx b/src/features/vcs/shelf.tsx index 0fe4a5e02..a6607984b 100644 --- a/src/features/vcs/shelf.tsx +++ b/src/features/vcs/shelf.tsx @@ -8,7 +8,7 @@ import { create } from "zustand"; import type { ApplyOutcome, FileStatus, ShelvedChangeList, ShelfDiffFile } from "@/bindings"; import { setContext } from "@/platform/context"; import { shortcutLabel } from "@/platform/keymap"; -import { openDiff, withChain, type DiffData } from "./diffInput"; +import { openDiff, openVcsDiff, withChain, type DiffData } from "./diffInput"; import { showCreatePatchForShelf } from "./patches"; import { executeAction } from "@/platform/actions"; import { call, commands } from "@/platform/ipc"; @@ -375,7 +375,10 @@ export async function showShelfDiff(arg?: unknown, local = false, newTab = false ); items = items.filter((_, i) => supported[i]); } - if (items.length) await openDiff(withChain(items, 0), { preview: !newTab }); + if (!items.length) return; + // "Show Diff in a New Tab" asks for an editor tab whatever the commit interface. + if (newTab) await openDiff(withChain(items, 0), { preview: false }); + else await openVcsDiff(withChain(items, 0), { preview: true }); } export async function importShelfPatches(paths?: string[]) {