From 7599871aa61aef4ed012de082ba017fab8d26c26 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Tue, 25 Aug 2026 21:36:08 -0400 Subject: [PATCH] feat(stack): move between a stack's members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `up`, `down`, `top`, `bottom` and `checkout` check out the branch ending each member, found from the marks — including from a lower member, where the stacked branch is read back out of the config that recorded the mark. Change-Id: I2d7c39ef3253bb0c49f84276a4d8a4e0810f0dd4 --- README.md | 11 ++ TODO.md | 8 ++ src/cmd/agent_notes.txt | 2 + src/cmd/stack/marks.rs | 33 +++++ src/cmd/stack/mod.rs | 48 +++++++ src/cmd/stack/nav.rs | 272 ++++++++++++++++++++++++++++++++++++++++ tests/stack_flows.rs | 102 +++++++++++++++ 7 files changed, 476 insertions(+) create mode 100644 src/cmd/stack/nav.rs diff --git a/README.md b/README.md index 1451f1f..3e8e583 100644 --- a/README.md +++ b/README.md @@ -314,6 +314,9 @@ atgc stack resubmit # ...just the reconcile, after a rebase/amend/reorder atgc stack link 12 15 17 # chain pulls that already exist, bottom first atgc stack merge # land the stack bottom-up (--through N stops partway) atgc stack view # the current branch's stack, top to bottom +atgc stack up / down # stand on the member above / below (takes a count) +atgc stack top / bottom # ...or the ends +atgc stack checkout part1 # ...or one by position or mark name ``` `stack create` pushes the current branch to the target's knot before it @@ -355,6 +358,14 @@ that conflicts leaves work to do before any reconcile means anything; `sync` stops after the rebase when the rebase stops, rather than planning against a branch that is half moved. +`atgc stack up`, `down`, `top`, `bottom` and `checkout` move between the +members. A member ends at a mark and a mark is a local branch, so this is +`git checkout` with the one hard part done for you: which branch is next. +They work from a lower member too — the stacked branch is found from the +config that recorded the mark — and walking past an end stops there and says +so rather than failing, so `up` can be run until it stops. Nothing is read +from or written to any host. + `atgc stack link 12 15 17` is for pulls that already exist: it makes 15 depend on 12 and 17 on 15, bottom first, without reopening anything, so each one keeps its number, its rounds and the review on it. Every other stack verb diff --git a/TODO.md b/TODO.md index c0d17c9..b898bb2 100644 --- a/TODO.md +++ b/TODO.md @@ -1299,6 +1299,14 @@ agents file five pull requests for one change. `init`ed and its layers `add`ed, never guessed at. With no marks left, the cuts come back out of the records' change-id headers, so a fresh clone still reconciles +- [x] `atgc stack up`, `down`, `top`, `bottom` and `checkout` — move between + a stack's members. The marks already say where each one ends, and the + branches are already there; what was missing was the reverse index, so + standing on a mark could not say which stack it cut. `owners_of` reads + it back out of the config that recorded it, which is the second reason + marks are recorded rather than inferred. Walking past an end stops + there rather than refusing: `up` run until it stops is how a script + finds the top - [x] `atgc stack link` — chain pull requests that already exist, bottom first, without reopening them. `dependentOn` is one field per record, but until now the only way to write it was to build a chain from a diff --git a/src/cmd/agent_notes.txt b/src/cmd/agent_notes.txt index 1755a8e..631be03 100644 --- a/src/cmd/agent_notes.txt +++ b/src/cmd/agent_notes.txt @@ -63,6 +63,8 @@ Stacked pull requests atgc stack resubmit # after any rebase, amend or reorder atgc stack link 12 15 17 # chain existing pulls, bottom first atgc stack merge # lands it bottom-up + atgc stack up / down # stand on another member (top, bottom, + # checkout ) Image previews Commands to create/edit PRs and issues accept Markdown bodies. Paths to diff --git a/src/cmd/stack/marks.rs b/src/cmd/stack/marks.rs index 9c3019a..91e8aa9 100644 --- a/src/cmd/stack/marks.rs +++ b/src/cmd/stack/marks.rs @@ -181,3 +181,36 @@ pub(crate) enum Placed { Moved { from: String }, Kept, } + +/// Every branch that records `mark` as one of its cuts. +/// +/// The reverse of [`recorded`], and the lookup the navigation verbs need: +/// standing on a mark, the stacked branch it belongs to is not derivable +/// from the commits — two branches can hold the same commit — so it is read +/// back out of the config that recorded it. +pub(crate) fn owners_of(mark: &str) -> Vec { + git::git_in( + Path::new("."), + // Lowercase: git normalises the variable half of a key, so this is + // the spelling `--get-regexp` both matches and prints, whatever + // `record` wrote. + &[ + "config", + "--local", + "--get-regexp", + r"^branch\..*\.atgcmark$", + ], + ) + .map(|out| { + out.lines() + .filter_map(|line| line.split_once(' ')) + .filter(|(_, value)| value.trim() == mark) + .filter_map(|(key, _)| { + key.strip_prefix("branch.") + .and_then(|rest| rest.strip_suffix(".atgcmark")) + }) + .map(str::to_string) + .collect::>() + }) + .unwrap_or_default() +} diff --git a/src/cmd/stack/mod.rs b/src/cmd/stack/mod.rs index c3b1950..a3a40e0 100644 --- a/src/cmd/stack/mod.rs +++ b/src/cmd/stack/mod.rs @@ -34,6 +34,7 @@ use anyhow::{Result, bail}; pub(crate) mod marks; +pub(crate) mod nav; pub(crate) mod read; pub(crate) mod write; @@ -429,6 +430,48 @@ pub(crate) enum Command { /// atgc stack mark --forget part1 #[command(verbatim_doc_comment)] Mark(read::MarkArgs), + /// Stand on the member above the current one + /// + /// A stack's members end at marks, and a mark is a local branch, so + /// moving between them is a checkout — the hard part is knowing which + /// branch is next, which is what these four verbs answer. Nothing is + /// read from or written to any host. + /// + /// Takes a number of layers, and stops at the top rather than refusing + /// to go that far. + /// + /// Examples: + /// atgc stack up + /// atgc stack up 2 + #[command(verbatim_doc_comment)] + Up(nav::NavArgs), + + /// Stand on the member below the current one + /// + /// The other half of `atgc stack up`; stops at the bottom. + /// + /// Examples: + /// atgc stack down + /// atgc stack down 2 + #[command(verbatim_doc_comment)] + Down(nav::NavArgs), + + /// Stand on the top member: the stacked branch itself + #[command(verbatim_doc_comment)] + Top(nav::EndArgs), + + /// Stand on the bottom member: the one that merges first + #[command(verbatim_doc_comment)] + Bottom(nav::EndArgs), + + /// Stand on one member by position from the bottom, or by mark name + /// + /// Examples: + /// atgc stack checkout 2 + /// atgc stack checkout part1 + #[command(verbatim_doc_comment)] + Checkout(nav::CheckoutArgs), + /// Chain pull requests that already exist into a stack /// /// `atgc stack link 12 15 17` makes 15 depend on 12 and 17 on 15: bottom @@ -571,6 +614,11 @@ pub(crate) async fn run(command: Command) -> Result<()> { Command::Create(args) => write::create(args).await, Command::Mark(args) => read::mark(args), Command::Link(args) => write::link(args).await, + Command::Up(args) => nav::up(args), + Command::Down(args) => nav::down(args), + Command::Top(args) => nav::top(args), + Command::Bottom(args) => nav::bottom(args), + Command::Checkout(args) => nav::checkout(args), Command::Rebase(args) => write::rebase(args).await, Command::Sync(args) => write::sync(args).await, Command::Merge(args) => write::merge(args).await, diff --git a/src/cmd/stack/nav.rs b/src/cmd/stack/nav.rs new file mode 100644 index 0000000..601cf14 --- /dev/null +++ b/src/cmd/stack/nav.rs @@ -0,0 +1,272 @@ +//! Moving between the members of a stack: `up`, `down`, `top`, `bottom`. +//! +//! A member of a stack ends at a mark, and a mark is a local branch, so +//! moving between members is `git checkout` and the only hard part is +//! knowing which branch is next. That knowledge is in two places — the marks +//! recorded against the stacked branch, and the order the commits sit in — +//! and neither is visible from the branch you are standing on. Doing it by +//! hand means `atgc stack mark` in one window and `git checkout` in the +//! other, and getting it wrong is silent: you amend on the layer above the +//! one you meant. +//! +//! Everything here is local. Nothing is read from a PDS and nothing is +//! written anywhere: these verbs move `HEAD` and say where it landed. +//! +//! The layers, bottom first, are the recorded marks that sit in the range +//! followed by the stacked branch itself, which ends the top member. A mark +//! placed on the tip commit is left out — it would name a member holding no +//! commits, which `stack create` refuses to open — so navigation and the +//! stack that would be published agree about how many members there are. + +use crate::clients::git::run as git; +use anyhow::{Result, bail}; +use std::path::Path; + +#[derive(clap::Args, Debug)] +pub(crate) struct NavArgs { + /// How many layers to move (default 1) + #[arg(default_value_t = 1)] + pub steps: usize, + /// Git remote pointing at the repo + #[arg(long, default_value = "origin")] + pub remote: String, + /// The branch the stack lands on, when it is not the remote's default + #[arg(long)] + pub target: Option, + /// Print one JSON object describing the move instead of the summary line + #[arg(long)] + pub json: bool, +} + +#[derive(clap::Args, Debug)] +pub(crate) struct EndArgs { + /// Git remote pointing at the repo + #[arg(long, default_value = "origin")] + pub remote: String, + /// The branch the stack lands on, when it is not the remote's default + #[arg(long)] + pub target: Option, + /// Print one JSON object describing the move instead of the summary line + #[arg(long)] + pub json: bool, +} + +#[derive(clap::Args, Debug)] +pub(crate) struct CheckoutArgs { + /// Which member to stand on: a position from the bottom, or a mark name + pub which: String, + /// Git remote pointing at the repo + #[arg(long, default_value = "origin")] + pub remote: String, + /// The branch the stack lands on, when it is not the remote's default + #[arg(long)] + pub target: Option, + /// Print one JSON object describing the move instead of the summary line + #[arg(long)] + pub json: bool, +} + +/// What a navigation verb did. +#[derive(serde::Serialize, Debug, PartialEq)] +pub(in crate::cmd) struct MovedJson { + /// The branch that was checked out before, which equals `to` when the + /// move was already where it was going. + pub from: String, + pub to: String, + /// 1-based from the bottom of the stack. + pub position: usize, + pub total: usize, + /// `false` when nothing was checked out, because the end was already + /// underfoot. Not an error: rerunning `up` at the top is how a script + /// finds out it is at the top. + pub moved: bool, + /// Every layer, bottom first. + pub layers: Vec, +} + +/// The branches ending each member of a stack, bottom first, and where the +/// checkout is standing in them. +struct Layers { + branches: Vec, + /// Index into `branches` of the branch currently checked out. + at: usize, +} + +/// Work out the stack the checkout is standing in. +/// +/// The branch you are on is either the stacked branch — it has marks +/// recorded against it — or one of its marks, in which case the stacked +/// branch is whichever branch recorded it. That second lookup is why marks +/// are stored in git config rather than inferred: it is a reverse index, and +/// there is nothing else to search. +fn layers(remote: &str, target: Option<&str>) -> Result { + let here = git::current_branch()?; + let stacked = stacked_branch(&here)?; + + let target = match target { + Some(t) => t.to_string(), + None => git::remote_default_branch(remote).unwrap_or_else(|| "main".to_string()), + }; + let base = format!("{remote}/{target}"); + if !git::ref_exists(&base) { + bail!( + "{base} is not in this checkout, so there is no range to find the stack's \ + members in\n\ + fetch it, or name another target with --target" + ); + } + // The range is the stacked branch's, not HEAD's: standing on a lower + // member, `HEAD` stops partway up and the layers above it would vanish + // from the listing exactly when `up` needs them. + let commits = commits_between(&base, &stacked)?; + let placed = super::marks::positions(&stacked, &commits); + + let mut branches: Vec = placed + .into_iter() + .filter(|(at, _)| *at + 1 < commits.len()) + .map(|(_, name)| name) + .collect(); + branches.push(stacked.clone()); + + let Some(at) = branches.iter().position(|b| *b == here) else { + // Reachable when the mark you are standing on has fallen out of the + // range — the state a plain `git rebase` leaves behind, which + // `atgc stack mark` lists and this cannot navigate. + bail!( + "{here} is a mark of {stacked} but no longer sits in {base}..{stacked}\n\ + `atgc stack mark` lists the marks that are stranded; `atgc stack rebase` \ + carries them" + ); + }; + Ok(Layers { branches, at }) +} + +/// The stacked branch `here` belongs to: itself when it has marks, and +/// otherwise the branch that recorded it as one. +fn stacked_branch(here: &str) -> Result { + if !super::marks::recorded(here).is_empty() { + return Ok(here.to_string()); + } + let owners = super::marks::owners_of(here); + match owners.len() { + 1 => Ok(owners.into_iter().next().expect("one owner")), + 0 => bail!( + "{here} is not part of a stack: it has no marks, and no branch records it as one\n\ + `atgc stack mark HEAD~3` cuts a branch into members; `atgc pr view` is the \ + command for a flat pull request" + ), + // Two branches marking the same name is a repository somebody has + // reshaped by hand. Naming them is the whole fix, and guessing which + // stack was meant would move HEAD somewhere unasked for. + _ => bail!( + "{here} is recorded as a mark of more than one branch ({})\n\ + `atgc stack mark --forget {here}` on the one it no longer cuts", + owners.join(", "), + ), + } +} + +/// The commits of `base..branch`, oldest first. +/// +/// [`crate::clients::git::patch::commits_since`] answers about `HEAD`, which +/// is the wrong end when the point is to look at layers above where you are +/// standing. +fn commits_between(base: &str, branch: &str) -> Result> { + let out = git::git_in( + Path::new("."), + &["rev-list", "--reverse", &format!("{base}..{branch}")], + )?; + Ok(out.lines().map(str::to_string).collect()) +} + +pub(crate) fn up(args: NavArgs) -> Result<()> { + step(args, 1) +} + +pub(crate) fn down(args: NavArgs) -> Result<()> { + step(args, -1) +} + +/// Move `steps` layers in `direction`, stopping at the end rather than +/// refusing: "as far as it goes" is what somebody asking for three steps in +/// a two-layer stack means, and an error there would only be re-run with a +/// smaller number. +fn step(args: NavArgs, direction: isize) -> Result<()> { + crate::term::jsonout::init(args.json); + let layers = layers(&args.remote, args.target.as_deref())?; + let wanted = layers.at as isize + direction * args.steps as isize; + let target = wanted.clamp(0, layers.branches.len() as isize - 1) as usize; + go(layers, target, args.json) +} + +pub(crate) fn top(args: EndArgs) -> Result<()> { + crate::term::jsonout::init(args.json); + let layers = layers(&args.remote, args.target.as_deref())?; + let last = layers.branches.len() - 1; + go(layers, last, args.json) +} + +pub(crate) fn bottom(args: EndArgs) -> Result<()> { + crate::term::jsonout::init(args.json); + let layers = layers(&args.remote, args.target.as_deref())?; + go(layers, 0, args.json) +} + +/// Stand on a member named by position or by mark. +pub(crate) fn checkout(args: CheckoutArgs) -> Result<()> { + crate::term::jsonout::init(args.json); + let layers = layers(&args.remote, args.target.as_deref())?; + let total = layers.branches.len(); + // A position first, because the listings print one and a branch named + // "2" is a branch somebody will have to rename anyway. + let target = match args.which.parse::() { + Ok(n) if (1..=total).contains(&n) => n - 1, + Ok(n) => bail!("there is no member {n}: this stack has {total}"), + Err(_) => layers + .branches + .iter() + .position(|b| *b == args.which) + .ok_or_else(|| { + anyhow::anyhow!( + "{} is not a member of this stack ({})", + args.which, + layers.branches.join(", "), + ) + })?, + }; + go(layers, target, args.json) +} + +/// Check out layer `target` and say where that is, or say that it is already +/// underfoot. +fn go(layers: Layers, target: usize, json: bool) -> Result<()> { + let from = layers.branches[layers.at].clone(); + let to = layers.branches[target].clone(); + let moved = target != layers.at; + if moved { + // git's own refusal is the right one for a dirty tree: it names the + // files, which is what somebody has to act on. + git::git_in(Path::new("."), &["checkout", &to])?; + } + let report = MovedJson { + from, + to: to.clone(), + position: target + 1, + total: layers.branches.len(), + moved, + layers: layers.branches, + }; + if json { + return crate::term::jsonout::emit(&report); + } + let subject = git::git_in(Path::new("."), &["log", "-1", "--format=%s", &to]) + .map(|s| s.trim().to_string()) + .unwrap_or_default(); + let where_ = format!("{}/{}", report.position, report.total); + if moved { + println!("{to} ({where_}) {subject}"); + } else { + println!("already on {to} ({where_}) {subject}"); + } + Ok(()) +} diff --git a/tests/stack_flows.rs b/tests/stack_flows.rs index 9237058..bd042be 100644 --- a/tests/stack_flows.rs +++ b/tests/stack_flows.rs @@ -2298,3 +2298,105 @@ fn link_json_describes_the_chain_bottom_first() { assert_eq!(members[0]["changed"], false, "{report:#}"); assert_eq!(members[1]["changed"], true, "{report:#}"); } + +// --------------------------------------------------------------------------- +// navigation +// --------------------------------------------------------------------------- + +/// A three-commit branch cut into three members: marks at the bottom two +/// commits, the branch itself ending the top one. +fn marked_three(label: &str) -> Scenario { + let world = Scenario::new(label); + three_commit_branch(&world); + world.run(&["stack", "mark", "part1", "HEAD~2"]).success(); + world.run(&["stack", "mark", "part2", "HEAD~1"]).success(); + world +} + +/// The branch a scenario's checkout is standing on. +fn on(world: &Scenario) -> String { + world + .checkout + .git(&["rev-parse", "--abbrev-ref", "HEAD"]) + .trim() + .to_string() +} + +/// `up` and `down` walk the members, and each step is an ordinary checkout. +#[test] +fn up_and_down_walk_the_members() { + let world = marked_three("nav-walk"); + + world.run(&["stack", "bottom"]).success(); + assert_eq!(on(&world), "part1"); + world.run(&["stack", "up"]).success(); + assert_eq!(on(&world), "part2"); + world.run(&["stack", "up"]).success(); + assert_eq!(on(&world), "feature"); + world.run(&["stack", "down", "2"]).success(); + assert_eq!(on(&world), "part1"); + world.run(&["stack", "top"]).success(); + assert_eq!(on(&world), "feature"); +} + +/// Walking past the end stops there and says so, rather than failing: a +/// script that runs `up` until it stops needs the answer, not an error. +#[test] +fn walking_past_the_top_stops_and_says_so() { + let world = marked_three("nav-past-the-end"); + + let run = world.run(&["stack", "up", "9"]).success(); + + assert_eq!(on(&world), "feature"); + assert!(run.stdout.contains("already on"), "{}", run.stdout); +} + +/// Navigation works from a lower member too, where the branch underfoot is a +/// mark and the stack it belongs to has to be found from the config. +#[test] +fn navigating_from_a_lower_member_finds_the_stack_it_belongs_to() { + let world = marked_three("nav-from-below"); + world.checkout.git(&["checkout", "-q", "part1"]); + + let report = world.run(&["stack", "up", "--json"]).success().json(); + + assert_eq!(report["from"], "part1", "{report:#}"); + assert_eq!(report["to"], "part2", "{report:#}"); + assert_eq!(report["position"], 2, "{report:#}"); + assert_eq!(report["total"], 3, "{report:#}"); + assert_eq!(report["moved"], true, "{report:#}"); + let layers = report["layers"].as_array().expect("layers"); + assert_eq!(layers[0], "part1", "bottom first: {report:#}"); + assert_eq!(layers[2], "feature", "{report:#}"); +} + +/// `checkout` takes a position or a mark name, and refuses a position the +/// stack does not have. +#[test] +fn checkout_takes_a_position_or_a_name() { + let world = marked_three("nav-checkout"); + + world.run(&["stack", "checkout", "2"]).success(); + assert_eq!(on(&world), "part2"); + world.run(&["stack", "checkout", "part1"]).success(); + assert_eq!(on(&world), "part1"); + + let run = world.run(&["stack", "checkout", "9"]); + assert_ne!(run.code, Some(0), "{}", run.stdout); + assert!(run.stderr.contains("this stack has 3"), "{}", run.stderr); + assert_eq!(on(&world), "part1", "a refusal must not move HEAD"); +} + +/// A branch that is not part of a stack is told so, and pointed at the +/// commands that are for it. +#[test] +fn navigating_an_unstacked_branch_is_refused_with_a_pointer() { + let world = Scenario::new("nav-unstacked"); + three_commit_branch(&world); + + let run = world.run(&["stack", "up"]); + + assert_ne!(run.code, Some(0), "{}", run.stdout); + assert!(run.stderr.contains("not part of a stack"), "{}", run.stderr); + assert!(run.stderr.contains("stack mark"), "{}", run.stderr); +} -- 2.51.2