From aaa7aa7ec88386269fa076917bd28294a57fa36d Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 08:06:49 +0700 Subject: [PATCH 1/9] update issue detail panel --- .../src/views/repo_detail/issue_detail.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/workspace/src/views/repo_detail/issue_detail.rs b/crates/workspace/src/views/repo_detail/issue_detail.rs index 43ba3fb..1cfe6bf 100644 --- a/crates/workspace/src/views/repo_detail/issue_detail.rs +++ b/crates/workspace/src/views/repo_detail/issue_detail.rs @@ -21,16 +21,13 @@ use utils::relative_time; /// Detail panel of a single issue. pub struct IssueDetailView { - focus_handle: FocusHandle, /// Repo store holding the issues and their statuses. store: Entity, issue_id: EventId, + contents: HashMap, /// Input state of the "leave a comment" textarea. comment_input: Entity, - /// Issue/comment bodies as shared strings, keyed by event ID, so - /// re-renders don't clone full contents again (events are immutable, - /// so the cache never needs invalidation). - contents: HashMap, + focus_handle: FocusHandle, } impl IssueDetailView { @@ -283,7 +280,13 @@ impl Render for IssueDetailView { let content = self .contents .entry(issue.id) - .or_insert_with(|| SharedString::from(issue.content.clone())) + .or_insert_with(|| { + if issue.content.is_empty() { + SharedString::from("No description provided.") + } else { + SharedString::from(&issue.content) + } + }) .clone(); ( @@ -338,8 +341,7 @@ impl Render for IssueDetailView { h_flex() .gap_1() .child( - UserAvatar::new(author.clone()) - .picture(picture), + UserAvatar::new(&author).picture(picture), ) .child(author), ) -- 2.54.0 From 6f1256757f8fe10c65332bb021f65f3d60ecdadc Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 08:45:40 +0700 Subject: [PATCH 2/9] add upstream and fork in ui --- crates/signed_core/src/model.rs | 103 ++++++++++- crates/workspace/src/views/mod.rs | 1 + .../workspace/src/views/repo_detail/about.rs | 12 +- crates/workspace/src/views/repo_detail/mod.rs | 150 +++++++++++++++- crates/workspace/src/views/repo_list.rs | 68 +++++--- .../src/views/sidebar/create_repo_dialog.rs | 14 +- crates/workspace/src/views/sidebar/mod.rs | 16 +- docs/PLAN.md | 163 ++++++++++++++++++ docs/TODO.md | 11 +- 9 files changed, 471 insertions(+), 67 deletions(-) create mode 100644 docs/PLAN.md diff --git a/crates/signed_core/src/model.rs b/crates/signed_core/src/model.rs index 2dd190c..f2a712f 100644 --- a/crates/signed_core/src/model.rs +++ b/crates/signed_core/src/model.rs @@ -3,6 +3,8 @@ use std::collections::HashSet; use gpui::SharedString; use nostr::prelude::*; +use crate::RepoAddr; + /// Parsed NIP-34 repository announcement (plain data, ready for the UI). #[derive(Debug, Clone, PartialEq, Eq)] pub struct Announcement { @@ -28,11 +30,53 @@ pub struct Announcement { pub maintainers: Vec, /// Value of a `u` tag, if any: this repository is a subordinate fork of /// the referenced upstream (NIP-34). - pub upstream: Option, + pub upstream: Option, /// Hashtags labelling the repository (`t` tags). pub hashtags: Vec, } +/// The `u` tag of a fork announcement (NIP-34): the repository this one is a +/// subordinate fork of. The first value is the upstream coordinate +/// (`30617::`) or a git URL; the second is an optional relay hint +/// for the upstream. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Upstream { + /// Raw first value of the `u` tag (coordinate or git URL). + pub raw: String, + /// The upstream `30617::` coordinate, when the `u` tag + /// references a NIP-34 repository; `None` for the git-URL form. + pub addr: Option, + /// Relay hint for the upstream, if the `u` tag carries one. + pub relay_hint: Option, +} + +impl Upstream { + /// Parse the `u` tag values. The first is the upstream coordinate or a + /// git URL (the coordinate form may append `|git-url`; the coordinate is + /// the part before the first `|`), the second an optional relay hint. + fn parse(raw: &str, relay_hint: Option<&str>) -> Self { + let coordinate = raw.split('|').next().unwrap_or(raw); + let addr = coordinate + .parse::() + .ok() + .filter(|c| c.kind == Kind::GitRepoAnnouncement); + Self { + raw: raw.to_owned(), + addr, + relay_hint: relay_hint.and_then(|hint| RelayUrl::parse(hint).ok()), + } + } + + /// Text for display: the upstream coordinate when it is a NIP-34 + /// repository, otherwise the raw `u` value (git-URL form). + pub fn display(&self) -> SharedString { + match &self.addr { + Some(addr) => SharedString::from(addr.to_string()), + None => SharedString::from(self.raw.clone()), + } + } +} + /// Subject of a NIP-34 issue or pull request event: the `subject` tag, /// falling back to the first non-empty line of the content. pub fn activity_subject(event: &Event) -> SharedString { @@ -193,7 +237,7 @@ impl Announcement { let mut relays: Vec = Vec::new(); let mut euc: Option = None; let mut maintainers: Vec = Vec::new(); - let mut upstream: Option = None; + let mut upstream: Option = None; for tag in event.tags.iter() { match Nip34Tag::parse(tag.as_slice()) { @@ -208,9 +252,13 @@ impl Announcement { } // The `u` tag is not modelled by the SDK's `Nip34Tag`; parse it - // manually (first value wins). + // manually (first wins). if upstream.is_none() && tag.kind() == "u" { - upstream = tag.content().map(str::to_owned); + let values = tag.as_slice(); + let raw = values.get(1).map(String::as_str).unwrap_or_default(); + if !raw.is_empty() { + upstream = Some(Upstream::parse(raw, values.get(2).map(String::as_str))); + } } } @@ -391,14 +439,55 @@ mod tests { fn parses_upstream_tag() { let event = announcement_event(&[ &["d", "my-fork"], - &["u", "30617:abc:upstream|https://example.com/upstream.git"], + &[ + "u", + "30617:68d81165918100b7da43fc28f7d1fc12554466e1115886b9e7bb326f65ec4272:upstream|https://example.com/upstream.git", + "wss://relay.example.com", + ], ]); let announcement = Announcement::from_event(&event).expect("parses"); + let upstream = announcement.upstream.expect("parses the u tag"); + // The coordinate part resolves to a repository address; the raw + // value keeps the `|git-url` suffix. assert_eq!( - announcement.upstream.as_deref(), - Some("30617:abc:upstream|https://example.com/upstream.git") + upstream.addr, + Some(crate::repo_addr( + PublicKey::from_hex(MAINTAINER_HEX).expect("valid pubkey"), + "upstream" + )) + ); + assert_eq!( + upstream.raw, + "30617:68d81165918100b7da43fc28f7d1fc12554466e1115886b9e7bb326f65ec4272:upstream|https://example.com/upstream.git" + ); + assert_eq!( + upstream.relay_hint, + Some(RelayUrl::parse("wss://relay.example.com").expect("valid relay")) + ); + assert_eq!( + upstream.display().to_string(), + "30617:68d81165918100b7da43fc28f7d1fc12554466e1115886b9e7bb326f65ec4272:upstream" + ); + } + + #[test] + fn parses_git_url_upstream() { + // The `u` tag may reference a non-nostr upstream by git URL only; + // there is no repository address to navigate to. + let event = announcement_event(&[ + &["d", "my-fork"], + &["u", "https://example.com/upstream.git"], + ]); + + let announcement = Announcement::from_event(&event).expect("parses"); + let upstream = announcement.upstream.expect("parses the u tag"); + + assert_eq!(upstream.addr, None); + assert_eq!( + upstream.display().to_string(), + "https://example.com/upstream.git" ); } diff --git a/crates/workspace/src/views/mod.rs b/crates/workspace/src/views/mod.rs index 9c95334..1ec23e7 100644 --- a/crates/workspace/src/views/mod.rs +++ b/crates/workspace/src/views/mod.rs @@ -3,5 +3,6 @@ mod repo_list; pub(crate) mod sidebar; pub use repo_detail::RepoDetailView; +pub(crate) use repo_detail::open_repo_panel; pub use repo_list::RepoListView; pub use sidebar::SidebarPanel; diff --git a/crates/workspace/src/views/repo_detail/about.rs b/crates/workspace/src/views/repo_detail/about.rs index 290954e..e0b9535 100644 --- a/crates/workspace/src/views/repo_detail/about.rs +++ b/crates/workspace/src/views/repo_detail/about.rs @@ -60,6 +60,7 @@ fn announcement_rows(announcement: &Announcement, cx: &App) -> AnyElement { cx, )); } + if let Some(euc) = &announcement.euc { rows.push(row( "Earliest Commit", @@ -67,13 +68,11 @@ fn announcement_rows(announcement: &Announcement, cx: &App) -> AnyElement { cx, )); } + if let Some(upstream) = &announcement.upstream { - rows.push(row( - "Upstream", - text(SharedString::from(upstream.clone())), - cx, - )); + rows.push(row("Upstream", text(upstream.display()), cx)); } + if !announcement.hashtags.is_empty() { rows.push(row( "Hashtags", @@ -81,6 +80,7 @@ fn announcement_rows(announcement: &Announcement, cx: &App) -> AnyElement { cx, )); } + if !announcement.clone.is_empty() { rows.push(row( "Clone URLs", @@ -92,6 +92,7 @@ fn announcement_rows(announcement: &Announcement, cx: &App) -> AnyElement { cx, )); } + if !announcement.relays.is_empty() { rows.push(row( "Grasp Relays", @@ -103,6 +104,7 @@ fn announcement_rows(announcement: &Announcement, cx: &App) -> AnyElement { cx, )); } + if !announcement.maintainers.is_empty() { rows.push(row( "Maintainers", diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index 16038c2..dfce1af 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -1,6 +1,7 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::path::{Component, Path, PathBuf}; use std::rc::Rc; +use std::time::Duration; use anyhow::Error; use assets::CustomIconName; @@ -26,9 +27,9 @@ use gpui_component::{ VirtualListScrollHandle, h_flex, v_flex, }; use nostr::prelude::{EventId, RelayUrl, ToBech32}; -use signed_core::Announcement; +use signed_core::{Announcement, RepoAddr, filters}; use signed_git::{CommitList, FileCommit}; -use signed_state::{Backend, GitStore, LocalReposStore, ProfileStore, RepoStore}; +use signed_state::{Backend, GitStore, LocalReposStore, ProfileStore, RepoListStore, RepoStore}; use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{DropdownButton, PixelAvatar, UserAvatar, copy_row}; @@ -188,6 +189,9 @@ pub struct RepoDetailView { tasks: Vec>>, /// Subscriptions keeping the selectors' confirm events alive. _subscriptions: Vec, + /// Upstream repository (from this fork's `u` tag) the user asked to + /// open, while its announcement is still being fetched. + pending_upstream: Option, } impl RepoDetailView { @@ -315,6 +319,7 @@ impl RepoDetailView { focus_handle: cx.focus_handle(), tasks: Vec::new(), _subscriptions: subscriptions, + pending_upstream: None, } } @@ -959,6 +964,73 @@ impl RepoDetailView { }); } + /// Open the upstream repository (the `u` tag of this fork's announcement). + /// When the upstream announcement is not in the local database yet, + /// subscribe for it and open the panel as soon as it lands. + fn open_upstream(&mut self, window: &mut Window, cx: &mut Context) { + if self.pending_upstream.is_some() { + return; + } + + let Some(announcement) = self.announcement(cx).cloned() else { + return; + }; + + let Some(addr) = announcement.upstream.and_then(|upstream| upstream.addr) else { + return; + }; + + if let Some(found) = RepoListStore::global(cx) + .read(cx) + .announcements + .iter() + .find(|a| a.addr() == addr) + .cloned() + { + open_repo_panel(&self.dock_area, &found, window, &mut *cx); + return; + } + + Backend::global(cx).update(cx, |backend, cx| { + backend.subscribe_bootstrap(vec![filters::announcement(&addr)], cx); + }); + self.pending_upstream = Some(addr); + + let task = cx.spawn_in(window, async move |this, cx| { + for _ in 0..60 { + cx.background_executor() + .timer(Duration::from_millis(250)) + .await; + let opened = this.update_in(cx, |this, window, cx| { + let Some(addr) = this.pending_upstream.clone() else { + return true; + }; + let found = RepoListStore::global(cx) + .read(cx) + .announcements + .iter() + .find(|a| a.addr() == addr) + .cloned(); + match found { + Some(found) => { + this.pending_upstream = None; + open_repo_panel(&this.dock_area, &found, window, &mut *cx); + true + } + None => false, + } + })?; + if opened { + return Ok(()); + } + } + this.update(cx, |this, _cx| this.pending_upstream = None)?; + Ok(()) + }); + + self.tasks.push(task); + } + /// Check out `name` (a branch or tag picked in the header) and refresh /// the explorer once the switch completes. fn switch_ref( @@ -1332,6 +1404,7 @@ impl RepoDetailView { .text_ellipsis() .child(description), ) + .when_some(fork_row(&announcement, cx), |this, row| this.child(row)) .child( h_flex() .mt_2() @@ -2015,3 +2088,76 @@ fn nostr_clone_url(announcement: &Announcement, nip05: Option<&str>) -> SharedSt SharedString::from(url) } + +/// The "Forked from …" row of the detail header: a clickable link to the +/// upstream repository when the `u` tag references a NIP-34 repo, +/// plain text when it only carries a git URL. +fn fork_row(announcement: &Announcement, cx: &mut Context) -> Option { + let upstream = announcement.upstream.as_ref()?; + + let (label, clickable) = match &upstream.addr { + Some(addr) => { + // Prefer the upstream's display name when its announcement + // is already known locally fall back to its repository id. + let name = RepoListStore::global(cx) + .read(cx) + .announcements + .iter() + .find(|a| a.addr() == *addr) + .map(|a| { + a.name + .clone() + .unwrap_or_else(|| SharedString::from(a.id.clone())) + }) + .unwrap_or_else(|| SharedString::from(addr.identifier.clone())); + (SharedString::from(format!("Forked from {name}")), true) + } + None => (upstream.display(), false), + }; + + let row = h_flex() + .gap_1() + .items_center() + .min_w_0() + .text_sm() + .text_color(cx.theme().muted_foreground) + .child(Icon::new(CustomIconName::GitBranch).small()) + .child(div().whitespace_nowrap().text_ellipsis().child(label)); + + Some(if clickable { + row.id("fork-upstream") + .cursor_pointer() + .hover(|this| this.text_color(cx.theme().foreground)) + .on_click(cx.listener(|this, _ev, window, cx| this.open_upstream(window, cx))) + .into_any_element() + } else { + row.into_any_element() + }) +} + +/// Open `announcement` as a repository panel in the dock's center, returning +/// the new detail view. Shared by the explore list, the sidebar and fork +/// links so every entry point opens repositories identically. +pub(crate) fn open_repo_panel( + dock_area: &WeakEntity, + announcement: &Announcement, + window: &mut Window, + cx: &mut App, +) -> Entity { + let detail = + cx.new(|cx| RepoDetailView::new(dock_area.clone(), announcement.clone(), window, cx)); + + if let Some(dock_area) = dock_area.upgrade() { + dock_area.update(cx, |dock_area, cx| { + dock_area.add_panel_view( + panel_handle(detail.clone()), + DockPlacement::Center, + None, + window, + cx, + ); + }); + } + + detail +} diff --git a/crates/workspace/src/views/repo_list.rs b/crates/workspace/src/views/repo_list.rs index 0582f59..7008656 100644 --- a/crates/workspace/src/views/repo_list.rs +++ b/crates/workspace/src/views/repo_list.rs @@ -1,7 +1,7 @@ use std::rc::Rc; use assets::CustomIconName; -use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; +use dock::{BasePanel, DockArea, Panel, PanelEvent}; use gpui::prelude::*; use gpui::{ AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, @@ -19,7 +19,7 @@ use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{SegmentButton, UserAvatar}; use utils::relative_time; -use super::RepoDetailView; +use super::open_repo_panel; const COLUMNS: usize = 2; const CARD_HEIGHT: f32 = 40. + 64. + 48. + 2. + 6.; @@ -173,21 +173,7 @@ impl RepoListView { window: &mut Window, cx: &mut Context, ) { - let dock_area = self.dock_area.clone(); - let detail = - cx.new(|cx| RepoDetailView::new(dock_area.clone(), announcement.clone(), window, cx)); - - if let Some(dock_area) = dock_area.upgrade() { - dock_area.update(cx, |dock_area, cx| { - dock_area.add_panel_view( - panel_handle(detail), - DockPlacement::Center, - None, - window, - cx, - ); - }); - } + open_repo_panel(&self.dock_area, announcement, window, &mut *cx); } fn render_card( @@ -215,6 +201,26 @@ impl RepoListView { .map(|label| SharedString::from(format!("Updated {label}"))) .unwrap_or_default(); + // Fork badge: the upstream's display name when its announcement is + // known locally, otherwise its repository id from the `u` tag. + let fork_label: Option = + announcement.upstream.as_ref().and_then(|upstream| { + let addr = upstream.addr.as_ref()?; + let name = self + .store + .read(cx) + .announcements + .iter() + .find(|a| a.addr() == *addr) + .map(|a| { + a.name + .clone() + .unwrap_or_else(|| SharedString::from(a.id.clone())) + }) + .unwrap_or_else(|| SharedString::from(addr.identifier.clone())); + Some(SharedString::from(format!("forked from {name}"))) + }); + v_flex() .id(ix) .flex_1() @@ -228,11 +234,29 @@ impl RepoListView { .child( h_flex() .h_10() - .text_sm() - .font_semibold() - .whitespace_nowrap() - .text_ellipsis() - .child(name), + .gap_1p5() + .items_center() + .child( + div() + .min_w_0() + .text_sm() + .font_semibold() + .whitespace_nowrap() + .text_ellipsis() + .child(name), + ) + .when_some(fork_label, |this, label| { + this.child( + h_flex() + .gap_1() + .items_center() + .text_xs() + .text_color(cx.theme().muted_foreground) + .whitespace_nowrap() + .child(Icon::new(CustomIconName::GitBranch).small()) + .child(label), + ) + }), ) .child( div() diff --git a/crates/workspace/src/views/sidebar/create_repo_dialog.rs b/crates/workspace/src/views/sidebar/create_repo_dialog.rs index 21eb909..bad974f 100644 --- a/crates/workspace/src/views/sidebar/create_repo_dialog.rs +++ b/crates/workspace/src/views/sidebar/create_repo_dialog.rs @@ -1,4 +1,4 @@ -use dock::{DockArea, DockPlacement, panel_handle}; +use dock::DockArea; use gpui::prelude::*; use gpui::{App, Entity, PathPromptOptions, SharedString, WeakEntity, Window, div, px}; use gpui_base::input::TextareaState; @@ -11,7 +11,7 @@ use settings::SettingsStore; use signed_core::Announcement; use signed_state::Backend; -use super::super::RepoDetailView; +use super::super::open_repo_panel; use super::grasp_servers::{GraspServersState, grasp_servers_field, load_user_grasp_servers}; /// Shared state for the Create Repository dialog, so async results can be rendered. @@ -262,13 +262,5 @@ fn open_repo( window: &mut Window, cx: &mut App, ) { - let Some(dock_area) = dock_area.upgrade() else { - return; - }; - - let panel = cx.new(|cx| RepoDetailView::new(dock_area.downgrade(), announcement, window, cx)); - - dock_area.update(cx, |dock_area, cx| { - dock_area.add_panel_view(panel_handle(panel), DockPlacement::Center, None, window, cx); - }); + open_repo_panel(&dock_area, &announcement, window, cx); } diff --git a/crates/workspace/src/views/sidebar/mod.rs b/crates/workspace/src/views/sidebar/mod.rs index dbe6311..5d29f7e 100644 --- a/crates/workspace/src/views/sidebar/mod.rs +++ b/crates/workspace/src/views/sidebar/mod.rs @@ -19,7 +19,7 @@ use signed_state::{Backend, BackendEvent, LocalReposStore, Profile, ProfileStore use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{NavItem, PixelAvatar, UserAvatar, title_bar_drag_handlers}; -use super::{RepoDetailView, RepoListView}; +use super::{RepoDetailView, RepoListView, open_repo_panel}; mod create_repo_dialog; pub(crate) mod grasp_servers; @@ -158,19 +158,7 @@ impl SidebarPanel { window: &mut Window, cx: &mut Context, ) { - let detail = cx.new(|cx| { - RepoDetailView::new(self.dock_area.clone(), announcement.clone(), window, cx) - }); - - let _ = self.dock_area.update(cx, |dock_area, cx| { - dock_area.add_panel_view( - panel_handle(detail), - DockPlacement::Center, - None, - window, - cx, - ); - }); + open_repo_panel(&self.dock_area, announcement, window, &mut *cx); } /// Open a local repository's detail view in the dock's center; the diff --git a/docs/PLAN.md b/docs/PLAN.md new file mode 100644 index 0000000..9409642 --- /dev/null +++ b/docs/PLAN.md @@ -0,0 +1,163 @@ +# Plan + +Two work streams: + +1. **Fork support (display + navigation UI)** — show when a repository is a fork and let the user jump to the upstream repository. +2. **Pull request improvement** — bring PR creation/updating in line with the other NIP-34 clients (nak, ngit). + +--- + +## 1. Fork support + +### Background: what NIP-34 says about forks + +NIP-34 has no fork event kind — a fork is an ordinary kind-30617 announcement by another author (or the same author under a different `d`). Fork-ness is expressed by two tags: + +- **`u` tag** on the fork's announcement: + `["u", "30617::|", "", ""]`. + Including `u` means the author does **not** assert maintainership of the primary project (the fork is a *subordinate* of the upstream). +- **EUC** (`r` tag with `euc` marker): shared between the fork and its upstream (and other mirrors), so clients can group them. For a permanent fork, the EUC is the first commit after the fork point. + +### Current state + +- `Announcement::from_event` parses the `u` tag into an opaque string (`crates/signed_core/src/model.rs:212`; only the first value is kept, manually, because the SDK's `Nip34Tag` has no `Upstream` variant). +- `effective_maintainers` excludes the fork author (`model.rs:250`) — already correct per NIP-34. +- The About dialog shows the raw upstream string as a plain row (`crates/workspace/src/views/repo_detail/about.rs:68`). +- Nothing shows fork-ness in the repo list or the repo detail header, and there is no way to navigate to the upstream. + +### Goal + +- **Repo list card** (`crates/workspace/src/views/repo_list.rs::render_card`): show a "Forked from " badge instead of/in addition to the description, with a fork icon. +- **Repo detail header** (`crates/workspace/src/views/repo_detail/mod.rs::render_header`): show a "Forked from " text button near the repo name. +- **Clicking the upstream** opens the upstream repository as a center panel (same as clicking any repo card). + +### Design + +#### 1.1 Structured `Upstream` model (`signed_core`) + +Add a structured type and keep the manual parse: + +```rust +pub struct Upstream { + /// `30617::` (navigable) or a git URL (not navigable). + pub target: UpstreamTarget, + pub relay_hint: Option, + pub author: Option, +} + +pub enum UpstreamTarget { + /// Parseable via the SDK `Coordinate` (`30617::`). + Repo(RepoAddr), + /// Git https URL form: no NIP-34 announcement, not navigable. + GitUrl(Url), +} +``` + +- Change `Announcement.upstream: Option` to `Option`; parse all three `u` values (the SDK's `Nip34Tag::parse` is not usable here — keep the manual `tag.kind() == "u"` branch and extend it). +- `RepoAddr` is the SDK `Coordinate` (`crates/signed_core/src/addr.rs`), so `Coordinate::from_str` gives the upstream address directly; validate it is kind `30617`. +- Update `about.rs` (renders `upstream`), `effective_maintainers`, and the `model.rs` tests (`parses_upstream_tag`, `effective_maintainers_exclude_owner_for_subordinate_forks`). + +#### 1.2 Resolving the upstream announcement + +Opening a panel needs an `Announcement` (`RepoDetailView::new`), so resolve the upstream announcement before (or while) opening: + +1. **Lookup, no fetch**: the global `RepoListStore` holds every announcement in the local database (`crates/signed_state/src/repo_list.rs:51`). Look up the upstream `RepoAddr` there — covers the common case (upstream already browsed/known) with zero network. +2. **Miss → fetch, then open**: add a `Backend` method (e.g. `fetch_announcement(addr) -> Task>`) doing a one-shot query with `filters::announcement(addr)` (`crates/signed_core/src/filters.rs:21`), mirroring the bootstrap fetch in `RepoStore::subscribe_remote` (`crates/signed_state/src/repo.rs:196`). Show the upstream as a disabled/loading row until it resolves; on failure fall back to showing the raw address. +3. **Git-URL upstreams**: not navigable — render as plain text with a copy action (like `copy_row` in `signed_ui`), no panel. + +#### 1.3 Shared "open repo panel" helper + +The open-panel sequence is currently duplicated three times: + +- `crates/workspace/src/views/repo_list.rs:170` (`RepoListView::open_repo`) +- `crates/workspace/src/views/sidebar/mod.rs:155` (`SidebarPanel::open_repo`) +- `crates/workspace/src/views/sidebar/create_repo_dialog.rs:259` (`open_repo`) + +Extract one helper (e.g. `open_repo_panel(dock_area, announcement, window, cx)` in the `workspace` views layer) and reuse it from all three plus the new fork button, so the fork navigation behaves exactly like clicking a repo card. + +#### 1.4 Repo list badge + +In `render_card`, when `announcement.upstream` is set: + +- Resolve the upstream's display name via the `RepoListStore` lookup (1.2); fall back to the raw address string. +- Render a small "Forked from " line (fork icon + `text_xs` muted), replacing or joining the description line. Add a `git-fork.svg` asset to `crates/assets/assets/icons/` + a `CustomIconName::GitFork` variant (lucide's `git-fork`), or reuse `git-branch.svg` if an asset addition is undesirable. + +#### 1.5 Repo detail header + +In `render_header` (`crates/workspace/src/views/repo_detail/mod.rs:1231`), next to the repo name: + +- "Forked from " as a **text button** (`gpui_base::Button` or styled `div`), which calls the shared open-panel helper with the resolved upstream announcement. +- Keep the About dialog row in sync: make it the same clickable control (or at least the same resolved display name). +- Handle "upstream not in store yet": spawn the `fetch_announcement` task; button shows a subtle loading state; on success open the panel (needs `window`/`cx` — the task is spawned on the view, `apply_announcement`-style flow). + +#### 1.6 (Stretch) Fork grouping by EUC + +`RepoListStore` already has `euc` per announcement; add a "N forks" count on the detail header by scanning announcements sharing the same EUC, with a filter or navigation into the explore list. Not required for the first iteration. + +### Checklist + +- [ ] `signed_core`: `Upstream`/`UpstreamTarget` types + full `u`-tag parse; `Announcement.upstream` type change; tests. +- [ ] `Backend::fetch_announcement(addr)` one-shot fetch. +- [ ] Shared `open_repo_panel` helper; switch the three existing call sites. +- [ ] Repo list card fork badge (+ `git-fork.svg` asset if used). +- [ ] Repo detail header "Forked from" button + About row sync. +- [ ] Manual test: fork with coordinate upstream (navigates), fork with git-URL upstream (copy only), upstream announcement absent (fetch-then-open). + +--- + +## 2. Pull request improvement + +### Why + +Current PR creation (`RepoStore::open_pull_request`, `crates/signed_state/src/repo.rs:626`; dialog `crates/workspace/src/views/repo_detail/pull_requests.rs:288`) requires pasting `git format-patch` output, publishes no `merge-base`/`branch-name`, cannot update an existing PR, and advertises clone URLs the author usually cannot push to. Compared with nak (`pr send`/`pr update`/`pr merge`) and ngit (push-based PRs with merge-base inference), the gaps are: + +| Area | Today | Fix (phase) | +| --- | --- | --- | +| Patch generation | manual paste | generate from a local checkout (P2) | +| `merge-base` tag | always `None` | compute vs state HEAD (P1) | +| `branch-name` tag | never | send local branch name (P1) | +| PR updates (kind 1619) | not producible | author-only update flow (P1) | +| Update reader trusts any author | `latest_update` has no author filter | filter by PR author (P1) | +| `clone` URL truthfulness | repo mirrors (author can't push) | push tip to grasp first (P3) | +| Multi-commit series | one oversized event | NIP-10 chain / size-aware (P3) | +| Pre-publish validation | none | `git am --check` dry run (P2) | +| Draft on create | no | optional 1633 status (P1) | +| Merge provenance | plain 1631 | `merge-commit`/`applied-as-commits` (P4) | + +### Phase 1 — Correctness & interop (small, surgical) + +1. **Compute and publish `merge-base`, `branch-name`, `r` EUC** in `open_pull_request`: + - Target tip = `RepoStore.head` ref from the state announcement (`refs`/`head`, `crates/signed_state/src/repo.rs:28-30`); add `signed_git::merge_base(repo, a, b)` (shell out like `apply_patch`). + - Fill the `GitPullRequest` builder's existing `merge_base`/`branch_name` fields (currently hardcoded `None`, `repo.rs:691-702`); pass the branch name and tip through from the dialog. + - Add the `r` EUC tag manually to the PR event (the SDK builder omits it; NIP-34 recommends it for subscription efficiency). +2. **Add `RepoStore::update_pull_request(root, new_tip, …)`** producing a kind-1619 event via the SDK `GitPullRequestUpdate` builder (`E`/`P`/`K` NIP-22 tags) plus a chained root-revision patch (`t root-revision`, `e` reply to the original root patch). Author-only, mirroring nak's `pr update`. Wire a button into `pull_request_detail.rs`. +3. **Fix `latest_update`** (`crates/workspace/src/views/repo_detail/pull_request_detail.rs:1125`): filter by the root PR's author (nak and ngit both restrict tip updates to the PR author). +4. **Draft toggle** in the new-PR dialog: publish a 1633 status right after the PR event (reuse `set_status`). + +### Phase 2 — UX: replace the paste + +5. **Local-repo picker** replaces the paste textarea (keep it as an advanced fallback): user picks a git checkout (or the app's `GitCache` mirror), source branch and target branch. The app then: + - resolves the tip (`git rev-parse`), + - computes `merge-base` vs the target tip, + - runs `format-patch base..tip --stdout` itself (add `signed_git::format_patch_between`, like nak), + - **dry-runs `git am --3way --check`** against the cached clone before publishing (`signed_git::apply_patch` infra, `crates/signed_git/src/lib.rs:176`), surfacing "does not apply" before anything hits the relays. + +### Phase 3 — Truthful clone URLs (interop) + +6. **Push before publishing**: add `signed_git::push_commit_ref(path, url, commit, ref)` and reuse the grasp-push infrastructure (`grasp_base_url`, `push_to_grasp_servers`, `crates/signed_state/src/backend.rs:1472,1500`) to push the tip to `refs/nostr/` on the announced grasp servers (nak's `gitPushCommitToGraspRefs`). On success the `clone` tag carries the real URL; on failure fall back to the current patch-event model with a warning. +7. **Size-aware publishing**: split multi-commit mboxes into a NIP-10-chained 1617 series (each < 60 KB per NIP-34) or go PR-only above that size; adopt ngit's patch→PR upgrade (new PR + close-status for the original patch). +8. Optional: GRASP-06 `/prs//.git` + kind-10317 user grasp-list fallback (ngit's server-selection cascade). Fork support (section 1) makes the fork's own grasp server a natural push target here. + +### Phase 4 — Merge provenance + +9. In `merge_pull_request` (`crates/signed_state/src/repo.rs:817`), publish the 1631 status with `merge-commit` (or `applied-as-commits`) and `q` tags so nak/ngit/GitWorkshop show merge provenance correctly. + +### Checklist + +- [ ] P1: merge-base + branch-name + `r` EUC on creation. +- [ ] P1: `update_pull_request` (1619) + UI button; author check. +- [ ] P1: `latest_update` author filter. +- [ ] P1: draft toggle on create. +- [ ] P2: local checkout picker + generated patch + pre-publish apply check. +- [ ] P3: push tip to grasp, truthful `clone` tags, size-aware series. +- [ ] P4: merge status tags. diff --git a/docs/TODO.md b/docs/TODO.md index 6f64fa2..695a787 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -1,12 +1,11 @@ # TODO -## Local repository scan +## Fork support -- [ ] Make the scanned directories configurable (currently fixed to Desktop and Documents). - -## Create repository dialog - -- [ ] Remember the folder picked in the create-repository dialog and default to it next time (currently defaults to Desktop). +- [ ] Add UI for fork (see `PLAN.md` section 1): + - [ ] Fork badge on repo list cards (`repo_list.rs::render_card`). + - [ ] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. + - [ ] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). ## Performance: render path -- 2.54.0 From c054f61593d58d853bd993073565838a97d3dbec Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 09:14:04 +0700 Subject: [PATCH 3/9] wip --- crates/signed_core/src/lib.rs | 2 +- crates/signed_state/src/profile.rs | 9 +- crates/signed_state/src/repo.rs | 168 ++++++++++++++++-- crates/signed_state/src/repo_list.rs | 3 +- crates/workspace/src/views/repo_detail/mod.rs | 6 +- .../views/repo_detail/pull_request_detail.rs | 141 ++++++++++++++- .../src/views/repo_detail/pull_requests.rs | 53 +++++- crates/workspace/src/views/sidebar/mod.rs | 3 +- docs/PLAN.md | 12 +- docs/TODO.md | 17 +- 10 files changed, 373 insertions(+), 41 deletions(-) diff --git a/crates/signed_core/src/lib.rs b/crates/signed_core/src/lib.rs index b784804..5948d4d 100644 --- a/crates/signed_core/src/lib.rs +++ b/crates/signed_core/src/lib.rs @@ -13,6 +13,6 @@ pub use annotations::{COVER_NOTE_KIND, cover_note, labels_and_subject, subject_o pub use clone_url::{CloneTarget, parse_clone_url}; pub use comments::{CommentThread, comment_threads}; pub use deletions::Deletions; -pub use model::{Announcement, activity_subject, pull_request_patch}; +pub use model::{Announcement, activity_subject, pull_request_patch, pull_request_patches}; pub use state::{build_state, parse_state}; pub use status::{RepoStatus, references_root, resolve_status}; diff --git a/crates/signed_state/src/profile.rs b/crates/signed_state/src/profile.rs index 00fa896..22624f9 100644 --- a/crates/signed_state/src/profile.rs +++ b/crates/signed_state/src/profile.rs @@ -163,7 +163,8 @@ impl ProfileStore { /// Load recently seen profiles from the local database. fn load(&mut self, cx: &mut Context) { - let client = Backend::global(cx).read(cx).client(); + let backend = Backend::global(cx); + let client = backend.read(cx).client(); let work = cx.background_spawn(async move { let filter = Filter::new().kind(Kind::Metadata).limit(200); @@ -197,7 +198,8 @@ impl ProfileStore { /// Re-read the latest metadata of an author from the local database. fn apply_author(&mut self, public_key: PublicKey, cx: &mut Context) { - let client = Backend::global(cx).read(cx).client(); + let backend = Backend::global(cx); + let client = backend.read(cx).client(); let work = cx.background_spawn(async move { let filter = Filter::new().kind(Kind::Metadata).author(public_key); @@ -238,7 +240,8 @@ impl ProfileStore { return; } - let client = Backend::global(cx).read(cx).client(); + let backend = Backend::global(cx); + let client = backend.read(cx).client(); let work = cx.background_spawn(async move { let filter = Filter::new().kind(Kind::Metadata).authors(authors); diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index ec4a156..a5a154c 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -3,12 +3,14 @@ use std::collections::{HashMap, HashSet}; use std::time::Duration; use anyhow::Error; +use bitcoin_hashes::sha1::Hash as Sha1Hash; use gpui::{AppContext, Context, Subscription, Task}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; use signed_core::{ Announcement, COVER_NOTE_KIND, Deletions, RepoAddr, RepoStatus, build_state, cover_note, - filters, labels_and_subject, parse_state, pull_request_patch, subject_override, + filters, labels_and_subject, parse_state, pull_request_patch, pull_request_patches, + subject_override, }; use crate::backend::{Backend, BackendEvent}; @@ -235,7 +237,8 @@ impl RepoStore { fn run_refresh(&mut self, cx: &mut Context) { self.refreshing = true; - let client = Backend::global(cx).read(cx).client(); + let backend = Backend::global(cx); + let client = backend.read(cx).client(); let addr = self.addr.clone(); let work = cx.background_spawn(async move { @@ -623,17 +626,21 @@ impl RepoStore { /// carry a real commit id for other NIP-34 clients to verify and apply /// the proposal. The `clone` tag carries the announced mirror URLs; the /// linked patch is the source of truth until the commit is pushed there. + /// `branch_name` lands in the PR's `branch-name` tag (NIP-34); `draft` + /// publishes a kind-1633 status right after the PR event. pub fn open_pull_request( &mut self, subject: Option, description: String, + branch_name: Option, patch: String, + draft: bool, cx: &mut Context, ) { self.last_error = None; let Some(current_commit) = - patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) + patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -666,8 +673,8 @@ impl RepoStore { } let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags(patch_tags); - let patch_task = - Backend::global(cx).update(cx, |backend, cx| backend.send(patch_builder, cx)); + let backend = Backend::global(cx); + let patch_task = backend.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 { @@ -688,7 +695,7 @@ impl RepoStore { content: description, subject, labels: Vec::new(), - branch_name: None, + branch_name, // NIP-34: PRs carry at least one clone URL where the // tip commit can be downloaded; use the repository's // announced mirrors until a push backend exists. @@ -703,10 +710,146 @@ impl RepoStore { } .into_event_builder(); - Backend::global(cx).update(cx, |backend, cx| backend.send(builder, cx)) + // NIP-34: the `r` EUC tag lets clients subscribe to all PRs of this repository. + let builder = match this.announcement.as_ref().and_then(|a| a.euc.clone()) { + Some(euc) => builder.tag(Tag::parse(["r", &euc]).expect("valid r tag")), + None => builder, + }; + + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| backend.send(builder, cx)) })?; - if let Err(e) = pr_task.await { + let pr_event = match pr_task.await { + Ok(event) => event, + Err(e) => { + return this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + }); + } + }; + + // NIP-34: a draft PR carries a kind-1633 status event; publish + // it right after the PR event so viewers never show it open. + if draft { + this.update(cx, |this, cx| { + this.set_status(&pr_event, RepoStatus::Draft, cx); + })?; + } + + Ok(()) + })); + } + + /// Update a pull request: publish a revision patch event chained to the + /// original root patch (`t root-revision` and a NIP-10 `e` reply, per + /// NIP-34), then a kind-1619 PR update event carrying the new tip. + /// + /// Only the PR author may update it; other authors must open a new PR. + pub fn update_pull_request(&mut self, root: &Event, patch: String, cx: &mut Context) { + self.last_error = None; + + let backend = Backend::global(cx); + + let Some(user) = backend.read(cx).current_user() else { + self.last_error = Some("Sign in to update the pull request".into()); + cx.notify(); + return; + }; + + if user != root.pubkey { + self.last_error = Some("Only the pull request author can update it".into()); + cx.notify(); + return; + } + + let Some(current_commit) = + patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) + else { + self.last_error = Some( + "Patch must be `git format-patch` output with a `From ` header".into(), + ); + cx.notify(); + return; + }; + + // NIP-34: the first patch of a revision replies to the original + // root patch (the PR's `e` tag; fall back to the oldest patch of + // the linked set for PRs without one). + let root_patch_id = root.tags.event_ids().next().or_else(|| { + pull_request_patches(root, self.patches.iter()) + .first() + .map(|p| p.id) + }); + + let commit_hex = current_commit.to_string(); + let mut patch_tags = vec![ + Tag::coordinate(self.addr.clone(), None), + Tag::public_key(self.addr.public_key), + Tag::parse(["t", "root-revision"]).expect("valid root-revision tag"), + ]; + if let Some(root_patch_id) = root_patch_id + && let Ok(tag) = Tag::parse(["e", &root_patch_id.to_hex(), "", "reply"]) + { + patch_tags.push(tag); + } + // NIP-34: the `r` EUC tag lets clients subscribe to all patches of + // this repository; `commit`/`r` tags reference the new tip. + if let Some(euc) = self.announcement.as_ref().and_then(|a| a.euc.clone()) + && let Ok(tag) = Tag::parse(["r", &euc]) + { + patch_tags.push(tag); + } + if let Ok(tag) = Tag::parse(["commit", &commit_hex]) { + patch_tags.push(tag); + } + if let Ok(tag) = Tag::parse(["r", &commit_hex]) { + patch_tags.push(tag); + } + let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags(patch_tags); + + let backend = Backend::global(cx); + let patch_task = backend.update(cx, |backend, cx| backend.send(patch_builder, cx)); + + let root = root.clone(); + let clone: Vec = self + .announcement + .as_ref() + .map(|a| a.clone.clone()) + .unwrap_or_default(); + let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); + + self.tasks.push(cx.spawn(async move |this, cx| { + if let Err(e) = patch_task.await { + return this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + }); + } + + let update_task = this.update(cx, |this, cx| { + let builder = GitPullRequestUpdate { + repository: this.addr.clone(), + pull_request_event: root.id, + pull_request_author: root.pubkey, + current_commit, + clone: clone.clone(), + merge_base: None, + } + .into_event_builder(); + + // NIP-34: the `r` EUC tag lets clients subscribe to all PR + // updates of this repository; the SDK builder omits it. + let builder = match euc.as_deref() { + Some(euc) => builder.tag(Tag::parse(["r", euc]).expect("valid r tag")), + None => builder, + }; + + backend.update(cx, |backend, cx| backend.send(builder, cx)) + })?; + + if let Err(e) = update_task.await { return this.update(cx, |this, cx| { this.last_error = Some(e.to_string()); cx.notify(); @@ -729,7 +872,8 @@ impl RepoStore { .map(Announcement::effective_maintainers) .unwrap_or_default(); - let Some(user) = Backend::global(cx).read(cx).current_user() else { + let backend = Backend::global(cx); + let Some(user) = backend.read(cx).current_user() else { self.last_error = Some("Sign in to change the status".into()); cx.notify(); return; @@ -761,7 +905,8 @@ impl RepoStore { pub fn publish_state(&mut self, cx: &mut Context) { self.last_error = None; - let Some(user) = Backend::global(cx).read(cx).current_user() else { + let backend = Backend::global(cx); + let Some(user) = backend.read(cx).current_user() else { self.last_error = Some("Sign in to publish repository state".into()); cx.notify(); return; @@ -924,7 +1069,8 @@ fn resolve_statuses( .chain(pull_requests) .map(|root| { let events = by_root.get(&root.id).map(Vec::as_slice).unwrap_or(&[]); - let status = signed_core::resolve_status(events.iter().copied(), &root.pubkey, maintainers); + let status = + signed_core::resolve_status(events.iter().copied(), &root.pubkey, maintainers); (root.id, status) }) .collect() diff --git a/crates/signed_state/src/repo_list.rs b/crates/signed_state/src/repo_list.rs index 480e679..4b31623 100644 --- a/crates/signed_state/src/repo_list.rs +++ b/crates/signed_state/src/repo_list.rs @@ -202,7 +202,8 @@ impl RepoListStore { fn run_refresh(&mut self, cx: &mut Context) { self.refreshing = true; - let client = Backend::global(cx).read(cx).client(); + let backend = Backend::global(cx); + let client = backend.read(cx).client(); let author = self.author; let work = cx.background_spawn(async move { diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index dfce1af..6550103 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -991,7 +991,8 @@ impl RepoDetailView { return; } - Backend::global(cx).update(cx, |backend, cx| { + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| { backend.subscribe_bootstrap(vec![filters::announcement(&addr)], cx); }); self.pending_upstream = Some(addr); @@ -1001,6 +1002,7 @@ impl RepoDetailView { cx.background_executor() .timer(Duration::from_millis(250)) .await; + let opened = this.update_in(cx, |this, window, cx| { let Some(addr) = this.pending_upstream.clone() else { return true; @@ -1020,10 +1022,12 @@ impl RepoDetailView { None => false, } })?; + if opened { return Ok(()); } } + this.update(cx, |this, _cx| this.pending_upstream = None)?; Ok(()) }); diff --git a/crates/workspace/src/views/repo_detail/pull_request_detail.rs b/crates/workspace/src/views/repo_detail/pull_request_detail.rs index ef52da5..927790f 100644 --- a/crates/workspace/src/views/repo_detail/pull_request_detail.rs +++ b/crates/workspace/src/views/repo_detail/pull_request_detail.rs @@ -12,6 +12,8 @@ use gpui::{ }; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::clipboard::Clipboard; +use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; +use gpui_component::form::{field, v_form}; use gpui_component::input::{Textarea, TextareaState}; use gpui_component::list::ListItem; use gpui_component::scroll::{ScrollableElement, Scrollbar}; @@ -20,12 +22,13 @@ use gpui_component::tab::{Tab, TabBar}; use gpui_component::tag::Tag; use gpui_component::tree::{TreeEntry, TreeState, tree}; use gpui_component::{ - ActiveTheme, Icon, Sizable, StyledExt, VirtualListScrollHandle, h_flex, v_flex, v_virtual_list, + ActiveTheme, Icon, Sizable, StyledExt, VirtualListScrollHandle, WindowExt, h_flex, v_flex, + v_virtual_list, }; use nostr::prelude::{Event, EventId, Kind, Nip34Tag, PublicKey}; 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 signed_state::{Backend, GitStore, ProfileStore, RepoStore}; use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{UserAvatar, placeholder, status_badge, tree_row}; use utils::{relative_time, relative_time_secs}; @@ -183,7 +186,7 @@ impl PullRequestDetailView { cx.notify(); return; }; - let update = latest_update(store.pull_requests.iter(), &root.id); + let update = latest_update(store.pull_requests.iter(), root); let tip = update .and_then(current_commit_of) .or_else(|| current_commit_of(root)); @@ -1002,7 +1005,7 @@ impl PullRequestDetailView { /// Always-visible header: status badge and title, like the issue panel. fn render_header(&self, cx: &mut Context) -> AnyElement { let current_commit = self.current_commit.clone(); - let (title, status, branch) = { + let (title, status, branch, author) = { let store = self.store.read(cx); let Some(root) = store .pull_requests @@ -1015,9 +1018,14 @@ impl PullRequestDetailView { activity_subject(root), store.status_of(root), branch_name_of(root), + root.pubkey, ) }; + // Only the PR author may publish revisions (kind 1619, NIP-34). + let backend = Backend::global(cx); + let can_update = backend.read(cx).current_user() == Some(author); + v_flex() .px_4() .mb_4() @@ -1048,6 +1056,38 @@ impl PullRequestDetailView { .label(branch), ) }) + .when(can_update, |this| { + this.child( + Button::new("update-pr") + .ghost() + .small() + .icon(CustomIconName::GitPullRequest) + .label("Update") + .tooltip("Publish a new revision of this pull request") + .on_click(cx.listener({ + let store = self.store.clone(); + let pr_id = self.pr_id; + move |_this, _event, window, cx| { + let root = store + .read(cx) + .pull_requests + .iter() + .find(|pr| { + pr.id == pr_id && pr.kind == Kind::GitPullRequest + }) + .cloned(); + if let Some(root) = root { + open_update_pull_request_dialog( + store.clone(), + root, + window, + cx, + ); + } + } + })), + ) + }) .when_some(current_commit, |this, id| { this.child( h_flex() @@ -1064,6 +1104,68 @@ impl PullRequestDetailView { } } +/// Open the "update pull request" dialog: a patch input that submits a new +/// revision through [`RepoStore::update_pull_request`] when confirmed. +fn open_update_pull_request_dialog( + store: Entity, + root: Event, + window: &mut Window, + cx: &mut App, +) { + let patch = cx.new(|cx| { + TextareaState::new(window, cx).placeholder("Paste the updated `git format-patch` output...") + }); + // Both the dialog body and the submit button capture the root event; + // share it instead of cloning into each closure. + let root = Rc::new(root); + + window.open_dialog(cx, move |dialog, _window, _cx| { + let store = store.clone(); + let patch = patch.clone(); + let root = root.clone(); + + dialog + .width(px(520.)) + .margin_top(px(50.)) + .content(move |body, _window, _cx| { + body.child( + DialogHeader::new() + .child(DialogTitle::new().child("Update pull request")) + .child(DialogDescription::new().child( + "Publish a new revision with the output of `git format-patch`.", + )), + ) + .child( + v_form().child( + field() + .label("Patch") + .child(Textarea::new(&patch).h(px(160.))), + ), + ) + .child( + DialogFooter::new().justify_end().child( + Button::new("submit") + .primary() + .label("Update pull request") + .tooltip("Update pull request") + .on_click({ + let store = store.clone(); + let patch = patch.clone(); + let root = root.clone(); + move |_event, window, cx| { + let patch = patch.read(cx).value().to_string(); + store.update(cx, |store, cx| { + store.update_pull_request(&root, patch, cx); + }); + window.close_dialog(cx); + } + }), + ), + ) + }) + }); +} + /// One sidebar section title. fn sidebar_title(text: &str, cx: &App) -> AnyElement { div() @@ -1121,11 +1223,13 @@ fn branch_name_of(event: &Event) -> Option { } /// 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(); +/// `E` tag pointing at the root PR event. Only updates by the PR author +/// count: the tip of a PR is only mutable by its author (NIP-34). +fn latest_update<'a>(events: impl Iterator, root: &Event) -> Option<&'a Event> { + let root_hex = root.id.to_hex(); events .filter(|e| e.kind == Kind::GitPullRequestUpdate) + .filter(|e| e.pubkey == root.pubkey) .filter(|e| { e.tags .iter() @@ -1262,16 +1366,35 @@ mod tests { ); let events = [unrelated, revision(200), root.clone(), revision(300)]; - let latest = latest_update(events.iter(), &root.id).expect("an update"); + let latest = latest_update(events.iter(), &root).expect("an update"); assert_eq!(latest.created_at.as_secs(), 300); assert_eq!(latest.kind, Kind::GitPullRequestUpdate); } + #[test] + fn latest_update_ignores_other_authors() { + let root = pr_root(); + let root_hex = root.id.to_hex(); + let other = Keys::new( + SecretKey::from_hex("0000000000000000000000000000000000000000000000000000000000000002") + .expect("valid secret key"), + ); + let stranger = EventBuilder::new(Kind::GitPullRequestUpdate, "") + .tags([Tag::parse(["E", &root_hex]).expect("valid tag")]) + .custom_created_at(Timestamp::from(999)) + .finalize(&other) + .expect("signed event"); + + // The tip of a PR is only mutable by its author: a newer update + // from anyone else must not win. + assert!(latest_update([&stranger, &root].into_iter(), &root).is_none()); + } + #[test] fn latest_update_ignores_roots_without_revisions() { let root = pr_root(); - assert!(latest_update([&root].into_iter(), &root.id).is_none()); + assert!(latest_update([&root].into_iter(), &root).is_none()); } #[test] diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index 7084d11..a7554df 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -8,6 +8,7 @@ use gpui::{ SharedString, Size, WeakEntity, Window, div, px, size, }; use gpui_component::button::{Button, ButtonVariants}; +use gpui_component::checkbox::Checkbox; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputState, Textarea, TextareaState}; @@ -282,9 +283,15 @@ impl PullRequestsView { } } -/// Open the "new pull request" dialog: a title, an optional description and -/// a patch input that submit through [`RepoStore::open_pull_request`] when -/// confirmed. +/// State of the new pull request dialog, so the draft checkbox re-renders. +#[derive(Default)] +struct NewPullRequestDialogState { + draft: bool, +} + +/// Open the "new pull request" dialog: a title, an optional description, +/// an optional branch name and a patch input that submit through +/// [`RepoStore::open_pull_request`] when confirmed. pub(super) fn open_new_pull_request_dialog( store: Entity, window: &mut Window, @@ -293,19 +300,24 @@ pub(super) fn open_new_pull_request_dialog( 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 branch = cx.new(|cx| InputState::new(window, cx).placeholder("Branch name (optional)")); let patch = cx .new(|cx| TextareaState::new(window, cx).placeholder("Paste `git format-patch` output...")); + let state = cx.new(|_| NewPullRequestDialogState::default()); window.open_dialog(cx, move |dialog, _window, _cx| { let subject = subject.clone(); let description = description.clone(); + let branch = branch.clone(); let patch = patch.clone(); let store = store.clone(); + let state = state.clone(); dialog .width(px(520.)) .margin_top(px(50.)) - .content(move |body, _window, _cx| { + .content(move |body, _window, cx| { + let draft = state.read(cx).draft; body.child( DialogHeader::new() .child(DialogTitle::new().child("New pull request")) @@ -327,10 +339,29 @@ pub(super) fn open_new_pull_request_dialog( .label("Description") .child(Textarea::new(&description).h(px(96.))), ) + .child( + field() + .label("Branch") + .description("Optional: the branch the change is proposed from") + .child(Input::new(&branch)), + ) .child( field() .label("Patch") .child(Textarea::new(&patch).h(px(160.))), + ) + .child( + field().child( + Checkbox::new("pr-draft") + .label("Create as draft") + .checked(draft) + .on_click({ + let state = state.clone(); + move |checked, _window, cx| { + state.update(cx, |state, _| state.draft = *checked); + } + }), + ), ), ) .child( @@ -342,17 +373,29 @@ pub(super) fn open_new_pull_request_dialog( .on_click({ let subject = subject.clone(); let description = description.clone(); + let branch = branch.clone(); let patch = patch.clone(); let store = store.clone(); + let state = state.clone(); move |_event, window, cx| { let subject = subject.read(cx).value().to_string(); let description = description.read(cx).value().to_string(); + let branch = branch.read(cx).value().to_string(); let patch = patch.read(cx).value().to_string(); let subject = (!subject.is_empty()).then_some(subject); + let branch = (!branch.is_empty()).then_some(branch); + let draft = state.read(cx).draft; store.update(cx, |store, cx| { - store.open_pull_request(subject, description, patch, cx); + store.open_pull_request( + subject, + description, + branch, + patch, + draft, + cx, + ); }); window.close_dialog(cx); diff --git a/crates/workspace/src/views/sidebar/mod.rs b/crates/workspace/src/views/sidebar/mod.rs index 5d29f7e..e9973f9 100644 --- a/crates/workspace/src/views/sidebar/mod.rs +++ b/crates/workspace/src/views/sidebar/mod.rs @@ -100,7 +100,8 @@ impl SidebarPanel { fn refresh_my_repos(&mut self, cx: &mut Context) { self.my_repos_subscription = None; - let author = Backend::global(cx).read(cx).current_user(); + let backend = Backend::global(cx); + let author = backend.read(cx).current_user(); self.my_repos = author.map(|author| cx.new(|cx| RepoListStore::new(Some(author), cx))); if let Some(store) = self.my_repos.as_ref() { diff --git a/docs/PLAN.md b/docs/PLAN.md index 9409642..a57534b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -124,7 +124,7 @@ Current PR creation (`RepoStore::open_pull_request`, `crates/signed_state/src/re | Draft on create | no | optional 1633 status (P1) | | Merge provenance | plain 1631 | `merge-commit`/`applied-as-commits` (P4) | -### Phase 1 — Correctness & interop (small, surgical) +### Phase 1 — Correctness & interop (small, surgical) ✅ implemented 1. **Compute and publish `merge-base`, `branch-name`, `r` EUC** in `open_pull_request`: - Target tip = `RepoStore.head` ref from the state announcement (`refs`/`head`, `crates/signed_state/src/repo.rs:28-30`); add `signed_git::merge_base(repo, a, b)` (shell out like `apply_patch`). @@ -134,6 +134,8 @@ Current PR creation (`RepoStore::open_pull_request`, `crates/signed_state/src/re 3. **Fix `latest_update`** (`crates/workspace/src/views/repo_detail/pull_request_detail.rs:1125`): filter by the root PR's author (nak and ngit both restrict tip updates to the PR author). 4. **Draft toggle** in the new-PR dialog: publish a 1633 status right after the PR event (reuse `set_status`). +**Status:** items 1 (partial — `branch-name` + `r` EUC done; `merge-base` remains `None` because the paste-based flow has no access to the author's git objects to compute a merge base; it becomes computable in Phase 2 when the patch is generated from a local checkout), 2, 3, 4 are implemented. + ### Phase 2 — UX: replace the paste 5. **Local-repo picker** replaces the paste textarea (keep it as an advanced fallback): user picks a git checkout (or the app's `GitCache` mirror), source branch and target branch. The app then: @@ -154,10 +156,10 @@ Current PR creation (`RepoStore::open_pull_request`, `crates/signed_state/src/re ### Checklist -- [ ] P1: merge-base + branch-name + `r` EUC on creation. -- [ ] P1: `update_pull_request` (1619) + UI button; author check. -- [ ] P1: `latest_update` author filter. -- [ ] P1: draft toggle on create. +- [x] P1: merge-base + branch-name + `r` EUC on creation (merge-base deferred to P2 — not computable from a pasted patch). +- [x] P1: `update_pull_request` (1619) + UI button; author check. +- [x] P1: `latest_update` author filter. +- [x] P1: draft toggle on create. - [ ] P2: local checkout picker + generated patch + pre-publish apply check. - [ ] P3: push tip to grasp, truthful `clone` tags, size-aware series. - [ ] P4: merge status tags. diff --git a/docs/TODO.md b/docs/TODO.md index 695a787..c5cbf8e 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -2,10 +2,19 @@ ## Fork support -- [ ] Add UI for fork (see `PLAN.md` section 1): - - [ ] Fork badge on repo list cards (`repo_list.rs::render_card`). - - [ ] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. - - [ ] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). +- [x] Add UI for fork (see `PLAN.md` section 1): + - [x] Fork badge on repo list cards (`repo_list.rs::render_card`). + - [x] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. + - [x] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). + +## Pull request improvement (see `PLAN.md` section 2) + +- [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog. +- [x] P1: `RepoStore::update_pull_request` (kind 1619 + root-revision patch) with an author-only "Update" button on the PR detail header. +- [x] P1: `latest_update` filters by PR author. +- [ ] P2: local checkout picker + generated patch + pre-publish apply check (also enables `merge-base`). +- [ ] P3: push tip to grasp, truthful `clone` tags, size-aware patch series. +- [ ] P4: `merge-commit`/`applied-as-commits` tags on merge status. ## Performance: render path -- 2.54.0 From db3eaff4b9db032448c0ac46cc8c9c8e73b463c5 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 09:53:50 +0700 Subject: [PATCH 4/9] improve pull request flow --- crates/signed_git/src/lib.rs | 373 +++++++++++++++ crates/signed_state/src/backend.rs | 58 ++- crates/signed_state/src/repo.rs | 446 ++++++++++++++---- .../src/views/repo_detail/pull_requests.rs | 426 ++++++++++++++++- docs/PLAN.md | 165 ------- docs/TODO.md | 21 +- 6 files changed, 1210 insertions(+), 279 deletions(-) delete mode 100644 docs/PLAN.md diff --git a/crates/signed_git/src/lib.rs b/crates/signed_git/src/lib.rs index 4413542..835ec77 100644 --- a/crates/signed_git/src/lib.rs +++ b/crates/signed_git/src/lib.rs @@ -195,6 +195,185 @@ pub fn apply_patch(repo_path: &Path, patch: &str) -> Result<()> { Ok(()) } +/// The merge base of two revisions (branch names, remote-tracking refs or +/// commit ids) in the repository at `repo_path`. `Ok(None)` when the +/// revisions share no common ancestor; unresolvable revisions are errors. +pub fn merge_base(repo_path: &Path, a: &str, b: &str) -> Result> { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["merge-base", a, b]) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git merge-base`")?; + + match output.status.code() { + // Exit 1: no common ancestor (a valid outcome for a proposal). + Some(1) => Ok(None), + Some(0) => Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_owned(), + )), + _ => bail!( + "git merge-base failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ), + } +} + +/// The `git format-patch` series of `base..tip` (mbox), like +/// `git format-patch --stdout`. Fails when the range has no commits. The +/// mbox is returned untrimmed; trailing newlines are part of the format. +pub fn format_patch_between(repo_path: &Path, base: &str, tip: &str) -> Result { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["format-patch", "--stdout", &format!("{base}..{tip}")]) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git format-patch`")?; + + if !output.status.success() { + bail!( + "git format-patch failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + let patch = String::from_utf8_lossy(&output.stdout).into_owned(); + if patch.trim().is_empty() { + bail!("no commits between {base} and {tip}"); + } + Ok(patch) +} + +/// Whether `patch` (a `git format-patch` series) applies to the working +/// tree of `repo_path`, without modifying anything +/// (`git apply --check --3way`). Best-effort: useful to surface conflicts +/// before a patch is published or applied. +pub fn patch_applies(repo_path: &Path, patch: &str) -> Result<()> { + let mut child = Command::new("git") + .arg("apply") + .args(["--check", "--3way", "--whitespace=nowarn", "-"]) + .current_dir(repo_path) + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("failed to spawn `git apply --check`")?; + + child + .stdin + .as_mut() + .expect("stdin piped") + .write_all(patch.as_bytes())?; + + let output = child.wait_with_output()?; + if !output.status.success() { + bail!( + "patch does not apply: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + Ok(()) +} + +/// Push `commit` to `reference` (e.g. `refs/nostr/`) on the git +/// server at `url`, from the repository at `repo_path`. GRASP servers host +/// the `refs/nostr` namespace so anyone can contribute a commit; nak pushes +/// pull request tips there before publishing the PR event, and readers +/// fetch the ref to get the commit behind a PR's `c` tag. +pub fn push_commit_ref(repo_path: &Path, url: &str, commit: &str, reference: &str) -> Result<()> { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["push"]) + .arg(url) + .arg(format!("{commit}:{reference}")) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git push`")?; + + if !output.status.success() { + bail!( + "git push failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + Ok(()) +} + +/// Split a `git format-patch` series into its individual patches (mbox +/// messages). Each message begins with a `From <40-hex> ` boundary line; +/// `>From` quoting inside bodies means no false positives. A single patch +/// yields one element; a malformed input yields one element covering it. +pub fn split_patch_series(patch: &str) -> Vec<&str> { + let mut starts = vec![0usize]; + let mut search_from = 1; + while let Some(rel) = patch[search_from..].find("\nFrom ") { + let ix = search_from + rel + 1; + let hex = patch[ix + 5..] + .split(|c: char| !c.is_ascii_hexdigit()) + .next() + .unwrap_or(""); + if hex.len() == 40 { + starts.push(ix); + } + search_from = ix + 1; + } + + starts + .iter() + .enumerate() + .map(|(i, &start)| { + let end = starts.get(i + 1).copied().unwrap_or(patch.len()); + &patch[start..end] + }) + .collect() +} + +/// The commit HEAD points to in the repository at `repo_path`, or `None` +/// when the repository has no commits yet (unborn HEAD). +pub fn head_commit_id(repo_path: &Path) -> Result> { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["rev-parse", "HEAD"]) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git rev-parse`")?; + + if !output.status.success() { + return Ok(None); + } + Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_owned(), + )) +} + +/// The commits in `base..HEAD` of the repository at `repo_path`, oldest +/// first (the order `git am` creates them); `HEAD` alone when `base` is +/// `None`. An empty range yields an empty list. +pub fn commits_since(repo_path: &Path, base: Option<&str>) -> Result> { + let output = match base { + Some(base) => git_in( + repo_path, + &["rev-list", "--reverse", &format!("{base}..HEAD")], + )?, + // No `base` (unborn HEAD): there is nothing to walk yet. + None => match git_in(repo_path, &["rev-parse", "HEAD"]) { + Ok(head) => head, + Err(_) => return Ok(Vec::new()), + }, + }; + Ok(output + .lines() + .map(str::to_owned) + .filter(|line| !line.is_empty()) + .collect()) +} + fn clone(url: &str, path: &Path) -> Result { // GRASP servers announce `grasp:////` clone URLs; // the transport is git smart HTTP, so rewrite the scheme for gix. @@ -1884,6 +2063,200 @@ mod tests { git_run(repo.workdir().expect("workdir"), &["commit", "-m", message]); } + #[test] + fn merge_base_finds_the_fork_point_and_reports_unrelated_history() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + // A feature branch and a mainline commit diverge from the initial + // commit; it is their merge base. + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "feature commit"); + git_run(&path, &["checkout", "main"]); + std::fs::write(path.join("main.txt"), "main\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "mainline commit"); + + assert_eq!( + merge_base(&path, "feature", "main") + .expect("merge base") + .as_deref(), + Some(initial.as_str()) + ); + + // An orphan branch shares no history with main: `Ok(None)`. + git_run(&path, &["checkout", "--orphan", "orphan"]); + std::fs::write(path.join("orphan.txt"), "orphan\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "orphan commit"); + assert_eq!(merge_base(&path, "orphan", "main").expect("ok"), None); + + // An unresolvable revision is an error, not a missing ancestor. + assert!(merge_base(&path, "orphan", "no-such-ref").is_err()); + } + + #[test] + fn format_patch_between_produces_the_series_and_rejects_empty_ranges() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "feature commit"); + + let patch = format_patch_between(&path, &initial, "feature").expect("patch"); + assert!(patch.contains("Subject: [PATCH] feature commit")); + assert!(patch.contains("feature.txt")); + + // An empty range has no commits to send. + assert!(format_patch_between(&path, "feature", "feature").is_err()); + } + + #[test] + fn patch_applies_checks_without_modifying_the_tree() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "feature commit"); + let patch = format_patch_between(&path, &initial, "feature").expect("patch"); + + // A clone of the initial state accepts the series... + let clone = dir.path().join("clone"); + git_run( + dir.path(), + &[ + "clone", + "-q", + path.to_str().unwrap(), + clone.to_str().unwrap(), + ], + ); + git_run(&clone, &["checkout", "-q", &initial]); + assert!(patch_applies(&clone, &patch).is_ok()); + // ...and the check must not have modified the working tree. + assert!(!clone.join("feature.txt").exists()); + + // A conflicting file makes the same series fail the check. + std::fs::write(clone.join("feature.txt"), "conflicting\n").expect("write"); + assert!(patch_applies(&clone, &patch).is_err()); + } + + #[test] + fn push_commit_ref_pushes_to_the_event_namespace() { + // A bare "server" repository reachable via a `file://` URL, like a + // grasp server's `{base}/{owner}/{repo-id}.git` layout. + let server = tempfile::tempdir().unwrap(); + let server_repo = server.path().join("npub1test").join("my-repo.git"); + std::fs::create_dir_all(server_repo.parent().unwrap()).unwrap(); + let init_status = Command::new("git") + .args(["init", "--bare", "-q"]) + .arg(&server_repo) + .status() + .expect("spawn git init --bare"); + assert!(init_status.success()); + + let (dir, repo) = fixture(&[("a.txt", b"one")]); + commit_all(&repo, "initial"); + let dir = dir.path(); + let tip = git_in(dir, &["rev-parse", "HEAD"]).expect("tip"); + + let url = format!("file://{}/npub1test/my-repo.git", server.path().display()); + push_commit_ref(dir, &url, &tip, "refs/nostr/abcd1234").expect("push"); + + let refs = git_in(&server_repo, &["show-ref"]).expect("server refs"); + assert!(refs.contains("refs/nostr/abcd1234")); + } + + #[test] + fn split_patch_series_splits_real_multi_commit_mboxes() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("one.txt"), "one\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "first commit"); + std::fs::write(path.join("two.txt"), "two\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "second commit"); + + let series = format_patch_between(&path, &initial, "feature").expect("series"); + let parts = split_patch_series(&series); + + assert_eq!(parts.len(), 2); + assert!(parts[0].contains("Subject: [PATCH 1/2] first commit")); + assert!(parts[1].contains("Subject: [PATCH 2/2] second commit")); + // Each part starts its own mbox message with its own commit id. + let first = parts[0].lines().next().expect("first header"); + let second = parts[1].lines().next().expect("second header"); + assert!(first.starts_with("From ") && first.len() >= 45); + assert_ne!(first, second); + } + + #[test] + fn split_patch_series_keeps_single_patches_whole() { + let patch = "From abcdefabcdefabcdefabcdefabcdefabcdefab Mon Sep 17 00:00:00 2001\nFrom: A \nSubject: [PATCH] fix\n\n---\n"; + let parts = split_patch_series(patch); + assert_eq!(parts.len(), 1); + assert_eq!(parts[0], patch); + } + + #[test] + fn head_commit_and_commits_since_track_applied_commits() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + assert_eq!( + head_commit_id(&path).expect("head").as_deref(), + Some(initial.as_str()) + ); + // No commits yet: `HEAD` alone. + assert_eq!( + commits_since(&path, None).expect("commits"), + vec![initial.clone()] + ); + + std::fs::write(path.join("one.txt"), "one\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "first commit"); + let first = head_commit_id(&path).expect("head").expect("on a branch"); + + std::fs::write(path.join("two.txt"), "two\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "second commit"); + let second = head_commit_id(&path).expect("head").expect("on a branch"); + + // Oldest first, like the order `git am` creates them. + assert_eq!( + commits_since(&path, Some(&initial)).expect("commits"), + vec![first.clone(), second.clone()] + ); + assert_eq!( + commits_since(&path, Some(&first)).expect("commits"), + vec![second] + ); + } + + #[test] + fn head_commit_reports_unborn_repositories() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let status = Command::new("git") + .args(["init", "-q"]) + .arg(&path) + .status() + .expect("spawn git init"); + assert!(status.success()); + + assert_eq!(head_commit_id(&path).expect("head"), None); + assert_eq!( + commits_since(&path, None).expect("commits"), + Vec::::new() + ); + } + #[test] fn init_repository_creates_main_branch_and_readme() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 5236925..190130c 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1302,6 +1302,55 @@ impl Backend { }) } + /// Broadcast and locally store an already-signed event, like + /// [`Self::send`] without the signing step. Callers that signed early + /// (e.g. to learn the event id before pushing a commit to the grasp + /// servers) publish through this. + pub fn publish_event( + &mut self, + event: Event, + cx: &mut Context, + ) -> Task> { + let client = self.client.clone(); + + cx.spawn(async move |this, cx| { + let work = cx.background_spawn(async move { + let output = client.send_event(&event).await?; + + if output.success.is_empty() && !output.failed.is_empty() { + let reasons = output + .failed + .values() + .cloned() + .collect::>() + .join(", "); + return Err(anyhow!("event not accepted by any relay: {reasons}")); + } + + Ok(event.clone()) + }); + + let result = work.await; + + match &result { + Ok(event) => { + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::Published(Box::new(event.clone()))); + }) + .ok(); + } + Err(e) => { + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::error(e.to_string())); + }) + .ok(); + } + } + + result + }) + } + /// Publish a NIP-34 repository announcement (kind 30617) with the /// current signer. The returned task yields the published event, so /// callers can show inline progress/errors. @@ -1414,7 +1463,12 @@ async fn connect_repo_relays_only( let relays = &relays; let sync_opts = sync_opts.clone(); async move { - if let Err(e) = client.sync(filter).with(relays.iter()).opts(sync_opts).await { + if let Err(e) = client + .sync(filter) + .with(relays.iter()) + .opts(sync_opts) + .await + { log::warn!("repo relay negentropy sync failed: {e}"); } } @@ -1469,7 +1523,7 @@ fn with_master_key(uri: &str, keys: &Keys) -> String { /// A `https://` (or `http://` for `ws://` grasp servers, like /// ngit) base URL for a grasp server. The repository then lives at /// `{base}/{npub}/{repo-id}.git`. -fn grasp_base_url(relay: &RelayUrl) -> Option { +pub(crate) fn grasp_base_url(relay: &RelayUrl) -> Option { // `domain()` drops the port; parse the full URL to keep it (local dev // grasp servers commonly run on a custom port). let parsed = Url::parse(relay.as_str()).ok()?; diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index a5a154c..ed66c7c 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -1,10 +1,11 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::path::PathBuf; use std::time::Duration; use anyhow::Error; use bitcoin_hashes::sha1::Hash as Sha1Hash; -use gpui::{AppContext, Context, Subscription, Task}; +use gpui::{AppContext, AsyncApp, Context, Subscription, Task, WeakEntity}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; use signed_core::{ @@ -13,13 +14,17 @@ use signed_core::{ subject_override, }; -use crate::backend::{Backend, BackendEvent}; +use crate::backend::{Backend, BackendEvent, grasp_base_url}; 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. const REFRESH_DEBOUNCE: Duration = Duration::from_millis(300); +/// Maximum size of one patch event, following NIP-34's guidance that +/// patches should be used when each event is under 60kb. +const MAX_PATCH_EVENT_BYTES: usize = 60 * 1024; + /// Per-repository store: announcement, state, issues, patches, PRs, /// comments and their resolved statuses. Always derived from the local /// database. @@ -52,6 +57,9 @@ pub struct RepoStore { version: u64, /// Error of the last action initiated from this store, if any. pub last_error: Option, + /// Non-fatal warning of the last action (e.g. a PR published without + /// its commit reaching a grasp server), if any. + pub last_warning: Option, /// Relays announced by this repository (NIP-34 `relays` tag) that we /// have already been asked to connect to and fetch from, to avoid /// re-subscribing on every refresh. @@ -130,6 +138,7 @@ impl RepoStore { labels: Vec::new(), version: 0, last_error: None, + last_warning: None, repo_relays: HashSet::new(), root_fetches: HashSet::new(), refreshing: false, @@ -620,14 +629,23 @@ impl RepoStore { /// (kind 1617) carrying the `git format-patch` output, which the PR /// references via an `e` tag (NIP-34). /// - /// The patch is published first so the PR can reference its id. The - /// proposed commit is parsed from the patch's `From ` header; - /// without one publishing is refused, because the PR's `c` tag must - /// carry a real commit id for other NIP-34 clients to verify and apply - /// the proposal. The `clone` tag carries the announced mirror URLs; the - /// linked patch is the source of truth until the commit is pushed there. + /// The patch series is published first (one kind-1617 event per commit, + /// chained with NIP-10 `e` replies, each under [`MAX_PATCH_EVENT_BYTES`]) + /// so the PR can reference the root patch's id. The proposed commit is + /// parsed from the series' last `From ` header (the tip); without + /// one publishing is refused, because the PR's `c` tag must carry a real + /// commit id for other NIP-34 clients to verify and apply the proposal. + /// The `clone` tag carries the announced mirror URLs, and when + /// `push_from` is set the tip is pushed to those servers under + /// `refs/nostr/` (best-effort) before the PR is published, so + /// the commit is actually downloadable there; the linked patch stays the + /// source of truth either way. + /// /// `branch_name` lands in the PR's `branch-name` tag (NIP-34); `draft` - /// publishes a kind-1633 status right after the PR event. + /// publishes a kind-1633 status right after the PR event. `merge_base` + /// is the hex commit the proposed branch forked from, computed from a + /// local checkout when the patch was generated there. + #[allow(clippy::too_many_arguments)] pub fn open_pull_request( &mut self, subject: Option, @@ -635,12 +653,36 @@ impl RepoStore { branch_name: Option, patch: String, draft: bool, + merge_base: Option, + push_from: Option, cx: &mut Context, ) { self.last_error = None; + self.last_warning = None; - let Some(current_commit) = - patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) + let series: Vec = signed_git::split_patch_series(&patch) + .into_iter() + .map(str::to_owned) + .collect(); + if let Some(oversized) = series + .iter() + .find(|part| part.len() > MAX_PATCH_EVENT_BYTES) + { + self.last_error = Some(format!( + "patch too large ({} bytes; NIP-34 suggests keeping each patch under {} bytes)", + oversized.len(), + MAX_PATCH_EVENT_BYTES + )); + cx.notify(); + return; + } + + // The tip of the series is its last commit; `git format-patch` + // orders patches oldest first. + let Some(current_commit) = series + .last() + .and_then(|part| patch_current_commit(part)) + .and_then(|hex| hex.parse::().ok()) else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -649,35 +691,41 @@ impl RepoStore { return; }; - let Ok(root_marker) = Tag::parse(["t", "root"]) else { - return; - }; - let commit_hex = current_commit.to_string(); - let mut patch_tags = vec![ - Tag::coordinate(self.addr.clone(), None), - Tag::public_key(self.addr.public_key), - root_marker, - ]; - // NIP-34: the `r` EUC tag lets clients subscribe to all patches of - // this repository; `commit`/`r` tags reference the proposed commit. - if let Some(euc) = self.announcement.as_ref().and_then(|a| a.euc.clone()) - && let Ok(tag) = Tag::parse(["r", &euc]) - { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["commit", &commit_hex]) { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["r", &commit_hex]) { - patch_tags.push(tag); - } - let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags(patch_tags); - let backend = Backend::global(cx); - let patch_task = backend.update(cx, |backend, cx| backend.send(patch_builder, cx)); + if backend.read(cx).current_user().is_none() { + self.last_error = Some("Sign in to open a pull request".into()); + cx.notify(); + return; + } + let signer = backend.read(cx).signer(); + + let addr = self.addr.clone(); + let owner = self.addr.public_key; + let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); + let (push_owner, push_repo_id, push_relays) = self + .announcement + .as_ref() + .map(|a| { + let owner = a.owner.to_bech32().unwrap_or_else(|_| a.owner.to_hex()); + (owner, a.id.clone(), a.relays.clone()) + }) + .unwrap_or_default(); self.tasks.push(cx.spawn(async move |this, cx| { - let patch_event = match patch_task.await { + // The PR references the root patch event so viewers can find + // the patch without carrying it inline. + let root_patch = match publish_patch_series( + &this, + cx, + &addr, + owner, + euc.as_deref(), + &series, + "root", + None, + ) + .await + { Ok(event) => event, Err(e) => { return this.update(cx, |this, cx| { @@ -687,9 +735,7 @@ impl RepoStore { } }; - // 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 = this.update(cx, |this, _cx| { let builder = GitPullRequest { repository: this.addr.clone(), content: description, @@ -697,30 +743,82 @@ impl RepoStore { labels: Vec::new(), branch_name, // NIP-34: PRs carry at least one clone URL where the - // tip commit can be downloaded; use the repository's - // announced mirrors until a push backend exists. + // tip commit can be downloaded; the announced mirrors + // are also the servers the tip is pushed to below. clone: this .announcement .as_ref() .map(|a| a.clone.clone()) .unwrap_or_default(), current_commit, - root_patch_event: Some(patch_event.id), - merge_base: None, + root_patch_event: Some(root_patch.id), + merge_base: merge_base + .and_then(|hex| hex.parse::().ok()), } .into_event_builder(); - // NIP-34: the `r` EUC tag lets clients subscribe to all PRs of this repository. - let builder = match this.announcement.as_ref().and_then(|a| a.euc.clone()) { + // NIP-34: the `r` EUC tag lets clients subscribe to all + // PRs of this repository; the SDK builder omits it. + match this.announcement.as_ref().and_then(|a| a.euc.clone()) { Some(euc) => builder.tag(Tag::parse(["r", &euc]).expect("valid r tag")), None => builder, - }; - - let backend = Backend::global(cx); - backend.update(cx, |backend, cx| backend.send(builder, cx)) + } })?; - let pr_event = match pr_task.await { + // Sign before publishing so the tip can be pushed to the grasp + // servers under `refs/nostr/` (nak's convention): + // readers fetch that ref to get the commit behind the `c` tag. + let event = cx + .background_spawn({ + let signer = signer.clone(); + async move { builder.finalize_async(&signer).await } + }) + .await?; + + if let Some(path) = push_from.as_ref() { + let tip = current_commit.to_string(); + let reference = format!("refs/nostr/{}", event.id.to_hex()); + let (pushed, failures) = cx + .background_spawn({ + let path = path.clone(); + let tip = tip.clone(); + let reference = reference.clone(); + let owner = push_owner.clone(); + let repo_id = push_repo_id.clone(); + let relays = push_relays.clone(); + async move { + let mut failures = Vec::new(); + let mut pushed = 0; + for relay in &relays { + let Some(base) = grasp_base_url(relay) else { + continue; + }; + let url = format!("{base}/{owner}/{repo_id}.git"); + match signed_git::push_commit_ref(&path, &url, &tip, &reference) { + Ok(()) => pushed += 1, + Err(e) => failures.push(format!("{relay}: {e}")), + } + } + (pushed, failures) + } + }) + .await; + if pushed == 0 { + this.update(cx, |this, cx| { + this.last_warning = Some(format!( + "Pull request published, but the commit could not be pushed to any grasp server ({}); the patch is still the source of truth", + failures.join("; ") + )); + cx.notify(); + })?; + } + } + + let publish_task = this.update(cx, |_this, cx| { + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| backend.publish_event(event, cx)) + })?; + let pr_event = match publish_task.await { Ok(event) => event, Err(e) => { return this.update(cx, |this, cx| { @@ -742,13 +840,15 @@ impl RepoStore { })); } - /// Update a pull request: publish a revision patch event chained to the - /// original root patch (`t root-revision` and a NIP-10 `e` reply, per - /// NIP-34), then a kind-1619 PR update event carrying the new tip. + /// Update a pull request: publish revision patch events chained to the + /// original root patch (`t root-revision` and a NIP-10 `e` reply on the + /// first, per NIP-34), then a kind-1619 PR update event carrying the + /// new tip. /// /// Only the PR author may update it; other authors must open a new PR. pub fn update_pull_request(&mut self, root: &Event, patch: String, cx: &mut Context) { self.last_error = None; + self.last_warning = None; let backend = Backend::global(cx); @@ -764,8 +864,28 @@ impl RepoStore { return; } - let Some(current_commit) = - patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) + let series: Vec = signed_git::split_patch_series(&patch) + .into_iter() + .map(str::to_owned) + .collect(); + if let Some(oversized) = series + .iter() + .find(|part| part.len() > MAX_PATCH_EVENT_BYTES) + { + self.last_error = Some(format!( + "patch too large ({} bytes; NIP-34 suggests keeping each patch under {} bytes)", + oversized.len(), + MAX_PATCH_EVENT_BYTES + )); + cx.notify(); + return; + } + + // The new tip of the PR is the last commit of the series. + let Some(current_commit) = series + .last() + .and_then(|part| patch_current_commit(part)) + .and_then(|hex| hex.parse::().ok()) else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -783,45 +903,29 @@ impl RepoStore { .map(|p| p.id) }); - let commit_hex = current_commit.to_string(); - let mut patch_tags = vec![ - Tag::coordinate(self.addr.clone(), None), - Tag::public_key(self.addr.public_key), - Tag::parse(["t", "root-revision"]).expect("valid root-revision tag"), - ]; - if let Some(root_patch_id) = root_patch_id - && let Ok(tag) = Tag::parse(["e", &root_patch_id.to_hex(), "", "reply"]) - { - patch_tags.push(tag); - } - // NIP-34: the `r` EUC tag lets clients subscribe to all patches of - // this repository; `commit`/`r` tags reference the new tip. - if let Some(euc) = self.announcement.as_ref().and_then(|a| a.euc.clone()) - && let Ok(tag) = Tag::parse(["r", &euc]) - { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["commit", &commit_hex]) { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["r", &commit_hex]) { - patch_tags.push(tag); - } - let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags(patch_tags); - - let backend = Backend::global(cx); - let patch_task = backend.update(cx, |backend, cx| backend.send(patch_builder, cx)); - + let addr = self.addr.clone(); + let owner = self.addr.public_key; + let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); let root = root.clone(); let clone: Vec = self .announcement .as_ref() .map(|a| a.clone.clone()) .unwrap_or_default(); - let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); self.tasks.push(cx.spawn(async move |this, cx| { - if let Err(e) = patch_task.await { + if let Err(e) = publish_patch_series( + &this, + cx, + &addr, + owner, + euc.as_deref(), + &series, + "root-revision", + root_patch_id, + ) + .await + { return this.update(cx, |this, cx| { this.last_error = Some(e.to_string()); cx.notify(); @@ -846,6 +950,7 @@ impl RepoStore { None => builder, }; + let backend = Backend::global(cx); backend.update(cx, |backend, cx| backend.send(builder, cx)) })?; @@ -953,7 +1058,10 @@ impl RepoStore { /// 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. + /// a kind-1631 (Applied) status event with merge provenance: the commits + /// `git am` created (`applied-as-commits` + `r` tags) and the applied + /// patch events (`q` tags, plus `e` reply tags for every patch beyond + /// the root, per NIP-34). /// /// Only the repository author may merge. The clone is created on demand /// from the announcement's clone URLs when needed. Patch application @@ -961,6 +1069,7 @@ impl RepoStore { /// no longer applies) surface in [`Self::last_error`]. pub fn merge_pull_request(&mut self, root: &Event, cx: &mut Context) { self.last_error = None; + self.last_warning = None; let is_author = Backend::global(cx) .read(cx) @@ -979,21 +1088,46 @@ impl RepoStore { .map(|a| a.clone.iter().map(ToString::to_string).collect()) .unwrap_or_default(); let patch = pull_request_patch(root, self.patches.iter()); + // The applied patch events, for the status tags below. + let patches: Vec = pull_request_patches(root, self.patches.iter()) + .into_iter() + .cloned() + .collect(); + let relay_hint = self + .announcement + .as_ref() + .and_then(|a| a.relays.first()) + .map(ToString::to_string) + .unwrap_or_default(); + let euc = self.announcement.as_ref().and_then(|a| a.euc.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) + .ok_or_else(|| anyhow::anyhow!("repository has no worktree"))? + .to_path_buf(); + // The commits created by the apply: everything between the + // previous HEAD and the new one, oldest first. + let previous = signed_git::head_commit_id(&workdir)?; + signed_git::apply_patch(&workdir, &patch)?; + let applied = signed_git::commits_since(&workdir, previous.as_deref())?; + Ok::<_, Error>(applied) }); self.tasks.push(cx.spawn(async move |this, cx| { match apply.await { - Ok(()) => { + Ok(applied) => { this.update(cx, |this, cx| { - this.set_status(&root, RepoStatus::Applied, cx); + this.publish_applied_status( + &root, + &patches, + &applied, + &relay_hint, + euc.as_deref(), + cx, + ); })?; } Err(e) => { @@ -1007,6 +1141,62 @@ impl RepoStore { })); } + /// Publish a kind-1631 (Applied) status event for `root` after a merge: + /// `applied-as-commits` + `r` tags for the commits `git am` created, + /// `q` tags for the applied patch events, and `e` reply tags for every + /// patch of the series beyond the root (NIP-34). + fn publish_applied_status( + &mut self, + root: &Event, + patches: &[Event], + applied: &[String], + relay_hint: &str, + euc: Option<&str>, + cx: &mut Context, + ) { + let mut tags = vec![ + Tag::parse(["e", &root.id.to_hex(), "", "root"]).expect("valid root tag"), + Tag::public_key(self.addr.public_key), + Tag::public_key(root.pubkey), + Tag::coordinate(self.addr.clone(), None), + ]; + if let Some(euc) = euc + && let Ok(tag) = Tag::parse(["r", euc]) + { + tags.push(tag); + } + // The applied patch events: a `q` tag per event, plus an `e` reply + // for every event beyond the root (chain parts and revisions), so + // their statuses resolve to Applied too. + for (ix, patch) in patches.iter().enumerate() { + if let Ok(tag) = + Tag::parse(["q", &patch.id.to_hex(), relay_hint, &patch.pubkey.to_hex()]) + { + tags.push(tag); + } + if ix > 0 + && let Ok(tag) = Tag::parse(["e", &patch.id.to_hex(), "", "reply"]) + { + tags.push(tag); + } + } + // The commits `git am` created on top of the previous HEAD. + if !applied.is_empty() { + let mut applied_tag = vec!["applied-as-commits".to_string()]; + applied_tag.extend(applied.iter().cloned()); + if let Ok(tag) = Tag::parse(applied_tag) { + tags.push(tag); + } + for commit in applied { + if let Ok(tag) = Tag::parse(["r", commit]) { + tags.push(tag); + } + } + } + + self.send(EventBuilder::new(Kind::GitStatusApplied, "").tags(tags), cx); + } + fn send(&mut self, builder: EventBuilder, cx: &mut Context) { self.last_error = None; @@ -1092,6 +1282,78 @@ fn patch_current_commit(patch: &str) -> Option<&str> { hex.split_whitespace().next().filter(|hex| hex.len() == 40) } +/// Publish a `git format-patch` series as chained kind-1617 events and +/// return the root event (the one a PR references). The first part carries +/// `first_marker` (`t root`, or `t root-revision` with an `e` reply to +/// `reply_to` for revisions); every later part replies to the previous one +/// (NIP-34). Every part gets the repository coordinate, the owner, its own +/// `commit`/`r` tags, and the repository EUC when known. +#[allow(clippy::too_many_arguments)] +async fn publish_patch_series( + this: &WeakEntity, + cx: &mut AsyncApp, + addr: &RepoAddr, + owner: PublicKey, + euc: Option<&str>, + series: &[String], + first_marker: &str, + reply_to: Option, +) -> Result { + let mut root: Option = None; + let mut previous = reply_to; + + for (ix, part) in series.iter().enumerate() { + let Some(commit) = patch_current_commit(part).filter(|hex| hex.len() == 40) else { + return Err(anyhow::anyhow!( + "patch {} of the series has no `From ` header", + ix + 1 + )); + }; + + let mut tags = vec![Tag::coordinate(addr.clone(), None), Tag::public_key(owner)]; + if ix == 0 { + if let Ok(tag) = Tag::parse(["t", first_marker]) { + tags.push(tag); + } + if let Some(root_id) = reply_to + && let Ok(tag) = Tag::parse(["e", &root_id.to_hex(), "", "reply"]) + { + tags.push(tag); + } + } else if let Some(previous) = previous + && let Ok(tag) = Tag::parse(["e", &previous.to_hex(), "", "reply"]) + { + tags.push(tag); + } + if let Some(euc) = euc + && let Ok(tag) = Tag::parse(["r", euc]) + { + tags.push(tag); + } + if let Ok(tag) = Tag::parse(["commit", commit]) { + tags.push(tag); + } + if let Ok(tag) = Tag::parse(["r", commit]) { + tags.push(tag); + } + + let builder = EventBuilder::new(Kind::GitPatch, part.clone()).tags(tags); + + let task = this.update(cx, |_this, cx| { + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| backend.send(builder, cx)) + })?; + let event = task.await?; + + if root.is_none() { + root = Some(event.clone()); + } + previous = Some(event.id); + } + + root.ok_or_else(|| anyhow::anyhow!("patch series is empty")) +} + /// Build a NIP-22 kind-1111 comment: uppercase `E`/`K`/`P` tags scope the /// thread root, lowercase `e`/`k`/`p` the direct parent (or the root for a /// top-level comment). An `a` tag with the repository coordinate (not part diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index a7554df..916edbf 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -1,12 +1,14 @@ +use std::path::{Path, PathBuf}; use std::rc::Rc; use assets::CustomIconName; use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; use gpui::prelude::*; use gpui::{ - AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, - SharedString, Size, WeakEntity, Window, div, px, size, + AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, PathPromptOptions, + Pixels, Render, SharedString, Size, WeakEntity, Window, div, px, size, }; +use gpui_component::alert::Alert; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::checkbox::Checkbox; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; @@ -14,11 +16,13 @@ use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputState, Textarea, TextareaState}; use gpui_component::scroll::Scrollbar; use gpui_component::{ - ActiveTheme, Icon, VirtualListScrollHandle, WindowExt, h_flex, v_flex, v_virtual_list, + ActiveTheme, Disableable, Icon, IconName, VirtualListScrollHandle, WindowExt, h_flex, v_flex, + v_virtual_list, }; use nostr::prelude::{EventId, Kind}; use signed_core::{RepoStatus, activity_subject}; -use signed_state::{ProfileStore, RepoStore}; +use signed_git::{format_patch_between, merge_base, patch_applies}; +use signed_state::{GitStore, ProfileStore, RepoStore}; use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{SegmentButton, UserAvatar, placeholder, status_badge}; use utils::relative_time; @@ -283,15 +287,55 @@ impl PullRequestsView { } } -/// State of the new pull request dialog, so the draft checkbox re-renders. +/// A patch series generated from a local repository, with the metadata +/// derived from it. +struct GeneratedPatch { + /// The `git format-patch` series (fills the patch textarea). + patch: String, + /// The merge base with the target branch, as hex. + merge_base: Option, +} + +/// State of the new pull request dialog, so the async generation, the +/// apply check and the draft checkbox re-render. #[derive(Default)] struct NewPullRequestDialogState { draft: bool, + /// The last generated patch series; its merge base is reused at submit + /// only while the patch textarea is unchanged. + generated: Option, + /// Result of the pre-publish applicability check against the app's + /// mirror clone of the target repository. + apply_check: Option>, + /// A patch generation is in flight. + generating: bool, + /// Error of the last generation attempt. + error: Option, +} + +impl NewPullRequestDialogState { + /// Text and whether it is good news, for the line under the patch field. + fn apply_check_message(&self) -> Option<(SharedString, bool)> { + match &self.apply_check { + Some(Ok(())) => Some(( + "Applies cleanly to the repository's default branch".into(), + true, + )), + Some(Err(error)) => Some(( + format!("May not apply cleanly to the repository's default branch: {error}").into(), + false, + )), + None => None, + } + } } /// Open the "new pull request" dialog: a title, an optional description, /// an optional branch name and a patch input that submit through -/// [`RepoStore::open_pull_request`] when confirmed. +/// [`RepoStore::open_pull_request`] when confirmed. The patch can either be +/// pasted, or generated from a local checkout: pick a repository, a source +/// and a target branch, and the app runs `git format-patch` itself and +/// checks the series against the app's mirror clone of the target. pub(super) fn open_new_pull_request_dialog( store: Entity, window: &mut Window, @@ -301,6 +345,9 @@ pub(super) fn open_new_pull_request_dialog( let description = cx.new(|cx| TextareaState::new(window, cx).placeholder("Describe the change...")); let branch = cx.new(|cx| InputState::new(window, cx).placeholder("Branch name (optional)")); + let repo_path = cx.new(|cx| InputState::new(window, cx).placeholder("Pick a local checkout…")); + let source = cx.new(|cx| InputState::new(window, cx).placeholder("Source branch")); + let target = cx.new(|cx| InputState::new(window, cx).placeholder("Target branch")); let patch = cx .new(|cx| TextareaState::new(window, cx).placeholder("Paste `git format-patch` output...")); let state = cx.new(|_| NewPullRequestDialogState::default()); @@ -309,21 +356,28 @@ pub(super) fn open_new_pull_request_dialog( let subject = subject.clone(); let description = description.clone(); let branch = branch.clone(); + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); let patch = patch.clone(); let store = store.clone(); let state = state.clone(); dialog - .width(px(520.)) + .width(px(560.)) .margin_top(px(50.)) .content(move |body, _window, cx| { + let generating = state.read(cx).generating; let draft = state.read(cx).draft; + let error = state.read(cx).error.clone(); + let apply_check = state.read(cx).apply_check_message(); body.child( DialogHeader::new() .child(DialogTitle::new().child("New pull request")) .child( - DialogDescription::new() - .child("Propose a change with the output of `git format-patch`."), + DialogDescription::new().child( + "Propose a change with the output of `git format-patch`.", + ), ), ) .child( @@ -339,6 +393,104 @@ pub(super) fn open_new_pull_request_dialog( .label("Description") .child(Textarea::new(&description).h(px(96.))), ) + .child( + field() + .label("Local repository") + .description( + "Generate the patch from a local checkout; leave empty to paste it", + ) + .child( + h_flex() + .gap_1() + .items_center() + .child( + div() + .flex_1() + .child(Input::new(&repo_path).disabled(true)), + ) + .child( + Button::new("choose-checkout") + .icon(IconName::FolderOpen) + .ghost() + .tooltip("Choose local checkout") + .on_click({ + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); + let patch = patch.clone(); + let branch = branch.clone(); + let state = state.clone(); + let store = store.clone(); + move |_ev, window, cx| { + choose_local_repo( + &repo_path, + &source, + &target, + &patch, + &branch, + &state, + &store, + window, + cx, + ); + } + }), + ) + .child( + Button::new("generate-patch") + .ghost() + .label("Generate") + .tooltip( + "Generate the patch from the local checkout", + ) + .loading(generating) + .disabled(generating) + .on_click({ + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); + let patch = patch.clone(); + let branch = branch.clone(); + let state = state.clone(); + let store = store.clone(); + move |_ev, window, cx| { + let path = + repo_path.read(cx).value().to_string(); + let source = + source.read(cx).value().to_string(); + let target = + target.read(cx).value().to_string(); + if !path.is_empty() + && !source.is_empty() + && !target.is_empty() + { + generate_patch( + &state, + &patch, + &branch, + path, + source, + target, + &store, + window, + cx, + ); + } + } + }), + ), + ), + ) + .child( + field() + .label("Source branch") + .child(Input::new(&source)), + ) + .child( + field() + .label("Target branch") + .child(Input::new(&target)), + ) .child( field() .label("Branch") @@ -346,9 +498,31 @@ pub(super) fn open_new_pull_request_dialog( .child(Input::new(&branch)), ) .child( - field() - .label("Patch") - .child(Textarea::new(&patch).h(px(160.))), + field().label("Patch").child( + v_flex() + .gap_1() + .child(Textarea::new(&patch).h(px(140.))) + .when_some(apply_check, |this, (message, ok)| { + this.child( + div() + .text_xs() + .text_color(if ok { + cx.theme().success + } else { + cx.theme().warning + }) + .child(message), + ) + }) + .when_some(error, |this, message| { + this.child( + div() + .text_xs() + .text_color(cx.theme().danger) + .child(message), + ) + }), + ), ) .child( field().child( @@ -370,15 +544,21 @@ pub(super) fn open_new_pull_request_dialog( .primary() .label("Create pull request") .tooltip("Create pull request") + .loading(generating) + .disabled(generating) .on_click({ let subject = subject.clone(); let description = description.clone(); let branch = branch.clone(); let patch = patch.clone(); + let repo_path = repo_path.clone(); let store = store.clone(); let state = state.clone(); move |_event, window, cx| { + if state.read(cx).generating { + return; + } let subject = subject.read(cx).value().to_string(); let description = description.read(cx).value().to_string(); let branch = branch.read(cx).value().to_string(); @@ -386,6 +566,21 @@ pub(super) fn open_new_pull_request_dialog( let subject = (!subject.is_empty()).then_some(subject); let branch = (!branch.is_empty()).then_some(branch); let draft = state.read(cx).draft; + // The generated merge base stays valid + // only while the patch is unchanged; an + // edited patch falls back to none. + let merge_base = state + .read(cx) + .generated + .as_ref() + .filter(|generated| generated.patch == patch) + .and_then(|generated| generated.merge_base.clone()); + // The checkout (when set) is where the + // tip commit is pushed from, so other + // clients can fetch it. + let repo_path = repo_path.read(cx).value().to_string(); + let push_from = (!repo_path.is_empty()) + .then(|| PathBuf::from(repo_path)); store.update(cx, |store, cx| { store.open_pull_request( @@ -394,6 +589,8 @@ pub(super) fn open_new_pull_request_dialog( branch, patch, draft, + merge_base, + push_from, cx, ); }); @@ -407,6 +604,188 @@ pub(super) fn open_new_pull_request_dialog( }); } +/// Prompt for a local checkout, fill the source/target defaults (the +/// checkout's current branch and the repository's announced HEAD) and +/// generate the patch series right away. +#[allow(clippy::too_many_arguments)] +fn choose_local_repo( + repo_path: &Entity, + source: &Entity, + target: &Entity, + patch: &Entity, + branch: &Entity, + state: &Entity, + store: &Entity, + window: &mut Window, + cx: &mut App, +) { + let handle = window.window_handle(); + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); + let patch = patch.clone(); + let branch = branch.clone(); + let state = state.clone(); + let store = store.clone(); + // The announced HEAD branch is the natural target default. + let target_default = store.read(cx).head.clone().unwrap_or_default(); + + let prompt = cx.prompt_for_paths(PathPromptOptions { + files: false, + directories: true, + multiple: false, + prompt: Some("Choose local checkout".into()), + }); + + cx.spawn(async move |cx| { + if let Ok(Ok(Some(mut paths))) = prompt.await + && let Some(path) = paths.pop() + { + let path = path.to_string_lossy().to_string(); + + // The checkout's current branch is the source default; resolve + // it off the UI thread. + let current = cx + .background_executor() + .spawn({ + let path = path.clone(); + async move { + gix::open(Path::new(&path)) + .ok() + .and_then(|repo| signed_git::current_branch(&repo).ok().flatten()) + } + }) + .await; + + let _ = handle.update(cx, |_, window, cx| { + repo_path.update(cx, |input, cx| { + input.set_value(path.clone(), window, cx); + }); + source.update(cx, |input, cx| { + input.set_value(current.clone().unwrap_or_default(), window, cx); + }); + target.update(cx, |input, cx| { + input.set_value(target_default.clone(), window, cx); + }); + + if let Some(current) = current + && !current.is_empty() + && !target_default.is_empty() + { + generate_patch( + &state, + &patch, + &branch, + path, + current, + target_default, + &store, + window, + cx, + ); + } + }); + } + }) + .detach(); +} + +/// Generate the patch series `source..target` of the local checkout at +/// `repo_path`, fill the patch textarea and record the merge base and the +/// pre-publish applicability check in `state`. +#[allow(clippy::too_many_arguments)] +fn generate_patch( + state: &Entity, + patch_input: &Entity, + branch_input: &Entity, + repo_path: String, + source: String, + target: String, + store: &Entity, + window: &mut Window, + cx: &mut App, +) { + state.update(cx, |state, cx| { + state.generating = true; + state.error = None; + state.apply_check = None; + cx.notify(); + }); + + let cache = GitStore::global(cx).cache().clone(); + let (addr, clone_urls) = { + let store = store.read(cx); + ( + store.addr().clone(), + store + .announcement + .as_ref() + .map(|a| { + a.clone + .iter() + .map(ToString::to_string) + .collect::>() + }) + .unwrap_or_default(), + ) + }; + + let handle = window.window_handle(); + let state = state.clone(); + let patch_input = patch_input.clone(); + let branch_input = branch_input.clone(); + + let task = cx.spawn(async move |cx| { + // The branch-name tag defaults to the source branch; keep a copy + // for the UI update after the background generation moves it. + let source_label = source.clone(); + let generated = cx + .background_executor() + .spawn(async move { + let base = + merge_base(Path::new(&repo_path), &source, &target)?.ok_or_else(|| { + anyhow::anyhow!("{source} and {target} share no common ancestor") + })?; + let patch = format_patch_between(Path::new(&repo_path), &base, &source)?; + // Best-effort: does the series apply to the current default + // branch of the app's mirror clone of the target repository? + let check = cache + .ensure_clone(&addr, &clone_urls) + .ok() + .and_then(|repo| repo.workdir().map(|workdir| workdir.to_path_buf())) + .map(|workdir| patch_applies(&workdir, &patch).map_err(|e| e.to_string())); + Ok::<_, anyhow::Error>((patch, Some(base), check)) + }) + .await; + + let _ = handle.update(cx, |_, window, cx| match generated { + Ok((patch, merge_base, check)) => { + patch_input.update(cx, |input, cx| { + input.set_value(patch.clone(), window, cx); + }); + // The branch-name tag defaults to the source branch. + if branch_input.read(cx).value().is_empty() { + branch_input.update(cx, |input, cx| { + input.set_value(source_label.clone(), window, cx); + }); + } + state.update(cx, |state, cx| { + state.generating = false; + state.generated = Some(GeneratedPatch { patch, merge_base }); + state.apply_check = check; + cx.notify(); + }); + } + Err(error) => state.update(cx, |state, cx| { + state.generating = false; + state.error = Some(error.to_string().into()); + cx.notify(); + }), + }); + }); + task.detach(); +} + impl BasePanel for PullRequestsView { fn panel_name(&self) -> &'static str { "pull-requests" @@ -480,10 +859,33 @@ impl Render for PullRequestsView { let scroll_handle = self.scroll_handle.clone(); let view = cx.entity().clone(); + // Non-fatal warnings and errors of the last action (e.g. creating + // or updating a PR), shown as dismissible banners above the list. + let (last_error, last_warning) = { + let store = self.store.read(cx); + (store.last_error.clone(), store.last_warning.clone()) + }; + v_flex() .size_full() .image_cache(image_cache("pull-requests", MAX_IMAGES)) .child(self.render_header(cx)) + .when_some(last_warning, |this, warning| { + this.child(Alert::warning("pr-warning", warning).banner().on_close({ + let store = self.store.clone(); + move |_event, _window, cx| { + store.update(cx, |store, _| store.last_warning = None); + } + })) + }) + .when_some(last_error, |this, error| { + this.child(Alert::error("pr-error", error).banner().on_close({ + let store = self.store.clone(); + move |_event, _window, cx| { + store.update(cx, |store, _| store.last_error = None); + } + })) + }) .child( v_flex() .relative() diff --git a/docs/PLAN.md b/docs/PLAN.md deleted file mode 100644 index a57534b..0000000 --- a/docs/PLAN.md +++ /dev/null @@ -1,165 +0,0 @@ -# Plan - -Two work streams: - -1. **Fork support (display + navigation UI)** — show when a repository is a fork and let the user jump to the upstream repository. -2. **Pull request improvement** — bring PR creation/updating in line with the other NIP-34 clients (nak, ngit). - ---- - -## 1. Fork support - -### Background: what NIP-34 says about forks - -NIP-34 has no fork event kind — a fork is an ordinary kind-30617 announcement by another author (or the same author under a different `d`). Fork-ness is expressed by two tags: - -- **`u` tag** on the fork's announcement: - `["u", "30617::|", "", ""]`. - Including `u` means the author does **not** assert maintainership of the primary project (the fork is a *subordinate* of the upstream). -- **EUC** (`r` tag with `euc` marker): shared between the fork and its upstream (and other mirrors), so clients can group them. For a permanent fork, the EUC is the first commit after the fork point. - -### Current state - -- `Announcement::from_event` parses the `u` tag into an opaque string (`crates/signed_core/src/model.rs:212`; only the first value is kept, manually, because the SDK's `Nip34Tag` has no `Upstream` variant). -- `effective_maintainers` excludes the fork author (`model.rs:250`) — already correct per NIP-34. -- The About dialog shows the raw upstream string as a plain row (`crates/workspace/src/views/repo_detail/about.rs:68`). -- Nothing shows fork-ness in the repo list or the repo detail header, and there is no way to navigate to the upstream. - -### Goal - -- **Repo list card** (`crates/workspace/src/views/repo_list.rs::render_card`): show a "Forked from " badge instead of/in addition to the description, with a fork icon. -- **Repo detail header** (`crates/workspace/src/views/repo_detail/mod.rs::render_header`): show a "Forked from " text button near the repo name. -- **Clicking the upstream** opens the upstream repository as a center panel (same as clicking any repo card). - -### Design - -#### 1.1 Structured `Upstream` model (`signed_core`) - -Add a structured type and keep the manual parse: - -```rust -pub struct Upstream { - /// `30617::` (navigable) or a git URL (not navigable). - pub target: UpstreamTarget, - pub relay_hint: Option, - pub author: Option, -} - -pub enum UpstreamTarget { - /// Parseable via the SDK `Coordinate` (`30617::`). - Repo(RepoAddr), - /// Git https URL form: no NIP-34 announcement, not navigable. - GitUrl(Url), -} -``` - -- Change `Announcement.upstream: Option` to `Option`; parse all three `u` values (the SDK's `Nip34Tag::parse` is not usable here — keep the manual `tag.kind() == "u"` branch and extend it). -- `RepoAddr` is the SDK `Coordinate` (`crates/signed_core/src/addr.rs`), so `Coordinate::from_str` gives the upstream address directly; validate it is kind `30617`. -- Update `about.rs` (renders `upstream`), `effective_maintainers`, and the `model.rs` tests (`parses_upstream_tag`, `effective_maintainers_exclude_owner_for_subordinate_forks`). - -#### 1.2 Resolving the upstream announcement - -Opening a panel needs an `Announcement` (`RepoDetailView::new`), so resolve the upstream announcement before (or while) opening: - -1. **Lookup, no fetch**: the global `RepoListStore` holds every announcement in the local database (`crates/signed_state/src/repo_list.rs:51`). Look up the upstream `RepoAddr` there — covers the common case (upstream already browsed/known) with zero network. -2. **Miss → fetch, then open**: add a `Backend` method (e.g. `fetch_announcement(addr) -> Task>`) doing a one-shot query with `filters::announcement(addr)` (`crates/signed_core/src/filters.rs:21`), mirroring the bootstrap fetch in `RepoStore::subscribe_remote` (`crates/signed_state/src/repo.rs:196`). Show the upstream as a disabled/loading row until it resolves; on failure fall back to showing the raw address. -3. **Git-URL upstreams**: not navigable — render as plain text with a copy action (like `copy_row` in `signed_ui`), no panel. - -#### 1.3 Shared "open repo panel" helper - -The open-panel sequence is currently duplicated three times: - -- `crates/workspace/src/views/repo_list.rs:170` (`RepoListView::open_repo`) -- `crates/workspace/src/views/sidebar/mod.rs:155` (`SidebarPanel::open_repo`) -- `crates/workspace/src/views/sidebar/create_repo_dialog.rs:259` (`open_repo`) - -Extract one helper (e.g. `open_repo_panel(dock_area, announcement, window, cx)` in the `workspace` views layer) and reuse it from all three plus the new fork button, so the fork navigation behaves exactly like clicking a repo card. - -#### 1.4 Repo list badge - -In `render_card`, when `announcement.upstream` is set: - -- Resolve the upstream's display name via the `RepoListStore` lookup (1.2); fall back to the raw address string. -- Render a small "Forked from " line (fork icon + `text_xs` muted), replacing or joining the description line. Add a `git-fork.svg` asset to `crates/assets/assets/icons/` + a `CustomIconName::GitFork` variant (lucide's `git-fork`), or reuse `git-branch.svg` if an asset addition is undesirable. - -#### 1.5 Repo detail header - -In `render_header` (`crates/workspace/src/views/repo_detail/mod.rs:1231`), next to the repo name: - -- "Forked from " as a **text button** (`gpui_base::Button` or styled `div`), which calls the shared open-panel helper with the resolved upstream announcement. -- Keep the About dialog row in sync: make it the same clickable control (or at least the same resolved display name). -- Handle "upstream not in store yet": spawn the `fetch_announcement` task; button shows a subtle loading state; on success open the panel (needs `window`/`cx` — the task is spawned on the view, `apply_announcement`-style flow). - -#### 1.6 (Stretch) Fork grouping by EUC - -`RepoListStore` already has `euc` per announcement; add a "N forks" count on the detail header by scanning announcements sharing the same EUC, with a filter or navigation into the explore list. Not required for the first iteration. - -### Checklist - -- [ ] `signed_core`: `Upstream`/`UpstreamTarget` types + full `u`-tag parse; `Announcement.upstream` type change; tests. -- [ ] `Backend::fetch_announcement(addr)` one-shot fetch. -- [ ] Shared `open_repo_panel` helper; switch the three existing call sites. -- [ ] Repo list card fork badge (+ `git-fork.svg` asset if used). -- [ ] Repo detail header "Forked from" button + About row sync. -- [ ] Manual test: fork with coordinate upstream (navigates), fork with git-URL upstream (copy only), upstream announcement absent (fetch-then-open). - ---- - -## 2. Pull request improvement - -### Why - -Current PR creation (`RepoStore::open_pull_request`, `crates/signed_state/src/repo.rs:626`; dialog `crates/workspace/src/views/repo_detail/pull_requests.rs:288`) requires pasting `git format-patch` output, publishes no `merge-base`/`branch-name`, cannot update an existing PR, and advertises clone URLs the author usually cannot push to. Compared with nak (`pr send`/`pr update`/`pr merge`) and ngit (push-based PRs with merge-base inference), the gaps are: - -| Area | Today | Fix (phase) | -| --- | --- | --- | -| Patch generation | manual paste | generate from a local checkout (P2) | -| `merge-base` tag | always `None` | compute vs state HEAD (P1) | -| `branch-name` tag | never | send local branch name (P1) | -| PR updates (kind 1619) | not producible | author-only update flow (P1) | -| Update reader trusts any author | `latest_update` has no author filter | filter by PR author (P1) | -| `clone` URL truthfulness | repo mirrors (author can't push) | push tip to grasp first (P3) | -| Multi-commit series | one oversized event | NIP-10 chain / size-aware (P3) | -| Pre-publish validation | none | `git am --check` dry run (P2) | -| Draft on create | no | optional 1633 status (P1) | -| Merge provenance | plain 1631 | `merge-commit`/`applied-as-commits` (P4) | - -### Phase 1 — Correctness & interop (small, surgical) ✅ implemented - -1. **Compute and publish `merge-base`, `branch-name`, `r` EUC** in `open_pull_request`: - - Target tip = `RepoStore.head` ref from the state announcement (`refs`/`head`, `crates/signed_state/src/repo.rs:28-30`); add `signed_git::merge_base(repo, a, b)` (shell out like `apply_patch`). - - Fill the `GitPullRequest` builder's existing `merge_base`/`branch_name` fields (currently hardcoded `None`, `repo.rs:691-702`); pass the branch name and tip through from the dialog. - - Add the `r` EUC tag manually to the PR event (the SDK builder omits it; NIP-34 recommends it for subscription efficiency). -2. **Add `RepoStore::update_pull_request(root, new_tip, …)`** producing a kind-1619 event via the SDK `GitPullRequestUpdate` builder (`E`/`P`/`K` NIP-22 tags) plus a chained root-revision patch (`t root-revision`, `e` reply to the original root patch). Author-only, mirroring nak's `pr update`. Wire a button into `pull_request_detail.rs`. -3. **Fix `latest_update`** (`crates/workspace/src/views/repo_detail/pull_request_detail.rs:1125`): filter by the root PR's author (nak and ngit both restrict tip updates to the PR author). -4. **Draft toggle** in the new-PR dialog: publish a 1633 status right after the PR event (reuse `set_status`). - -**Status:** items 1 (partial — `branch-name` + `r` EUC done; `merge-base` remains `None` because the paste-based flow has no access to the author's git objects to compute a merge base; it becomes computable in Phase 2 when the patch is generated from a local checkout), 2, 3, 4 are implemented. - -### Phase 2 — UX: replace the paste - -5. **Local-repo picker** replaces the paste textarea (keep it as an advanced fallback): user picks a git checkout (or the app's `GitCache` mirror), source branch and target branch. The app then: - - resolves the tip (`git rev-parse`), - - computes `merge-base` vs the target tip, - - runs `format-patch base..tip --stdout` itself (add `signed_git::format_patch_between`, like nak), - - **dry-runs `git am --3way --check`** against the cached clone before publishing (`signed_git::apply_patch` infra, `crates/signed_git/src/lib.rs:176`), surfacing "does not apply" before anything hits the relays. - -### Phase 3 — Truthful clone URLs (interop) - -6. **Push before publishing**: add `signed_git::push_commit_ref(path, url, commit, ref)` and reuse the grasp-push infrastructure (`grasp_base_url`, `push_to_grasp_servers`, `crates/signed_state/src/backend.rs:1472,1500`) to push the tip to `refs/nostr/` on the announced grasp servers (nak's `gitPushCommitToGraspRefs`). On success the `clone` tag carries the real URL; on failure fall back to the current patch-event model with a warning. -7. **Size-aware publishing**: split multi-commit mboxes into a NIP-10-chained 1617 series (each < 60 KB per NIP-34) or go PR-only above that size; adopt ngit's patch→PR upgrade (new PR + close-status for the original patch). -8. Optional: GRASP-06 `/prs//.git` + kind-10317 user grasp-list fallback (ngit's server-selection cascade). Fork support (section 1) makes the fork's own grasp server a natural push target here. - -### Phase 4 — Merge provenance - -9. In `merge_pull_request` (`crates/signed_state/src/repo.rs:817`), publish the 1631 status with `merge-commit` (or `applied-as-commits`) and `q` tags so nak/ngit/GitWorkshop show merge provenance correctly. - -### Checklist - -- [x] P1: merge-base + branch-name + `r` EUC on creation (merge-base deferred to P2 — not computable from a pasted patch). -- [x] P1: `update_pull_request` (1619) + UI button; author check. -- [x] P1: `latest_update` author filter. -- [x] P1: draft toggle on create. -- [ ] P2: local checkout picker + generated patch + pre-publish apply check. -- [ ] P3: push tip to grasp, truthful `clone` tags, size-aware series. -- [ ] P4: merge status tags. diff --git a/docs/TODO.md b/docs/TODO.md index c5cbf8e..47df0cd 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -2,19 +2,24 @@ ## Fork support -- [x] Add UI for fork (see `PLAN.md` section 1): - - [x] Fork badge on repo list cards (`repo_list.rs::render_card`). - - [x] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. - - [x] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). +- [x] Fork badge on repo list cards (`repo_list.rs::render_card`). +- [x] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. +- [x] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). -## Pull request improvement (see `PLAN.md` section 2) +## Pull request improvement - [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog. - [x] P1: `RepoStore::update_pull_request` (kind 1619 + root-revision patch) with an author-only "Update" button on the PR detail header. - [x] P1: `latest_update` filters by PR author. -- [ ] P2: local checkout picker + generated patch + pre-publish apply check (also enables `merge-base`). -- [ ] P3: push tip to grasp, truthful `clone` tags, size-aware patch series. -- [ ] P4: `merge-commit`/`applied-as-commits` tags on merge status. +- [x] P2: local checkout picker in the new-PR dialog (folder picker + source/target branches + Generate): `signed_git::{merge_base, format_patch_between, patch_applies}`; `merge-base` tag now published; best-effort apply check shown under the patch field. +- [x] P3: push tip to grasp servers under `refs/nostr/` before publishing (from the local checkout); multi-commit series published as NIP-10-chained 1617 events with a 60 KB per-patch cap; PR list shows dismissible error/warning banners (incl. push failures). +- [x] P4: merge status tags — `merge_pull_request` publishes 1631 with `applied-as-commits` + `r` per applied commit and `q`/`e`-reply tags per applied patch event. + +### Pull request follow-ups + +- [ ] GRASP-06 `/prs//.git` contributor endpoints + kind-10317 user grasp-list fallback. +- [ ] Merge button in the PR detail view (`merge_pull_request` is store-only today), then fetch-and-merge (`merge-commit`) when the push backend is guaranteed. +- [ ] Local-checkout generation for the update-PR dialog (currently paste-only). ## Performance: render path -- 2.54.0 From e6e9a58be3ddf249b27d23cf81c0b059da48f563 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 10:03:53 +0700 Subject: [PATCH 5/9] update document --- docs/PR_FLOW.md | 139 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 docs/PR_FLOW.md diff --git a/docs/PR_FLOW.md b/docs/PR_FLOW.md new file mode 100644 index 0000000..1ab69f2 --- /dev/null +++ b/docs/PR_FLOW.md @@ -0,0 +1,139 @@ +# Pull request flow + +How a pull request moves through Signed from creation to merge. A PR is a +kind-1618 root event whose content is the markdown description; its changes +live in a NIP-10-chained series of kind-1617 patch events (one per commit), +whose root the PR references via an `e` tag. Revisions publish new patch +events plus kind-1619 updates; statuses (kind 1630-1633) resolve the PR's +state. + +## Whole lifecycle + +```mermaid +graph TD + A["New pull request dialog"] --> B{"Patch source"} + B -->|"Paste"| C["Paste git format-patch output"] + B -->|"Local checkout"| D["Browse for checkout"] + D --> E["Defaults: source = current branch, target = announced HEAD"] + E --> F["Generate: merge-base plus format-patch base..tip"] + F --> G["Apply check vs mirror clone - non-blocking warning"] + C --> H["Submit"] + F --> H + G --> H + H --> I["split_patch_series: one part per commit"] + I --> J{"Any part over 60 KB?"} + J -->|"Yes"| K["Refuse with message"] + J -->|"No"| L["tip = last part's From commit"] + L --> M["Publish kind-1617 patch series: first has t root, later parts e-reply chained"] + M --> N["Build kind-1618 PR event: c = tip, e = root patch, branch-name, merge-base"] + N --> O["Sign early - learn the event id"] + O --> P["Push tip to refs/nostr/event-id on every announced grasp server"] + P -->|"All rejected"| Q["last_warning banner in PR list"] + P --> R["Publish kind-1618 PR event"] + Q --> R + R --> S{"Draft?"} + S -->|"Yes"| T["Publish kind-1633 draft status"] + S -->|"No"| U["PR open"] + T --> U + U --> V{"Author updates?"} + V -->|"Yes"| W["Publish revision patch series: first has t root-revision and e-replies to the original root"] + W --> X["Publish kind-1619 update: E/P NIP-22 tags, c = new tip"] + X --> U + V -->|"No"| Y{"Repository author merges?"} + Y -->|"Yes"| Z["Apply the series with git am on the mirror clone"] + Z --> AA["applied = rev-list previous-head..HEAD"] + AA --> AB["Publish kind-1631 applied status: applied-as-commits plus r per commit, q plus e-reply per patch event"] + AB --> AC["PR merged"] + Y -->|"Close instead"| AD["Publish kind-1632 closed status"] + AD --> AE["PR closed"] +``` + +Key points of the write side: + +- **Merge base**: only computable in the local-checkout path + (`signed_git::merge_base`); the paste path publishes none. The dialog + reuses it at submit only while the patch textarea is unchanged. +- **Patch series**: each commit becomes its own kind-1617 event so no event + grows past NIP-34's 60 KB guidance; the PR's `c` tag carries the *last* + commit of the series (the tip), and each part carries its own + `commit`/`r` tags. +- **Push before publish**: the tip is pushed to every announced grasp + server under `refs/nostr/` (nak's convention) so the announced + `clone` URLs really can serve the commit. Failure is non-fatal — the + patch events remain the source of truth — and surfaces as a + `last_warning` banner. + +## Creating a pull request - event ordering + +```mermaid +sequenceDiagram + participant User + participant App + participant Checkout as Local checkout + participant Grasp as Grasp servers + participant Relays as Nostr relays + + User->>App: pick checkout and branches, Generate + App->>Checkout: merge-base(source, target) + Checkout-->>App: base commit + App->>Checkout: format-patch base..tip + Checkout-->>App: patch series + App->>App: split series, check per-part size + loop each patch of the series + App->>Relays: publish kind-1617 (first: t root, later: e reply) + end + App->>App: build and sign kind-1618 PR event + App->>Grasp: push tip to refs/nostr/event-id + Grasp-->>App: accepted or rejected (best-effort) + App->>Relays: publish kind-1618 PR event + opt draft + App->>Relays: publish kind-1633 draft status + end +``` + +## Updating and merging + +```mermaid +sequenceDiagram + participant Author + participant Relays as Nostr relays + participant Maintainer + participant Clone as Mirror clone + + Note over Author,Relays: Update - PR author only + Author->>Relays: publish revision patch series (t root-revision, e reply to original root) + Author->>Relays: publish kind-1619 update (E/P tags, c = new tip) + + Note over Maintainer,Clone: Merge - repository author only (store-only today) + Maintainer->>Clone: git am the patch series + Clone-->>Maintainer: applied commits (rev-list previous-head..HEAD) + Maintainer->>Relays: publish kind-1631 applied status + Note over Relays: applied-as-commits and r per commit, q and e-reply per applied patch event +``` + +## Reading side + +```mermaid +graph TD + A["PR root kind-1618"] --> B{"Newest status event by author or maintainer?"} + B -->|"1633"| C["Draft"] + B -->|"1631"| D["Applied / merged"] + B -->|"1632"| E["Closed"] + B -->|"1630 or none"| F["Open"] + A --> G{"Newest kind-1619 update by PR author?"} + G -->|"Yes"| H["tip = update's c tag"] + G -->|"No"| I["tip = root's c tag"] + A --> J{"Patch set present?"} + J -->|"Yes"| K["Root patch via e tag, follow reply chain (newest wins per revision)"] + J -->|"No"| L["Diff merge-base..tip from the git clone"] +``` + +Reader rules that keep the flow consistent: + +- **Status**: only status events by the root author or a repository + maintainer count; the newest wins, `Open` is the default. +- **Tip**: only kind-1619 updates by the PR author move the tip — a + stranger's update is ignored. +- **Diff**: the patch set is preferred (NIP-34 `e`-linked chain); PRs from + other clients without patch events fall back to diffing + `merge-base..tip` in the local clone. -- 2.54.0 From 15224277bc95f9558bb6222ce9381931d5ebac4f Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 11:00:00 +0700 Subject: [PATCH 6/9] update pull request ui --- .../src/views/repo_detail/commits.rs | 61 +- .../workspace/src/views/repo_detail/diff.rs | 315 ++++--- crates/workspace/src/views/repo_detail/mod.rs | 12 +- .../src/views/repo_detail/new_pull_request.rs | 859 ++++++++++++++++++ .../src/views/repo_detail/pull_requests.rs | 526 +---------- docs/TODO.md | 13 +- 6 files changed, 1104 insertions(+), 682 deletions(-) create mode 100644 crates/workspace/src/views/repo_detail/new_pull_request.rs diff --git a/crates/workspace/src/views/repo_detail/commits.rs b/crates/workspace/src/views/repo_detail/commits.rs index 3c757b6..14ce94c 100644 --- a/crates/workspace/src/views/repo_detail/commits.rs +++ b/crates/workspace/src/views/repo_detail/commits.rs @@ -1,5 +1,5 @@ use gpui::prelude::*; -use gpui::{AnyElement, App, Context, WeakEntity, div, px}; +use gpui::{AnyElement, App, Context, Window, div, px}; use gpui_component::scroll::Scrollbar; use gpui_component::spinner::Spinner; use gpui_component::{ActiveTheme, Sizable, h_flex, v_flex, v_virtual_list}; @@ -12,17 +12,12 @@ use super::RepoDetailView; /// Height of one commit row in the virtual list. pub(super) const COMMIT_ROW_HEIGHT: f32 = 56.; -/// One row of the commit list: id, summary, author and relative time. -/// Clicking a row opens the diff of that commit in a new panel. -fn commit_row( +pub(super) fn commit_row( ix: usize, commit: &FileCommit, - view: &WeakEntity, + on_click: impl Fn(&mut Window, &mut App) + 'static, cx: &App, ) -> AnyElement { - let view = view.clone(); - let id = commit.id.clone(); - h_flex() .id(ix) .px_4() @@ -70,11 +65,7 @@ fn commit_row( .child(relative_time_secs(commit.time)), ), ) - .on_click(move |_event, window, cx| { - if let Some(view) = view.upgrade() { - view.update(cx, |this, cx| this.open_commit_diff(&id, window, cx)); - } - }) + .on_click(move |_event, window, cx| on_click(window, cx)) .into_any_element() } @@ -114,22 +105,34 @@ impl RepoDetailView { .w_full() .min_h_0() .child( - v_virtual_list( - view, - "repo-commits", - sizes, - move |this, range, _window, cx| { - let commits = this - .all_commits - .as_ref() - .map(|list| list.commits.as_slice()) - .unwrap_or(&[]); - let view = cx.entity().downgrade(); - range - .map(|ix| commit_row(ix, &commits[ix], &view, cx)) - .collect() - }, - ) + v_virtual_list(view, "commits", sizes, move |this, range, _window, cx| { + let view = cx.entity().downgrade(); + let commits = this + .all_commits + .as_ref() + .map(|list| list.commits.as_slice()) + .unwrap_or(&[]); + + range + .map(|ix| { + let id = commits[ix].id.clone(); + let view = view.clone(); + + commit_row( + ix, + &commits[ix], + move |window, cx| { + if let Some(view) = view.upgrade() { + view.update(cx, |this, cx| { + this.open_commit_diff(&id, window, cx) + }); + } + }, + cx, + ) + }) + .collect() + }) .track_scroll(&scroll_handle) .size_full(), ) diff --git a/crates/workspace/src/views/repo_detail/diff.rs b/crates/workspace/src/views/repo_detail/diff.rs index 8d0cacc..e724352 100644 --- a/crates/workspace/src/views/repo_detail/diff.rs +++ b/crates/workspace/src/views/repo_detail/diff.rs @@ -28,22 +28,13 @@ use super::helpers::{ /// Width of the changed-files column. const TREE_WIDTH: f32 = 260.; -/// Detail panel showing the diff of one commit. -pub struct CommitDiffView { - focus_handle: FocusHandle, - /// Local clone the commit lives in. - worktree: PathBuf, - /// Display name of the repository the commit belongs to. - repo_name: SharedString, - /// The commit being shown (header and tab title). Starts as an id-only - /// stub; [`Self::load`] replaces it with the full metadata, which the - /// history list intentionally omits. - commit: FileCommit, - /// Loaded diff; `None` while loading or after a failure. +/// The tree + per-file diff body shared by the commit diff panel and the +/// compare view of the new-pull-request panel. Owns the changed-files +/// explorer and the virtual list of the selected file's hunks; the host +/// feeds it a [`CommitDiff`] via [`DiffPane::set_diff`]. +pub struct DiffPane { + /// Loaded diff; `None` until [`Self::set_diff`] is called. diff: Option, - /// The diff is being computed 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. @@ -55,114 +46,60 @@ pub struct CommitDiffView { item_sizes: Rc>>, /// Virtual list state of the diff rows. scroll_handle: VirtualListScrollHandle, - /// In-flight tasks; pruned on every push (see [`helpers::track`]). - tasks: Vec>>, } -impl CommitDiffView { - pub fn new( - worktree: PathBuf, - repo_name: SharedString, - commit_id: String, - window: &mut Window, - cx: &mut Context, - ) -> Self { - let tree_state = cx.new(|cx| TreeState::new(cx)); - - // Defer until the window is ready, like the repository detail view. - cx.defer_in(window, |this, window, cx| { - this.load(window, cx); - }); - +impl DiffPane { + pub fn new(cx: &mut Context) -> Self { Self { - focus_handle: cx.focus_handle(), - worktree, - repo_name, - commit: FileCommit { - id: commit_id, - summary: String::new(), - description: None, - author: String::new(), - time: 0, - }, diff: None, - loading: true, - error: None, - tree_state, + tree_state: cx.new(|cx| TreeState::new(cx)), selected_file: None, rows: Vec::new(), item_sizes: Rc::new(Vec::new()), scroll_handle: VirtualListScrollHandle::new(), - tasks: Vec::new(), } } - /// Load the commit diff (and the full commit metadata) on a background - /// task and populate the tree. - fn load(&mut self, window: &mut Window, cx: &mut Context) { - self.loading = true; - self.error = None; - cx.notify(); + /// The loaded diff, for stats and badges in the host's header. + pub fn diff(&self) -> Option<&CommitDiff> { + self.diff.as_ref() + } - let worktree = self.worktree.clone(); - let id = self.commit.id.clone(); - - let task = cx.spawn_in(window, async move |this, cx| { - let commit = cx - .background_spawn({ - let worktree = worktree.clone(); - let id = id.clone(); - async move { signed_git::worktree_commit(&worktree, &id) } - }) - .await; - let diff = cx - .background_spawn({ - let worktree = worktree.clone(); - let id = id.clone(); - async move { signed_git::worktree_commit_diff(&worktree, &id) } - }) - .await; - - this.update_in(cx, |this, _window, cx| { - this.loading = false; - if let Ok(Some(commit)) = commit { - this.commit = commit; - } - 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(()) + /// Replace the diff and rebuild the tree and the selected file's rows. + pub fn set_diff(&mut self, diff: CommitDiff, cx: &mut Context) { + 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())); + self.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); }); + self.selected_file = first.clone(); + self.diff = Some(diff); + if let Some(path) = first { + self.set_diff_rows(path.as_ref()); + } + } - self.tasks.push(task); + /// Forget the diff (e.g. when the compared branches changed): clear the + /// tree, the selection and the diff rows. + pub fn clear(&mut self, cx: &mut Context) { + self.diff = None; + self.selected_file = None; + self.rows = Vec::new(); + self.item_sizes = Rc::new(Vec::new()); + self.tree_state.update(cx, |state, cx| { + state.set_items(Vec::new(), cx); + }); } /// Show the diff of the file at `path` (selected in the tree). @@ -226,8 +163,8 @@ impl CommitDiffView { .p_2(), ) }) - .when(self.diff.is_none() && !self.loading, |this| { - this.child(placeholder("Failed to load diff", cx)) + .when(self.diff.is_none(), |this| { + this.child(placeholder("No changes", cx)) }), ) .into_any_element() @@ -235,23 +172,12 @@ impl CommitDiffView { /// 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); + return placeholder("No changes", cx); }; let Some(path) = self.selected_file.clone() else { return if diff.files.is_empty() { - placeholder("No files changed in this commit", cx) + placeholder("No files changed", cx) } else { placeholder("Select a file", cx) }; @@ -377,11 +303,126 @@ impl CommitDiffView { .child(div().id("commit-diff-body").flex_1().min_h_0().child(body)) .into_any_element() } +} + +impl Render for DiffPane { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + h_flex() + .size_full() + .min_h_0() + .bg(cx.theme().background) + .child(self.render_tree_column(cx)) + .child(self.render_detail_column(cx)) + } +} + +/// Detail panel showing the diff of one commit: a metadata header plus the +/// shared [`DiffPane`] body. +pub struct CommitDiffView { + focus_handle: FocusHandle, + /// Local clone the commit lives in. + worktree: PathBuf, + /// Display name of the repository the commit belongs to. + repo_name: SharedString, + /// The commit being shown (header and tab title). Starts as an id-only + /// stub; [`Self::load`] replaces it with the full metadata, which the + /// history list intentionally omits. + commit: FileCommit, + /// The diff is being computed on a background task. + loading: bool, + error: Option, + /// Changed-files explorer and per-file diff, shared with the compare + /// view of the new-pull-request panel. + pane: Entity, + /// In-flight tasks; pruned on every push (see [`helpers::track`]). + tasks: Vec>>, +} + +impl CommitDiffView { + pub fn new( + worktree: PathBuf, + repo_name: SharedString, + commit_id: String, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let pane = cx.new(DiffPane::new); + + // Defer until the window is ready, like the repository detail view. + cx.defer_in(window, |this, window, cx| { + this.load(window, cx); + }); + + Self { + focus_handle: cx.focus_handle(), + worktree, + repo_name, + commit: FileCommit { + id: commit_id, + summary: String::new(), + description: None, + author: String::new(), + time: 0, + }, + loading: true, + error: None, + pane, + tasks: Vec::new(), + } + } + + /// Load the commit diff (and the full commit metadata) on a background + /// task and populate the tree. + fn load(&mut self, window: &mut Window, cx: &mut Context) { + self.loading = true; + self.error = None; + cx.notify(); + + let worktree = self.worktree.clone(); + let id = self.commit.id.clone(); + + let task = cx.spawn_in(window, async move |this, cx| { + let commit = cx + .background_spawn({ + let worktree = worktree.clone(); + let id = id.clone(); + async move { signed_git::worktree_commit(&worktree, &id) } + }) + .await; + let diff = cx + .background_spawn({ + let worktree = worktree.clone(); + let id = id.clone(); + async move { signed_git::worktree_commit_diff(&worktree, &id) } + }) + .await; + + this.update_in(cx, |this, _window, cx| { + this.loading = false; + if let Ok(Some(commit)) = commit { + this.commit = commit; + } + match diff { + Ok(diff) => { + this.pane.update(cx, |pane, cx| pane.set_diff(diff, cx)); + } + Err(error) => { + this.error = Some(error.to_string().into()); + } + } + cx.notify(); + })?; + + Ok(()) + }); + + self.tasks.push(task); + } /// Header: commit id, summary, author/time and overall change stats. fn render_header(&self, cx: &mut Context) -> AnyElement { let commit = &self.commit; - let (files, insertions, deletions) = self.diff.as_ref().map_or((0, 0, 0), |diff| { + let (files, insertions, deletions) = self.pane.read(cx).diff().map_or((0, 0, 0), |diff| { ( diff.files.len(), diff.files.iter().map(|file| file.insertions).sum(), @@ -481,6 +522,19 @@ impl Focusable for CommitDiffView { impl Render for CommitDiffView { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let body: AnyElement = if self.loading { + v_flex() + .size_full() + .items_center() + .justify_center() + .child(Spinner::new().small()) + .into_any_element() + } else if let Some(error) = self.error.clone() { + placeholder(&error, cx) + } else { + self.pane.clone().into_any_element() + }; + v_resizable("commit-diff") .child( resizable_panel() @@ -490,15 +544,6 @@ impl Render for CommitDiffView { .bg(cx.theme().background) .child(self.render_header(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)), - ), - ) + .child(resizable_panel().child(body)) } } diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index 6550103..0954e89 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -41,6 +41,7 @@ mod helpers; mod init_dialog; mod issue_detail; mod issues; +mod new_pull_request; mod pull_request_detail; mod pull_requests; @@ -53,7 +54,8 @@ use commits::COMMIT_ROW_HEIGHT; use diff::CommitDiffView; use helpers::{ShareTargets, TreeItemSeed, build_tree_items, is_markdown_path, tree_items}; use issues::{IssuesView, open_new_issue_dialog}; -use pull_requests::{PullRequestsView, open_new_pull_request_dialog}; +use new_pull_request::open_new_pull_request_panel; +use pull_requests::PullRequestsView; /// What kind of ref the header selectors switch to. #[derive(Clone, Copy, PartialEq, Eq)] @@ -1361,7 +1363,13 @@ impl RepoDetailView { } RepoAction::NewPR => { if let Some(store) = this.store.clone() { - open_new_pull_request_dialog(store, window, cx); + open_new_pull_request_panel( + this.dock_area.clone(), + store, + this.display_name(cx), + window, + cx, + ); } } RepoAction::About => { diff --git a/crates/workspace/src/views/repo_detail/new_pull_request.rs b/crates/workspace/src/views/repo_detail/new_pull_request.rs new file mode 100644 index 0000000..81c4d9c --- /dev/null +++ b/crates/workspace/src/views/repo_detail/new_pull_request.rs @@ -0,0 +1,859 @@ +//! The "new pull request" panel: pick a local checkout, a base and a +//! compare branch (GitHub-style), review the diff and the commit list, then +//! publish the PR with only a title and an optional description. The patch +//! series is generated from the checkout at submit time; there is no patch +//! input. + +use std::path::{Path, PathBuf}; +use std::rc::Rc; + +use assets::CustomIconName; +use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; +use gpui::prelude::*; +use gpui::{ + AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, PathPromptOptions, + Pixels, Render, SharedString, Size, Subscription, Task, WeakEntity, Window, div, px, relative, + size, +}; +use gpui_base::Button as BaseButton; +use gpui_component::button::{Button, ButtonVariants}; +use gpui_component::combobox::{ + Caret, Combobox, ComboboxEvent, ComboboxState, ComboboxTriggerContext, +}; +use gpui_component::input::{Input, InputEvent, InputState, Textarea, TextareaState}; +use gpui_component::scroll::Scrollbar; +use gpui_component::searchable_list::SearchableVec; +use gpui_component::spinner::Spinner; +use gpui_component::{ + ActiveTheme, Disableable, Icon, IconName, Sizable, VirtualListScrollHandle, h_flex, v_flex, + v_virtual_list, +}; +use signed_git::{ + format_patch_between, merge_base, worktree_commit_range_commits, worktree_commit_range_diff, +}; +use signed_state::RepoStore; +use signed_ui::placeholder; + +use super::commits::{COMMIT_ROW_HEIGHT, commit_row}; +use super::diff::{CommitDiffView, DiffPane}; + +/// The "new pull request" panel of a repository. +/// +/// Both branch selectors list the branches of a user-chosen local checkout; +/// the compare view (Files/Commits tabs) is built from `merge-base..compare` +/// in that checkout, and the patch series published with the PR is generated +/// from the same range at submit time. +pub struct NewPullRequestView { + focus_handle: FocusHandle, + /// Dock area the panel lives in; commit diffs are opened there. + dock_area: WeakEntity, + /// Store of the target repository (for the announced HEAD default). + store: Entity, + /// Display name of the repository, for the panel title. + repo_name: SharedString, + /// The user's checkout: where both branches live and where the tip is + /// pushed from. + repo_path: Option, + /// Branches of the checkout, backing both selectors. + branches: Vec, + /// Selected base branch (the target of the PR). + base: SharedString, + /// Selected compare branch (the source of the PR). + compare: SharedString, + base_select: Entity>>, + compare_select: Entity>>, + /// Title input (required). + subject: Entity, + /// Description input (optional). + description: Entity, + /// Merge base of the selected branches; `None` until the compare loads. + merge_base: Option, + /// Commits in `merge_base..compare`, newest first. + commits: Option>, + /// The compare is being computed. + loading: bool, + /// Error of the last compare or submit attempt. + error: Option, + /// A submit (patch generation + publish) is in flight. + submitting: bool, + /// Bumped on every branch switch; stale compare results are discarded. + compare_generation: u64, + /// Active tab: 0 = Files, 1 = Commits. + active_tab: usize, + /// The compare diff (Files tab). + pane: Entity, + /// Virtual list state of the Commits tab. + scroll_handle: VirtualListScrollHandle, + item_sizes: Rc>>, + _subscriptions: Vec, + tasks: Vec>>, +} + +impl NewPullRequestView { + pub fn new( + dock_area: WeakEntity, + store: Entity, + repo_name: SharedString, + window: &mut Window, + cx: &mut Context, + ) -> Self { + 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 (optional)")); + let pane = cx.new(DiffPane::new); + + let base_select: Entity>> = cx.new(|cx| { + ComboboxState::new( + SearchableVec::new(Vec::::new()), + Vec::new(), + window, + cx, + ) + .searchable(true) + }); + let compare_select: Entity>> = cx.new(|cx| { + ComboboxState::new( + SearchableVec::new(Vec::::new()), + Vec::new(), + window, + cx, + ) + .searchable(true) + }); + + let subscriptions = vec![ + // Re-evaluate the Create button's enabled state as the title + // changes. + cx.subscribe(&subject, |_this, _state, _event: &InputEvent, cx| { + cx.notify(); + }), + cx.subscribe_in(&base_select, window, |this, _state, event, window, cx| { + if let ComboboxEvent::Change(values) = event + && let Some(name) = values.first() + { + this.base = name.clone(); + this.reload_compare(window, cx); + } + }), + cx.subscribe_in( + &compare_select, + window, + |this, _state, event, window, cx| { + if let ComboboxEvent::Change(values) = event + && let Some(name) = values.first() + { + this.compare = name.clone(); + this.reload_compare(window, cx); + } + }, + ), + ]; + + Self { + focus_handle: cx.focus_handle(), + dock_area, + store, + repo_name, + repo_path: None, + branches: Vec::new(), + base: SharedString::default(), + compare: SharedString::default(), + base_select, + compare_select, + subject, + description, + merge_base: None, + commits: None, + loading: false, + error: None, + submitting: false, + compare_generation: 0, + active_tab: 0, + pane, + scroll_handle: VirtualListScrollHandle::new(), + item_sizes: Rc::new(Vec::new()), + _subscriptions: subscriptions, + tasks: Vec::new(), + } + } + + /// Prompt for a local checkout; on success populate the branch selectors + /// (defaults: the announced HEAD branch for the base, the checkout's + /// current branch for the compare) and load the compare. + fn choose_checkout(&mut self, window: &mut Window, cx: &mut Context) { + let handle = window.window_handle(); + + let prompt = cx.prompt_for_paths(PathPromptOptions { + files: false, + directories: true, + multiple: false, + prompt: Some("Choose local checkout".into()), + }); + + let task = cx.spawn(async move |this, cx| { + if let Ok(Ok(Some(mut paths))) = prompt.await + && let Some(path) = paths.pop() + { + let path = path.to_string_lossy().to_string(); + // Branches and the current branch are read off the UI thread. + let info = cx + .background_executor() + .spawn({ + let path = path.clone(); + async move { + let repo = gix::open(Path::new(&path)).ok()?; + let branches = + signed_git::worktree_branches(Path::new(&path)).unwrap_or_default(); + let current = signed_git::current_branch(&repo).ok().flatten(); + Some((branches, current)) + } + }) + .await; + + let _ = handle.update(cx, |_, window, cx| { + let _ = this.update(cx, |this, cx| { + this.apply_checkout(path, info, window, cx); + }); + }); + } + Ok(()) + }); + self.tasks.push(task); + } + + /// Apply a picked checkout: fill the selectors and load the compare. + fn apply_checkout( + &mut self, + path: String, + info: Option<(Vec, Option)>, + window: &mut Window, + cx: &mut Context, + ) { + let Some((branches, current)) = info else { + self.error = Some("The chosen folder is not a git repository".into()); + self.repo_path = None; + self.branches.clear(); + self.merge_base = None; + self.commits = None; + self.pane.update(cx, |pane, cx| pane.clear(cx)); + cx.notify(); + return; + }; + if branches.is_empty() { + self.error = Some("The repository has no branches yet".into()); + self.repo_path = None; + self.branches.clear(); + cx.notify(); + return; + } + + // Defaults: the announced HEAD branch when the checkout has it + // (falling back to `main`, then the first branch); the checkout's + // current branch for the compare side. + let announced = self.store.read(cx).head.clone(); + let base = announced + .as_ref() + .filter(|branch| branches.contains(branch)) + .cloned() + .or_else(|| branches.iter().find(|branch| *branch == "main").cloned()) + .unwrap_or_else(|| branches[0].clone()); + let compare = current + .filter(|branch| branches.contains(branch)) + .unwrap_or_else(|| base.clone()); + + self.repo_path = Some(PathBuf::from(&path)); + self.error = None; + self.branches = branches.into_iter().map(SharedString::from).collect(); + + let branches = self.branches.clone(); + let base = SharedString::from(base.clone()); + let compare = SharedString::from(compare.clone()); + self.base = base.clone(); + self.compare = compare.clone(); + self.base_select.update(cx, |state, cx| { + state.set_items(SearchableVec::from(branches.clone()), window, cx); + state.set_selected_values(&[base], window, cx); + }); + self.compare_select.update(cx, |state, cx| { + state.set_items(SearchableVec::from(branches), window, cx); + state.set_selected_values(&[compare], window, cx); + }); + + self.reload_compare(window, cx); + } + + /// (Re)compute `merge_base..compare` of the selected branches on a + /// background task: the merge base, the commit list and the diff. + fn reload_compare(&mut self, window: &mut Window, cx: &mut Context) { + let Some(repo_path) = self.repo_path.clone() else { + return; + }; + let base = self.base.to_string(); + let compare = self.compare.to_string(); + + self.loading = true; + self.error = None; + self.compare_generation += 1; + let generation = self.compare_generation; + cx.notify(); + + if base == compare { + self.loading = false; + self.merge_base = None; + self.commits = None; + self.pane.update(cx, |pane, cx| pane.clear(cx)); + self.error = Some("Choose different base and compare branches".into()); + cx.notify(); + return; + } + + let task = cx.spawn_in(window, async move |this, cx| { + let result = cx + .background_spawn({ + let repo_path = repo_path.clone(); + let base = base.clone(); + let compare = compare.clone(); + async move { + let merge_base = merge_base(Path::new(&repo_path), &base, &compare)? + .ok_or_else(|| { + anyhow::anyhow!("{base} and {compare} share no common ancestor") + })?; + let commits = worktree_commit_range_commits( + Path::new(&repo_path), + &merge_base, + &compare, + )?; + let diff = worktree_commit_range_diff( + Path::new(&repo_path), + &merge_base, + &compare, + )?; + Ok::<_, anyhow::Error>((merge_base, commits, diff)) + } + }) + .await; + + this.update_in(cx, |this, _window, cx| { + // A stale result (the branches changed mid-flight) must not + // clobber a newer compare; the newer task clears the flag. + if generation != this.compare_generation { + return; + } + this.loading = false; + match result { + Ok((merge_base, commits, diff)) => { + this.merge_base = Some(merge_base); + let count = commits.len(); + this.item_sizes = Rc::new(vec![size(px(0.), px(COMMIT_ROW_HEIGHT)); count]); + this.commits = Some(commits); + this.pane.update(cx, |pane, cx| pane.set_diff(diff, cx)); + } + Err(error) => { + this.merge_base = None; + this.commits = None; + this.pane.update(cx, |pane, cx| pane.clear(cx)); + this.error = Some(error.to_string().into()); + } + } + cx.notify(); + })?; + + Ok(()) + }); + self.tasks.push(task); + } + + /// Publish the pull request: generate the patch series from the checkout + /// on a background task, hand it to the store, and close the panel once + /// the publish is underway (errors surface in the pull request list). + fn submit(&mut self, window: &mut Window, cx: &mut Context) { + if self.submitting || self.loading { + return; + } + let Some(repo_path) = self.repo_path.clone() else { + return; + }; + let Some(merge_base) = self.merge_base.clone() else { + return; + }; + let subject = self.subject.read(cx).value().to_string(); + let description = self.description.read(cx).value().to_string(); + let branch_name = self.compare.to_string(); + let store = self.store.clone(); + let dock_area = self.dock_area.clone(); + let entity = cx.entity().clone(); + + self.submitting = true; + self.error = None; + cx.notify(); + + let task = cx.spawn_in(window, async move |this, cx| { + // Regenerate the series at submit time so the published patch + // covers the current tip of the compare branch. + let patch = cx + .background_spawn({ + let repo_path = repo_path.clone(); + let merge_base = merge_base.clone(); + let branch_name = branch_name.clone(); + async move { + format_patch_between(Path::new(&repo_path), &merge_base, &branch_name) + } + }) + .await; + + let patch = match patch { + Ok(patch) if !patch.is_empty() => patch, + Ok(_) => { + this.update_in(cx, |this, _window, cx| { + this.submitting = false; + this.error = Some("No commits between the branches to propose".into()); + cx.notify(); + })?; + return Ok(()); + } + Err(error) => { + this.update_in(cx, |this, _window, cx| { + this.submitting = false; + this.error = Some(format!("Failed to generate the patch: {error}").into()); + cx.notify(); + })?; + return Ok(()); + } + }; + + this.update_in(cx, |this, window, cx| { + this.submitting = false; + store.update(cx, |store, cx| { + store.open_pull_request( + (!subject.is_empty()).then_some(subject), + description, + Some(branch_name), + patch, + false, + Some(merge_base), + Some(repo_path), + cx, + ); + }); + // Close the panel once the publish is underway. + cx.defer_in(window, { + let dock_area = dock_area.clone(); + let entity = entity.clone(); + move |_, window, cx| { + if let Some(dock_area) = dock_area.upgrade() { + dock_area.update(cx, |dock, cx| { + dock.remove_panel(entity, window, cx); + }); + } + } + }); + cx.notify(); + })?; + + Ok(()) + }); + self.tasks.push(task); + } + + /// Open the diff of `commit_id` (from the Commits tab) in a new panel. + fn open_commit_diff(&mut self, commit_id: &str, window: &mut Window, cx: &mut Context) { + let Some(repo_path) = self.repo_path.clone() else { + return; + }; + let Some(dock_area) = self.dock_area.upgrade() else { + return; + }; + + let panel = cx.new(|cx| { + CommitDiffView::new( + repo_path, + self.repo_name.clone(), + commit_id.into(), + window, + cx, + ) + }); + + dock_area.update(cx, |dock_area, cx| { + dock_area.add_panel_view(panel_handle(panel), DockPlacement::Center, None, window, cx); + }); + } + + /// The compare bar: base/compare selectors, the checkout chooser and the + /// Create button. + fn render_compare_bar(&self, cx: &mut Context) -> AnyElement { + let has_checkout = self.repo_path.is_some(); + let checkout = self.repo_path.clone(); + let can_submit = has_checkout + && !self.loading + && !self.submitting + && self.merge_base.is_some() + && self + .commits + .as_ref() + .is_some_and(|commits| !commits.is_empty()) + && !self.subject.read(cx).value().is_empty(); + + h_flex() + .px_4() + .h_12() + .w_full() + .gap_2() + .items_center() + .child( + div() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child("base"), + ) + .child( + div().w(px(140.)).child( + Combobox::new(&self.base_select) + .placeholder("branch") + .appearance(false) + .menu_width(px(220.)) + .disabled(!has_checkout) + .bg(cx.theme().muted) + .rounded(cx.theme().radius) + .render_trigger(|ctx, _window, cx| { + render_ref_trigger(ctx, CustomIconName::GitBranch, cx) + }), + ), + ) + .child( + Icon::new(IconName::ArrowRight) + .small() + .text_color(cx.theme().muted_foreground), + ) + .child( + div() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child("compare"), + ) + .child( + div().w(px(140.)).child( + Combobox::new(&self.compare_select) + .placeholder("branch") + .appearance(false) + .menu_width(px(220.)) + .disabled(!has_checkout) + .bg(cx.theme().muted) + .rounded(cx.theme().radius) + .render_trigger(|ctx, _window, cx| { + render_ref_trigger(ctx, CustomIconName::GitBranch, cx) + }), + ), + ) + .child( + Button::new("choose-checkout") + .icon(IconName::Folder) + .ghost() + .tooltip(checkout.as_ref().map_or_else( + || "Choose a local checkout".into(), + |path| path.display().to_string(), + )) + .on_click(cx.listener(|this, _event, window, cx| { + this.choose_checkout(window, cx); + })), + ) + .child(div().flex_1()) + .child( + Button::new("create-pr") + .primary() + .label("Create pull request") + .loading(self.submitting) + .disabled(!can_submit) + .on_click(cx.listener(|this, _event, window, cx| { + this.submit(window, cx); + })), + ) + .into_any_element() + } + + /// The title and description inputs. + fn render_inputs(&self, _cx: &mut Context) -> AnyElement { + v_flex() + .px_4() + .py_2() + .w_full() + .gap_2() + .child(Input::new(&self.subject)) + .child(Textarea::new(&self.description).h_32()) + .into_any_element() + } + + /// The Files/Commits tab bar, mirroring the repository panel's. + fn render_tabs(&self, cx: &mut Context) -> AnyElement { + let files = self.pane.read(cx).diff().map_or(0, |diff| diff.files.len()); + let commits = self.commits.as_ref().map_or(0, |commits| commits.len()); + + h_flex() + .px_4() + .h_9() + .w_full() + .gap_2() + .items_center() + .border_b_1() + .border_color(cx.theme().border) + .child( + BaseButton::new("files-tab") + .flex() + .items_center() + .h_8() + .px_2() + .gap_2() + .child( + h_flex() + .gap_1() + .text_sm() + .child(Icon::new(CustomIconName::GitFile).small()) + .child("Files"), + ) + .child(count_badge(files, cx)) + .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.active_tab == 0) + .when(self.active_tab == 0, |this| { + this.bg(cx.theme().button_active) + }) + .on_click(cx.listener(|this, _event, _window, cx| { + this.active_tab = 0; + cx.notify(); + })), + ) + .child( + BaseButton::new("commits-tab") + .flex() + .items_center() + .h_8() + .px_2() + .gap_2() + .child( + h_flex() + .gap_1() + .text_sm() + .child(Icon::new(CustomIconName::GitCommit).small()) + .child("Commits"), + ) + .child(count_badge(commits, cx)) + .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.active_tab == 1) + .when(self.active_tab == 1, |this| { + this.bg(cx.theme().button_active) + }) + .on_click(cx.listener(|this, _event, _window, cx| { + this.active_tab = 1; + cx.notify(); + })), + ) + .into_any_element() + } + + /// The active tab's body. + fn render_content(&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 self.repo_path.is_none() { + return placeholder("Choose a local checkout to compare branches", cx); + } + if self.commits.is_none() && self.error.is_some() { + return placeholder("Nothing to compare", cx); + } + match self.active_tab { + 0 => self.pane.clone().into_any_element(), + _ => self.render_commits_tab(cx), + } + } + + /// The Commits tab: `merge_base..compare` in a virtual list; clicking a + /// row opens the commit's diff in a new panel. + fn render_commits_tab(&self, cx: &mut Context) -> AnyElement { + let Some(commits) = self.commits.as_ref() else { + return placeholder("No commits", cx); + }; + if commits.is_empty() { + return placeholder("No commits between the branches", cx); + } + + let view = cx.entity().clone(); + let sizes = self.item_sizes.clone(); + let scroll_handle = self.scroll_handle.clone(); + + v_flex() + .relative() + .flex_1() + .w_full() + .min_h_0() + .child( + v_virtual_list( + view, + "pr-commits", + sizes, + move |this, range, _window, cx| { + let commits = this.commits.as_deref().unwrap_or(&[]); + let view = cx.entity().downgrade(); + range + .map(|ix| { + let id = commits[ix].id.clone(); + let view = view.clone(); + commit_row( + ix, + &commits[ix], + move |window, cx| { + if let Some(view) = view.upgrade() { + view.update(cx, |this, cx| { + this.open_commit_diff(&id, window, cx) + }); + } + }, + 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() + } +} + +/// The count badge of a tab, styled like the repository panel's. +fn count_badge(count: usize, cx: &App) -> impl IntoElement { + h_flex() + .justify_center() + .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(count.to_string())) +} + +/// The trigger of a branch selector: icon + current selection (or +/// placeholder) + caret. `Combobox` replaces its default trigger entirely. +fn render_ref_trigger( + ctx: &ComboboxTriggerContext>, + icon: CustomIconName, + cx: &App, +) -> AnyElement { + let muted = cx.theme().muted_foreground; + + h_flex() + .w_full() + .min_w_0() + .gap_1() + .items_center() + .child(Icon::new(icon).small().flex_shrink_0()) + .child( + div() + .flex_1() + .min_w_0() + .overflow_hidden() + .text_ellipsis() + .whitespace_nowrap() + .when(ctx.selection().is_empty(), |this| this.text_color(muted)) + .child( + ctx.selection() + .first() + .map(|(_, item)| item.clone()) + .or_else(|| ctx.placeholder().cloned()) + .unwrap_or_default(), + ), + ) + .child(Caret::new(ctx.size()).text_color(muted)) + .into_any_element() +} + +/// Open the "new pull request" panel for `store` in the center dock. +pub(super) fn open_new_pull_request_panel( + dock_area: WeakEntity, + store: Entity, + repo_name: SharedString, + window: &mut Window, + cx: &mut App, +) { + let panel = + cx.new(|cx| NewPullRequestView::new(dock_area.clone(), store, repo_name, window, cx)); + + let _ = dock_area.update(cx, |dock_area, cx| { + dock_area.add_panel_view(panel_handle(panel), DockPlacement::Center, None, window, cx); + }); +} + +impl BasePanel for NewPullRequestView { + fn panel_name(&self) -> &'static str { + "new-pull-request" + } +} + +impl Panel for NewPullRequestView { + fn title(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + div().child(SharedString::from(format!( + "{}/new-pull-request", + self.repo_name + ))) + } +} + +impl EventEmitter for NewPullRequestView {} + +impl Focusable for NewPullRequestView { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for NewPullRequestView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + v_flex() + .id("new-pr") + .size_full() + .child(self.render_compare_bar(cx)) + .child(self.render_inputs(cx)) + .when_some(self.error.clone(), |this, error| { + this.child( + h_flex() + .px_4() + .py_1() + .w_full() + .text_xs() + .text_color(cx.theme().danger) + .child(error), + ) + }) + .child(self.render_tabs(cx)) + .child( + v_flex() + .flex_1() + .min_h_0() + .w_full() + .child(self.render_content(cx)), + ) + } +} diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index 916edbf..269cbda 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -1,32 +1,23 @@ -use std::path::{Path, PathBuf}; use std::rc::Rc; use assets::CustomIconName; use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; use gpui::prelude::*; use gpui::{ - AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, PathPromptOptions, - Pixels, Render, SharedString, Size, WeakEntity, Window, div, px, size, + AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, + SharedString, Size, WeakEntity, Window, div, px, size, }; use gpui_component::alert::Alert; -use gpui_component::button::{Button, ButtonVariants}; -use gpui_component::checkbox::Checkbox; -use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; -use gpui_component::form::{field, v_form}; -use gpui_component::input::{Input, InputState, Textarea, TextareaState}; use gpui_component::scroll::Scrollbar; -use gpui_component::{ - ActiveTheme, Disableable, Icon, IconName, VirtualListScrollHandle, WindowExt, h_flex, v_flex, - v_virtual_list, -}; +use gpui_component::{ActiveTheme, Icon, VirtualListScrollHandle, h_flex, v_flex, v_virtual_list}; use nostr::prelude::{EventId, Kind}; use signed_core::{RepoStatus, activity_subject}; -use signed_git::{format_patch_between, merge_base, patch_applies}; -use signed_state::{GitStore, ProfileStore, RepoStore}; +use signed_state::{ProfileStore, RepoStore}; use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{SegmentButton, UserAvatar, placeholder, status_badge}; use utils::relative_time; +use super::new_pull_request::open_new_pull_request_panel; use super::pull_request_detail::PullRequestDetailView; /// Height of one pull request row in the virtual list; same layout as an @@ -280,512 +271,19 @@ impl PullRequestsView { .icon(Icon::new(CustomIconName::CirclePlus)) .primary() .on_click(cx.listener(|this, _event, window, cx| { - open_new_pull_request_dialog(this.store.clone(), window, cx); + open_new_pull_request_panel( + this.dock_area.clone(), + this.store.clone(), + this.repo_name.clone(), + window, + cx, + ); })), ) .into_any_element() } } -/// A patch series generated from a local repository, with the metadata -/// derived from it. -struct GeneratedPatch { - /// The `git format-patch` series (fills the patch textarea). - patch: String, - /// The merge base with the target branch, as hex. - merge_base: Option, -} - -/// State of the new pull request dialog, so the async generation, the -/// apply check and the draft checkbox re-render. -#[derive(Default)] -struct NewPullRequestDialogState { - draft: bool, - /// The last generated patch series; its merge base is reused at submit - /// only while the patch textarea is unchanged. - generated: Option, - /// Result of the pre-publish applicability check against the app's - /// mirror clone of the target repository. - apply_check: Option>, - /// A patch generation is in flight. - generating: bool, - /// Error of the last generation attempt. - error: Option, -} - -impl NewPullRequestDialogState { - /// Text and whether it is good news, for the line under the patch field. - fn apply_check_message(&self) -> Option<(SharedString, bool)> { - match &self.apply_check { - Some(Ok(())) => Some(( - "Applies cleanly to the repository's default branch".into(), - true, - )), - Some(Err(error)) => Some(( - format!("May not apply cleanly to the repository's default branch: {error}").into(), - false, - )), - None => None, - } - } -} - -/// Open the "new pull request" dialog: a title, an optional description, -/// an optional branch name and a patch input that submit through -/// [`RepoStore::open_pull_request`] when confirmed. The patch can either be -/// pasted, or generated from a local checkout: pick a repository, a source -/// and a target branch, and the app runs `git format-patch` itself and -/// checks the series against the app's mirror clone of the target. -pub(super) 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 branch = cx.new(|cx| InputState::new(window, cx).placeholder("Branch name (optional)")); - let repo_path = cx.new(|cx| InputState::new(window, cx).placeholder("Pick a local checkout…")); - let source = cx.new(|cx| InputState::new(window, cx).placeholder("Source branch")); - let target = cx.new(|cx| InputState::new(window, cx).placeholder("Target branch")); - let patch = cx - .new(|cx| TextareaState::new(window, cx).placeholder("Paste `git format-patch` output...")); - let state = cx.new(|_| NewPullRequestDialogState::default()); - - window.open_dialog(cx, move |dialog, _window, _cx| { - let subject = subject.clone(); - let description = description.clone(); - let branch = branch.clone(); - let repo_path = repo_path.clone(); - let source = source.clone(); - let target = target.clone(); - let patch = patch.clone(); - let store = store.clone(); - let state = state.clone(); - - dialog - .width(px(560.)) - .margin_top(px(50.)) - .content(move |body, _window, cx| { - let generating = state.read(cx).generating; - let draft = state.read(cx).draft; - let error = state.read(cx).error.clone(); - let apply_check = state.read(cx).apply_check_message(); - body.child( - DialogHeader::new() - .child(DialogTitle::new().child("New pull request")) - .child( - DialogDescription::new().child( - "Propose a change with the output of `git format-patch`.", - ), - ), - ) - .child( - v_form() - .child( - field() - .label("Title") - .required(true) - .child(Input::new(&subject)), - ) - .child( - field() - .label("Description") - .child(Textarea::new(&description).h(px(96.))), - ) - .child( - field() - .label("Local repository") - .description( - "Generate the patch from a local checkout; leave empty to paste it", - ) - .child( - h_flex() - .gap_1() - .items_center() - .child( - div() - .flex_1() - .child(Input::new(&repo_path).disabled(true)), - ) - .child( - Button::new("choose-checkout") - .icon(IconName::FolderOpen) - .ghost() - .tooltip("Choose local checkout") - .on_click({ - let repo_path = repo_path.clone(); - let source = source.clone(); - let target = target.clone(); - let patch = patch.clone(); - let branch = branch.clone(); - let state = state.clone(); - let store = store.clone(); - move |_ev, window, cx| { - choose_local_repo( - &repo_path, - &source, - &target, - &patch, - &branch, - &state, - &store, - window, - cx, - ); - } - }), - ) - .child( - Button::new("generate-patch") - .ghost() - .label("Generate") - .tooltip( - "Generate the patch from the local checkout", - ) - .loading(generating) - .disabled(generating) - .on_click({ - let repo_path = repo_path.clone(); - let source = source.clone(); - let target = target.clone(); - let patch = patch.clone(); - let branch = branch.clone(); - let state = state.clone(); - let store = store.clone(); - move |_ev, window, cx| { - let path = - repo_path.read(cx).value().to_string(); - let source = - source.read(cx).value().to_string(); - let target = - target.read(cx).value().to_string(); - if !path.is_empty() - && !source.is_empty() - && !target.is_empty() - { - generate_patch( - &state, - &patch, - &branch, - path, - source, - target, - &store, - window, - cx, - ); - } - } - }), - ), - ), - ) - .child( - field() - .label("Source branch") - .child(Input::new(&source)), - ) - .child( - field() - .label("Target branch") - .child(Input::new(&target)), - ) - .child( - field() - .label("Branch") - .description("Optional: the branch the change is proposed from") - .child(Input::new(&branch)), - ) - .child( - field().label("Patch").child( - v_flex() - .gap_1() - .child(Textarea::new(&patch).h(px(140.))) - .when_some(apply_check, |this, (message, ok)| { - this.child( - div() - .text_xs() - .text_color(if ok { - cx.theme().success - } else { - cx.theme().warning - }) - .child(message), - ) - }) - .when_some(error, |this, message| { - this.child( - div() - .text_xs() - .text_color(cx.theme().danger) - .child(message), - ) - }), - ), - ) - .child( - field().child( - Checkbox::new("pr-draft") - .label("Create as draft") - .checked(draft) - .on_click({ - let state = state.clone(); - move |checked, _window, cx| { - state.update(cx, |state, _| state.draft = *checked); - } - }), - ), - ), - ) - .child( - DialogFooter::new().justify_end().child( - Button::new("submit") - .primary() - .label("Create pull request") - .tooltip("Create pull request") - .loading(generating) - .disabled(generating) - .on_click({ - let subject = subject.clone(); - let description = description.clone(); - let branch = branch.clone(); - let patch = patch.clone(); - let repo_path = repo_path.clone(); - let store = store.clone(); - let state = state.clone(); - - move |_event, window, cx| { - if state.read(cx).generating { - return; - } - let subject = subject.read(cx).value().to_string(); - let description = description.read(cx).value().to_string(); - let branch = branch.read(cx).value().to_string(); - let patch = patch.read(cx).value().to_string(); - let subject = (!subject.is_empty()).then_some(subject); - let branch = (!branch.is_empty()).then_some(branch); - let draft = state.read(cx).draft; - // The generated merge base stays valid - // only while the patch is unchanged; an - // edited patch falls back to none. - let merge_base = state - .read(cx) - .generated - .as_ref() - .filter(|generated| generated.patch == patch) - .and_then(|generated| generated.merge_base.clone()); - // The checkout (when set) is where the - // tip commit is pushed from, so other - // clients can fetch it. - let repo_path = repo_path.read(cx).value().to_string(); - let push_from = (!repo_path.is_empty()) - .then(|| PathBuf::from(repo_path)); - - store.update(cx, |store, cx| { - store.open_pull_request( - subject, - description, - branch, - patch, - draft, - merge_base, - push_from, - cx, - ); - }); - - window.close_dialog(cx); - } - }), - ), - ) - }) - }); -} - -/// Prompt for a local checkout, fill the source/target defaults (the -/// checkout's current branch and the repository's announced HEAD) and -/// generate the patch series right away. -#[allow(clippy::too_many_arguments)] -fn choose_local_repo( - repo_path: &Entity, - source: &Entity, - target: &Entity, - patch: &Entity, - branch: &Entity, - state: &Entity, - store: &Entity, - window: &mut Window, - cx: &mut App, -) { - let handle = window.window_handle(); - let repo_path = repo_path.clone(); - let source = source.clone(); - let target = target.clone(); - let patch = patch.clone(); - let branch = branch.clone(); - let state = state.clone(); - let store = store.clone(); - // The announced HEAD branch is the natural target default. - let target_default = store.read(cx).head.clone().unwrap_or_default(); - - let prompt = cx.prompt_for_paths(PathPromptOptions { - files: false, - directories: true, - multiple: false, - prompt: Some("Choose local checkout".into()), - }); - - cx.spawn(async move |cx| { - if let Ok(Ok(Some(mut paths))) = prompt.await - && let Some(path) = paths.pop() - { - let path = path.to_string_lossy().to_string(); - - // The checkout's current branch is the source default; resolve - // it off the UI thread. - let current = cx - .background_executor() - .spawn({ - let path = path.clone(); - async move { - gix::open(Path::new(&path)) - .ok() - .and_then(|repo| signed_git::current_branch(&repo).ok().flatten()) - } - }) - .await; - - let _ = handle.update(cx, |_, window, cx| { - repo_path.update(cx, |input, cx| { - input.set_value(path.clone(), window, cx); - }); - source.update(cx, |input, cx| { - input.set_value(current.clone().unwrap_or_default(), window, cx); - }); - target.update(cx, |input, cx| { - input.set_value(target_default.clone(), window, cx); - }); - - if let Some(current) = current - && !current.is_empty() - && !target_default.is_empty() - { - generate_patch( - &state, - &patch, - &branch, - path, - current, - target_default, - &store, - window, - cx, - ); - } - }); - } - }) - .detach(); -} - -/// Generate the patch series `source..target` of the local checkout at -/// `repo_path`, fill the patch textarea and record the merge base and the -/// pre-publish applicability check in `state`. -#[allow(clippy::too_many_arguments)] -fn generate_patch( - state: &Entity, - patch_input: &Entity, - branch_input: &Entity, - repo_path: String, - source: String, - target: String, - store: &Entity, - window: &mut Window, - cx: &mut App, -) { - state.update(cx, |state, cx| { - state.generating = true; - state.error = None; - state.apply_check = None; - cx.notify(); - }); - - let cache = GitStore::global(cx).cache().clone(); - let (addr, clone_urls) = { - let store = store.read(cx); - ( - store.addr().clone(), - store - .announcement - .as_ref() - .map(|a| { - a.clone - .iter() - .map(ToString::to_string) - .collect::>() - }) - .unwrap_or_default(), - ) - }; - - let handle = window.window_handle(); - let state = state.clone(); - let patch_input = patch_input.clone(); - let branch_input = branch_input.clone(); - - let task = cx.spawn(async move |cx| { - // The branch-name tag defaults to the source branch; keep a copy - // for the UI update after the background generation moves it. - let source_label = source.clone(); - let generated = cx - .background_executor() - .spawn(async move { - let base = - merge_base(Path::new(&repo_path), &source, &target)?.ok_or_else(|| { - anyhow::anyhow!("{source} and {target} share no common ancestor") - })?; - let patch = format_patch_between(Path::new(&repo_path), &base, &source)?; - // Best-effort: does the series apply to the current default - // branch of the app's mirror clone of the target repository? - let check = cache - .ensure_clone(&addr, &clone_urls) - .ok() - .and_then(|repo| repo.workdir().map(|workdir| workdir.to_path_buf())) - .map(|workdir| patch_applies(&workdir, &patch).map_err(|e| e.to_string())); - Ok::<_, anyhow::Error>((patch, Some(base), check)) - }) - .await; - - let _ = handle.update(cx, |_, window, cx| match generated { - Ok((patch, merge_base, check)) => { - patch_input.update(cx, |input, cx| { - input.set_value(patch.clone(), window, cx); - }); - // The branch-name tag defaults to the source branch. - if branch_input.read(cx).value().is_empty() { - branch_input.update(cx, |input, cx| { - input.set_value(source_label.clone(), window, cx); - }); - } - state.update(cx, |state, cx| { - state.generating = false; - state.generated = Some(GeneratedPatch { patch, merge_base }); - state.apply_check = check; - cx.notify(); - }); - } - Err(error) => state.update(cx, |state, cx| { - state.generating = false; - state.error = Some(error.to_string().into()); - cx.notify(); - }), - }); - }); - task.detach(); -} - impl BasePanel for PullRequestsView { fn panel_name(&self) -> &'static str { "pull-requests" diff --git a/docs/TODO.md b/docs/TODO.md index 47df0cd..f822530 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -8,10 +8,19 @@ ## Pull request improvement -- [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog. +### New pull request panel (replaces the dialog) + +- [x] "New pull request" (PR list header + repo header `New PR`) opens a center panel instead of the paste dialog: + - [x] Base/compare branch selectors fed from a user-chosen local checkout (GitHub-style; defaults: announced HEAD for base, checkout's current branch for compare). + - [x] Files/Commits tabs like the repo panel: diff of `merge-base..compare` (shared `DiffPane` widget, also extracted for the commit diff panel) + virtual commit list with count badge; clicking a commit opens its diff panel. + - [x] Only two inputs: title (required, gates the Create button) and description (optional). + - [x] Patch is generated from the checkout at submit time (`format_patch_between` on the stored merge base); panel closes after publishing, errors surface in the PR list banner. +- [x] Removed with the dialog: paste textarea, draft checkbox, branch-name input and the mirror-clone apply-check hint (store behavior unchanged: `open_pull_request` still publishes the series + `branch-name`/`merge-base`/`r` tags and pushes the tip). + +- [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog (dialog since replaced by the panel above). - [x] P1: `RepoStore::update_pull_request` (kind 1619 + root-revision patch) with an author-only "Update" button on the PR detail header. - [x] P1: `latest_update` filters by PR author. -- [x] P2: local checkout picker in the new-PR dialog (folder picker + source/target branches + Generate): `signed_git::{merge_base, format_patch_between, patch_applies}`; `merge-base` tag now published; best-effort apply check shown under the patch field. +- [x] P2: local checkout picker in the new-PR dialog (folder picker + source/target branches + Generate): `signed_git::{merge_base, format_patch_between, patch_applies}`; `merge-base` tag now published; best-effort apply check shown under the patch field (superseded by the panel's live compare view). - [x] P3: push tip to grasp servers under `refs/nostr/` before publishing (from the local checkout); multi-commit series published as NIP-10-chained 1617 events with a 60 KB per-patch cap; PR list shows dismissible error/warning banners (incl. push failures). - [x] P4: merge status tags — `merge_pull_request` publishes 1631 with `applied-as-commits` + `r` per applied commit and `q`/`e`-reply tags per applied patch event. -- 2.54.0 From c03ce50c826e39213dfc71519ed6de783eb92f90 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 13:51:45 +0700 Subject: [PATCH 7/9] update ui --- .../src/views/repo_detail/new_pull_request.rs | 194 +++++++++--------- 1 file changed, 102 insertions(+), 92 deletions(-) diff --git a/crates/workspace/src/views/repo_detail/new_pull_request.rs b/crates/workspace/src/views/repo_detail/new_pull_request.rs index 81c4d9c..f7362c1 100644 --- a/crates/workspace/src/views/repo_detail/new_pull_request.rs +++ b/crates/workspace/src/views/repo_detail/new_pull_request.rs @@ -15,7 +15,7 @@ use gpui::{ Pixels, Render, SharedString, Size, Subscription, Task, WeakEntity, Window, div, px, relative, size, }; -use gpui_base::Button as BaseButton; +use gpui_base::{Button as BaseButton, StyledExt}; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::combobox::{ Caret, Combobox, ComboboxEvent, ComboboxState, ComboboxTriggerContext, @@ -97,10 +97,9 @@ impl NewPullRequestView { window: &mut Window, cx: &mut Context, ) -> Self { - 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 (optional)")); let pane = cx.new(DiffPane::new); + let subject = cx.new(|cx| InputState::new(window, cx).placeholder("Title")); + let description = cx.new(|cx| TextareaState::new(window, cx).placeholder("Describe...")); let base_select: Entity>> = cx.new(|cx| { ComboboxState::new( @@ -111,6 +110,7 @@ impl NewPullRequestView { ) .searchable(true) }); + let compare_select: Entity>> = cx.new(|cx| { ComboboxState::new( SearchableVec::new(Vec::::new()), @@ -181,8 +181,6 @@ impl NewPullRequestView { /// (defaults: the announced HEAD branch for the base, the checkout's /// current branch for the compare) and load the compare. fn choose_checkout(&mut self, window: &mut Window, cx: &mut Context) { - let handle = window.window_handle(); - let prompt = cx.prompt_for_paths(PathPromptOptions { files: false, directories: true, @@ -190,32 +188,36 @@ impl NewPullRequestView { prompt: Some("Choose local checkout".into()), }); - let task = cx.spawn(async move |this, cx| { - if let Ok(Ok(Some(mut paths))) = prompt.await - && let Some(path) = paths.pop() - { - let path = path.to_string_lossy().to_string(); - // Branches and the current branch are read off the UI thread. - let info = cx - .background_executor() - .spawn({ - let path = path.clone(); - async move { - let repo = gix::open(Path::new(&path)).ok()?; - let branches = - signed_git::worktree_branches(Path::new(&path)).unwrap_or_default(); - let current = signed_git::current_branch(&repo).ok().flatten(); - Some((branches, current)) - } - }) - .await; + let task = cx.spawn_in(window, async move |this, cx| { + // `Ok(Ok(Some(paths)))` means the user picked a folder; a + // cancel (or a picker failure) resolves to anything else. + let picked = match prompt.await { + Ok(Ok(Some(mut paths))) => paths.pop(), + _ => None, + }; + let Some(path) = picked else { + return Ok(()); + }; + let path = path.to_string_lossy().to_string(); + + // Branches and the current branch are read off the UI thread. + let info = cx + .background_spawn({ + let path = path.clone(); + async move { + let repo = gix::open(Path::new(&path)).ok()?; + let branches = + signed_git::worktree_branches(Path::new(&path)).unwrap_or_default(); + let current = signed_git::current_branch(&repo).ok().flatten(); + Some((branches, current)) + } + }) + .await; + + this.update_in(cx, |this, window, cx| { + this.apply_checkout(path, info, window, cx); + })?; - let _ = handle.update(cx, |_, window, cx| { - let _ = this.update(cx, |this, cx| { - this.apply_checkout(path, info, window, cx); - }); - }); - } Ok(()) }); self.tasks.push(task); @@ -496,54 +498,59 @@ impl NewPullRequestView { h_flex() .px_4() - .h_12() + .h_16() .w_full() .gap_2() - .items_center() + .items_end() .child( - div() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child("base"), + v_flex() + .gap_1() + .child( + div() + .text_xs() + .font_semibold() + .text_color(cx.theme().muted_foreground) + .child("Merge Into"), + ) + .child( + div().w(px(140.)).child( + Combobox::new(&self.base_select) + .placeholder("branch") + .appearance(false) + .menu_width(px(220.)) + .disabled(!has_checkout) + .bg(cx.theme().muted) + .rounded(cx.theme().radius) + .render_trigger(|ctx, _window, cx| { + render_ref_trigger(ctx, CustomIconName::GitBranch, cx) + }), + ), + ), ) .child( - div().w(px(140.)).child( - Combobox::new(&self.base_select) - .placeholder("branch") - .appearance(false) - .menu_width(px(220.)) - .disabled(!has_checkout) - .bg(cx.theme().muted) - .rounded(cx.theme().radius) - .render_trigger(|ctx, _window, cx| { - render_ref_trigger(ctx, CustomIconName::GitBranch, cx) - }), - ), - ) - .child( - Icon::new(IconName::ArrowRight) - .small() - .text_color(cx.theme().muted_foreground), - ) - .child( - div() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child("compare"), - ) - .child( - div().w(px(140.)).child( - Combobox::new(&self.compare_select) - .placeholder("branch") - .appearance(false) - .menu_width(px(220.)) - .disabled(!has_checkout) - .bg(cx.theme().muted) - .rounded(cx.theme().radius) - .render_trigger(|ctx, _window, cx| { - render_ref_trigger(ctx, CustomIconName::GitBranch, cx) - }), - ), + v_flex() + .gap_1() + .child( + div() + .text_xs() + .font_semibold() + .text_color(cx.theme().muted_foreground) + .child("Pull From"), + ) + .child( + div().w(px(140.)).child( + Combobox::new(&self.compare_select) + .placeholder("branch") + .appearance(false) + .menu_width(px(220.)) + .disabled(!has_checkout) + .bg(cx.theme().muted) + .rounded(cx.theme().radius) + .render_trigger(|ctx, _window, cx| { + render_ref_trigger(ctx, CustomIconName::GitBranch, cx) + }), + ), + ), ) .child( Button::new("choose-checkout") @@ -561,7 +568,7 @@ impl NewPullRequestView { .child( Button::new("create-pr") .primary() - .label("Create pull request") + .icon(IconName::Plus) .loading(self.submitting) .disabled(!can_submit) .on_click(cx.listener(|this, _event, window, cx| { @@ -575,11 +582,10 @@ impl NewPullRequestView { fn render_inputs(&self, _cx: &mut Context) -> AnyElement { v_flex() .px_4() - .py_2() .w_full() .gap_2() .child(Input::new(&self.subject)) - .child(Textarea::new(&self.description).h_32()) + .child(Textarea::new(&self.description).h_24()) .into_any_element() } @@ -590,7 +596,7 @@ impl NewPullRequestView { h_flex() .px_4() - .h_9() + .pb_4() .w_full() .gap_2() .items_center() @@ -834,20 +840,24 @@ impl Render for NewPullRequestView { v_flex() .id("new-pr") .size_full() - .child(self.render_compare_bar(cx)) - .child(self.render_inputs(cx)) - .when_some(self.error.clone(), |this, error| { - this.child( - h_flex() - .px_4() - .py_1() - .w_full() - .text_xs() - .text_color(cx.theme().danger) - .child(error), - ) - }) - .child(self.render_tabs(cx)) + .child( + v_flex() + .gap_4() + .child(self.render_compare_bar(cx)) + .child(self.render_inputs(cx)) + .when_some(self.error.clone(), |this, error| { + this.child( + h_flex() + .px_4() + .py_1() + .w_full() + .text_xs() + .text_color(cx.theme().danger) + .child(error), + ) + }) + .child(self.render_tabs(cx)), + ) .child( v_flex() .flex_1() -- 2.54.0 From b9054346f71fd843c874e8bf257399a839b30077 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 15:26:50 +0700 Subject: [PATCH 8/9] add send patch panel --- crates/signed_core/src/filters.rs | 2 +- crates/signed_core/src/model.rs | 8 +- crates/signed_state/src/repo.rs | 109 +++---- crates/signed_ui/src/status_badge.rs | 12 +- crates/workspace/src/views/repo_detail/mod.rs | 36 ++- .../src/views/repo_detail/new_pull_request.rs | 10 +- .../src/views/repo_detail/pull_requests.rs | 69 +++-- .../src/views/repo_detail/send_patch.rs | 265 ++++++++++++++++++ docs/TODO.md | 5 + 9 files changed, 422 insertions(+), 94 deletions(-) create mode 100644 crates/workspace/src/views/repo_detail/send_patch.rs diff --git a/crates/signed_core/src/filters.rs b/crates/signed_core/src/filters.rs index 6291ee5..b8f3253 100644 --- a/crates/signed_core/src/filters.rs +++ b/crates/signed_core/src/filters.rs @@ -6,11 +6,11 @@ use crate::RepoAddr; /// Kinds that make up the activity of a repository. pub const ACTIVITY_KINDS: [Kind; 9] = [ + Kind::Comment, Kind::GitPatch, Kind::GitPullRequest, Kind::GitPullRequestUpdate, Kind::GitIssue, - Kind::Comment, Kind::GitStatusOpen, Kind::GitStatusApplied, Kind::GitStatusClosed, diff --git a/crates/signed_core/src/model.rs b/crates/signed_core/src/model.rs index f2a712f..7a33340 100644 --- a/crates/signed_core/src/model.rs +++ b/crates/signed_core/src/model.rs @@ -35,10 +35,10 @@ pub struct Announcement { pub hashtags: Vec, } -/// The `u` tag of a fork announcement (NIP-34): the repository this one is a -/// subordinate fork of. The first value is the upstream coordinate -/// (`30617::`) or a git URL; the second is an optional relay hint -/// for the upstream. +/// The `u` tag of a fork announcement (NIP-34) +/// the repository this one is a subordinate fork of. The first value is +/// the upstream coordinate (`30617::`) or a git URL. +/// The second is an optional relay hint for the upstream. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Upstream { /// Raw first value of the `u` tag (coordinate or git URL). diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index ed66c7c..3f482e3 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -5,7 +5,7 @@ use std::time::Duration; use anyhow::Error; use bitcoin_hashes::sha1::Hash as Sha1Hash; -use gpui::{AppContext, AsyncApp, Context, Subscription, Task, WeakEntity}; +use gpui::{AppContext, AsyncApp, Context, SharedString, Subscription, Task, WeakEntity}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; use signed_core::{ @@ -157,12 +157,22 @@ impl RepoStore { store } + /// Returns the repository's address. pub fn addr(&self) -> &RepoAddr { &self.addr } - /// Filters that make up a repository: announcement, state, activity and - /// deletions targeting it. + /// Returns the repository's name, or "Unknown" if not known. + pub fn name(&self) -> SharedString { + self.announcement + .as_ref() + .map_or(SharedString::default(), |a| { + a.name.clone().unwrap_or(SharedString::from("Unknown")) + }) + } + + /// Filters that make up a repository: announcement, state, + /// activity and deletions targeting it. fn repo_filters(addr: &RepoAddr) -> Vec { let mut filters = vec![ // Announcement and state share author and identifier, so they @@ -202,8 +212,7 @@ impl RepoStore { }); } - /// Fetch this repository's events from the bootstrap relays (one-shot, - /// auto-closing subscription). + /// Fetch this repository's events from the bootstrap relays fn subscribe_remote(&mut self, cx: &mut Context) { let backend = Backend::global(cx); let addr = self.addr.clone(); @@ -215,10 +224,8 @@ impl RepoStore { /// Re-query the local database and update all fields. /// - /// Debounced: a short delay collapses bursts of requests (e.g. per-event - /// `NostrUpdate`s), and requests that arrive while a query is running are - /// folded into one follow-up query. The query and processing run on a - /// background thread; only the results are applied on the main thread. + /// The query and processing run on a background thread, + /// only the results are applied on the main thread. pub fn refresh(&mut self, cx: &mut Context) { if self.refreshing { self.refresh_dirty = true; @@ -242,7 +249,6 @@ impl RepoStore { self.tasks.push(task); } - /// One query + apply cycle (debounced entry point). fn run_refresh(&mut self, cx: &mut Context) { self.refreshing = true; @@ -297,13 +303,15 @@ impl RepoStore { // NIP-22 comments reference their root via an `E`/`e` tag rather // than the repository's `a` tag, so query them by the root events // of this repository. - let db = client.database(); let mut seen_comments: HashSet = comments.iter().map(|e| e.id).collect(); + let db = client.database(); + let roots = issues .iter() .chain(&patches) .chain(&pull_requests) .map(|e| e.id); + for filter in filters::comments_for(roots) { for event in db.query(filter).await? { if seen_comments.insert(event.id) { @@ -312,16 +320,17 @@ impl RepoStore { } } - // Status events may omit their `a` tag (NIP-34 makes it - // optional), so also query them by the root events they - // reference. - let db = client.database(); + // Status events may omit their `a` tag, + // so also query them by the root events they reference. let mut seen_statuses: HashSet = statuses.iter().map(|e| e.id).collect(); + let db = client.database(); + let roots = issues .iter() .chain(&patches) .chain(&pull_requests) .map(|e| e.id); + for root in roots { for event in db.query(filters::statuses_for([root])).await? { if seen_statuses.insert(event.id) { @@ -330,17 +339,18 @@ impl RepoStore { } } - // Cover notes (1624) and label events (1985) reference their - // target via an `e` tag, so query them per root like comments - // and statuses. - let db = client.database(); + // Cover notes (1624) and label events (1985) reference + // so query them per root like comments and statuses. let mut seen_cover_notes: HashSet = cover_notes.iter().map(|e| e.id).collect(); let mut seen_labels: HashSet = labels.iter().map(|e| e.id).collect(); + let db = client.database(); + let roots = issues .iter() .chain(&patches) .chain(&pull_requests) .map(|e| e.id); + for root in roots { for event in db.query(filters::annotations_for([root])).await? { if deletions.is_deleted(&event) { @@ -368,12 +378,15 @@ impl RepoStore { .as_ref() .map(Announcement::effective_maintainers) .unwrap_or_default(); + let status_by_root = resolve_statuses(&issues, &patches, &pull_requests, &statuses, &maintainers); + let open_issue_count = issues .iter() .filter(|issue| status_of(&status_by_root, issue) == RepoStatus::Open) .count(); + let open_pr_count = pull_requests .iter() .filter(|pr| { @@ -426,8 +439,8 @@ impl RepoStore { let again = this.update(cx, |this, cx| { this.announcement = announcement; - // The announcement may list relays for this repository's - // activity; connect to any we haven't fetched from yet. + // The announcement may list relays for this repository's activity, + // connect to any we haven't fetched from yet. let relays = this .announcement .as_ref() @@ -462,11 +475,13 @@ impl RepoStore { .chain(&this.pull_requests) .map(|e| e.id) .collect::>(); + let new_roots: Vec = roots .iter() .filter(|id| !this.root_fetches.contains(id)) .copied() .collect(); + if !new_roots.is_empty() { this.root_fetches.extend(new_roots.iter().copied()); // Batch the per-root filters: one statuses filter and one @@ -476,6 +491,7 @@ impl RepoStore { let mut root_filters = filters::comments_for(new_roots.clone()); root_filters.push(filters::statuses_for(new_roots.iter().copied())); root_filters.push(filters::annotations_for(new_roots)); + let announced: Vec = this.repo_relays.iter().cloned().collect(); let backend = Backend::global(cx); backend.update(cx, |backend, cx| { @@ -495,8 +511,7 @@ impl RepoStore { } })?; - // Requests that arrived while the refresh was running are - // coalesced into one follow-up refresh. + // Requests that arrived while the refresh was running are coalesced into one follow-up refresh. if again { this.update(cx, |this, cx| this.refresh(cx))?; } @@ -505,21 +520,19 @@ impl RepoStore { })); } - /// Resolve the status of a root event (issue / patch / PR) per NIP-34: - /// a lookup into the map built on the last refresh. + /// Resolve the status of a root event (issue / patch / PR) per NIP-34 pub fn status_of(&self, root: &Event) -> RepoStatus { status_of(&self.status_by_root, root) } - /// Refresh generation, incremented on every applied refresh. Views use - /// it to key their derived-data caches (filtered lists, counts) so - /// renders that change nothing stay O(1). + /// Refresh generation, incremented on every applied refresh. + /// Views use it to key their derived-data caches. pub fn version(&self) -> u64 { self.version } - /// The effective cover note of `root` (kind 1624), if any: the latest - /// note authored by the root author or a maintainer. + /// The effective cover note of `root` (kind 1624), if any: + /// the latest note authored by the root author or a maintainer. pub fn cover_note_of(&self, root: &Event) -> Option<&Event> { let maintainers = self .announcement @@ -557,14 +570,13 @@ impl RepoStore { /// Number of open issues: issues whose resolved status is /// [`RepoStatus::Open`] (issues without status events default to open). - /// Cached on the last refresh. pub fn issue_count(&self) -> usize { self.open_issue_count } /// 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`]. Cached on the last refresh. + /// [`RepoStatus::Open`]. pub fn pull_request_count(&self) -> usize { self.open_pr_count } @@ -596,15 +608,13 @@ impl RepoStore { .filter(move |e| signed_core::references_root(e, root)) } - /// Comment on a root event (issue / PR) per NIP-34 (kind 1111), using - /// the SDK's NIP-22 `CommentBuilder` so other NIP-34 clients (ngit, - /// GitWorkshop) can thread the comment. + /// Comment on a root event (issue / PR) per NIP-34 (kind 1111) pub fn comment(&mut self, root: &Event, content: String, cx: &mut Context) { self.reply(root, None, content, cx); } - /// Reply to `parent` (a comment on `root`) with a NIP-22 threaded - /// comment; `None` publishes a top-level comment on the root itself. + /// Reply to `parent` (a comment on `root`) with a NIP-22 threaded comment, + /// `None` publishes a top-level comment on the root itself. pub fn reply( &mut self, root: &Event, @@ -664,6 +674,7 @@ impl RepoStore { .into_iter() .map(str::to_owned) .collect(); + if let Some(oversized) = series .iter() .find(|part| part.len() > MAX_PATCH_EVENT_BYTES) @@ -677,8 +688,7 @@ impl RepoStore { return; } - // The tip of the series is its last commit; `git format-patch` - // orders patches oldest first. + // The tip of the series is its last commit; `git format-patch` orders patches oldest first. let Some(current_commit) = series .last() .and_then(|part| patch_current_commit(part)) @@ -692,16 +702,18 @@ impl RepoStore { }; let backend = Backend::global(cx); + let signer = backend.read(cx).signer(); + if backend.read(cx).current_user().is_none() { self.last_error = Some("Sign in to open a pull request".into()); cx.notify(); return; } - let signer = backend.read(cx).signer(); let addr = self.addr.clone(); let owner = self.addr.public_key; let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); + let (push_owner, push_repo_id, push_relays) = self .announcement .as_ref() @@ -742,9 +754,8 @@ impl RepoStore { subject, labels: Vec::new(), branch_name, - // NIP-34: PRs carry at least one clone URL where the - // tip commit can be downloaded; the announced mirrors - // are also the servers the tip is pushed to below. + // NIP-34: PRs carry at least one clone URL where the tip commit can be downloaded, + // the announced mirrors are also the servers the tip is pushed to below. clone: this .announcement .as_ref() @@ -757,8 +768,7 @@ impl RepoStore { } .into_event_builder(); - // NIP-34: the `r` EUC tag lets clients subscribe to all - // PRs of this repository; the SDK builder omits it. + // NIP-34: the `r` EUC tag lets clients subscribe to all PRs of this repository match this.announcement.as_ref().and_then(|a| a.euc.clone()) { Some(euc) => builder.tag(Tag::parse(["r", &euc]).expect("valid r tag")), None => builder, @@ -803,6 +813,7 @@ impl RepoStore { } }) .await; + if pushed == 0 { this.update(cx, |this, cx| { this.last_warning = Some(format!( @@ -818,6 +829,7 @@ impl RepoStore { let backend = Backend::global(cx); backend.update(cx, |backend, cx| backend.publish_event(event, cx)) })?; + let pr_event = match publish_task.await { Ok(event) => event, Err(e) => { @@ -828,8 +840,8 @@ impl RepoStore { } }; - // NIP-34: a draft PR carries a kind-1633 status event; publish - // it right after the PR event so viewers never show it open. + // NIP-34: a draft PR carries a kind-1633 status event, + // publish it right after the PR event so viewers never show it open. if draft { this.update(cx, |this, cx| { this.set_status(&pr_event, RepoStatus::Draft, cx); @@ -842,8 +854,7 @@ impl RepoStore { /// Update a pull request: publish revision patch events chained to the /// original root patch (`t root-revision` and a NIP-10 `e` reply on the - /// first, per NIP-34), then a kind-1619 PR update event carrying the - /// new tip. + /// first, per NIP-34), then a kind-1619 PR update event carrying the new tip. /// /// Only the PR author may update it; other authors must open a new PR. pub fn update_pull_request(&mut self, root: &Event, patch: String, cx: &mut Context) { diff --git a/crates/signed_ui/src/status_badge.rs b/crates/signed_ui/src/status_badge.rs index 5b58537..771b023 100644 --- a/crates/signed_ui/src/status_badge.rs +++ b/crates/signed_ui/src/status_badge.rs @@ -10,11 +10,11 @@ use signed_core::RepoStatus; pub fn status_badge(status: RepoStatus, cx: &App) -> AnyElement { let (icon, label, tooltip, bg, fg) = match status { RepoStatus::Open => ( - CustomIconName::GitIssueDone, + CustomIconName::GitIssueOpen, "open", "Issue is open", - cx.theme().primary, - cx.theme().primary_foreground, + cx.theme().secondary, + cx.theme().secondary_foreground, ), RepoStatus::Closed => ( CustomIconName::GitIssueClosed, @@ -31,11 +31,11 @@ pub fn status_badge(status: RepoStatus, cx: &App) -> AnyElement { cx.theme().accent_foreground, ), RepoStatus::Applied => ( - CustomIconName::GitIssueOpen, + CustomIconName::GitIssueDone, "applied", "Issue is completed", - cx.theme().secondary, - cx.theme().secondary_foreground, + cx.theme().primary, + cx.theme().primary_foreground, ), }; diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index 0954e89..d6279ec 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -44,6 +44,7 @@ mod issues; mod new_pull_request; mod pull_request_detail; mod pull_requests; +mod send_patch; use about::open_about_dialog; use browser::{ @@ -54,8 +55,10 @@ use commits::COMMIT_ROW_HEIGHT; use diff::CommitDiffView; use helpers::{ShareTargets, TreeItemSeed, build_tree_items, is_markdown_path, tree_items}; use issues::{IssuesView, open_new_issue_dialog}; -use new_pull_request::open_new_pull_request_panel; use pull_requests::PullRequestsView; +use send_patch::open_send_patch_panel; + +use crate::views::repo_detail::new_pull_request::open_new_pull_panel; /// What kind of ref the header selectors switch to. #[derive(Clone, Copy, PartialEq, Eq)] @@ -67,13 +70,17 @@ enum RefKind { } /// Header actions dispatched by the dropdown menus of the header buttons. +/// `pub(super)`: the pull-request list panel offers the same New-PR / Send- +/// patch actions in its own dropdown. #[derive(Clone, Action, PartialEq, Eq)] #[action(namespace = repo_detail, no_json)] -enum RepoAction { +pub(super) enum RepoAction { /// Open the "new issue" dialog. NewIssue, /// Open the "new pull request" dialog. NewPR, + /// Open the "send patch" panel. + SendPatch, /// Open the about dialog. About, /// Re-push the repository to its grasp servers. @@ -1363,13 +1370,12 @@ impl RepoDetailView { } RepoAction::NewPR => { if let Some(store) = this.store.clone() { - open_new_pull_request_panel( - this.dock_area.clone(), - store, - this.display_name(cx), - window, - cx, - ); + open_new_pull_panel(this.dock_area.clone(), store, window, cx); + } + } + RepoAction::SendPatch => { + if let Some(store) = this.store.clone() { + open_send_patch_panel(this.dock_area.clone(), store, window, cx); } } RepoAction::About => { @@ -1517,8 +1523,18 @@ impl RepoDetailView { .gap_2() .text_sm() .child(Icon::new(IconName::Plus)) - .child("New PR") + .child("New Pull Request") }) + .menu_element( + Box::new(RepoAction::SendPatch), + |_, _| { + h_flex() + .gap_2() + .text_sm() + .child(Icon::new(IconName::File)) + .child("Send Patch") + }, + ) }), ) .child( diff --git a/crates/workspace/src/views/repo_detail/new_pull_request.rs b/crates/workspace/src/views/repo_detail/new_pull_request.rs index f7362c1..6b7ef6c 100644 --- a/crates/workspace/src/views/repo_detail/new_pull_request.rs +++ b/crates/workspace/src/views/repo_detail/new_pull_request.rs @@ -93,10 +93,10 @@ impl NewPullRequestView { pub fn new( dock_area: WeakEntity, store: Entity, - repo_name: SharedString, window: &mut Window, cx: &mut Context, ) -> Self { + let repo_name = store.read(cx).name(); let pane = cx.new(DiffPane::new); let subject = cx.new(|cx| InputState::new(window, cx).placeholder("Title")); let description = cx.new(|cx| TextareaState::new(window, cx).placeholder("Describe...")); @@ -796,16 +796,14 @@ fn render_ref_trigger( .into_any_element() } -/// Open the "new pull request" panel for `store` in the center dock. -pub(super) fn open_new_pull_request_panel( +/// Open the "new pull request" panel in the center dock. +pub(super) fn open_new_pull_panel( dock_area: WeakEntity, store: Entity, - repo_name: SharedString, window: &mut Window, cx: &mut App, ) { - let panel = - cx.new(|cx| NewPullRequestView::new(dock_area.clone(), store, repo_name, window, cx)); + let panel = cx.new(|cx| NewPullRequestView::new(dock_area.clone(), store, window, cx)); let _ = dock_area.update(cx, |dock_area, cx| { dock_area.add_panel_view(panel_handle(panel), DockPlacement::Center, None, window, cx); diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index 269cbda..e7e42ce 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -7,18 +7,23 @@ use gpui::{ AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, SharedString, Size, WeakEntity, Window, div, px, size, }; +use gpui_base::Button as BaseButton; use gpui_component::alert::Alert; use gpui_component::scroll::Scrollbar; -use gpui_component::{ActiveTheme, Icon, VirtualListScrollHandle, h_flex, v_flex, v_virtual_list}; +use gpui_component::{ + ActiveTheme, Icon, IconName, VirtualListScrollHandle, h_flex, v_flex, v_virtual_list, +}; use nostr::prelude::{EventId, Kind}; use signed_core::{RepoStatus, activity_subject}; use signed_state::{ProfileStore, RepoStore}; use signed_ui::image_cache::{MAX_IMAGES, image_cache}; -use signed_ui::{SegmentButton, UserAvatar, placeholder, status_badge}; +use signed_ui::{DropdownButton, SegmentButton, UserAvatar, placeholder, status_badge}; use utils::relative_time; -use super::new_pull_request::open_new_pull_request_panel; +use super::RepoAction; +use super::new_pull_request::open_new_pull_panel; use super::pull_request_detail::PullRequestDetailView; +use super::send_patch::open_send_patch_panel; /// Height of one pull request row in the virtual list; same layout as an /// issue row. @@ -36,8 +41,7 @@ enum PullRequestFilter { Closed, /// Pull requests whose resolved status is [`RepoStatus::Draft`]. Draft, - /// Pull requests whose resolved status is [`RepoStatus::Applied`] - /// (i.e. merged). + /// Pull requests whose resolved status is [`RepoStatus::Applied`]. Merged, } @@ -213,7 +217,7 @@ impl PullRequestsView { .child( h_flex() .h_12() - .gap_2() + .gap_1() .child( SegmentButton::new("all", "All") .icon(Icon::new(CustomIconName::GitPullRequest)) @@ -267,18 +271,42 @@ impl PullRequestsView { ) .child(div().flex_1()) .child( - SegmentButton::new("new-pr", "New pull request") - .icon(Icon::new(CustomIconName::CirclePlus)) - .primary() - .on_click(cx.listener(|this, _event, window, cx| { - open_new_pull_request_panel( - this.dock_area.clone(), - this.store.clone(), - this.repo_name.clone(), - window, - cx, - ); - })), + h_flex().items_center().child( + DropdownButton::new("new-pr-actions") + .action( + BaseButton::new("new-pr") + .child( + h_flex() + .h_8() + .px_2() + .gap_1() + .rounded(cx.theme().radius) + .bg(cx.theme().primary) + .hover(|this| this.bg(cx.theme().primary_hover)) + .text_sm() + .text_color(cx.theme().primary_foreground) + .child(Icon::new(IconName::Plus)) + .child("New"), + ) + .on_click(cx.listener(|this, _event, window, cx| { + open_new_pull_panel( + this.dock_area.clone(), + this.store.clone(), + window, + cx, + ); + })), + ) + .dropdown_menu(|menu, _, _| { + menu.menu_element(Box::new(RepoAction::SendPatch), |_, _| { + h_flex() + .gap_2() + .text_sm() + .child(Icon::new(IconName::File)) + .child("Send Patch") + }) + }), + ), ) .into_any_element() } @@ -367,6 +395,11 @@ impl Render for PullRequestsView { v_flex() .size_full() .image_cache(image_cache("pull-requests", MAX_IMAGES)) + .on_action(cx.listener(|this, action: &RepoAction, window, cx| { + if action == &RepoAction::SendPatch { + open_send_patch_panel(this.dock_area.clone(), this.store.clone(), window, cx); + } + })) .child(self.render_header(cx)) .when_some(last_warning, |this, warning| { this.child(Alert::warning("pr-warning", warning).banner().on_close({ diff --git a/crates/workspace/src/views/repo_detail/send_patch.rs b/crates/workspace/src/views/repo_detail/send_patch.rs new file mode 100644 index 0000000..9159553 --- /dev/null +++ b/crates/workspace/src/views/repo_detail/send_patch.rs @@ -0,0 +1,265 @@ +use assets::CustomIconName; +use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; +use gpui::prelude::*; +use gpui::{ + AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Render, SharedString, + Subscription, WeakEntity, Window, div, px, +}; +use gpui_component::button::{Button, ButtonVariants}; +use gpui_component::input::{Input, InputEvent, InputState, Textarea, TextareaState}; +use gpui_component::{ActiveTheme, Disableable, Icon, IconName, Sizable, h_flex, v_flex}; +use signed_state::RepoStore; + +pub struct SendPatchView { + focus_handle: FocusHandle, + /// Dock area the panel lives in. + dock_area: WeakEntity, + /// Store of the target repository. + store: Entity, + /// Display name of the repository, for the panel title. + repo_name: SharedString, + /// Title input (required). + subject: Entity, + /// Description input (optional). + description: Entity, + /// The pasted `git format-patch` output (required). + patch: Entity, + /// A submit is in flight. + submitting: bool, + /// Error of the last submit attempt (keeps the panel open). + error: Option, + _subscriptions: Vec, +} + +impl SendPatchView { + pub fn new( + dock_area: WeakEntity, + store: Entity, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let repo_name = store.read(cx).name(); + let subject = cx.new(|cx| InputState::new(window, cx).placeholder("Title")); + let description = cx + .new(|cx| TextareaState::new(window, cx).placeholder("Describe the change (optional)")); + let patch = cx.new(|cx| { + TextareaState::new(window, cx).placeholder("Paste `git format-patch` output here...") + }); + + // Re-evaluate the Send button's enabled state as the inputs change. + let subscriptions = vec![ + cx.subscribe(&subject, |_this, _state, _event: &InputEvent, cx| { + cx.notify(); + }), + cx.subscribe(&patch, |_this, _state, _event: &InputEvent, cx| { + cx.notify(); + }), + ]; + + Self { + focus_handle: cx.focus_handle(), + dock_area, + store, + repo_name, + subject, + description, + patch, + submitting: false, + error: None, + _subscriptions: subscriptions, + } + } + + /// Publish the pull request from the pasted patch. The store validates + /// synchronously (patch shape, per-part size, sign-in); on failure the + /// panel stays open with the error inline, on success it closes — async + /// publish failures surface in the pull request list's banner. + fn submit(&mut self, window: &mut Window, cx: &mut Context) { + if self.submitting { + return; + } + let subject = self.subject.read(cx).value().to_string(); + let description = self.description.read(cx).value().to_string(); + let patch = self.patch.read(cx).value().to_string(); + if patch.is_empty() { + return; + } + let store = self.store.clone(); + let dock_area = self.dock_area.clone(); + let entity = cx.entity().clone(); + + self.submitting = true; + self.error = None; + cx.notify(); + + // Errors the store detects before publishing are returned + // synchronously through `last_error`. + let sync_error = store.update(cx, |store, cx| { + store.open_pull_request( + (!subject.is_empty()).then_some(subject), + description, + None, + patch, + false, + None, + None, + cx, + ); + store.last_error.clone() + }); + + if let Some(error) = sync_error { + self.submitting = false; + self.error = Some(error.into()); + cx.notify(); + return; + } + + // Close the panel once the publish is underway. + cx.defer_in(window, { + let dock_area = dock_area.clone(); + let entity = entity.clone(); + move |_, window, cx| { + if let Some(dock_area) = dock_area.upgrade() { + dock_area.update(cx, |dock, cx| { + dock.remove_panel(entity, window, cx); + }); + } + } + }); + cx.notify(); + } + + /// Top bar: a short caption and the Send button. + fn render_header(&self, cx: &mut Context) -> AnyElement { + let can_submit = !self.submitting + && !self.subject.read(cx).value().is_empty() + && !self.patch.read(cx).value().is_empty(); + + h_flex() + .px_4() + .h_12() + .w_full() + .gap_2() + .items_center() + .border_b_1() + .border_color(cx.theme().border) + .child( + h_flex() + .flex_1() + .min_w_0() + .gap_2() + .items_center() + .text_sm() + .text_color(cx.theme().muted_foreground) + .child(Icon::new(IconName::FileText).small().flex_shrink_0()) + .child( + div() + .min_w_0() + .overflow_hidden() + .text_ellipsis() + .whitespace_nowrap() + .child("Send a patch from `git format-patch` output"), + ), + ) + .child( + Button::new("send-patch") + .icon(CustomIconName::CirclePlus) + .label("Send patch") + .primary() + .loading(self.submitting) + .disabled(!can_submit) + .on_click(cx.listener(|this, _event, window, cx| { + this.submit(window, cx); + })), + ) + .into_any_element() + } + + /// Title and description inputs. + fn render_inputs(&self, cx: &mut Context) -> AnyElement { + v_flex() + .px_4() + .py_2() + .w_full() + .gap_2() + .border_b_1() + .border_color(cx.theme().border) + .child(Input::new(&self.subject)) + .child(Textarea::new(&self.description).h(px(64.))) + .into_any_element() + } + + /// The patch textarea, the main content of the panel. + fn render_patch(&self, cx: &mut Context) -> AnyElement { + v_flex() + .px_4() + .py_2() + .w_full() + .gap_2() + .child( + div() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child("Patch — `git format-patch` output"), + ) + .child(Textarea::new(&self.patch).h(px(240.))) + .into_any_element() + } +} + +/// Open the "send patch" panel for `store` in the center dock. +pub(super) fn open_send_patch_panel( + dock_area: WeakEntity, + store: Entity, + window: &mut Window, + cx: &mut App, +) { + let panel = cx.new(|cx| SendPatchView::new(dock_area.clone(), store, window, cx)); + + let _ = dock_area.update(cx, |dock_area, cx| { + dock_area.add_panel_view(panel_handle(panel), DockPlacement::Center, None, window, cx); + }); +} + +impl BasePanel for SendPatchView { + fn panel_name(&self) -> &'static str { + "send-patch" + } +} + +impl Panel for SendPatchView { + fn title(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + div().child(SharedString::from(format!("{}/send-patch", self.repo_name))) + } +} + +impl EventEmitter for SendPatchView {} + +impl Focusable for SendPatchView { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for SendPatchView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + v_flex() + .id("send-patch") + .size_full() + .child(self.render_header(cx)) + .child(self.render_inputs(cx)) + .when_some(self.error.clone(), |this, error| { + this.child( + h_flex() + .px_4() + .py_1() + .w_full() + .text_xs() + .text_color(cx.theme().danger) + .child(error), + ) + }) + .child(self.render_patch(cx)) + } +} diff --git a/docs/TODO.md b/docs/TODO.md index f822530..55f7568 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -17,6 +17,11 @@ - [x] Patch is generated from the checkout at submit time (`format_patch_between` on the stored merge base); panel closes after publishing, errors surface in the PR list banner. - [x] Removed with the dialog: paste textarea, draft checkbox, branch-name input and the mirror-clone apply-check hint (store behavior unchanged: `open_pull_request` still publishes the series + `branch-name`/`merge-base`/`r` tags and pushes the tip). +### Send patch panel (classic paste flow) + +- [x] "Send patch" entry in the repo header PRs dropdown (`RepoAction::SendPatch`) and a "New pull request ▾ Send patch" dropdown replacing the PR list's plain new-PR button. +- [x] `send_patch.rs` center panel: title + optional description + `git format-patch` paste area; submits through `RepoStore::open_pull_request` (no checkout, no `branch-name`/`merge-base`). Synchronous store errors (malformed/oversized patch, sign-in) keep the panel open with an inline error; the panel closes once the publish is underway. + - [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog (dialog since replaced by the panel above). - [x] P1: `RepoStore::update_pull_request` (kind 1619 + root-revision patch) with an author-only "Update" button on the PR detail header. - [x] P1: `latest_update` filters by PR author. -- 2.54.0 From 8c8633f5f66dfac9393e952625b749eb71db0b91 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Wed, 2 Sep 2026 17:44:04 +0700 Subject: [PATCH 9/9] update send patch panel --- .../src/views/repo_detail/send_patch.rs | 104 +++++++++--------- docs/TODO.md | 1 + 2 files changed, 53 insertions(+), 52 deletions(-) diff --git a/crates/workspace/src/views/repo_detail/send_patch.rs b/crates/workspace/src/views/repo_detail/send_patch.rs index 9159553..f9279f4 100644 --- a/crates/workspace/src/views/repo_detail/send_patch.rs +++ b/crates/workspace/src/views/repo_detail/send_patch.rs @@ -1,13 +1,14 @@ -use assets::CustomIconName; use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; use gpui::prelude::*; use gpui::{ AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Render, SharedString, Subscription, WeakEntity, Window, div, px, }; -use gpui_component::button::{Button, ButtonVariants}; +use gpui_base::{Button as BaseButton, StyledExt}; use gpui_component::input::{Input, InputEvent, InputState, Textarea, TextareaState}; -use gpui_component::{ActiveTheme, Disableable, Icon, IconName, Sizable, h_flex, v_flex}; +use gpui_component::scroll::ScrollableElement; +use gpui_component::spinner::Spinner; +use gpui_component::{ActiveTheme, Icon, IconName, Sizable, h_flex, v_flex}; use signed_state::RepoStore; pub struct SendPatchView { @@ -43,7 +44,7 @@ impl SendPatchView { let description = cx .new(|cx| TextareaState::new(window, cx).placeholder("Describe the change (optional)")); let patch = cx.new(|cx| { - TextareaState::new(window, cx).placeholder("Paste `git format-patch` output here...") + TextareaState::new(window, cx).placeholder("diff --git a/file.txt b/file.txt\nindex 1234567..abcdefg 100644\n--- a/file.txt\n+++ b/file.txt") }); // Re-evaluate the Send button's enabled state as the inputs change. @@ -130,44 +131,40 @@ impl SendPatchView { cx.notify(); } - /// Top bar: a short caption and the Send button. - fn render_header(&self, cx: &mut Context) -> AnyElement { + fn render_footer(&self, cx: &mut Context) -> AnyElement { let can_submit = !self.submitting && !self.subject.read(cx).value().is_empty() && !self.patch.read(cx).value().is_empty(); h_flex() .px_4() - .h_12() + .h_16() .w_full() .gap_2() .items_center() - .border_b_1() + .border_t_1() .border_color(cx.theme().border) + .child(div().flex_1()) .child( - h_flex() - .flex_1() - .min_w_0() - .gap_2() - .items_center() + BaseButton::new("send-patch") + .h_flex() + .h_8() + .px_2() + .gap_1() .text_sm() - .text_color(cx.theme().muted_foreground) - .child(Icon::new(IconName::FileText).small().flex_shrink_0()) - .child( - div() - .min_w_0() - .overflow_hidden() - .text_ellipsis() - .whitespace_nowrap() - .child("Send a patch from `git format-patch` output"), - ), - ) - .child( - Button::new("send-patch") - .icon(CustomIconName::CirclePlus) - .label("Send patch") - .primary() - .loading(self.submitting) + .items_center() + .justify_center() + .bg(cx.theme().primary) + .text_color(cx.theme().primary_foreground) + .hover(|this| this.bg(cx.theme().primary_hover)) + .active(|this| this.bg(cx.theme().primary_active)) + .map(|this| { + if self.submitting { + this.child(Spinner::new().small()) + } else { + this.child(Icon::new(IconName::ArrowUp)).child("Send patch") + } + }) .disabled(!can_submit) .on_click(cx.listener(|this, _event, window, cx| { this.submit(window, cx); @@ -176,22 +173,20 @@ impl SendPatchView { .into_any_element() } - /// Title and description inputs. - fn render_inputs(&self, cx: &mut Context) -> AnyElement { + fn render_inputs(&self, _cx: &mut Context) -> AnyElement { v_flex() .px_4() .py_2() .w_full() .gap_2() - .border_b_1() - .border_color(cx.theme().border) .child(Input::new(&self.subject)) .child(Textarea::new(&self.description).h(px(64.))) .into_any_element() } - /// The patch textarea, the main content of the panel. fn render_patch(&self, cx: &mut Context) -> AnyElement { + const MSG: &str = "You can paste a git diff or a git format-patch patch series here."; + v_flex() .px_4() .py_2() @@ -201,14 +196,13 @@ impl SendPatchView { div() .text_xs() .text_color(cx.theme().muted_foreground) - .child("Patch — `git format-patch` output"), + .child(MSG), ) - .child(Textarea::new(&self.patch).h(px(240.))) + .child(Textarea::new(&self.patch).h_56()) .into_any_element() } } -/// Open the "send patch" panel for `store` in the center dock. pub(super) fn open_send_patch_panel( dock_area: WeakEntity, store: Entity, @@ -247,19 +241,25 @@ impl Render for SendPatchView { v_flex() .id("send-patch") .size_full() - .child(self.render_header(cx)) - .child(self.render_inputs(cx)) - .when_some(self.error.clone(), |this, error| { - this.child( - h_flex() - .px_4() - .py_1() - .w_full() - .text_xs() - .text_color(cx.theme().danger) - .child(error), - ) - }) - .child(self.render_patch(cx)) + .child( + v_flex() + .overflow_y_scrollbar() + .flex_1() + .w_full() + .child(self.render_inputs(cx)) + .when_some(self.error.clone(), |this, error| { + this.child( + h_flex() + .px_4() + .py_1() + .w_full() + .text_xs() + .text_color(cx.theme().danger) + .child(error), + ) + }) + .child(self.render_patch(cx)), + ) + .child(self.render_footer(cx)) } } diff --git a/docs/TODO.md b/docs/TODO.md index 55f7568..05e68b9 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -34,6 +34,7 @@ - [ ] GRASP-06 `/prs//.git` contributor endpoints + kind-10317 user grasp-list fallback. - [ ] Merge button in the PR detail view (`merge_pull_request` is store-only today), then fetch-and-merge (`merge-commit`) when the push backend is guaranteed. - [ ] Local-checkout generation for the update-PR dialog (currently paste-only). +- [ ] Fork-aware compare in the New PR panel: today both branch selectors come from the user-picked local checkout, so a cross-fork PR (GitHub's "compare across forks") requires the fork's branch to exist locally. Add picking the fork repository from announced repos (its 30617 may point at this repo via the `u` tag, or share the EUC) + a branch, fetch it into the `GitCache` mirror, and run the `merge-base`/diff/`format-patch` flow against the base repo's mirror — like `choose_checkout` today but repo-driven. ## Performance: render path -- 2.54.0