diff --git a/src/cli.rs b/src/cli.rs index 63cff15..5f55d88 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -53,18 +53,30 @@ Example: gw home")] Home, - /// Create new branch from origin/main + /// Create new branch from origin/main (or the current branch with --stack) #[command(long_about = "\ -Create a new branch off a freshly fetched origin/main. +Create a new branch with an unambiguous base. -If you already edited on your home branch, gw new carries those changes onto the -new branch -- nothing is stranded on `main`. +From your home branch, gw new branches off a freshly fetched origin/main. If you +already edited there, those changes are carried onto the new branch -- nothing is +stranded on `main`. (Dirty changes are based on the current HEAD, so creating the +branch never hits a merge conflict; if local main lagged, gw points you at a +rebase afterwards.) -Example: - gw new feature/add-login")] +From a feature branch, the base is ambiguous, so gw new refuses unless you say +which you mean: --stack bases on the CURRENT branch (for stacked PRs, with a +`gh pr create -B ` hint), or run `gw home` first to start fresh from main. + +Examples: + gw new feature/add-login # from home: branch off a fresh origin/main + gw new feature/child --stack # from a feature branch: stack on top of it")] New { /// Name of the branch to create (e.g., feature/add-login) branch: Option, + + /// Base the new branch on the current branch instead of origin/main (stacked PRs) + #[arg(long)] + stack: bool, }, /// Delete merged branch and return to home diff --git a/src/commands/new.rs b/src/commands/new.rs index c012631..cbfab29 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -1,16 +1,38 @@ -//! `gw new` command - Create new branch from origin/main +//! `gw new` command - Create a new branch with a structurally unambiguous base. +//! +//! Three invariants make accidental mistakes unrepresentable: +//! +//! 1. **No ambiguous base.** A base is auto-chosen only where exactly one makes +//! sense: the home branch (→ `origin/main`). From any other branch the base +//! is ambiguous (sibling vs stack), so `gw new` refuses and demands `--stack` +//! (base on the current branch) or returning home first. +//! 2. **No implicit merge.** When the working tree is dirty, the start point is +//! the current HEAD, so creating the branch never performs a working-tree +//! merge and therefore can never conflict or fail cryptically. +//! 3. **No silent displacement.** Uncommitted work only ever travels onto a +//! branch whose base the user explicitly established, and it is always +//! reported. use crate::error::{GwError, Result}; use crate::git; use crate::output; +use crate::state::{RepoType, WorkingDirState}; /// Execute the `new` command -pub fn run(branch_name: Option, verbose: bool) -> Result<()> { +pub fn run(branch_name: Option, stack: bool, verbose: bool) -> Result<()> { // Ensure we're in a git repo if !git::is_git_repo() { return Err(GwError::NotAGitRepository); } + // A detached HEAD has no branch context, so we can't tell home from a + // feature branch nor stack on anything. Refuse rather than guess. + if git::is_detached_head() { + return Err(GwError::Other( + "Cannot run gw new from detached HEAD. Checkout a branch first.".to_string(), + )); + } + let branch_name = branch_name.ok_or(GwError::BranchNameRequired)?; println!(); @@ -31,22 +53,96 @@ pub fn run(branch_name: Option, verbose: bool) -> Result<()> { return Err(GwError::BranchAlreadyExists(branch_name)); } - // Fetch latest - output::info("Fetching from origin..."); - git::fetch_prune(verbose)?; - output::success("Fetched"); + let current = git::current_branch()?; + let repo_type = RepoType::detect()?; + let home_branch = repo_type.home_branch(); + let on_home = current == home_branch; + + // Invariant 1: the base must be unambiguous. + if stack && on_home { + // --stack means "stack on the feature branch I'm on"; on home that's + // meaningless -- plain `gw new` already starts fresh from origin/main. + output::error(&format!( + "--stack requires a non-home branch, but you are on '{}'.", + current + )); + output::hints(&[ + "gw new feature/your-feature # start fresh from origin/main", + "git checkout && gw new feature/child --stack # stack on a feature branch", + ]); + return Err(GwError::Other( + "--stack requires a non-home current branch".to_string(), + )); + } + if !stack && !on_home { + // Refuse to silently base on origin/main from a feature branch: that + // would strip uncommitted work off the branch and pick a base the user + // never chose. Force the explicit decision instead. + output::error(&format!( + "You are on '{}', not the home branch '{}'.", + current, home_branch + )); + output::hints(&[ + &format!("gw new {branch_name} --stack # stack on {current}"), + &format!("gw home && gw new {branch_name} # start fresh from {home_branch}"), + ]); + return Err(GwError::Other( + "gw new outside the home branch needs --stack (or run gw home first)".to_string(), + )); + } - // Detect default remote branch (origin/main or origin/master) - let default_remote = git::get_default_remote_branch()?; + let working_dir = WorkingDirState::detect(); + let dirty = !working_dir.is_clean(); - // Create branch from default remote - git::checkout_new_branch(&branch_name, &default_remote, verbose)?; + // Resolve the start point per invariants 1 & 2. + // + // - --stack: base on the current branch's HEAD (local; no fetch needed). + // - home + clean: base on a freshly fetched origin/main. + // - home + dirty: base on the current HEAD so carrying the working tree + // needs no merge; if local main lags origin/main, defer the catch-up to a + // clean rebase after committing. + let mut behind_count = 0usize; + let (start_point, base_label, pr_base): (String, String, Option) = if stack { + (current.clone(), current.clone(), Some(current.clone())) + } else { + output::info("Fetching from origin..."); + git::fetch_prune(verbose)?; + output::success("Fetched"); + let default_remote = git::get_default_remote_branch()?; + + if dirty { + behind_count = git::commit_count(¤t, &default_remote).unwrap_or(0); + (current.clone(), current.clone(), None) + } else { + (default_remote.clone(), default_remote, None) + } + }; + + // Invariant 3: surface that uncommitted work is moving onto the new branch. + if dirty { + output::warn(&format!( + "Working directory has changes ({}); they will move onto {}", + working_dir.description(), + output::bold(&branch_name) + )); + } + + // Create the branch. The start point is always the current HEAD when dirty, + // so this never performs a working-tree merge. + git::checkout_new_branch(&branch_name, &start_point, verbose)?; output::success(&format!( "Created branch {} from {}", output::bold(&branch_name), - default_remote + base_label )); + if behind_count > 0 { + output::warn(&format!( + "local {} is behind origin/{} ({} commit(s)); rebase after committing", + home_branch, home_branch, behind_count + )); + } + // Show current position let commit_short = git::short_commit()?; let commit_msg = git::head_commit_message()?; @@ -54,12 +150,22 @@ pub fn run(branch_name: Option, verbose: bool) -> Result<()> { output::ready("Ready to work", &branch_name); println!("Base: {commit_short} {commit_msg}"); - output::hints(&[ - "# Make changes, then:", - "git add && git commit -m \"feat: description\"", - &format!("git push -u origin {branch_name}"), - "gh pr create -a \"@me\" -t \"Title\"", - ]); + // Build the next-step hints, inserting a rebase step when local main lagged + // and a `-B ` PR base for stacked branches. + let mut hint_lines: Vec = vec![ + "# Make changes, then:".to_string(), + "git add && git commit -m \"feat: description\"".to_string(), + ]; + if behind_count > 0 { + hint_lines.push("git rebase origin/main # local main was behind; catch up".to_string()); + } + hint_lines.push(format!("git push -u origin {branch_name}")); + hint_lines.push(match &pr_base { + Some(base) => format!("gh pr create -a \"@me\" -B {base} -t \"Title\""), + None => "gh pr create -a \"@me\" -t \"Title\"".to_string(), + }); + let hint_refs: Vec<&str> = hint_lines.iter().map(String::as_str).collect(); + output::hints(&hint_refs); Ok(()) } diff --git a/src/main.rs b/src/main.rs index 383a568..3e890b2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,7 +18,7 @@ fn main() -> ExitCode { let result = match cli.command { Commands::Home => commands::home::run(cli.verbose), - Commands::New { branch } => commands::new::run(branch, cli.verbose), + Commands::New { branch, stack } => commands::new::run(branch, stack, cli.verbose), Commands::Cleanup { branch } => commands::cleanup::run(branch, cli.verbose), Commands::Status => commands::status::run(), Commands::Pause { message } => commands::pause::run(message, cli.verbose), diff --git a/tests/new_test.rs b/tests/new_test.rs new file mode 100644 index 0000000..3feffcc --- /dev/null +++ b/tests/new_test.rs @@ -0,0 +1,204 @@ +//! Integration tests for `gw new`, focused on the `--stack` flag. +//! +//! Plain `gw new` branches off `origin/main`; `--stack` branches off the +//! CURRENT branch instead so stacked PRs can be built. `--stack` on the home +//! branch is refused, since plain `gw new` already covers that case. + +use std::path::Path; +use std::process::{Command, Output}; + +use regex::Regex; +use tempfile::TempDir; + +fn strip_ansi(s: &str) -> String { + let re = Regex::new(r"\x1b\[[0-9;]*m").unwrap(); + re.replace_all(s, "").to_string() +} + +fn run_git(dir: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .expect("Failed to run git command"); + if !output.status.success() { + panic!( + "git {} failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr) + ); + } + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +fn run_gw(dir: &Path, args: &[&str]) -> Output { + let gw_path = env!("CARGO_BIN_EXE_gw"); + Command::new(gw_path) + .args(args) + .current_dir(dir) + .env("NO_COLOR", "1") + .output() + .expect("Failed to run gw command") +} + +/// A local repo on `main` with an `origin` it can fetch from. +fn setup_repo() -> TempDir { + let origin = TempDir::new().unwrap(); + run_git(origin.path(), &["init", "--bare", "--initial-branch=main"]); + + let local = TempDir::new().unwrap(); + run_git(local.path(), &["init", "--initial-branch=main"]); + run_git(local.path(), &["config", "user.email", "test@example.com"]); + run_git(local.path(), &["config", "user.name", "Test User"]); + std::fs::write(local.path().join("README.md"), "# Test").unwrap(); + run_git(local.path(), &["add", "."]); + run_git(local.path(), &["commit", "-m", "Initial commit"]); + let origin_url = format!("file://{}", origin.path().display()); + run_git(local.path(), &["remote", "add", "origin", &origin_url]); + run_git(local.path(), &["push", "-u", "origin", "main"]); + + // Keep both TempDirs alive for the duration of the test by leaking the + // origin; the local dir is what the caller drives. + std::mem::forget(origin); + local +} + +fn current_branch(dir: &Path) -> String { + run_git(dir, &["rev-parse", "--abbrev-ref", "HEAD"]) +} + +#[test] +fn test_stack_branches_off_current_branch() { + let local = setup_repo(); + let dir = local.path(); + + // Build a parent feature branch with its own commit. + assert!(run_gw(dir, &["new", "feature/parent"]).status.success()); + std::fs::write(dir.join("parent.txt"), "parent work").unwrap(); + run_git(dir, &["add", "."]); + run_git(dir, &["commit", "-m", "feat: parent work"]); + let parent_head = run_git(dir, &["rev-parse", "HEAD"]); + + // Stack a child on top of the parent. + let output = run_gw(dir, &["new", "feature/child", "--stack"]); + assert!( + output.status.success(), + "gw new --stack failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let out = strip_ansi(&String::from_utf8_lossy(&output.stdout)); + assert!( + out.contains("from feature/parent"), + "expected base to be the parent branch: {out}" + ); + // PR hint must carry the explicit base so GitHub uses the parent. + assert!( + out.contains("-B feature/parent"), + "expected stacked PR hint with -B feature/parent: {out}" + ); + + assert_eq!(current_branch(dir), "feature/child"); + // The child must start at the parent's HEAD, not origin/main. + assert_eq!(run_git(dir, &["rev-parse", "HEAD"]), parent_head); +} + +#[test] +fn test_stack_on_home_branch_is_refused() { + let local = setup_repo(); + let dir = local.path(); + + let output = run_gw(dir, &["new", "feature/child", "--stack"]); + assert!( + !output.status.success(), + "gw new --stack on home should fail" + ); + + let err = strip_ansi(&String::from_utf8_lossy(&output.stderr)); + assert!( + err.contains("--stack requires a non-home branch"), + "expected refusal message on stderr: {err}" + ); + // The branch must not have been created. + assert_eq!(current_branch(dir), "main"); +} + +#[test] +fn test_plain_new_from_home_bases_on_origin_main() { + let local = setup_repo(); + let dir = local.path(); + + let origin_main = run_git(dir, &["rev-parse", "origin/main"]); + + // Plain `gw new` from the home branch bases on origin/main and emits no + // `-B` PR hint. + let output = run_gw(dir, &["new", "feature/x"]); + assert!( + output.status.success(), + "gw new failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let out = strip_ansi(&String::from_utf8_lossy(&output.stdout)); + assert!( + !out.contains("-B "), + "plain new should not emit a -B PR hint: {out}" + ); + assert_eq!(current_branch(dir), "feature/x"); + assert_eq!(run_git(dir, &["rev-parse", "HEAD"]), origin_main); +} + +#[test] +fn test_plain_new_on_feature_branch_is_refused() { + let local = setup_repo(); + let dir = local.path(); + + // Move onto a feature branch. + assert!(run_gw(dir, &["new", "feature/parent"]).status.success()); + + // Plain `gw new` (no --stack) from a feature branch must refuse rather than + // silently base on origin/main. + let output = run_gw(dir, &["new", "feature/sibling"]); + assert!( + !output.status.success(), + "plain gw new on a feature branch should fail" + ); + + let err = strip_ansi(&String::from_utf8_lossy(&output.stderr)); + assert!( + err.contains("not the home branch"), + "expected refusal pointing at the home branch: {err}" + ); + // Nothing created; still on the feature branch. + assert_eq!(current_branch(dir), "feature/parent"); + assert!( + run_gw(dir, &["status"]).status.success(), + "feature/sibling should not exist" + ); +} + +#[test] +fn test_new_from_home_carries_uncommitted_changes() { + let local = setup_repo(); + let dir = local.path(); + + // Dirty the home branch, then start a branch. The work must travel onto the + // new branch (still uncommitted), not be stranded on main. + std::fs::write(dir.join("wip.txt"), "in progress").unwrap(); + + let output = run_gw(dir, &["new", "feature/x"]); + assert!( + output.status.success(), + "gw new with dirty tree failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + assert_eq!(current_branch(dir), "feature/x"); + // The untracked file is still present and uncommitted on the new branch. + assert!(dir.join("wip.txt").exists()); + let status = run_git(dir, &["status", "--porcelain"]); + assert!( + status.contains("wip.txt"), + "uncommitted change should remain on the new branch: {status}" + ); +}