From 2e0d4de1363f460007dc11cb484377b6022e1a15 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:46:49 +0800 Subject: [PATCH] feat(github): show a pull request's checks, reviews and merge state (#1000) The PR detail now leads with what it is usually opened to find out: - Checks: check runs and commit statuses on the head commit, failures first, with durations and links to their logs. A section with more than five folds the passed and skipped ones into one row. - Reviews: one row per reviewer (approved, changes requested, commented, requested), folding the way GitHub's sidebar reads them. - A merge-state line under the branches ("Waiting on 1 check", "1 check failing", "Merge conflicts", "Ready to merge", ...). Checks and reviews that cannot be read are left out instead of failing the whole detail. While a check is running, just the checks are re-read every 20 seconds (signed in only); once every verdict is in, the detail is read again for the new merge state. The list pins the pull request of the pane's branch under the repo row, looked up through the branch's upstream so a fork's branch is found under the fork's owner. Long sections fold: comments past four keep the first and the latest two, long descriptions and comments are clamped behind "Show full text", and reviewers and changed files show a few with a "show all" row. --- assets/icons/github/check-failed.svg | 1 + assets/icons/github/check-passed.svg | 1 + assets/icons/github/check-pending.svg | 1 + assets/icons/github/check-skipped.svg | 1 + crates/tty7-core/src/core/github/api.rs | 233 ++++++++- crates/tty7-core/src/core/github/mod.rs | 5 +- crates/tty7-core/src/core/github/model.rs | 565 +++++++++++++++++++- src/ui/assets.rs | 13 + src/ui/github/detail.rs | 610 +++++++++++++++++++++- src/ui/github/mod.rs | 375 +++++++++++++ src/ui/i18n/en.rs | 48 ++ src/ui/i18n/ja.rs | 48 ++ src/ui/i18n/mod.rs | 34 ++ src/ui/i18n/zh.rs | 48 ++ src/ui/panel_github.rs | 295 ++++++++++- 15 files changed, 2242 insertions(+), 36 deletions(-) create mode 100644 assets/icons/github/check-failed.svg create mode 100644 assets/icons/github/check-passed.svg create mode 100644 assets/icons/github/check-pending.svg create mode 100644 assets/icons/github/check-skipped.svg diff --git a/assets/icons/github/check-failed.svg b/assets/icons/github/check-failed.svg new file mode 100644 index 00000000..7cbf8d98 --- /dev/null +++ b/assets/icons/github/check-failed.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/github/check-passed.svg b/assets/icons/github/check-passed.svg new file mode 100644 index 00000000..1ef4c564 --- /dev/null +++ b/assets/icons/github/check-passed.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/github/check-pending.svg b/assets/icons/github/check-pending.svg new file mode 100644 index 00000000..af65af00 --- /dev/null +++ b/assets/icons/github/check-pending.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/github/check-skipped.svg b/assets/icons/github/check-skipped.svg new file mode 100644 index 00000000..90fc554b --- /dev/null +++ b/assets/icons/github/check-skipped.svg @@ -0,0 +1 @@ + diff --git a/crates/tty7-core/src/core/github/api.rs b/crates/tty7-core/src/core/github/api.rs index 66e4f67d..247c43cc 100644 --- a/crates/tty7-core/src/core/github/api.rs +++ b/crates/tty7-core/src/core/github/api.rs @@ -8,7 +8,8 @@ use serde::de::DeserializeOwned; use super::model::{ - Comment, Detail, Item, Kind, PrFile, RawComment, RawFile, RawIssue, RawPull, StateFilter, + Checks, Comment, Detail, Item, ItemState, Kind, PrFile, RawCheckRuns, RawCombinedStatus, + RawComment, RawFile, RawIssue, RawPull, RawReview, StateFilter, }; use super::remote::RepoSlug; @@ -267,12 +268,32 @@ pub fn detail(t: &dyn Transport, slug: &RepoSlug, number: u64) -> Result(&t.get(&format!("{base}/pulls/{number}"))?)?.into_item(); + let mut raw: RawPull = decode(&t.get(&format!("{base}/pulls/{number}"))?)?; + let teams = raw.requested_team_names(&slug.owner); + let requested = std::mem::take(&mut raw.requested_reviewers); + let (pr_item, info) = raw.into_item(); // `/pulls/{n}` is the one that knows about drafts; the issue's // labels and comment count are kept, which `/pulls` omits. item.state = pr_item.state; + // Checks and reviews are the panel's summary, not the pull request + // itself: one that cannot be read is left out rather than failing + // the whole view (an old commit's checks can be gone, a token can be + // scoped away from them). + if !info.head_sha.is_empty() { + checks_out = checks(t, slug, &info.head_sha) + .inspect_err(|e| log::warn!("github: checks of {}#{number}: {e}", slug.full())) + .ok(); + } + reviewers_out = t + .get(&format!( + "{base}/pulls/{number}/reviews?per_page={DETAIL_PAGE}" + )) + .and_then(|r| decode::>(&r)) + .inspect_err(|e| log::warn!("github: reviews of {}#{number}: {e}", slug.full())) + .ok() + .map(|reviews| super::model::reviewers(&item.author, reviews, requested, teams)); pull = Some(info); let reply = t.get(&format!( "{base}/pulls/{number}/files?per_page={DETAIL_PAGE}" @@ -294,6 +315,50 @@ pub fn detail(t: &dyn Transport, slug: &RepoSlug, number: u64) -> Result Result { + let base = repo_path(slug); + let sha = super::remote::escape_path(sha); + let runs: RawCheckRuns = decode(&t.get(&format!( + "{base}/commits/{sha}/check-runs?per_page={DETAIL_PAGE}" + ))?)?; + let statuses: RawCombinedStatus = decode(&t.get(&format!( + "{base}/commits/{sha}/status?per_page={DETAIL_PAGE}" + ))?)?; + Ok(super::model::checks(runs, statuses)) +} + +/// The pull request branch `branch` of `head_owner`'s fork (or of the +/// repository itself) opened against `slug`: the open one if there is one, +/// else the most recently updated. `None` when the branch has none. +pub fn pull_for_branch( + t: &dyn Transport, + slug: &RepoSlug, + head_owner: &str, + branch: &str, +) -> Result, ApiError> { + let head = escape_query(&format!("{head_owner}:{branch}")); + let path = format!( + "{}/pulls?state=all&head={head}&sort=updated&direction=desc&per_page=10", + repo_path(slug) + ); + let items: Vec = decode::>(&t.get(&path)?)? + .into_iter() + .map(|p| p.into_item().0) + .collect(); + let open = items + .iter() + .position(|i| matches!(i.state, ItemState::Open | ItemState::Draft)); + Ok(match open { + Some(i) => items.into_iter().nth(i), + None => items.into_iter().next(), }) } @@ -301,7 +366,7 @@ pub fn detail(t: &dyn Transport, slug: &RepoSlug, number: u64) -> Result = checks.items.iter().map(|c| c.name.as_str()).collect(); + assert_eq!( + names, + vec!["ci/deploy", "test", "lint", "docs"], + "failures first" + ); + assert_eq!( + checks.items[2].completed_at - checks.items[2].started_at, + 18 + ); + assert_eq!(checks.counted(), 3, "a skipped check has no verdict"); + assert_eq!(checks.rollup(), Some(CheckState::Failed)); + assert!(!checks.truncated); + + let reviewers = d.reviewers.as_ref().unwrap(); + assert_eq!( + reviewers + .iter() + .map(|r| (r.login.as_str(), r.state)) + .collect::>(), + vec![ + ("mara", ReviewState::Approved), + ("jonas", ReviewState::Requested), + ("l0ng-ai/core", ReviewState::Requested), + ], + "the author's own reply is not a review" + ); + } + + #[test] + fn checks_and_reviews_that_cannot_be_read_leave_the_detail_standing() { + let mut t = Fixture::new(); + pull_fixture(&mut t, PULL_31); + // Nothing answers for check runs, statuses or reviews. + let d = detail(&t, &slug(), 31).unwrap(); + assert!(d.pull.is_some()); + assert!(d.checks.is_none()); + assert!(d.reviewers.is_none()); + } + + #[test] + fn a_pull_request_without_a_head_sha_asks_for_no_checks() { + let mut t = Fixture::new(); + pull_fixture( + &mut t, + r#"{"number": 31, "title": "x", "state": "open", "head": {"ref": "gone"}}"#, + ); + let d = detail(&t, &slug(), 31).unwrap(); + assert!(d.checks.is_none()); + assert!( + !t.asked + .lock() + .unwrap() + .iter() + .any(|p| p.contains("/commits/")), + "no commit to ask about" + ); + } + + #[test] + fn a_branchs_pull_request_prefers_the_open_one() { + let mut t = Fixture::new(); + t.on( + "/repos/l0ng-ai/tty7/pulls?state=all&head=bob%3Afeat%2Fpanel&sort=updated&direction=desc&per_page=10", + r#"[{"number": 20, "title": "Second try", "state": "closed", "merged_at": null}, + {"number": 13, "title": "Add panel", "state": "open", "draft": true}]"#, + false, + ); + let got = pull_for_branch(&t, &slug(), "bob", "feat/panel") + .unwrap() + .unwrap(); + assert_eq!(got.number, 13); + assert_eq!(got.state, ItemState::Draft); + + let mut t = Fixture::new(); + t.on( + "/repos/l0ng-ai/tty7/pulls?state=all&head=bob%3Aold&sort=updated&direction=desc&per_page=10", + r#"[{"number": 20, "title": "Merged", "state": "closed", "merged_at": "2026-09-01T10:00:00Z"}]"#, + false, + ); + assert_eq!( + pull_for_branch(&t, &slug(), "bob", "old") + .unwrap() + .unwrap() + .number, + 20, + "no open one: the latest" + ); + + let mut t = Fixture::new(); + t.on( + "/repos/l0ng-ai/tty7/pulls?state=all&head=bob%3Anone&sort=updated&direction=desc&per_page=10", + "[]", + false, + ); + assert_eq!(pull_for_branch(&t, &slug(), "bob", "none"), Ok(None)); + } + #[test] fn a_failed_sub_request_fails_the_detail() { let mut t = Fixture::new(); diff --git a/crates/tty7-core/src/core/github/mod.rs b/crates/tty7-core/src/core/github/mod.rs index fea56f6a..b4977b55 100644 --- a/crates/tty7-core/src/core/github/mod.rs +++ b/crates/tty7-core/src/core/github/mod.rs @@ -16,6 +16,9 @@ pub mod remote; pub mod token; pub use api::{ApiError, ListPage, ListQuery, Reply, Transport}; -pub use model::{Comment, Detail, Item, ItemState, Kind, Label, PrFile, PullInfo, StateFilter}; +pub use model::{ + Check, CheckState, Checks, Comment, Detail, Item, ItemState, Kind, Label, MergeState, PrFile, + PullInfo, Readiness, ReviewState, Reviewer, StateFilter, readiness, +}; pub use remote::{GitHubRemote, RepoSlug}; pub use token::{Token, TokenSource}; diff --git a/crates/tty7-core/src/core/github/model.rs b/crates/tty7-core/src/core/github/model.rs index 43bd0b4a..9c54014d 100644 --- a/crates/tty7-core/src/core/github/model.rs +++ b/crates/tty7-core/src/core/github/model.rs @@ -86,11 +86,178 @@ pub struct Comment { #[derive(Clone, Debug, PartialEq, Eq, Default)] pub struct PullInfo { pub head_ref: String, + /// The head commit — what the checks ran on. Empty if GitHub left it out. + pub head_sha: String, pub base_ref: String, pub additions: u32, pub deletions: u32, pub changed_files: u32, pub commits: u32, + pub merge_state: MergeState, +} + +/// GitHub's `mergeable_state`: whether the pull request could be merged now, +/// and if not, the broad reason. GitHub computes it lazily, so the first read +/// of a freshly pushed pull request is often `Unknown`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub enum MergeState { + /// Mergeable, everything required has passed. + Clean, + /// Mergeable, but a check that is not required is failing. + Unstable, + /// Branch protection says no: a required check or review is missing. + Blocked, + /// The base branch has moved on and protection requires being up to date. + Behind, + /// Merge conflicts. + Dirty, + Draft, + #[default] + Unknown, +} + +impl MergeState { + fn parse(s: Option<&str>) -> MergeState { + match s { + // "has_hooks" is clean with a pre-receive hook in the way — for + // the panel's purposes, clean. + Some("clean") | Some("has_hooks") => MergeState::Clean, + Some("unstable") => MergeState::Unstable, + Some("blocked") => MergeState::Blocked, + Some("behind") => MergeState::Behind, + Some("dirty") => MergeState::Dirty, + Some("draft") => MergeState::Draft, + _ => MergeState::Unknown, + } + } +} + +/// Where one CI check stands. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum CheckState { + // Declared in the order the panel lists them: what needs attention first. + Failed, + Pending, + Passed, + /// Skipped, neutral, or stale — ran (or was never going to) without a + /// verdict either way. Not counted toward "N of M passed". + Skipped, +} + +/// One CI check on a pull request's head commit: a check run (GitHub Actions +/// and any other GitHub App) or a commit status (the older API some external +/// CI still reports through). GitHub's own page lists both side by side. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Check { + pub name: String, + pub state: CheckState, + /// Unix seconds, zero when unknown. A commit status has no start time. + pub started_at: i64, + pub completed_at: i64, + /// Its log or details page, when there is one. + pub url: Option, +} + +/// Every check on a head commit, the ones that need attention first. +#[derive(Clone, Debug, PartialEq, Eq, Default)] +pub struct Checks { + pub items: Vec, + /// More checks exist than were fetched. + pub truncated: bool, +} + +impl Checks { + pub fn count(&self, state: CheckState) -> usize { + self.items.iter().filter(|c| c.state == state).count() + } + + /// The checks with a verdict to give: everything but the skipped ones. + pub fn counted(&self) -> usize { + self.items.len() - self.count(CheckState::Skipped) + } + + /// The whole set in one state, the way a list row or a badge shows it: + /// any failure fails it, else anything running keeps it pending. + pub fn rollup(&self) -> Option { + if self.items.is_empty() { + return None; + } + [CheckState::Failed, CheckState::Pending, CheckState::Passed] + .into_iter() + .find(|s| self.count(*s) > 0) + .or(Some(CheckState::Skipped)) + } + + fn sort(&mut self) { + // Stable: within a state, GitHub's own order (by run, then context). + self.items.sort_by_key(|c| c.state); + } +} + +/// Where one reviewer stands on a pull request. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum ReviewState { + Approved, + ChangesRequested, + /// Left review comments without a verdict. + Commented, + /// Asked for a review that has not come in (or was asked again after one). + Requested, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Reviewer { + /// A user's login, or `org/team` for a requested team. + pub login: String, + pub state: ReviewState, +} + +/// Whether an open pull request can be merged, and if not, the one thing +/// most worth saying about why. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Readiness { + Ready, + Conflicts, + ChecksFailing(usize), + WaitingOnChecks(usize), + ChangesRequested, + ReviewRequired, + Behind, + Blocked, +} + +/// Folds the merge state, the checks and the reviews into one line's worth, +/// the way GitHub's merge box leads with its most pressing reason. `None` for +/// anything but an open, non-draft pull request, and while GitHub has not +/// worked the merge state out yet. +pub fn readiness( + state: ItemState, + merge: MergeState, + checks: Option<&Checks>, + reviewers: Option<&[Reviewer]>, +) -> Option { + if state != ItemState::Open { + return None; + } + let count = |s| checks.map_or(0, |c| c.count(s)); + let has = |s| reviewers.is_some_and(|r| r.iter().any(|r| r.state == s)); + Some(match merge { + MergeState::Unknown | MergeState::Draft => return None, + MergeState::Dirty => Readiness::Conflicts, + _ if count(CheckState::Failed) > 0 && merge != MergeState::Clean => { + Readiness::ChecksFailing(count(CheckState::Failed)) + } + _ if count(CheckState::Pending) > 0 && merge != MergeState::Clean => { + Readiness::WaitingOnChecks(count(CheckState::Pending)) + } + _ if has(ReviewState::ChangesRequested) && merge != MergeState::Clean => { + Readiness::ChangesRequested + } + MergeState::Behind => Readiness::Behind, + MergeState::Clean | MergeState::Unstable => Readiness::Ready, + MergeState::Blocked if !has(ReviewState::Approved) => Readiness::ReviewRequired, + MergeState::Blocked => Readiness::Blocked, + }) } /// One file a pull request touches, from `/pulls/{n}/files`. @@ -157,6 +324,12 @@ pub struct Detail { /// `Some` for a pull request. pub files: Option>, pub files_truncated: bool, + /// The head commit's CI. `None` for an issue, and for a pull request + /// whose checks could not be read — a detail does not fail over them. + pub checks: Option, + /// Reviews in and reviews asked for, one entry per reviewer. `None` as + /// for `checks`. + pub reviewers: Option>, } // ---- wire ------------------------------------------------------------------ @@ -219,6 +392,14 @@ pub(crate) struct RawIssue { pub(crate) struct RawBranchRef { #[serde(default, rename = "ref")] name: String, + #[serde(default)] + sha: String, +} + +#[derive(Deserialize)] +pub(crate) struct RawTeam { + #[serde(default)] + slug: String, } /// An entry of `/pulls`, or `/pulls/{n}`. @@ -257,6 +438,13 @@ pub(crate) struct RawPull { changed_files: Option, #[serde(default)] commits: Option, + /// Only on `/pulls/{n}`. + #[serde(default)] + mergeable_state: Option, + #[serde(default)] + pub(crate) requested_reviewers: Vec, + #[serde(default)] + pub(crate) requested_teams: Vec, } #[derive(Deserialize)] @@ -292,6 +480,60 @@ pub(crate) struct RawFile { patch: Option, } +#[derive(Deserialize)] +pub(crate) struct RawReview { + #[serde(default)] + user: Option, + #[serde(default)] + state: String, +} + +/// `/commits/{sha}/check-runs`. +#[derive(Deserialize)] +pub(crate) struct RawCheckRuns { + #[serde(default)] + pub(crate) total_count: usize, + #[serde(default)] + pub(crate) check_runs: Vec, +} + +#[derive(Deserialize)] +pub(crate) struct RawCheckRun { + #[serde(default)] + name: String, + #[serde(default)] + status: String, + #[serde(default)] + conclusion: Option, + #[serde(default)] + started_at: Option, + #[serde(default)] + completed_at: Option, + #[serde(default)] + html_url: Option, +} + +/// `/commits/{sha}/status`: the latest status per context. +#[derive(Deserialize)] +pub(crate) struct RawCombinedStatus { + #[serde(default)] + pub(crate) total_count: usize, + #[serde(default)] + pub(crate) statuses: Vec, +} + +#[derive(Deserialize)] +pub(crate) struct RawStatus { + #[serde(default)] + context: String, + #[serde(default)] + state: String, + #[serde(default)] + target_url: Option, + #[serde(default)] + updated_at: String, +} + fn login(user: Option) -> String { user.map(|u| u.login).unwrap_or_default() } @@ -355,13 +597,16 @@ impl RawPull { self.merged_at.is_some(), self.draft.unwrap_or(false), ); + let (head_ref, head_sha) = self.head.map(|h| (h.name, h.sha)).unwrap_or_default(); let info = PullInfo { - head_ref: self.head.map(|h| h.name).unwrap_or_default(), + head_ref, + head_sha, base_ref: self.base.map(|b| b.name).unwrap_or_default(), additions: self.additions.unwrap_or(0), deletions: self.deletions.unwrap_or(0), changed_files: self.changed_files.unwrap_or(0), commits: self.commits.unwrap_or(0), + merge_state: MergeState::parse(self.mergeable_state.as_deref()), }; let item = Item { number: self.number, @@ -379,6 +624,134 @@ impl RawPull { } } +impl RawCheckRun { + pub(crate) fn into_check(self) -> Check { + let state = match (self.status.as_str(), self.conclusion.as_deref()) { + ("completed", Some("success")) => CheckState::Passed, + ("completed", Some("skipped" | "neutral" | "stale")) => CheckState::Skipped, + // failure, cancelled, timed_out, action_required, startup_failure + ("completed", _) => CheckState::Failed, + // queued, in_progress, waiting, requested, pending + _ => CheckState::Pending, + }; + Check { + name: self.name, + state, + started_at: self.started_at.as_deref().map_or(0, timestamp), + completed_at: self.completed_at.as_deref().map_or(0, timestamp), + url: self.html_url.filter(|u| !u.is_empty()), + } + } +} + +impl RawStatus { + pub(crate) fn into_check(self) -> Check { + let state = match self.state.as_str() { + "success" => CheckState::Passed, + "pending" => CheckState::Pending, + // failure, error + _ => CheckState::Failed, + }; + Check { + name: self.context, + state, + started_at: 0, + completed_at: if state == CheckState::Pending { + 0 + } else { + timestamp(&self.updated_at) + }, + url: self.target_url.filter(|u| !u.is_empty()), + } + } +} + +/// Check runs and commit statuses as one list, the ones needing attention +/// first. `/check-runs` already keeps only each check's latest run, and +/// `/status` each context's latest status. +pub(crate) fn checks(runs: RawCheckRuns, statuses: RawCombinedStatus) -> Checks { + let truncated = + runs.total_count > runs.check_runs.len() || statuses.total_count > statuses.statuses.len(); + let mut checks = Checks { + items: runs + .check_runs + .into_iter() + .map(RawCheckRun::into_check) + .chain(statuses.statuses.into_iter().map(RawStatus::into_check)) + .collect(), + truncated, + }; + checks.sort(); + checks +} + +/// Each reviewer's standing, from the reviews in (oldest first, as GitHub +/// lists them) and the reviews still asked for. +/// +/// A verdict sticks until the same reviewer gives another or it is dismissed; +/// a later plain comment does not undo an approval, the way GitHub's own +/// sidebar reads. Asking again puts a reviewer back to "requested" whatever +/// they said before. The author's replies in their own review threads are +/// reviews by GitHub's count, but not a reviewer. +pub(crate) fn reviewers( + author: &str, + reviews: Vec, + requested_users: Vec, + requested_teams: Vec, +) -> Vec { + let mut out: Vec = Vec::new(); + for review in reviews { + let login = login(review.user); + if login.is_empty() || login == author { + continue; + } + let verdict = match review.state.as_str() { + "APPROVED" => Some(ReviewState::Approved), + "CHANGES_REQUESTED" => Some(ReviewState::ChangesRequested), + "COMMENTED" | "DISMISSED" => None, + // PENDING: the viewer's own unsubmitted review. + _ => continue, + }; + match out.iter_mut().find(|r| r.login == login) { + Some(r) => match verdict { + Some(v) => r.state = v, + None if review.state == "DISMISSED" => r.state = ReviewState::Commented, + None => {} + }, + None => out.push(Reviewer { + login, + state: verdict.unwrap_or(ReviewState::Commented), + }), + } + } + let requested = requested_users + .into_iter() + .map(|u| u.login) + .chain(requested_teams) + .filter(|l| !l.is_empty()); + for login in requested { + match out.iter_mut().find(|r| r.login == login) { + Some(r) => r.state = ReviewState::Requested, + None => out.push(Reviewer { + login, + state: ReviewState::Requested, + }), + } + } + out +} + +impl RawPull { + /// The teams asked to review, as `org/team` when the org is known. + pub(crate) fn requested_team_names(&self, org: &str) -> Vec { + self.requested_teams + .iter() + .filter(|t| !t.slug.is_empty()) + .map(|t| format!("{org}/{}", t.slug)) + .collect() + } +} + impl RawComment { pub(crate) fn into_comment(self) -> Comment { Comment { @@ -491,6 +864,196 @@ mod tests { assert_eq!(timestamp("garbage"), 0); } + fn review(login: &str, state: &str) -> RawReview { + RawReview { + user: Some(RawUser { + login: login.into(), + }), + state: state.into(), + } + } + + fn states(r: &[Reviewer]) -> Vec<(&str, ReviewState)> { + r.iter().map(|r| (r.login.as_str(), r.state)).collect() + } + + #[test] + fn a_verdict_sticks_until_the_reviewer_gives_another() { + let got = reviewers( + "author", + vec![ + review("mara", "CHANGES_REQUESTED"), + review("mara", "APPROVED"), + review("mara", "COMMENTED"), + review("jonas", "COMMENTED"), + review("kai", "APPROVED"), + review("kai", "DISMISSED"), + review("author", "COMMENTED"), + review("lee", "PENDING"), + ], + Vec::new(), + Vec::new(), + ); + assert_eq!( + states(&got), + vec![ + ("mara", ReviewState::Approved), + ("jonas", ReviewState::Commented), + ("kai", ReviewState::Commented), + ] + ); + } + + #[test] + fn asking_again_puts_a_reviewer_back_to_requested() { + let got = reviewers( + "author", + vec![review("mara", "CHANGES_REQUESTED")], + vec![RawUser { + login: "mara".into(), + }], + vec!["acme/core".into()], + ); + assert_eq!( + states(&got), + vec![ + ("mara", ReviewState::Requested), + ("acme/core", ReviewState::Requested), + ] + ); + } + + fn run(status: &str, conclusion: Option<&str>) -> RawCheckRun { + RawCheckRun { + name: format!("{status}/{conclusion:?}"), + status: status.into(), + conclusion: conclusion.map(str::to_string), + started_at: None, + completed_at: None, + html_url: None, + } + } + + #[test] + fn check_runs_fold_into_four_states() { + for (status, conclusion, want) in [ + ("completed", Some("success"), CheckState::Passed), + ("completed", Some("failure"), CheckState::Failed), + ("completed", Some("cancelled"), CheckState::Failed), + ("completed", Some("timed_out"), CheckState::Failed), + ("completed", Some("action_required"), CheckState::Failed), + ("completed", Some("skipped"), CheckState::Skipped), + ("completed", Some("neutral"), CheckState::Skipped), + ("queued", None, CheckState::Pending), + ("in_progress", None, CheckState::Pending), + ("waiting", None, CheckState::Pending), + ] { + assert_eq!( + run(status, conclusion).into_check().state, + want, + "{status} {conclusion:?}" + ); + } + } + + fn checks_of(states: &[CheckState]) -> Checks { + Checks { + items: states + .iter() + .map(|s| Check { + name: String::new(), + state: *s, + started_at: 0, + completed_at: 0, + url: None, + }) + .collect(), + truncated: false, + } + } + + #[test] + fn a_rollup_is_failed_before_pending_before_passed() { + use CheckState::*; + assert_eq!(checks_of(&[]).rollup(), None); + assert_eq!(checks_of(&[Passed, Pending, Failed]).rollup(), Some(Failed)); + assert_eq!( + checks_of(&[Passed, Pending, Skipped]).rollup(), + Some(Pending) + ); + assert_eq!(checks_of(&[Passed, Skipped]).rollup(), Some(Passed)); + assert_eq!(checks_of(&[Skipped]).rollup(), Some(Skipped)); + } + + #[test] + fn a_merge_box_leads_with_its_most_pressing_reason() { + use CheckState::*; + let approved = [Reviewer { + login: "mara".into(), + state: ReviewState::Approved, + }]; + let changes = [Reviewer { + login: "mara".into(), + state: ReviewState::ChangesRequested, + }]; + let r = |merge, checks: &[CheckState], reviews: &[Reviewer]| { + readiness( + ItemState::Open, + merge, + Some(&checks_of(checks)), + Some(reviews), + ) + }; + assert_eq!( + r(MergeState::Blocked, &[Passed, Pending], &approved), + Some(Readiness::WaitingOnChecks(1)) + ); + assert_eq!( + r(MergeState::Blocked, &[Failed, Failed, Pending], &approved), + Some(Readiness::ChecksFailing(2)) + ); + assert_eq!( + r(MergeState::Dirty, &[Failed], &approved), + Some(Readiness::Conflicts) + ); + assert_eq!( + r(MergeState::Blocked, &[Passed], &changes), + Some(Readiness::ChangesRequested) + ); + assert_eq!( + r(MergeState::Blocked, &[Passed], &[]), + Some(Readiness::ReviewRequired) + ); + assert_eq!( + r(MergeState::Blocked, &[Passed], &approved), + Some(Readiness::Blocked) + ); + assert_eq!( + r(MergeState::Behind, &[Passed], &approved), + Some(Readiness::Behind) + ); + // Clean means everything required is in, whatever optional check is + // still running. + assert_eq!( + r(MergeState::Clean, &[Pending], &[]), + Some(Readiness::Ready) + ); + assert_eq!( + r(MergeState::Unstable, &[Failed], &[]), + Some(Readiness::ChecksFailing(1)) + ); + assert_eq!(r(MergeState::Unknown, &[Passed], &approved), None); + assert_eq!( + readiness(ItemState::Draft, MergeState::Clean, None, None), + None, + "a draft is not up for merging" + ); + assert_eq!( + readiness(ItemState::Merged, MergeState::Clean, None, None), + None + ); + } + fn file(patch: Option<&str>, additions: u32, deletions: u32) -> PrFile { PrFile { path: "src/a file.rs".into(), diff --git a/src/ui/assets.rs b/src/ui/assets.rs index eaeb6eda..ac9c095e 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -117,6 +117,19 @@ fn agent_icon(path: &str) -> Option<&'static [u8]> { "icons/github/pr-closed.svg" => include_bytes!("../../assets/icons/github/pr-closed.svg"), "icons/github/pr-merged.svg" => include_bytes!("../../assets/icons/github/pr-merged.svg"), "icons/github/pr-draft.svg" => include_bytes!("../../assets/icons/github/pr-draft.svg"), + // …and its checks': passed, failed, running, skipped. + "icons/github/check-passed.svg" => { + include_bytes!("../../assets/icons/github/check-passed.svg") + } + "icons/github/check-failed.svg" => { + include_bytes!("../../assets/icons/github/check-failed.svg") + } + "icons/github/check-pending.svg" => { + include_bytes!("../../assets/icons/github/check-pending.svg") + } + "icons/github/check-skipped.svg" => { + include_bytes!("../../assets/icons/github/check-skipped.svg") + } _ => return None, }; Some(bytes) diff --git a/src/ui/github/detail.rs b/src/ui/github/detail.rs index fa63daed..42b4bdcc 100644 --- a/src/ui/github/detail.rs +++ b/src/ui/github/detail.rs @@ -3,9 +3,10 @@ //! Title, state, author, labels, the body and the conversation as Markdown — //! through gpui-component's `TextView`, after `core::github::markdown` has //! turned images into links and disarmed non-web link targets — and, for a -//! pull request, its branches and changed files. A file opens in the diff -//! overlay, the same surface a commit's files open in, fed the patch GitHub -//! sent rather than one git read. +//! pull request, its branches, where it stands on merging, its checks, its +//! reviewers and its changed files. A file opens in the diff overlay, the +//! same surface a commit's files open in, fed the patch GitHub sent rather +//! than one git read. use std::sync::Arc; @@ -13,12 +14,17 @@ use gpui::{AnyElement, Context, SharedString, Window, div, prelude::*, px, rems} use gpui_component::{ActiveTheme as _, Icon, IconName, Sizable as _, h_flex, v_flex}; use tty7_core::core::git::diff::{CommitLabel, DiffBudget, DiffSnapshot, DiffSource}; -use tty7_core::core::github::{Comment, Detail, PrFile, RepoSlug}; +use tty7_core::core::github::{ + Check, CheckState, Checks, Comment, Detail, PrFile, Readiness, RepoSlug, ReviewState, Reviewer, + readiness, +}; use crate::ui::app::{CONTENT_INSET, Tty7App}; -use crate::ui::github::now_unix; +use crate::ui::github::{Fold, now_unix}; use crate::ui::i18n::{L10nKey, t, t_fmt, t_plural}; -use crate::ui::panel_github::{describe_error, github_tile, label_chip, state_glyph, state_label}; +use crate::ui::panel_github::{ + check_glyph, check_label, describe_error, github_tile, label_chip, state_glyph, state_label, +}; use crate::ui::right_panel::{ HEADING, META, META_MONO, ROW_FILL_RADIUS, ROW_INSET, TEXT, TEXT_INSET, git_badge, }; @@ -28,6 +34,27 @@ use crate::ui::scm::status::{status_color, status_glyph}; const ROW_H: f32 = 26.; const SECTION_GAP: f32 = 16.; +/// Past this many checks, the ones that passed or were skipped fold into one +/// row: a repository with a build matrix has dozens, and the one that failed +/// is what the section is read for. +const CHECKS_FOLD_AT: usize = 5; +/// Past this many changed files, only the first [`FILES_SHOWN_FOLDED`] show +/// until the list is unfolded — the conversation under it stays in reach. +const FILES_FOLD_AT: usize = 10; +const FILES_SHOWN_FOLDED: usize = 8; +/// Past this many reviewers, only the first few show. +const REVIEWERS_FOLD_AT: usize = 5; +/// Past this many comments, the middle folds away the way GitHub's own +/// timeline does: the opening one stays, and the latest few. +const COMMENTS_FOLD_AT: usize = 4; +const COMMENTS_KEPT_LATEST: usize = 2; +/// A description or comment longer than this is clamped to [`CLAMP_HEIGHT`] +/// until unfolded. Judged on the source, since the rendered height is only +/// known after layout; a pasted log or a long template trips it, a few +/// paragraphs do not. +const CLAMP_LINES: usize = 18; +const CLAMP_CHARS: usize = 1600; +const CLAMP_HEIGHT: f32 = 16.; impl Tty7App { /// The rows pinned over the detail, and the detail itself. @@ -74,13 +101,34 @@ impl Tty7App { let mut body = v_flex() .pb(px(16.)) - .child(self.github_detail_head(&detail, cx)) - .child(markdown_block( - format!("gh-body-{}-{number}", slug.full()), - &detail.body, - t(L10nKey::GitHubNoDescription), + .child(self.github_detail_head(&detail, cx)); + // What a pull request is usually opened to find out — can it go in, + // and if not, what is it waiting for — before what it says. + if let Some(checks) = detail.checks.as_ref().filter(|c| !c.items.is_empty()) { + let unfolded = self.github_unfolded(slug, number, Fold::Checks); + let toggle = self.github_fold_toggle(slug, number, Fold::Checks, cx); + body = body.child(checks_section( + checks, + &detail.item.html_url, + unfolded, + toggle, cx, )); + } + if let Some(reviewers) = detail.reviewers.as_ref().filter(|r| !r.is_empty()) { + let unfolded = self.github_unfolded(slug, number, Fold::Reviews); + let toggle = self.github_fold_toggle(slug, number, Fold::Reviews, cx); + body = body.child(reviews_section(reviewers, unfolded, toggle, cx)); + } + body = body.child(self.github_long_markdown( + format!("gh-body-{}-{number}", slug.full()), + &detail.body, + t(L10nKey::GitHubNoDescription), + slug, + number, + Fold::Body, + cx, + )); if let Some(err) = error { // A refresh that failed over a detail already on screen: keep the // detail, and say why it may be out of date. @@ -94,6 +142,26 @@ impl Tty7App { (pinned, body.into_any_element()) } + fn github_unfolded(&self, slug: &RepoSlug, number: u64, fold: Fold) -> bool { + self.github.unfolded.contains(&(slug.clone(), number, fold)) + } + + fn github_fold_toggle( + &self, + slug: &RepoSlug, + number: u64, + fold: Fold, + cx: &mut Context, + ) -> impl Fn(&gpui::ClickEvent, &mut Window, &mut gpui::App) + 'static { + let key = (slug.clone(), number, fold); + cx.listener(move |this, _, _window, cx| { + if !this.github.unfolded.remove(&key) { + this.github.unfolded.insert(key.clone()); + } + cx.notify(); + }) + } + fn github_back_row( &self, repo: &RepoKey, @@ -247,6 +315,15 @@ impl Tty7App { .child(t_plural(L10nKey::GitHubCommits, pull.commits as usize, &[])), ), ); + let ready = readiness( + item.state, + pull.merge_state, + detail.checks.as_ref(), + detail.reviewers.as_deref(), + ); + if let Some(ready) = ready { + head = head.child(readiness_line(ready, cx)); + } } if !item.labels.is_empty() { head = head.child( @@ -274,8 +351,53 @@ impl Tty7App { t_plural(L10nKey::GitHubComments, count, &[]), cx, )); - for c in &detail.comments { - section = section.child(comment_block(slug, number, c, now, fg, muted, cx)); + let total = detail.comments.len(); + let foldable = total > COMMENTS_FOLD_AT; + let unfolded = self.github_unfolded(slug, number, Fold::Comments); + let hidden = if foldable && !unfolded { + 1..total - COMMENTS_KEPT_LATEST + } else { + 0..0 + }; + for (i, c) in detail.comments.iter().enumerate() { + if hidden.contains(&i) { + if i == hidden.start { + let toggle = self.github_fold_toggle(slug, number, Fold::Comments, cx); + let text = t_plural(L10nKey::GitHubShowHiddenComments, hidden.len(), &[]); + section = section.child( + div().pt(px(10.)).px(px(CONTENT_INSET)).child( + fold_row("panel-github-fold-comments", None, text, false, cx) + .on_click(toggle), + ), + ); + } + continue; + } + let body = self.github_long_markdown( + format!("gh-comment-{}-{number}-{}", slug.full(), c.id), + &c.body, + "", + slug, + number, + Fold::Comment(c.id), + cx, + ); + section = section.child(comment_block(c, now, fg, muted, body)); + } + if foldable && unfolded { + let toggle = self.github_fold_toggle(slug, number, Fold::Comments, cx); + section = section.child( + div().pt(px(10.)).px(px(CONTENT_INSET)).child( + fold_row( + "panel-github-fold-comments", + None, + t(L10nKey::GitHubShowLess).to_string(), + true, + cx, + ) + .on_click(toggle), + ), + ); } if detail.comments_truncated { section = section.child(more_on_github( @@ -287,6 +409,55 @@ impl Tty7App { section.into_any_element() } + /// Markdown that is clamped, with a row to unfold it, when its source + /// runs long. + #[allow(clippy::too_many_arguments)] + fn github_long_markdown( + &self, + id: String, + source: &str, + empty: &str, + slug: &RepoSlug, + number: u64, + fold: Fold, + cx: &mut Context, + ) -> AnyElement { + let block = markdown_block(id.clone(), source, empty, cx); + if !runs_long(source) { + return block; + } + let unfolded = self.github_unfolded(slug, number, fold); + let toggle = self.github_fold_toggle(slug, number, fold, cx); + let text = if unfolded { + t(L10nKey::GitHubShowLess) + } else { + t(L10nKey::GitHubShowFullText) + }; + v_flex() + .child(if unfolded { + block + } else { + div() + .max_h(rems(CLAMP_HEIGHT)) + .overflow_hidden() + .child(block) + .into_any_element() + }) + .child( + div().pt(px(4.)).px(px(CONTENT_INSET)).child( + fold_row( + SharedString::from(format!("{id}-fold")), + None, + text.to_string(), + unfolded, + cx, + ) + .on_click(toggle), + ), + ) + .into_any_element() + } + fn github_detail_files( &self, repo: &RepoKey, @@ -339,13 +510,37 @@ impl Tty7App { .child(div().text_color(added_ink).child(format!("+{added}"))) .child(div().text_color(removed_ink).child(format!("−{removed}"))), ); + let number = detail.item.number; + let foldable = files.len() > FILES_FOLD_AT; + let unfolded = self.github_unfolded(slug, number, Fold::Files); + // The file open in the diff overlay stays listed, folded or not. + let shown = |i: usize, f: &PrFile| { + !foldable + || unfolded + || i < FILES_SHOWN_FOLDED + || focused.as_deref() == Some(f.path.as_str()) + }; let mut rows = v_flex().px(px(CONTENT_INSET)); for (i, file) in files.iter().enumerate() { + if !shown(i, file) { + continue; + } let selected = focused.as_deref() == Some(file.path.as_str()); rows = rows.child( self.github_file_row(i, file, selected, repo, &source, &head_ref, &files, cx), ); } + if foldable { + let text = if unfolded { + t(L10nKey::GitHubShowLess).to_string() + } else { + t_plural(L10nKey::GitHubShowAllFiles, files.len(), &[]) + }; + let toggle = self.github_fold_toggle(slug, number, Fold::Files, cx); + rows = rows.child( + fold_row("panel-github-fold-files", None, text, unfolded, cx).on_click(toggle), + ); + } let mut section = v_flex().mt(px(SECTION_GAP)).child(heading).child(rows); if detail.files_truncated { section = section.child(more_on_github( @@ -460,6 +655,351 @@ impl Tty7App { } } +/// The merge box's one line: a glyph in the colour of how close it is, and +/// the most pressing reason it is not in yet. +fn readiness_line(ready: Readiness, cx: &gpui::App) -> AnyElement { + let theme = cx.theme(); + let (glyph, ink, text) = match ready { + Readiness::Ready => ( + CheckState::Passed, + theme.success, + t(L10nKey::GitHubReadyToMerge).to_string(), + ), + Readiness::Conflicts => ( + CheckState::Failed, + theme.danger, + t(L10nKey::GitHubMergeConflicts).to_string(), + ), + Readiness::ChecksFailing(n) => ( + CheckState::Failed, + theme.danger, + t_plural(L10nKey::GitHubChecksFailing, n, &[]), + ), + Readiness::ChangesRequested => ( + CheckState::Failed, + theme.danger, + t(L10nKey::GitHubReviewChangesRequested).to_string(), + ), + Readiness::WaitingOnChecks(n) => ( + CheckState::Pending, + theme.warning, + t_plural(L10nKey::GitHubWaitingOnChecks, n, &[]), + ), + Readiness::ReviewRequired => ( + CheckState::Pending, + theme.warning, + t(L10nKey::GitHubReviewRequired).to_string(), + ), + Readiness::Behind => ( + CheckState::Pending, + theme.warning, + t(L10nKey::GitHubBehindBase).to_string(), + ), + Readiness::Blocked => ( + CheckState::Failed, + theme.warning, + t(L10nKey::GitHubMergeBlocked).to_string(), + ), + }; + h_flex() + .items_center() + .gap(px(6.)) + .child(check_glyph(glyph, cx)) + .child( + div() + .text_size(rems(META)) + .font_weight(gpui::FontWeight::MEDIUM) + .text_color(ink) + .child(text), + ) + .into_any_element() +} + +/// `18s`, `1m 12s`, `14m`, `2h 5m` — as short as GitHub's own check list. +pub(crate) fn short_duration(secs: i64) -> String { + let secs = secs.max(0); + let (h, m, s) = (secs / 3600, secs / 60 % 60, secs % 60); + match (h, m, s) { + (0, 0, s) => format!("{s}s"), + (0, m, 0) => format!("{m}m"), + (0, m, s) if m < 10 => format!("{m}m {s}s"), + (0, m, _) => format!("{m}m"), + (h, 0, _) => format!("{h}h"), + (h, m, _) => format!("{h}h {m}m"), + } +} + +/// How long a check took, or has been running. Nothing for a check that +/// never said when it started (a commit status, a queued run). +fn check_duration(check: &Check, now: i64) -> Option { + if check.started_at <= 0 { + return None; + } + let end = match check.state { + CheckState::Pending => now, + _ if check.completed_at >= check.started_at => check.completed_at, + _ => return None, + }; + Some(short_duration(end - check.started_at)) +} + +fn checks_section( + checks: &Checks, + html_url: &str, + unfolded: bool, + toggle: impl Fn(&gpui::ClickEvent, &mut Window, &mut gpui::App) + 'static, + cx: &gpui::App, +) -> AnyElement { + let theme = cx.theme(); + let muted = theme.muted_foreground; + let mono = theme.mono_font_family.clone(); + let sf = cx.global::().sidebar; + let now = now_unix(); + let counted = checks.counted(); + let summary = if counted == 0 { + t(L10nKey::GitHubChecksNoneCounted).to_string() + } else { + t_fmt( + L10nKey::GitHubChecksPassed, + &[ + ("passed", &checks.count(CheckState::Passed).to_string()), + ("total", &counted.to_string()), + ], + ) + }; + let heading = h_flex() + .items_center() + .gap(px(6.)) + .min_h(px(22.)) + .px(px(TEXT_INSET)) + .child( + div() + .text_size(rems(HEADING)) + .font_weight(gpui::FontWeight::MEDIUM) + .text_color(muted) + .child(t(L10nKey::GitHubChecks)), + ) + .child(div().flex_1()) + .child(div().text_size(rems(META)).text_color(muted).child(summary)); + let foldable = checks.items.len() > CHECKS_FOLD_AT; + let folded = foldable && !unfolded; + let quiet = |s: CheckState| matches!(s, CheckState::Passed | CheckState::Skipped); + let mut rows = v_flex().px(px(CONTENT_INSET)); + for (i, check) in checks.items.iter().enumerate() { + if folded && quiet(check.state) { + continue; + } + let duration = check_duration(check, now); + let state = check_label(check.state); + let mut row = h_flex() + .id(("panel-github-check", i)) + .items_center() + .gap(px(8.)) + .h(px(ROW_H)) + .w_full() + .min_w_0() + .px(px(ROW_INSET)) + .rounded(ROW_FILL_RADIUS) + .tooltip(move |window, cx| { + gpui_component::tooltip::Tooltip::new(state).build(window, cx) + }) + .child(check_glyph(check.state, cx)) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(TEXT)) + .text_color(if check.state == CheckState::Skipped { + muted + } else { + gpui::rgb(sf.text_resting).into() + }) + .child(check.name.clone()), + ) + .children(duration.map(|d| { + div() + .flex_none() + .text_size(rems(META_MONO)) + .font_family(mono.clone()) + .text_color(muted) + .child(d) + })); + if let Some(url) = check.url.clone() { + row = row + .cursor_pointer() + .hover(|s| s.bg(gpui::rgb(sf.hover))) + .on_click(move |_, _window, cx| cx.open_url(&url)); + } + rows = rows.child(row); + } + if foldable { + let (passed, skipped) = ( + checks.count(CheckState::Passed), + checks.count(CheckState::Skipped), + ); + let (glyph, text) = if unfolded { + (None, t(L10nKey::GitHubShowLess).to_string()) + } else { + let mut parts = Vec::new(); + if passed > 0 { + parts.push(t_plural(L10nKey::GitHubPassedCount, passed, &[])); + } + if skipped > 0 { + parts.push(t_plural(L10nKey::GitHubSkippedCount, skipped, &[])); + } + let glyph = if passed > 0 { + CheckState::Passed + } else { + CheckState::Skipped + }; + (Some(glyph), parts.join(" · ")) + }; + rows = rows.child( + fold_row("panel-github-fold-checks", glyph, text, unfolded, cx).on_click(toggle), + ); + } + let mut section = v_flex().mt(px(SECTION_GAP)).child(heading).child(rows); + if checks.truncated { + section = section.child(more_on_github( + "panel-github-more-checks", + format!("{html_url}/checks"), + cx, + )); + } + section.into_any_element() +} + +/// The row a folded section ends in: what is tucked away (or "show less"), +/// and a chevron saying which way it goes. +fn fold_row( + id: impl Into, + glyph: Option, + text: String, + unfolded: bool, + cx: &gpui::App, +) -> gpui::Stateful { + let sf = cx.global::().sidebar; + let muted = cx.theme().muted_foreground; + h_flex() + .id(id) + .items_center() + .gap(px(8.)) + .h(px(ROW_H)) + .w_full() + .min_w_0() + .px(px(ROW_INSET)) + .rounded(ROW_FILL_RADIUS) + .cursor_pointer() + .hover(|s| s.bg(gpui::rgb(sf.hover))) + .children(glyph.map(|g| check_glyph(g, cx))) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(META)) + .text_color(muted) + .child(text), + ) + .child( + Icon::new(if unfolded { + IconName::ChevronUp + } else { + IconName::ChevronDown + }) + .xsmall() + .text_color(muted), + ) +} + +fn reviews_section( + reviewers: &[Reviewer], + unfolded: bool, + toggle: impl Fn(&gpui::ClickEvent, &mut Window, &mut gpui::App) + 'static, + cx: &gpui::App, +) -> AnyElement { + let theme = cx.theme(); + let muted = theme.muted_foreground; + let sf = cx.global::().sidebar; + let foldable = reviewers.len() > REVIEWERS_FOLD_AT; + let shown = if foldable && !unfolded { + REVIEWERS_FOLD_AT - 1 + } else { + reviewers.len() + }; + let mut rows = v_flex().px(px(CONTENT_INSET)); + for (i, r) in reviewers.iter().enumerate().take(shown) { + let (label, ink) = match r.state { + ReviewState::Approved => (L10nKey::GitHubReviewApproved, theme.success), + ReviewState::ChangesRequested => (L10nKey::GitHubReviewChangesRequested, theme.danger), + ReviewState::Commented => (L10nKey::GitHubReviewCommented, muted), + ReviewState::Requested => (L10nKey::GitHubReviewRequested, muted), + }; + let initial = r + .login + .chars() + .next() + .map(|c| c.to_uppercase().to_string()) + .unwrap_or_default(); + rows = rows.child( + h_flex() + .id(("panel-github-reviewer", i)) + .items_center() + .gap(px(8.)) + .h(px(ROW_H)) + .w_full() + .min_w_0() + .px(px(ROW_INSET)) + .child( + div() + .flex_none() + .flex() + .items_center() + .justify_center() + .size(px(16.)) + .rounded_full() + .bg(theme.foreground.opacity(0.08)) + .text_size(rems(10. / 16.)) + .font_weight(gpui::FontWeight::SEMIBOLD) + .text_color(muted) + .child(initial), + ) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(TEXT)) + .text_color(gpui::rgb(sf.text_resting)) + .child(r.login.clone()), + ) + .child( + div() + .flex_none() + .text_size(rems(META)) + .text_color(ink) + .child(t(label)), + ), + ); + } + if foldable { + let text = if unfolded { + t(L10nKey::GitHubShowLess).to_string() + } else { + t_plural(L10nKey::GitHubShowAllReviewers, reviewers.len(), &[]) + }; + rows = rows.child( + fold_row("panel-github-fold-reviews", None, text, unfolded, cx).on_click(toggle), + ); + } + v_flex() + .mt(px(SECTION_GAP)) + .child(section_heading(t(L10nKey::GitHubReviews).to_string(), cx)) + .child(rows) + .into_any_element() +} + /// Headings a step or two over the body rather than the document-sized ramp /// the text view defaults to: an issue's `## What happened?` is a label in a /// 280px column, not a page title. @@ -554,15 +1094,12 @@ fn renders_safely(node: &gpui_component::text::markdown_ast::Node) -> bool { .is_none_or(|children| children.iter().all(renders_safely)) } -#[allow(clippy::too_many_arguments)] fn comment_block( - slug: &RepoSlug, - number: u64, c: &Comment, now: i64, fg: gpui::Hsla, muted: gpui::Hsla, - cx: &gpui::App, + body: AnyElement, ) -> AnyElement { let when = (c.created_at > 0).then(|| relative_time(now, c.created_at)); v_flex() @@ -582,15 +1119,15 @@ fn comment_block( ) .children(when.map(|w| div().text_color(muted).child(w))), ) - .child(markdown_block( - format!("gh-comment-{}-{number}-{}", slug.full(), c.id), - &c.body, - "", - cx, - )) + .child(body) .into_any_element() } +/// Whether a Markdown source is long enough to clamp. +fn runs_long(source: &str) -> bool { + source.chars().count() > CLAMP_CHARS || source.lines().count() > CLAMP_LINES +} + fn more_on_github(id: &'static str, url: String, cx: &gpui::App) -> AnyElement { h_flex() .id(id) @@ -607,3 +1144,30 @@ fn more_on_github(id: &'static str, url: String, cx: &gpui::App) -> AnyElement { .child(Icon::new(IconName::ExternalLink).xsmall()) .into_any_element() } + +#[cfg(test)] +mod tests { + use super::{runs_long, short_duration}; + + #[test] + fn a_pasted_log_runs_long_and_a_few_paragraphs_do_not() { + assert!(!runs_long("It crashes.\n\nSteps:\n1. open\n2. resize")); + assert!(runs_long(&"line\n".repeat(40))); + assert!(runs_long(&"word ".repeat(400))); + } + + #[test] + fn durations_read_as_short_as_githubs() { + assert_eq!(short_duration(18), "18s"); + assert_eq!(short_duration(120), "2m"); + assert_eq!(short_duration(72), "1m 12s"); + assert_eq!(short_duration(14 * 60 + 5), "14m"); + assert_eq!(short_duration(3600), "1h"); + assert_eq!(short_duration(2 * 3600 + 5 * 60), "2h 5m"); + assert_eq!( + short_duration(-3), + "0s", + "a clock skew is not negative time" + ); + } +} diff --git a/src/ui/github/mod.rs b/src/ui/github/mod.rs index 8ce8cc00..21eac732 100644 --- a/src/ui/github/mod.rs +++ b/src/ui/github/mod.rs @@ -20,6 +20,12 @@ //! tabs or panes back and forth does not refetch; an entry older than //! [`STALE_AFTER`] is revalidated in the background while the old rows stay //! on screen. +//! +//! Two things are fresher than that. A pull request on screen with checks +//! still running has just its checks re-read every [`CHECKS_POLL`], signed in +//! only, so "waiting on 1 check" turns into a verdict without anyone pressing +//! refresh. And the pane's branch is read every [`BRANCH_TTL`], so the pull +//! request pinned over the list follows a `git switch`. pub(crate) mod detail; @@ -33,6 +39,9 @@ use tty7_core::core::github::{ ApiError, Detail, GitHubRemote, Item, Kind, RepoSlug, StateFilter, Transport, }; +use tty7_core::core::config::RightPanelTab; +use tty7_core::core::github::CheckState; + use crate::ui::app::Tty7App; use crate::ui::host_ops::SharedHost; use crate::ui::scm::panel::RepoLookup; @@ -41,6 +50,13 @@ use crate::ui::scm::state::RepoKey; /// How old a cached list or detail may get before a render revalidates it. pub(crate) const STALE_AFTER: Duration = Duration::from_secs(120); +/// How often an open pull request's running checks are re-read. Two requests +/// a go — well inside a signed-in rate limit, which is the only one polled. +pub(crate) const CHECKS_POLL: Duration = Duration::from_secs(20); + +/// How long the pane's branch is trusted before it is read again. +const BRANCH_TTL: Duration = Duration::from_secs(10); + /// How far a list reads on by itself through pages that filter down to /// nothing (a pull-request-heavy repository's `/issues`), before it leaves /// the rest to "load more". @@ -82,6 +98,41 @@ pub(crate) struct DetailCache { pub(crate) error: Option, pub(crate) fetched: Option, seq: u64, + /// A checks poll is scheduled or in flight. + polling: bool, +} + +/// The pane's branch, and the branch it pushes to. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct BranchHead { + pub(crate) branch: String, + /// `origin/feat/x`, when it tracks one. + pub(crate) upstream: Option, +} + +#[derive(Default)] +pub(crate) struct BranchLookup { + /// `None` on a detached HEAD, or before the first answer. + pub(crate) head: Option, + pub(crate) read_at: Option, + pub(crate) loading: bool, +} + +/// Which pull request a branch would have: opened against `slug`, from +/// `owner`'s `branch`. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub(crate) struct BranchPullKey { + pub(crate) slug: RepoSlug, + pub(crate) owner: String, + pub(crate) branch: String, +} + +#[derive(Default)] +pub(crate) struct BranchPullCache { + pub(crate) item: Option, + pub(crate) loading: bool, + pub(crate) error: bool, + pub(crate) fetched: Option, } #[derive(Default)] @@ -105,6 +156,24 @@ pub(crate) struct GitHubPanelState { /// The list row under the pointer, by number: the one that shows its /// labels and age. pub(crate) hovered: Option, + pub(crate) branches: HashMap, + pub(crate) branch_pulls: HashMap, + /// The long sections of a detail the user unfolded. + pub(crate) unfolded: std::collections::HashSet<(RepoSlug, u64, Fold)>, +} + +/// A detail section that folds when it runs long. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(crate) enum Fold { + Checks, + Files, + Reviews, + /// The comments between the first and the last few. + Comments, + /// The description, clamped when long. + Body, + /// One long comment, by id. + Comment(u64), } /// What the panel can say about the active pane's repository. @@ -114,6 +183,7 @@ pub(crate) enum GhTarget { NotARepo, NoRemote, Ready { + host: SharedHost, repo: RepoKey, remotes: Arc>, chosen: GitHubRemote, @@ -221,6 +291,7 @@ impl Tty7App { Some(chosen) => { let chosen = chosen.clone(); GhTarget::Ready { + host, repo, remotes, chosen, @@ -377,6 +448,7 @@ impl Tty7App { } }; if !due { + self.github_poll_checks(&key, cx); return; } let Some(connection) = self.github_connection(cx) else { @@ -414,6 +486,213 @@ impl Tty7App { .detach(); } + /// Re-read the checks of the pull request on screen, after a pause, while + /// any of them are still running. Safe to call every frame: one poll per + /// detail is ever scheduled. Stops by itself once the detail is closed, + /// the tab is hidden, or every check has a verdict — and then marks the + /// detail due, so the merge state is read again with the verdicts in. + fn github_poll_checks(&mut self, key: &(RepoSlug, u64), cx: &mut Context) { + let Some(entry) = self.github.details.get(key) else { + return; + }; + let pending = entry.detail.as_ref().is_some_and(|d| { + d.checks + .as_ref() + .is_some_and(|c| c.count(CheckState::Pending) > 0) + && d.pull.as_ref().is_some_and(|p| !p.head_sha.is_empty()) + }); + // Signed out, the hourly allowance is 60 requests; a poll would spend + // it in ten minutes. Refresh still works by hand. + let signed_in = self + .github + .connection + .as_ref() + .is_some_and(|c| c.transport.authenticated()); + if entry.polling + || entry.loading + || !pending + || !signed_in + || !self.github_detail_shown(key) + { + return; + } + if let Some(entry) = self.github.details.get_mut(key) { + entry.polling = true; + } + let key = key.clone(); + let timer = cx.background_executor().timer(CHECKS_POLL); + cx.spawn(async move |this, cx| { + timer.await; + let request = this + .update(cx, |this, _cx| { + let entry = this.github.details.get_mut(&key)?; + let sha = entry + .detail + .as_ref()? + .pull + .as_ref() + .map(|p| p.head_sha.clone())?; + let seq = entry.seq; + let transport = this.github.connection.as_ref()?.transport.clone(); + if !this.github_detail_shown(&key) { + return None; + } + Some((sha, seq, transport)) + }) + .ok() + .flatten(); + let Some((sha, seq, transport)) = request else { + let _ = this.update(cx, |this, _cx| { + if let Some(entry) = this.github.details.get_mut(&key) { + entry.polling = false; + } + }); + return; + }; + let slug = key.0.clone(); + let result = off_ui(move || api::checks(&*transport, &slug, &sha)).await; + let _ = this.update(cx, |this, cx| { + let Some(entry) = this.github.details.get_mut(&key) else { + return; + }; + entry.polling = false; + // A full read landed (or started) meanwhile: it has the + // newer word on the checks. + if entry.seq != seq || entry.loading { + return; + } + let Some(Ok(checks)) = result else { + return; + }; + let settled = checks.count(CheckState::Pending) == 0; + if let Some(detail) = entry.detail.as_mut() { + Arc::make_mut(detail).checks = Some(checks); + } + if settled { + entry.fetched = None; + } + cx.notify(); + }); + }) + .detach(); + } + + /// Whether `key`'s detail is what the right panel is showing. + fn github_detail_shown(&self, key: &(RepoSlug, u64)) -> bool { + self.right_panel_visible + && self.right_panel_tab == RightPanelTab::GitHub + && self.github.open.as_ref() == Some(key) + } + + /// The branch `repo` is on, re-read every [`BRANCH_TTL`]. + pub(crate) fn github_branch( + &mut self, + host: SharedHost, + repo: &RepoKey, + cx: &mut Context, + ) -> Option { + let entry = self.github.branches.entry(repo.clone()).or_default(); + let due = !entry.loading && entry.read_at.is_none_or(|t| t.elapsed() > BRANCH_TTL); + let head = entry.head.clone(); + if due { + entry.loading = true; + let root = repo.root.clone(); + let repo = repo.clone(); + crate::ui::host_ops::HostOps::run( + host, + cx, + move |h| { + use tty7_core::core::git::git; + let branch = git(h, &root, &["symbolic-ref", "-q", "--short", "HEAD"])? + .trim() + .to_string(); + if branch.is_empty() { + return None; + } + let upstream = git( + h, + &root, + &[ + "rev-parse", + "--abbrev-ref", + "--symbolic-full-name", + &format!("{branch}@{{upstream}}"), + ], + ) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()); + Some(BranchHead { branch, upstream }) + }, + move |this, head, cx| { + let entry = this.github.branches.entry(repo).or_default(); + entry.loading = false; + entry.read_at = Some(Instant::now()); + if entry.head != head { + entry.head = head; + cx.notify(); + } + }, + ); + } + head + } + + /// The pull request the pane's branch has on the repository shown, if + /// any — fetched once and revalidated like a list. + pub(crate) fn github_branch_pull( + &mut self, + host: SharedHost, + repo: &RepoKey, + remotes: &[GitHubRemote], + chosen: &GitHubRemote, + cx: &mut Context, + ) -> Option { + let head = self.github_branch(host, repo, cx)?; + let key = branch_pull_key(&head, remotes, chosen)?; + let entry = self.github.branch_pulls.entry(key.clone()).or_default(); + let item = entry.item.clone(); + let due = !entry.loading + && !entry.error + && entry.fetched.is_none_or(|t| t.elapsed() > STALE_AFTER); + if !due { + return item; + } + let Some(connection) = self.github_connection(cx) else { + return item; + }; + if let Some(entry) = self.github.branch_pulls.get_mut(&key) { + entry.loading = true; + } + cx.spawn(async move |this, cx| { + let transport = connection.transport.clone(); + let q = key.clone(); + let Some(result) = + off_ui(move || api::pull_for_branch(&*transport, &q.slug, &q.owner, &q.branch)) + .await + else { + return; + }; + let _ = this.update(cx, |this, cx| { + let entry = this.github.branch_pulls.entry(key.clone()).or_default(); + entry.loading = false; + match result { + Ok(item) => { + entry.item = item; + entry.error = false; + entry.fetched = Some(Instant::now()); + } + Err(e) => { + log::warn!("github: pull request of {}:{}: {e}", key.owner, key.branch); + entry.error = true; + } + } + cx.notify(); + }); + }) + .detach(); + item + } + /// Start over for the repository on screen: resolve the token again (a /// `gh auth login` since the last try counts), re-read the remotes, and /// mark the visible list and detail due. @@ -441,6 +720,13 @@ impl Tty7App { entry.seq += 1; } self.github.details.retain(|_, e| e.detail.is_some()); + for entry in self.github.branches.values_mut() { + entry.read_at = None; + } + for entry in self.github.branch_pulls.values_mut() { + entry.error = false; + entry.fetched = None; + } cx.notify(); } @@ -471,9 +757,98 @@ impl Tty7App { } } +/// Where to look for `head`'s pull request: its upstream's remote, when that +/// is a GitHub remote (a fork's branch lives in the fork), else the repository +/// shown under the local branch's name. `None` when it tracks a branch +/// somewhere other than GitHub. +pub(crate) fn branch_pull_key( + head: &BranchHead, + remotes: &[GitHubRemote], + chosen: &GitHubRemote, +) -> Option { + let (owner, branch) = match &head.upstream { + None => (chosen.slug.owner.clone(), head.branch.clone()), + Some(upstream) => { + // The longest remote name that prefixes it: `origin` must not + // claim `origin-old/x`, and a remote may itself contain a `/`. + let remote = remotes + .iter() + .filter(|r| { + upstream + .strip_prefix(r.remote.as_str()) + .is_some_and(|rest| rest.starts_with('/')) + }) + .max_by_key(|r| r.remote.len())?; + let branch = upstream[remote.remote.len() + 1..].to_string(); + (remote.slug.owner.clone(), branch) + } + }; + (!branch.is_empty()).then(|| BranchPullKey { + slug: chosen.slug.clone(), + owner, + branch, + }) +} + /// Seconds since the epoch, for relative times. pub(crate) fn now_unix() -> i64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map_or(0, |d| d.as_secs() as i64) } + +#[cfg(test)] +mod tests { + use super::*; + + fn remote(name: &str, owner: &str) -> GitHubRemote { + GitHubRemote { + remote: name.into(), + slug: RepoSlug { + owner: owner.into(), + name: "widgets".into(), + }, + } + } + + fn head(branch: &str, upstream: Option<&str>) -> BranchHead { + BranchHead { + branch: branch.into(), + upstream: upstream.map(str::to_string), + } + } + + #[test] + fn a_forks_branch_is_looked_for_under_the_forks_owner() { + let remotes = [remote("origin", "me"), remote("upstream", "acme")]; + let key = branch_pull_key( + &head("local-name", Some("origin/feat/x")), + &remotes, + &remotes[1], + ) + .unwrap(); + assert_eq!(key.slug.owner, "acme", "asked of the repository shown"); + assert_eq!((key.owner.as_str(), key.branch.as_str()), ("me", "feat/x")); + } + + #[test] + fn an_untracked_branch_is_looked_for_under_its_own_name() { + let remotes = [remote("origin", "acme")]; + let key = branch_pull_key(&head("feat/y", None), &remotes, &remotes[0]).unwrap(); + assert_eq!( + (key.owner.as_str(), key.branch.as_str()), + ("acme", "feat/y") + ); + } + + #[test] + fn the_longest_remote_name_claims_the_upstream() { + let remotes = [remote("origin", "a"), remote("origin-old", "b")]; + let key = branch_pull_key(&head("x", Some("origin-old/x")), &remotes, &remotes[0]).unwrap(); + assert_eq!(key.owner, "b"); + assert!( + branch_pull_key(&head("x", Some("gitlab/x")), &remotes, &remotes[0]).is_none(), + "a branch pushed somewhere else has no pull request here" + ); + } +} diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 22f913e7..3ee0a81a 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -2106,6 +2106,33 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::GitHubCommits => "{count} commits", L10nKey::GitHubOpenedAt => "opened {when}", L10nKey::GitHubUpdatedAt => "updated {when}", + L10nKey::GitHubChecks => "Checks", + L10nKey::GitHubChecksPassed => "{passed} of {total} passed", + L10nKey::GitHubChecksNoneCounted => "No checks with a result", + L10nKey::GitHubCheckPassed => "Passed", + L10nKey::GitHubCheckFailed => "Failed", + L10nKey::GitHubCheckPending => "In progress", + L10nKey::GitHubCheckSkipped => "Skipped", + L10nKey::GitHubReviews => "Reviews", + L10nKey::GitHubReviewApproved => "Approved", + L10nKey::GitHubReviewChangesRequested => "Changes requested", + L10nKey::GitHubReviewCommented => "Commented", + L10nKey::GitHubReviewRequested => "Requested", + L10nKey::GitHubReadyToMerge => "Ready to merge", + L10nKey::GitHubMergeConflicts => "Merge conflicts", + L10nKey::GitHubReviewRequired => "Review required", + L10nKey::GitHubBehindBase => "Behind the base branch", + L10nKey::GitHubMergeBlocked => "Blocked by branch protection", + L10nKey::GitHubThisBranch => "This branch", + L10nKey::GitHubChecksFailing => "{count} checks failing", + L10nKey::GitHubWaitingOnChecks => "Waiting on {count} checks", + L10nKey::GitHubShowLess => "Show less", + L10nKey::GitHubShowAllFiles => "Show all {count} files", + L10nKey::GitHubPassedCount => "{count} passed", + L10nKey::GitHubSkippedCount => "{count} skipped", + L10nKey::GitHubShowFullText => "Show full text", + L10nKey::GitHubShowHiddenComments => "Show {count} more comments", + L10nKey::GitHubShowAllReviewers => "Show all {count} reviewers", } } @@ -2273,6 +2300,27 @@ pub fn translate_variant_en(key: L10nKey, branch: &'static str) -> Option<&'stat (L10nKey::GitHubCommits, "zero") => "No commits", (L10nKey::GitHubCommits, "one") => "1 commit", (L10nKey::GitHubCommits, "other") => "{count} commits", + (L10nKey::GitHubChecksFailing, "zero") => "No checks failing", + (L10nKey::GitHubChecksFailing, "one") => "1 check failing", + (L10nKey::GitHubChecksFailing, "other") => "{count} checks failing", + (L10nKey::GitHubWaitingOnChecks, "zero") => "Not waiting on checks", + (L10nKey::GitHubWaitingOnChecks, "one") => "Waiting on 1 check", + (L10nKey::GitHubWaitingOnChecks, "other") => "Waiting on {count} checks", + (L10nKey::GitHubShowAllFiles, "zero") => "No files", + (L10nKey::GitHubShowAllFiles, "one") => "Show 1 file", + (L10nKey::GitHubShowAllFiles, "other") => "Show all {count} files", + (L10nKey::GitHubPassedCount, "zero") => "None passed", + (L10nKey::GitHubPassedCount, "one") => "1 passed", + (L10nKey::GitHubPassedCount, "other") => "{count} passed", + (L10nKey::GitHubSkippedCount, "zero") => "None skipped", + (L10nKey::GitHubSkippedCount, "one") => "1 skipped", + (L10nKey::GitHubSkippedCount, "other") => "{count} skipped", + (L10nKey::GitHubShowHiddenComments, "zero") => "No more comments", + (L10nKey::GitHubShowHiddenComments, "one") => "Show 1 more comment", + (L10nKey::GitHubShowHiddenComments, "other") => "Show {count} more comments", + (L10nKey::GitHubShowAllReviewers, "zero") => "No reviewers", + (L10nKey::GitHubShowAllReviewers, "one") => "Show 1 reviewer", + (L10nKey::GitHubShowAllReviewers, "other") => "Show all {count} reviewers", _ => return None, }; Some(res) diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index dfebc354..2730f897 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -2163,6 +2163,33 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::GitHubCommits => "{count} 件のコミット", L10nKey::GitHubOpenedAt => "作成 {when}", L10nKey::GitHubUpdatedAt => "更新 {when}", + L10nKey::GitHubChecks => "チェック", + L10nKey::GitHubChecksPassed => "{total} 件中 {passed} 件成功", + L10nKey::GitHubChecksNoneCounted => "結果のあるチェックはありません", + L10nKey::GitHubCheckPassed => "成功", + L10nKey::GitHubCheckFailed => "失敗", + L10nKey::GitHubCheckPending => "実行中", + L10nKey::GitHubCheckSkipped => "スキップ", + L10nKey::GitHubReviews => "レビュー", + L10nKey::GitHubReviewApproved => "承認済み", + L10nKey::GitHubReviewChangesRequested => "変更をリクエスト", + L10nKey::GitHubReviewCommented => "コメント済み", + L10nKey::GitHubReviewRequested => "リクエスト中", + L10nKey::GitHubReadyToMerge => "マージ可能", + L10nKey::GitHubMergeConflicts => "マージコンフリクトあり", + L10nKey::GitHubReviewRequired => "レビューが必要", + L10nKey::GitHubBehindBase => "ベースブランチより遅れています", + L10nKey::GitHubMergeBlocked => "ブランチ保護によりブロック", + L10nKey::GitHubThisBranch => "このブランチ", + L10nKey::GitHubChecksFailing => "{count} 件のチェックが失敗", + L10nKey::GitHubWaitingOnChecks => "{count} 件のチェックを待機中", + L10nKey::GitHubShowLess => "折りたたむ", + L10nKey::GitHubShowAllFiles => "{count} 件のファイルをすべて表示", + L10nKey::GitHubPassedCount => "{count} 件成功", + L10nKey::GitHubSkippedCount => "{count} 件スキップ", + L10nKey::GitHubShowFullText => "全文を表示", + L10nKey::GitHubShowHiddenComments => "ほか {count} 件のコメントを表示", + L10nKey::GitHubShowAllReviewers => "{count} 人のレビュアーをすべて表示", }) } @@ -2323,6 +2350,27 @@ pub fn translate_variant_ja(key: L10nKey, branch: &'static str) -> Option<&'stat (L10nKey::GitHubCommits, "zero") => "コミットはありません", (L10nKey::GitHubCommits, "one") => "1 件のコミット", (L10nKey::GitHubCommits, "other") => "{count} 件のコミット", + (L10nKey::GitHubChecksFailing, "zero") => "失敗しているチェックはありません", + (L10nKey::GitHubChecksFailing, "one") => "1 件のチェックが失敗", + (L10nKey::GitHubChecksFailing, "other") => "{count} 件のチェックが失敗", + (L10nKey::GitHubWaitingOnChecks, "zero") => "待機中のチェックはありません", + (L10nKey::GitHubWaitingOnChecks, "one") => "1 件のチェックを待機中", + (L10nKey::GitHubWaitingOnChecks, "other") => "{count} 件のチェックを待機中", + (L10nKey::GitHubShowAllFiles, "zero") => "ファイルなし", + (L10nKey::GitHubShowAllFiles, "one") => "1 件のファイルを表示", + (L10nKey::GitHubShowAllFiles, "other") => "{count} 件のファイルをすべて表示", + (L10nKey::GitHubPassedCount, "zero") => "成功なし", + (L10nKey::GitHubPassedCount, "one") => "1 件成功", + (L10nKey::GitHubPassedCount, "other") => "{count} 件成功", + (L10nKey::GitHubSkippedCount, "zero") => "スキップなし", + (L10nKey::GitHubSkippedCount, "one") => "1 件スキップ", + (L10nKey::GitHubSkippedCount, "other") => "{count} 件スキップ", + (L10nKey::GitHubShowHiddenComments, "zero") => "ほかのコメントはありません", + (L10nKey::GitHubShowHiddenComments, "one") => "ほか 1 件のコメントを表示", + (L10nKey::GitHubShowHiddenComments, "other") => "ほか {count} 件のコメントを表示", + (L10nKey::GitHubShowAllReviewers, "zero") => "レビュアーなし", + (L10nKey::GitHubShowAllReviewers, "one") => "1 人のレビュアーを表示", + (L10nKey::GitHubShowAllReviewers, "other") => "{count} 人のレビュアーをすべて表示", _ => return None, }; Some(res) diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 746dbdb1..bfb2d548 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1659,6 +1659,33 @@ l10n_keys! { GitHubCommits, GitHubOpenedAt, GitHubUpdatedAt, + GitHubChecks, + GitHubChecksPassed, + GitHubChecksNoneCounted, + GitHubCheckPassed, + GitHubCheckFailed, + GitHubCheckPending, + GitHubCheckSkipped, + GitHubReviews, + GitHubReviewApproved, + GitHubReviewChangesRequested, + GitHubReviewCommented, + GitHubReviewRequested, + GitHubReadyToMerge, + GitHubMergeConflicts, + GitHubReviewRequired, + GitHubBehindBase, + GitHubMergeBlocked, + GitHubThisBranch, + GitHubChecksFailing, + GitHubWaitingOnChecks, + GitHubShowLess, + GitHubShowAllFiles, + GitHubPassedCount, + GitHubSkippedCount, + GitHubShowFullText, + GitHubShowHiddenComments, + GitHubShowAllReviewers, } /// The source control strings that are translated but not yet displayed. @@ -1975,6 +2002,13 @@ mod tests { L10nKey::HomeTimeMonthsAgo, L10nKey::GitHubComments, L10nKey::GitHubCommits, + L10nKey::GitHubChecksFailing, + L10nKey::GitHubWaitingOnChecks, + L10nKey::GitHubShowAllFiles, + L10nKey::GitHubPassedCount, + L10nKey::GitHubSkippedCount, + L10nKey::GitHubShowHiddenComments, + L10nKey::GitHubShowAllReviewers, ]; for key in plural_keys { for branch in ["zero", "one", "other"] { diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 7c1fc618..975aebc5 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1963,6 +1963,33 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::GitHubCommits => "{count} 个提交", L10nKey::GitHubOpenedAt => "创建 {when}", L10nKey::GitHubUpdatedAt => "更新 {when}", + L10nKey::GitHubChecks => "检查", + L10nKey::GitHubChecksPassed => "{total} 项通过 {passed} 项", + L10nKey::GitHubChecksNoneCounted => "没有得出结果的检查", + L10nKey::GitHubCheckPassed => "通过", + L10nKey::GitHubCheckFailed => "失败", + L10nKey::GitHubCheckPending => "进行中", + L10nKey::GitHubCheckSkipped => "已跳过", + L10nKey::GitHubReviews => "审查", + L10nKey::GitHubReviewApproved => "已批准", + L10nKey::GitHubReviewChangesRequested => "要求修改", + L10nKey::GitHubReviewCommented => "已评论", + L10nKey::GitHubReviewRequested => "待审查", + L10nKey::GitHubReadyToMerge => "可以合并", + L10nKey::GitHubMergeConflicts => "有合并冲突", + L10nKey::GitHubReviewRequired => "需要审查", + L10nKey::GitHubBehindBase => "落后于目标分支", + L10nKey::GitHubMergeBlocked => "被分支保护规则阻止", + L10nKey::GitHubThisBranch => "当前分支", + L10nKey::GitHubChecksFailing => "{count} 项检查失败", + L10nKey::GitHubWaitingOnChecks => "等待 {count} 项检查", + L10nKey::GitHubShowLess => "收起", + L10nKey::GitHubShowAllFiles => "显示全部 {count} 个文件", + L10nKey::GitHubPassedCount => "{count} 项通过", + L10nKey::GitHubSkippedCount => "{count} 项跳过", + L10nKey::GitHubShowFullText => "展开全文", + L10nKey::GitHubShowHiddenComments => "显示另外 {count} 条评论", + L10nKey::GitHubShowAllReviewers => "显示全部 {count} 位审查人", }) } @@ -2099,6 +2126,27 @@ pub fn translate_variant_zh(key: L10nKey, branch: &'static str) -> Option<&'stat (L10nKey::GitHubCommits, "zero") => "没有提交", (L10nKey::GitHubCommits, "one") => "1 个提交", (L10nKey::GitHubCommits, "other") => "{count} 个提交", + (L10nKey::GitHubChecksFailing, "zero") => "没有失败的检查", + (L10nKey::GitHubChecksFailing, "one") => "1 项检查失败", + (L10nKey::GitHubChecksFailing, "other") => "{count} 项检查失败", + (L10nKey::GitHubWaitingOnChecks, "zero") => "无需等待检查", + (L10nKey::GitHubWaitingOnChecks, "one") => "等待 1 项检查", + (L10nKey::GitHubWaitingOnChecks, "other") => "等待 {count} 项检查", + (L10nKey::GitHubShowAllFiles, "zero") => "没有文件", + (L10nKey::GitHubShowAllFiles, "one") => "显示 1 个文件", + (L10nKey::GitHubShowAllFiles, "other") => "显示全部 {count} 个文件", + (L10nKey::GitHubPassedCount, "zero") => "没有通过", + (L10nKey::GitHubPassedCount, "one") => "1 项通过", + (L10nKey::GitHubPassedCount, "other") => "{count} 项通过", + (L10nKey::GitHubSkippedCount, "zero") => "没有跳过", + (L10nKey::GitHubSkippedCount, "one") => "1 项跳过", + (L10nKey::GitHubSkippedCount, "other") => "{count} 项跳过", + (L10nKey::GitHubShowHiddenComments, "zero") => "没有更多评论", + (L10nKey::GitHubShowHiddenComments, "one") => "显示另外 1 条评论", + (L10nKey::GitHubShowHiddenComments, "other") => "显示另外 {count} 条评论", + (L10nKey::GitHubShowAllReviewers, "zero") => "没有审查人", + (L10nKey::GitHubShowAllReviewers, "one") => "显示 1 位审查人", + (L10nKey::GitHubShowAllReviewers, "other") => "显示全部 {count} 位审查人", _ => return None, }; Some(res) diff --git a/src/ui/panel_github.rs b/src/ui/panel_github.rs index a040c2fc..4333eec3 100644 --- a/src/ui/panel_github.rs +++ b/src/ui/panel_github.rs @@ -10,7 +10,7 @@ use gpui_component::menu::{DropdownMenu as _, PopupMenuItem}; use gpui_component::{ActiveTheme as _, Icon, IconName, Sizable as _, h_flex, v_flex}; use tty7_core::core::github::{ - ApiError, GitHubRemote, Item, ItemState, Kind, Label, RepoSlug, StateFilter, + ApiError, CheckState, GitHubRemote, Item, ItemState, Kind, Label, RepoSlug, StateFilter, }; use crate::ui::app::{CONTENT_INSET, TILE_GLYPH_XS, TILE_SIZE_XS, Tty7App}; @@ -48,7 +48,7 @@ impl Tty7App { // other tab has, just to hold ↻. Refresh sits with the repository's // other actions instead — in the repo row, and in a detail's header. let title = self.panel_title(t(L10nKey::PanelGitHubTitle), None, None, window, cx); - let (repo, remotes, chosen) = match target { + let (host, repo, remotes, chosen) = match target { GhTarget::NoPane => { let body = self.panel_empty( t(L10nKey::PanelNoWorkingDirectory), @@ -78,10 +78,11 @@ impl Tty7App { return self.github_shell(title, Vec::new(), body, false); } GhTarget::Ready { + host, repo, remotes, chosen, - } => (repo, remotes, chosen), + } => (host, repo, remotes, chosen), }; // A detail belongs to the repository it was opened from. If the pane @@ -98,10 +99,20 @@ impl Tty7App { let query = self.github_query(&chosen.slug); self.github_ensure_list(&query, cx); - let mut pinned = vec![ - self.github_repo_row(&repo, &remotes, &chosen, cx), - self.github_switch_row(cx), - ]; + let mut pinned = vec![self.github_repo_row(&repo, &remotes, &chosen, cx)]; + // The pull request the pane is working on, one click from the list + // whichever way the list is switched. + if let Some(item) = self.github_branch_pull(host, &repo, &remotes, &chosen, cx) { + let branch = self + .github + .branches + .get(&repo) + .and_then(|b| b.head.as_ref()) + .map(|h| h.branch.clone()) + .unwrap_or_default(); + pinned.push(self.github_branch_pull_row(&chosen.slug, &branch, &item, cx)); + } + pinned.push(self.github_switch_row(cx)); if let Some(label) = self.github.label.clone() { pinned.push(self.github_label_filter_row(&label, cx)); } @@ -287,6 +298,102 @@ impl Tty7App { .into_any_element() } + /// The pane's branch's pull request, under a caption naming the branch: + /// its state, number and title, and — once its detail has been read — how + /// its checks stand. A list row in every other respect, so it rests + /// unfilled and lights up on hover, or while its detail is the one open. + fn github_branch_pull_row( + &self, + slug: &RepoSlug, + branch: &str, + item: &Item, + cx: &mut Context, + ) -> AnyElement { + let sf = cx.global::().sidebar; + let muted = cx.theme().muted_foreground; + let mono = cx.theme().mono_font_family.clone(); + let number = item.number; + let key = (slug.clone(), number); + let rollup = self + .github + .details + .get(&key) + .and_then(|d| d.detail.as_ref()) + .and_then(|d| d.checks.as_ref()) + .and_then(|c| c.rollup()); + let open = self.github.open.as_ref() == Some(&key); + let slug = slug.clone(); + let title = SharedString::from(item.title.clone()); + let caption = h_flex() + .items_center() + .gap(px(6.)) + .min_w_0() + .px(px(TEXT_INSET)) + .text_size(rems(META)) + .text_color(muted) + .child( + Icon::empty() + .path("icons/git-branch.svg") + .xsmall() + .text_color(muted), + ) + .child(div().flex_none().child(t(L10nKey::GitHubThisBranch))) + .child( + div() + .min_w_0() + .truncate() + .font_family(mono.clone()) + .child(branch.to_string()), + ); + let row = h_flex() + .id("panel-github-branch-pull") + .items_center() + .gap(px(8.)) + .h(px(ROW_H)) + .w_full() + .px(px(ROW_INSET)) + .rounded(ROW_FILL_RADIUS) + .cursor_pointer() + .when(open, |r| r.bg(gpui::rgb(sf.selected))) + .hover(|s| s.bg(gpui::rgb(sf.hover))) + .tooltip(move |window, cx| { + gpui_component::tooltip::Tooltip::new(title.clone()).build(window, cx) + }) + .on_click(cx.listener(move |this, _, _window, cx| { + this.github_open_detail(slug.clone(), number, cx); + })) + .child(div().flex_none().child(state_glyph(item.state, true, cx))) + .child( + div() + .flex_none() + .text_size(rems(META)) + .font_family(mono) + .text_color(muted) + .child(format!("#{number}")), + ) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(TEXT)) + .text_color(gpui::rgb(if open { + sf.text_selected + } else { + sf.text_resting + })) + .child(item.title.clone()), + ) + .children(rollup.map(|s| check_glyph(s, cx))); + v_flex() + .flex_none() + .gap(px(4.)) + .pt(px(PINNED_GAP)) + .child(caption) + .child(div().px(px(CONTENT_INSET)).child(row)) + .into_any_element() + } + /// Issues | Pull Requests on the left, Open | Closed on the right. fn github_switch_row(&self, cx: &mut Context) -> AnyElement { let kind = self.github.kind; @@ -627,6 +734,33 @@ pub(crate) fn state_glyph(state: ItemState, is_pr: bool, cx: &gpui::App) -> AnyE .into_any_element() } +/// A check's glyph: a shape per state, coloured the way the rest of the +/// panel colours success, failure and waiting. +pub(crate) fn check_glyph(state: CheckState, cx: &gpui::App) -> AnyElement { + let theme = cx.theme(); + let (path, color) = match state { + CheckState::Passed => ("icons/github/check-passed.svg", theme.success), + CheckState::Failed => ("icons/github/check-failed.svg", theme.danger), + CheckState::Pending => ("icons/github/check-pending.svg", theme.warning), + CheckState::Skipped => ("icons/github/check-skipped.svg", theme.muted_foreground), + }; + gpui::svg() + .path(path) + .flex_none() + .size(px(GLYPH)) + .text_color(color) + .into_any_element() +} + +pub(crate) fn check_label(state: CheckState) -> &'static str { + t(match state { + CheckState::Passed => L10nKey::GitHubCheckPassed, + CheckState::Failed => L10nKey::GitHubCheckFailed, + CheckState::Pending => L10nKey::GitHubCheckPending, + CheckState::Skipped => L10nKey::GitHubCheckSkipped, + }) +} + pub(crate) fn state_label(state: ItemState) -> &'static str { t(match state { ItemState::Open => L10nKey::GitHubOpen, @@ -939,4 +1073,151 @@ mod gpui_tests { test_window::quiesce(&mut vcx, Some(&root)); let _ = std::fs::remove_dir_all(&root); } + + /// A signed-in fake with one pull request on the pane's branch, whose one + /// check is running until `done` is set. + struct PullFake { + asked: Mutex>, + done: std::sync::atomic::AtomicBool, + } + + impl Transport for PullFake { + fn get(&self, path: &str) -> Result { + self.asked.lock().unwrap().push(path.to_string()); + let done = self.done.load(std::sync::atomic::Ordering::SeqCst); + let body = + if path.starts_with("/repos/acme/widgets/pulls?state=all&head=acme%3Afeat%2Fx&") { + r#"[{"number": 31, "title": "Handle empty summaries", "state": "open", + "head": {"ref": "feat/x", "sha": "abc"}, "base": {"ref": "main"}}]"# + } else if path.starts_with("/repos/acme/widgets/issues?") { + "[]" + } else if path == "/repos/acme/widgets/issues/31" { + r#"{"number": 31, "title": "Handle empty summaries", "state": "open", + "user": {"login": "ada"}, "pull_request": {"merged_at": null}}"# + } else if path.starts_with("/repos/acme/widgets/issues/31/comments") + || path.starts_with("/repos/acme/widgets/pulls/31/files") + { + "[]" + } else if path == "/repos/acme/widgets/pulls/31" { + r#"{"number": 31, "title": "Handle empty summaries", "state": "open", + "user": {"login": "ada"}, "mergeable_state": "blocked", + "head": {"ref": "feat/x", "sha": "abc"}, "base": {"ref": "main"}, + "requested_reviewers": [{"login": "jonas"}]}"# + } else if path.starts_with("/repos/acme/widgets/pulls/31/reviews") { + r#"[{"user": {"login": "mara"}, "state": "APPROVED"}]"# + } else if path.starts_with("/repos/acme/widgets/commits/abc/check-runs") { + if done { + r#"{"total_count": 1, "check_runs": [{"name": "test", + "status": "completed", "conclusion": "success"}]}"# + } else { + r#"{"total_count": 1, "check_runs": [{"name": "test", + "status": "in_progress"}]}"# + } + } else if path.starts_with("/repos/acme/widgets/commits/abc/status") { + r#"{"total_count": 0, "statuses": []}"# + } else { + return Err(ApiError::NotFound); + }; + Ok(Reply { + body: body.as_bytes().to_vec(), + has_next: false, + }) + } + + fn authenticated(&self) -> bool { + true + } + } + + #[gpui::test] + fn the_branchs_pull_request_is_pinned_and_its_running_checks_polled(cx: &mut TestAppContext) { + use tty7_core::core::github::CheckState; + + let root = scratch("branch-pull"); + git(&root, &["init", "--quiet", "-b", "feat/x"]); + git( + &root, + &["remote", "add", "origin", "https://github.com/acme/widgets"], + ); + + let fake = Arc::new(PullFake { + asked: Mutex::new(Vec::new()), + done: std::sync::atomic::AtomicBool::new(false), + }); + let (app, mut vcx, mut pane) = test_window::harness_with_pane(cx); + let transport: Arc = fake.clone(); + app.update_in(&mut vcx, |app, _, cx| { + app.github.connector = Some(Arc::new(move || Connection { + transport: transport.clone(), + })); + app.right_panel_visible = true; + app.right_panel_tab = RightPanelTab::GitHub; + cx.notify(); + }); + DaemonMsg::Cwd(root.clone()) + .encode(&mut pane) + .expect("the pane's socket takes the cwd"); + + settle(&app, &mut vcx, "the branch's pull request", |app| { + app.github + .branch_pulls + .values() + .any(|p| p.item.as_ref().is_some_and(|i| i.number == 31)) + }); + + let slug = app.update_in(&mut vcx, |app, _, cx| { + let slug = app.github.branch_pulls.keys().next().unwrap().slug.clone(); + app.github_open_detail(slug.clone(), 31, cx); + slug + }); + let key = (slug.clone(), 31); + let checks = |app: &Tty7App| { + app.github + .details + .get(&key) + .and_then(|d| d.detail.as_ref()) + .and_then(|d| d.checks.clone()) + }; + settle(&app, &mut vcx, "the detail with its checks", |app| { + checks(app).is_some() + }); + let reviewers = app.update_in(&mut vcx, |app, _, _| { + app.github.details[&key] + .detail + .as_ref() + .unwrap() + .reviewers + .clone() + .unwrap() + }); + assert_eq!(reviewers.len(), 2, "one approval, one request"); + assert_eq!( + app.update_in(&mut vcx, |app, _, _| checks(app).unwrap().rollup()), + Some(CheckState::Pending) + ); + + let reads_of_pull = || { + fake.asked + .lock() + .unwrap() + .iter() + .filter(|p| *p == "/repos/acme/widgets/pulls/31") + .count() + }; + assert_eq!(reads_of_pull(), 1); + + fake.done.store(true, std::sync::atomic::Ordering::SeqCst); + vcx.executor() + .advance_clock(crate::ui::github::CHECKS_POLL + std::time::Duration::from_secs(1)); + settle(&app, &mut vcx, "the poll's verdict", |app| { + checks(app).and_then(|c| c.rollup()) == Some(CheckState::Passed) + }); + // With every verdict in, the detail is read again for the merge state. + settle(&app, &mut vcx, "the detail read again", |_| { + reads_of_pull() >= 2 + }); + + test_window::quiesce(&mut vcx, Some(&root)); + let _ = std::fs::remove_dir_all(&root); + } }