fix(vcs): delete every selected branch, not only the right-clicked one (#2108)

In the Git Log Branches pane, Delete removed only the right-clicked branch and ignored the multi-selection: the menu was built from the clicked node alone and the action read only the first selected ref.

Delete now acts on the whole selection when the right-clicked branch is part of it. Current branches are skipped, each repository is focused for its branches and the original focus is restored afterwards, and all remote branches share one confirmation.

Siblings covered: the toolbar button, and the View Commits and Delete Tracked Branch notification actions, which now stay on the repository the branch was deleted from.

Co-authored-by: muutot <[email protected]>
Цей коміт міститься в:
Muuтаmuutot authored and GitHub committed 2026-11-08 20:45:49 +08:00
1 джерело 49a113c875
коміт 278936dd7d
8 files changed
+303 -25

No files matched your search

+20 -7
Переглянути файл
@@ -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))}
/>
<IconButton
icon="locate"
@@ -574,7 +574,7 @@ export function BranchesPanel() {
const sel = selected.has(n.key) ? targets.filter((r) => 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);
+171
Переглянути файл
@@ -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<typeof import("../dialogs")>()),
confirmDialog: mocks.confirm,
}));
vi.mock("../ops", async (original) => ({
...(await original<typeof import("../ops")>()),
runOp: mocks.local,
runRemote: mocks.remote,
}));
vi.mock("../store", async (original) => ({
...(await original<typeof import("../store")>()),
focusRepo: mocks.focus,
}));
vi.mock("@/workbench/contributions", async (original) => ({
...(await original<typeof import("@/workbench/contributions")>()),
notifications: () => ({ notify: mocks.notify }),
}));
vi.mock("../CompareBranches", async (original) => ({
...(await original<typeof import("../CompareBranches")>()),
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> = {}): Branch => ({
name,
remote: false,
current: false,
upstream: null,
ahead: 0,
behind: 0,
lastCommitMs: null,
...opts,
});
const target = (name: string, repo = "/p/a", opts: Partial<Branch> = {}) => ({
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");
});
+40 -1
Переглянути файл
@@ -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<typeof import("../branchOps")>()),
deleteBranches,
}));
vi.mock("@/platform/ipc", async (importOriginal) => {
const real = await importOriginal<typeof import("@/platform/ipc")>();
@@ -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(<BranchesPanel />);
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();
+11 -2
Переглянути файл
@@ -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;
+48 -5
Переглянути файл
@@ -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 <remote> --delete <name>`); `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<boolean> {
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. */
+9 -10
Переглянути файл
@@ -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",
+2
Переглянути файл
@@ -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",
+2
Переглянути файл
@@ -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智能签出可以储藏这些更改,签出后再取消储藏。",