From 009482085e9c6f67a135ef99f1aa04b2ab78fa4f Mon Sep 17 00:00:00 2001 From: Keaton Johnstone Date: Wed, 28 Oct 2026 11:52:10 -0600 Subject: [PATCH] Compare branches: the switcher's branch picker, each branch against its stack base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repository's "Compare branches…" row, also in the source picker, turns the switcher into a picker over its local branches, then the bases the picked one can be compared with, opening or resuming the branch review of the pair. Each branch is offered against its stack base (the nearest unmerged branch below it, found for every branch with one git log --simplify-by-decoration ^), else the default branch; a default-branch base stores no base, so it resumes the Branch row's review. Esc steps back. DESIGN §4.1, PLAN §2.2 and §11; state.switcher mode and detail, source_picker.compare; switcher_branches. --- crates/dv/src/automation/state.rs | 16 +- crates/dv/src/automation/tests.rs | 4 +- crates/dv_core/src/git/branches.rs | 248 +++++++++++++++++++++ crates/dv_core/src/git/mod.rs | 1 + crates/dv_ui/src/palette/mod.rs | 8 +- crates/dv_ui/src/services/reviews.rs | 16 ++ crates/dv_ui/src/shell/mod.rs | 19 ++ crates/dv_ui/src/switcher/branches.rs | 195 +++++++++++++++++ crates/dv_ui/src/switcher/items.rs | 109 ++++++++-- crates/dv_ui/src/switcher/mod.rs | 103 ++++++++- crates/dv_ui/src/switcher/render.rs | 67 +++++- crates/dv_ui/src/switcher/source.rs | 125 +++++++++-- crates/dv_ui/src/switcher/tests.rs | 301 +++++++++++++++++++++++++- docs/DESIGN.md | 23 +- docs/PLAN.md | 14 +- tests/e2e/README.md | 5 +- tests/e2e/review_chrome.jsonl | 2 +- tests/e2e/switcher_branches.jsonl | 37 ++++ 18 files changed, 1218 insertions(+), 75 deletions(-) create mode 100644 crates/dv_core/src/git/branches.rs create mode 100644 crates/dv_ui/src/switcher/branches.rs create mode 100644 tests/e2e/switcher_branches.jsonl diff --git a/crates/dv/src/automation/state.rs b/crates/dv/src/automation/state.rs index 400de2e..34ddcd8 100644 --- a/crates/dv/src/automation/state.rs +++ b/crates/dv/src/automation/state.rs @@ -84,7 +84,7 @@ fn badge(badge: Option<&ReviewBadge>) -> Value { /// `state.switcher`. pub fn switcher(shell: &Shell, cx: &App) -> Value { let Some(switcher) = shell.switcher() else { - return json!({"open": false, "query": null, "selected": null, "items": []}); + return json!({"open": false, "mode": null, "query": null, "selected": null, "items": []}); }; let switcher = switcher.read(cx); let listing = switcher.listing(); @@ -97,10 +97,11 @@ pub fn switcher(shell: &Shell, cx: &App) -> Value { "key": item.key, "kind": item.kind_key(), "target": match &item.kind { - ItemKind::Target(offer) => Some(offer.kind.key()), + ItemKind::Target(offer) | ItemKind::Base { offer, .. } => Some(offer.kind.key()), _ => None, }, "label": item.label(), + "detail": item.detail(), "starting": item.is_starting(), "repo": repo.map(|repo| repo.path.clone()), "review": badge(item.review()), @@ -109,6 +110,7 @@ pub fn switcher(shell: &Shell, cx: &App) -> Value { .collect(); json!({ "open": true, + "mode": switcher.mode().key(), "query": switcher.query(), "selected": switcher.selected_key(), "items": items, @@ -118,7 +120,7 @@ pub fn switcher(shell: &Shell, cx: &App) -> Value { /// `state.source_picker`. pub fn source_picker(shell: &Shell, cx: &App) -> Value { let Some(picker) = shell.source_picker() else { - return json!({"open": false, "selected": null, "items": []}); + return json!({"open": false, "selected": null, "items": [], "compare": false}); }; let picker = picker.read(cx); let items: Vec = picker @@ -132,10 +134,16 @@ pub fn source_picker(shell: &Shell, cx: &App) -> Value { }) }) .collect(); + let selected = if picker.compare_selected() { + Some("compare") + } else { + picker.selected().map(|offer| offer.kind.key()) + }; json!({ "open": true, - "selected": picker.selected().map(|offer| offer.kind.key()), + "selected": selected, "items": items, + "compare": picker.offers_compare(), }) } diff --git a/crates/dv/src/automation/tests.rs b/crates/dv/src/automation/tests.rs index 1a0b949..df28027 100644 --- a/crates/dv/src/automation/tests.rs +++ b/crates/dv/src/automation/tests.rs @@ -228,11 +228,11 @@ fn state_describes_the_empty_shell(cx: &mut TestAppContext) { assert_eq!(state["backends"], json!([])); assert_eq!( state["switcher"], - json!({"open": false, "query": null, "selected": null, "items": []}) + json!({"open": false, "mode": null, "query": null, "selected": null, "items": []}) ); assert_eq!( state["source_picker"], - json!({"open": false, "selected": null, "items": []}) + json!({"open": false, "selected": null, "items": [], "compare": false}) ); assert_eq!( state["file_list"], diff --git a/crates/dv_core/src/git/branches.rs b/crates/dv_core/src/git/branches.rs new file mode 100644 index 0000000..77cbe74 --- /dev/null +++ b/crates/dv_core/src/git/branches.rs @@ -0,0 +1,248 @@ +//! The repository's local branches and where each one stacks (PLAN §2.2 "Branches"): what the +//! switcher's branch picker offers (DESIGN §4.1), a `branch` target's branch and its base. +//! +//! **Stack bases.** A branch started from another unmerged branch (`main → feat-a → feat-b`) +//! is reviewed against that branch, not the default one: its *stack base* is the nearest +//! branch tip among its first-parent ancestors that isn't in the default branch's history. +//! One `git log --branches --simplify-by-decoration ^` finds them all: it keeps +//! only the commits a branch points at, outside the default branch, and rewrites each one's +//! parents to its nearest such commit, so a branch's first rewritten parent is its stack +//! base's tip; a parent left out (in the default branch's history) means the default branch. +//! A branch merged into the default branch is outside the walk and has no stack base either. + +use std::collections::HashMap; + +use super::{GitError, GitRepo, Oid, stdout_text}; + +/// A local branch (`refs/heads/`). +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Branch { + /// Its name without `refs/heads/`. + pub name: String, + pub tip: Oid, + /// Its tip's committer date, in seconds since the epoch. + pub committed_at: i64, + /// The unmerged branch it stacks on (module docs); `None` when it stacks on the default + /// branch, is merged into it, or there is no default branch to tell. + pub stack_base: Option, +} + +impl GitRepo { + /// Every local branch, the most recently committed first, each with its stack base over + /// `default_base` (module docs; `GitRepo::default_branch`). Two git commands, whatever + /// the number of branches. + pub fn branches(&self, default_base: Option<&str>) -> Result, GitError> { + let output = self.git( + &[ + "for-each-ref", + "--sort=-committerdate", + "--format=%(refname)%09%(objectname)%09%(committerdate:unix)", + "refs/heads/", + ], + &[0], + )?; + let text = stdout_text("for-each-ref", &output)?; + let mut branches: Vec = text + .lines() + .filter_map(|line| { + let mut fields = line.split('\t'); + let name = fields.next()?.strip_prefix("refs/heads/")?; + let tip = Oid::parse(fields.next()?)?; + let committed_at = fields.next()?.parse().unwrap_or(0); + Some(Branch { + name: name.to_string(), + tip, + committed_at, + stack_base: None, + }) + }) + .collect(); + let Some(default_base) = default_base else { + return Ok(branches); + }; + let tips = self.unmerged_tips(default_base)?; + for branch in &mut branches { + branch.stack_base = tips + .get(branch.tip.as_str()) + .and_then(|tip| tip.parent.as_deref()) + .and_then(|parent| tips.get(parent)) + .and_then(|parent| parent.names.first().cloned()); + } + Ok(branches) + } + + /// The commits local branches point at outside `default_base`'s history, each with its + /// first parent in the simplified history (module docs) and the branches at it, sorted. + fn unmerged_tips(&self, default_base: &str) -> Result, GitError> { + let exclude = format!("^{default_base}"); + let output = self.git( + &[ + "log", + "--no-show-signature", + "--simplify-by-decoration", + "--decorate-refs=refs/heads/", + "--format=%H%x09%P%x09%D", + "--branches", + "--end-of-options", + &exclude, + "--", + ], + &[0], + )?; + let text = stdout_text("log", &output)?; + Ok(text + .lines() + .filter_map(|line| { + let mut fields = line.split('\t'); + let commit = fields.next()?.to_string(); + let parent = fields.next()?.split_whitespace().next().map(str::to_string); + // `HEAD -> feat, feat-copy`: ref names hold no spaces, so `, ` parts them. + let mut names: Vec = fields + .next() + .unwrap_or_default() + .split(", ") + .map(|name| name.strip_prefix("HEAD -> ").unwrap_or(name)) + .filter(|name| !name.is_empty() && *name != "HEAD") + .map(str::to_string) + .collect(); + names.sort(); + Some((commit, Tip { parent, names })) + }) + .collect()) + } +} + +/// A commit a branch points at, in the simplified history. +struct Tip { + parent: Option, + names: Vec, +} + +#[cfg(test)] +mod tests { + use dv_test_support::FixtureRepo; + + use crate::testing::git_repo; + + fn bases(repo: &FixtureRepo, default: Option<&str>) -> Vec<(String, Option)> { + let mut found: Vec<(String, Option)> = git_repo(repo) + .branches(default) + .expect("branches list") + .into_iter() + .map(|branch| (branch.name, branch.stack_base)) + .collect(); + found.sort(); + found + } + + fn pair(name: &str, base: Option<&str>) -> (String, Option) { + (name.to_string(), base.map(str::to_string)) + } + + #[test] + fn a_stack_finds_each_branch_below() { + let repo = FixtureRepo::new(); + repo.write("f.txt", "m1\n"); + repo.commit("m1"); + repo.branch("merged"); + repo.write("f.txt", "m2\n"); + repo.commit("m2"); + repo.branch("feat-a") + .checkout("feat-a") + .write("a.txt", "a\n"); + repo.commit("a1"); + repo.branch("feat-b") + .checkout("feat-b") + .write("b.txt", "b\n"); + repo.commit("b1"); + repo.branch("feat-c") + .checkout("feat-c") + .write("c.txt", "c\n"); + repo.commit("c1"); + // A branch on the default branch's history, and the default branch moving on. + repo.checkout("main").branch("side").checkout("side"); + repo.write("s.txt", "s\n"); + repo.commit("s1"); + repo.checkout("main").write("f.txt", "m3\n"); + repo.commit("m3"); + assert_eq!( + bases(&repo, Some("main")), + vec![ + pair("feat-a", None), + pair("feat-b", Some("feat-a")), + pair("feat-c", Some("feat-b")), + pair("main", None), + pair("merged", None), + pair("side", None), + ] + ); + } + + #[test] + fn no_default_branch_means_no_stack_bases() { + let repo = FixtureRepo::new(); + repo.write("f.txt", "m1\n"); + repo.commit("m1"); + repo.branch("feat-a") + .checkout("feat-a") + .write("a.txt", "a\n"); + repo.commit("a1"); + repo.branch("feat-b") + .checkout("feat-b") + .write("b.txt", "b\n"); + repo.commit("b1"); + assert_eq!( + bases(&repo, None), + vec![ + pair("feat-a", None), + pair("feat-b", None), + pair("main", None) + ] + ); + } + + #[test] + fn a_branch_at_another_ones_tip_stacks_below_both() { + let repo = FixtureRepo::new(); + repo.write("f.txt", "m1\n"); + repo.commit("m1"); + repo.branch("feat-a") + .checkout("feat-a") + .write("a.txt", "a\n"); + repo.commit("a1"); + // `feat-a2` points where `feat-a` does: neither stacks on the other. + repo.branch("feat-a2"); + repo.branch("feat-b") + .checkout("feat-b") + .write("b.txt", "b\n"); + repo.commit("b1"); + assert_eq!( + bases(&repo, Some("main")), + vec![ + pair("feat-a", None), + pair("feat-a2", None), + pair("feat-b", Some("feat-a")), + pair("main", None), + ] + ); + } + + #[test] + fn newest_first() { + let repo = FixtureRepo::new(); + repo.write("f.txt", "m1\n"); + repo.commit("m1"); + repo.branch("older"); + repo.write("f.txt", "m2\n"); + repo.commit("m2"); + repo.branch("newer"); + let names: Vec = git_repo(&repo) + .branches(Some("main")) + .expect("branches list") + .into_iter() + .map(|branch| branch.name) + .collect(); + assert_eq!(names.first().map(String::as_str), Some("main")); + assert_eq!(names.last().map(String::as_str), Some("older")); + } +} diff --git a/crates/dv_core/src/git/mod.rs b/crates/dv_core/src/git/mod.rs index 3f08b68..a7039b8 100644 --- a/crates/dv_core/src/git/mod.rs +++ b/crates/dv_core/src/git/mod.rs @@ -4,6 +4,7 @@ //! Revisions from users and reviews are passed after `--end-of-options`, so a revision that //! starts with `-` is looked up as a revision and never read as an option. +pub mod branches; pub mod command; pub(crate) mod temp_index; diff --git a/crates/dv_ui/src/palette/mod.rs b/crates/dv_ui/src/palette/mod.rs index b9186f5..f9f3f2b 100644 --- a/crates/dv_ui/src/palette/mod.rs +++ b/crates/dv_ui/src/palette/mod.rs @@ -151,7 +151,13 @@ pub fn build( let target = match &item.kind { ItemKind::Target(offer) => Some(offer.kind), ItemKind::Review { .. } => None, - ItemKind::Start { .. } | ItemKind::Retry { .. } | ItemKind::OpenFolder => continue, + // The branch picker is the switcher's: the palette lists the reviews it opens. + ItemKind::Start { .. } + | ItemKind::Retry { .. } + | ItemKind::OpenFolder + | ItemKind::CompareBranches + | ItemKind::Branch { .. } + | ItemKind::Base { .. } => continue, }; let (Some((location, request)), Some(repo)) = ( listing.request(ix), diff --git a/crates/dv_ui/src/services/reviews.rs b/crates/dv_ui/src/services/reviews.rs index 44ebefc..460ab1a 100644 --- a/crates/dv_ui/src/services/reviews.rs +++ b/crates/dv_ui/src/services/reviews.rs @@ -48,6 +48,7 @@ use std::sync::Arc; use dv_backend::{Connection, ConnectionKey}; use dv_core::diff::file_list; +use dv_core::git::branches::Branch; use dv_core::label::{LabelContext, Labels}; use dv_core::review::{ Author, Home, HomeError, ListEntry, LoadedReview, ReviewId, ReviewStore, ReviewSummary, home, @@ -134,6 +135,10 @@ pub struct RepoRow { /// Its reviews, newest first. pub reviews: Vec, pub labels: LabelContext, + /// Its local branches, the most recently committed first, each with its stack base: what + /// the switcher's branch picker offers (DESIGN §4.1). Empty until a refresh read them, or + /// when it couldn't. + pub branches: Vec, /// When it was last used, for the order rows are listed in. pub last_used: u64, } @@ -179,6 +184,7 @@ impl RepoRow { working: None, reviews: Vec::new(), labels: LabelContext::default(), + branches: Vec::new(), last_used: repo.last_used(), } } @@ -189,6 +195,8 @@ struct Scan { working: Result, reviews: Result, Failure>, labels: LabelContext, + /// Empty when they couldn't be read: the picker then offers none, the row the rest. + branches: Vec, toplevel: String, /// The repository's location as its backend resolved it: its worktree root. root: Location, @@ -630,6 +638,7 @@ impl Reviews { Refreshed::Scanned(scan) => { resolved = Some(scan.root); row.labels = scan.labels; + row.branches = scan.branches; row.unreachable = None; row.stopped = None; row.retry = false; @@ -1041,10 +1050,17 @@ fn scan(backend: Arc) -> Scan { additions: files.iter().map(|file| u64::from(file.additions)).sum(), deletions: files.iter().map(|file| u64::from(file.deletions)).sum(), }); + let branches = repo + .branches(labels.default_base.as_deref()) + .unwrap_or_else(|error| { + log::warn!("couldn't list the branches of {toplevel}: {error}"); + Vec::new() + }); Scan { working, reviews, labels, + branches, toplevel, root, } diff --git a/crates/dv_ui/src/shell/mod.rs b/crates/dv_ui/src/shell/mod.rs index cc4c76a..5bdf948 100644 --- a/crates/dv_ui/src/shell/mod.rs +++ b/crates/dv_ui/src/shell/mod.rs @@ -852,6 +852,22 @@ impl Shell { cx.notify(); } + /// Opens the switcher as the branch picker on the repository at `location` (the source + /// picker's "Compare branches…"), in place of the open menu. + pub fn open_branch_picker( + &mut self, + location: Location, + window: &mut Window, + cx: &mut Context, + ) { + self.open_switcher(Some(&location), window, cx); + if let Some(switcher) = &self.switcher { + switcher.view.update(cx, |switcher, cx| { + switcher.compare_branches(location, window, cx) + }); + } + } + /// Opens the source picker over the open review's repository. Like the switcher, it /// replaces an open menu and opens over an open sheet. pub fn open_source_picker(&mut self, window: &mut Window, cx: &mut Context) { @@ -1169,6 +1185,9 @@ impl Shell { ) .detach(); } + SourcePickerEvent::CompareBranches { location } => { + self.open_branch_picker(location.clone(), window, cx); + } SourcePickerEvent::Dismiss => { self.close_menu(window, cx); } diff --git a/crates/dv_ui/src/switcher/branches.rs b/crates/dv_ui/src/switcher/branches.rs new file mode 100644 index 0000000..b2cf93d --- /dev/null +++ b/crates/dv_ui/src/switcher/branches.rs @@ -0,0 +1,195 @@ +//! The switcher's branch picker (DESIGN §4.1 "Compare branches…", PLAN §2.2): one +//! repository's local branches, then the bases the picked one can be compared with, each pair +//! opening or resuming its `branch` review. It is a mode of the switcher ([`Mode`]): the same +//! panel, filter field and keys, its rows listed from the repository's row +//! (`RepoRow::branches`, read with its reviews). +//! +//! **Branches.** Every local branch but the default one, the most recently committed first, +//! each beside the base it is offered against — its stack base (`dv_core::git::branches`), +//! else the default branch — and that pair's open review. Picking one lists its bases. +//! +//! **Bases.** The stack base first ("stack base"), then the default branch ("default +//! branch"), then every other local branch, the most recently committed first. Picking one +//! opens the branch against it; against the default branch that is the target with no base, +//! the switcher's own Branch row's and `dv review new --branch`'s, so it resumes their review. + +use dv_core::{Location, Target}; + +use super::items::{ + Entry, Item, ItemKind, Listing, ReviewBadge, TargetKind, TargetOffer, header, matches, + repo_keys, +}; +use crate::services::reviews::RepoRow; + +/// What the switcher lists. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub enum Mode { + /// The known repositories, their targets and reviews. + #[default] + Repos, + /// The branch picker's first step: the branches of the repository at `location`. + Branches { location: Location }, + /// Its second step: the bases `branch` can be compared with. + Bases { location: Location, branch: String }, +} + +impl Mode { + /// `"repos"`, `"branches"` or `"bases"`, for automation state. + pub fn key(&self) -> &'static str { + match self { + Self::Repos => "repos", + Self::Branches { .. } => "branches", + Self::Bases { .. } => "bases", + } + } + + /// The filter field's placeholder. + pub fn placeholder(&self) -> String { + match self { + Self::Repos => super::PLACEHOLDER.to_string(), + Self::Branches { .. } => "Pick a branch to review".to_string(), + Self::Bases { branch, .. } => format!("Compare {branch} with…"), + } + } +} + +/// Whether `row` has a branch to compare: one other than the default branch. +pub fn has_branches(row: &RepoRow) -> bool { + row.branches + .iter() + .any(|branch| Some(branch.name.as_str()) != row.labels.default_base.as_deref()) +} + +/// The target of `branch` against `base`: with no base when `base` is the default branch. +pub fn branch_target(branch: &str, base: &str, default_base: Option<&str>) -> Target { + Target::Branch { + branch: branch.to_string(), + base: (Some(base) != default_base).then(|| base.to_string()), + } +} + +/// The base `name` is offered against in the first step: its stack base, else the default +/// branch. +fn offered_base(row: &RepoRow, name: &str) -> Option { + row.branches + .iter() + .find(|branch| branch.name == name) + .and_then(|branch| branch.stack_base.clone()) + .or_else(|| row.labels.default_base.clone()) +} + +/// The row of `location` among `rows`, with its key. +fn find<'a>(rows: &[&'a RepoRow], location: &Location) -> Option<(&'a RepoRow, String)> { + let keys = repo_keys(rows); + rows.iter() + .zip(keys) + .find(|(row, _)| &row.location == location) + .map(|(row, key)| (*row, key)) +} + +/// A listing of one repository's `items`, under its header, those whose text holds every word +/// of `query` kept. +fn listing(row: &RepoRow, key: String, items: Vec, query: &str) -> Listing { + let words: Vec = query.split_whitespace().map(str::to_lowercase).collect(); + let mut listing = Listing { + repos: vec![header(row, key)], + entries: vec![Entry::Repo(0)], + items: Vec::new(), + }; + for mut item in items { + let text = format!("{} {}", item.label(), item.detail().unwrap_or_default()).to_lowercase(); + if !matches(&text, &words) { + continue; + } + item.repo = Some(0); + listing.entries.push(Entry::Item(listing.items.len())); + listing.items.push(item); + } + listing +} + +/// The first step's listing for the repository at `location` (module docs, "Branches"); empty +/// when it isn't among `rows` or can't be read. +pub fn build_branches(rows: &[&RepoRow], location: &Location, query: &str) -> Listing { + let Some((row, key)) = find(rows, location) else { + return Listing::default(); + }; + if !row.is_readable() { + return listing(row, key, Vec::new(), query); + } + let default_base = row.labels.default_base.as_deref(); + let items = row + .branches + .iter() + .filter(|branch| Some(branch.name.as_str()) != default_base) + .map(|branch| { + let base = offered_base(row, &branch.name); + let review = base.as_deref().and_then(|base| { + row.open_review_of(&branch_target(&branch.name, base, default_base)) + .map(ReviewBadge::of) + }); + Item { + key: format!("{key}/pick/{}", branch.name), + repo: None, + kind: ItemKind::Branch { + name: branch.name.clone(), + base, + review, + }, + } + }) + .collect(); + listing(row, key, items, query) +} + +/// The second step's listing: the bases `branch` of the repository at `location` can be +/// compared with (module docs, "Bases"). +pub fn build_bases(rows: &[&RepoRow], location: &Location, branch: &str, query: &str) -> Listing { + let Some((row, key)) = find(rows, location) else { + return Listing::default(); + }; + if !row.is_readable() { + return listing(row, key, Vec::new(), query); + } + let default_base = row.labels.default_base.as_deref(); + let stack_base = row + .branches + .iter() + .find(|candidate| candidate.name == branch) + .and_then(|candidate| candidate.stack_base.as_deref()); + let mut bases: Vec<(&str, Option<&'static str>)> = Vec::new(); + if let Some(stack_base) = stack_base { + bases.push((stack_base, Some("stack base"))); + } + if let Some(default_base) = default_base + && Some(default_base) != stack_base + { + bases.push((default_base, Some("default branch"))); + } + for candidate in &row.branches { + let name = candidate.name.as_str(); + if name != branch && bases.iter().all(|(listed, _)| *listed != name) { + bases.push((name, None)); + } + } + let items = bases + .into_iter() + .map(|(base, note)| { + let target = branch_target(branch, base, default_base); + Item { + key: format!("{key}/base/{base}"), + repo: None, + kind: ItemKind::Base { + offer: TargetOffer { + kind: TargetKind::Branch, + review: row.open_review_of(&target).map(ReviewBadge::of), + target, + label: base.to_string(), + }, + note, + }, + } + }) + .collect(); + listing(row, key, items, query) +} diff --git a/crates/dv_ui/src/switcher/items.rs b/crates/dv_ui/src/switcher/items.rs index 3a11581..dcf2b41 100644 --- a/crates/dv_ui/src/switcher/items.rs +++ b/crates/dv_ui/src/switcher/items.rs @@ -50,7 +50,7 @@ pub struct ReviewBadge { } impl ReviewBadge { - fn of(summary: &ReviewSummary) -> Self { + pub(super) fn of(summary: &ReviewSummary) -> Self { Self { id: summary.id.clone(), status: summary.status, @@ -119,14 +119,33 @@ pub enum ItemKind { Retry { starting: bool }, /// The folder picker, to add a repository. OpenFolder, + /// "Compare branches…": the branch picker over the repository's branches ([`super::branches`]). + CompareBranches, + /// The branch picker's first step: a branch, compared with `base` unless another is picked. + Branch { + name: String, + /// The base it is offered against: its stack base, else the default branch; `None` with + /// neither. + base: Option, + /// The open review of the branch against `base`. + review: Option, + }, + /// The branch picker's second step: a base to compare the picked branch with. + Base { + offer: TargetOffer, + /// Why it is offered first: "stack base" or "default branch". + note: Option<&'static str>, + }, } /// A pickable row. #[derive(Clone, Debug, PartialEq, Eq)] pub struct Item { /// A stable name for the row: `/working`, `/staged`, `/branch`, - /// `/review/`, `/start`, `/retry` or `open_folder`, where `` - /// is the repository's name (its full path when two known repositories share a name). + /// `/review/`, `/start`, `/retry`, `/compare` or + /// `open_folder`, where `` is the repository's name (its full path when two known + /// repositories share a name); in the branch picker `/pick/` and + /// `/base/`. pub key: String, /// Its repository, an index into [`Listing::repos`]. pub repo: Option, @@ -141,6 +160,18 @@ impl Item { ItemKind::Review { label, .. } | ItemKind::Start { label, .. } => label, ItemKind::Retry { .. } => "Retry", ItemKind::OpenFolder => "Open folder…", + ItemKind::CompareBranches => "Compare branches…", + ItemKind::Branch { name, .. } => name, + ItemKind::Base { offer, .. } => &offer.label, + } + } + + /// What a branch picker's row reads after its label, dimmed: "vs feat-a", "stack base". + pub fn detail(&self) -> Option { + match &self.kind { + ItemKind::Branch { base, .. } => base.as_ref().map(|base| format!("vs {base}")), + ItemKind::Base { note, .. } => note.map(str::to_string), + _ => None, } } @@ -158,11 +189,17 @@ impl Item { match &self.kind { ItemKind::Target(offer) => offer.review.as_ref(), ItemKind::Review { badge, .. } => Some(badge), - ItemKind::Start { .. } | ItemKind::Retry { .. } | ItemKind::OpenFolder => None, + ItemKind::Branch { review, .. } => review.as_ref(), + ItemKind::Base { offer, .. } => offer.review.as_ref(), + ItemKind::Start { .. } + | ItemKind::Retry { .. } + | ItemKind::OpenFolder + | ItemKind::CompareBranches => None, } } - /// `"target"`, `"review"`, `"start"`, `"retry"` or `"open_folder"`, for automation state. + /// `"target"`, `"review"`, `"start"`, `"retry"`, `"open_folder"`, `"compare"`, `"branch"` + /// or `"base"`, for automation state. pub fn kind_key(&self) -> &'static str { match self.kind { ItemKind::Target(_) => "target", @@ -170,6 +207,9 @@ impl Item { ItemKind::Start { .. } => "start", ItemKind::Retry { .. } => "retry", ItemKind::OpenFolder => "open_folder", + ItemKind::CompareBranches => "compare", + ItemKind::Branch { .. } => "branch", + ItemKind::Base { .. } => "base", } } } @@ -240,20 +280,32 @@ impl Listing { .position(|entry| *entry == Entry::Item(item)) } - /// What picking `item` opens, with its repository; `None` for "Open folder…". + /// What picking `item` opens, with its repository; `None` for "Open folder…" and the rows + /// that lead to another step of the branch picker. pub fn request(&self, item: usize) -> Option<(Location, OpenRequest)> { let item = self.items.get(item)?; let location = self.repos.get(item.repo?)?.location.clone(); let request = match &item.kind { ItemKind::Target(offer) => OpenRequest::Target(offer.target.clone()), ItemKind::Review { badge, .. } => OpenRequest::Review(badge.id.clone()), - ItemKind::Start { .. } | ItemKind::Retry { .. } | ItemKind::OpenFolder => { + ItemKind::Base { offer, .. } => OpenRequest::Target(offer.target.clone()), + ItemKind::Start { .. } + | ItemKind::Retry { .. } + | ItemKind::OpenFolder + | ItemKind::CompareBranches + | ItemKind::Branch { .. } => { return None; } }; Some((location, request)) } + /// The repository of `item`. + pub fn location_of(&self, item: usize) -> Option<&Location> { + let item = self.items.get(item)?; + Some(&self.repos.get(item.repo?)?.location) + } + /// The repository picking `item` connects to, when it is a **Start** or **Retry** row /// (whose start doesn't run already). pub fn start(&self, item: usize) -> Option { @@ -301,7 +353,7 @@ impl Listing { } /// The `key` of every row of `rows`: their names, or their paths where names repeat. -fn repo_keys(rows: &[&RepoRow]) -> Vec { +pub(super) fn repo_keys(rows: &[&RepoRow]) -> Vec { rows.iter() .map(|row| { let shared = rows.iter().filter(|other| other.name == row.name).count() > 1; @@ -340,20 +392,33 @@ fn summary(row: &RepoRow) -> RepoSummary { } /// Whether every word of `query` is in `haystack` (already lowercase). -fn matches(haystack: &str, words: &[String]) -> bool { +pub(super) fn matches(haystack: &str, words: &[String]) -> bool { words.iter().all(|word| haystack.contains(word.as_str())) } +/// The header of `row`'s repository, keyed `key`. +pub(super) fn header(row: &RepoRow, key: String) -> RepoHeader { + let (distro, shown_path) = match &row.location { + Location::Wsl { distro, path } => (Some(distro.clone()), path.clone()), + Location::Local(_) => (None, row.path.clone()), + }; + RepoHeader { + location: row.location.clone(), + key, + name: row.name.clone(), + path: row.path.clone(), + shown_path, + distro, + summary: summary(row), + } +} + /// The listing for `rows` (most recently used first) under `query`. pub fn build(rows: &[&RepoRow], query: &str) -> Listing { let words: Vec = query.split_whitespace().map(str::to_lowercase).collect(); let keys = repo_keys(rows); let mut listing = Listing::default(); for (row, key) in rows.iter().zip(keys) { - let (distro, shown_path) = match &row.location { - Location::Wsl { distro, path } => (Some(distro.clone()), path.clone()), - Location::Local(_) => (None, row.path.clone()), - }; let repo_text = format!("{} {}", row.name, row.path).to_lowercase(); let mut items: Vec = target_offers(row) .into_iter() @@ -409,6 +474,14 @@ pub fn build(rows: &[&RepoRow], query: &str) -> Listing { }, }); } + // The branch picker, for a repository with a branch to compare (DESIGN §4.1). + if row.is_readable() && super::branches::has_branches(row) { + items.push(Item { + key: format!("{key}/compare"), + repo: None, + kind: ItemKind::CompareBranches, + }); + } items.retain(|item| { let status = item .review() @@ -421,15 +494,7 @@ pub fn build(rows: &[&RepoRow], query: &str) -> Listing { continue; } let repo = listing.repos.len(); - listing.repos.push(RepoHeader { - location: row.location.clone(), - key, - name: row.name.clone(), - path: row.path.clone(), - shown_path, - distro, - summary: summary(row), - }); + listing.repos.push(header(row, key)); listing.entries.push(Entry::Repo(repo)); for mut item in items { item.repo = Some(repo); diff --git a/crates/dv_ui/src/switcher/mod.rs b/crates/dv_ui/src/switcher/mod.rs index 8fe97ef..eef27a6 100644 --- a/crates/dv_ui/src/switcher/mod.rs +++ b/crates/dv_ui/src/switcher/mod.rs @@ -13,11 +13,17 @@ //! recomputes them (it does when the switcher opens, and when a connection a row goes through //! starts reconnecting, stops or fails, or comes back), keeping the selected row. //! +//! A repository's "Compare branches…" row turns the switcher into the branch picker +//! ([`branches`]): its branches, then the bases the picked one can be compared with, each pair +//! opening or resuming its `branch` review. There `Esc` steps back (bases → branches → the +//! repositories, when the picker was entered from them) before it closes. +//! //! [`source`] is the title bar's other menu, the current repository's targets. //! //! Automation targets: `switcher`, `switcher/filter`, `switcher/repo/`, `switcher/` //! for each row ([`items::Item::key`]). +pub mod branches; pub mod items; mod render; pub mod source; @@ -35,7 +41,8 @@ use gpui_component::input::{InputEvent, InputState}; use crate::actions::switcher::{Confirm, Dismiss, SelectNext, SelectPrevious}; use crate::services::reviews::{OpenRequest, Reviews}; -use items::Listing; +use branches::Mode; +use items::{ItemKind, Listing}; /// The panel's width (one-off, like the board's sheets: wide enough for a path and a review). pub const SWITCHER_WIDTH: gpui::Pixels = gpui::px(560.); @@ -64,6 +71,10 @@ pub struct Switcher { reviews: Entity, input: Entity, query: String, + mode: Mode, + /// Whether `Esc` in the branch picker's first step returns to the repositories: it was + /// entered from their "Compare branches…" row, not opened on it (the source picker's). + back_to_repos: bool, listing: Listing, /// Into `listing.items`. selected: usize, @@ -101,6 +112,8 @@ impl Switcher { reviews, input, query: String::new(), + mode: Mode::Repos, + back_to_repos: false, listing: Listing::default(), selected: 0, started: None, @@ -134,6 +147,46 @@ impl Switcher { &self.listing } + pub fn mode(&self) -> &Mode { + &self.mode + } + + /// Opens the branch picker on the repository at `location` (the source picker's "Compare + /// branches…"): `Esc` in its first step then closes the switcher. + pub fn compare_branches( + &mut self, + location: Location, + window: &mut Window, + cx: &mut Context, + ) { + self.back_to_repos = false; + self.set_mode(Mode::Branches { location }, None, window, cx); + } + + /// Shows `mode`'s rows with the filter cleared, the selection on `select` (a row's key) + /// when it is listed, else on the first row. + fn set_mode( + &mut self, + mode: Mode, + select: Option, + window: &mut Window, + cx: &mut Context, + ) { + self.mode = mode; + self.query.clear(); + let placeholder = self.mode.placeholder(); + self.input.update(cx, |input, cx| { + input.set_value("", window, cx); + input.set_placeholder(placeholder, window, cx); + }); + self.listing = self.build(cx); + self.selected = select + .and_then(|key| self.listing.position(&key)) + .unwrap_or(0); + self.scroll_to_selected(); + cx.notify(); + } + /// The selected row's key. pub fn selected_key(&self) -> Option<&str> { self.listing @@ -155,7 +208,14 @@ impl Switcher { fn build(&self, cx: &gpui::App) -> Listing { let reviews = self.reviews.read(cx); - items::build(&reviews.rows(), &self.query) + let rows = reviews.rows(); + match &self.mode { + Mode::Repos => items::build(&rows, &self.query), + Mode::Branches { location } => branches::build_branches(&rows, location, &self.query), + Mode::Bases { location, branch } => { + branches::build_bases(&rows, location, branch, &self.query) + } + } } /// Lists the rows again (the reviews changed, or the filter), keeping the selected row @@ -222,19 +282,48 @@ impl Switcher { } } - fn confirm(&mut self, _: &Confirm, _: &mut Window, cx: &mut Context) { - self.pick(self.selected, cx); + fn confirm(&mut self, _: &Confirm, window: &mut Window, cx: &mut Context) { + self.pick(self.selected, window, cx); } - fn dismiss(&mut self, _: &Dismiss, _: &mut Window, cx: &mut Context) { - cx.emit(SwitcherEvent::Dismiss); + /// `Esc`: a step back in the branch picker (module docs), else closes. + fn dismiss(&mut self, _: &Dismiss, window: &mut Window, cx: &mut Context) { + let repo_key = self.listing.repos.first().map(|repo| repo.key.clone()); + match self.mode.clone() { + Mode::Bases { location, branch } => { + let select = repo_key.map(|key| format!("{key}/pick/{branch}")); + self.set_mode(Mode::Branches { location }, select, window, cx); + } + Mode::Branches { .. } if self.back_to_repos => { + let select = repo_key.map(|key| format!("{key}/compare")); + self.set_mode(Mode::Repos, select, window, cx); + } + Mode::Branches { .. } | Mode::Repos => cx.emit(SwitcherEvent::Dismiss), + } } /// Picks row `ix` (Enter on it, or a click); a start that runs already does nothing. - fn pick(&mut self, ix: usize, cx: &mut Context) { + fn pick(&mut self, ix: usize, window: &mut Window, cx: &mut Context) { let Some(item) = self.listing.items.get(ix) else { return; }; + match &item.kind { + ItemKind::CompareBranches => { + if let Some(location) = self.listing.location_of(ix).cloned() { + self.back_to_repos = true; + self.set_mode(Mode::Branches { location }, None, window, cx); + } + return; + } + ItemKind::Branch { name, .. } => { + let branch = name.clone(); + if let Some(location) = self.listing.location_of(ix).cloned() { + self.set_mode(Mode::Bases { location, branch }, None, window, cx); + } + return; + } + _ => {} + } self.selected = ix; if item.is_starting() { cx.notify(); diff --git a/crates/dv_ui/src/switcher/render.rs b/crates/dv_ui/src/switcher/render.rs index d203172..599fb79 100644 --- a/crates/dv_ui/src/switcher/render.rs +++ b/crates/dv_ui/src/switcher/render.rs @@ -9,6 +9,7 @@ use gpui::{ }; use gpui_component::input::Input; +use super::branches::Mode; use super::items::{Entry, Item, ItemKind, RepoHeader, RepoSummary, TargetKind}; use super::{SWITCHER_WIDTH, Switcher}; use crate::automation::AutomationTargetExt as _; @@ -85,6 +86,25 @@ impl Switcher { .flex() .flex_col() .p(theme.spacing.sm); + let branch_rows = self.listing.items.is_empty() && !self.listing.entries.is_empty(); + if branch_rows && self.mode != Mode::Repos { + let title = if self.query.trim().is_empty() { + "No branches to compare".to_string() + } else { + format!("No branches match “{}”", self.query.trim()) + }; + return list + .child(repo_header(&self.listing.repos[0], true, theme)) + .child( + div() + .flex() + .justify_center() + .py(theme.spacing.xl) + .text_color(c.fg2) + .child(title), + ) + .automation_target("switcher/empty"); + } if self.listing.entries.is_empty() { let known = !self.reviews.read(cx).rows().is_empty(); let (title, note) = if known { @@ -157,6 +177,8 @@ impl Switcher { ItemKind::Start { .. } => DvIcon::Play, ItemKind::Retry { .. } => DvIcon::RotateCw, ItemKind::OpenFolder => DvIcon::FolderOpen, + ItemKind::CompareBranches => DvIcon::ArrowLeftRight, + ItemKind::Branch { .. } | ItemKind::Base { .. } => DvIcon::GitBranch, }; let right: Option = match item.review() { Some(badge) => Some( @@ -180,13 +202,17 @@ impl Switcher { }) .into_any_element(), ), - None if matches!(item.kind, ItemKind::Target(_)) && selected => Some( - div() - .text_spec(&theme.text.secondary) - .text_color(c.fg3) - .child("New review") - .into_any_element(), - ), + None if matches!(item.kind, ItemKind::Target(_) | ItemKind::Base { .. }) + && selected => + { + Some( + div() + .text_spec(&theme.text.secondary) + .text_color(c.fg3) + .child("New review") + .into_any_element(), + ) + } None => None, }; let footer = item.kind == ItemKind::OpenFolder; @@ -218,7 +244,7 @@ impl Switcher { this.hover(move |style| style.bg(hover)) } }) - .on_click(cx.listener(move |this, _, _, cx| this.pick(ix, cx))) + .on_click(cx.listener(move |this, _, window, cx| this.pick(ix, window, cx))) .child(icon(glyph, px(14.), glyph_color)) .child( div() @@ -228,6 +254,13 @@ impl Switcher { .when(inert, |this| this.text_color(c.fg3)) .child(SharedString::from(item.label().to_string())), ) + .children(item.detail().map(|detail| { + div() + .flex_none() + .text_spec(&theme.text.secondary) + .text_color(c.fg3) + .child(SharedString::from(detail)) + })) .child(div().flex_1()) .children(right) .automation_target(format!("switcher/{}", item.key)) @@ -272,8 +305,22 @@ impl Switcher { .text_spec(&theme.text.secondary) .text_color(c.fg3) .child(hint(vec!["up", "down"], "move")) - .child(hint(vec!["enter"], "open")) - .child(hint(vec!["escape"], "close")), + .child(hint( + vec!["enter"], + if matches!(self.mode, Mode::Branches { .. }) { + "next" + } else { + "open" + }, + )) + .child(hint( + vec!["escape"], + match self.mode { + Mode::Repos => "close", + Mode::Branches { .. } if !self.back_to_repos => "close", + _ => "back", + }, + )), ) .into_any_element() } diff --git a/crates/dv_ui/src/switcher/source.rs b/crates/dv_ui/src/switcher/source.rs index af6423e..b7dfeca 100644 --- a/crates/dv_ui/src/switcher/source.rs +++ b/crates/dv_ui/src/switcher/source.rs @@ -1,11 +1,14 @@ //! The source picker (DESIGN §4.1, the title bar's `[source ▾]`): a menu of the open review's //! repository's targets — uncommitted changes, staged changes and the branch against the //! default branch ([`super::items::target_offers`]) — each opening or resuming that target's -//! review. The current review's target is checked. +//! review. The current review's target is checked. Below them, "Compare branches…" opens the +//! switcher's branch picker on the repository ([`super::branches`]) when it has a branch to +//! compare. //! //! It has the keyboard itself while open: `↑`/`↓` (wrapping), `Enter`, `Esc`. //! -//! Automation targets: `source_picker`, `source_picker/`. +//! Automation targets: `source_picker`, `source_picker/`, +//! `source_picker/compare`. use dv_core::{Location, Target}; use gpui::{ @@ -29,7 +32,14 @@ pub const SOURCE_PICKER_WIDTH: gpui::Pixels = gpui::px(340.); /// What was picked. #[derive(Clone, Debug, PartialEq, Eq)] pub enum SourcePickerEvent { - Open { location: Location, target: Target }, + Open { + location: Location, + target: Target, + }, + /// "Compare branches…": the switcher's branch picker on `location`. + CompareBranches { + location: Location, + }, Dismiss, } @@ -42,6 +52,9 @@ pub struct SourcePicker { /// repository's own (one homed in another worktree, PLAN §2.4). current: Option, offers: Vec, + /// Whether "Compare branches…" is offered, after the offers. + compare: bool, + /// Into the offers, then "Compare branches…". selected: usize, _subscription: Subscription, } @@ -70,10 +83,11 @@ impl SourcePicker { location, current, offers: Vec::new(), + compare: false, selected: 0, _subscription: subscription, }; - this.offers = this.build(cx); + (this.offers, this.compare) = this.build(cx); this.selected = this .offers .iter() @@ -90,35 +104,58 @@ impl SourcePicker { self.offers.get(self.selected) } - fn build(&self, cx: &gpui::App) -> Vec { - self.reviews - .read(cx) - .row(&self.location) - .map(target_offers) - .unwrap_or_default() + /// Whether "Compare branches…" is offered. + pub fn offers_compare(&self) -> bool { + self.compare + } + + /// Whether "Compare branches…" is selected. + pub fn compare_selected(&self) -> bool { + self.compare && self.selected == self.offers.len() + } + + /// The rows: the offers, then "Compare branches…" when offered. + fn len(&self) -> usize { + self.offers.len() + usize::from(self.compare) + } + + fn build(&self, cx: &gpui::App) -> (Vec, bool) { + let reviews = self.reviews.read(cx); + let Some(row) = reviews.row(&self.location) else { + return (Vec::new(), false); + }; + let offers = target_offers(row); + let compare = !offers.is_empty() && super::branches::has_branches(row); + (offers, compare) } fn rebuild(&mut self, cx: &mut Context) { + let compare_selected = self.compare_selected(); let keep = self.selected().map(|offer| offer.kind); - let offers = self.build(cx); - if offers != self.offers { + let (offers, compare) = self.build(cx); + if offers != self.offers || compare != self.compare { self.offers = offers; - self.selected = keep - .and_then(|kind| self.offers.iter().position(|offer| offer.kind == kind)) - .unwrap_or(0); + self.compare = compare; + self.selected = if compare_selected && compare { + self.offers.len() + } else { + keep.and_then(|kind| self.offers.iter().position(|offer| offer.kind == kind)) + .unwrap_or(0) + }; cx.notify(); } } fn select_next(&mut self, _: &SelectNext, _: &mut Window, cx: &mut Context) { - if !self.offers.is_empty() { - self.selected = (self.selected + 1) % self.offers.len(); + let len = self.len(); + if len > 0 { + self.selected = (self.selected + 1) % len; cx.notify(); } } fn select_previous(&mut self, _: &SelectPrevious, _: &mut Window, cx: &mut Context) { - let len = self.offers.len(); + let len = self.len(); if len > 0 { self.selected = (self.selected + len - 1) % len; cx.notify(); @@ -134,6 +171,13 @@ impl SourcePicker { } fn pick(&mut self, ix: usize, cx: &mut Context) { + if self.compare && ix == self.offers.len() { + self.selected = ix; + cx.emit(SourcePickerEvent::CompareBranches { + location: self.location.clone(), + }); + return; + } if let Some(offer) = self.offers.get(ix) { self.selected = ix; cx.emit(SourcePickerEvent::Open { @@ -193,19 +237,62 @@ impl SourcePicker { })) .automation_target(format!("source_picker/{}", offer.kind.key())) } + + /// "Compare branches…", after the offers (module docs). + fn render_compare(&self, theme: &DvTheme, cx: &mut Context) -> AnyElement { + let c = &theme.colors; + let ix = self.offers.len(); + let selected = self.selected == ix; + let hover = c.control_hover; + div() + .id("source-compare") + .flex() + .items_center() + .gap(theme.spacing.md) + .h(px(30.)) + .px(theme.spacing.md) + .rounded(theme.radii.control) + .cursor_pointer() + .map(|this| { + if selected { + this.bg(c.sel) + } else { + this.hover(move |style| style.bg(hover)) + } + }) + .on_click(cx.listener(move |this, _, _, cx| this.pick(ix, cx))) + .child( + div() + .flex_none() + .w(px(14.)) + .child(icon(DvIcon::ArrowLeftRight, px(13.), c.fg2)), + ) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .child("Compare branches…"), + ) + .automation_target("source_picker/compare") + .into_any_element() + } } impl Render for SourcePicker { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let theme = cx.dv_theme().clone(); let c = &theme.colors; - let rows: Vec = self + let mut rows: Vec = self .offers .clone() .iter() .enumerate() .map(|(ix, offer)| self.render_offer(ix, offer, &theme, cx)) .collect(); + if self.compare { + rows.push(self.render_compare(&theme, cx)); + } div() .key_context(keymap::SOURCE_PICKER) .track_focus(&self.focus_handle) diff --git a/crates/dv_ui/src/switcher/tests.rs b/crates/dv_ui/src/switcher/tests.rs index 9f78da7..2770d68 100644 --- a/crates/dv_ui/src/switcher/tests.rs +++ b/crates/dv_ui/src/switcher/tests.rs @@ -3,12 +3,15 @@ use dv_core::{Location, Target}; use gpui::{Entity, TestAppContext, VisualTestContext}; use pretty_assertions::assert_eq; -use super::items::{Entry, RepoSummary, build}; +use super::branches::{Mode, build_bases, build_branches, has_branches}; +use super::items::{Entry, ItemKind, RepoSummary, build}; use super::*; use crate::failure::Failure; use crate::services::reviews::{RepoRow, ReviewRow, WorkingSummary}; use crate::shell::Shell; use crate::test_support::{add_repo, fixture_at, open_shell, settle}; +use dv_core::Oid; +use dv_core::git::branches::Branch; use dv_core::label::LabelContext; fn row(name: &str, path: &str) -> RepoRow { @@ -35,6 +38,7 @@ fn row(name: &str, path: &str) -> RepoRow { branch: Some("feat".into()), default_base: Some("main".into()), }, + branches: Vec::new(), last_used: 0, } } @@ -511,7 +515,11 @@ fn the_source_picker_switches_the_target_by_keyboard(cx: &mut TestAppContext) { Some("working") ); }); + // "Compare branches…" follows the targets: the repository has a branch to compare. + assert!(picker.read_with(cx, |picker, _| picker.offers_compare())); cx.simulate_keystrokes("down down down"); + assert!(picker.read_with(cx, |picker, _| picker.compare_selected())); + cx.simulate_keystrokes("down"); picker.read_with(cx, |picker, _| { assert_eq!( picker.selected().map(|offer| offer.kind.key()), @@ -519,7 +527,7 @@ fn the_source_picker_switches_the_target_by_keyboard(cx: &mut TestAppContext) { "wraps" ) }); - cx.simulate_keystrokes("up enter"); + cx.simulate_keystrokes("up up enter"); settle(cx, &shell, "the branch review", |shell, cx| { shell.session().is_some_and(|session| { matches!(session.read(cx).review().target, Target::Branch { .. }) @@ -758,3 +766,292 @@ fn an_open_switcher_follows_a_connection_going_down_and_coming_back(cx: &mut Tes }); assert_eq!(connection.state(), ConnectionState::Connected); } + +fn branch(name: &str, stack_base: Option<&str>) -> Branch { + Branch { + name: name.to_string(), + tip: Oid::parse(&"a".repeat(40)).unwrap(), + committed_at: 0, + stack_base: stack_base.map(str::to_string), + } +} + +/// A repository with the stack `main → feat-a → feat-b` and an unrelated `side`. +fn stacked_row() -> RepoRow { + let mut api = row("api-server", "D:\\code\\api-server"); + api.branches = vec![ + branch("feat-b", Some("feat-a")), + branch("side", None), + branch("feat-a", None), + branch("main", None), + ]; + api +} + +fn details(listing: &items::Listing) -> Vec<(&str, Option)> { + listing + .items + .iter() + .map(|item| (item.key.as_str(), item.detail())) + .collect() +} + +#[test] +fn a_repo_with_branches_offers_compare_branches_last() { + let api = stacked_row(); + let listing = build(&[&api], ""); + assert_eq!( + keys(&listing), + [ + "api-server/working", + "api-server/staged", + "api-server/branch", + "api-server/compare", + "open_folder", + ] + ); + assert_eq!(listing.items[3].label(), "Compare branches…"); + assert_eq!(listing.request(3), None, "it leads to the branch picker"); + // Only the default branch: nothing to compare. + let mut lone = row("web", "D:\\code\\web"); + lone.branches = vec![branch("main", None)]; + assert!(!has_branches(&lone)); + assert!(!keys(&build(&[&lone], "")).contains(&"web/compare")); +} + +#[test] +fn the_branch_picker_offers_each_branch_against_its_stack_base() { + let mut api = stacked_row(); + let stacked = summary( + Target::Branch { + branch: "feat-b".into(), + base: Some("feat-a".into()), + }, + "D:\\code\\api-server", + ReviewStatus::ChangesRequested, + ); + api.reviews.push(ReviewRow::Ok { + summary: Box::new(stacked.clone()), + label: "feat-b vs feat-a".into(), + }); + let listing = build_branches(&[&api], &api.location, ""); + // Newest first, the default branch left out. + assert_eq!( + details(&listing), + [ + ("api-server/pick/feat-b", Some("vs feat-a".to_string())), + ("api-server/pick/side", Some("vs main".to_string())), + ("api-server/pick/feat-a", Some("vs main".to_string())), + ] + ); + assert_eq!( + listing.items[0].review().map(|badge| &badge.id), + Some(&stacked.id) + ); + assert_eq!(listing.items[1].review(), None); + assert_eq!(listing.request(0), None, "a branch leads to its bases"); + assert_eq!(listing.repos.len(), 1); + // The filter keeps the branches holding every word. + assert_eq!( + keys(&build_branches(&[&api], &api.location, "feat")), + ["api-server/pick/feat-b", "api-server/pick/feat-a"] + ); +} + +#[test] +fn the_bases_list_the_stack_base_then_the_default_branch_then_the_rest() { + let api = stacked_row(); + let listing = build_bases(&[&api], &api.location, "feat-b", ""); + assert_eq!( + details(&listing), + [ + ("api-server/base/feat-a", Some("stack base".to_string())), + ("api-server/base/main", Some("default branch".to_string())), + ("api-server/base/side", None), + ] + ); + // Against its stack base the target names the base; against the default branch it is the + // target with no base, the switcher's own Branch row's and `dv review new --branch`'s. + assert_eq!( + listing.request(0), + Some(( + api.location.clone(), + OpenRequest::Target(Target::Branch { + branch: "feat-b".into(), + base: Some("feat-a".into()), + }) + )) + ); + assert_eq!( + listing.request(1), + Some(( + api.location.clone(), + OpenRequest::Target(Target::Branch { + branch: "feat-b".into(), + base: None, + }) + )) + ); + // A branch with no stack base starts with the default branch. + let listing = build_bases(&[&api], &api.location, "feat-a", ""); + assert_eq!( + keys(&listing), + [ + "api-server/base/main", + "api-server/base/feat-b", + "api-server/base/side" + ] + ); + assert!(matches!( + listing.items[0].kind, + ItemKind::Base { + note: Some("default branch"), + .. + } + )); +} + +/// A repository with the stack `main → feat-a → feat-b`, `feat-b` checked out. +fn stacked_fixture(path: &std::path::Path) -> dv_test_support::FixtureRepo { + let repo = fixture_at(path); + repo.commit("wip"); + repo.branch("feat-a") + .checkout("feat-a") + .write("a.txt", "a\n"); + repo.commit("a1"); + repo.branch("feat-b") + .checkout("feat-b") + .write("b.txt", "b\n"); + repo.commit("b1"); + repo +} + +fn mode(shell: &Entity, cx: &mut VisualTestContext) -> Option { + let switcher = switcher(shell, cx)?; + switcher.read_with(cx, |switcher, _| Some(switcher.mode().clone())) +} + +#[gpui::test] +fn the_branch_picker_opens_a_stacked_branch_against_its_base(cx: &mut TestAppContext) { + let (_dir, shell, cx) = open_shell(cx); + let repos = tempfile::tempdir().unwrap(); + let api = stacked_fixture(&repos.path().join("api-server")); + add_repo(&api, &shell, cx); + let location = Location::Local(api.path().to_path_buf()); + + cx.simulate_keystrokes("secondary-o"); + settle(cx, &shell, "the rows", |_, _| true); + let view = switcher(&shell, cx).expect("Ctrl O opens the switcher"); + assert!(view.update(cx, |view, cx| view.select_key("api-server/compare", cx))); + + // Enter on "Compare branches…" lists the branches, the checked-out one stacked on feat-a. + cx.simulate_keystrokes("enter"); + assert_eq!( + mode(&shell, cx), + Some(Mode::Branches { + location: location.clone() + }) + ); + view.read_with(cx, |view, _| { + assert_eq!( + details(view.listing()), + [ + ("api-server/pick/feat-b", Some("vs feat-a".to_string())), + ("api-server/pick/feat-a", Some("vs main".to_string())), + ] + ); + }); + assert_eq!( + selected(&shell, cx).as_deref(), + Some("api-server/pick/feat-b") + ); + + // Enter lists its bases, the stack base first; Esc steps back to the branches, and again + // to the repositories, where "Compare branches…" keeps the selection. + cx.simulate_keystrokes("enter"); + assert_eq!( + mode(&shell, cx), + Some(Mode::Bases { + location: location.clone(), + branch: "feat-b".into() + }) + ); + assert_eq!( + selected(&shell, cx).as_deref(), + Some("api-server/base/feat-a") + ); + cx.simulate_keystrokes("escape"); + assert_eq!( + selected(&shell, cx).as_deref(), + Some("api-server/pick/feat-b") + ); + cx.simulate_keystrokes("escape"); + assert_eq!(mode(&shell, cx), Some(Mode::Repos)); + assert_eq!(selected(&shell, cx).as_deref(), Some("api-server/compare")); + + // Into the picker again, the filter narrows the branches, and the pair opens. + cx.simulate_keystrokes("enter"); + cx.simulate_input("feat-b"); + settle(cx, &shell, "the filter", |_, _| true); + cx.simulate_keystrokes("enter enter"); + settle(cx, &shell, "the stacked review", |shell, _| { + shell.session().is_some() + }); + shell.read_with(cx, |shell, cx| { + assert!(shell.switcher().is_none()); + let session = shell.session().unwrap().read(cx); + assert_eq!( + session.review().target, + Target::Branch { + branch: "feat-b".into(), + base: Some("feat-a".into()), + } + ); + assert_eq!(session.target_label(), "feat-b vs feat-a"); + // Only feat-b's own commit, not feat-a's. + let files: Vec<&str> = session + .target_files() + .iter() + .map(|entry| entry.path.as_str()) + .collect(); + assert_eq!(files, ["b.txt"]); + }); +} + +#[gpui::test] +fn the_source_picker_opens_the_branch_picker_and_esc_closes_it(cx: &mut TestAppContext) { + let (_dir, shell, cx) = open_shell(cx); + let repos = tempfile::tempdir().unwrap(); + let api = stacked_fixture(&repos.path().join("api-server")); + add_repo(&api, &shell, cx); + let location = Location::Local(api.path().to_path_buf()); + let task = cx.update(|window, cx| { + shell.update(cx, |shell, cx| { + shell.open( + location.clone(), + OpenRequest::Target(Target::Working), + window, + cx, + ) + }) + }); + cx.run_until_parked(); + settle(cx, &shell, "the review", |shell, _| { + shell.session().is_some() + }); + cx.foreground_executor().block_test(task).unwrap(); + + cx.update(|window, cx| shell.update(cx, |shell, cx| shell.open_source_picker(window, cx))); + settle(cx, &shell, "the repo's row", |_, _| true); + cx.simulate_keystrokes("up enter"); + assert!(shell.read_with(cx, |shell, _| shell.source_picker().is_none())); + assert_eq!( + mode(&shell, cx), + Some(Mode::Branches { + location: location.clone() + }) + ); + // Opened on the picker, Esc in its first step closes the switcher. + cx.simulate_keystrokes("escape"); + assert!(switcher(&shell, cx).is_none()); +} diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 8329e6c..2026c30 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -201,8 +201,22 @@ inactive: material falls back to its inactive appearance (OS-managed); dv dims n whether or not the switcher is open, so it never opens on a stale Start or "reconnecting…". Then its rows (30 high, indented): **Uncommitted changes**, **Staged changes** and, with a branch other than the default checked out, **Branch `feat` vs `main`**, each beside its open review (status chip, "round n", open-comment badge; "New review" on the selected row - without one), then the repo's other open reviews by their target label. Picking a row opens or - resumes that review (PLAN §2.4). A repo that can't be opened offers no rows. The footer holds **Open + without one), then the repo's other open reviews by their target label, then, when it has a branch + other than the default one, **Compare branches…** (arrows glyph). Picking a row opens or + resumes that review (PLAN §2.4). A repo that can't be opened offers no rows. + **Compare branches…** turns the switcher into the branch picker for that repo, in the same panel: + the field cleared, its placeholder "Pick a branch to review", the repo's header over one row per + local branch but the default one (branch glyph, the most recently committed first): the name, then + "vs feat-a" in `fg3`, the base it is offered against (its stack base, PLAN §2.2, else the default + branch), and that pair's open review as on the switcher's rows. `Enter` on one lists its bases, + the placeholder "Compare feat-b with…": the stack base first ("stack base" in `fg3`), then the + default branch ("default branch"), then every other local branch; `Enter` on one opens or resumes + the `branch` review of the pair ("New review" on the selected row without one), against the + default branch the target with no base, the switcher's own **Branch** row's. Typing filters + either step by name; `Esc` steps back (bases → branches → the repos, the selection back on the row + picked) and closes from the branches when the picker was opened from the source picker. The + footer's hints read "next" for `Enter` on the branches and "back" for `Esc` where it steps back. With + nothing to list it reads "No branches to compare" ("No branches match “…”" for a filter). The footer holds **Open folder…** (the system's folder picker; the folder's repo is added and selected) and the keys. Opened over the "Finish round" sheet (§4.2), it and the source picker hang above the sheet, which the scrim then covers too; closing them returns to the sheet as it was. Status @@ -210,8 +224,9 @@ inactive: material falls back to its inactive appearance (OS-managed); dv dims n working": the agent's first reply, address or comment after changes were requested, PLAN §2.4) blue, `approved`/`submitted` green, `closed` grey; always worded. - **Source picker** (`⇄ ▾`): a 340-wide menu under its button with the open review's repo's targets - (the switcher's three rows) and their open reviews; the current target is checked. `↑`/`↓`/`Enter`/ - `Esc`; picking one opens or resumes its review. + (the switcher's three rows) and their open reviews; the current target is checked; then, when the + repo has a branch to compare, **Compare branches…**, which opens the switcher as the branch picker + on that repo (above). `↑`/`↓`/`Enter`/`Esc`; picking one opens or resumes its review. - **Panel:** "Changes", the file count and the review's `+n −m`; under it, while the review offers it (a live target — `working`, `staged`, a branch — not finished, with a closed round; or a draft PR review whose PR has an earlier submitted review, reading **Since your last review**; diff --git a/docs/PLAN.md b/docs/PLAN.md index a8d1376..0c05bc8 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -241,6 +241,17 @@ local `main`, else `master`. `` is the ref its name stands for however it is `heads/feat`, `refs/heads/feat`; a local branch before a tag of the same name, as `git switch` picks), and that one ref gives both the tip and the checked-out test. Unrelated histories (no merge base) are an error, not an empty diff. +**Branches and stack bases** (`dv_core::git::branches`): the switcher's branch picker (DESIGN §4.1) +offers every local branch against its *stack base*, the nearest branch tip among its first-parent +ancestors outside the default branch's history (`main → feat-a → feat-b`: `feat-b`'s is `feat-a`), +else against the default branch, and lets the reviewer pick any other base. One `git log --branches +--simplify-by-decoration ^` finds every branch's: it keeps only commits a branch points at, +outside the default branch, and rewrites each one's parents to its nearest such commit, so a branch's +first rewritten parent is its stack base's tip; a parent left out means the default branch, and a +branch merged into the default one has none. It and `git for-each-ref refs/heads/` run with every +listing of a repo's row (§11), two commands whatever the number of branches. A suggestion only: a +lower branch rewritten without restacking the ones above it leaves them stacked on its old commits, +which the walk can't name, so they are offered against the default branch. `dv_core::target::Target::resolve` turns a target into a `DiffSpec` of two sides — a commit id, the empty tree, the index or the working tree. A target stores its spec *and* the resolved oids at each round, so a review never silently changes meaning (the first implementation froze `base` but left @@ -2053,7 +2064,8 @@ canonical paths in that open's background job. A distro is named as WSL spells i `\\wsl.localhost\Ubuntu\…` are one repo through one connection, which is named from `wsl.exe --list` — an explicit open of a distro dv hasn't seen lists WSL's distros first, off the UI thread — so every location it hands back is spelled WSL's way), `Reviews` (the known repos and, per repo, its -reviews (`ReviewStore::list`, an unreadable log a row of its own) and its uncommitted changes, +reviews (`ReviewStore::list`, an unreadable log a row of its own), its uncommitted changes and its +local branches with their stack bases (§2.2, for the branch picker), computed in the background when a repo is added, when the switcher opens, and when a connection a row goes through goes down or comes back, whether or not the switcher is open (one that comes back clears what its rows said about it at once), through `Locations::reach`: a diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 28f12e8..25e5273 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -225,6 +225,7 @@ Ubuntu, from the Linux GUI target dir (`~/.cache/dv-target-gui`), whose | `review_session` | the review plumbing without its screens: a seeded repo is `open`ed and listed with its uncommitted changes, its working review opens (named in full at once) and resumes, follows the agent's edits and commit live, and sees the agent's `review_op` through the watch; a WSL location is listed as unavailable (WSL's stand-in refuses to start anything); repos, review and seeded fixture survive a `restart` | | `setup` | Windows only. Setup (PLAN §12, DESIGN §4.3) on a fresh data dir with `DV_FIRST_RUN=1` and `mock_gh` signed in (`tests/fixtures/gh/auth_status.json`): the first-run screen over this machine's checks; a fake Claude Code folder (`home_file`), Check again, its skill's Install; Add to PATH writing the user `Path`'s stand-in; the git command copied through the clipboard seam; WSL's stand-in naming Ubuntu (listed, not reached), Debian (stopped; Start refused: the run reaches no distro but its sandbox's, why under its name) and `docker-desktop` and `podman-machine-default` (container tools' VMs, not listed); Continue (`Enter`) writing the marker and a review opening; Settings (`Ctrl ,`) showing a skill edited since as out of date and Update fixing it, `Esc` closing it; after a `restart`, no first run and the status bar's notice "dv-review skill out of date on Windows", whose Update fixes it; screenshots of the first-run screen, with the distros, Settings and the notice | | `setup_linux` | Linux only, `setup`'s sibling: the first-run screen over this machine's checks alone (no distros on Linux; the header empties `WSL_DISTRO_NAME`, so a run inside WSL names the machine `Linux` as a desktop would), in the scratch home with `mock_gh` signed in; a fake Claude Code folder, Check again, Install; Add to PATH linking `~/.local/bin/dv` to this dv and putting `~/.local/bin` on the shell's `PATH` through its startup file, the shell then asked again with the scratch home; the git command copied through the clipboard seam; Continue and a review opening; after a `restart` the notice "dv-review skill out of date on Linux" and its Update; screenshots of the first-run screen (dark, and set up in dark and light) and the notice | +| `switcher_branches` | the branch picker (DESIGN §4.1 "Compare branches…", PLAN §2.2): a stacked repo (`main → feat-a → feat-b`, and `side` off `main`) offers **Compare branches…** after its targets; it lists the branches newest first, each against its stack base (`feat-b` vs `feat-a`, the others vs `main`), a click on `feat-b` lists its bases (`feat-a` "stack base", `main` "default branch", then `side`), `Esc` steps back with the selection on `feat-b`, and `Enter` twice opens `feat-b` vs `feat-a` with only `b.txt`; the source picker's **Compare branches…** opens the picker on the repo, the branch's open review beside it, and `Esc` closes it; screenshots of the branches and the bases in dark, the stacked review, and the branches in light | | `update` | dv's own update (PLAN D9, DESIGN §4.1): nothing shows in an automation run (it checks no feed); `update_ready` through the seam shows "dv is ready" and Restart to update at the status bar's right beside an open review; with two other dv processes running from the install (the stand-in's `others`) its click opens the confirmation (`update.confirm`, `status/update/confirm`) and Not now keeps everything; a restart the stand-in fails reads "Update failed · details" (`update.failure`, its tooltip) with Retry; with an unsent comment (`J`, `C`, typed) the confirmation names it and Restart anyway records the restart (`update.restart_requested`) while dv keeps running, the draft intact; screenshots of the item in dark and light, both confirmations and the failure | | `backend_loopback` | a repo reached through the loopback `dv backend --stdio` (this `dv`'s own server, a child of the app), as a WSL repo is through its distro's (PLAN §5.2): `seed_repo` with `"backend": "loopback"`, its review opening and a comment posting through the connection (`state.backends`), the remote watch following the agent's edit, a `backend_kill` that reconnects by itself (the status bar's "loopback · reconnecting…", no banner) and a file written while it was down listed after the synthetic rescan, then `backend_block` and a kill that runs out of respawns (attempts 2 and 3: a death soon after a reconnect continues its count): `failed`, "loopback · disconnected" and the backend-down banner (Details, Copy diagnostics); meanwhile the open switcher's row reads "reconnecting…" keeping its rows, then offers **Retry** once the connection failed; unblocked, the banner's Retry connects again and the watch follows the agent once more; then `backend_stop` (the host stopped, as `wsl --shutdown` stops a distro): the respawn starts nothing, `stopped`, "loopback · stopped", the banner's **Start loopback** and the switcher's Start row, and the banner's Start boots it again; stopped once more, the switcher's Start row started with the start held (`backend_hold`): the row stays, inert (`starting`), with the selection, and `Enter` again does nothing, until the repo lists and its first row takes the selection; a review picked from the switcher while the connection reconnects opens once it is back, with no banner; one picked while the reconnect then fails waits as `shell.pending` (the backend banner the only banner, `shell.banner` `null`); with a round summary typed in the sheet meanwhile it waits on after the switcher's Retry row brings the connection back (the review under the sheet stays, the summary kept), and opens by itself once the sheet is cancelled, no banner left; and after a `restart` with `app_args: ["open", ]` the repo is reached through the loopback again, where a review picked with no review open and its server killed at once (under the open) opens once the connection is back (generation 2, no banner); screenshots of the review over the loopback, while reconnecting, the switcher reconnecting and offering Retry, the banner and status bar in dark Mica, dark Opaque and light, the review reconnected, the stopped banner and switcher row, the switcher while its Start runs, and the pick waiting under the backend banner and then for the sheet | | `review_screen` | the diff stream (DESIGN §4.1, §5): the board's `api-server` with a CRLF README under `core.autocrlf`, a binary, a file too large to show, a rename, an untracked file, a deletion and a long file with gaps, opened from the switcher by keyboard; `J`/`K`, `N`/`P`, the cursor (`↑`/`↓`, `←`/`→`), a `Shift` selection and `Esc`, `S` to unified and back, gap rows and `E`/`Shift E`, `PageDown` with the pinned header, the agent staging everything under the reader (the gaps stay open, the place stays), the sideways scroll, line-number clicks and shift-clicks, the file menu's Copy path, `V` collapsing a viewed file and its chevron, "Load anyway"; screenshots of each state in dark Mica, dark Opaque and light | @@ -280,8 +281,8 @@ assert on them rather than on screenshots, which are for style. | `system.requests` | what the app asked the OS to do, oldest first, recorded instead of performed: `{"kind": "open_url", "url"}`, `{"kind": "reveal", "path"}`, `{"kind": "pick_folder", "prompt"}`, `{"kind": "clipboard", "text"}` (nothing reaches the real clipboard) | | `system.answers_left` | folder-picker answers still queued | | `repos` | the known repositories (`/known_repos.json`), most recently used first: `{path, name, location, loading, error, stopped, retry, reconnecting, starting, working, reviews}`. `location` is the stored shape (`{"local": "D:\\…"}`, `{"wsl": {"distro", "path"}}`); `error` is `null` or `{message, detail}` (a WSL location's says why it can't be opened: `"Ubuntu isn't running"`, or its connection's failure, kept when the distro still isn't running after a failed start); `stopped` is the distro (or connection) of a repo whose host isn't running, whatever its connection did before (its row offers **Start**), else `null`; `retry` whether its connection failed with its host running (its row offers **Retry**); `reconnecting` whether its connection is reconnecting (the row keeps `working` and `reviews` from its last listing); `starting` whether a Start or Retry is connecting; `working` is `{files, additions, deletions}` of the uncommitted changes, `null` until computed (on `open`, and when the switcher opens) or when it can't be; `reviews` newest first, each `{id, target, target_label, status, round, threads: {open, addressed, resolved, annotation}}`, or `{id, error}` for a log that doesn't load | -| `switcher` | the switcher (`Ctrl O`): `{open, query, selected, items}`; closed it is `{open: false, query: null, selected: null, items: []}`. `items` are its pickable rows in order, each `{key, kind, target, label, starting, repo, review}`: `key` names the row (`/working`, `/staged`, `/branch`, `/review/`, `/start`, `/retry`, `open_folder`; `` is the repo's name, its full path when two known repos share a name), `kind` is `"target"`, `"review"`, `"start"` (**Start Ubuntu**, a stopped distro's one row), `"retry"` (**Retry**, a failed connection's one row) or `"open_folder"`, `starting` whether it is a Start or Retry whose start runs (drawn inert; picking it does nothing), `target` the target row's `"working"`/`"staged"`/`"branch"` (else `null`), `repo` its path, `review` its open review `{id, status, round, open_threads}` or `null`; `selected` is a `key` | -| `source_picker` | the source picker (the title bar's `⇄ ▾`): `{open, selected, items: [{kind, label, review}]}`, `selected` a `kind` | +| `switcher` | the switcher (`Ctrl O`): `{open, mode, query, selected, items}`; closed it is `{open: false, mode: null, query: null, selected: null, items: []}`. `mode` is `"repos"`, or the branch picker's `"branches"` (a repo's branches) and `"bases"` (the bases the picked branch can be compared with, DESIGN §4.1). `items` are its pickable rows in order, each `{key, kind, target, label, detail, starting, repo, review}`: `key` names the row (`/working`, `/staged`, `/branch`, `/review/`, `/start`, `/retry`, `/compare`, `open_folder`; in the branch picker `/pick/` and `/base/`; `` is the repo's name, its full path when two known repos share a name), `kind` is `"target"`, `"review"`, `"start"` (**Start Ubuntu**, a stopped distro's one row), `"retry"` (**Retry**, a failed connection's one row), `"open_folder"`, `"compare"` (**Compare branches…**), `"branch"` or `"base"`, `starting` whether it is a Start or Retry whose start runs (drawn inert; picking it does nothing), `target` the target or base row's `"working"`/`"staged"`/`"branch"` (else `null`), `detail` a branch row's `"vs "` or a base row's `"stack base"` / `"default branch"` (else `null`), `repo` its path, `review` its open review `{id, status, round, open_threads}` or `null`; `selected` is a `key` | +| `source_picker` | the source picker (the title bar's `⇄ ▾`): `{open, selected, items: [{kind, label, review}], compare}`, `selected` a `kind` or `"compare"`, `compare` whether it offers **Compare branches…** (`source_picker/compare`) after the items | | `file_list` | the panel's file list: `{query, shown, current_file}`; `shown` the paths the filter leaves, in list order; `current_file` the stream's current file (the highlighted row) | | `session` | the open review, or `null`: `review_id`, `repo` (the root of the review's home worktree, where its target resolves: PLAN §2.2; another worktree's than the row it was opened from for a working review made in a linked worktree, or a branch review of a branch checked out in one), `target` (stored shape: `"working"`, `"staged"`, `{"branch": {…}}`, …), `target_label` (`"working tree vs main"`), `status`, `round` (the one in progress; for a finished review the one that finished it, as the status bar says), `read_only` (approved, closed or submitted), `loading` (the file list is loading), `error` (`null` or `{message, detail}`), `viewed_error` (why the last viewed toggle failed, shown as `banner/viewed`; `null` once one lands or it is dismissed), `watch_notice` (why the worktree isn't watched, or `null`), `generation` (the load generation), `warnings` (problems in the log), `viewed` / `total` (files viewed this round / listed), `current_file` (the stream's current file: the cursor's while the cursor is on screen, else the file at the top), `view_mode` (`"split"` or `"unified"`, remembered per review in `/review_ui.json`), `pane` (`"stream"` or `"file_view"`: the pane `cursor`, `selection`, `composer` and `focused_thread` describe, the one the main area shows), `cursor` (`null` or `{path, side, line}`: `side` `"old"`/`"new"`, `line` its number on that side), `selection` (`null` or `{path, side, start_line, end_line}`, one file and one side) | | `session.files` | sorted by path: `{path, old_path, status, additions, deletions, binary, viewed, open_threads, state, kind, highlighted}`; `status` is `added`, `modified`, `deleted`, `renamed`, `copied`, `type_changed`, `untracked` or `unmerged`; `state` is `unloaded`, `loading`, `loaded` or `failed` (diffs load when the stream asks for them); `kind` is the loaded diff's `text`, `binary`, `too_large`, `submodule` or `unmerged`, else `null`; `highlighted` once syntax kinds landed | diff --git a/tests/e2e/review_chrome.jsonl b/tests/e2e/review_chrome.jsonl index 4950b0c..be1c33b 100644 --- a/tests/e2e/review_chrome.jsonl +++ b/tests/e2e/review_chrome.jsonl @@ -11,7 +11,7 @@ {"cmd": "open", "path": "${repo}", "expect_result": {"pointer": "/path", "equals": "${repo}"}} {"cmd": "wait_for", "pointer": "/repos/0/working/files", "equals": 7, "timeout_ms": 20000} {"cmd": "settle", "timeout_ms": 20000} -{"expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/query", "equals": ""}, {"pointer": "/switcher/selected", "equals": "api-server/working"}, {"pointer": "/switcher/items/0", "equals": {"key": "api-server/working", "kind": "target", "target": "working", "label": "Uncommitted changes", "starting": false, "repo": "${repo}", "review": null}}, {"pointer": "/switcher/items/1/key", "equals": "api-server/staged"}, {"pointer": "/switcher/items/2/key", "equals": "open_folder"}, {"pointer": "/repos/0/working", "equals": {"files": 7, "additions": 34, "deletions": 6}}]} +{"expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/query", "equals": ""}, {"pointer": "/switcher/selected", "equals": "api-server/working"}, {"pointer": "/switcher/items/0", "equals": {"key": "api-server/working", "kind": "target", "target": "working", "label": "Uncommitted changes", "detail": null, "starting": false, "repo": "${repo}", "review": null}}, {"pointer": "/switcher/items/1/key", "equals": "api-server/staged"}, {"pointer": "/switcher/items/2/key", "equals": "open_folder"}, {"pointer": "/repos/0/working", "equals": {"files": 7, "additions": 34, "deletions": 6}}]} {"cmd": "bounds", "target": "switcher/repo/api-server", "expect_result": {"pointer": "/visible", "not_equals": null}} {"cmd": "bounds", "target": "switcher", "expect_result": [{"pointer": "/w", "equals": 560}, {"pointer": "/y", "gte": 40}, {"pointer": "/y", "lte": 46}]} {"cmd": "screenshot", "name": "switcher-dark-mica"} diff --git a/tests/e2e/switcher_branches.jsonl b/tests/e2e/switcher_branches.jsonl new file mode 100644 index 0000000..1643a2f --- /dev/null +++ b/tests/e2e/switcher_branches.jsonl @@ -0,0 +1,37 @@ +{"scenario": {"window_size": "1440x900", "description": "The branch picker (DESIGN §4.1 \"Compare branches…\"): a stacked repo's switcher row \"Compare branches…\" lists its branches, each against its stack base (`feat-b` vs `feat-a`), then the picked branch's bases (stack base first, then the default branch); Esc steps back, Enter on a base opens the stacked `branch` review with only that branch's own changes, and the source picker's \"Compare branches…\" opens the picker again, Esc closing it; screenshots of both steps in dark and the branches in light."}} +{"cmd": "wait_ready", "timeout_ms": 20000} +{"cmd": "set_setting", "key": "mode", "value": "dark"} +{"cmd": "settle"} +{"cmd": "seed_repo", "name": "stack", "steps": [{"write": "README.md", "text": "# stack\n"}, {"commit": "base"}, {"branch": "feat-a"}, {"checkout": "feat-a"}, {"write": "a.txt", "text": "a\n"}, {"commit": "a1"}, {"branch": "feat-b"}, {"checkout": "feat-b"}, {"write": "b.txt", "text": "b\n"}, {"commit": "b1"}, {"checkout": "main"}, {"branch": "side"}, {"checkout": "side"}, {"write": "s.txt", "text": "s\n"}, {"commit": "s1"}, {"checkout": "feat-b"}]} +{"cmd": "open", "path": "${data_dir}/repos/stack", "expect": [{"pointer": "/switcher/open", "equals": true}, {"pointer": "/switcher/mode", "equals": "repos"}]} +{"cmd": "settle", "expect": [{"pointer": "/switcher/items/3/key", "equals": "stack/compare"}, {"pointer": "/switcher/items/3/kind", "equals": "compare"}, {"pointer": "/switcher/items/3/label", "equals": "Compare branches…"}]} + +// "Compare branches…" lists the branches, the newest first, each against its stack base. +{"cmd": "click", "target": "switcher/stack/compare"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/mode", "equals": "branches"}, {"pointer": "/switcher/query", "equals": ""}, {"pointer": "/switcher/selected", "equals": "stack/pick/side"}, {"pointer": "/switcher/items/0/key", "equals": "stack/pick/side"}, {"pointer": "/switcher/items/0/detail", "equals": "vs main"}, {"pointer": "/switcher/items/1/key", "equals": "stack/pick/feat-b"}, {"pointer": "/switcher/items/1/detail", "equals": "vs feat-a"}, {"pointer": "/switcher/items/2/key", "equals": "stack/pick/feat-a"}, {"pointer": "/switcher/items/2/detail", "equals": "vs main"}, {"pointer": "/switcher/items", "len": 3}]} +{"cmd": "screenshot", "name": "branches"} + +// A click on feat-b lists its bases, the stack base first; Esc steps back to the branches. +{"cmd": "click", "target": "switcher/stack/pick/feat-b"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/mode", "equals": "bases"}, {"pointer": "/switcher/selected", "equals": "stack/base/feat-a"}, {"pointer": "/switcher/items/0/detail", "equals": "stack base"}, {"pointer": "/switcher/items/1/key", "equals": "stack/base/main"}, {"pointer": "/switcher/items/1/detail", "equals": "default branch"}, {"pointer": "/switcher/items/2/key", "equals": "stack/base/side"}, {"pointer": "/switcher/items", "len": 3}]} +{"cmd": "screenshot", "name": "bases"} +{"cmd": "key", "keystroke": "escape"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/mode", "equals": "branches"}, {"pointer": "/switcher/selected", "equals": "stack/pick/feat-b"}]} + +// Enter twice opens feat-b against feat-a: only feat-b's own commit. +{"cmd": "key", "keystroke": "enter"} +{"cmd": "key", "keystroke": "enter"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/open", "equals": false}, {"pointer": "/session/target", "equals": {"branch": {"branch": "feat-b", "base": "feat-a"}}}, {"pointer": "/session/target_label", "equals": "feat-b vs feat-a"}, {"pointer": "/session/total", "equals": 1}, {"pointer": "/file_list/current_file", "equals": "b.txt"}]} +{"cmd": "screenshot", "name": "stacked-review"} + +// The source picker offers "Compare branches…" after its targets; it opens the picker, which +// lists the open review beside its branch, and Esc closes it. +{"cmd": "click", "target": "titlebar/source"} +{"cmd": "settle", "expect": [{"pointer": "/source_picker/open", "equals": true}, {"pointer": "/source_picker/compare", "equals": true}]} +{"cmd": "click", "target": "source_picker/compare"} +{"cmd": "settle", "expect": [{"pointer": "/source_picker/open", "equals": false}, {"pointer": "/switcher/mode", "equals": "branches"}, {"pointer": "/switcher/items/1/review/status", "equals": "open"}]} +{"cmd": "set_setting", "key": "mode", "value": "light"} +{"cmd": "settle"} +{"cmd": "screenshot", "name": "branches-light"} +{"cmd": "key", "keystroke": "escape"} +{"cmd": "settle", "expect": [{"pointer": "/switcher/open", "equals": false}]}