From d54caca451bb862440d33463f316e35042fbdaf7 Mon Sep 17 00:00:00 2001 From: Lars Jansen Date: Mon, 2 Nov 2026 11:07:36 +0100 Subject: [PATCH 1/3] feat: ask before a hard reset, deleting a branch with unique commits, a force push or dropping a stash --- README.md | 7 +-- ROADMAP.md | 29 ++++++----- crates/app/src/app.rs | 11 ++++ crates/app/src/graph.rs | 17 ++++-- crates/app/src/sidebar.rs | 16 +++--- crates/app/src/tests.rs | 105 ++++++++++++++++++++++++++++++++++++++ crates/app/src/ui.rs | 18 ++++--- crates/core/src/git.rs | 13 ++++- crates/core/tests/repo.rs | 43 ++++++++++++++++ 9 files changed, 225 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 9846e8a..2646b66 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,6 @@ benchmarks against the main branch, and one that makes GitCraft slower does not ## What works GitCraft is young. This is what it does today; [ROADMAP.md](ROADMAP.md) has what comes next. -Being an early version, it does not yet ask for confirmation before a hard reset, so take care -with that one. - **Start**: open a folder, clone from a URL with git's progress as it goes, or create a new repository. @@ -52,8 +50,11 @@ with that one. - **Discard**: throw away the changes to a file, a hunk or single lines, after you confirm. - **Branches and tags**: create, check out, merge, rebase, delete, from the sidebar or a right click. - **Commits from the graph**: check out, cherry-pick, revert, reset (soft, mixed, hard), copy the SHA. -- **Remotes**: fetch, pull and push, with how far ahead and behind you are. +- **Remotes**: fetch, pull and push, with how far ahead and behind you are. A branch that went its + own way, after an amend or a rebase, is force pushed with a lease, so nobody else's work is lost. - **Stashes**: stash, apply, pop and drop. +- **Safe by default**: a hard reset, deleting a branch with commits no other branch or tag has, a + force push and dropping a stash all ask first, saying what will be lost. GitCraft runs your own `git` for everything that changes a repository, so your config, hooks, credentials and commit signing work exactly as they do on the command line. diff --git a/ROADMAP.md b/ROADMAP.md index 011bfe3..3d69b20 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -23,26 +23,27 @@ drive it the way a user does, and keeps every benchmark as fast as before. - [x] Tabs: several repositories open at once, remembered between runs - [x] Live refresh: watch the working tree and refs, and reload by itself - [x] Discard changes per file, hunk or line, after a confirmation +- [x] Confirm destructive actions: hard reset, deleting a branch with commits no other branch or tag + has, force push (with lease, for a branch that went its own way), dropping a stash ## Next -1. **Confirm destructive actions**: hard reset, deleting a branch with unmerged commits, force push. -2. **Undo and redo** of the last action, through the reflog. -3. **Conflicts**: list conflicted files, resolve them in a three-way view (ours, theirs, result), +1. **Undo and redo** of the last action, through the reflog. +2. **Conflicts**: list conflicted files, resolve them in a three-way view (ours, theirs, result), mark them resolved, and continue or abort a merge, rebase or cherry-pick. -4. **Interactive rebase**: reorder, squash, fix up, reword and drop commits, by dragging in the +3. **Interactive rebase**: reorder, squash, fix up, reword and drop commits, by dragging in the graph. -5. **Credentials**: ask for a username, password or passphrase when git needs one. -6. **Push and pull options**: force with lease, push tags, pick the remote, pull with rebase. -7. **Remotes**: add, edit and remove. -8. **Search the graph** by message, author or SHA, and jump to any branch or tag. -9. **File history and blame.** -10. **Better diffs**: side by side, word-level changes, syntax colouring, images. -11. **Keyboard**: shortcuts for everything, and a command palette. -12. **Settings**: light and dark theme, font size, date format, default pull mode. -13. **Graph polish**: resizable columns, compact lanes, author avatars (optional, never by default +4. **Credentials**: ask for a username, password or passphrase when git needs one. +5. **Push and pull options**: force push on request, push tags, pick the remote, pull with rebase. +6. **Remotes**: add, edit and remove. +7. **Search the graph** by message, author or SHA, and jump to any branch or tag. +8. **File history and blame.** +9. **Better diffs**: side by side, word-level changes, syntax colouring, images. +10. **Keyboard**: shortcuts for everything, and a command palette. +11. **Settings**: light and dark theme, font size, date format, default pull mode. +12. **Graph polish**: resizable columns, compact lanes, author avatars (optional, never by default over the network). -14. **Packaging**: installers with the icon for Windows (MSI), macOS (.app in a .dmg) and Linux +13. **Packaging**: installers with the icon for Windows (MSI), macOS (.app in a .dmg) and Linux (AppImage, .deb, Flatpak), signed where the platform expects it. ## After 1.0 diff --git a/crates/app/src/app.rs b/crates/app/src/app.rs index 5ae9395..dc7b814 100644 --- a/crates/app/src/app.rs +++ b/crates/app/src/app.rs @@ -40,6 +40,8 @@ pub enum Action { Prompt(Prompt), /// Ask whether to go ahead first. Confirm(Confirm), + /// Delete a branch, asking first when no other branch or tag has all of its commits. + DeleteBranch(String), /// Open a repository in a tab of its own, or show its tab if it is open. OpenRepo(PathBuf), /// Show a tab, or the start screen for `None`. @@ -268,6 +270,15 @@ impl Workspace { } Action::Prompt(prompt) => self.prompt = Some((prompt, String::new())), Action::Confirm(confirm) => self.confirm = Some(confirm), + Action::DeleteBranch(name) => match self.repo.git.only_on(&name) { + Ok(0) => self.start(ctx, Some(Op::DeleteBranch(name))), + Ok(lost) => { + let lost = crate::theme::count(lost, "commit"); + let question = format!("Delete {name} and the {lost} on no other branch or tag?"); + self.confirm = Some(Confirm { question, verb: "Delete", op: Op::DeleteBranch(name) }); + } + Err(error) => self.error = Some(error.to_string()), + }, Action::OpenRepo(_) | Action::ShowTab(_) | Action::CloseTab(_) => unreachable!("the app handles tabs"), } } diff --git a/crates/app/src/graph.rs b/crates/app/src/graph.rs index 22fede4..2871105 100644 --- a/crates/app/src/graph.rs +++ b/crates/app/src/graph.rs @@ -6,7 +6,7 @@ use gitcraft_core::git::{Op, ResetMode}; use gitcraft_core::graph::{Half, Row}; use gitcraft_core::repo::{Label as Ref, RefKind}; -use crate::app::{Action, Prompt, Selection, Workspace}; +use crate::app::{Action, Confirm, Prompt, Selection, Workspace}; use crate::theme; /// The height of a row. @@ -209,7 +209,18 @@ pub fn commit_menu(ui: &mut Ui, sha: &str, labels: &[Ref], current: Option<&str> ("Mixed: keep working copy", ResetMode::Mixed), ("Hard: discard all changes", ResetMode::Hard), ] { - item(ui, text, Action::Run(Op::Reset { to: sha.to_owned(), mode })); + let op = Op::Reset { to: sha.to_owned(), mode }; + let action = if mode == ResetMode::Hard { + let question = format!( + "Hard reset {} to {}, throwing away every uncommitted change?", + current.unwrap_or("HEAD"), + &sha[..7] + ); + Action::Confirm(Confirm { question, verb: "Reset", op }) + } else { + Action::Run(op) + }; + item(ui, text, action); } }); if ui.button("Copy commit SHA").clicked() { @@ -232,7 +243,7 @@ pub fn ref_items(ui: &mut Ui, label: &Ref, current: Option<&str>, item: &mut imp item(ui, &format!("Merge {name} into {into}"), Action::Run(Op::Merge(name.clone()))); item(ui, &format!("Rebase {into} onto {name}"), Action::Run(Op::Rebase(name.clone()))); if kind == RefKind::Local { - item(ui, &format!("Delete {name}"), Action::Run(Op::DeleteBranch(name.clone()))); + item(ui, &format!("Delete {name}"), Action::DeleteBranch(name.clone())); } } } diff --git a/crates/app/src/sidebar.rs b/crates/app/src/sidebar.rs index 9403356..3f829e8 100644 --- a/crates/app/src/sidebar.rs +++ b/crates/app/src/sidebar.rs @@ -4,7 +4,7 @@ use eframe::egui::{self, Button, Ui}; use gitcraft_core::git::Op; use gitcraft_core::repo::{Label, RefKind}; -use crate::app::{Action, Selection, Workspace}; +use crate::app::{Action, Confirm, Selection, Workspace}; use crate::graph::ref_items; /// Shows the sidebar. @@ -53,13 +53,17 @@ pub fn show(ui: &mut Ui, ws: &Workspace, actions: &mut Vec) { |ui| { for (n, message) in ws.snapshot.stashes.iter().enumerate() { ui.add(Button::selectable(false, message).truncate()).context_menu(|ui| { - for (text, op) in [ - ("Apply stash", Op::StashApply(n)), - ("Pop stash", Op::StashPop(n)), - ("Delete stash", Op::StashDrop(n)), + let question = format!("Delete the stash \"{message}\"?"); + for (text, action) in [ + ("Apply stash", Action::Run(Op::StashApply(n))), + ("Pop stash", Action::Run(Op::StashPop(n))), + ( + "Delete stash", + Action::Confirm(Confirm { question, verb: "Delete", op: Op::StashDrop(n) }), + ), ] { if ui.button(text).clicked() { - actions.push(Action::Run(op)); + actions.push(action); ui.close(); } } diff --git a/crates/app/src/tests.rs b/crates/app/src/tests.rs index f75c9f5..57b8842 100644 --- a/crates/app/src/tests.rs +++ b/crates/app/src/tests.rs @@ -446,3 +446,108 @@ fn tabs_open_again_on_the_next_run() { let recent: Vec<_> = harness.state().recent.iter().map(|path| fs::canonicalize(path).unwrap()).collect(); assert_eq!(recent, [a.dir(), b.dir()]); } + +/// Picks `path` from the right-click menu of the widget labelled `on`, one item per step. +fn menu(harness: &mut Harness<'_, App>, on: &str, path: &[&str]) { + harness.get_by_label(on).click_secondary(); + harness.run(); + for item in path { + harness.get_by_label(item).click(); + settle(harness); + } +} + +fn rev(git: &Git, rev: &str) -> String { + git.run(&["rev-parse", rev]).unwrap() +} + +#[test] +fn a_hard_reset_asks_first() { + let (_dir, git) = repo(); + let first = rev(&git, "HEAD"); + commit(&git, "Second commit"); + fs::write(git.dir().join("file.txt"), "unsaved work").unwrap(); + let mut harness = app(Some(git.dir())); + let reset = ["Reset current branch to this commit", "Hard: discard all changes"]; + + menu(&mut harness, "First commit", &reset); + harness.get_by_label(&format!("Hard reset main to {}, throwing away every uncommitted change?", &first[..7])); + harness.get_by_label("Cancel").click(); + settle(&mut harness); + assert_eq!(read(&git, "file.txt"), "unsaved work"); + + menu(&mut harness, "First commit", &reset); + harness.get_by_label("Reset").click(); + settle(&mut harness); + assert_eq!(log(&git), "First commit\n"); + assert_eq!(read(&git, "file.txt"), "First commit"); +} + +#[test] +fn deleting_a_branch_asks_only_when_that_loses_commits() { + let (_dir, git) = repo(); + for name in ["merged", "topic"] { + git.run(&["branch", name]).unwrap(); + } + git.apply(&Op::Checkout("topic".into())).unwrap(); + commit(&git, "Work on topic"); + git.apply(&Op::Checkout("main".into())).unwrap(); + let mut harness = app(Some(git.dir())); + + menu(&mut harness, "merged", &["Delete merged"]); + assert_eq!(git.run(&["branch", "--list", "merged"]).unwrap(), ""); + assert!(harness.query_by_label("Delete").is_none(), "nothing is lost, so nothing is asked"); + + menu(&mut harness, "topic", &["Delete topic"]); + harness.get_by_label("Delete topic and the 1 commit on no other branch or tag?"); + harness.get_by_label("Cancel").click(); + settle(&mut harness); + assert_ne!(git.run(&["branch", "--list", "topic"]).unwrap(), ""); + + menu(&mut harness, "topic", &["Delete topic"]); + harness.get_by_label("Delete").click(); + settle(&mut harness); + assert_eq!(git.run(&["branch", "--list", "topic"]).unwrap(), ""); + assert!(harness.query_by_label("Work on topic").is_none()); +} + +#[test] +fn a_branch_that_went_its_own_way_is_force_pushed_once_confirmed() { + let (_dir, git) = repo(); + let remote = tempfile::tempdir().unwrap(); + Git::new(remote.path()).run(&["init", "-q", "--bare", "-b", "main"]).unwrap(); + git.run(&["remote", "add", "origin", remote.path().to_str().unwrap()]).unwrap(); + git.apply(&Op::Push { upstream: Some("origin".into()), force: false }).unwrap(); + git.apply(&Op::Commit { message: "Amended".into(), amend: true }).unwrap(); + let pushed = || Git::new(remote.path()).run(&["rev-parse", "main"]).unwrap(); + let before = pushed(); + let mut harness = app(Some(git.dir())); + + harness.get_by_label("Push 1").click(); + settle(&mut harness); + harness.get_by_label("Force push main, throwing away the 1 commit on origin/main that it does not have?"); + harness.get_by_label("Cancel").click(); + settle(&mut harness); + assert_eq!(pushed(), before); + + harness.get_by_label("Push 1").click(); + settle(&mut harness); + harness.get_by_label("Force push").click(); + settle(&mut harness); + assert_eq!(pushed(), rev(&git, "HEAD")); + assert_eq!(harness.state().workspace().unwrap().error, None); +} + +#[test] +fn deleting_a_stash_asks_first() { + let (_dir, git) = repo(); + fs::write(git.dir().join("file.txt"), "stashed").unwrap(); + git.apply(&Op::Stash("wip".into())).unwrap(); + let mut harness = app(Some(git.dir())); + + menu(&mut harness, "On main: wip", &["Delete stash"]); + harness.get_by_label("Delete the stash \"On main: wip\"?"); + harness.get_by_label("Delete").click(); + settle(&mut harness); + assert_eq!(git.run(&["stash", "list"]).unwrap(), ""); +} diff --git a/crates/app/src/ui.rs b/crates/app/src/ui.rs index c9c8a88..0725f17 100644 --- a/crates/app/src/ui.rs +++ b/crates/app/src/ui.rs @@ -6,7 +6,7 @@ use eframe::egui::{self, Button, Key, Role, Ui, WidgetInfo}; use gitcraft_core::git::Op; use gitcraft_core::repo::{RefKind, Repo}; -use crate::app::{Action, Selection, Workspace}; +use crate::app::{Action, Confirm, Selection, Workspace}; use crate::start::Start; use crate::{details, diff, graph, sidebar, theme}; @@ -208,14 +208,20 @@ fn toolbar(ui: &mut Ui, ws: &Workspace, actions: &mut Vec) { Some(Selection::Commit(id)) => id.to_string(), _ => "HEAD".to_owned(), }; + let push = match (&status.branch, &status.upstream) { + // A branch that went its own way since the last push only gets there by force. + (Some(branch), Some(upstream)) if status.ahead > 0 && status.behind > 0 => { + let lost = theme::count(status.behind as usize, "commit"); + let question = + format!("Force push {branch}, throwing away the {lost} on {upstream} that it does not have?"); + Action::Confirm(Confirm { question, verb: "Force push", op: Op::Push { upstream: None, force: true } }) + } + _ => Action::Run(Op::Push { upstream: status.upstream.is_none().then(|| remote.to_owned()), force: false }), + }; let buttons = [ ("Fetch", true, Action::Run(Op::Fetch)), (&count("Pull", status.behind), true, Action::Run(Op::Pull { rebase: false })), - ( - &count("Push", status.ahead), - status.branch.is_some(), - Action::Run(Op::Push { upstream: status.upstream.is_none().then(|| remote.to_owned()), force: false }), - ), + (&count("Push", status.ahead), status.branch.is_some(), push), ("Branch", true, Action::Prompt(crate::app::Prompt::Branch { at })), ( "Stash", diff --git a/crates/core/src/git.rs b/crates/core/src/git.rs index 47dc7aa..5c9c22f 100644 --- a/crates/core/src/git.rs +++ b/crates/core/src/git.rs @@ -34,6 +34,15 @@ impl Git { self.run_with(&op.args(), op.input(), &[0]) } + /// How many commits of `branch` no other branch, remote branch or tag has: deleting it loses + /// them. + pub fn only_on(&self, branch: &str) -> Result { + // Branch names cannot hold glob characters, so `--exclude` leaves out this branch alone. + let (tip, others) = (format!("refs/heads/{branch}"), format!("--exclude={branch}")); + let count = self.run(&["rev-list", "--count", &tip, "--not", &others, "--branches", "--tags", "--remotes"])?; + count.trim().parse().map_err(|_| Error::Parse(count)) + } + /// Runs `git` with `input` on standard input, accepting the exit codes in `ok`. pub(crate) fn run_with(&self, args: &[impl AsRef], input: Option<&[u8]>, ok: &[i32]) -> Result { let output = self.output(args, input)?; @@ -185,7 +194,7 @@ pub enum Op { /// Where it starts. at: String, }, - /// Delete a merged branch. + /// Delete a branch, also when that loses commits (see [`Git::only_on`]). DeleteBranch(String), /// Merge into the current branch. Merge(String), @@ -256,7 +265,7 @@ impl Op { Self::Commit { amend, .. } => (&["commit", "-q", "--cleanup=strip", "-F", "-"], flag(*amend, "--amend")), Self::Checkout(target) => (&["checkout", "-q"], vec![target]), Self::Branch { name, at } => (&["checkout", "-q", "-b"], vec![name, at]), - Self::DeleteBranch(name) => (&["branch", "-d"], vec![name]), + Self::DeleteBranch(name) => (&["branch", "-D"], vec![name]), Self::Merge(target) => (&["merge", "--no-edit"], vec![target]), Self::Rebase(onto) => (&["rebase"], vec![onto]), Self::CherryPick(commit) => (&["cherry-pick"], vec![commit]), diff --git a/crates/core/tests/repo.rs b/crates/core/tests/repo.rs index e8ca645..5a7fcf0 100644 --- a/crates/core/tests/repo.rs +++ b/crates/core/tests/repo.rs @@ -226,6 +226,34 @@ fn commit_amend_branch_and_checkout() { assert_eq!(Status::read(&git).unwrap().branch, None); } +#[test] +fn a_branch_counts_the_commits_no_other_ref_has() { + let (_dir, git) = repo(); + commit(&git, "a.txt", "a\n", "Root"); + git.apply(&Op::Branch { name: "topic".into(), at: "HEAD".into() }).unwrap(); + commit(&git, "t.txt", "1\n", "One"); + commit(&git, "t.txt", "2\n", "Two"); + git.apply(&Op::Checkout("main".into())).unwrap(); + assert_eq!(git.only_on("main").unwrap(), 0); + assert_eq!(git.only_on("topic").unwrap(), 2); + + // A tag, a remote branch or another branch keeps them, also one whose name starts the same. + git.apply(&Op::Tag { name: "v1".into(), at: "topic~1".into() }).unwrap(); + assert_eq!(git.only_on("topic").unwrap(), 1); + git.run(&["update-ref", "refs/remotes/origin/topic", "topic"]).unwrap(); + assert_eq!(git.only_on("topic").unwrap(), 0); + git.run(&["update-ref", "-d", "refs/remotes/origin/topic"]).unwrap(); + git.apply(&Op::Branch { name: "topic-2".into(), at: "topic".into() }).unwrap(); + assert_eq!(git.only_on("topic").unwrap(), 0); + git.apply(&Op::Checkout("main".into())).unwrap(); + git.apply(&Op::DeleteBranch("topic-2".into())).unwrap(); + + // Deleting goes ahead anyway: the app asks first. + git.apply(&Op::DeleteBranch("topic".into())).unwrap(); + assert_eq!(git.run(&["branch", "--list", "topic"]).unwrap(), ""); + assert!(git.only_on("topic").is_err()); +} + #[test] fn rebase_cherry_pick_revert_and_reset() { let (_dir, git) = repo(); @@ -306,6 +334,21 @@ fn push_fetch_and_pull_through_a_remote() { let status = Status::read(&git).unwrap(); assert_eq!((status.ahead, status.behind), (0, 0)); assert!(git.dir().join("b.txt").exists()); + + // After an amend the branch went its own way: only a force push gets it there. + git.apply(&Op::Commit { message: "Amended".into(), amend: true }).unwrap(); + let status = Status::read(&git).unwrap(); + assert_eq!((status.ahead, status.behind), (1, 1)); + assert!(git.apply(&Op::Push { upstream: None, force: false }).is_err()); + git.apply(&Op::Push { upstream: None, force: true }).unwrap(); + assert_eq!(Git::new(remote.path()).run(&["rev-parse", "main"]).unwrap(), git.run(&["rev-parse", "HEAD"]).unwrap()); + + // The lease refuses to overwrite what someone else pushed since the last fetch. + other.apply(&Op::Pull { rebase: true }).unwrap(); + commit(&other, "c.txt", "c\n", "Later elsewhere"); + other.apply(&Op::Push { upstream: None, force: false }).unwrap(); + git.apply(&Op::Commit { message: "Amended again".into(), amend: true }).unwrap(); + assert!(git.apply(&Op::Push { upstream: None, force: true }).is_err()); } #[test] From fd1e32e89de599c3ea50f645465f6d2904a21c5f Mon Sep 17 00:00:00 2001 From: Lars Jansen Date: Mon, 2 Nov 2026 11:14:09 +0100 Subject: [PATCH 2/3] test: let the remote branch move elsewhere without a second working tree --- crates/core/tests/repo.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/core/tests/repo.rs b/crates/core/tests/repo.rs index 5a7fcf0..84ce6f4 100644 --- a/crates/core/tests/repo.rs +++ b/crates/core/tests/repo.rs @@ -343,10 +343,8 @@ fn push_fetch_and_pull_through_a_remote() { git.apply(&Op::Push { upstream: None, force: true }).unwrap(); assert_eq!(Git::new(remote.path()).run(&["rev-parse", "main"]).unwrap(), git.run(&["rev-parse", "HEAD"]).unwrap()); - // The lease refuses to overwrite what someone else pushed since the last fetch. - other.apply(&Op::Pull { rebase: true }).unwrap(); - commit(&other, "c.txt", "c\n", "Later elsewhere"); - other.apply(&Op::Push { upstream: None, force: false }).unwrap(); + // The lease refuses to overwrite the remote branch once it changed since the last fetch. + Git::new(remote.path()).run(&["update-ref", "refs/heads/main", "main~1"]).unwrap(); git.apply(&Op::Commit { message: "Amended again".into(), amend: true }).unwrap(); assert!(git.apply(&Op::Push { upstream: None, force: true }).is_err()); } From dbbc85fd03719a7a9a8d3aaf076f3efafc30d387 Mon Sep 17 00:00:00 2001 From: Lars Jansen Date: Mon, 2 Nov 2026 11:20:21 +0100 Subject: [PATCH 3/3] test: settle after opening a menu, as the watcher may reload meanwhile --- crates/app/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/app/src/tests.rs b/crates/app/src/tests.rs index 57b8842..3bb4414 100644 --- a/crates/app/src/tests.rs +++ b/crates/app/src/tests.rs @@ -188,7 +188,7 @@ fn stage_and_commit_a_change() { harness.get_by_role_and_label(Role::Button, "First commit"); harness.get_by_label("Uncommitted changes").click(); - harness.run(); + settle(&mut harness); harness.get_by_label("Stage").click(); settle(&mut harness); harness.state_mut().workspace_mut().unwrap().summary = "Second commit".into(); @@ -450,7 +450,7 @@ fn tabs_open_again_on_the_next_run() { /// Picks `path` from the right-click menu of the widget labelled `on`, one item per step. fn menu(harness: &mut Harness<'_, App>, on: &str, path: &[&str]) { harness.get_by_label(on).click_secondary(); - harness.run(); + settle(harness); for item in path { harness.get_by_label(item).click(); settle(harness);