diff --git a/src/commands/new.rs b/src/commands/new.rs index cbfab29..cf8bd8d 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -136,6 +136,14 @@ pub fn run(branch_name: Option, stack: bool, verbose: bool) -> Result<() base_label )); + // Record the stacked base locally so `gw status` can suggest the right PR + // base (`-B `) before the PR exists. (A stale entry -- e.g. parent + // merged before this branch's PR is opened -- is left for the parent/child + // guard work to handle; `gh pr create` errors loudly on a missing base.) + if let Some(base) = &pr_base { + git::set_branch_base(&branch_name, base, verbose)?; + } + if behind_count > 0 { output::warn(&format!( "local {} is behind origin/{} ({} commit(s)); rebase after committing", diff --git a/src/commands/status.rs b/src/commands/status.rs index 5ede04e..02c7813 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -98,6 +98,20 @@ pub fn run() -> Result<()> { } } + // Locally recorded stacked base (`gw new --stack`). Once a PR exists, + // GitHub's base is authoritative and shown above, so this only fills the + // pre-PR gap. Filtered to a real parent (not the default branch / self). + let recorded_base = if current != home_branch { + git::branch_base(¤t).filter(|b| b != &default_branch && b != ¤t) + } else { + None + }; + if pr_info.is_none() { + if let Some(base) = &recorded_base { + output::info(&format!("Base: {} (stacked, PR not created yet)", base)); + } + } + // Stash count let stash_count = git::stash_count(); if stash_count > 0 { @@ -113,6 +127,7 @@ pub fn run() -> Result<()> { pr_info.as_ref(), has_remote, base_pr_merged.as_deref(), + recorded_base.as_deref(), ); next_action.display(¤t); diff --git a/src/commands/sync.rs b/src/commands/sync.rs index 54d3ae9..c00ca33 100644 --- a/src/commands/sync.rs +++ b/src/commands/sync.rs @@ -166,6 +166,10 @@ pub fn run(verbose: bool) -> Result<()> { output::info(" Force pushing..."); git::force_push_with_lease(¤t, verbose)?; + // The branch now targets the default branch, so it is no longer stacked -- + // drop any locally recorded base so `gw status` stops treating it as such. + git::unset_branch_base(¤t, verbose)?; + println!(); output::ready("Synced", ¤t); output::hints(&[ diff --git a/src/git/mutation.rs b/src/git/mutation.rs index 7681a96..085a97e 100644 --- a/src/git/mutation.rs +++ b/src/git/mutation.rs @@ -149,6 +149,42 @@ pub fn force_push_with_lease(branch: &str, verbose: bool) -> Result<()> { git_run(&["push", "--force-with-lease", "origin", branch], verbose) } +/// Record the base branch a branch is stacked on (`branch..gwBase`). +/// +/// Lets the workflow know a branch is stacked before its PR exists, so +/// `gw status` can suggest `gh pr create -B `. Git drops the whole +/// `[branch ""]` section when the branch is deleted, so this needs no +/// explicit cleanup on `gw cleanup`. +pub fn set_branch_base(branch: &str, base: &str, verbose: bool) -> Result<()> { + git_run( + &["config", &format!("branch.{branch}.gwBase"), base], + verbose, + ) +} + +/// Clear a branch's recorded base (`branch..gwBase`). +/// +/// A no-op (not an error) when the key is absent, so callers can clear +/// unconditionally — e.g. `gw sync` after restacking a branch onto the default +/// branch, where it is no longer stacked. +pub fn unset_branch_base(branch: &str, verbose: bool) -> Result<()> { + if verbose { + output::action(&format!("git config --unset branch.{branch}.gwBase")); + } + let output = Command::new("git") + .args(["config", "--unset", &format!("branch.{branch}.gwBase")]) + .output() + .map_err(|e| GwError::GitCommandFailed(format!("Failed to execute git: {e}")))?; + // Exit code 5 = "key was not present"; treat as already-clear. + match output.status.code() { + Some(0) | Some(5) => Ok(()), + _ => { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(GwError::GitCommandFailed(stderr)) + } + } +} + /// Add a new worktree at the given path with a new branch from a start point pub fn worktree_add(path: &str, branch: &str, start_point: &str, verbose: bool) -> Result<()> { git_run( diff --git a/src/git/query.rs b/src/git/query.rs index 5dbb5fa..40ec667 100644 --- a/src/git/query.rs +++ b/src/git/query.rs @@ -96,6 +96,17 @@ pub fn remote_branch_exists(branch: &str) -> Result { } } +/// Read the recorded base branch for a branch (`branch..gwBase`). +/// +/// `gw new --stack` records the parent here so the workflow knows a branch is +/// stacked *before* its PR exists; once a PR exists, GitHub's base is the source +/// of truth instead. Returns `None` when unset (the branch targets the default +/// branch). It is a local config read — no network. +pub fn branch_base(branch: &str) -> Option { + let value = git_output(&["config", "--get", &format!("branch.{branch}.gwBase")]).ok()?; + if value.is_empty() { None } else { Some(value) } +} + /// Get the current HEAD commit hash pub fn head_commit() -> Result { git_output(&["rev-parse", "HEAD"]) diff --git a/src/state/next_action.rs b/src/state/next_action.rs index cb7c36c..c6c029e 100644 --- a/src/state/next_action.rs +++ b/src/state/next_action.rs @@ -18,8 +18,9 @@ pub enum NextAction { CommitChanges, /// Has unpushed commits, should push PushChanges, - /// Pushed but no PR, should create PR - CreatePr, + /// Pushed but no PR, should create PR. `base` is the stacked parent branch + /// to pass as `-B`, or `None` when the PR targets the default branch. + CreatePr { base: Option }, /// PR is open, waiting for review/CI WaitingForReview { pr_number: u64 }, /// PR is merged, should cleanup @@ -39,6 +40,12 @@ impl NextAction { /// /// # Arguments /// * `base_pr_merged` - If Some(branch_name), the base PR for that branch was merged + /// * `recorded_base` - If Some(branch_name), the locally recorded stacked base + /// (`gw new --stack`), already filtered to a real parent (not the default + /// branch). Used to suggest `-B ` when creating the PR. + // The detected state genuinely depends on this many independent inputs; + // bundling them into a struct would only move the noise to the call site. + #[allow(clippy::too_many_arguments)] pub fn detect( current_branch: &str, home_branch: &str, @@ -47,6 +54,7 @@ impl NextAction { pr_info: Option<&PrInfo>, has_remote: bool, base_pr_merged: Option<&str>, + recorded_base: Option<&str>, ) -> Self { // On home branch if current_branch == home_branch { @@ -101,9 +109,11 @@ impl NextAction { return NextAction::PushChanges; } - // Pushed but no PR → create PR + // Pushed but no PR → create PR (carry the stacked base if recorded) if pr_info.is_none() && has_remote { - return NextAction::CreatePr; + return NextAction::CreatePr { + base: recorded_base.map(String::from), + }; } // PR is open → waiting @@ -150,10 +160,18 @@ impl NextAction { println!(); println!(" git push -u origin {}", branch); } - NextAction::CreatePr => { + NextAction::CreatePr { base } => { output::action("Next: create pull request"); println!(); - println!(" gh pr create -a \"@me\" -t \"...\""); + match base { + Some(base) => { + println!( + " gh pr create -a \"@me\" -B {} -t \"...\" # stacked on {}", + base, base + ) + } + None => println!(" gh pr create -a \"@me\" -t \"...\""), + } } NextAction::WaitingForReview { pr_number } => { if *pr_number > 0 { @@ -215,7 +233,7 @@ impl NextAction { NextAction::SyncHomeWithUpstream { .. } => "sync with upstream", NextAction::CommitChanges => "commit changes", NextAction::PushChanges => "push to remote", - NextAction::CreatePr => "create PR", + NextAction::CreatePr { .. } => "create PR", NextAction::WaitingForReview { .. } => "waiting for review", NextAction::Cleanup => "cleanup branch", NextAction::RebaseNeeded => "rebase needed", @@ -241,6 +259,7 @@ mod tests { None, false, None, + None, ); assert_eq!(action, NextAction::StartNewWork); } @@ -255,6 +274,7 @@ mod tests { None, false, None, + None, ); assert_eq!(action, NextAction::SyncHomeWithUpstream { behind_count: 5 }); } @@ -269,6 +289,7 @@ mod tests { None, true, None, + None, ); assert_eq!(action, NextAction::CommitChanges); } @@ -283,6 +304,7 @@ mod tests { None, true, None, + None, ); assert_eq!(action, NextAction::PushChanges); } @@ -297,6 +319,7 @@ mod tests { None, false, None, + None, ); assert_eq!(action, NextAction::PushChanges); } @@ -311,8 +334,29 @@ mod tests { None, true, None, + None, + ); + assert_eq!(action, NextAction::CreatePr { base: None }); + } + + #[test] + fn test_pushed_no_pr_with_recorded_base_suggests_stacked_pr() { + let action = NextAction::detect( + "feature/child", + "main", + &WorkingDirState::Clean, + &SyncState::Synced, + None, + true, + None, + Some("feature/parent"), + ); + assert_eq!( + action, + NextAction::CreatePr { + base: Some("feature/parent".to_string()) + } ); - assert_eq!(action, NextAction::CreatePr); } #[test] @@ -326,6 +370,7 @@ mod tests { Some(&pr), true, None, + None, ); assert_eq!(action, NextAction::WaitingForReview { pr_number: 42 }); } @@ -350,6 +395,7 @@ mod tests { Some(&pr), true, None, + None, ); assert_eq!(action, NextAction::Cleanup); } @@ -365,6 +411,7 @@ mod tests { Some(&pr), true, None, + None, ); assert_eq!(action, NextAction::PrClosed { pr_number: 42 }); } @@ -379,6 +426,7 @@ mod tests { None, true, None, + None, ); assert_eq!(action, NextAction::RebaseNeeded); } @@ -396,6 +444,7 @@ mod tests { None, true, None, + None, ); assert_eq!(action, NextAction::ResolveDivergence); } @@ -411,6 +460,7 @@ mod tests { Some(&pr), true, None, + None, ); assert_eq!(action, NextAction::CommitChanges); } @@ -435,6 +485,7 @@ mod tests { Some(&pr), true, None, + None, ); // Merged PR takes priority - cleanup first assert_eq!(action, NextAction::Cleanup); @@ -451,6 +502,7 @@ mod tests { Some(&pr), true, Some("feature/base"), + None, ); assert_eq!( action, @@ -471,6 +523,7 @@ mod tests { Some(&pr), true, Some("feature/base"), + None, ); // SyncNeeded should take priority over WaitingForReview assert_eq!( diff --git a/tests/new_test.rs b/tests/new_test.rs index 3feffcc..44e3507 100644 --- a/tests/new_test.rs +++ b/tests/new_test.rs @@ -101,6 +101,70 @@ fn test_stack_branches_off_current_branch() { 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); + + // The stacked base is recorded locally so the PR base can be suggested + // before the PR exists. + assert_eq!( + run_git(dir, &["config", "--get", "branch.feature/child.gwBase"]), + "feature/parent" + ); +} + +#[test] +fn test_status_surfaces_recorded_stacked_base() { + let local = setup_repo(); + let dir = local.path(); + + // Parent branch with an open-PR-shaped history, pushed. + assert!(run_gw(dir, &["new", "feature/parent"]).status.success()); + run_git(dir, &["commit", "--allow-empty", "-m", "feat: parent"]); + run_git(dir, &["push", "-u", "origin", "feature/parent"]); + + // Stack a child (records gwBase), commit and push so it's PR-ready. + assert!( + run_gw(dir, &["new", "feature/child", "--stack"]) + .status + .success() + ); + run_git(dir, &["commit", "--allow-empty", "-m", "feat: child"]); + run_git(dir, &["push", "-u", "origin", "feature/child"]); + + // With no PR yet, status must surface the recorded base and put it in the + // create-PR hint as `-B`, so the stacked base can't be forgotten. + let output = run_gw(dir, &["status"]); + assert!( + output.status.success(), + "gw status failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let out = strip_ansi(&String::from_utf8_lossy(&output.stdout)); + assert!( + out.contains("Base: feature/parent (stacked"), + "expected the pre-PR stacked base to be shown: {out}" + ); + assert!( + out.contains("gh pr create") && out.contains("-B feature/parent"), + "expected the create-PR hint to carry -B feature/parent: {out}" + ); +} + +#[test] +fn test_plain_new_records_no_base() { + let local = setup_repo(); + let dir = local.path(); + + assert!(run_gw(dir, &["new", "feature/x"]).status.success()); + + // Plain new (base = origin/main) must not record a stacked base. + let output = Command::new("git") + .args(["config", "--get", "branch.feature/x.gwBase"]) + .current_dir(dir) + .output() + .unwrap(); + assert!( + !output.status.success(), + "plain new should not record a gwBase" + ); } #[test]