From 9298be8798b1e213fd8fa8e8c1e5cf768ad699c6 Mon Sep 17 00:00:00 2001 From: Shohruh Ismoilov Date: Sat, 7 Nov 2026 07:36:15 +0500 Subject: [PATCH] fix(vcs): allow explicitly overwriting a local branch (#1851) The New Branch dialog refused any existing local name, so a branch could not be moved to a selected commit. When the name matches a local branch of the target repository, the dialog now offers an unchecked Overwrite existing branch option (git switch -C / git branch --force); the consent clears when the name changes, and Git's checkout checks keep the old tip and local changes if switching fails. New Branch from a Git Log commit also targets that commit's repository in a multi-repository project. Co-authored-by: shohruh-genius <7658451+shohruh-genius@users.noreply.github.com> --- crates/ide-host/src/ops/vcs/commands.rs | 16 ++++- crates/ide-host/tests/vcs_flow.rs | 2 +- crates/ide-vcs/src/lib.rs | 13 +++- crates/ide-vcs/src/write.rs | 15 ++++- crates/ide-vcs/tests/git_workflows.rs | 30 +++++++++ e2e/newBranch.spec.ts | 86 ++++++++++++++++++------- src-tauri/src/commands/vcs.rs | 3 +- src/bindings.ts | 2 +- src/features/vcs/branchOps.ts | 41 +++++++----- src/features/vcs/dialogs.tsx | 15 +++-- src/features/vcs/historyActions.tsx | 2 +- src/platform/locales/text/en.json | 2 + src/platform/locales/text/zh.json | 2 + 13 files changed, 177 insertions(+), 52 deletions(-) diff --git a/crates/ide-host/src/ops/vcs/commands.rs b/crates/ide-host/src/ops/vcs/commands.rs index 29f4d58a9..d4adf0741 100644 --- a/crates/ide-host/src/ops/vcs/commands.rs +++ b/crates/ide-host/src/ops/vcs/commands.rs @@ -583,12 +583,15 @@ pub async fn vcs_create_branch( name: String, start: Option, checkout: bool, + overwrite: Option, ) -> IdeResult<()> { host.require_trust(window)?; let project = host.project(window)?; let recorded = checkout.then_some("Git checkout"); blocking(move || { - project.with_repo_recorded(repo.as_deref(), recorded, |r| r.create_branch(&name, start.as_deref(), checkout)) + project.with_repo_recorded(repo.as_deref(), recorded, |r| { + r.create_branch_with_overwrite(&name, start.as_deref(), checkout, overwrite.unwrap_or(false)) + }) }) .await } @@ -1335,7 +1338,16 @@ pub(crate) fn register(d: &mut Dispatcher) { vcs_checkout(&c.host, &c.window, c.arg("repo")?, c.arg("branch")?, c.arg("create")?, c.arg("options")?).await }); d.add("vcs_create_branch", |c| async move { - vcs_create_branch(&c.host, &c.window, c.arg("repo")?, c.arg("name")?, c.arg("start")?, c.arg("checkout")?).await + vcs_create_branch( + &c.host, + &c.window, + c.arg("repo")?, + c.arg("name")?, + c.arg("start")?, + c.arg("checkout")?, + c.arg("overwrite")?, + ) + .await }); d.add( "vcs_undo_commit", diff --git a/crates/ide-host/tests/vcs_flow.rs b/crates/ide-host/tests/vcs_flow.rs index 031b628da..27db76065 100644 --- a/crates/ide-host/tests/vcs_flow.rs +++ b/crates/ide-host/tests/vcs_flow.rs @@ -151,7 +151,7 @@ async fn change_commit_branch_stash_and_merge_a_conflict() { assert!(ops::vcs::vcs_line_hunks(&f.host, WINDOW, lib_path.clone()).await.unwrap().is_empty()); // A branch changes the line its way. - ops::vcs::vcs_create_branch(&f.host, WINDOW, None, "feature".into(), None, true).await.unwrap(); + ops::vcs::vcs_create_branch(&f.host, WINDOW, None, "feature".into(), None, true, None).await.unwrap(); let theirs = "pub fn format_price(cents: u64) -> String { // feature"; f.edit_line(lib, theirs).await; f.save(lib).await; diff --git a/crates/ide-vcs/src/lib.rs b/crates/ide-vcs/src/lib.rs index b2be6a05f..07dd0cd89 100644 --- a/crates/ide-vcs/src/lib.rs +++ b/crates/ide-vcs/src/lib.rs @@ -1087,8 +1087,19 @@ impl Repo { /// Creates branch `name` at `start` (default HEAD), optionally switching to it. A /// remote-tracking start point (`origin/x`) becomes the new branch's upstream. pub fn create_branch(&self, name: &str, start: Option<&str>, checkout: bool) -> IdeResult<()> { + self.create_branch_with_overwrite(name, start, checkout, false) + } + + /// Creates a branch or moves its tip when overwrite is explicitly enabled. + pub fn create_branch_with_overwrite( + &self, + name: &str, + start: Option<&str>, + checkout: bool, + overwrite: bool, + ) -> IdeResult<()> { let _scope = crate::ExecutionScope::enter(self.execution_allowed.load(std::sync::atomic::Ordering::Acquire)); - write::create_branch(self, name, start, checkout) + write::create_branch(self, name, start, checkout, overwrite) } /// Undo Commit: resets the current branch to the parent of `commit` (which must be /// HEAD and not a merge) keeping its changes staged; undoing the initial commit diff --git a/crates/ide-vcs/src/write.rs b/crates/ide-vcs/src/write.rs index 1e4549744..6262afe7c 100644 --- a/crates/ide-vcs/src/write.rs +++ b/crates/ide-vcs/src/write.rs @@ -641,7 +641,13 @@ fn stash_count(r: &Repo) -> IdeResult { } /// Creates `name` at `start` (default HEAD); a remote-tracking start point is tracked. -pub(crate) fn create_branch(r: &Repo, name: &str, start: Option<&str>, checkout: bool) -> IdeResult<()> { +pub(crate) fn create_branch( + r: &Repo, + name: &str, + start: Option<&str>, + checkout: bool, + overwrite: bool, +) -> IdeResult<()> { let name = name.trim(); let valid = !name.is_empty() && !name.starts_with('-') @@ -658,7 +664,12 @@ pub(crate) fn create_branch(r: &Repo, name: &str, start: Option<&str>, checkout: let repo = r.gix()?; resolve(&repo, s)?; } - let mut args = if checkout { vec!["switch", "-c", name] } else { vec!["branch", "--", name] }; + let mut args = match (checkout, overwrite) { + (true, false) => vec!["switch", "-c", name], + (true, true) => vec!["switch", "-C", name], + (false, false) => vec!["branch", "--", name], + (false, true) => vec!["branch", "--force", "--", name], + }; args.extend(start); run(r.root(), &args, None).map(|_| ()) } diff --git a/crates/ide-vcs/tests/git_workflows.rs b/crates/ide-vcs/tests/git_workflows.rs index 4103350ec..226de37f1 100644 --- a/crates/ide-vcs/tests/git_workflows.rs +++ b/crates/ide-vcs/tests/git_workflows.rs @@ -950,6 +950,36 @@ fn branch_create_checkout_merge_rebase_delete() { assert!(!repo.branches().unwrap().iter().any(|b| b.name == "unmerged")); } +#[test] +fn branch_overwrite_preserves_the_ref_if_checkout_fails() { + let fx = Fixture::new(); + fx.write("a.txt", "base\n"); + let base = fx.commit_all("base"); + fx.write("a.txt", "main\n"); + let tip = fx.commit_all("main"); + let repo = fx.repo(); + repo.create_branch("topic", None, false).unwrap(); + + repo.create_branch_with_overwrite("topic", Some(&base), false, true).unwrap(); + assert_eq!(fx.git(&["rev-parse", "topic"]), base); + assert_eq!(fx.git(&["rev-parse", "HEAD"]), tip); + + repo.create_branch_with_overwrite("topic", Some(&tip), false, true).unwrap(); + fx.write("a.txt", "dirty\n"); + assert!(repo.create_branch_with_overwrite("topic", Some(&base), true, true).is_err()); + assert_eq!(fx.git(&["rev-parse", "topic"]), tip); + assert_eq!(repo.branch_info().unwrap().head.as_deref(), Some("main")); + assert_eq!(fx.read("a.txt"), "dirty\n"); + + fx.write("a.txt", "main\n"); + repo.create_branch_with_overwrite("topic", Some(&base), true, true).unwrap(); + assert_eq!(repo.branch_info().unwrap().head.as_deref(), Some("topic")); + assert_eq!(fx.git(&["rev-parse", "HEAD"]), base); + + repo.create_branch_with_overwrite("topic", Some(&tip), true, true).unwrap(); + assert_eq!(fx.git(&["rev-parse", "HEAD"]), tip); +} + #[test] fn new_branch_from_remote_branch_tracks_it() { let (fx, _bare) = Fixture::with_origin(); diff --git a/e2e/newBranch.spec.ts b/e2e/newBranch.spec.ts index f07067393..acaf0e1f6 100644 --- a/e2e/newBranch.spec.ts +++ b/e2e/newBranch.spec.ts @@ -7,30 +7,47 @@ import { expect, ipcCalls, stubDesktop, test } from "./fixtures"; const ROOT = "/dev/acme-api"; -async function openNewBranchDialog(page: Page) { +async function openNewBranchDialog(page: Page, from?: string) { await stubDesktop(page); - await page.evaluate(async (root) => { - const h = window.__ipc.handlers; - h.vcs_roots = () => [{ root, operation: null, head: "main", headCommit: "1111111" }]; - h.vcs_branch_info = () => ({ - head: "main", - headCommit: "1111111", - upstream: null, - ahead: 0, - behind: 0, - operation: null, - }); - h.vcs_status = () => []; - h.vcs_create_branch = () => null; - const vcsModule = "/src/features/vcs/store.ts"; - const { useVcs, refreshNow } = await import(/* @vite-ignore */ vcsModule); - useVcs.setState({ available: true, root, repo: root, branchesLoaded: false, stashesLoaded: false }); - await refreshNow(); - const actionsModule = "/src/platform/actions.ts"; - const { executeAction } = await import(/* @vite-ignore */ actionsModule); - void executeAction("vcs.newBranch"); - }, ROOT); - const dialog = page.getByRole("dialog", { name: "Create New Branch" }); + await page.evaluate( + async ({ root, from }) => { + const h = window.__ipc.handlers; + h.vcs_roots = () => [{ root, operation: null, head: "main", headCommit: "1111111" }]; + h.vcs_branch_info = () => ({ + head: "main", + headCommit: "1111111", + upstream: null, + ahead: 0, + behind: 0, + operation: null, + }); + h.vcs_status = () => []; + h.vcs_branches = () => [ + { + name: "main", + current: true, + remote: false, + upstream: null, + ahead: 0, + behind: 0, + lastCommitMs: null, + commit: "1111111", + }, + ]; + h.vcs_create_branch = () => null; + const vcsModule = "/src/features/vcs/store.ts"; + const { useVcs, refreshNow, loadBranches } = await import(/* @vite-ignore */ vcsModule); + useVcs.setState({ available: true, root, repo: root, branchesLoaded: false, stashesLoaded: false }); + await refreshNow(); + await loadBranches(); + const actionsModule = "/src/platform/actions.ts"; + const { executeAction } = await import(/* @vite-ignore */ actionsModule); + if (from) void executeAction("vcs.log.newBranch", { id: from, summary: "Selected commit", repo: root }); + else void executeAction("vcs.newBranch"); + }, + { root: ROOT, from }, + ); + const dialog = page.getByRole("dialog", { name: from ? `Create New Branch from ${from}` : "Create New Branch" }); const input = dialog.getByRole("textbox", { name: "New branch name:" }); await expect(input).toBeVisible(); return { dialog, input }; @@ -48,6 +65,29 @@ test("a typed space becomes a separator and Create stays enabled", async ({ work expect(call).toMatchObject({ name: "fix-login-redirect" }); }); +test("overwrites a local branch only after explicitly opting in", async ({ workbench: page }) => { + const from = "2".repeat(40); + const { dialog, input } = await openNewBranchDialog(page, from); + await input.fill("main"); + const create = dialog.getByRole("button", { name: "Create", exact: true }); + await expect(create).toBeDisabled(); + const overwrite = dialog.getByRole("checkbox", { name: "Overwrite existing branch" }); + await expect(overwrite).toBeVisible(); + await expect(overwrite).not.toBeChecked(); + await overwrite.check(); + await expect(create).toBeEnabled(); + await input.fill("new-branch"); + await expect(overwrite).toHaveCount(0); + await input.fill("main"); + await expect(overwrite).not.toBeChecked(); + await expect(create).toBeDisabled(); + await overwrite.check(); + await create.click(); + await expect + .poll(() => ipcCalls(page, "vcs_create_branch")) + .toEqual([{ repo: ROOT, name: "main", start: from, checkout: true, overwrite: true }]); +}); + test("a space typed in the middle keeps the caret after it", async ({ workbench: page }) => { const { input } = await openNewBranchDialog(page); await input.fill("ab"); diff --git a/src-tauri/src/commands/vcs.rs b/src-tauri/src/commands/vcs.rs index 29cc34b92..cef9f7eb7 100644 --- a/src-tauri/src/commands/vcs.rs +++ b/src-tauri/src/commands/vcs.rs @@ -268,8 +268,9 @@ pub async fn vcs_create_branch( name: String, start: Option, checkout: bool, + overwrite: Option, ) -> IdeResult<()> { - ide_host::ops::vcs::vcs_create_branch(&state.host, window.label(), repo, name, start, checkout).await + ide_host::ops::vcs::vcs_create_branch(&state.host, window.label(), repo, name, start, checkout, overwrite).await } /// Undo Commit: moves HEAD back to the commit's parent, keeping its changes staged. diff --git a/src/bindings.ts b/src/bindings.ts index 6e0904ec4..5ff5d418f 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -534,7 +534,7 @@ export const commands = { /** "Restore" of the tag-deleted notification. */ vcsRestoreTag: (repo: string | null, deleted: DeletedTag) => typedError(__TAURI_INVOKE("vcs_restore_tag", { repo, deleted })), /** New Branch: creates `name` at `start` (HEAD when absent), optionally checking it out. */ - vcsCreateBranch: (repo: string | null, name: string, start: string | null, checkout: boolean) => typedError(__TAURI_INVOKE("vcs_create_branch", { repo, name, start, checkout })), + vcsCreateBranch: (repo: string | null, name: string, start: string | null, checkout: boolean, overwrite: boolean | null) => typedError(__TAURI_INVOKE("vcs_create_branch", { repo, name, start, checkout, overwrite })), /** * Undo Commit: moves HEAD back to the commit's parent, keeping its changes staged. * Returns the undone commit's message. diff --git a/src/features/vcs/branchOps.ts b/src/features/vcs/branchOps.ts index d2f6c0dfa..16a5693ac 100644 --- a/src/features/vcs/branchOps.ts +++ b/src/features/vcs/branchOps.ts @@ -35,15 +35,18 @@ function rememberBranch(name: string | null | undefined) { } /** Valid-enough git ref name check (mirrors `git check-ref-format` basics). */ -export function validateBranchName(name: string): string | null { +export function validateBranchName( + name: string, + overwrite = false, + branches: readonly Branch[] = useVcs.getState().branches, +): string | null { const n = name.trim(); if (!n) return "Branch name must not be empty"; if (/[\s~^:?*[\\\p{Cc}]/u.test(n)) return "Branch name contains invalid characters"; if (n.startsWith("-") || n.startsWith("/") || n.endsWith("/") || n.endsWith(".") || n.endsWith(".lock")) return "Invalid branch name"; if (n.includes("..") || n.includes("//") || n.includes("@{") || n === "@") return "Invalid branch name"; - const exists = useVcs.getState().branches.some((b) => !b.remote && b.name === n); - if (exists) return `Branch '${n}' already exists`; + if (!overwrite && branches.some((b) => !b.remote && b.name === n)) return tx`Branch '${n}' already exists`; return null; } @@ -154,16 +157,18 @@ export async function checkoutBranch(name: string) { } /** New Branch: creates a branch from HEAD (or `from`) and, by default, checks it out. */ -export async function newBranch(from?: string) { - const repo = currentRepo(); +export async function newBranch(from?: string, repo = currentRepo()) { + const state = useVcs.getState(); + const selected = repo === state.repo; + const previous = selected ? state.branch?.head : repo ? state.heads[repo]?.head : null; + const branches = + selected && state.branchesLoaded + ? state.branches + : await call(commands.vcsBranches(repo)).catch(() => (selected ? state.branches : [])); // IDEA opens the dialog with a suggestion instead of an empty field: a // remote's name without the prefix, the source ref as `-copy`, or the // current branch for a plain New Branch from HEAD. - const suggestion = from - ? from.includes("/") - ? shortRemoteName(from) - : `${from}-copy` - : (useVcs.getState().branch?.head ?? ""); + const suggestion = from ? (from.includes("/") ? shortRemoteName(from) : `${from}-copy`) : (previous ?? ""); const r = await promptText({ title: from ? tx`Create New Branch from ${from}` : "Create New Branch", label: "New branch name:", @@ -172,15 +177,15 @@ export async function newBranch(from?: string) { mono: true, okLabel: "Create", sanitize: cleanUpBranchNameOnTyping, - validate: validateBranchName, + validate: (name, overwrite) => validateBranchName(name, overwrite, branches), + overwriteBranch: (name) => branches.some((b) => !b.remote && b.name === name.trim()), validateInitial: true, checkbox: { label: "Checkout branch", initial: true }, }); if (!r) return; const checkout = r.checked; - const previous = useVcs.getState().branch?.head; if (checkout) await saveAll(); - const create = () => call(commands.vcsCreateBranch(repo, r.value, from ?? null, checkout)); + const create = () => call(commands.vcsCreateBranch(repo, r.value, from ?? null, checkout, r.overwrite ?? false)); const ok = await runOp(tx`Create branch ${r.value}`, create, { success: checkout ? `Created and checked out ${r.value}` : `Created branch ${r.value}`, }); @@ -525,9 +530,13 @@ export async function newBranchInRepos(roots: readonly string[], from?: string) await eachRepo( roots, (root) => - runOp(tx`Create branch ${name}`, () => call(commands.vcsCreateBranch(root, name, from ?? null, r.checked)), { - refresh: false, - }).then((done) => (done === undefined ? undefined : true)), + runOp( + tx`Create branch ${name}`, + () => call(commands.vcsCreateBranch(root, name, from ?? null, r.checked, false)), + { + refresh: false, + }, + ).then((done) => (done === undefined ? undefined : true)), r.checked ? `Created and checked out ${name}` : `Created branch ${name}`, { undo: tx`delete branch ${name}`, diff --git a/src/features/vcs/dialogs.tsx b/src/features/vcs/dialogs.tsx index 82bfefd18..b2089679f 100644 --- a/src/features/vcs/dialogs.tsx +++ b/src/features/vcs/dialogs.tsx @@ -42,18 +42,20 @@ export interface PromptOptions { okLabel?: string; mono?: boolean; /** Returns an error message, or null when valid. */ - validate?: (value: string) => string | null; + validate?: (value: string, overwrite: boolean) => string | null; /** Rewrites the field as the user types or pastes (e.g. spaces in a branch name). */ sanitize?: (value: string) => string; /** Show the prefilled value's problem (e.g. the branch exists) before it is edited. */ validateInitial?: boolean; /** Optional checkbox below the field. */ checkbox?: { label: string; initial: boolean }; + overwriteBranch?: (value: string) => boolean; } export interface PromptResult { value: string; checked: boolean; + overwrite?: boolean; } export function promptText(opts: PromptOptions): Promise { @@ -75,11 +77,13 @@ function PromptDialog(props: PromptOptions & { onDone: (r: PromptResult | null) const { ref, clean } = useSanitizedField(sanitize); const [value, setValue] = useState(initial); const [checked, setChecked] = useState(checkbox?.initial ?? false); + const [overwrite, setOverwrite] = useState(false); const [touched, setTouched] = useState(!!props.validateInitial && initial !== ""); - const error = validate ? validate(value) : value.trim() ? null : "Value must not be empty"; + const canOverwrite = props.overwriteBranch?.(value) ?? false; + const error = validate ? validate(value, canOverwrite && overwrite) : value.trim() ? null : "Value must not be empty"; const submit = () => { setTouched(true); - if (!error) onDone({ value: value.trim(), checked }); + if (!error) onDone({ value: value.trim(), checked, ...(canOverwrite ? { overwrite } : {}) }); }; return ( { setTouched(true); - setValue(clean(e)); + const next = clean(e); + if (next.trim() !== value.trim()) setOverwrite(false); + setValue(next); }} /> {touched && error && value &&
{error}
} {checkbox && } + {canOverwrite && } ); diff --git a/src/features/vcs/historyActions.tsx b/src/features/vcs/historyActions.tsx index 4becc6a35..96091d9ad 100644 --- a/src/features/vcs/historyActions.tsx +++ b/src/features/vcs/historyActions.tsx @@ -445,7 +445,7 @@ export function registerHistoryActions(): () => void { category: "Git Log", icon: "branch", when: VCS, - run: withCommit((c) => newBranch(c.id)), + run: withCommit((c) => newBranch(c.id, commitRepo(c) ?? currentRepo())), }, { id: "vcs.log.newTag", diff --git a/src/platform/locales/text/en.json b/src/platform/locales/text/en.json index 113684bc4..3963bbfe7 100644 --- a/src/platform/locales/text/en.json +++ b/src/platform/locales/text/en.json @@ -1,4 +1,6 @@ { + "Overwrite existing branch": "Overwrite existing branch", + "Branch '{n}' already exists": "Branch '{n}' already exists", "Cannot copy reference": "Cannot copy reference", "Decompiled assembly (read-only).": "Decompiled assembly (read-only).", "Connect through:": "Connect through:", diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index c70435fb7..d0097a991 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -448,6 +448,8 @@ "Checkout Tag or Revision": "签出标签或修订", "Checkout Tag or Revision…": "签出标签或修订…", "Checkout branch": "签出分支", + "Overwrite existing branch": "覆盖现有分支", + "Branch '{n}' already exists": "分支 '{n}' 已存在", "Checks out the selected pull request, a number given as argument, or asks for one.": "签出所选拉取请求或参数指定编号的拉取请求，否则询问编号。", "Cherry-Pick": "拣选", "Children": "子项",