From 7fb7275233ab2e4c5abed28f1cdd2756eba619d1 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 22 Aug 2026 20:38:22 +0700 Subject: [PATCH] add pull request viewer --- crates/signed_core/src/lib.rs | 2 +- crates/signed_core/src/model.rs | 244 +++++ crates/signed_git/src/lib.rs | 301 +++++- crates/signed_state/src/repo.rs | 103 +- .../workspace/src/views/repo_detail/diff.rs | 139 +-- .../src/views/repo_detail/helpers.rs | 131 ++- crates/workspace/src/views/repo_detail/mod.rs | 5 +- .../views/repo_detail/pull_request_detail.rs | 971 ++++++++++++++++++ .../src/views/repo_detail/pull_requests.rs | 28 +- 9 files changed, 1737 insertions(+), 187 deletions(-) create mode 100644 crates/workspace/src/views/repo_detail/pull_request_detail.rs diff --git a/crates/signed_core/src/lib.rs b/crates/signed_core/src/lib.rs index 338bee3..0a0e74c 100644 --- a/crates/signed_core/src/lib.rs +++ b/crates/signed_core/src/lib.rs @@ -9,6 +9,6 @@ pub mod status; pub use addr::{RepoAddr, repo_addr}; pub use clone_url::{CloneTarget, parse_clone_url}; pub use deletions::Deletions; -pub use model::{activity_subject, Announcement}; +pub use model::{Announcement, activity_subject, pull_request_patch}; pub use state::parse_state; pub use status::{RepoStatus, references_root, resolve_status}; diff --git a/crates/signed_core/src/model.rs b/crates/signed_core/src/model.rs index 0be9773..1ce232c 100644 --- a/crates/signed_core/src/model.rs +++ b/crates/signed_core/src/model.rs @@ -50,6 +50,123 @@ pub fn activity_subject(event: &Event) -> SharedString { .unwrap_or(SharedString::from("Untitled")) } +/// The patch set of a pull request: the root patch event (kind `1617`) the +/// PR references via its `e` tag, plus every patch of the set chained to it +/// with NIP-10 `e` reply tags, in series order (oldest first). When the PR +/// has no `e` tag, falls back to the patch producing the PR's tip commit +/// (its `commit`/`r` tag, per NIP-34) and walks the reply chain backward to +/// the root. +/// +/// Returns an empty list when no patch event can be linked to the PR. +pub fn pull_request_patches<'a>( + pr: &Event, + patches: impl IntoIterator, +) -> Vec<&'a Event> { + let patches: Vec<&'a Event> = patches.into_iter().collect(); + + // The PR references its root patch via an `e` tag; follow the NIP-10 + // reply chain forward from there (each patch of the set replies to the + // previous one). Among several replies (a revision), the newest wins. + if let Some(root_id) = pr.tags.event_ids().next() + && let Some(root) = patches.iter().find(|patch| patch.id == root_id) + { + return forward_series(root, &patches); + } + + // No `e` tag: the last patch of the set carries the PR's tip commit in + // its `commit`/`r` tag; walk the reply chain backward to the root. + let Some(tip) = current_commit_of(pr) else { + return Vec::new(); + }; + let Some(last) = patches + .iter() + .filter(|patch| patch_produces_commit(patch, &tip)) + .max_by_key(|patch| patch.created_at) + .copied() + else { + return Vec::new(); + }; + + let mut series = vec![last]; + loop { + let Some(prev_id) = series.last().unwrap().tags.event_ids().next() else { + break; + }; + let Some(prev) = patches + .iter() + .find(|patch| patch.id == prev_id && !series.contains(patch)) + .copied() + else { + break; + }; + series.push(prev); + } + series.reverse(); + series +} + +/// The patch content of a pull request: the contents of every patch event of +/// its patch set (see [`pull_request_patches`]) joined in series order, +/// falling back to the PR's own content for older PRs that carried the +/// patch inline. +pub fn pull_request_patch<'a>(pr: &Event, patches: impl IntoIterator) -> String { + let patches: Vec<&'a Event> = patches.into_iter().collect(); + let series = pull_request_patches(pr, patches.iter().copied()); + if series.is_empty() { + return pr.content.clone(); + } + series + .iter() + .map(|patch| patch.content.as_str()) + .collect::>() + .join("\n") +} + +/// The chain of patches replying to `root` (NIP-10 `e` tags), oldest first. +fn forward_series<'a>(root: &'a Event, patches: &[&'a Event]) -> Vec<&'a Event> { + let mut series = vec![root]; + loop { + let next = patches + .iter() + .filter(|patch| !series.contains(patch)) + .filter(|patch| { + patch + .tags + .event_ids() + .any(|id| id == series.last().unwrap().id) + }) + .max_by_key(|patch| patch.created_at); + let Some(next) = next else { + break; + }; + series.push(next); + } + series +} + +/// The `c` tag of an event (tip of the proposed branch), as hex. +fn current_commit_of(event: &Event) -> Option { + event + .tags + .iter() + .find_map(|tag| match Nip34Tag::parse(tag.as_slice()) { + Ok(Nip34Tag::CurrentCommit(commit)) => Some(commit.to_string()), + _ => None, + }) +} + +/// Whether `patch` produces `commit` (its `commit` or `r` tag), so clients +/// can find existing patches for a specific commit. +fn patch_produces_commit(patch: &Event, commit: &str) -> bool { + patch + .tags + .iter() + .any(|tag| match Nip34Tag::parse(tag.as_slice()) { + Ok(Nip34Tag::Commit(c) | Nip34Tag::Reference(c)) => c.to_string() == commit, + _ => false, + }) +} + impl Announcement { /// Parse a kind `30617` event. Returns `None` if the kind is wrong or the `d` tag is missing. pub fn from_event(event: &Event) -> Option { @@ -230,4 +347,131 @@ mod tests { assert!(announcement.name.is_none()); assert!(announcement.web.is_empty()); } + + /// Build a signed PR event with the given tags and content. + fn pr_event(content: &str, tags: Vec) -> Event { + EventBuilder::new(Kind::GitPullRequest, content) + .tags(tags) + .finalize(&keys()) + .expect("signed event") + } + + #[test] + fn pull_request_patch_prefers_linked_patch_event() { + let patch = EventBuilder::new(Kind::GitPatch, "patch-content") + .finalize(&keys()) + .expect("signed event"); + let pr = pr_event("description", vec![Tag::event(patch.id)]); + + assert_eq!(pull_request_patch(&pr, [&patch]), "patch-content"); + } + + #[test] + fn pull_request_patch_falls_back_to_inline_content() { + // Older PRs carried the patch in the content; no linked patch event. + let pr = pr_event("patch-inline", vec![]); + + assert_eq!(pull_request_patch(&pr, [] as [&Event; 0]), "patch-inline"); + } + + #[test] + fn pull_request_patch_ignores_unrelated_patch_events() { + let patch = EventBuilder::new(Kind::GitPatch, "patch-content") + .finalize(&keys()) + .expect("signed event"); + let pr = pr_event("description", vec![]); + + assert_eq!(pull_request_patch(&pr, [&patch]), "description"); + } + + /// Build a signed patch event with a controlled `created_at`. + fn patch_event(content: &str, tags: Vec, created_at: u64) -> Event { + EventBuilder::new(Kind::GitPatch, content) + .tags(tags) + .custom_created_at(Timestamp::from(created_at)) + .finalize(&keys()) + .expect("signed event") + } + + #[test] + fn pull_request_patch_joins_the_whole_patch_set() { + // NIP-34: a PR references the root patch; later patches of the set + // reply to the previous one (NIP-10 `e` tags). + let root = patch_event("patch-one", vec![], 100); + let second = patch_event("patch-two", vec![Tag::event(root.id)], 200); + let pr = pr_event("description", vec![Tag::event(root.id)]); + + assert_eq!( + pull_request_patch(&pr, [&root, &second]), + "patch-one\npatch-two" + ); + assert_eq!( + pull_request_patches(&pr, [&root, &second]), + vec![&root, &second] + ); + } + + #[test] + fn pull_request_patches_walks_the_reply_chain_in_order() { + let root = patch_event("patch-one", vec![], 100); + let second = patch_event("patch-two", vec![Tag::event(root.id)], 200); + let third = patch_event("patch-three", vec![Tag::event(second.id)], 300); + let pr = pr_event("description", vec![Tag::event(root.id)]); + + let series = pull_request_patches(&pr, [&third, &root, &second]); + assert_eq!( + series + .iter() + .map(|p| p.content.as_str()) + .collect::>(), + vec!["patch-one", "patch-two", "patch-three"] + ); + } + + #[test] + fn pull_request_patches_ignores_unrelated_replies() { + let root = patch_event("patch-one", vec![], 100); + let other = patch_event("other-patch", vec![Tag::event(root.id)], 250); + // A patch replying to a different root is not part of the set. + let stranger = patch_event("stranger", vec![], 150); + let pr = pr_event("description", vec![Tag::event(root.id)]); + + let series = pull_request_patches(&pr, [&root, &other, &stranger]); + assert_eq!( + series + .iter() + .map(|p| p.content.as_str()) + .collect::>(), + vec!["patch-one", "other-patch"] + ); + } + + #[test] + fn pull_request_patches_finds_the_set_via_the_tip_commit() { + // PRs without an `e` tag: the last patch of the set carries the tip + // commit in its `r` tag; walk the reply chain backward to the root. + let root = patch_event("patch-one", vec![], 100); + let tip = "1111111111111111111111111111111111111111"; + let last = patch_event( + "patch-two", + vec![ + Tag::event(root.id), + Tag::parse(["r", tip]).expect("valid tag"), + ], + 200, + ); + let pr = pr_event( + "description", + vec![Tag::parse(["c", tip]).expect("valid tag")], + ); + + let series = pull_request_patches(&pr, [&root, &last]); + assert_eq!( + series + .iter() + .map(|p| p.content.as_str()) + .collect::>(), + vec!["patch-one", "patch-two"] + ); + } } diff --git a/crates/signed_git/src/lib.rs b/crates/signed_git/src/lib.rs index 3ccd4f4..3b3b483 100644 --- a/crates/signed_git/src/lib.rs +++ b/crates/signed_git/src/lib.rs @@ -61,7 +61,12 @@ impl GitCache { for url in clone_urls { match clone(url, &path) { - Ok(repo) => return Ok(repo), + Ok(repo) => { + // The initial clone uses the default refspecs; also + // fetch the `refs/nostr/*` PR refs. + fetch_all(&repo).ok(); + return Ok(repo); + } Err(e) => last_err = Some(e), } } @@ -73,11 +78,23 @@ impl GitCache { } } -/// Fetch all configured refspecs from `origin`. +/// Fetch all configured refspecs from `origin`, plus the `refs/nostr/*` +/// namespace where GRASP mirrors serve pull request branches (one ref per +/// PR event id, as used by ngit). pub fn fetch_all(repo: &gix::Repository) -> Result<()> { + let options = gix::remote::ref_map::Options { + extra_refspecs: vec![ + gix::refspec::parse( + gix::bstr::BStr::new("+refs/nostr/*:refs/nostr/*"), + gix::refspec::parse::Operation::Fetch, + )? + .to_owned(), + ], + ..Default::default() + }; repo.find_remote("origin")? .connect(gix::remote::Direction::Fetch)? - .prepare_fetch(Discard, Default::default())? + .prepare_fetch(Discard, options)? .receive(Discard, &IS_INTERRUPTED)?; Ok(()) } @@ -483,10 +500,6 @@ pub fn worktree_commit_diff(workdir: &Path, id: &str) -> Result { } fn commit_diff(repo: &gix::Repository, id: &str) -> Result { - use gix::diff::blob::platform::prepare_diff::Operation; - use gix::object::tree::diff::Change; - use gix::objs::tree::EntryKind; - let commit_id = repo.rev_parse_single(id.as_bytes())?; let commit = commit_id.object()?.into_commit(); let new_tree = commit.tree()?; @@ -494,8 +507,65 @@ fn commit_diff(repo: &gix::Repository, id: &str) -> Result { Some(parent) => Some(parent.object()?.into_commit().tree()?), None => None, }; + tree_diff(repo, old_tree.as_ref(), &new_tree) +} - let changes = repo.diff_tree_to_tree(old_tree.as_ref(), Some(&new_tree), None)?; +/// The changes between two commits (`base`..`tip`), like `git diff base tip`. +/// Same file handling as [`worktree_commit_diff`] (directories and +/// submodules are skipped, files are sorted by path). +pub fn worktree_commit_range_diff(workdir: &Path, base: &str, tip: &str) -> Result { + let repo = open_with_cache(workdir)?; + let base_tree = repo + .rev_parse_single(base.as_bytes())? + .object()? + .into_commit() + .tree()?; + let tip_tree = repo + .rev_parse_single(tip.as_bytes())? + .object()? + .into_commit() + .tree()?; + tree_diff(&repo, Some(&base_tree), &tip_tree) +} + +/// Commits in the range `base`..`tip`, newest first, like `git log base..tip`. +pub fn worktree_commit_range_commits( + workdir: &Path, + base: &str, + tip: &str, +) -> Result> { + use gix::traverse::commit::simple::CommitTimeOrder; + + let repo = open_with_cache(workdir)?; + let base_id = repo.rev_parse_single(base.as_bytes())?; + let tip_id = repo.rev_parse_single(tip.as_bytes())?; + let walk = repo + .rev_walk([tip_id]) + .sorting(gix::revision::walk::Sorting::ByCommitTime( + CommitTimeOrder::NewestFirst, + )) + .with_hidden([base_id]); + + let mut commits = Vec::new(); + for info in walk.all()? { + let info = info?; + commits.push(file_commit(&info.object()?, false)?); + } + Ok(commits) +} + +/// The changes between two trees, used by both [`commit_diff`] and +/// [`worktree_commit_range_diff`]. +fn tree_diff( + repo: &gix::Repository, + old_tree: Option<&gix::Tree<'_>>, + new_tree: &gix::Tree<'_>, +) -> Result { + use gix::diff::blob::platform::prepare_diff::Operation; + use gix::object::tree::diff::Change; + use gix::objs::tree::EntryKind; + + let changes = repo.diff_tree_to_tree(old_tree, Some(new_tree), None)?; let mut cache = repo.diff_resource_cache_for_tree_diff()?; let mut files = Vec::new(); @@ -630,6 +700,88 @@ pub fn patch_diffs(patch: &str) -> Result { Ok(CommitDiff { files }) } +/// Commits of a `git format-patch` output (a single patch or a patch +/// series), parsed from the mbox envelope headers of each patch: commit id, +/// author, summary and author time. Entries appear in patch order (oldest +/// first, as produced by `git format-patch`). +pub fn patch_commits(patch: &str) -> Vec { + let lines: Vec<&str> = patch.lines().collect(); + let mut commits = Vec::new(); + let mut i = 0; + + while i < lines.len() { + // A patch starts with its `From ` envelope line. + let Some(rest) = lines[i].strip_prefix("From ") else { + i += 1; + continue; + }; + let Some(id) = rest.split_whitespace().next() else { + i += 1; + continue; + }; + if id.len() != 40 { + i += 1; + continue; + } + + let mut author = String::new(); + let mut summary = String::new(); + let mut time = 0i64; + + // Envelope headers of this patch, up to the blank line separating + // them from the commit message. + i += 1; + while i < lines.len() && !lines[i].is_empty() { + let header = lines[i]; + if let Some(value) = header.strip_prefix("From: ") { + author = name_from_address(value); + } else if let Some(value) = header.strip_prefix("Subject: ") { + summary = strip_patch_prefix(value); + } else if let Some(value) = header.strip_prefix("Date: ") { + time = gix::date::parse(value.trim(), None) + .map(|t| t.seconds) + .unwrap_or(0); + } + i += 1; + } + + commits.push(FileCommit { + id: id.to_string(), + summary, + description: None, + author, + time, + }); + } + + commits +} + +/// The name part of a `From: Name ` header value. +fn name_from_address(from: &str) -> String { + match from.trim().find('<') { + Some(ix) => from[..ix].trim().to_string(), + None => from.trim().to_string(), + } +} + +/// Strip the `[PATCH]`, `[PATCH 1/2]`, `[RFC PATCH]` … prefix from a patch +/// `Subject:` header. +fn strip_patch_prefix(subject: &str) -> String { + let trimmed = subject.trim(); + let Some(rest) = trimmed.strip_prefix('[') else { + return trimmed.to_string(); + }; + let Some(end) = rest.find(']') else { + return trimmed.to_string(); + }; + if rest[..end].to_ascii_lowercase().contains("patch") { + rest[end + 1..].trim().to_string() + } else { + trimmed.to_string() + } +} + /// Parse one file's diff section: everything after its `diff --git` header /// up to the next section (or the end of the patch). Returns the section /// and the index of the first unconsumed line. @@ -1650,6 +1802,52 @@ mod tests { assert_eq!(deleted.hunks[0].lines[0].new, None); } + #[test] + fn commit_range_diff_lists_changes_between_two_commits() { + let (dir, repo) = fixture(&[("a.txt", b"a\n"), ("b.txt", b"b\n")]); + commit_all(&repo, "first"); + let base = repo.head_id().expect("head").to_string(); + + std::fs::write(dir.path().join("a.txt"), b"changed\n").expect("write"); + std::fs::write(dir.path().join("c.txt"), b"new\n").expect("write"); + commit_all(&repo, "second"); + let tip = repo.head_id().expect("head").to_string(); + + let diff = worktree_commit_range_diff(dir.path(), &base, &tip).expect("diff"); + + let by_path: HashMap<&str, &FileDiff> = diff + .files + .iter() + .map(|file| (file.path.as_str(), file)) + .collect(); + assert_eq!(by_path.len(), 2); + assert_eq!(by_path["a.txt"].status, DiffStatus::Modified); + assert_eq!(by_path["a.txt"].insertions, 1); + assert_eq!(by_path["a.txt"].deletions, 1); + assert_eq!(by_path["c.txt"].status, DiffStatus::Added); + // b.txt is unchanged between the two commits. + assert!(diff.files.iter().all(|file| file.path != "b.txt")); + } + + #[test] + fn commit_range_commits_lists_only_new_commits_newest_first() { + let (dir, repo) = fixture(&[("a.txt", b"one\n")]); + commit_all(&repo, "one"); + let base = repo.head_id().expect("head").to_string(); + + std::fs::write(dir.path().join("a.txt"), b"two\n").expect("write"); + commit_all(&repo, "two"); + std::fs::write(dir.path().join("a.txt"), b"three\n").expect("write"); + commit_all(&repo, "three"); + let tip = repo.head_id().expect("head").to_string(); + + let commits = worktree_commit_range_commits(dir.path(), &base, &tip).expect("commits"); + + assert_eq!(commits.len(), 2); + assert_eq!(commits[0].summary, "three"); + assert_eq!(commits[1].summary, "two"); + } + #[test] fn commit_diff_reports_binary_files_without_hunks() { let (_dir, repo) = fixture(&[("blob.bin", b"\x00\x01\x02")]); @@ -1881,6 +2079,93 @@ index 3..4 100644 assert_eq!(diff.files[1].deletions, 1); } + #[test] + fn patch_commits_lists_every_patch_in_order() { + let patch = r#"From 1111111111111111111111111111111111111111 Mon Sep 17 00:00:00 2001 +From: Alice +Date: Tue, 1 Aug 2023 10:00:00 +0200 +Subject: [PATCH 1/2] first + +body one +--- + a.txt | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/a.txt b/a.txt +@@ -1 +1,2 @@ + a ++b + +From 2222222222222222222222222222222222222222 Mon Sep 17 00:00:00 2001 +From: Bob +Date: Wed, 2 Aug 2023 11:30:00 +0000 +Subject: [PATCH 2/2] second + +body two +--- + b.txt | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/b.txt b/b.txt +@@ -1 +1,2 @@ + x ++y +"#; + + let commits = patch_commits(patch); + assert_eq!(commits.len(), 2); + + assert_eq!(commits[0].id, "1111111111111111111111111111111111111111"); + assert_eq!(commits[0].summary, "first"); + assert_eq!(commits[0].author, "Alice"); + assert_eq!(commits[0].time, 1690876800); + + assert_eq!(commits[1].id, "2222222222222222222222222222222222222222"); + assert_eq!(commits[1].summary, "second"); + assert_eq!(commits[1].author, "Bob"); + assert_eq!(commits[1].time, 1690975800); + } + + #[test] + fn patch_commits_strips_patch_subject_prefixes() { + let patch = r#"From 1111111111111111111111111111111111111111 Mon Sep 17 00:00:00 2001 +From: A +Subject: [RFC PATCH v3 4/7] the real title + +--- +"#; + + let commits = patch_commits(patch); + assert_eq!(commits.len(), 1); + assert_eq!(commits[0].summary, "the real title"); + } + + #[test] + fn patch_commits_handles_missing_headers() { + // A hand-written patch without author/date headers still lists a + // commit; time stays 0 and the author falls back to the raw value. + let patch = r#"From 1111111111111111111111111111111111111111 Mon Sep 17 00:00:00 2001 +Subject: [PATCH] plain + +--- +"#; + + let commits = patch_commits(patch); + assert_eq!(commits.len(), 1); + assert_eq!(commits[0].summary, "plain"); + assert_eq!(commits[0].author, ""); + assert_eq!(commits[0].time, 0); + } + + #[test] + fn patch_commits_ignores_non_patch_lines() { + assert!(patch_commits("").is_empty()); + assert!(patch_commits("just some text\nFrom 123\n").is_empty()); + // A diff-only body (no mbox envelope) has no commits. + let patch = "diff --git a/x b/x\n--- a/x\n+++ b/x\n"; + assert!(patch_commits(patch).is_empty()); + } + #[test] fn marks_binary_sections() { let patch = r#"diff --git a/img.png b/img.png diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 98f5882..8d7c8ff 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -3,7 +3,9 @@ use std::time::Duration; use anyhow::Error; use gpui::{AppContext, Context, Subscription, Task}; use nostr_sdk::prelude::*; -use signed_core::{Announcement, Deletions, RepoAddr, RepoStatus, filters, parse_state}; +use signed_core::{ + Announcement, Deletions, RepoAddr, RepoStatus, filters, parse_state, pull_request_patch, +}; use crate::backend::{Backend, BackendEvent}; use crate::git_store::GitStore; @@ -211,6 +213,7 @@ impl RepoStore { }); self.tasks.retain(|task| !task.is_ready()); + self.tasks.push(cx.spawn(async move |this, cx| { let (announcement, state, issues, patches, pull_requests, statuses, comments) = match work.await { @@ -277,7 +280,7 @@ impl RepoStore { /// Number of open issues: issues whose resolved status is /// [`RepoStatus::Open`] (issues without status events default to open). - pub fn open_issue_count(&self) -> usize { + pub fn issue_count(&self) -> usize { self.issues .iter() .filter(|issue| self.status_of(issue) == RepoStatus::Open) @@ -287,7 +290,7 @@ impl RepoStore { /// Number of open pull requests: root PR events (not PR updates, whose /// status is carried by the root) with a resolved status of /// [`RepoStatus::Open`]. - pub fn open_pull_request_count(&self) -> usize { + pub fn pull_request_count(&self) -> usize { self.pull_requests .iter() .filter(|pr| pr.kind == Kind::GitPullRequest && self.status_of(pr) == RepoStatus::Open) @@ -336,52 +339,81 @@ impl RepoStore { self.send(builder, cx); } - /// Open a pull request on this repository: a root PR event whose content - /// is the `git format-patch` output of the proposed changes. + /// Open a pull request on this repository: a root PR event (kind 1618) + /// whose content is the markdown description, plus a root patch event + /// (kind 1617) carrying the `git format-patch` output, which the PR + /// references via an `e` tag (NIP-34). /// - /// The branch metadata (branch name, clone URL, merge base, root patch) - /// isn't known to the UI yet and is left empty; the proposed commit is - /// parsed from the patch's `From ` header, falling back to an - /// empty hash for hand-written content. + /// The patch is published first and the PR is sent once the patch + /// event's id is known, so the two always arrive together. The branch + /// metadata (branch name, clone URL, merge base) isn't known to the UI + /// yet and is left empty; the proposed commit is parsed from the patch's + /// `From ` header, falling back to an empty hash for hand-written + /// content. pub fn open_pull_request( &mut self, subject: Option, - content: String, + description: String, + patch: String, cx: &mut Context, ) { - let current_commit = patch_current_commit(&content) + self.last_error = None; + + let current_commit = patch_current_commit(&patch) .and_then(|hex| hex.parse().ok()) .unwrap_or_else(|| bitcoin_hashes::Sha1::from_byte_array([0u8; 20])); - let builder = GitPullRequest { - repository: self.addr.clone(), - content, - subject, - labels: Vec::new(), - branch_name: None, - clone: Vec::new(), - current_commit, - root_patch_event: None, - merge_base: None, - } - .into_event_builder(); - - self.send(builder, cx); - } - - /// Send a root patch (`git format-patch` output) to this repository. - pub fn send_root_patch(&mut self, patch: String, cx: &mut Context) { let Ok(root_marker) = Tag::parse(["t", "root"]) else { return; }; - - let builder = EventBuilder::new(Kind::GitPatch, patch).tags([ + let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags([ Tag::coordinate(self.addr.clone(), None), Tag::public_key(self.addr.public_key), root_marker, ]); - self.send(builder, cx); + let patch_task = + Backend::global(cx).update(cx, |backend, cx| backend.send(patch_builder, cx)); + + self.tasks.push(cx.spawn(async move |this, cx| { + let patch_event = match patch_task.await { + Ok(event) => event, + Err(e) => { + return this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + }); + } + }; + + // The PR references the patch event so viewers can find the + // patch without carrying it inline. + let pr_task = this.update(cx, |this, cx| { + let builder = GitPullRequest { + repository: this.addr.clone(), + content: description, + subject, + labels: Vec::new(), + branch_name: None, + clone: Vec::new(), + current_commit, + root_patch_event: Some(patch_event.id), + merge_base: None, + } + .into_event_builder(); + + Backend::global(cx).update(cx, |backend, cx| backend.send(builder, cx)) + })?; + + if let Err(e) = pr_task.await { + return this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + }); + } + + Ok(()) + })); } /// Set the status of a root event (requires being the root author or a maintainer). @@ -400,8 +432,9 @@ impl RepoStore { self.send(builder, cx); } - /// Merge a pull request: apply its patch (`git format-patch` output) to - /// the local clone of this repository, then publish the merged status. + /// Merge a pull request: apply its patch (the content of the linked + /// root patch event) to the local clone of this repository, then publish + /// the merged status. /// /// Only the repository author may merge. The clone is created on demand /// from the announcement's clone URLs when the repository hasn't been @@ -427,7 +460,7 @@ impl RepoStore { .as_ref() .map(|a| a.clone.iter().map(ToString::to_string).collect()) .unwrap_or_default(); - let patch = root.content.clone(); + let patch = pull_request_patch(root, self.patches.iter()); let root = root.clone(); let apply = cx.background_spawn(async move { diff --git a/crates/workspace/src/views/repo_detail/diff.rs b/crates/workspace/src/views/repo_detail/diff.rs index 37262a1..74d7215 100644 --- a/crates/workspace/src/views/repo_detail/diff.rs +++ b/crates/workspace/src/views/repo_detail/diff.rs @@ -1,8 +1,3 @@ -//! Commit diff viewer: a panel showing every file a commit changed, with a -//! tree of the changed files on the left and the line diff of the selected -//! file on the right. Opened from the repository detail view by clicking a -//! commit in the Commits tab or the latest-commit button in the header. - use std::path::PathBuf; use std::rc::Rc; @@ -18,34 +13,20 @@ use gpui_component::resizable::{resizable_panel, v_resizable}; use gpui_component::scroll::{ScrollableElement, Scrollbar}; use gpui_component::spinner::Spinner; use gpui_component::tag::Tag; -use gpui_component::tree::{TreeEntry, TreeItem, TreeState, tree}; +use gpui_component::tree::{TreeEntry, TreeState, tree}; use gpui_component::{ ActiveTheme, Sizable, StyledExt, VirtualListScrollHandle, h_flex, v_flex, v_virtual_list, }; -use signed_git::{CommitDiff, DiffHunk, DiffLine, DiffLineKind, DiffStatus, FileCommit, FileDiff}; +use signed_git::{CommitDiff, DiffStatus, FileCommit, FileDiff}; use utils::relative_time_secs; -use super::helpers::{build_tree_items, placeholder, tree_items, tree_row}; +use super::helpers::{ + DIFF_ROW_HEIGHT, DiffRow, build_tree_items, diff_rows, find_item, placeholder, render_diff_row, + tree_items, tree_row, +}; /// Width of the changed-files column. const TREE_WIDTH: f32 = 260.; -/// Width of one line-number gutter in a diff row. -const GUTTER_WIDTH: f32 = 44.; -/// Height of one row in the virtual diff list. -const DIFF_ROW_HEIGHT: f32 = 20.; - -/// One row of the virtual diff list: a hunk header, or a line of a hunk. -#[derive(Clone, Copy)] -enum DiffRow { - Hunk { - old_start: u32, - old_lines: u32, - new_start: u32, - new_lines: u32, - }, - /// Line `line` of hunk `hunk` of the selected file's diff. - Line { hunk: usize, line: usize }, -} /// Detail panel showing the diff of one commit. pub struct CommitDiffView { @@ -329,7 +310,7 @@ impl CommitDiffView { return Vec::new(); }; range - .map(|ix| Self::render_diff_row(&file.hunks, this.rows[ix], cx)) + .map(|ix| render_diff_row(&file.hunks, this.rows[ix], cx)) .collect() }, ) @@ -397,82 +378,6 @@ impl CommitDiffView { .into_any_element() } - /// One row of the virtual diff list: a hunk header or a single line. - fn render_diff_row(hunks: &[DiffHunk], row: DiffRow, cx: &App) -> AnyElement { - match row { - DiffRow::Hunk { - old_start, - old_lines, - new_start, - new_lines, - } => div() - .px_2() - .w_full() - .h(px(DIFF_ROW_HEIGHT)) - .font_family(cx.theme().mono_font_family.clone()) - .text_xs() - .bg(cx.theme().muted) - .border_y(px(1.)) - .border_color(cx.theme().border) - .text_color(cx.theme().muted_foreground) - .child(SharedString::from(format!( - "@@ -{},{} +{},{} @@", - old_start, old_lines, new_start, new_lines - ))) - .into_any_element(), - DiffRow::Line { hunk, line } => Self::render_diff_line(&hunks[hunk].lines[line], cx), - } - } - - /// One diff line: old and new line numbers in gutters, then the content, - /// tinted by kind (addition / deletion / context). - fn render_diff_line(line: &DiffLine, cx: &App) -> AnyElement { - let bg = match line.kind { - DiffLineKind::Addition => Some(cx.theme().success.opacity(0.2)), - DiffLineKind::Deletion => Some(cx.theme().danger.opacity(0.2)), - DiffLineKind::Context => None, - }; - let gutter = cx.theme().muted_foreground; - - // Fixed height and nowrap: the virtual list assumes every row has - // the same height, so long lines are clipped instead of wrapped. - h_flex() - .w_full() - .h(px(DIFF_ROW_HEIGHT)) - .items_center() - .font_family(cx.theme().mono_font_family.clone()) - .text_xs() - .when_some(bg, |this, bg| this.bg(bg)) - .child( - div() - .w(px(GUTTER_WIDTH)) - .flex_none() - .pr_2() - .text_right() - .text_color(gutter) - .child(line.old.map(|n| n.to_string()).unwrap_or_default()), - ) - .child( - div() - .w(px(GUTTER_WIDTH)) - .flex_none() - .pr_2() - .text_right() - .text_color(gutter) - .child(line.new.map(|n| n.to_string()).unwrap_or_default()), - ) - .child( - div() - .flex_1() - .min_w_0() - .overflow_hidden() - .whitespace_nowrap() - .text_color(cx.theme().foreground) - .child(line.text.clone()), - ) - .into_any_element() - } - /// Header: commit id, summary, author/time and overall change stats. fn render_header(&self, cx: &mut Context) -> AnyElement { let commit = &self.commit; @@ -550,36 +455,6 @@ impl CommitDiffView { } } -/// Find a tree item by id, searching into nested children. -fn find_item<'a>(items: &'a [TreeItem], id: Option<&str>) -> Option<&'a TreeItem> { - let id = id?; - items.iter().find_map(|item| { - if item.id.as_ref() == id { - Some(item) - } else { - find_item(&item.children, Some(id)) - } - }) -} - -/// The rows of `file`'s diff: one header row per hunk, then its lines. -fn diff_rows(file: &FileDiff) -> Vec { - let mut rows = Vec::new(); - for (hunk_ix, hunk) in file.hunks.iter().enumerate() { - rows.push(DiffRow::Hunk { - old_start: hunk.old_start, - old_lines: hunk.old_lines, - new_start: hunk.new_start, - new_lines: hunk.new_lines, - }); - rows.extend((0..hunk.lines.len()).map(|line| DiffRow::Line { - hunk: hunk_ix, - line, - })); - } - rows -} - impl Panel for CommitDiffView { fn panel_name(&self) -> &'static str { "commit_diff" diff --git a/crates/workspace/src/views/repo_detail/helpers.rs b/crates/workspace/src/views/repo_detail/helpers.rs index 43fbbe9..b536807 100644 --- a/crates/workspace/src/views/repo_detail/helpers.rs +++ b/crates/workspace/src/views/repo_detail/helpers.rs @@ -1,17 +1,15 @@ -//! Pure helpers for the repository detail view: file-tree building, code -//! preview helpers and small element builders. - use std::collections::HashMap; use std::path::{Path, PathBuf}; use assets::CustomIconName; use gpui::prelude::*; -use gpui::{AnyElement, App, Window, div, px}; +use gpui::{AnyElement, App, SharedString, Window, div, px}; use gpui_component::list::ListItem; use gpui_component::tooltip::Tooltip; use gpui_component::tree::{TreeEntry, TreeItem}; use gpui_component::{ActiveTheme, Icon, IconName, Sizable, h_flex, v_flex}; use signed_core::RepoStatus; +use signed_git::{DiffHunk, DiffLine, DiffLineKind, FileDiff}; /// A `Send` file-tree node: the tree is built on a background thread and /// converted into [`TreeItem`]s (which hold `Rc` state, so they cannot @@ -269,6 +267,131 @@ pub(super) fn status_badge(status: RepoStatus, cx: &App) -> AnyElement { .into_any_element() } +/// Width of one line-number gutter in a diff row. +pub(super) const GUTTER_WIDTH: f32 = 44.; +/// Height of one row in a virtual diff list. +pub(super) const DIFF_ROW_HEIGHT: f32 = 20.; + +/// One row of a virtual diff list: a hunk header, or a line of a hunk. +/// Shared by the commit diff and pull request diff viewers. +#[derive(Clone, Copy)] +pub(super) enum DiffRow { + Hunk { + old_start: u32, + old_lines: u32, + new_start: u32, + new_lines: u32, + }, + /// Line `line` of hunk `hunk` of the selected file's diff. + Line { hunk: usize, line: usize }, +} + +/// The rows of `file`'s diff: one header row per hunk, then its lines. +pub(super) fn diff_rows(file: &FileDiff) -> Vec { + let mut rows = Vec::new(); + for (hunk_ix, hunk) in file.hunks.iter().enumerate() { + rows.push(DiffRow::Hunk { + old_start: hunk.old_start, + old_lines: hunk.old_lines, + new_start: hunk.new_start, + new_lines: hunk.new_lines, + }); + rows.extend((0..hunk.lines.len()).map(|line| DiffRow::Line { + hunk: hunk_ix, + line, + })); + } + rows +} + +/// One row of the virtual diff list: a hunk header or a single line. +pub(super) fn render_diff_row(hunks: &[DiffHunk], row: DiffRow, cx: &App) -> AnyElement { + match row { + DiffRow::Hunk { + old_start, + old_lines, + new_start, + new_lines, + } => div() + .px_2() + .w_full() + .h(px(DIFF_ROW_HEIGHT)) + .font_family(cx.theme().mono_font_family.clone()) + .text_xs() + .bg(cx.theme().muted) + .border_y(px(1.)) + .border_color(cx.theme().border) + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(format!( + "@@ -{},{} +{},{} @@", + old_start, old_lines, new_start, new_lines + ))) + .into_any_element(), + DiffRow::Line { hunk, line } => render_diff_line(&hunks[hunk].lines[line], cx), + } +} + +/// One diff line: old and new line numbers in gutters, then the content, +/// tinted by kind (addition / deletion / context). +pub(super) fn render_diff_line(line: &DiffLine, cx: &App) -> AnyElement { + let bg = match line.kind { + DiffLineKind::Addition => Some(cx.theme().success.opacity(0.2)), + DiffLineKind::Deletion => Some(cx.theme().danger.opacity(0.2)), + DiffLineKind::Context => None, + }; + let gutter = cx.theme().muted_foreground; + + // Fixed height and nowrap: the virtual list assumes every row has + // the same height, so long lines are clipped instead of wrapped. + h_flex() + .w_full() + .h(px(DIFF_ROW_HEIGHT)) + .items_center() + .font_family(cx.theme().mono_font_family.clone()) + .text_xs() + .when_some(bg, |this, bg| this.bg(bg)) + .child( + div() + .w(px(GUTTER_WIDTH)) + .flex_none() + .pr_2() + .text_right() + .text_color(gutter) + .child(line.old.map(|n| n.to_string()).unwrap_or_default()), + ) + .child( + div() + .w(px(GUTTER_WIDTH)) + .flex_none() + .pr_2() + .text_right() + .text_color(gutter) + .child(line.new.map(|n| n.to_string()).unwrap_or_default()), + ) + .child( + div() + .flex_1() + .min_w_0() + .overflow_hidden() + .whitespace_nowrap() + .text_color(cx.theme().foreground) + .child(line.text.clone()), + ) + .into_any_element() +} + +/// Find a tree item by id, searching into nested children. +pub(super) fn find_item<'a>(items: &'a [TreeItem], id: Option<&str>) -> Option<&'a TreeItem> { + let id = id?; + items.iter().find_map(|item| { + if item.id.as_ref() == id { + Some(item) + } else { + find_item(&item.children, Some(id)) + } + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index 655d184..3797d04 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -34,6 +34,7 @@ mod diff; mod helpers; mod issue_detail; mod issues; +mod pull_request_detail; mod pull_requests; use browser::{ @@ -945,8 +946,8 @@ impl RepoDetailView { fn render_header(&self, cx: &mut Context) -> AnyElement { let store = self.store.read(cx); let announcement = store.announcement.as_ref().unwrap_or(&self.initial); - let issue_count = store.open_issue_count(); - let pull_request_count = store.open_pull_request_count(); + let issue_count = store.issue_count(); + let pull_request_count = store.pull_request_count(); let name = self.display_name(cx); let description = announcement.description(); diff --git a/crates/workspace/src/views/repo_detail/pull_request_detail.rs b/crates/workspace/src/views/repo_detail/pull_request_detail.rs new file mode 100644 index 0000000..346b8e6 --- /dev/null +++ b/crates/workspace/src/views/repo_detail/pull_request_detail.rs @@ -0,0 +1,971 @@ +use std::path::PathBuf; +use std::rc::Rc; + +use dock::{Panel, PanelEvent}; +use gpui::prelude::*; +use gpui::{ + AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, + ScrollStrategy, SharedString, Size, Subscription, Task, WeakEntity, Window, div, px, size, +}; +use gpui_component::avatar::Avatar; +use gpui_component::button::{Button, ButtonVariants}; +use gpui_component::clipboard::Clipboard; +use gpui_component::input::{Textarea, TextareaState}; +use gpui_component::list::ListItem; +use gpui_component::resizable::{resizable_panel, v_resizable}; +use gpui_component::scroll::{ScrollableElement, Scrollbar}; +use gpui_component::spinner::Spinner; +use gpui_component::tree::{TreeEntry, TreeState, tree}; +use gpui_component::{ + ActiveTheme, Sizable, StyledExt, VirtualListScrollHandle, h_flex, v_flex, v_virtual_list, +}; +use nostr::prelude::{Event, EventId, Kind, Nip34Tag}; +use signed_core::{activity_subject, pull_request_patch}; +use signed_git::{CommitDiff, FileCommit, FileDiff, patch_commits, patch_diffs}; +use signed_state::{GitStore, ProfileStore, RepoStore}; +use utils::{relative_time, relative_time_secs}; + +use super::helpers::{ + DIFF_ROW_HEIGHT, DiffRow, build_tree_items, diff_rows, find_item, placeholder, render_diff_row, + status_badge, tree_items, tree_row, +}; + +/// Width of the changed-files column. +const TREE_WIDTH: f32 = 260.; + +/// Detail panel of a single pull request. +pub struct PullRequestDetailView { + focus_handle: FocusHandle, + /// Repo store holding the PR, its status and comments. + store: Entity, + /// Event id of the root PR event (kind 1618; updates are revisions). + pr_id: EventId, + /// Input state of the "leave a comment" textarea. + comment_input: Entity, + /// Root PR's content, shown as plain text. + description: SharedString, + /// Tip commit of the PR: the latest update's `c` tag, else the root's. + current_commit: Option, + /// Commits of the patch series, in patch order (oldest first). + commits: Vec, + /// Parsed file changes of the patch; `None` while loading or on failure. + diff: Option, + /// The patch is being parsed on a background task. + loading: bool, + error: Option, + /// Changed-files explorer state. + tree_state: Entity, + /// Path of the file whose diff is shown in the detail column. + selected_file: Option, + /// Rows of the selected file's diff (hunk headers + lines). + rows: Vec, + /// Per-row heights of [`Self::rows`]. + item_sizes: Rc>>, + /// Virtual list state of the diff rows. + scroll_handle: VirtualListScrollHandle, + /// In-flight tasks; finished tasks are pruned on every push, so the vec + /// stays bounded by the number of concurrent loads. + tasks: Vec>>, + /// Subscriptions keeping the view live as the store refreshes. + _subscriptions: Vec, +} + +impl PullRequestDetailView { + pub fn new( + store: Entity, + pr_id: EventId, + window: &mut Window, + cx: &mut Context, + ) -> Self { + // PR author avatars stay in the shared cache until the panel + // closes; free them then. + crate::image_cache::clear_on_release(&cx.entity(), window, cx); + + let tree_state = cx.new(|cx| TreeState::new(cx)); + let comment_input = + cx.new(|cx| TextareaState::new(window, cx).placeholder("Leave a comment…")); + + // Re-render when the store refreshes (new comments, status changes). + let subscriptions = vec![cx.observe(&store, |_this, _store, cx| cx.notify())]; + + // Defer loading until the window is ready, like the commit diff view. + cx.defer_in(window, |this, window, cx| { + this.load(window, cx); + }); + + Self { + focus_handle: cx.focus_handle(), + store, + pr_id, + comment_input, + description: SharedString::default(), + current_commit: None, + commits: Vec::new(), + diff: None, + loading: true, + error: None, + tree_state, + selected_file: None, + rows: Vec::new(), + item_sizes: Rc::new(Vec::new()), + scroll_handle: VirtualListScrollHandle::new(), + tasks: Vec::new(), + _subscriptions: subscriptions, + } + } + + /// Snapshot the PR events from the store, then compute the file changes + /// and commit list on a background task and populate the tree. + /// + /// The changes come from the PR's patch set (NIP-34 `e`-linked patch + /// events) when present; otherwise they live in the git repository + /// (`c`, `clone` and `merge-base` tags, per NIP-34), so the clone is + /// fetched and the `merge-base..tip` range is diffed. + fn load(&mut self, window: &mut Window, cx: &mut Context) { + self.loading = true; + self.error = None; + cx.notify(); + + let cache = GitStore::global(cx).cache().clone(); + + let (description, patch, current_commit, merge_base, clone_urls, addr, has_patch_link) = { + let store = self.store.read(cx); + let Some(root) = store + .pull_requests + .iter() + .find(|pr| pr.id == self.pr_id && pr.kind == Kind::GitPullRequest) + else { + self.loading = false; + self.error = Some("Pull request not found".into()); + cx.notify(); + return; + }; + let update = latest_update(store.pull_requests.iter(), &root.id); + let tip = update + .and_then(current_commit_of) + .or_else(|| current_commit_of(root)); + let base = update + .and_then(merge_base_of) + .or_else(|| merge_base_of(root)); + let clone_urls = clone_urls_of(root).or_else(|| { + store + .announcement + .as_ref() + .map(|a| a.clone.iter().map(ToString::to_string).collect()) + }); + ( + root.content.clone(), + pull_request_patch(root, store.patches.iter()), + tip, + base, + clone_urls.unwrap_or_default(), + store.addr().clone(), + root.tags.event_ids().next().is_some(), + ) + }; + self.description = description.into(); + + let task = cx.spawn_in(window, async move |this, cx| { + // Parse the nostr patch set first. + let nostr_diff = cx + .background_spawn({ + let patch = patch.clone(); + async move { patch_diffs(&patch) } + }) + .await; + let nostr_commits = cx + .background_spawn({ + let patch = patch.clone(); + async move { patch_commits(&patch) } + }) + .await; + + // PRs without patch events (e.g. published by ngit) carry their + // changes in the git repository: fetch the clone and diff the + // `merge-base..tip` range. + let use_nostr = match &nostr_diff { + Ok(diff) => has_patch_link || !diff.files.is_empty(), + Err(_) => true, + }; + let git = if use_nostr { + None + } else { + let cache = cache.clone(); + let addr = addr.clone(); + let clone_urls = clone_urls.clone(); + let base = merge_base.clone(); + let tip = current_commit.clone(); + Some( + cx.background_spawn(async move { + let repo = cache.ensure_clone(&addr, &clone_urls)?; + let workdir = repo + .workdir() + .ok_or_else(|| anyhow::anyhow!("repository has no worktree"))?; + let tip = + tip.ok_or_else(|| anyhow::anyhow!("pull request has no tip commit"))?; + let base = match base { + Some(base) => base, + // No `merge-base` tag: use the merge base of the + // tip with the default branch. + None => { + let head = repo + .head_id() + .map_err(|_| anyhow::anyhow!("repository has no HEAD"))?; + let tip_id = repo.rev_parse_single(tip.as_bytes())?; + repo.merge_base(tip_id, head)?.to_string() + } + }; + let diff = signed_git::worktree_commit_range_diff(workdir, &base, &tip)?; + let commits = + signed_git::worktree_commit_range_commits(workdir, &base, &tip)?; + Ok::<_, anyhow::Error>((diff, commits)) + }) + .await, + ) + }; + + let (diff, commits) = match git { + Some(Ok((diff, commits))) => (Ok(diff), commits), + Some(Err(error)) => (Err(error), Vec::new()), + None => (nostr_diff, nostr_commits), + }; + + this.update_in(cx, |this, _window, cx| { + this.loading = false; + this.current_commit = current_commit.map(SharedString::from); + this.commits = commits; + match diff { + Ok(diff) => { + let mut paths: Vec = diff + .files + .iter() + .map(|file| PathBuf::from(&file.path)) + .collect(); + paths.sort(); + let items = tree_items(build_tree_items(&paths), true); + let first = diff + .files + .first() + .map(|file| SharedString::from(file.path.as_str())); + this.tree_state.update(cx, |state, cx| { + state.set_items(items.clone(), cx); + let item = find_item(&items, first.as_deref()); + state.set_selected_item(item, cx); + }); + this.selected_file = first.clone(); + this.diff = Some(diff); + if let Some(path) = first { + this.set_diff_rows(path.as_ref()); + } + } + Err(error) => { + this.error = Some(error.to_string().into()); + } + } + cx.notify(); + })?; + + Ok(()) + }); + + self.tasks.retain(|task| !task.is_ready()); + self.tasks.push(task); + } + + /// Show the diff of the file at `path` (selected in the tree). + fn select_file(&mut self, path: &str, cx: &mut Context) { + self.selected_file = Some(path.into()); + self.set_diff_rows(path); + cx.notify(); + } + + /// Rebuild the virtual list state for the file at `path` and scroll back + /// to the top. + fn set_diff_rows(&mut self, path: &str) { + let Some(diff) = self.diff.as_ref() else { + return; + }; + let Some(file) = diff.files.iter().find(|file| file.path == path) else { + return; + }; + self.rows = diff_rows(file); + self.item_sizes = Rc::new(vec![size(px(0.), px(DIFF_ROW_HEIGHT)); self.rows.len()]); + self.scroll_handle.scroll_to_item(0, ScrollStrategy::Top); + } + + /// One row of the changed-files tree: icon + name, indented by depth. + fn render_tree_item( + ix: usize, + entry: &TreeEntry, + selected: bool, + view: &WeakEntity, + ) -> ListItem { + let view = view.clone(); + let id = entry.item().id.clone(); + + tree_row(ix, entry, selected, move |_window, cx| { + if let Some(view) = view.upgrade() { + view.update(cx, |this, cx| this.select_file(&id, cx)); + } + }) + } + + /// Left column: the changed-files tree. + fn render_tree_column(&self, cx: &mut Context) -> AnyElement { + let tree_state = self.tree_state.clone(); + let view = cx.entity().downgrade(); + + v_flex() + .h_full() + .w(px(TREE_WIDTH)) + .flex_none() + .border_r_1() + .border_color(cx.theme().border) + .child( + div() + .flex_1() + .min_h_0() + .when(self.diff.is_some(), |this| { + this.child( + tree(&tree_state, move |ix, entry, selected, _window, _cx| { + Self::render_tree_item(ix, entry, selected, &view) + }) + .p_2(), + ) + }) + .when(self.diff.is_none() && !self.loading, |this| { + this.child(placeholder("Failed to load diff", cx)) + }), + ) + .into_any_element() + } + + /// Right column: header of the selected file plus its diff. + fn render_detail_column(&self, cx: &mut Context) -> AnyElement { + if self.loading { + return v_flex() + .size_full() + .items_center() + .justify_center() + .child(Spinner::new().small()) + .into_any_element(); + } + if let Some(error) = self.error.clone() { + return placeholder(&error, cx); + } + let Some(diff) = self.diff.as_ref() else { + return placeholder("Failed to load diff", cx); + }; + let Some(path) = self.selected_file.clone() else { + return if diff.files.is_empty() { + placeholder("No files changed in this pull request", cx) + } else { + placeholder("Select a file", cx) + }; + }; + let Some(file) = diff.files.iter().find(|file| file.path == path.as_ref()) else { + return placeholder("File not found", cx); + }; + self.render_file_diff(file, cx.entity(), cx) + } + + /// The diff of one file: a header with status and stats, then the hunks + /// in a virtual list (a large diff is never materialized per frame). + fn render_file_diff(&self, file: &FileDiff, view: Entity, cx: &App) -> AnyElement { + let status_label = match file.status { + signed_git::DiffStatus::Added => "A", + signed_git::DiffStatus::Modified => "M", + signed_git::DiffStatus::Deleted => "D", + signed_git::DiffStatus::Renamed => "R", + signed_git::DiffStatus::Copied => "C", + }; + let status_color = match file.status { + signed_git::DiffStatus::Added => cx.theme().success, + signed_git::DiffStatus::Modified => cx.theme().info, + signed_git::DiffStatus::Deleted => cx.theme().danger, + signed_git::DiffStatus::Renamed | signed_git::DiffStatus::Copied => { + cx.theme().muted_foreground + } + }; + let title = match &file.old_path { + Some(old) => format!("{old} → {}", file.path), + None => file.path.clone(), + }; + + let body: AnyElement = if file.binary { + placeholder("Diff not available", cx) + } else if file.hunks.is_empty() { + placeholder("No content changes", cx) + } else { + let sizes = self.item_sizes.clone(); + let scroll_handle = self.scroll_handle.clone(); + v_flex() + .size_full() + .relative() + .child( + v_virtual_list( + view, + "pr-diff-rows", + sizes, + move |this, range, _window, cx| { + let Some(diff) = this.diff.as_ref() else { + return Vec::new(); + }; + let Some(path) = this.selected_file.as_deref() else { + return Vec::new(); + }; + let Some(file) = diff.files.iter().find(|file| file.path == path) + else { + return Vec::new(); + }; + range + .map(|ix| render_diff_row(&file.hunks, this.rows[ix], cx)) + .collect() + }, + ) + .track_scroll(&scroll_handle) + .size_full(), + ) + .child( + div() + .absolute() + .top_0() + .left_0() + .right_0() + .bottom_0() + .child(Scrollbar::vertical(&scroll_handle)), + ) + .into_any_element() + }; + + v_flex() + .flex_1() + .min_w_0() + .h_full() + .child( + h_flex() + .px_3() + .h_9() + .gap_2() + .items_center() + .child( + div() + .text_xs() + .font_semibold() + .text_color(status_color) + .child(status_label), + ) + .child( + div() + .flex_1() + .min_w_0() + .text_xs() + .font_semibold() + .text_ellipsis() + .whitespace_nowrap() + .child(title), + ) + .when(!file.binary, |this| { + this.child( + h_flex() + .gap_2() + .text_xs() + .child( + div() + .text_color(cx.theme().success) + .child(format!("+{}", file.insertions)), + ) + .child( + div() + .text_color(cx.theme().danger) + .child(format!("-{}", file.deletions)), + ), + ) + }), + ) + .child(div().id("pr-diff-body").flex_1().min_h_0().child(body)) + .into_any_element() + } + + /// Top panel: title, status, author, description, commits and comments, + /// one scrollable column with the comment form pinned at the bottom. + fn render_conversation(&mut self, cx: &mut Context) -> AnyElement { + if self.loading { + return v_flex() + .size_full() + .items_center() + .justify_center() + .child(Spinner::new().small()) + .into_any_element(); + } + if let Some(error) = self.error.clone() { + return placeholder(&error, cx); + } + + let (title, author, picture, status, age, root_id, branch) = { + let store = self.store.read(cx); + let Some(root) = store + .pull_requests + .iter() + .find(|pr| pr.id == self.pr_id && pr.kind == Kind::GitPullRequest) + else { + return placeholder("Pull request not found", cx); + }; + let profile = ProfileStore::global(cx).read(cx).get(&root.pubkey); + ( + activity_subject(root), + profile.name(), + profile.picture(), + store.status_of(root), + relative_time(root.created_at), + root.id, + branch_name_of(root), + ) + }; + let current_commit = self.current_commit.clone(); + + v_flex() + .size_full() + .child( + v_flex() + .flex_1() + .min_h_0() + .px_4() + .py_3() + .gap_4() + .overflow_y_scrollbar() + .child( + // Title row: status badge + subject. + h_flex() + .gap_2() + .items_center() + .child(status_badge(status, cx)) + .child(div().font_semibold().child(title)), + ) + .child( + // Author, age, tip commit and branch. + h_flex() + .gap_2() + .items_center() + .text_sm() + .child( + h_flex() + .gap_1() + .child( + Avatar::new() + .name(author.clone()) + .when_some(picture, |this, url| this.src(url)) + .small(), + ) + .child(author), + ) + .child(SharedString::from("opened")) + .child( + div() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(age)), + ) + .when_some(current_commit, |this, id| { + this.child( + div() + .font_family(cx.theme().mono_font_family.clone()) + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(id.clone()), + ) + .child(Clipboard::new("pr-commit").value(&id)) + }) + .when_some(branch, |this, branch| { + this.child( + div() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(branch), + ) + }), + ) + .when(!self.description.is_empty(), |this| { + // Description, plain text for now. + this.child(div().text_sm().child(self.description.clone())) + }) + .child(self.render_commits(cx)) + .child(self.render_comments(&root_id, cx)), + ) + .child( + h_flex() + .px_4() + .py_3() + .border_t_1() + .border_color(cx.theme().border) + .child(self.render_form(&root_id, cx)), + ) + .into_any_element() + } + + /// The commits of the patch series: id, summary, author and time. + fn render_commits(&self, cx: &App) -> AnyElement { + if self.commits.is_empty() { + return div().into_any_element(); + } + + v_flex() + .gap_2() + .child( + div() + .text_xs() + .font_semibold() + .text_color(cx.theme().muted_foreground) + .child("Commits"), + ) + .children(self.commits.iter().map(|commit| { + let meta = commit_meta(commit); + h_flex() + .gap_2() + .items_center() + .text_sm() + .child( + div() + .font_family(cx.theme().mono_font_family.clone()) + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(commit.id.clone()), + ) + .child( + div() + .flex_1() + .min_w_0() + .text_ellipsis() + .whitespace_nowrap() + .child(commit.summary.clone()), + ) + .when(!meta.is_empty(), |this| { + this.child( + div() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(meta)), + ) + }) + })) + .into_any_element() + } + + fn render_comments(&mut self, id: &EventId, cx: &mut Context) -> AnyElement { + let store = self.store.read(cx); + let comments: Vec<&Event> = store.comments_of(id).collect(); + + v_flex() + .gap_3() + .when(!comments.is_empty(), |this| { + this.child( + div() + .text_xs() + .font_semibold() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(format!("Comments ({})", comments.len()))), + ) + }) + .children(comments.iter().map(|comment| { + let profile = ProfileStore::global(cx).read(cx).get(&comment.pubkey); + let author = profile.name(); + let picture = profile.picture(); + let age = relative_time(comment.created_at); + + v_flex() + .gap_1() + .child( + h_flex() + .gap_2() + .text_sm() + .child( + h_flex() + .gap_1() + .child( + Avatar::new() + .name(author.clone()) + .when_some(picture, |this, url| this.src(url)) + .xsmall(), + ) + .child(author), + ) + .child( + div() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(age)), + ), + ) + .child( + div() + .text_sm() + .child(SharedString::from(comment.content.clone())), + ) + })) + .into_any_element() + } + + fn render_form(&mut self, id: &EventId, _cx: &mut Context) -> AnyElement { + let comment_input = self.comment_input.clone(); + let store = self.store.clone(); + let id = id.to_owned(); + + v_flex() + .flex_1() + .min_w_0() + .gap_2() + .child(Textarea::new(&self.comment_input).h(px(72.))) + .child( + h_flex().justify_end().child( + Button::new("pr-comment") + .primary() + .label("Comment") + .tooltip("Post comment") + .on_click(move |_event, window, cx| { + let content = comment_input.read(cx).value().trim().to_string(); + if content.is_empty() { + return; + } + let Some(root) = store + .read(cx) + .pull_requests + .iter() + .find(|pr| pr.id == id) + .cloned() + else { + return; + }; + store.update(cx, |store, cx| { + store.comment(&root, content, cx); + }); + comment_input.update(cx, |input, cx| { + input.set_value("", window, cx); + }); + }), + ), + ) + .into_any_element() + } +} + +/// The `c` tag of a PR event (tip of the proposed branch), as hex. +fn current_commit_of(event: &Event) -> Option { + event + .tags + .iter() + .find_map(|tag| match Nip34Tag::parse(tag.as_slice()) { + Ok(Nip34Tag::CurrentCommit(commit)) => Some(commit.to_string()), + _ => None, + }) +} + +/// The `merge-base` tag of a PR event (most recent common ancestor with the +/// target branch), as hex. +fn merge_base_of(event: &Event) -> Option { + event + .tags + .iter() + .find_map(|tag| match Nip34Tag::parse(tag.as_slice()) { + Ok(Nip34Tag::MergeBase(commit)) => Some(commit.to_string()), + _ => None, + }) +} + +/// The `clone` tag of a PR event (URLs where the proposed branch can be +/// fetched), or `None` if the PR has none. +fn clone_urls_of(event: &Event) -> Option> { + event + .tags + .iter() + .find_map(|tag| match Nip34Tag::parse(tag.as_slice()) { + Ok(Nip34Tag::Clone(urls)) => Some(urls.iter().map(ToString::to_string).collect()), + _ => None, + }) +} + +/// The `branch-name` tag of a PR event, if any. +fn branch_name_of(event: &Event) -> Option { + event + .tags + .iter() + .find_map(|tag| match Nip34Tag::parse(tag.as_slice()) { + Ok(Nip34Tag::BranchName(name)) => Some(name), + _ => None, + }) +} + +/// The latest PR update (kind 1619) revising `root`, found via its NIP-22 +/// `E` tag pointing at the root PR event. +fn latest_update<'a>(events: impl Iterator, root: &EventId) -> Option<&'a Event> { + let root_hex = root.to_hex(); + events + .filter(|e| e.kind == Kind::GitPullRequestUpdate) + .filter(|e| { + e.tags + .iter() + .any(|t| t.kind() == "E" && t.content() == Some(root_hex.as_str())) + }) + .max_by_key(|e| e.created_at) +} + +/// One-line commit metadata for the commits list: author and relative time, +/// whichever is available. +fn commit_meta(commit: &FileCommit) -> String { + let author = commit.author.trim(); + let time = commit.time > 0; + match (author.is_empty(), time) { + (false, true) => format!("{author} · {}", relative_time_secs(commit.time)), + (false, false) => author.to_string(), + (true, true) => relative_time_secs(commit.time), + (true, false) => String::new(), + } +} + +impl Panel for PullRequestDetailView { + fn panel_name(&self) -> &'static str { + "pull-request-detail" + } + + fn title(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let subject = self + .store + .read(cx) + .pull_requests + .iter() + .find(|pr| pr.id == self.pr_id && pr.kind == Kind::GitPullRequest) + .map(activity_subject) + .unwrap_or_else(|| SharedString::from("Pull request")); + + div().text_sm().child(subject) + } +} + +impl EventEmitter for PullRequestDetailView {} + +impl Focusable for PullRequestDetailView { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for PullRequestDetailView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + v_resizable("pull-request-detail") + .child( + resizable_panel() + .size(px(320.)) + .size_range(px(160.)..px(600.)) + .flex_none() + .bg(cx.theme().background) + .child(self.render_conversation(cx)), + ) + .child( + resizable_panel().child( + h_flex() + .size_full() + .min_h_0() + .bg(cx.theme().background) + .child(self.render_tree_column(cx)) + .child(self.render_detail_column(cx)), + ), + ) + } +} + +#[cfg(test)] +mod tests { + use nostr::prelude::*; + + use super::*; + + const COMMIT_HEX: &str = "1111111111111111111111111111111111111111"; + const OTHER_ROOT_HEX: &str = "2222222222222222222222222222222222222222"; + + fn keys() -> Keys { + Keys::new( + SecretKey::from_hex("0000000000000000000000000000000000000000000000000000000000000001") + .expect("valid secret key"), + ) + } + + /// Build a signed event with a controlled `created_at`. + fn signed(kind: Kind, tags: Vec, created_at: u64) -> Event { + EventBuilder::new(kind, "") + .tags(tags) + .custom_created_at(Timestamp::from(created_at)) + .finalize(&keys()) + .expect("signed event") + } + + fn pr_root() -> Event { + signed( + Kind::GitPullRequest, + vec![ + Tag::parse(["c", COMMIT_HEX]).expect("valid tag"), + Tag::parse(["branch-name", "feature/x"]).expect("valid tag"), + ], + 100, + ) + } + + #[test] + fn reads_current_commit_and_branch_name() { + let pr = pr_root(); + assert_eq!(current_commit_of(&pr).as_deref(), Some(COMMIT_HEX)); + assert_eq!(branch_name_of(&pr).as_deref(), Some("feature/x")); + } + + #[test] + fn returns_none_without_pr_tags() { + let pr = signed(Kind::GitPullRequest, vec![], 100); + assert_eq!(current_commit_of(&pr), None); + assert_eq!(branch_name_of(&pr), None); + } + + #[test] + fn latest_update_picks_newest_revision_of_the_root() { + let root = pr_root(); + let root_hex = root.id.to_hex(); + + let revision = |created_at: u64| { + signed( + Kind::GitPullRequestUpdate, + vec![Tag::parse(["E", &root_hex]).expect("valid tag")], + created_at, + ) + }; + // An update revising a different PR must be ignored even though it + // is newer. + let unrelated = signed( + Kind::GitPullRequestUpdate, + vec![Tag::parse(["E", OTHER_ROOT_HEX]).expect("valid tag")], + 999, + ); + + let events = [unrelated, revision(200), root.clone(), revision(300)]; + let latest = latest_update(events.iter(), &root.id).expect("an update"); + + assert_eq!(latest.created_at.as_secs(), 300); + assert_eq!(latest.kind, Kind::GitPullRequestUpdate); + } + + #[test] + fn latest_update_ignores_roots_without_revisions() { + let root = pr_root(); + assert!(latest_update([&root].into_iter(), &root.id).is_none()); + } + + #[test] + fn commit_meta_combines_author_and_time() { + let commit = |author: &str, time: i64| FileCommit { + id: COMMIT_HEX.into(), + summary: "summary".into(), + description: None, + author: author.into(), + time, + }; + + assert_eq!(commit_meta(&commit("Alice", 0)), "Alice"); + assert_eq!(commit_meta(&commit("", 0)), ""); + assert!(!commit_meta(&commit("", 1_000_000)).is_empty()); + assert!(!commit_meta(&commit("Alice", 1_000_000)).is_empty()); + } +} diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index 692b570..c99b42e 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -1,7 +1,8 @@ use std::rc::Rc; +use std::sync::Arc; use assets::CustomIconName; -use dock::{DockArea, Panel, PanelEvent}; +use dock::{DockArea, DockPlacement, Panel, PanelEvent}; use gpui::prelude::*; use gpui::{ AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, @@ -23,6 +24,7 @@ use signed_state::{ProfileStore, RepoStore}; use utils::relative_time; use super::helpers::{placeholder, status_badge}; +use super::pull_request_detail::PullRequestDetailView; /// Height of one pull request row in the virtual list: same layout as an /// issue row (8px vertical padding (`py_2`) on top and bottom, a 32px title @@ -120,7 +122,12 @@ impl PullRequestsView { let Some(dock_area) = self.dock_area.upgrade() else { return; }; - // TODO + + let panel = cx.new(|cx| PullRequestDetailView::new(self.store.clone(), pr_id, window, cx)); + + dock_area.update(cx, |dock_area, cx| { + dock_area.add_panel(Arc::new(panel), DockPlacement::Bottom, window, cx); + }); } /// Render one row of the pull request list; `ix` is the row index and @@ -419,15 +426,19 @@ impl PullRequestsView { } } -/// Open the "new pull request" dialog: a title and a patch input that -/// submit through [`RepoStore::open_pull_request`] when confirmed. +/// Open the "new pull request" dialog: a title, an optional description and +/// a patch input that submit through [`RepoStore::open_pull_request`] when +/// confirmed. fn open_new_pull_request_dialog(store: Entity, window: &mut Window, cx: &mut App) { let subject = cx.new(|cx| InputState::new(window, cx).placeholder("Pull request title")); + let description = + cx.new(|cx| TextareaState::new(window, cx).placeholder("Describe the change…")); let patch = cx.new(|cx| TextareaState::new(window, cx).placeholder("Paste `git format-patch` output…")); window.open_dialog(cx, move |dialog, _window, _cx| { let subject = subject.clone(); + let description = description.clone(); let patch = patch.clone(); let store = store.clone(); @@ -451,6 +462,11 @@ fn open_new_pull_request_dialog(store: Entity, window: &mut Window, c .required(true) .child(Input::new(&subject)), ) + .child( + field() + .label("Description") + .child(Textarea::new(&description).h(px(96.))), + ) .child( field() .label("Patch") @@ -465,16 +481,18 @@ fn open_new_pull_request_dialog(store: Entity, window: &mut Window, c .tooltip("Create pull request") .on_click({ let subject = subject.clone(); + let description = description.clone(); let patch = patch.clone(); let store = store.clone(); move |_event, window, cx| { let subject = subject.read(cx).value().to_string(); + let description = description.read(cx).value().to_string(); let patch = patch.read(cx).value().to_string(); let subject = (!subject.is_empty()).then_some(subject); store.update(cx, |store, cx| { - store.open_pull_request(subject, patch, cx); + store.open_pull_request(subject, description, patch, cx); }); window.close_dialog(cx);