From 385718d74d8718b3b9dfc82ad65c626c26169c95 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 22 Aug 2026 16:57:24 +0700 Subject: [PATCH] update --- crates/signed_git/src/lib.rs | 630 ++++++++++++++++++ crates/signed_state/src/repo.rs | 64 ++ .../src/views/repo_detail/browser.rs | 4 +- .../workspace/src/views/repo_detail/diff.rs | 4 +- .../src/views/repo_detail/helpers.rs | 18 +- .../workspace/src/views/repo_detail/issues.rs | 1 + crates/workspace/src/views/repo_detail/mod.rs | 25 +- .../src/views/repo_detail/pull_requests.rs | 320 ++++++--- 8 files changed, 938 insertions(+), 128 deletions(-) diff --git a/crates/signed_git/src/lib.rs b/crates/signed_git/src/lib.rs index 064b6db..3ccd4f4 100644 --- a/crates/signed_git/src/lib.rs +++ b/crates/signed_git/src/lib.rs @@ -604,6 +604,348 @@ fn commit_diff(repo: &gix::Repository, id: &str) -> Result { Ok(CommitDiff { files }) } +/// Parse a `git format-patch` output (a single patch or a patch series) +/// into the same [`CommitDiff`] structure used for commit diffs. +/// +/// The mbox envelope (From/Subject/… headers, commit body and diffstat) +/// is skipped; every `diff --git` section becomes one [`FileDiff`]. Paths +/// are taken from the section headers, with git's C-style quoting undone. +/// Sections without hunks (pure renames, mode changes, binary files) are +/// reported without lines. +pub fn patch_diffs(patch: &str) -> Result { + let lines: Vec<&str> = patch.lines().collect(); + let mut files = Vec::new(); + let mut i = 0; + + while i < lines.len() { + let Some(header) = lines[i].strip_prefix("diff --git ") else { + i += 1; + continue; + }; + let (file, next) = parse_diff_section(header, &lines, i + 1)?; + files.push(file); + i = next; + } + + Ok(CommitDiff { files }) +} + +/// 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. +fn parse_diff_section(header: &str, lines: &[&str], start: usize) -> Result<(FileDiff, usize)> { + let (header_old, header_new) = header_paths(header)?; + // The `---`/`+++` lines name the two sides unambiguously (the header + // can't distinguish spaces); fall back to the header for sections + // without them (pure renames, mode changes). + let mut old_path = header_old; + let mut new_path = header_new; + + let mut status = DiffStatus::Modified; + let mut binary = false; + let mut hunks = Vec::new(); + let mut insertions = 0usize; + let mut deletions = 0usize; + let mut i = start; + + while i < lines.len() { + let line = lines[i]; + + // The next file's section starts at this line. + if line.starts_with("diff --git ") { + break; + } + i += 1; + + if line.starts_with("@@ -") { + let (hunk, next) = parse_hunk(lines, i - 1)?; + i = next; + insertions += hunk + .lines + .iter() + .filter(|line| line.kind == DiffLineKind::Addition) + .count(); + deletions += hunk + .lines + .iter() + .filter(|line| line.kind == DiffLineKind::Deletion) + .count(); + hunks.push(hunk); + } else if let Some(rest) = line.strip_prefix("--- ") { + if rest == "/dev/null" { + status = DiffStatus::Added; + } else { + old_path = diff_line_path(rest, "a/")?; + } + } else if let Some(rest) = line.strip_prefix("+++ ") { + if rest == "/dev/null" { + status = DiffStatus::Deleted; + } else { + new_path = diff_line_path(rest, "b/")?; + } + } else if line.starts_with("new file mode ") { + status = DiffStatus::Added; + } else if line.starts_with("deleted file mode ") { + status = DiffStatus::Deleted; + } else if line.starts_with("copy from ") { + status = DiffStatus::Copied; + } else if line.starts_with("rename from ") { + status = DiffStatus::Renamed; + } else if line.starts_with("Binary files ") || line.starts_with("GIT binary patch") { + binary = true; + // A literal binary patch may follow; skip it without consuming + // the next section's header. + while i < lines.len() && !lines[i].starts_with("diff --git ") { + i += 1; + } + break; + } + // Everything else (index/mode/similarity lines) is ignored. + } + + Ok(( + FileDiff { + path: new_path, + old_path: matches!(status, DiffStatus::Renamed | DiffStatus::Copied) + .then_some(old_path), + status, + insertions, + deletions, + binary, + hunks, + }, + i, + )) +} + +/// Parse one hunk: the `@@ -a,b +c,d @@` header plus every body line up to +/// the next hunk header, the next `diff --git` section or the end of the +/// patch. Returns the hunk and the index of the first unconsumed line. +fn parse_hunk(lines: &[&str], start: usize) -> Result<(DiffHunk, usize)> { + let (old_start, old_lines, new_start, new_lines) = hunk_header(lines[start])?; + + let mut diff_lines = Vec::new(); + let mut old = old_start; + let mut new = new_start; + let mut i = start + 1; + + while i < lines.len() { + let line = lines[i]; + let Some(kind) = line_prefix_kind(line) else { + break; + }; + i += 1; + + // Context lines advance both counters, deletions only the old one + // and additions only the new one, so every line ends up with its + // real line number in both versions. + let (old_no, new_no) = match kind { + DiffLineKind::Context => { + let numbers = (Some(old), Some(new)); + old += 1; + new += 1; + numbers + } + DiffLineKind::Addition => { + let number = Some(new); + new += 1; + (None, number) + } + DiffLineKind::Deletion => { + let number = Some(old); + old += 1; + (number, None) + } + }; + diff_lines.push(DiffLine { + kind, + old: old_no, + new: new_no, + text: line[1..].to_owned(), + }); + } + + Ok(( + DiffHunk { + old_start, + old_lines, + new_start, + new_lines, + lines: diff_lines, + }, + i, + )) +} + +/// The kind of a hunk body line, from its first character; lines that don't +/// belong to the hunk (headers, `\ No newline…`, the next section) yield +/// `None`. +fn line_prefix_kind(line: &str) -> Option { + match line.as_bytes().first()? { + b' ' => Some(DiffLineKind::Context), + b'+' => Some(DiffLineKind::Addition), + b'-' => Some(DiffLineKind::Deletion), + _ => None, + } +} + +/// Parse a unified-diff hunk header `@@ -a,b +c,d @@`; omitted line counts +/// default to 1. +fn hunk_header(header: &str) -> Result<(u32, u32, u32, u32)> { + let rest = header + .strip_prefix("@@ ") + .context("malformed hunk header")?; + let (old_spec, rest) = rest.split_once(' ').context("malformed hunk header")?; + let new_spec = rest.split_once(' ').map(|(new, _)| new).unwrap_or(rest); + + fn parse(spec: &str) -> Result<(u32, u32)> { + let spec = spec.strip_prefix(['-', '+']).unwrap_or(spec); + let (start, count) = match spec.split_once(',') { + Some((start, count)) => (start, count.parse::()?), + None => (spec, 1), + }; + Ok((start.parse::()?, count)) + } + + let (old_start, old_lines) = parse(old_spec)?; + let (new_start, new_lines) = parse(new_spec)?; + Ok((old_start, old_lines, new_start, new_lines)) +} + +/// The old and new paths of a `diff --git a/X b/Y` header, with git's +/// C-style quoting undone. +/// +/// Git only quotes paths containing characters that need escaping (non-ASCII +/// bytes, `"`, `\`); plain spaces are left unquoted, so the two sides of an +/// unquoted header are split at the last ` b/`. +fn header_paths(header: &str) -> Result<(String, String)> { + if header.starts_with('"') { + // Quoted paths include the `a/` / `b/` prefix inside the quotes. + let (old, rest) = take_quoted(header).context("unterminated quoted path")?; + let rest = rest.trim_start(); + let new = if rest.starts_with('"') { + take_quoted(rest).context("unterminated quoted path")?.0 + } else { + rest.split_whitespace().next().unwrap_or(rest) + }; + let old = old + .strip_prefix("a/") + .context("old path without `a/` prefix")?; + let new = new + .strip_prefix("b/") + .context("new path without `b/` prefix")?; + Ok((unquote_path(old)?, unquote_path(new)?)) + } else { + let (old, rest) = header + .rsplit_once(" b/") + .context("malformed diff --git header")?; + let old = old + .strip_prefix("a/") + .context("old path without `a/` prefix")?; + Ok((old.to_owned(), rest.to_owned())) + } +} + +/// The path of a `--- a/X` / `+++ b/Y` line: the prefix stripped, git's +/// trailing padding tab (for paths containing spaces) removed and C-style +/// quoting undone. These lines name the two sides unambiguously, unlike the +/// `diff --git` header. +fn diff_line_path(line: &str, prefix: &str) -> Result { + let line = line.trim_end_matches('\t'); + if line.starts_with('"') { + let (path, _) = take_quoted(line).context("unterminated quoted path")?; + let path = path + .strip_prefix(prefix) + .context("diff line path without `a/` or `b/` prefix")?; + unquote_path(path) + } else { + Ok(line + .strip_prefix(prefix) + .context("diff line path without `a/` or `b/` prefix")? + .to_owned()) + } +} + +/// The content of a git C-style quoted path (opening `"`, escaped content, +/// closing `"`) and the rest of the input; `None` if unterminated. +/// +/// Iterates by character so the returned slices always land on UTF-8 +/// boundaries, even for non-ASCII paths. +fn take_quoted(input: &str) -> Option<(&str, &str)> { + let mut end = 1; // byte after the opening quote + let mut rest = &input[1..]; + while let Some(ch) = rest.chars().next() { + let len = ch.len_utf8(); + match ch { + '\\' => { + // Consume the escaped character too (it may be multi-byte). + let escaped = rest[len..].chars().next()?; + let consumed = len + escaped.len_utf8(); + end += consumed; + rest = &rest[consumed..]; + } + '"' => return Some((&input[1..end], &input[end + len..])), + _ => { + end += len; + rest = &rest[len..]; + } + } + } + None +} + +/// Undo git's C-style path quoting (`\NNN` octal escapes, `\"`, `\\`). +fn unquote_path(path: &str) -> Result { + if !path.contains('\\') { + return Ok(path.to_owned()); + } + + let mut out = Vec::with_capacity(path.len()); + let mut bytes = path.as_bytes(); + while let Some((&b, rest)) = bytes.split_first() { + bytes = rest; + if b == b'\\' { + match bytes.split_first() { + Some((&b'"', rest)) | Some((&b'\\', rest)) => { + out.push(b); + bytes = rest; + } + Some((&b'n', rest)) => { + out.push(b'\n'); + bytes = rest; + } + Some((&b't', rest)) => { + out.push(b'\t'); + bytes = rest; + } + Some((&d1, rest)) if (b'0'..=b'7').contains(&d1) => { + let Some((&d2, rest)) = rest.split_first() else { + bail!("malformed octal escape in quoted path"); + }; + let Some((&d3, rest)) = rest.split_first() else { + bail!("malformed octal escape in quoted path"); + }; + if !(b'0'..=b'7').contains(&d2) || !(b'0'..=b'7').contains(&d3) { + bail!("malformed octal escape in quoted path"); + } + let code = + (d1 - b'0') as u16 * 64 + (d2 - b'0') as u16 * 8 + (d3 - b'0') as u16; + if code > u8::MAX as u16 { + bail!("octal escape out of range in quoted path"); + } + out.push(code as u8); + bytes = rest; + } + _ => bail!("malformed escape in quoted path"), + } + } else { + out.push(b); + } + } + + String::from_utf8(out).context("invalid UTF-8 in quoted path") +} + /// Collects the hunks of one blob diff while tracking per-line numbers. /// /// The unified-diff headers give the 1-based start line of the hunk in each @@ -1401,4 +1743,292 @@ mod tests { assert_eq!(file.insertions, 0); assert_eq!(file.deletions, 0); } + + #[test] + fn parses_format_patch_output() { + let patch = r#"From 1f6c0c5f3f1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a Mon Sep 17 00:00:00 2001 +From: A +Subject: [PATCH] fix + +fix the thing + +--- + src/lib.rs | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/lib.rs b/src/lib.rs +index 1234567..89abcde 100644 +--- a/src/lib.rs ++++ b/src/lib.rs +@@ -1,3 +1,3 @@ + fn main() { +- println!("old"); ++ println!("new"); + } +"#; + let diff = patch_diffs(patch).expect("parse"); + + assert_eq!(diff.files.len(), 1); + let file = &diff.files[0]; + assert_eq!(file.path, "src/lib.rs"); + assert_eq!(file.old_path, None); + assert_eq!(file.status, DiffStatus::Modified); + assert_eq!(file.insertions, 1); + assert_eq!(file.deletions, 1); + + let hunk = &file.hunks[0]; + assert_eq!(hunk.old_start, 1); + assert_eq!(hunk.old_lines, 3); + assert_eq!(hunk.new_start, 1); + assert_eq!(hunk.new_lines, 3); + assert_eq!(hunk.lines.len(), 4); + assert_eq!(hunk.lines[0].kind, DiffLineKind::Context); + assert_eq!(hunk.lines[0].old, Some(1)); + assert_eq!(hunk.lines[0].new, Some(1)); + assert_eq!(hunk.lines[1].kind, DiffLineKind::Deletion); + assert_eq!(hunk.lines[1].old, Some(2)); + assert_eq!(hunk.lines[1].new, None); + assert_eq!(hunk.lines[2].kind, DiffLineKind::Addition); + assert_eq!(hunk.lines[2].old, None); + assert_eq!(hunk.lines[2].new, Some(2)); + assert_eq!(hunk.lines[3].kind, DiffLineKind::Context); + assert_eq!(hunk.lines[3].old, Some(3)); + assert_eq!(hunk.lines[3].new, Some(3)); + } + + #[test] + fn parses_new_file_as_added() { + let patch = r#"diff --git a/README.md b/README.md +new file mode 100644 +index 0000000..1234567 +--- /dev/null ++++ b/README.md +@@ -0,0 +1 @@ ++# hello +"#; + let diff = patch_diffs(patch).expect("parse"); + + let file = &diff.files[0]; + assert_eq!(file.path, "README.md"); + assert_eq!(file.status, DiffStatus::Added); + assert_eq!(file.old_path, None); + assert_eq!(file.insertions, 1); + assert_eq!(file.deletions, 0); + assert_eq!(file.hunks[0].old_start, 0); + assert_eq!(file.hunks[0].old_lines, 0); + assert_eq!(file.hunks[0].new_start, 1); + } + + #[test] + fn parses_renames_with_old_path() { + let patch = r#"diff --git a/old.rs b/new.rs +similarity index 85% +rename from old.rs +rename to new.rs +index 123..456 100644 +--- a/old.rs ++++ b/new.rs +@@ -1 +1 @@ +-fn main() {} ++fn main() { println!("hi"); } +"#; + let diff = patch_diffs(patch).expect("parse"); + + let file = &diff.files[0]; + assert_eq!(file.path, "new.rs"); + assert_eq!(file.old_path.as_deref(), Some("old.rs")); + assert_eq!(file.status, DiffStatus::Renamed); + assert_eq!(file.insertions, 1); + assert_eq!(file.deletions, 1); + } + + #[test] + fn parses_patch_series_and_skips_envelope() { + let patch = r#"From aaaa Mon Sep 17 00:00:00 2001 +From: A +Subject: [PATCH 1/2] one + +--- + a.txt | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/a.txt b/a.txt +index 1..2 100644 +--- a/a.txt ++++ b/a.txt +@@ -1 +1,2 @@ + a ++b + +From bbbb Mon Sep 17 00:00:00 2001 +From: A +Subject: [PATCH 2/2] two + +diff --git a/b.txt b/b.txt +index 3..4 100644 +--- a/b.txt ++++ b/b.txt +@@ -1 +1 @@ +-x ++y +"#; + let diff = patch_diffs(patch).expect("parse"); + + assert_eq!(diff.files.len(), 2); + assert_eq!(diff.files[0].path, "a.txt"); + assert_eq!(diff.files[0].insertions, 1); + assert_eq!(diff.files[1].path, "b.txt"); + assert_eq!(diff.files[1].deletions, 1); + } + + #[test] + fn marks_binary_sections() { + let patch = r#"diff --git a/img.png b/img.png +index 123..456 100644 +Binary files a/img.png and b/img.png differ +"#; + let diff = patch_diffs(patch).expect("parse"); + + assert!(diff.files[0].binary); + assert!(diff.files[0].hunks.is_empty()); + } + + #[test] + fn unquotes_quoted_paths() { + let patch = r#"diff --git "a/weird file.rs" "b/weird file.rs" +index 123..456 100644 +--- "a/weird file.rs" ++++ "b/weird file.rs" +@@ -1 +1 @@ +-x ++y +"#; + let diff = patch_diffs(patch).expect("parse"); + + assert_eq!(diff.files[0].path, "weird file.rs"); + assert_eq!(diff.files[0].status, DiffStatus::Modified); + } + + #[test] + fn unquotes_non_ascii_quoted_paths() { + let patch = r#"diff --git "a/说明.md" "b/说明.md" +index 123..456 100644 +--- "a/说明.md" ++++ "b/说明.md" +@@ -1 +1 @@ +-x ++y +"#; + let diff = patch_diffs(patch).expect("parse"); + + assert_eq!(diff.files[0].path, "说明.md"); + assert_eq!(diff.files[0].status, DiffStatus::Modified); + } + + #[test] + fn unquotes_octal_escaped_paths() { + let patch = r#"diff --git "a/\345\270\226.md" "b/\345\270\226.md" +index 123..456 100644 +--- "a/\345\270\226.md" ++++ "b/\345\270\226.md" +@@ -1 +1 @@ +-x ++y +"#; + let diff = patch_diffs(patch).expect("parse"); + + assert_eq!(diff.files[0].path, "帖.md"); + assert_eq!(diff.files[0].status, DiffStatus::Modified); + } + + #[test] + fn empty_or_unparseable_patch_yields_no_files() { + assert_eq!(patch_diffs("").expect("parse").files.len(), 0); + assert_eq!(patch_diffs("just some text").expect("parse").files.len(), 0); + assert_eq!( + patch_diffs("---\nnot a patch\n") + .expect("parse") + .files + .len(), + 0 + ); + } + + #[test] + fn parses_real_format_patch_output() { + // Build a commit touching a mix of file kinds, then feed genuine + // `git format-patch` output through the parser: quoted paths (space + // in the name), octal-escaped paths (UTF-8 name), a rename-free + // modification, an addition and a binary deletion. + let (dir, repo) = fixture(&[ + ("src/main.rs", b"fn main() {\n println!(\"one\");\n}\n"), + ("my file.txt", b"hello\n"), + ("\u{8bf4}\u{660e}.md", "# \u{8bf4}\u{660e}\n".as_bytes()), + ("img.png", b"\x89PNG\r\n\x1a\n\x00binary"), + ]); + commit_all(&repo, "initial"); + + std::fs::write( + dir.path().join("src/main.rs"), + b"fn main() {\n println!(\"two\");\n println!(\"three\");\n}\n", + ) + .expect("write"); + std::fs::write(dir.path().join("my file.txt"), b"hello world\n").expect("write"); + std::fs::write( + dir.path().join("\u{8bf4}\u{660e}.md"), + "# \u{8bf4}\u{660e}\nupdated\n", + ) + .expect("write"); + std::fs::remove_file(dir.path().join("img.png")).expect("remove"); + std::fs::write(dir.path().join("new file.md"), b"# new\n").expect("write"); + commit_all(&repo, "changes"); + + let output = Command::new("git") + .current_dir(dir.path()) + .env("GIT_AUTHOR_NAME", "Test Author") + .env("GIT_AUTHOR_EMAIL", "test@example.com") + .env("GIT_COMMITTER_NAME", "Test Author") + .env("GIT_COMMITTER_EMAIL", "test@example.com") + .args(["format-patch", "-1", "--stdout"]) + .output() + .expect("spawn git format-patch"); + assert!(output.status.success(), "git format-patch failed"); + let patch = String::from_utf8(output.stdout).expect("patch is utf-8"); + + let diff = patch_diffs(&patch).expect("parse real format-patch output"); + + let by_path = |path: &str| { + diff.files + .iter() + .find(|file| file.path == path) + .unwrap_or_else(|| panic!("missing file {path:?}")) + }; + + // Space in the name: git quotes the path in the header. + let file = by_path("my file.txt"); + assert_eq!(file.status, DiffStatus::Modified); + assert_eq!(file.insertions, 1); + + // UTF-8 name: git emits the path as octal escapes. + let file = by_path("\u{8bf4}\u{660e}.md"); + assert_eq!(file.status, DiffStatus::Modified); + assert_eq!(file.insertions, 1); + + let file = by_path("src/main.rs"); + assert_eq!(file.status, DiffStatus::Modified); + assert_eq!(file.insertions, 2); + assert_eq!(file.deletions, 1); + assert!(!file.hunks.is_empty()); + + let file = by_path("new file.md"); + assert_eq!(file.status, DiffStatus::Added); + assert_eq!(file.insertions, 1); + + // Binary deletion: git emits no ---/+++ lines, only the mode and + // the "Binary files" marker. + let file = by_path("img.png"); + assert_eq!(file.status, DiffStatus::Deleted); + assert!(file.binary); + assert!(file.hunks.is_empty()); + } } diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index a89bdf8..98f5882 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -6,6 +6,7 @@ use nostr_sdk::prelude::*; use signed_core::{Announcement, Deletions, RepoAddr, RepoStatus, filters, parse_state}; use crate::backend::{Backend, BackendEvent}; +use crate::git_store::GitStore; /// Delay between a refresh request and the actual re-query, so bursts of /// events (e.g. per-event `NostrUpdate`s) collapse into one query. @@ -293,6 +294,13 @@ impl RepoStore { .count() } + /// Whether `user` is the author (owner) of this repository: the public + /// key of the repository address. Only the author may manage the + /// repository's pull requests (close / reopen / merge). + pub fn is_author(&self, user: &PublicKey) -> bool { + &self.addr.public_key == user + } + /// Open an issue on this repository. pub fn open_issue(&mut self, subject: Option, content: String, cx: &mut Context) { let builder = GitIssue { @@ -392,6 +400,62 @@ 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. + /// + /// Only the repository author may merge. The clone is created on demand + /// from the announcement's clone URLs when the repository hasn't been + /// mirrored locally yet. Patch application runs on a background thread + /// (`git am`); failures (e.g. a patch that no longer applies) surface in + /// [`Self::last_error`] and no status is sent. + pub fn merge_pull_request(&mut self, root: &Event, cx: &mut Context) { + self.last_error = None; + + let is_author = Backend::global(cx) + .read(cx) + .current_user() + .is_some_and(|user| self.is_author(&user)); + if !is_author { + self.last_error = Some("Only the repository author can merge pull requests".into()); + return; + } + + let cache = GitStore::global(cx).cache().clone(); + let addr = self.addr.clone(); + let clone_urls: Vec = self + .announcement + .as_ref() + .map(|a| a.clone.iter().map(ToString::to_string).collect()) + .unwrap_or_default(); + let patch = root.content.clone(); + let root = root.clone(); + + let apply = 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"))?; + signed_git::apply_patch(workdir, &patch) + }); + + self.tasks.push(cx.spawn(async move |this, cx| { + match apply.await { + Ok(()) => { + this.update(cx, |this, cx| { + this.set_status(&root, RepoStatus::Applied, cx); + })?; + } + Err(e) => { + this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + })?; + } + } + Ok(()) + })); + } + fn send(&mut self, builder: EventBuilder, cx: &mut Context) { self.last_error = None; diff --git a/crates/workspace/src/views/repo_detail/browser.rs b/crates/workspace/src/views/repo_detail/browser.rs index 212441a..41af8c6 100644 --- a/crates/workspace/src/views/repo_detail/browser.rs +++ b/crates/workspace/src/views/repo_detail/browser.rs @@ -262,7 +262,7 @@ impl RepoDetailView { .selectable(true) .scrollable(true) .p_4() - .text_xs() + .text_sm() .into_any_element() } @@ -306,7 +306,7 @@ impl RepoDetailView { .bordered(false) .rounded_none() .h_full() - .text_xs() + .text_sm() .into_any_element() } } diff --git a/crates/workspace/src/views/repo_detail/diff.rs b/crates/workspace/src/views/repo_detail/diff.rs index f928abd..37262a1 100644 --- a/crates/workspace/src/views/repo_detail/diff.rs +++ b/crates/workspace/src/views/repo_detail/diff.rs @@ -25,7 +25,7 @@ use gpui_component::{ use signed_git::{CommitDiff, DiffHunk, DiffLine, DiffLineKind, DiffStatus, FileCommit, FileDiff}; use utils::relative_time_secs; -use super::helpers::{build_tree_items, placeholder, track, tree_items, tree_row}; +use super::helpers::{build_tree_items, placeholder, tree_items, tree_row}; /// Width of the changed-files column. const TREE_WIDTH: f32 = 260.; @@ -181,7 +181,7 @@ impl CommitDiffView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Show the diff of the file at `path` (selected in the tree). diff --git a/crates/workspace/src/views/repo_detail/helpers.rs b/crates/workspace/src/views/repo_detail/helpers.rs index a79d07c..43fbbe9 100644 --- a/crates/workspace/src/views/repo_detail/helpers.rs +++ b/crates/workspace/src/views/repo_detail/helpers.rs @@ -4,10 +4,9 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use anyhow::Error; use assets::CustomIconName; use gpui::prelude::*; -use gpui::{AnyElement, App, Task, Window, div, px}; +use gpui::{AnyElement, App, Window, div, px}; use gpui_component::list::ListItem; use gpui_component::tooltip::Tooltip; use gpui_component::tree::{TreeEntry, TreeItem}; @@ -89,13 +88,6 @@ where }) } -/// Track `task` until it completes; finished tasks are pruned on every push -/// so the vec stays bounded by the number of in-flight loads. -pub(super) fn track(tasks: &mut Vec>>, task: Task>) { - tasks.retain(|task| !task.is_ready()); - tasks.push(task); -} - /// Build nested tree items from a flat, sorted (dirs-first) entry list. /// /// Returns [`TreeItemSeed`]s so the build can run off the main thread; a @@ -238,8 +230,8 @@ pub(super) fn status_badge(status: RepoStatus, cx: &App) -> AnyElement { CustomIconName::GitIssueOpen, "open", "Issue is open", - cx.theme().secondary, - cx.theme().secondary_foreground, + cx.theme().primary, + cx.theme().primary_foreground, ), RepoStatus::Closed => ( CustomIconName::GitIssueClosed, @@ -259,8 +251,8 @@ pub(super) fn status_badge(status: RepoStatus, cx: &App) -> AnyElement { CustomIconName::GitIssueOpen, "applied", "Issue is completed", - cx.theme().primary, - cx.theme().primary_foreground, + cx.theme().secondary, + cx.theme().secondary_foreground, ), }; diff --git a/crates/workspace/src/views/repo_detail/issues.rs b/crates/workspace/src/views/repo_detail/issues.rs index 706c65b..9421fb1 100644 --- a/crates/workspace/src/views/repo_detail/issues.rs +++ b/crates/workspace/src/views/repo_detail/issues.rs @@ -216,6 +216,7 @@ impl IssuesView { .gap_3() .border_b_1() .border_color(cx.theme().border) + .bg(cx.theme().muted.opacity(0.5)) .child( h_flex() .h_12() diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index 38b9718..655d184 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -42,7 +42,7 @@ use browser::{ }; use commits::COMMIT_ROW_HEIGHT; use diff::CommitDiffView; -use helpers::{TreeItemSeed, build_tree_items, is_markdown_path, track, tree_items}; +use helpers::{TreeItemSeed, build_tree_items, is_markdown_path, tree_items}; use issues::IssuesView; use pull_requests::PullRequestsView; @@ -350,7 +350,7 @@ impl RepoDetailView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Apply the loaded repository data: explorer tree, README preview, ref @@ -522,7 +522,7 @@ impl RepoDetailView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Queue `path` for the per-file commit query; requests are batched into @@ -588,7 +588,7 @@ impl RepoDetailView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Walk all commits reachable from HEAD on a background task, for the @@ -630,7 +630,7 @@ impl RepoDetailView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Open a new panel showing the diff of `commit_id` (all files it @@ -683,8 +683,15 @@ impl RepoDetailView { return; }; - let panel = cx - .new(|cx| PullRequestsView::new(self.store.clone(), self.display_name(cx), window, cx)); + let panel = cx.new(|cx| { + PullRequestsView::new( + self.dock_area.clone(), + self.store.clone(), + self.display_name(cx), + window, + cx, + ) + }); dock_area.update(cx, |dock_area, cx| { dock_area.add_panel(Arc::new(panel), DockPlacement::Center, window, cx); @@ -760,7 +767,7 @@ impl RepoDetailView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Restore a selector to `previous`, or clear it (after a failed switch). @@ -884,7 +891,7 @@ impl RepoDetailView { Ok(()) }); - track(&mut self.tasks, task); + self.tasks.push(task); } /// Drop the oldest previews beyond the cache caps, keeping the currently diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index f066eab..692b570 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -1,38 +1,35 @@ -//! Pull requests panel: a bottom panel listing every pull request of the -//! repository with its title, event id, author, age and status, filterable -//! by status via the header's All/Open/Closed/Draft/Merged filter. - use std::rc::Rc; use assets::CustomIconName; +use dock::{DockArea, Panel, PanelEvent}; use gpui::prelude::*; use gpui::{ AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, - SharedString, Size, Window, div, px, size, + SharedString, Size, WeakEntity, Window, div, px, relative, size, }; +use gpui_base::Button as BaseButton; use gpui_component::avatar::Avatar; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; -use dock::{Panel, PanelEvent}; use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputState, Textarea, TextareaState}; use gpui_component::scroll::Scrollbar; -use gpui_component::tooltip::Tooltip; use gpui_component::{ - ActiveTheme, Icon, IconName, Selectable, Sizable, StyledExt, VirtualListScrollHandle, - WindowExt, h_flex, v_flex, v_virtual_list, + ActiveTheme, Icon, Sizable, VirtualListScrollHandle, WindowExt, h_flex, v_flex, v_virtual_list, }; -use nostr::prelude::{Event, Kind}; +use nostr::prelude::{Event, EventId, Kind}; use signed_core::{RepoStatus, activity_subject}; use signed_state::{ProfileStore, RepoStore}; use utils::relative_time; -use super::helpers::placeholder; +use super::helpers::{placeholder, status_badge}; /// Height of one pull request row in the virtual list: same layout as an -/// issue row (12px padding on top and bottom, a 14px title line and a 24px -/// meta line), so the row totals ~71px. -const PR_ROW_HEIGHT: f32 = 71.; +/// issue row (8px vertical padding (`py_2`) on top and bottom, a 32px title +/// line (`h_8`) and a 24px meta line (`h_6`), plus the 1px bottom border), +/// so the row totals 73px. The status badge (`size_7`, 28px) is shorter +/// than the content. +const PR_ROW_HEIGHT: f32 = 73.; /// Status filter of the pull request list, chosen via the header's filter /// buttons. @@ -66,6 +63,8 @@ impl PullRequestFilter { pub struct PullRequestsView { focus_handle: FocusHandle, + /// Dock area the detail panels are added to. + dock_area: WeakEntity, /// Repo store holding the pull requests and their statuses. store: Entity, /// Display name of the repository, for the panel title. @@ -88,6 +87,7 @@ pub struct PullRequestsView { impl PullRequestsView { pub fn new( + dock_area: WeakEntity, store: Entity, repo_name: SharedString, window: &mut Window, @@ -99,6 +99,7 @@ impl PullRequestsView { Self { focus_handle: cx.focus_handle(), + dock_area, store, repo_name, filter: PullRequestFilter::Open, @@ -109,7 +110,23 @@ impl PullRequestsView { } } - fn render_row(&self, ix: usize, pr: &Event, cx: &App) -> AnyElement { + /// Open the detail panel of `pr_id` at the bottom of the dock area. + fn open_pull_request_detail( + &mut self, + pr_id: EventId, + window: &mut Window, + cx: &mut Context, + ) { + let Some(dock_area) = self.dock_area.upgrade() else { + return; + }; + // TODO + } + + /// Render one row of the pull request list; `ix` is the row index and + /// `pr_ix` the index of the pull request in the store's `pull_requests`. + fn render_row(&self, ix: usize, pr_ix: usize, cx: &mut Context) -> AnyElement { + let pr = &self.store.read(cx).pull_requests[pr_ix]; let title = activity_subject(pr); let id_hex = pr.id.to_hex(); let profile = ProfileStore::global(cx).read(cx).get(&pr.pubkey); @@ -117,22 +134,27 @@ impl PullRequestsView { let picture = profile.picture(); let age = relative_time(pr.created_at); let status = self.store.read(cx).status_of(pr); + let pr_id = pr.id; h_flex() .id(ix) - .h(px(PR_ROW_HEIGHT)) .w_full() .gap_4() - .p_3() + .px_4() + .py_2() .border_b_1() .border_color(cx.theme().border) .items_start() - .child(Self::render_status(status, cx)) + .on_click(cx.listener(move |this, _event, window, cx| { + this.open_pull_request_detail(pr_id, window, cx); + })) + .child(status_badge(status, cx)) .child( v_flex() .flex_1() .child( div() + .h_8() .min_w_0() .text_ellipsis() .whitespace_nowrap() @@ -142,6 +164,7 @@ impl PullRequestsView { ) .child( h_flex() + .h_6() .gap_2() .text_xs() .child( @@ -151,7 +174,7 @@ impl PullRequestsView { Avatar::new() .name(author.clone()) .when_some(picture, |this, url| this.src(url)) - .xsmall(), + .small(), ) .child(div().child(author)), ) @@ -161,126 +184,211 @@ impl PullRequestsView { .text_color(cx.theme().muted_foreground) .child(SharedString::from(&id_hex[..8])), ) - .child(div().child(age)), + .child(SharedString::from(age)), ), ) - .into_any_element() - } - - fn render_status(status: RepoStatus, cx: &App) -> AnyElement { - let (icon, label, tooltip, bg, fg) = match status { - RepoStatus::Open => ( - CustomIconName::GitPullRequest, - "open", - "Pull request is open", - cx.theme().secondary, - cx.theme().secondary_foreground, - ), - RepoStatus::Closed => ( - CustomIconName::GitPullRequestClosed, - "closed", - "Pull request is closed", - cx.theme().warning, - cx.theme().warning_foreground, - ), - RepoStatus::Draft => ( - CustomIconName::GitPullRequestDraft, - "draft", - "Pull request is a draft", - cx.theme().accent, - cx.theme().accent_foreground, - ), - RepoStatus::Applied => ( - CustomIconName::GitPullRequestMerged, - "merged", - "Pull request is merged", - cx.theme().primary, - cx.theme().primary_foreground, - ), - }; - - v_flex() - .id(label) - .flex_shrink_0() - .size_6() - .items_center() - .justify_center() - .rounded(cx.theme().radius) - .bg(bg) - .child(Icon::new(icon).xsmall().text_color(fg)) - .tooltip(move |window, cx| Tooltip::new(tooltip).build(window, cx)) + .hover(|this| this.bg(cx.theme().list_hover)) .into_any_element() } fn render_header(&self, cx: &mut Context) -> AnyElement { + let store = self.store.read(cx); + let (total, open, closed, draft, merged) = store.pull_requests.iter().fold( + (0usize, 0usize, 0usize, 0usize, 0usize), + |(total, open, closed, draft, merged), pr| match store.status_of(pr) { + RepoStatus::Open => (total + 1, open + 1, closed, draft, merged), + RepoStatus::Closed => (total + 1, open, closed + 1, draft, merged), + RepoStatus::Draft => (total + 1, open, closed, draft + 1, merged), + RepoStatus::Applied => (total + 1, open, closed, draft, merged + 1), + }, + ); + h_flex() + .px_4() .w_full() - .items_center() .gap_3() - .px_3() - .pb_2() .border_b_1() .border_color(cx.theme().border) - .child( - div() - .text_sm() - .text_color(cx.theme().muted_foreground) - .font_semibold() - .child("Pull Requests"), - ) + .bg(cx.theme().muted.opacity(0.5)) .child( h_flex() - .gap_1() + .h_12() + .gap_2() .child( - Button::new("all") - .icon(CustomIconName::GitPullRequest) - .label("All") - .ghost() + BaseButton::new("all") + .flex() + .items_center() + .h_7() + .px_2() + .gap_1() + .child(Icon::new(CustomIconName::GitPullRequest)) + .child(div().text_sm().child("All")) + .child( + h_flex() + .justify_center() + .ml_2() + .px_1() + .py_0p5() + .min_w_4() + .text_size(px(8.)) + .bg(cx.theme().muted) + .text_color(cx.theme().muted_foreground) + .rounded(cx.theme().radius) + .line_height(relative(1.)) + .child(SharedString::from(total.to_string())), + ) + .text_color(cx.theme().button_foreground) + .rounded(cx.theme().radius) + .hover(|this| this.bg(cx.theme().button_hover)) + .active(|this| this.bg(cx.theme().button_active)) .selected(self.filter == PullRequestFilter::All) + .when(self.filter == PullRequestFilter::All, |this| { + this.bg(cx.theme().button_active) + }) .on_click(cx.listener(|this, _event, _window, cx| { this.filter = PullRequestFilter::All; cx.notify(); })), ) .child( - Button::new("open") - .icon(CustomIconName::GitPullRequest) - .label("Open") - .ghost() + BaseButton::new("open") + .flex() + .items_center() + .h_7() + .px_2() + .gap_1() + .child(Icon::new(CustomIconName::GitPullRequest)) + .child(div().text_sm().child("Open")) + .child( + h_flex() + .justify_center() + .ml_2() + .px_1() + .py_0p5() + .min_w_4() + .text_size(px(8.)) + .bg(cx.theme().muted) + .text_color(cx.theme().muted_foreground) + .rounded(cx.theme().radius) + .line_height(relative(1.)) + .child(SharedString::from(open.to_string())), + ) + .text_color(cx.theme().button_foreground) + .rounded(cx.theme().radius) + .hover(|this| this.bg(cx.theme().button_hover)) + .active(|this| this.bg(cx.theme().button_active)) .selected(self.filter == PullRequestFilter::Open) + .when(self.filter == PullRequestFilter::Open, |this| { + this.bg(cx.theme().button_active) + }) .on_click(cx.listener(|this, _event, _window, cx| { this.filter = PullRequestFilter::Open; cx.notify(); })), ) .child( - Button::new("closed") - .icon(CustomIconName::GitPullRequestClosed) - .label("Closed") - .ghost() + BaseButton::new("closed") + .flex() + .items_center() + .h_7() + .px_2() + .gap_1() + .child(Icon::new(CustomIconName::GitPullRequestClosed)) + .child(div().text_sm().child("Closed")) + .child( + h_flex() + .justify_center() + .ml_2() + .px_1() + .py_0p5() + .min_w_4() + .text_size(px(8.)) + .bg(cx.theme().muted) + .text_color(cx.theme().muted_foreground) + .rounded(cx.theme().radius) + .line_height(relative(1.)) + .child(SharedString::from(closed.to_string())), + ) + .text_color(cx.theme().button_foreground) + .rounded(cx.theme().radius) + .hover(|this| this.bg(cx.theme().button_hover)) + .active(|this| this.bg(cx.theme().button_active)) .selected(self.filter == PullRequestFilter::Closed) + .when(self.filter == PullRequestFilter::Closed, |this| { + this.bg(cx.theme().button_active) + }) .on_click(cx.listener(|this, _event, _window, cx| { this.filter = PullRequestFilter::Closed; cx.notify(); })), ) .child( - Button::new("draft") - .icon(CustomIconName::GitPullRequestDraft) - .label("Draft") - .ghost() + BaseButton::new("draft") + .flex() + .items_center() + .h_7() + .px_2() + .gap_1() + .child(Icon::new(CustomIconName::GitPullRequestDraft)) + .child(div().text_sm().child("Draft")) + .child( + h_flex() + .justify_center() + .ml_2() + .px_1() + .py_0p5() + .min_w_4() + .text_size(px(8.)) + .bg(cx.theme().muted) + .text_color(cx.theme().muted_foreground) + .rounded(cx.theme().radius) + .line_height(relative(1.)) + .child(SharedString::from(draft.to_string())), + ) + .text_color(cx.theme().button_foreground) + .rounded(cx.theme().radius) + .hover(|this| this.bg(cx.theme().button_hover)) + .active(|this| this.bg(cx.theme().button_active)) .selected(self.filter == PullRequestFilter::Draft) + .when(self.filter == PullRequestFilter::Draft, |this| { + this.bg(cx.theme().button_active) + }) .on_click(cx.listener(|this, _event, _window, cx| { this.filter = PullRequestFilter::Draft; cx.notify(); })), ) .child( - Button::new("merged") - .icon(CustomIconName::GitPullRequestMerged) - .label("Merged") - .ghost() + BaseButton::new("merged") + .flex() + .items_center() + .h_7() + .px_2() + .gap_1() + .child(Icon::new(CustomIconName::GitPullRequestMerged)) + .child(div().text_sm().child("Merged")) + .child( + h_flex() + .justify_center() + .ml_2() + .px_1() + .py_0p5() + .min_w_4() + .text_size(px(8.)) + .bg(cx.theme().muted) + .text_color(cx.theme().muted_foreground) + .rounded(cx.theme().radius) + .line_height(relative(1.)) + .child(SharedString::from(merged.to_string())), + ) + .text_color(cx.theme().button_foreground) + .rounded(cx.theme().radius) + .hover(|this| this.bg(cx.theme().button_hover)) + .active(|this| this.bg(cx.theme().button_active)) .selected(self.filter == PullRequestFilter::Merged) + .when(self.filter == PullRequestFilter::Merged, |this| { + this.bg(cx.theme().button_active) + }) .on_click(cx.listener(|this, _event, _window, cx| { this.filter = PullRequestFilter::Merged; cx.notify(); @@ -290,10 +398,19 @@ impl PullRequestsView { // Spacer: pushes the button to the right edge. .child(div().flex_1()) .child( - Button::new("new-pr") - .icon(IconName::Plus) - .label("New pull request") - .primary() + BaseButton::new("new-pr") + .flex() + .items_center() + .h_7() + .px_2() + .gap_1() + .child(Icon::new(CustomIconName::CirclePlus)) + .child(div().text_sm().child("New pull request")) + .text_color(cx.theme().button_primary_foreground) + .rounded(cx.theme().radius) + .bg(cx.theme().button_primary) + .hover(|this| this.bg(cx.theme().button_primary_hover)) + .active(|this| this.bg(cx.theme().button_primary_active)) .on_click(cx.listener(|this, _event, window, cx| { open_new_pull_request_dialog(this.store.clone(), window, cx); })), @@ -433,11 +550,10 @@ impl Render for PullRequestsView { .when(count > 0, |this| { this.child( v_virtual_list(view, "prl", sizes, move |this, range, _window, cx| { - let prs = &this.store.read(cx).pull_requests; range .map(|ix| { let pr_ix = this.visible_prs[ix]; - this.render_row(pr_ix, &prs[pr_ix], cx) + this.render_row(ix, pr_ix, cx) }) .collect() })