From bdb480c0325f32d8554000f03ac3e1f0f874e5dc Mon Sep 17 00:00:00 2001 From: logan-sevendwarves Date: Mon, 17 Aug 2026 15:57:41 -0400 Subject: [PATCH] feat(pr): open stack PRs and attach by branch name Restacked branches keep their GitHub PR link so o/O can open PRs even when local tip OID drifted from remote. O opens every PR in the selected stack. - gh fetch: 30s timeout, open PRs only, name-based attach with OID preference - PR column: approved green check, open yellow #N, short merged/closed labels - Help overlay and footer document o/O/y; r retries GitHub enrichment --- README.md | 14 +- src/adapters/github.rs | 42 +-- src/adapters/platform.rs | 10 +- src/app.rs | 74 ++++- src/app/state.rs | 1 + src/integration_tests/github_enrichment.rs | 137 ++++++++-- src/integration_tests/mod.rs | 1 + src/integration_tests/pr_url_actions.rs | 301 +++++++++++++++++++++ src/integration_tests/tui_rendering.rs | 184 ++++++++++++- src/lib.rs | 2 +- src/main.rs | 27 ++ src/model/branch.rs | 9 + src/model/topology.rs | 7 + src/ui/layout.rs | 4 +- src/ui/panels.rs | 91 +++++-- src/ui/tree.rs | 76 +++++- 16 files changed, 863 insertions(+), 117 deletions(-) create mode 100644 src/integration_tests/pr_url_actions.rs diff --git a/README.md b/README.md index f626ec2..b273e9b 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ sections remain directly above the first visible branch they own. | `a` | Toggle Active / Archive view | | `X`, then `y` / `n` | Confirm or cancel guarded deletion of one exact local branch | | `r` | Force repository reconciliation | -| `o` / `y` | Open or copy the selected PR URL | +| `o` / `O` / `y` | Open the selected PR URL, open every PR in the selected stack, or copy the selected PR URL | | `?` | Show help and provider status | | `Esc` | Close help/message or cancel a filter edit | | `q`, `Ctrl-C` | Quit | @@ -214,17 +214,19 @@ missing, busy, or corrupt metadata produces an explicit topology-unavailable state; every Git-local branch remains visible as an independent root. The tool never writes or migrates Graphite files. -GitHub enrichment is optional. A single-flight, TTL-limited bounded `gh pr list` -request retrieves PRs in all states, and results attach only when branch name and tip -object ID still match. Missing auth, offline operation, timeout, or malformed -JSON is shown as provider state and does not affect local navigation. +GitHub enrichment is optional. A single-flight, TTL-limited bounded `gh pr list --state open` +request attaches PRs by local branch name. A matching tip object ID is preferred when +several PRs share a name, but drifted local commits still get the branch PR so `o` / `O` +can open it. Missing auth, offline operation, timeout, or malformed JSON is shown as +provider state (`?` help, GitHub line) and does not affect local navigation. ## Troubleshooting | Symptom | Meaning / action | |---|---| | `topology unavailable` | Graphite metadata is missing or incompatible; Git branches are still complete | -| PR details are blank | Run `gh auth status`; local behavior does not require GitHub | +| PR details are blank | Wait until `?` shows `GitHub: loaded`; run `gh auth status`; press `r` to retry. Local navigation does not require GitHub | +| `o` does nothing | Select a branch row (not a stack label), wait for a yellow `#N`, then press `o`. Hover does not open. Archive view and `/` search block `o`. | | Checkout is disabled | The branch is current, another worktree owns it, or Git has an operation in progress | | Git blocks checkout | Commit/move the conflicting work yourself; stackmap deliberately performs no cleanup | | Shift-arrow does not jump | This is terminal encoding, not Vim. Use `J` / `K`, or configure the terminal to send `ESC [ 1 ; 2 A` / `ESC [ 1 ; 2 B` for Shift+Up / Shift+Down. Stackmap enables complete modifier reporting when the terminal supports the enhanced keyboard protocol. | diff --git a/src/adapters/github.rs b/src/adapters/github.rs index 72c32cb..2dfecd1 100644 --- a/src/adapters/github.rs +++ b/src/adapters/github.rs @@ -8,6 +8,9 @@ use serde::Deserialize; use super::command::{CommandError, CommandOutput, run_bounded}; use crate::model::{BranchId, PullRequest, PullRequestStatus}; +const GITHUB_LIST_TIMEOUT: Duration = Duration::from_secs(30); +const GITHUB_LIST_OUTPUT_LIMIT: usize = 8 * 1024 * 1024; + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] struct GhPullRequest { @@ -75,7 +78,7 @@ pub fn fetch(cwd: &Path) -> Result, GitHubError> { "pr", "list", "--state", - "all", + "open", "--limit", "1000", "--json", @@ -85,8 +88,8 @@ pub fn fetch(cwd: &Path) -> Result, GitHubError> { OsStr::new("gh"), args, cwd, - Duration::from_secs(3), - 4 * 1024 * 1024, + GITHUB_LIST_TIMEOUT, + GITHUB_LIST_OUTPUT_LIMIT, ) .map_err(GitHubError::from)?; parse_output(output) @@ -109,24 +112,23 @@ pub fn parse_json(bytes: &[u8]) -> Result, GitHubError> { .map_err(|error| GitHubError::Malformed(Arc::from(error.to_string())))?; Ok(values .into_iter() - .filter_map(|value| { - Some(PrMatch { - branch: BranchId::new(value.head_ref_name), - oid: Arc::from(value.head_ref_oid?), - pull_request: PullRequest { - number: value.number, - title: Arc::from(value.title), - url: Arc::from(value.url), - status: match value.state.as_str() { - "MERGED" => PullRequestStatus::Merged, - "CLOSED" => PullRequestStatus::Closed, - _ if value.review_decision.as_deref() == Some("APPROVED") => { - PullRequestStatus::Approved - } - _ => PullRequestStatus::Open, - }, + .filter(|value| !value.head_ref_name.is_empty()) + .map(|value| PrMatch { + branch: BranchId::new(value.head_ref_name), + oid: Arc::from(value.head_ref_oid.unwrap_or_default()), + pull_request: PullRequest { + number: value.number, + title: Arc::from(value.title), + url: Arc::from(value.url), + status: match value.state.as_str() { + "MERGED" => PullRequestStatus::Merged, + "CLOSED" => PullRequestStatus::Closed, + _ if value.review_decision.as_deref() == Some("APPROVED") => { + PullRequestStatus::Approved + } + _ => PullRequestStatus::Open, }, - }) + }, }) .collect()) } diff --git a/src/adapters/platform.rs b/src/adapters/platform.rs index 777b450..c3f3c59 100644 --- a/src/adapters/platform.rs +++ b/src/adapters/platform.rs @@ -9,10 +9,18 @@ const PLATFORM_TIMEOUT: Duration = Duration::from_secs(3); const OUTPUT_LIMIT: usize = 64 * 1024; pub fn open_url(url: &str) -> Result<()> { + open_urls(&[url]) +} + +pub fn open_urls(urls: &[impl AsRef]) -> Result<()> { + if urls.is_empty() { + return Ok(()); + } let cwd = std::env::current_dir()?; + let arguments: Vec<&str> = urls.iter().map(|url| url.as_ref()).collect(); let output = run_bounded( OsStr::new("open"), - [url], + arguments, &cwd, PLATFORM_TIMEOUT, OUTPUT_LIMIT, diff --git a/src/app.rs b/src/app.rs index d724f38..2d01066 100644 --- a/src/app.rs +++ b/src/app.rs @@ -12,7 +12,7 @@ use crate::model::topology::{ ArchiveMode, Emphasis, OrderMode, ProjectionOptions, ProjectionScope, TopologyIndex, TopologyProjection, }; -use crate::model::{Branch, BranchId, RepositorySnapshot}; +use crate::model::{Branch, BranchId, PullRequest, PullRequestStatus, RepositorySnapshot}; use crate::refresh::upstream::{ MAX_TARGETS, UpstreamBatch, UpstreamCommand, UpstreamRequest, UpstreamTarget, }; @@ -228,14 +228,14 @@ impl App { Vec::new() }; if let Some(current) = &self.snapshot { - let current_prs: std::collections::HashMap<_, _> = current + let current_prs: HashMap = current .branches .iter() .filter_map(|branch| { branch .pr .clone() - .map(|pr| ((branch.id.clone(), branch.oid.clone()), pr)) + .map(|pull_request| (branch.id.clone(), pull_request)) }) .collect(); if !current_prs.is_empty() { @@ -243,9 +243,7 @@ impl App { let branches = Arc::make_mut(&mut snapshot.branches); for branch in branches { if branch.pr.is_none() { - branch.pr = current_prs - .get(&(branch.id.clone(), branch.oid.clone())) - .cloned(); + branch.pr = current_prs.get(&branch.id).cloned(); } } } @@ -526,15 +524,7 @@ impl App { }; let mut branches = snapshot.branches.to_vec(); for branch in &mut branches { - branch.pr = None; - } - for result in matches { - if let Some(branch) = branches - .iter_mut() - .find(|branch| branch.id == result.branch && branch.oid == result.oid) - { - branch.pr = Some(result.pull_request); - } + branch.pr = pick_best_pull_request(&matches, branch); } self.snapshot = Some(Arc::new(RepositorySnapshot { branches: Arc::from(branches), @@ -794,6 +784,10 @@ impl App { .selected_url() .map(Action::OpenUrl) .unwrap_or(Action::None), + Key::Character('O') => self + .stack_pr_urls() + .map(Action::OpenUrls) + .unwrap_or(Action::None), Key::Character('y') => self .selected_url() .map(Action::CopyUrl) @@ -2090,6 +2084,32 @@ impl App { self.selected_branch()?.pr.as_ref().map(|pr| pr.url.clone()) } + fn stack_pr_urls(&self) -> Option>> { + if matches!(self.archive_mode, ArchiveMode::Archive) || self.selected_label.is_some() { + return None; + } + let (selected, snapshot, topology) = match (&self.selected, &self.snapshot, &self.topology) + { + (Some(selected), Some(snapshot), Some(topology)) => (selected, snapshot, topology), + _ => return None, + }; + let stack_branches = topology.stack_branches(selected)?; + let mut urls = Vec::new(); + let mut seen = HashSet::new(); + for branch_id in stack_branches { + let Some(pull_request) = snapshot + .branch(branch_id) + .and_then(|branch| branch.pr.as_ref()) + else { + continue; + }; + if seen.insert(Arc::clone(&pull_request.url)) { + urls.push(Arc::clone(&pull_request.url)); + } + } + if urls.is_empty() { None } else { Some(urls) } + } + pub fn begin_delete_confirmation(&mut self) { if self.selected_label.is_some() { self.message = Some(Arc::from("labels cannot be deleted")); @@ -3061,3 +3081,27 @@ fn reconciliation_notice( } } } + +fn pick_best_pull_request(matches: &[PrMatch], branch: &Branch) -> Option { + matches + .iter() + .filter(|candidate| candidate.branch == branch.id) + .max_by_key(|candidate| pull_request_match_score(candidate, &branch.oid)) + .map(|matched| matched.pull_request.clone()) +} + +fn pull_request_match_score(pr_match: &PrMatch, branch_oid: &str) -> (u8, bool, u64) { + ( + pull_request_status_rank(pr_match.pull_request.status), + pr_match.oid.as_ref() == branch_oid, + pr_match.pull_request.number, + ) +} + +fn pull_request_status_rank(status: PullRequestStatus) -> u8 { + match status { + PullRequestStatus::Open | PullRequestStatus::Approved => 2, + PullRequestStatus::Closed => 1, + PullRequestStatus::Merged => 0, + } +} diff --git a/src/app/state.rs b/src/app/state.rs index fd30100..876715d 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -19,6 +19,7 @@ pub enum Action { Checkout(BranchId), Delete(DeleteRequest), OpenUrl(Arc), + OpenUrls(Vec>), CopyUrl(Arc), PersistConfig(ConfigWriteRequest), } diff --git a/src/integration_tests/github_enrichment.rs b/src/integration_tests/github_enrichment.rs index 25fbaf3..990f880 100644 --- a/src/integration_tests/github_enrichment.rs +++ b/src/integration_tests/github_enrichment.rs @@ -6,17 +6,37 @@ use crate::adapters::github::{GitHubError, PrMatch, parse_json}; use crate::app::App; use crate::model::{BranchId, PullRequest, PullRequestStatus}; +fn pull_request(number: u64, status: PullRequestStatus) -> PullRequest { + PullRequest { + number, + title: Arc::from(format!("PR {number}")), + url: Arc::from(format!("https://example.invalid/pr/{number}")), + status, + } +} + +fn pr_match(branch: &str, oid: &str, number: u64, status: PullRequestStatus) -> PrMatch { + PrMatch { + branch: BranchId::new(branch), + oid: Arc::from(oid), + pull_request: pull_request(number, status), + } +} + #[test] -fn parses_batched_pr_json_and_ignores_unverifiable_entries() { +fn parses_batched_pr_json_including_missing_oid() { let matches = parse_json(include_bytes!("../../tests/fixtures/github/pr-list.json")).unwrap(); - assert_eq!(matches.len(), 1); + assert_eq!(matches.len(), 2); assert_eq!(matches[0].branch, BranchId::new("feature/stack-map")); assert_eq!(matches[0].pull_request.number, 42); assert_eq!(matches[0].pull_request.status, PullRequestStatus::Approved); + assert_eq!(matches[1].branch, BranchId::new("ambiguous")); + assert_eq!(matches[1].oid.as_ref(), ""); + assert_eq!(matches[1].pull_request.number, 7); } #[test] -fn delayed_results_match_current_branch_by_id_and_oid() { +fn name_and_oid_match_attaches_pull_request() { let mut app = App::default(); let snapshot = common::snapshot(vec![common::branch( "feature/stack-map", @@ -25,26 +45,105 @@ fn delayed_results_match_current_branch_by_id_and_oid() { true, )]); app.apply_snapshot(snapshot); - let result = PrMatch { - branch: BranchId::new("feature/stack-map"), - oid: Arc::from("wrong-oid"), - pull_request: PullRequest { - number: 42, - title: Arc::from("stale"), - url: Arc::from("https://example.invalid/42"), - status: PullRequestStatus::Open, - }, - }; - app.apply_prs(vec![result.clone()]); - assert!(app.selected_branch().unwrap().pr.is_none()); - app.apply_prs(vec![PrMatch { - oid: Arc::from("oid-feature/stack-map"), - ..result - }]); + app.apply_prs(vec![pr_match( + "feature/stack-map", + "oid-feature/stack-map", + 42, + PullRequestStatus::Open, + )]); assert_eq!( app.selected_branch().unwrap().pr.as_ref().unwrap().number, 42 ); +} + +#[test] +fn name_match_with_different_oid_still_attaches_pull_request() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![common::branch( + "feature/stack-map", + None, + "feature/stack-map", + true, + )]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![pr_match( + "feature/stack-map", + "remote-head-oid", + 42, + PullRequestStatus::Open, + )]); + assert_eq!( + app.selected_branch().unwrap().pr.as_ref().unwrap().number, + 42 + ); +} + +#[test] +fn pull_request_attaches_only_to_matching_branch_name() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![ + common::branch("feature/stack-map", None, "feature/stack-map", true), + common::branch("other-branch", None, "other-branch", false), + ]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![pr_match( + "feature/stack-map", + "remote-head-oid", + 42, + PullRequestStatus::Open, + )]); + + let snapshot = app.snapshot.as_ref().unwrap(); + let feature = snapshot + .branch(&BranchId::new("feature/stack-map")) + .unwrap(); + let other = snapshot.branch(&BranchId::new("other-branch")).unwrap(); + assert_eq!(feature.pr.as_ref().unwrap().number, 42); + assert!(other.pr.is_none()); +} + +#[test] +fn open_pull_request_wins_over_closed_for_same_branch_name() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![common::branch( + "feature/stack-map", + None, + "feature/stack-map", + true, + )]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![ + pr_match( + "feature/stack-map", + "closed-oid", + 10, + PullRequestStatus::Closed, + ), + pr_match("feature/stack-map", "open-oid", 20, PullRequestStatus::Open), + ]); + assert_eq!( + app.selected_branch().unwrap().pr.as_ref().unwrap().number, + 20 + ); +} + +#[test] +fn delayed_results_persist_across_snapshot_refresh_by_branch_name() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![common::branch( + "feature/stack-map", + None, + "feature/stack-map", + true, + )]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![pr_match( + "feature/stack-map", + "oid-feature/stack-map", + 42, + PullRequestStatus::Open, + )]); let mut next = (*common::snapshot(vec![common::branch( "feature/stack-map", diff --git a/src/integration_tests/mod.rs b/src/integration_tests/mod.rs index 6282c51..c38cc95 100644 --- a/src/integration_tests/mod.rs +++ b/src/integration_tests/mod.rs @@ -2,6 +2,7 @@ mod archive_workflow; mod common; mod github_enrichment; mod navigation_checkout; +mod pr_url_actions; mod refresh_pipeline; mod repository_snapshot; mod terminal_interaction; diff --git a/src/integration_tests/pr_url_actions.rs b/src/integration_tests/pr_url_actions.rs new file mode 100644 index 0000000..0de8762 --- /dev/null +++ b/src/integration_tests/pr_url_actions.rs @@ -0,0 +1,301 @@ +use super::common; + +use std::sync::Arc; + +use crate::adapters::github::PrMatch; +use crate::app::{Action, App, ConfigTarget, Overlay}; +use crate::events::Key; +use crate::model::topology::ArchiveMode; +use crate::model::{Branch, BranchId, GraphiteProvenance, PullRequest, PullRequestStatus}; + +fn pull_request(number: u64) -> PullRequest { + PullRequest { + number, + title: Arc::from(format!("PR {number}")), + url: Arc::from(format!("https://example.invalid/pr/{number}")), + status: PullRequestStatus::Open, + } +} + +fn pr_match(branch: &str, oid: &str, number: u64) -> PrMatch { + PrMatch { + branch: BranchId::new(branch), + oid: Arc::from(oid), + pull_request: pull_request(number), + } +} + +fn branch_with_pr( + name: &str, + parent: Option<&str>, + root: &str, + current: bool, + number: u64, +) -> Branch { + let mut branch = common::branch(name, parent, root, current); + branch.pr = Some(pull_request(number)); + branch +} + +fn tracked(name: &str, parent: Option<&str>, root: &str, trunk: &str, committed_at: i64) -> Branch { + let mut branch = common::branch(name, parent, root, false); + branch.trunk = Some(BranchId::new(trunk)); + branch.graphite = GraphiteProvenance::Tracked; + branch.committed_at = committed_at; + branch +} + +fn linear_stack_snapshot() -> Arc { + let mut snapshot = (*common::snapshot(vec![ + tracked("main", None, "main", "main", 1), + branch_with_pr("base", None, "base", false, 10), + branch_with_pr("middle", Some("base"), "base", false, 20), + branch_with_pr("tip", Some("middle"), "base", true, 30), + ])) + .clone(); + snapshot.graphite_children = Arc::from([ + (BranchId::new("main"), Arc::from([BranchId::new("base")])), + (BranchId::new("base"), Arc::from([BranchId::new("middle")])), + (BranchId::new("middle"), Arc::from([BranchId::new("tip")])), + ]); + Arc::new(snapshot) +} + +fn tracked_with_pr( + name: &str, + parent: Option<&str>, + root: &str, + trunk: &str, + committed_at: i64, + number: u64, + current: bool, +) -> Branch { + let mut branch = tracked(name, parent, root, trunk, committed_at); + branch.current = current; + branch.pr = Some(pull_request(number)); + branch +} + +fn fork_stack_snapshot() -> Arc { + let mut snapshot = (*common::snapshot(vec![ + tracked("staging", None, "staging", "staging", 1), + tracked_with_pr("1", None, "1", "staging", 2, 1, false), + tracked_with_pr("2", Some("1"), "1", "staging", 3, 2, false), + tracked("3", Some("2"), "1", "staging", 4), + tracked("4", Some("3"), "1", "staging", 5), + tracked("3b", Some("3"), "1", "staging", 6), + tracked_with_pr("3c", Some("3b"), "1", "staging", 7, 3, true), + ])) + .clone(); + snapshot.configured_trunks = Arc::from([BranchId::new("staging")]); + snapshot.trunks = snapshot.configured_trunks.clone(); + snapshot.graphite_children = Arc::from([ + (BranchId::new("staging"), Arc::from([BranchId::new("1")])), + (BranchId::new("1"), Arc::from([BranchId::new("2")])), + (BranchId::new("2"), Arc::from([BranchId::new("3")])), + ( + BranchId::new("3"), + Arc::from([BranchId::new("4"), BranchId::new("3b")]), + ), + (BranchId::new("3b"), Arc::from([BranchId::new("3c")])), + ]); + Arc::new(snapshot) +} + +#[test] +fn lowercase_o_opens_name_matched_pull_request_with_different_oid() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![common::branch( + "feature/stack-map", + None, + "feature/stack-map", + true, + )]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![pr_match("feature/stack-map", "remote-head-oid", 42)]); + assert_eq!( + app.handle_key(Key::Character('o')), + Action::OpenUrl(Arc::from("https://example.invalid/pr/42")) + ); +} + +#[test] +fn uppercase_o_includes_name_matched_pull_requests_in_stack() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![ + common::branch("base", None, "base", false), + common::branch("middle", Some("base"), "base", false), + common::branch("tip", Some("middle"), "base", true), + ]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![ + pr_match("base", "remote-base", 10), + pr_match("middle", "remote-middle", 20), + pr_match("tip", "remote-tip", 30), + ]); + app.selected = Some(BranchId::new("middle")); + assert_eq!( + app.handle_key(Key::Character('O')), + Action::OpenUrls(vec![ + Arc::from("https://example.invalid/pr/10"), + Arc::from("https://example.invalid/pr/20"), + Arc::from("https://example.invalid/pr/30"), + ]) + ); +} + +#[test] +fn lowercase_o_opens_selected_pull_request_url() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.selected = Some(BranchId::new("middle")); + assert_eq!( + app.handle_key(Key::Character('o')), + Action::OpenUrl(Arc::from("https://example.invalid/pr/20")) + ); +} + +#[test] +fn lowercase_o_is_noop_without_pull_request() { + let mut app = App::default(); + app.apply_snapshot(common::snapshot(vec![ + common::branch("main", None, "main", true), + common::branch("feature", None, "feature", false), + ])); + app.selected = Some(BranchId::new("feature")); + assert_eq!(app.handle_key(Key::Character('o')), Action::None); +} + +#[test] +fn lowercase_o_is_noop_in_archive_view() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.selected = Some(BranchId::new("middle")); + app.handle_key(Key::Character('a')); + assert_eq!(app.handle_key(Key::Character('o')), Action::None); +} + +#[test] +fn lowercase_o_is_noop_when_label_is_selected() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.selected = Some(BranchId::new("base")); + app.selected_label = Some(ConfigTarget::Stack(BranchId::new("base"))); + assert_eq!(app.handle_key(Key::Character('o')), Action::None); +} + +#[test] +fn search_overlay_still_types_o_into_filter() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.handle_key(Key::Character('/')); + assert!(matches!(app.overlay, Overlay::Search)); + assert_eq!(app.handle_key(Key::Character('o')), Action::None); + assert_eq!(app.filter, "o"); +} + +#[test] +fn uppercase_o_opens_all_stack_pull_requests_in_group_order() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.selected = Some(BranchId::new("middle")); + assert_eq!( + app.handle_key(Key::Character('O')), + Action::OpenUrls(vec![ + Arc::from("https://example.invalid/pr/10"), + Arc::from("https://example.invalid/pr/20"), + Arc::from("https://example.invalid/pr/30"), + ]) + ); +} + +#[test] +fn uppercase_o_on_side_fork_opens_only_child_stack_pull_requests() { + let mut app = App::default(); + app.apply_snapshot(fork_stack_snapshot()); + app.selected = Some(BranchId::new("3c")); + assert_eq!( + app.handle_key(Key::Character('O')), + Action::OpenUrls(vec![Arc::from("https://example.invalid/pr/3")]) + ); +} + +#[test] +fn uppercase_o_skips_branches_without_pull_requests() { + let mut snapshot = (*linear_stack_snapshot()).clone(); + let branches: Vec = snapshot + .branches + .iter() + .map(|branch| { + if branch.id == BranchId::new("middle") { + let mut without_pull_request = branch.clone(); + without_pull_request.pr = None; + without_pull_request + } else { + branch.clone() + } + }) + .collect(); + snapshot.branches = Arc::from(branches); + snapshot.branch_index = crate::model::RepositorySnapshot::index_branches(&snapshot.branches); + + let mut app = App::default(); + app.apply_snapshot(Arc::new(snapshot)); + app.selected = Some(BranchId::new("middle")); + assert_eq!( + app.handle_key(Key::Character('O')), + Action::OpenUrls(vec![ + Arc::from("https://example.invalid/pr/10"), + Arc::from("https://example.invalid/pr/30"), + ]) + ); +} + +#[test] +fn uppercase_o_deduplicates_duplicate_pull_request_urls() { + let mut snapshot = (*linear_stack_snapshot()).clone(); + let shared_url: Arc = Arc::from("https://example.invalid/pr/shared"); + let branches: Vec = snapshot + .branches + .iter() + .map(|branch| { + let mut updated = branch.clone(); + if branch.id == BranchId::new("middle") || branch.id == BranchId::new("tip") { + updated.pr = Some(PullRequest { + number: 99, + title: Arc::from("Shared"), + url: shared_url.clone(), + status: PullRequestStatus::Open, + }); + } + updated + }) + .collect(); + snapshot.branches = Arc::from(branches); + snapshot.branch_index = crate::model::RepositorySnapshot::index_branches(&snapshot.branches); + + let mut app = App::default(); + app.apply_snapshot(Arc::new(snapshot)); + app.selected = Some(BranchId::new("middle")); + assert_eq!( + app.handle_key(Key::Character('O')), + Action::OpenUrls(vec![Arc::from("https://example.invalid/pr/10"), shared_url,]) + ); +} + +#[test] +fn uppercase_o_is_noop_on_trunk() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.selected = Some(BranchId::new("main")); + assert_eq!(app.handle_key(Key::Character('O')), Action::None); +} + +#[test] +fn uppercase_o_is_noop_in_archive_view() { + let mut app = App::default(); + app.apply_snapshot(linear_stack_snapshot()); + app.selected = Some(BranchId::new("middle")); + app.archive_mode = ArchiveMode::Archive; + assert_eq!(app.handle_key(Key::Character('O')), Action::None); +} diff --git a/src/integration_tests/tui_rendering.rs b/src/integration_tests/tui_rendering.rs index 00b311e..1d44af3 100644 --- a/src/integration_tests/tui_rendering.rs +++ b/src/integration_tests/tui_rendering.rs @@ -10,7 +10,7 @@ use crate::model::{ BranchId, ConfiguredUpstream, DiffStat, DiffState, PullRequest, PullRequestStatus, RemoteRefEvidence, }; -use crate::ui::layout::{RenderGeometry, WidthMode, areas}; +use crate::ui::layout::{ColumnRange, RenderGeometry, WidthMode, areas}; use crate::ui::theme::{ TRUNK_COLOR_HEX, current_background, selected_background, stack_color, trunk_color, }; @@ -458,12 +458,91 @@ fn selected_rows_use_dark_content_and_current_rows_use_tinted_stack_fill() { ); } +fn sample_pull_request(status: PullRequestStatus) -> PullRequest { + PullRequest { + number: 42, + title: Arc::from("Feature"), + url: Arc::from("https://example.invalid/42"), + status, + } +} + +fn render_branch_pull_request( + branch_name: &str, + status: PullRequestStatus, + sibling_names: &[&str], + selected_name: Option<&str>, +) -> Terminal { + let mut primary = common::branch(branch_name, None, branch_name, false); + primary.pr = Some(sample_pull_request(status)); + let mut branches = vec![primary]; + for sibling_name in sibling_names { + branches.push(common::branch(sibling_name, None, sibling_name, false)); + } + let mut app = App::default(); + app.apply_snapshot(common::snapshot(branches)); + if let Some(name) = selected_name { + app.selected = Some(BranchId::new(name)); + } + let mut terminal = Terminal::new(TestBackend::new(100, 8)).unwrap(); + terminal + .draw(|frame| crate::ui::render(frame, &mut app, UNIX_EPOCH)) + .unwrap(); + terminal +} + +#[test] +fn approved_pull_request_checkmark_uses_green() { + let terminal = + render_branch_pull_request("feature-approved", PullRequestStatus::Approved, &[], None); + let check = terminal + .backend() + .buffer() + .content() + .iter() + .find(|cell| cell.symbol() == "✓") + .expect("approved checkmark"); + assert_eq!(check.fg, Color::Green); +} + +#[test] +fn open_pull_request_has_no_green_checkmark() { + let terminal = render_branch_pull_request("feature-open", PullRequestStatus::Open, &[], None); + let rendered = rendered_lines(&terminal).join("\n"); + assert!(rendered.contains("#42"), "{rendered}"); + assert!( + !terminal + .backend() + .buffer() + .content() + .iter() + .any(|cell| cell.symbol() == "✓" && cell.fg == Color::Green) + ); +} + +#[test] +fn approved_pull_request_checkmark_stays_green_when_selected() { + let terminal = render_branch_pull_request( + "approved", + PullRequestStatus::Approved, + &["selected"], + Some("approved"), + ); + let lines = rendered_lines(&terminal); + let (approved_y, _) = line_with(&lines, "approved"); + let check = (0..100) + .map(|x| &terminal.backend().buffer()[(x, approved_y as u16)]) + .find(|cell| cell.symbol() == "✓") + .expect("approved checkmark on selected row"); + assert_eq!(check.fg, Color::Green); +} + #[test] fn pull_request_status_replaces_the_number_for_terminal_states_and_approval() { for (status, expected) in [ - (PullRequestStatus::Approved, "Approved"), - (PullRequestStatus::Closed, "Closed"), - (PullRequestStatus::Merged, "Merged"), + (PullRequestStatus::Approved, "✓#42"), + (PullRequestStatus::Closed, "Clsd"), + (PullRequestStatus::Merged, "Mrgd"), ] { let mut branch = common::branch("feature", None, "feature", false); branch.pr = Some(PullRequest { @@ -486,6 +565,44 @@ fn pull_request_status_replaces_the_number_for_terminal_states_and_approval() { } } +#[test] +fn diverged_remote_and_approved_pr_stay_in_separate_metadata_columns() { + let mut branch = common::branch("feature-x", None, "feature-x", false); + branch.configured_upstream = ConfiguredUpstream::Diverged { + reference: Arc::from("origin/feature-x"), + ahead: 95, + behind: 2, + }; + branch.pr = Some(PullRequest { + number: 4593, + title: Arc::from("Feature"), + url: Arc::from("https://example.invalid/4593"), + status: PullRequestStatus::Approved, + }); + let mut app = App::default(); + app.apply_snapshot(common::snapshot(vec![branch])); + let mut terminal = Terminal::new(TestBackend::new(100, 8)).unwrap(); + terminal + .draw(|frame| crate::ui::render(frame, &mut app, UNIX_EPOCH)) + .unwrap(); + let geometry = RenderGeometry::new(100, WidthMode::Medium, LanePitch::Auto, 1); + let remote = geometry.remote.expect("remote column"); + let pull_request = geometry.pr.expect("PR column"); + let lines = rendered_lines(&terminal); + let (_, branch_line) = line_with(&lines, "feature-x"); + let remote_text = metadata_column_text(branch_line, remote); + let pull_request_text = metadata_column_text(branch_line, pull_request); + assert!(remote_text.contains("div")); + assert!(pull_request_text.contains("✓#4593")); + assert!(!pull_request_text.contains("div")); + assert!(!branch_line.contains("divApproved")); + assert!(!branch_line.contains("div✓")); +} + +fn metadata_column_text(line: &str, range: ColumnRange) -> String { + line.chars().skip(range.x).take(range.width).collect() +} + #[test] fn named_stack_renders_a_white_label_with_a_spacer_above_its_head() { let mut app = App::default(); @@ -1054,7 +1171,7 @@ fn metadata_columns_leave_time_diff_and_pr_edge_gutters() { geometry.diff.x + geometry.diff.width + 1 ); assert_eq!(remote.x, geometry.worktree.x + geometry.worktree.width); - assert_eq!(pr.x, remote.x + remote.width); + assert_eq!(pr.x, remote.x + remote.width + 1); assert_eq!(pr.x + pr.width + 1, geometry.width); } @@ -1568,23 +1685,20 @@ fn help_documents_archive_range_and_picker_keys() { "main", None, "main", true, )])); app.overlay = crate::app::Overlay::Help; - let backend = TestBackend::new(90, 28); + let backend = TestBackend::new(90, 48); let mut terminal = Terminal::new(backend).unwrap(); terminal .draw(|frame| crate::ui::render(frame, &mut app, UNIX_EPOCH)) .unwrap(); - let rendered: String = terminal - .backend() - .buffer() - .content() - .iter() - .map(|cell| cell.symbol()) - .collect(); + let rendered = rendered_lines(&terminal).join("\n"); assert!(rendered.contains("archive / restore selected local branch")); assert!(rendered.contains("guarded delete exact local branch")); assert!(rendered.contains("preview contiguous archive/restore range")); assert!(rendered.contains("order picker")); assert!(rendered.contains("color picker")); + assert!(rendered.contains("open selected PR / all stack PRs / copy URL")); + assert!(rendered.contains("GitHub:")); + assert!(rendered.contains("Graphite:")); assert!(rendered.contains("› selected")); assert!(rendered.contains("○ branch")); assert!(rendered.contains("● current")); @@ -1594,6 +1708,50 @@ fn help_documents_archive_range_and_picker_keys() { assert!(rendered.contains("⎇ worktree")); } +#[test] +fn help_shows_open_pr_keys_and_github_status_on_short_terminal() { + let mut app = App::default(); + app.apply_snapshot(common::snapshot(vec![common::branch( + "main", None, "main", true, + )])); + app.overlay = Overlay::Help; + let mut terminal = Terminal::new(TestBackend::new(80, 16)).unwrap(); + terminal + .draw(|frame| crate::ui::render(frame, &mut app, UNIX_EPOCH)) + .unwrap(); + let rendered = rendered_lines(&terminal).join("\n"); + assert!(rendered.contains("open selected PR / all stack PRs / copy URL")); + assert!(rendered.contains("GitHub:")); +} + +#[test] +fn footer_shows_open_pr_keys_when_selected_branch_has_pull_request() { + let mut app = App::default(); + let snapshot = common::snapshot(vec![common::branch( + "feature/stack-map", + None, + "feature/stack-map", + true, + )]); + app.apply_snapshot(snapshot); + app.apply_prs(vec![crate::adapters::github::PrMatch { + branch: BranchId::new("feature/stack-map"), + oid: Arc::from("remote-oid"), + pull_request: PullRequest { + number: 42, + title: Arc::from("PR"), + url: Arc::from("https://example.invalid/pr/42"), + status: PullRequestStatus::Open, + }, + }]); + let mut terminal = Terminal::new(TestBackend::new(160, 10)).unwrap(); + terminal + .draw(|frame| crate::ui::render(frame, &mut app, UNIX_EPOCH)) + .unwrap(); + let rendered = rendered_lines(&terminal).join("\n"); + assert!(rendered.contains("o/O/y PR")); +} + #[test] fn picker_modals_render_textual_choices_and_commit_hints() { let mut app = App::default(); diff --git a/src/lib.rs b/src/lib.rs index e40668b..b1bf65c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -63,7 +63,7 @@ pub mod runtime { /// Bounded macOS open and clipboard helpers. pub mod platform { - pub use crate::adapters::platform::{copy_text, open_url}; + pub use crate::adapters::platform::{copy_text, open_url, open_urls}; } } diff --git a/src/main.rs b/src/main.rs index 39627be..38cfa31 100644 --- a/src/main.rs +++ b/src/main.rs @@ -32,6 +32,7 @@ const CONFIG_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); enum PlatformResult { Open(Result<()>), + OpenMany { count: usize, result: Result<()> }, Copy(Result<()>), } @@ -258,10 +259,23 @@ fn run() -> Result<()> { app.platform_running = false; match result { PlatformResult::Open(Ok(())) => app.message = Some(Arc::from("PR opened")), + PlatformResult::OpenMany { + count, + result: Ok(()), + } => { + app.message = Some(if count == 1 { + Arc::from("PR opened") + } else { + Arc::from(format!("opened {count} PRs")) + }); + } PlatformResult::Copy(Ok(())) => app.message = Some(Arc::from("PR URL copied")), PlatformResult::Open(Err(error)) => { app.message = Some(Arc::from(format!("open failed: {error}"))) } + PlatformResult::OpenMany { + result: Err(error), .. + } => app.message = Some(Arc::from(format!("open failed: {error}"))), PlatformResult::Copy(Err(error)) => { app.message = Some(Arc::from(format!("copy failed: {error}"))) } @@ -324,6 +338,7 @@ fn run() -> Result<()> { Action::None => {} Action::Quit => break, Action::Refresh => { + next_github_fetch = Instant::now(); refresh.request(); } Action::PersistConfig(request) => { @@ -344,6 +359,13 @@ fn run() -> Result<()> { platform_send.clone(), )?; } + Action::OpenUrls(urls) => { + spawn_platform_action( + &mut app, + PlatformAction::OpenMany(urls), + platform_send.clone(), + )?; + } Action::CopyUrl(url) => { spawn_platform_action( &mut app, @@ -446,6 +468,7 @@ fn spawn_config_persistence( enum PlatformAction { Open(Arc), + OpenMany(Vec>), Copy(Arc), } @@ -464,6 +487,10 @@ fn spawn_platform_action( .spawn(move || { let result = match action { PlatformAction::Open(url) => PlatformResult::Open(platform::open_url(&url)), + PlatformAction::OpenMany(urls) => PlatformResult::OpenMany { + count: urls.len(), + result: platform::open_urls(&urls), + }, PlatformAction::Copy(url) => PlatformResult::Copy(platform::copy_text(&url)), }; let _ = sender.try_send(result); diff --git a/src/model/branch.rs b/src/model/branch.rs index ddc2e9b..65f7893 100644 --- a/src/model/branch.rs +++ b/src/model/branch.rs @@ -109,6 +109,15 @@ impl PullRequestStatus { Self::Merged => "Merged", } } + + pub fn column_text(self, number: u64) -> String { + match self { + Self::Open => format!("#{number}"), + Self::Approved => format!("✓#{number}"), + Self::Merged => "Mrgd".to_owned(), + Self::Closed => "Clsd".to_owned(), + } + } } #[derive(Clone, Copy, Debug, Eq, PartialEq)] diff --git a/src/model/topology.rs b/src/model/topology.rs index 4e6427e..85006fc 100644 --- a/src/model/topology.rs +++ b/src/model/topology.rs @@ -194,6 +194,13 @@ impl TopologyIndex { self.stack_by_branch.get(branch) } + pub fn stack_branches(&self, branch: &BranchId) -> Option<&[BranchId]> { + let stack_id = self.stack_for(branch)?; + self.groups + .get(stack_id) + .map(|group| group.branches.as_slice()) + } + pub fn stack_diff_endpoints(&self) -> Vec { self.groups .values() diff --git a/src/ui/layout.rs b/src/ui/layout.rs index ea3c7b8..834ce11 100644 --- a/src/ui/layout.rs +++ b/src/ui/layout.rs @@ -44,6 +44,7 @@ impl RenderGeometry { let diff_worktree_gap = 1; let worktree_width = if wide_worktree { 18 } else { 2 }; let remote_width = usize::from(show_remote) * 11; + let remote_pr_gap = usize::from(show_remote && show_pr); let pr_width = usize::from(show_pr) * 8; let pr_edge_gap = usize::from(show_pr); let metadata_width = time_width @@ -52,6 +53,7 @@ impl RenderGeometry { + diff_worktree_gap + worktree_width + remote_width + + remote_pr_gap + pr_width + pr_edge_gap; let metadata_start = width.saturating_sub(metadata_width); @@ -79,7 +81,7 @@ impl RenderGeometry { x: cursor, width: remote_width, }; - cursor += remote_width; + cursor += remote_width + remote_pr_gap; range }); let pr = show_pr.then_some(ColumnRange { diff --git a/src/ui/panels.rs b/src/ui/panels.rs index bc02e15..02caa0e 100644 --- a/src/ui/panels.rs +++ b/src/ui/panels.rs @@ -108,11 +108,24 @@ pub fn footer(frame: &mut Frame<'_>, area: Rect, app: &App) { } else { "archive" }; + let pull_request_keys = if matches!(app.archive_mode, ArchiveMode::Active) + && matches!(app.overlay, Overlay::None) + && matches!(app.mutation, MutationState::Idle) + && app + .selected_branch() + .is_some_and(|branch| branch.pr.is_some()) + { + " o/O/y PR" + } else { + "" + }; if has_supplementary { - format!(" a View {archive_target} x {archive_action} X delete ? help") + format!( + " a View {archive_target} x {archive_action} X delete{pull_request_keys} ? help" + ) } else { format!( - " {position}/{} {order} {} {pitch} a View {archive_target} x {archive_action} v range X delete ↑↓ {stack_navigation} ? help", + " {position}/{} {order} {} {pitch} a View {archive_target} x {archive_action} v range X delete ↑↓ {stack_navigation}{pull_request_keys} ? help", app.projection.navigation.len(), app.scope_label(), ) @@ -170,8 +183,22 @@ pub fn detail(frame: &mut Frame<'_>, area: Rect, app: &App) { } pub fn help(frame: &mut Frame<'_>, app: &App) { - let area = centered(frame.area(), 72, 25); + let text = help_contents(app); + let height = u16::try_from(text.lines().count()) + .unwrap_or(u16::MAX) + .saturating_add(2); + let area = centered(frame.area(), 78, height); frame.render_widget(Clear, area); + frame.render_widget( + Paragraph::new(text) + .wrap(Wrap { trim: false }) + .alignment(Alignment::Left) + .block(Block::default().title(" Help ").borders(Borders::ALL)), + area, + ); +} + +fn help_contents(app: &App) -> String { let graphite = app .snapshot .as_ref() @@ -183,32 +210,40 @@ pub fn help(frame: &mut Frame<'_>, app: &App) { GitHubState::Ready => "loaded".to_owned(), GitHubState::Unavailable(error) => format!("unavailable: {error}"), }; - let text = format!( - "Markers: › selected ○ branch ● current ◉ trunk\n ■ range * dirty ⎇ worktree\nRemote: ✓ pushed ↑ ahead ↓ behind ↕ diverged\n × gone ○ no remote ? unknown\n\n↑/↓ or j/k previous/next branch\nShift/Cmd+↑/↓ J/K adjacent stack head, otherwise ±10 rows\nAlt+↑/↓ g/G top/bottom branch of current section\nt / T Recent/Graphite toggle / order picker\n+ / - / 0 adjust / reset lane pitch\nh focus selected stack; repeat exits\nH focus trunk or all Untrunked; repeat exits\ns toggle stack spacing\na toggle Active / Archive view\nv + arrows preview contiguous archive/restore range\n/ filter branch names\nEnter ×2 arm / confirm protected git switch\nc / C cycle color / color picker\nn name / clear selected stack\nx archive / restore selected local branch\nX guarded delete exact local branch\nr full reconciliation\no / y open / copy PR URL\nEsc close message/help\nq or Ctrl-C quit\n\nLowercase x/v change local config only; uppercase X can delete one exact local ref after confirmation. No remote changes or fetch.\nArchive view shows dim, nonselectable ancestry for stack context.\nFocused sections pin their trunk/bottom row.\nRemote evidence uses local remote-tracking refs only; no fetch.\nColors: stack identity; yellow PR; green/red diff\nActive: {} / {} / {}\n\nGraphite: {graphite}\nGitHub: {github}", - match app.order_mode { - OrderMode::Recent => "recent order", - OrderMode::Alphabetical => "alphabetical order", - OrderMode::Graphite => "Graphite order", - OrderMode::Chronological => "time order", - }, + let order = match app.order_mode { + OrderMode::Recent => "recent order", + OrderMode::Alphabetical => "alphabetical order", + OrderMode::Graphite => "Graphite order", + OrderMode::Chronological => "time order", + }; + let separators = if app.separators { + "separators on" + } else { + "separators off" + }; + format!( + "GitHub: {github}\nGraphite: {graphite}\n\n\ +o / O / y open selected PR / all stack PRs / copy URL\n\ +Esc / ? close this help\n\ +q or Ctrl-C quit\n\n\ +Markers: › selected ○ branch ● current ◉ trunk\n ■ range * dirty ⎇ worktree\nRemote: ✓ pushed ↑ ahead ↓ behind ↕ diverged\n × gone ○ no remote ? unknown\n\n\ +↑/↓ or j/k previous/next branch\nShift/Cmd+↑/↓ J/K adjacent stack head, otherwise ±10 rows\nAlt+↑/↓ g/G top/bottom branch of current section\n\ +t / T Recent/Graphite toggle / order picker\n+ / - / 0 adjust / reset lane pitch\n\ +h focus selected stack; repeat exits\nH focus trunk or all Untrunked; repeat exits\n\ +s toggle stack spacing\na toggle Active / Archive view\n\ +v + arrows preview contiguous archive/restore range\n/ filter branch names\n\ +Enter ×2 / Enter git switch / edit selected label\nc / C contextual color / color picker\n\ +d toggle wide detail sidebar\ni toggle visual section boundary\n\ +n create missing stack/section label\nx archive / restore selected local branch\n\ +X guarded delete exact local branch\nr full reconciliation\n\n\ +Lowercase x/v change local config only; uppercase X can delete one exact local ref after confirmation. No remote changes or fetch.\n\ +Archive view shows dim, nonselectable ancestry for stack context.\n\ +Focused sections pin their trunk/bottom row.\n\ +Remote evidence uses local remote-tracking refs only; no fetch.\n\ +Colors: stack identity; yellow PR; green/red diff\n\ +Active: {order} / {} / {separators}", app.scope_label(), - if app.separators { - "separators on" - } else { - "separators off" - } - ); - let text = text.replace( - "Enter ×2 arm / confirm protected git switch\nc / C cycle color / color picker\nn name / clear selected stack", - "Enter ×2 / Enter git switch / edit selected label\nc / C contextual color / color picker\nd toggle wide detail sidebar\ni toggle visual section boundary\nn create missing stack/section label", - ); - frame.render_widget( - Paragraph::new(text) - .wrap(Wrap { trim: false }) - .alignment(Alignment::Left) - .block(Block::default().title(" Help ").borders(Borders::ALL)), - area, - ); + ) } pub fn order_picker(frame: &mut Frame<'_>, app: &App) { diff --git a/src/ui/tree.rs b/src/ui/tree.rs index 5c129cf..433c462 100644 --- a/src/ui/tree.rs +++ b/src/ui/tree.rs @@ -11,7 +11,10 @@ use crate::model::topology::{ ArchiveMode, ConnectorRow, DividerRow, Emphasis, ProjectedRow, ProjectionEntry, StackLabelRow, VisualSectionDividerRow, VisualSectionLabelRow, }; -use crate::model::{Branch, BranchId, ConfiguredUpstream, DiffState, RemoteRefEvidence}; +use crate::model::{ + Branch, BranchId, ConfiguredUpstream, DiffState, PullRequest, PullRequestStatus, + RemoteRefEvidence, +}; use super::layout::{ColumnRange, RenderGeometry, WidthMode}; use super::theme::{ @@ -33,6 +36,7 @@ const RIGHT: u8 = 8; struct RenderCell { symbol: String, style: Style, + preserve_foreground: bool, } impl Default for RenderCell { @@ -40,6 +44,7 @@ impl Default for RenderCell { Self { symbol: " ".into(), style: Style::default(), + preserve_foreground: false, } } } @@ -554,24 +559,17 @@ fn branch_line( paint_remote_status(&mut cells, branch, range, metadata_emphasis); } if let Some(range) = geometry.pr - && let Some(pr) = &branch.pr + && let Some(pull_request) = &branch.pr { - put_right( - &mut cells, - range, - match pr.status { - crate::model::PullRequestStatus::Open => format!("#{}", pr.number), - status => status.label().to_owned(), - } - .as_str(), - emphasized(Style::default().fg(Color::Yellow), metadata_emphasis), - ); + paint_pull_request(&mut cells, range, pull_request, metadata_emphasis); } } if selected { for cell in &mut cells { - cell.style = cell.style.fg(Color::Black); + if !cell.preserve_foreground { + cell.style = cell.style.fg(Color::Black); + } } } @@ -846,6 +844,47 @@ fn paint_diff(cells: &mut [RenderCell], range: ColumnRange, diff: &DiffState, em } } +fn paint_pull_request( + cells: &mut [RenderCell], + range: ColumnRange, + pull_request: &PullRequest, + emphasis: Emphasis, +) { + match pull_request.status { + PullRequestStatus::Open => { + put_right( + cells, + range, + &pull_request.status.column_text(pull_request.number), + emphasized(Style::default().fg(Color::Yellow), emphasis), + ); + } + PullRequestStatus::Approved => { + let combined = pull_request.status.column_text(pull_request.number); + let visible = truncate(&combined, range.width); + let visible_count = visible.chars().count(); + let start_x = range.x + range.width.saturating_sub(visible_count); + let check_style = emphasized(Style::default().fg(Color::Green), emphasis); + let number_style = emphasized(Style::default().fg(Color::Yellow), emphasis); + for (offset, character) in visible.chars().enumerate() { + let position = start_x + offset; + match character { + '✓' => write_symbol(cells, position, "✓", check_style, true), + _ => write_symbol(cells, position, &character.to_string(), number_style, false), + } + } + } + PullRequestStatus::Merged | PullRequestStatus::Closed => { + put_right( + cells, + range, + &pull_request.status.column_text(pull_request.number), + emphasized(Style::default().fg(Color::DarkGray), emphasis), + ); + } + } +} + fn paint_worktree( cells: &mut [RenderCell], range: ColumnRange, @@ -879,10 +918,21 @@ fn blank_cells(width: usize) -> Vec { } fn set_symbol(cells: &mut [RenderCell], x: usize, symbol: &str, style: Style) { + write_symbol(cells, x, symbol, style, false); +} + +fn write_symbol( + cells: &mut [RenderCell], + x: usize, + symbol: &str, + style: Style, + preserve_foreground: bool, +) { if let Some(cell) = cells.get_mut(x) { cell.symbol.clear(); cell.symbol.push_str(symbol); cell.style = style; + cell.preserve_foreground = preserve_foreground; } }