diff --git a/src/features/vcs/GitLog.tsx b/src/features/vcs/GitLog.tsx index 0b92336e5..b1d5e846c 100644 --- a/src/features/vcs/GitLog.tsx +++ b/src/features/vcs/GitLog.tsx @@ -86,7 +86,7 @@ import { logOptions, refLabels, useLogOptions, type LogOptions } from "./logOpti import { KIND_COLOR, baseName, dirName, fileCount, formatTime, relTo, type EffectiveKind } from "./model"; import { clearConsole, useVcsConsole } from "./ops"; import { currentRepo, currentRepoRoot, loadBranches, repoName, useVcs } from "./store"; -import { loadFavoriteBranches, useFavoriteBranches } from "./branchOps"; +import { canDeleteBranch, loadFavoriteBranches, useFavoriteBranches } from "./branchOps"; import { branchMenu, inRepo, tagMenu } from "./branchMenu"; import { branchFolderKeys, @@ -110,7 +110,7 @@ import { withLineStats, type LogChange, } from "./logChanges"; -import { canDeleteBranch, canUpdateBranch, useBranchesPane, type BranchTarget } from "./logPaneActions"; +import { canUpdateBranch, useBranchesPane, type BranchTarget } from "./logPaneActions"; import styles from "./GitLog.module.css"; import { tx } from "@/platform/i18n"; import { useWorkflowSettings } from "@/platform/workflowSettings"; @@ -496,8 +496,8 @@ export function BranchesPanel() { size="small" title="Delete Branch" action="vcs.log.branches.delete" - actionArg={target} - disabled={!canDeleteBranch(target ?? null)} + actionArg={targets} + disabled={!targets.some((t) => canDeleteBranch(t))} /> r.repo === n.repo).map((r) => r.ref) : [n.ref]; setLogFilters({ branches: sel, repositories: n.repo ? [n.repo] : [] }); }} - onContextMenu={(n, _s, e) => { + onContextMenu={(n, _sel, e) => { if (readOnly) { if (!n?.ref) return; e.preventDefault(); @@ -584,14 +584,27 @@ export function BranchesPanel() { ]); return; } + // The menu acts on the whole selection when the right-clicked branch is part of it. + const picked: BranchTarget[] = + n && selected.has(n.key) ? targets : n?.ref ? [{ ref: n.ref, branch: n.branch, repo: n.repo }] : []; const menu = n?.branch - ? branchMenu(n.branch, n.repo ? (useVcs.getState().heads[n.repo]?.head ?? null) : current) + ? branchMenu( + n.branch, + n.repo ? (useVcs.getState().heads[n.repo]?.head ?? null) : current, + () => {}, + picked, + ) : n?.kind === "tag" ? tagMenu(n.ref!) : n?.kind === "remote" || (n?.kind === "group" && n.key.endsWith("g:remote")) ? REMOTES_MENU : []; - const items = n?.repo ? inRepo(n.repo, menu) : menu; + // Delete scopes each selected target itself and must preserve the original focus. + const items = n?.repo + ? menu.flatMap((item) => + n.branch && "label" in item && item.label === "Delete" ? [item] : inRepo(n.repo!, [item]), + ) + : menu; if (!items.length) return; e.preventDefault(); showContextMenu(e, items); diff --git a/src/features/vcs/__tests__/deleteBranches.test.ts b/src/features/vcs/__tests__/deleteBranches.test.ts new file mode 100644 index 000000000..4a410db9b --- /dev/null +++ b/src/features/vcs/__tests__/deleteBranches.test.ts @@ -0,0 +1,171 @@ +import { beforeEach, expect, it, vi } from "vitest"; +import type { Branch } from "@/bindings"; + +const mocks = vi.hoisted(() => ({ + confirm: vi.fn(), + local: vi.fn(), + remote: vi.fn(), + focus: vi.fn(), + notify: vi.fn(), + range: vi.fn(), +})); +vi.mock("../dialogs", async (original) => ({ + ...(await original()), + confirmDialog: mocks.confirm, +})); +vi.mock("../ops", async (original) => ({ + ...(await original()), + runOp: mocks.local, + runRemote: mocks.remote, +})); +vi.mock("../store", async (original) => ({ + ...(await original()), + focusRepo: mocks.focus, +})); + +vi.mock("@/workbench/contributions", async (original) => ({ + ...(await original()), + notifications: () => ({ notify: mocks.notify }), +})); + +vi.mock("../CompareBranches", async (original) => ({ + ...(await original()), + showCommitRange: mocks.range, +})); + +import { canDeleteBranch, deleteBranches } from "../branchOps"; +import { branchMenu } from "../branchMenu"; +import { registerLogPaneActions, useBranchesPane } from "../logPaneActions"; +import { getAction } from "@/platform/actions"; +import { useVcs } from "../store"; + +const branch = (name: string, opts: Partial = {}): Branch => ({ + name, + remote: false, + current: false, + upstream: null, + ahead: 0, + behind: 0, + lastCommitMs: null, + ...opts, +}); +const target = (name: string, repo = "/p/a", opts: Partial = {}) => ({ + ref: name, + repo, + branch: branch(name, opts), +}); + +beforeEach(() => { + vi.resetAllMocks(); + useVcs.setState({ root: "/p", repo: "/p/a", branches: [], branchesLoaded: true }); + useBranchesPane.setState({ selected: [] }); + mocks.confirm.mockResolvedValue(true); + mocks.focus.mockImplementation(async (repo: string) => { + useVcs.setState({ repo }); + }); +}); + +it("restores the original repository after deleting across repositories", async () => { + const repos: (string | null)[] = []; + mocks.local.mockImplementation(async () => { + repos.push(useVcs.getState().repo); + }); + await deleteBranches([target("one", "/p/b"), target("two", "/p/a"), target("three", "/p/b")]); + expect(repos).toEqual(["/p/b", "/p/a", "/p/b"]); + expect(useVcs.getState().repo).toBe("/p/a"); +}); + +it("restores focus when deletion throws", async () => { + mocks.local.mockRejectedValue(new Error("failed")); + await expect(deleteBranches([target("one", "/p/b")])).rejects.toThrow("failed"); + expect(useVcs.getState().repo).toBe("/p/a"); +}); + +it("skips tags, groups and each repository's current branch", async () => { + const selected = [ + target("main", "/p/a", { current: true }), + target("other", "/p/b", { current: true }), + { ref: "v1", repo: "/p/a" }, + {}, + target("other", "/p/a"), + target("main", "/p/b"), + ]; + expect(selected.map((t) => canDeleteBranch("ref" in t ? t : null))).toEqual([false, false, false, false, true, true]); + await deleteBranches(selected); + expect(mocks.local.mock.calls.map(([title]) => title)).toEqual(["Delete branch other", "Delete branch main"]); +}); + +it("does nothing for only current branches or non-branch rows", async () => { + await deleteBranches([target("main", "/p/a", { current: true }), { ref: "v1" }, {}]); + expect(mocks.local).not.toHaveBeenCalled(); + expect(mocks.remote).not.toHaveBeenCalled(); + expect(mocks.confirm).not.toHaveBeenCalled(); + expect(mocks.focus).not.toHaveBeenCalled(); + expect(branchMenu(branch("main", { current: true }), "main").some((i) => "label" in i && i.label === "Delete")).toBe( + false, + ); +}); + +it.each([1, 2])("cancelling %s remote branches leaves the whole mixed selection untouched", async (count) => { + mocks.confirm.mockResolvedValue(false); + await deleteBranches([ + target("local"), + ...Array.from({ length: count }, (_, i) => target(`origin/r${i}`, "/p/b", { remote: true })), + ]); + expect(mocks.confirm).toHaveBeenCalledTimes(1); + expect(mocks.local).not.toHaveBeenCalled(); + expect(mocks.remote).not.toHaveBeenCalled(); + expect(mocks.focus).not.toHaveBeenCalled(); +}); + +it("confirms once for remote branches across repositories and deletes in the right repository", async () => { + const repos: (string | null)[] = []; + mocks.remote.mockImplementation(async () => { + repos.push(useVcs.getState().repo); + return true; + }); + await deleteBranches([ + target("origin/one", "/p/a", { remote: true }), + target("origin/two", "/p/b", { remote: true }), + ]); + expect(mocks.confirm).toHaveBeenCalledTimes(1); + expect(mocks.remote.mock.calls.map(([op]) => op.branch)).toEqual(["origin/one", "origin/two"]); + expect(repos).toEqual(["/p/a", "/p/b"]); + expect(useVcs.getState().repo).toBe("/p/a"); +}); + +it("the Delete action uses the whole pane selection without an explicit argument", async () => { + const dispose = registerLogPaneActions(); + try { + useBranchesPane.setState({ selected: [target("one"), target("two")] }); + await getAction("vcs.log.branches.delete")!.run({ ctx: {} }); + await vi.waitFor(() => expect(mocks.local).toHaveBeenCalledTimes(2)); + } finally { + dispose(); + } +}); + +it("Delete Tracked Branch keeps the repository captured before focus was restored", async () => { + mocks.local.mockResolvedValue({ name: "one", tip: "abc", upstream: "origin/one", unmerged: false }); + await deleteBranches([target("one", "/p/b")]); + expect(useVcs.getState().repo).toBe("/p/a"); + const notification = mocks.notify.mock.calls[0]![0]; + notification.actions.find((a: { label: string }) => a.label === "Delete Tracked Branch").run(); + await vi.waitFor(() => + expect(mocks.remote).toHaveBeenCalledWith({ op: "deleteBranch", branch: "origin/one" }, "origin/one", ["/p/b"]), + ); +}); + +it("View Commits keeps the deleted branch's repository after focus was restored", async () => { + mocks.local.mockResolvedValue({ name: "one", tip: "abc", upstream: null, unmerged: true }); + await deleteBranches([target("one", "/p/b")]); + const notification = mocks.notify.mock.calls[0]![0]; + notification.actions.find((a: { label: string }) => a.label === "View Commits").run(); + expect(mocks.range).toHaveBeenCalledWith("Commits of one", "HEAD", "abc", "/p/b"); +}); + +it("restores the primary repository when it was focused implicitly", async () => { + useVcs.setState({ repo: null, roots: ["/p/a", "/p/b"] }); + await deleteBranches([target("one", "/p/b")]); + expect(useVcs.getState().repo).toBe("/p/a"); +}); diff --git a/src/features/vcs/__tests__/logPanes.test.tsx b/src/features/vcs/__tests__/logPanes.test.tsx index 58b945216..0e8628470 100644 --- a/src/features/vcs/__tests__/logPanes.test.tsx +++ b/src/features/vcs/__tests__/logPanes.test.tsx @@ -5,6 +5,11 @@ import type { Branch, Commit } from "@/bindings"; const vcsCommitFiles = vi.fn(); const vcsDiffNumstat = vi.fn(); const vcsTags = vi.fn(); +const { deleteBranches } = vi.hoisted(() => ({ deleteBranches: vi.fn() })); +vi.mock("../branchOps", async (original) => ({ + ...(await original()), + deleteBranches, +})); vi.mock("@/platform/ipc", async (importOriginal) => { const real = await importOriginal(); @@ -22,6 +27,7 @@ vi.mock("@/platform/ipc", async (importOriginal) => { }; }); +import { canDeleteBranch } from "../branchOps"; import { buildBranchTree, filterBranchTree } from "../branches"; import { buildFileTree } from "../fileTree"; import { @@ -34,7 +40,7 @@ import { toLogChanges, useLogChanges, } from "../logChanges"; -import { canDeleteBranch, canUpdateBranch, useBranchesPane } from "../logPaneActions"; +import { canUpdateBranch, useBranchesPane } from "../logPaneActions"; import { BranchesPanel, CommitDetails, changesMenu, graphOptionsMenu } from "../GitLog"; import { LogSettingsPage } from "../LogSettingsPage"; import { DEFAULT_LOG_OPTIONS, useLogOptions } from "../logOptions"; @@ -75,6 +81,7 @@ afterEach(() => { vcsCommitFiles.mockReset(); vcsDiffNumstat.mockReset(); vcsTags.mockReset(); + deleteBranches.mockReset(); }); describe("changed files model", () => { @@ -213,6 +220,32 @@ describe("Branches pane", () => { expect(canUpdateBranch({ ref: "origin/x", branch: b("origin/x", { remote: true }) })).toBe(false); }); + it.each([true, false])("context Delete respects membership in the selection (%s)", async (inside) => { + vcsTags.mockResolvedValue({ status: "ok", data: ["v1"] }); + useVcs.setState({ + root: "/p", + repo: "/p", + branchesLoaded: true, + branches: [b("main", { current: true }), b("one"), b("two"), b("outside")], + }); + render(); + fireEvent.doubleClick(await screen.findByText("Tags")); + fireEvent.mouseDown(screen.getByText("one")); + fireEvent.mouseDown(screen.getByText("two"), { metaKey: true }); + fireEvent.mouseDown(await screen.findByText("v1"), { metaKey: true }); + fireEvent.mouseDown(screen.getByText("Local"), { metaKey: true }); + fireEvent.contextMenu(screen.getByText(inside ? "two" : "outside")); + const { useContextMenuStore, hideContextMenu } = await import("@/ui/Menu"); + const item = useContextMenuStore.getState().menu!.items.find((i) => "label" in i && i.label === "Delete"); + expect(item).toBeDefined(); + if (item && "run" in item) item.run?.(); + await waitFor(() => expect(deleteBranches).toHaveBeenCalledTimes(1)); + expect(deleteBranches.mock.calls[0]![0].map((t: { ref: string }) => t.ref)).toEqual( + inside ? ["one", "two", "v1"] : ["outside"], + ); + hideContextMenu(); + }); + it("has a vertical toolbar acting on the selected branch, and shows tags", async () => { vcsTags.mockResolvedValue({ status: "ok", data: ["v1"] }); useVcs.setState({ @@ -226,6 +259,12 @@ describe("Branches pane", () => { expect(screen.getByRole("toolbar", { name: "Branches" })).toHaveAttribute("aria-orientation", "vertical"); expect(screen.getByPlaceholderText("Branch or tag")).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Delete Branch" })).toBeDisabled(); + fireEvent.mouseDown(screen.getByText("main")); + expect(screen.getByRole("button", { name: "Delete Branch" })).toBeDisabled(); + fireEvent.contextMenu(screen.getByText("main")); + const { useContextMenuStore, hideContextMenu } = await import("@/ui/Menu"); + expect(useContextMenuStore.getState().menu!.items.some((i) => "label" in i && i.label === "Delete")).toBe(false); + hideContextMenu(); fireEvent.mouseDown(screen.getByText("feature")); expect(useBranchesPane.getState().selected.map((t) => t.ref)).toEqual(["feature"]); expect(screen.getByRole("button", { name: "Delete Branch" })).toBeEnabled(); diff --git a/src/features/vcs/branchMenu.ts b/src/features/vcs/branchMenu.ts index 1805f6440..b0f23fe94 100644 --- a/src/features/vcs/branchMenu.ts +++ b/src/features/vcs/branchMenu.ts @@ -10,6 +10,7 @@ import { checkoutDetached, checkoutInRepos, deleteBranch, + deleteBranches, deleteInRepos, deleteRemoteBranch, deleteTag, @@ -46,7 +47,13 @@ async function pullInto(remoteBranch: string, rebase: boolean) { ); } -export function branchMenu(b: Branch, current: string | null, done: () => void = () => {}): MenuItem[] { +/** The Branches pane's context menu: `selected` carries the multi-selection, which Delete acts on as a whole. */ +export function branchMenu( + b: Branch, + current: string | null, + done: () => void = () => {}, + selected: readonly { repo?: string; branch?: Branch }[] = [], +): MenuItem[] { const run = (fn: () => unknown) => () => { done(); void fn(); @@ -110,7 +117,9 @@ export function branchMenu(b: Branch, current: string | null, done: () => void = label: "Delete", danger: true, icon: "trash", - run: run(() => (b.remote ? deleteRemoteBranch(b.name) : deleteBranch(b.name))), + run: run(() => + selected.length ? deleteBranches(selected) : b.remote ? deleteRemoteBranch(b.name) : deleteBranch(b.name), + ), }, ); return items; diff --git a/src/features/vcs/branchOps.ts b/src/features/vcs/branchOps.ts index 16a5693ac..aa489ced9 100644 --- a/src/features/vcs/branchOps.ts +++ b/src/features/vcs/branchOps.ts @@ -10,7 +10,7 @@ import { showCommitRange } from "./CompareBranches"; import { confirmDialog, promptText } from "./dialogs"; import { cleanUpBranchNameOnTyping } from "@/platform/gitRefName"; import { runOp, runRemote, saveAll } from "./ops"; -import { conflictedFiles, currentRepo, refreshNow, repoName, useVcs } from "./store"; +import { conflictedFiles, currentRepo, currentRepoRoot, focusRepo, refreshNow, repoName, useVcs } from "./store"; import { tx } from "@/platform/i18n"; const recentKey = () => `openidea.vcs.recentBranches:${useVcs.getState().root ?? ""}`; @@ -309,7 +309,7 @@ export async function deleteBranch(name: string) { }, }, ...(deleted.unmerged - ? [{ label: "View Commits", run: () => showCommitRange(`Commits of ${name}`, "HEAD", deleted.tip) }] + ? [{ label: "View Commits", run: () => showCommitRange(`Commits of ${name}`, "HEAD", deleted.tip, repo) }] : []), ...(tracked && !otherTracking ? [ @@ -317,7 +317,7 @@ export async function deleteBranch(name: string) { label: "Delete Tracked Branch", run: () => { done(); - void deleteRemoteBranch(tracked); + void deleteRemoteBranch(tracked, repo); }, }, ] @@ -386,7 +386,7 @@ export async function pushTag(tag: string, remote: string) { } /** Deletes a branch on its remote (`git push --delete `); `name` is e.g. `origin/feature`. */ -export async function deleteRemoteBranch(name: string) { +export async function deleteRemoteBranch(name: string, repo = currentRepo()) { const ok = await confirmDialog({ title: "Delete Remote Branch", message: tx`Delete branch '${shortRemoteName(name)}' on the remote? The remote-tracking branch '${name}' is removed as well.`, @@ -394,7 +394,50 @@ export async function deleteRemoteBranch(name: string) { danger: true, }); if (!ok) return; - await runRemote({ op: "deleteBranch", branch: name }, name); + await runRemote({ op: "deleteBranch", branch: name }, name, [repo]); +} + +/** Delete is offered for branches other than the current one in their repository. */ +export function canDeleteBranch(t: { ref?: string; repo?: string; branch?: Branch } | null): boolean { + return !!t?.branch && !t.branch.current; +} + +async function confirmRemoteBranches(remote: readonly { branch: Branch }[]): Promise { + if (remote.length === 1) { + const name = remote[0]!.branch.name; + return confirmDialog({ + title: "Delete Remote Branch", + message: tx`Delete branch '${shortRemoteName(name)}' on the remote? The remote-tracking branch '${name}' is removed as well.`, + okLabel: "Delete", + danger: true, + }); + } + const names = remote.map((t) => shortRemoteName(t.branch.name)).join("', '"); + return confirmDialog({ + title: "Delete Remote Branches", + message: tx`Delete the ${remote.length} branches '${names}' on the remote? The remote-tracking branches are removed as well.`, + okLabel: "Delete", + danger: true, + }); +} + +/** Deletes selected branches, skipping current branches and restoring the focused repository. */ +export async function deleteBranches(targets: readonly { ref?: string; repo?: string; branch?: Branch }[]) { + const list = targets.filter((t): t is { repo?: string; branch: Branch } => canDeleteBranch(t)); + if (!list.length) return; + const previous = currentRepoRoot(); + const remote = list.filter((t) => t.branch.remote); + if (remote.length && !(await confirmRemoteBranches(remote))) return; + try { + for (const t of list) { + const repo = t.repo ?? previous; + if (repo && repo !== currentRepo()) await focusRepo(repo); + if (t.branch.remote) await runRemote({ op: "deleteBranch", branch: t.branch.name }, t.branch.name); + else await deleteBranch(t.branch.name); + } + } finally { + if (previous && previous !== currentRepo()) await focusRepo(previous); + } } /** Checkout and Rebase onto Current: checks out `name`, then rebases it onto the branch that was current. */ diff --git a/src/features/vcs/logPaneActions.ts b/src/features/vcs/logPaneActions.ts index 093fdaa49..e70aabf8c 100644 --- a/src/features/vcs/logPaneActions.ts +++ b/src/features/vcs/logPaneActions.ts @@ -7,7 +7,7 @@ import { create } from "zustand"; import type { Branch } from "@/bindings"; import { registerActions, type Action } from "@/platform/actions"; import { notifications, toolWindowService } from "@/workbench/contributions"; -import { deleteBranch, deleteRemoteBranch, newBranch, toggleFavoriteBranch, updateBranch } from "./branchOps"; +import { deleteBranches, newBranch, toggleFavoriteBranch, updateBranch } from "./branchOps"; import { changesArg, compareChangeWithLocal, @@ -38,6 +38,13 @@ function branchArg(arg: unknown): BranchTarget | null { return useBranchesPane.getState().selected[0] ?? null; } +/** The action's branches: an array argument as a whole, otherwise the single one or the pane's selection. */ +function branchTargets(arg: unknown): BranchTarget[] { + if (Array.isArray(arg)) return arg.filter((t): t is BranchTarget => !!t && typeof t === "object" && "ref" in t); + if (arg && typeof arg === "object" && "ref" in arg) return [arg as BranchTarget]; + return useBranchesPane.getState().selected; +} + const withBranch = (fn: (b: Branch) => unknown): Action["run"] => async ({ arg }) => { @@ -47,11 +54,6 @@ const withBranch = if (b) void fn(b); }; -/** Delete is offered for branches other than the current one. */ -export function canDeleteBranch(t: BranchTarget | null): boolean { - return !!t?.branch && !t.branch.current; -} - /** Update fast-forwards a local branch with an upstream (or updates the project for the current one). */ export function canUpdateBranch(t: BranchTarget | null): boolean { return !!t?.branch && !t.branch.remote && (t.branch.current || !!t.branch.upstream); @@ -117,10 +119,7 @@ export function registerLogPaneActions(): () => void { icon: "trash", when: VCS, hidden: true, - run: withBranch((b) => { - if (b.current) return; - return b.remote ? deleteRemoteBranch(b.name) : deleteBranch(b.name); - }), + run: ({ arg }) => deleteBranches(branchTargets(arg)), }, { id: "vcs.log.branches.toggleFavorite", diff --git a/src/platform/locales/text/en.json b/src/platform/locales/text/en.json index 00bbd701f..7af553708 100644 --- a/src/platform/locales/text/en.json +++ b/src/platform/locales/text/en.json @@ -1,4 +1,6 @@ { + "Delete Remote Branches": "Delete Remote Branches", + "Delete the {length} branches '{names}' on the remote? The remote-tracking branches are removed as well.": "Delete the {length} branches '{names}' on the remote? The remote-tracking branches are removed as well.", "Add or Remove Caret": "Add or Remove Caret", "Select Python Environment Folder": "Select Python Environment Folder", "Checking interpreter": "Checking interpreter", diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index 29e4477bd..1d73af218 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -763,6 +763,7 @@ "Delete Docker Object…": "删除 Docker 对象…", "Delete Keymap": "删除按键映射", "Delete Remote Branch": "删除远程分支", + "Delete Remote Branches": "删除远程分支", "Delete Rows": "删除行", "Delete Shelved Changes": "删除已搁置的更改", "Delete all rows of": "删除以下表的所有行：", @@ -4444,6 +4445,7 @@ "Create New Branch from {from}": "从 {from} 创建新分支", "Rename Branch {name}": "重命名分支 {name}", "Delete branch '{name}' on the remote? The remote-tracking branch '{tracking}' is removed as well.": "是否删除远程上的分支 '{name}'？远程跟踪分支 '{tracking}' 也将被移除。", + "Delete the {length} branches '{names}' on the remote? The remote-tracking branches are removed as well.": "是否删除远程上的 {length} 个分支 '{names}'？远程跟踪分支也将被移除。", "New branch name (in {count} repositories):": "新分支名称（在 {count} 个仓库中）：", "Delete branch '{name}' in {count} repositories? Unmerged commits are discarded.": "是否删除 {count} 个仓库中的分支 '{name}'？未合并的提交将被丢弃。", "Your local changes to the following files would be overwritten by checkout:\n{files}\n\nSmart Checkout can stash the changes, checkout and unstash them after that.": "签出将覆盖以下文件的本地更改：\n{files}\n\n智能签出可以储藏这些更改，签出后再取消储藏。",