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); + } }