From 1d224218df26710226802401a61b287b2d0730e9 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Fri, 4 Sep 2026 16:02:51 +0700 Subject: [PATCH] wip --- Cargo.lock | 8 - Cargo.toml | 4 - crates/assets/Cargo.toml | 1 - crates/dock/src/lib.rs | 1 - crates/paths/src/lib.rs | 73 +- crates/signed_core/src/comments.rs | 164 ---- crates/signed_core/src/lib.rs | 2 - crates/signed_git/src/lib.rs | 376 +++----- crates/signed_nostr/Cargo.toml | 3 - crates/signed_nostr/src/update.rs | 2 - crates/signed_state/src/backend.rs | 176 +--- crates/signed_state/src/checkouts.rs | 44 +- crates/signed_state/src/lib.rs | 4 +- crates/signed_state/src/refresh.rs | 79 ++ crates/signed_state/src/repo.rs | 181 +--- crates/signed_state/src/repo_list.rs | 48 +- crates/signed_ui/src/image_cache.rs | 139 --- crates/signed_ui/src/lib.rs | 2 - crates/workspace/Cargo.toml | 1 - crates/workspace/src/lib.rs | 1 - crates/workspace/src/views/dialog_state.rs | 36 + crates/workspace/src/views/mod.rs | 1 + .../workspace/src/views/repo_detail/diff.rs | 2 +- .../src/views/repo_detail/helpers.rs | 236 ++++- .../src/views/repo_detail/init_dialog.rs | 31 +- .../src/views/repo_detail/issue_detail.rs | 234 +---- .../workspace/src/views/repo_detail/issues.rs | 3 +- crates/workspace/src/views/repo_detail/mod.rs | 3 +- .../views/repo_detail/pull_request_detail.rs | 531 +---------- .../src/views/repo_detail/pull_requests.rs | 3 +- crates/workspace/src/views/repo_list.rs | 3 +- .../src/views/sidebar/create_repo_dialog.rs | 33 +- .../src/views/sidebar/grasp_servers.rs | 26 +- crates/workspace/src/views/sidebar/mod.rs | 3 +- .../src/views/sidebar/onboarding_dialog.rs | 28 +- .../src/views/sidebar/passphrase_dialog.rs | 30 +- desktop/Cargo.toml | 4 - docs/PLAN.md | 860 +++++------------- 38 files changed, 894 insertions(+), 2482 deletions(-) delete mode 100644 crates/signed_core/src/comments.rs create mode 100644 crates/signed_state/src/refresh.rs delete mode 100644 crates/signed_ui/src/image_cache.rs create mode 100644 crates/workspace/src/views/dialog_state.rs diff --git a/Cargo.lock b/Cargo.lock index 94ab2a5..ae229c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -291,7 +291,6 @@ dependencies = [ "anyhow", "gpui", "gpui-component", - "log", "rust-embed", "serde_json", ] @@ -7887,11 +7886,7 @@ dependencies = [ "dock", "gpui", "gpui-component", - "gpui_linux", - "gpui_macos", "gpui_platform", - "gpui_windows", - "log", "paths", "reqwest_client", "settings", @@ -7925,13 +7920,11 @@ name = "signed_nostr" version = "1.0.0" dependencies = [ "anyhow", - "nostr", "nostr-connect", "nostr-gossip-memory", "nostr-lmdb", "nostr-memory", "nostr-sdk", - "signed_core", "webbrowser", ] @@ -10801,7 +10794,6 @@ version = "1.0.0" dependencies = [ "anyhow", "assets", - "chrono", "dock", "futures", "gix", diff --git a/Cargo.toml b/Cargo.toml index a13c8ec..336d9e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,9 +12,6 @@ publish = false # GPUI gpui = { git = "https://github.com/zed-industries/zed" } gpui_platform = { git = "https://github.com/zed-industries/zed", features = ["font-kit", "x11", "wayland"] } -gpui_linux = { git = "https://github.com/zed-industries/zed" } -gpui_windows = { git = "https://github.com/zed-industries/zed" } -gpui_macos = { git = "https://github.com/zed-industries/zed" } gpui_tokio = { git = "https://github.com/zed-industries/zed" } reqwest_client = { git = "https://github.com/zed-industries/zed" } @@ -37,7 +34,6 @@ nostr-sdk = { git = "https://github.com/rust-nostr/nostr" } gix = { version = "0.87.1", default-features = false, features = ["sha1", "blocking-network-client", "blocking-http-transport-reqwest-rust-tls", "worktree-mutation"] } -chrono = { version = "0.4.38", features = ["wasmbind"] } smol = "2" futures = "0.3" flume = { version = "0.11.1", default-features = false, features = ["async", "select"] } diff --git a/crates/assets/Cargo.toml b/crates/assets/Cargo.toml index df76dfc..da6bf8a 100644 --- a/crates/assets/Cargo.toml +++ b/crates/assets/Cargo.toml @@ -8,7 +8,6 @@ publish.workspace = true gpui.workspace = true gpui-component.workspace = true anyhow.workspace = true -log.workspace = true rust-embed.workspace = true [dev-dependencies] diff --git a/crates/dock/src/lib.rs b/crates/dock/src/lib.rs index 70458ad..0dc653d 100644 --- a/crates/dock/src/lib.rs +++ b/crates/dock/src/lib.rs @@ -20,7 +20,6 @@ pub const TAB_BAR_HEIGHT: Pixels = px(44.); /// i18n shim resolving `Dock.*` keys to English, so the crate has no i18n dependency. pub(crate) fn t(key: &'static str) -> &'static str { match key { - "Dock.Unnamed" => "Unnamed", "Dock.Close" => "Close", "Dock.Zoom In" => "Zoom In", "Dock.Zoom Out" => "Zoom Out", diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 96f89fb..3e0b0e8 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -9,9 +9,6 @@ pub const APP_NAME: &str = "Signed"; /// Used in XDG-style paths on Linux and FreeBSD, and the macOS `~/.config` fallback. pub const APP_NAME_LOWERCASE: &str = "signed"; -/// A custom data directory override, set only by [`set_custom_data_dir`]. -static CUSTOM_DATA_DIR: OnceLock = OnceLock::new(); - /// The resolved data directory. /// On macOS, this is `~/Library/Application Support/Signed`. /// On Linux/FreeBSD, this is `$XDG_DATA_HOME/signed`. @@ -41,31 +38,10 @@ pub fn documents_dir() -> PathBuf { dirs::document_dir().unwrap_or_else(|| dirs::home_dir().unwrap_or_default()) } -/// Sets a custom directory for all user data, overriding the default data directory. -/// Must be called before any other path operation. -/// The directory is created when missing and canonicalized to an absolute path. -/// # Panics -/// Panics when called after [`data_dir`] or [`config_dir`] was initialized. -/// Panics when the directory cannot be created or canonicalized. -pub fn set_custom_data_dir(dir: &str) -> &'static PathBuf { - if CURRENT_DATA_DIR.get().is_some() || CONFIG_DIR.get().is_some() { - panic!("set_custom_data_dir called after data_dir or config_dir was initialized"); - } - - CUSTOM_DATA_DIR.get_or_init(|| { - let path = PathBuf::from(dir); - std::fs::create_dir_all(&path).expect("failed to create custom data directory"); - path.canonicalize() - .expect("failed to canonicalize custom data directory") - }) -} - /// Returns the path to the configuration directory. pub fn config_dir() -> &'static PathBuf { CONFIG_DIR.get_or_init(|| { - if let Some(custom_dir) = CUSTOM_DATA_DIR.get() { - custom_dir.join("config") - } else if cfg!(target_os = "windows") { + if cfg!(target_os = "windows") { dirs::config_dir() .expect("failed to determine RoamingAppData directory") .join(APP_NAME) @@ -85,9 +61,7 @@ pub fn config_dir() -> &'static PathBuf { /// Returns the path to the data directory. pub fn data_dir() -> &'static PathBuf { CURRENT_DATA_DIR.get_or_init(|| { - if let Some(custom_dir) = CUSTOM_DATA_DIR.get() { - custom_dir.clone() - } else if cfg!(target_os = "macos") { + if cfg!(target_os = "macos") { home_dir() .join("Library/Application Support") .join(APP_NAME) @@ -108,43 +82,6 @@ pub fn data_dir() -> &'static PathBuf { }) } -/// Returns the path to the cache directory. -pub fn cache_dir() -> &'static PathBuf { - static CACHE_DIR: OnceLock = OnceLock::new(); - CACHE_DIR.get_or_init(|| { - if cfg!(target_os = "macos") { - dirs::cache_dir() - .expect("failed to determine caches directory") - .join(APP_NAME) - } else if cfg!(target_os = "windows") { - dirs::cache_dir() - .expect("failed to determine LocalAppData directory") - .join(APP_NAME) - } else if cfg!(any(target_os = "linux", target_os = "freebsd")) { - if let Ok(flatpak_xdg_cache) = std::env::var("FLATPAK_XDG_CACHE_HOME") { - flatpak_xdg_cache.into() - } else { - dirs::cache_dir().expect("failed to determine XDG_CACHE_HOME directory") - } - .join(APP_NAME_LOWERCASE) - } else { - home_dir().join(".cache").join(APP_NAME_LOWERCASE) - } - }) -} - -/// Returns the path to the logs directory. -pub fn logs_dir() -> &'static PathBuf { - static LOGS_DIR: OnceLock = OnceLock::new(); - LOGS_DIR.get_or_init(|| { - if cfg!(target_os = "macos") { - home_dir().join("Library/Logs").join(APP_NAME) - } else { - data_dir().join("logs") - } - }) -} - /// Returns the path to the nostr database directory, LMDB. pub fn nostr_dir() -> &'static PathBuf { static NOSTR_DIR: OnceLock = OnceLock::new(); @@ -162,9 +99,3 @@ pub fn settings_file() -> &'static PathBuf { static SETTINGS_FILE: OnceLock = OnceLock::new(); SETTINGS_FILE.get_or_init(|| config_dir().join("settings.json")) } - -/// Returns the path to the `keymap.json` file. -pub fn keymap_file() -> &'static PathBuf { - static KEYMAP_FILE: OnceLock = OnceLock::new(); - KEYMAP_FILE.get_or_init(|| config_dir().join("keymap.json")) -} diff --git a/crates/signed_core/src/comments.rs b/crates/signed_core/src/comments.rs deleted file mode 100644 index 4aa285b..0000000 --- a/crates/signed_core/src/comments.rs +++ /dev/null @@ -1,164 +0,0 @@ -use std::collections::{HashMap, HashSet}; - -use nostr::prelude::*; - -/// A NIP-22 comment thread, a top-level comment on the root event, -/// nested replies are ordered oldest first at every level. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct CommentThread { - /// The thread's top-level comment. - pub comment: Event, - /// Replies to [`Self::comment`], nested recursively. - pub replies: Vec, -} - -/// The direct parent id of a comment, from its NIP-22 lowercase `e` tag. -/// `None` when no `e` tag is present. -fn comment_parent(event: &Event) -> Option { - event - .tags - .iter() - .find(|tag| tag.kind() == "e") - .and_then(Tag::content) - .and_then(|id| EventId::parse(id).ok()) -} - -/// Group the comments on a root issue, patch or PR into NIP-22 threads, -/// a comment whose parent is the root starts a thread. -/// -/// Other comments nest under their parent comment. -/// -/// Threads and replies are ordered oldest first, -/// replies with a missing parent are made top-level threads, so none are dropped. -pub fn comment_threads(root: &Event, comments: &[Event]) -> Vec { - // Index comments by their parent id. - // Comments without a parent tag reply to the root event itself. - let mut children: HashMap> = HashMap::new(); - for comment in comments { - let parent = comment_parent(comment).unwrap_or(root.id); - children.entry(parent).or_default().push(comment); - } - for list in children.values_mut() { - list.sort_by_key(|event| event.created_at); - } - - let mut visited: HashSet = HashSet::new(); - - fn build( - id: EventId, - children: &HashMap>, - visited: &mut HashSet, - ) -> Vec { - let Some(list) = children.get(&id) else { - return Vec::new(); - }; - let mut threads = Vec::new(); - for event in list { - // Guards against malformed reply cycles. - if visited.insert(event.id) { - threads.push(CommentThread { - comment: (*event).clone(), - replies: build(event.id, children, visited), - }); - } - } - threads - } - - let mut threads = build(root.id, &children, &mut visited); - - // Orphan replies have an unknown parent comment, so they never reach the root tree. - // Surface them as top-level threads so they are not dropped. - let mut orphans: Vec<&Event> = comments - .iter() - .filter(|event| !visited.contains(&event.id)) - .collect(); - orphans.sort_by_key(|event| event.created_at); - for comment in orphans { - if visited.insert(comment.id) { - threads.push(CommentThread { - comment: comment.clone(), - replies: build(comment.id, &children, &mut visited), - }); - } - } - - threads -} - -#[cfg(test)] -mod tests { - use super::*; - - fn comment(keys: &Keys, parent: Option<&Event>, content: &str, created_at: u64) -> Event { - let tags = parent - .map(|parent| vec![Tag::parse(["e", &parent.id.to_hex()]).expect("valid e tag")]) - .unwrap_or_default(); - EventBuilder::new(Kind::Comment, content) - .tags(tags) - .custom_created_at(Timestamp::from(created_at)) - .finalize(keys) - .expect("signed event") - } - - fn flatten(threads: &[CommentThread]) -> Vec { - let mut out = Vec::new(); - for thread in threads { - out.push(thread.comment.content.clone()); - out.extend(flatten(&thread.replies)); - } - out - } - - #[test] - fn nests_replies_under_their_parents() { - let keys = Keys::generate(); - let root = EventBuilder::new(Kind::GitIssue, "issue") - .finalize(&keys) - .expect("signed event"); - - let a = comment(&keys, Some(&root), "a", 100); - let a1 = comment(&keys, Some(&a), "a1", 200); - let a2 = comment(&keys, Some(&a), "a2", 300); - let b = comment(&keys, Some(&root), "b", 150); - - let threads = comment_threads(&root, &[a2, b, a, a1]); - - assert_eq!(flatten(&threads), vec!["a", "a1", "a2", "b"]); - } - - #[test] - fn comments_without_a_parent_tag_attach_to_the_root() { - let keys = Keys::generate(); - let root = EventBuilder::new(Kind::GitIssue, "issue") - .finalize(&keys) - .expect("signed event"); - - // Old-style comments carried no `e` tag at all. - let orphan = comment(&keys, None, "no parent", 100); - - let threads = comment_threads(&root, &[orphan]); - - assert_eq!(flatten(&threads), vec!["no parent"]); - } - - #[test] - fn orphan_replies_are_surfaced_as_top_level_threads() { - let keys = Keys::generate(); - let root = EventBuilder::new(Kind::GitIssue, "issue") - .finalize(&keys) - .expect("signed event"); - let a = comment(&keys, Some(&root), "a", 100); - - // `missing` is not in the comment set. - // Its reply should still show up. - let missing = EventBuilder::new(Kind::Comment, "missing") - .finalize(&keys) - .expect("signed event"); - let reply_to_missing = comment(&keys, Some(&missing), "reply to missing", 200); - - let threads = comment_threads(&root, &[a, reply_to_missing]); - - assert_eq!(flatten(&threads), vec!["a", "reply to missing"]); - } -} diff --git a/crates/signed_core/src/lib.rs b/crates/signed_core/src/lib.rs index 5948d4d..c73a298 100644 --- a/crates/signed_core/src/lib.rs +++ b/crates/signed_core/src/lib.rs @@ -1,7 +1,6 @@ pub mod addr; pub mod annotations; pub mod clone_url; -pub mod comments; pub mod deletions; pub mod filters; pub mod model; @@ -11,7 +10,6 @@ pub mod status; pub use addr::{RepoAddr, identifier_from_name, repo_addr}; pub use annotations::{COVER_NOTE_KIND, cover_note, labels_and_subject, subject_override}; 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, pull_request_patches}; pub use state::{build_state, parse_state}; diff --git a/crates/signed_git/src/lib.rs b/crates/signed_git/src/lib.rs index 8e398f7..ae4a80e 100644 --- a/crates/signed_git/src/lib.rs +++ b/crates/signed_git/src/lib.rs @@ -127,24 +127,13 @@ pub fn clone_repo(clone_urls: &[String], path: &Path) -> Result<()> { bail!("destination {} already exists", path.display()); } - let mut last_err: Option = None; - - for url in clone_urls { - match clone(url, path) { - Ok(repo) => { - // The initial clone uses the default refspecs. - // Also fetch the `refs/nostr/*` PR refs. - fetch_all(&repo).ok(); - return Ok(()); - } - Err(e) => last_err = Some(e), - } - } - - match last_err { - Some(e) => Err(e).context("failed to clone from any mirror"), - None => bail!("no clone URLs provided"), - } + try_each_url(clone_urls, "clone", |url| { + let repo = clone(url, path)?; + // The initial clone uses the default refspecs. + // Also fetch the `refs/nostr/*` PR refs. + fetch_all(&repo).ok(); + Ok(()) + }) } /// Fetch all configured refspecs from `origin`, plus the `refs/nostr/*` namespace. @@ -199,25 +188,14 @@ pub fn apply_patch(repo_path: &Path, patch: &str) -> Result<()> { /// /// 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 means 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() - ), + let repo = open_with_cache(repo_path)?; + let a = repo.rev_parse_single(a.as_bytes())?; + let b = repo.rev_parse_single(b.as_bytes())?; + match repo.merge_base(a, b) { + Ok(id) => Ok(Some(id.to_string())), + // No common ancestor, a valid outcome for a proposal. + Err(gix::repository::merge_base::Error::NotFound { .. }) => Ok(None), + Err(e) => Err(e.into()), } } @@ -248,34 +226,6 @@ pub fn format_patch_between(repo_path: &Path, base: &str, tip: &str) -> Result 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` on the server at `url`, from `repo_path`. pub fn push_commit_ref(repo_path: &Path, url: &str, commit: &str, reference: &str) -> Result<()> { let output = Command::new("git") @@ -331,14 +281,7 @@ pub fn split_patch_series(patch: &str) -> Vec<&str> { /// /// `None` when the repository has no commits yet, an 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`")?; + let output = git_output(repo_path, &["rev-parse", "HEAD"], "git rev-parse")?; if !output.status.success() { return Ok(None); @@ -371,13 +314,41 @@ pub fn commits_since(repo_path: &Path, base: Option<&str>) -> Result .collect()) } -fn clone(url: &str, path: &Path) -> Result { - // GRASP servers announce `grasp:////` clone URLs. - // The transport is git smart HTTP, so the scheme is rewritten for gix. - let url = url - .strip_prefix("grasp://") +/// Rewrite a grasp server URL to the https URL the git transport actually uses. +/// +/// GRASP servers announce `grasp:////` clone URLs. +/// The transport is git smart HTTP, so the scheme is rewritten for gix. +fn transport_url(url: &str) -> String { + url.strip_prefix("grasp://") .map(|rest| format!("https://{rest}")) - .unwrap_or_else(|| url.to_owned()); + .unwrap_or_else(|| url.to_owned()) +} + +/// Run `attempt` against each URL in `urls` until one succeeds. +/// +/// Returns the last error wrapped in `failed to {verb} from any mirror`, +/// or `no clone URLs provided` when the list is empty. +fn try_each_url(urls: &[String], verb: &str, mut attempt: F) -> Result<()> +where + F: FnMut(&str) -> Result<()>, +{ + let mut last_err: Option = None; + + for url in urls { + match attempt(url) { + Ok(()) => return Ok(()), + Err(e) => last_err = Some(e), + } + } + + match last_err { + Some(e) => Err(e).context(format!("failed to {verb} from any mirror")), + None => bail!("no clone URLs provided"), + } +} + +fn clone(url: &str, path: &Path) -> Result { + let url = transport_url(url); let url = gix::url::parse(url).context("invalid clone URL")?; let mut prepare = gix::prepare_clone(url, path)?; @@ -435,44 +406,44 @@ pub fn init_repository(path: &Path, name: &str, description: &str) -> Result Result<()> { - let url = format!("{base_url}/{owner}/{repo_id}.git"); - - let output = Command::new("git") - .arg("-C") - .arg(repo_path) - .args(["push"]) - .arg(&url) - .args(["refs/heads/main:refs/heads/main"]) - .env("GIT_TERMINAL_PROMPT", "0") - .stderr(Stdio::piped()) - .output() - .context("failed to spawn `git push`")?; - - if !output.status.success() { - bail!( - "git push to {base_url} failed: {}", - String::from_utf8_lossy(&output.stderr).trim() - ); - } - Ok(()) + push_refspecs( + repo_path, + base_url, + owner, + repo_id, + &["refs/heads/main:refs/heads/main"], + ) } /// Push every local branch and tag of the repository at `repo_path` to a grasp server. /// /// This mirrors an initialized repository's whole history. pub fn push_all(repo_path: &Path, base_url: &str, owner: &str, repo_id: &str) -> Result<()> { + push_refspecs( + repo_path, + base_url, + owner, + repo_id, + &["refs/heads/*:refs/heads/*", "refs/tags/*:refs/tags/*"], + ) +} + +/// Push `refspecs` to the grasp server URL derived from `base_url`, `owner` and `repo_id`. +fn push_refspecs( + repo_path: &Path, + base_url: &str, + owner: &str, + repo_id: &str, + refspecs: &[&str], +) -> Result<()> { let url = format!("{base_url}/{owner}/{repo_id}.git"); - let output = Command::new("git") - .arg("-C") - .arg(repo_path) - .args(["push"]) - .arg(&url) - .args(["refs/heads/*:refs/heads/*", "refs/tags/*:refs/tags/*"]) - .env("GIT_TERMINAL_PROMPT", "0") - .stderr(Stdio::piped()) - .output() - .context("failed to spawn `git push`")?; + let mut args: Vec<&str> = Vec::with_capacity(refspecs.len() + 2); + args.push("push"); + args.push(&url); + args.extend_from_slice(refspecs); + + let output = git_output(repo_path, &args, "git push")?; if !output.status.success() { bail!( @@ -480,7 +451,6 @@ pub fn push_all(repo_path: &Path, base_url: &str, owner: &str, repo_id: &str) -> String::from_utf8_lossy(&output.stderr).trim() ); } - Ok(()) } @@ -489,14 +459,11 @@ pub fn push_all(repo_path: &Path, base_url: &str, owner: &str, repo_id: &str) -> /// /// `None` for a repository without commits. pub fn root_commit(repo_path: &Path) -> Result> { - let output = Command::new("git") - .arg("-C") - .arg(repo_path) - .args(["rev-list", "--max-parents=0", "HEAD"]) - .env("GIT_TERMINAL_PROMPT", "0") - .stderr(Stdio::piped()) - .output() - .context("failed to spawn `git rev-list`")?; + let output = git_output( + repo_path, + &["rev-list", "--max-parents=0", "HEAD"], + "git rev-list", + )?; // An unborn HEAD with no commits yet makes `rev-list` fail. // There is no unique commit to report then. @@ -519,15 +486,8 @@ pub fn ensure_origin(repo_path: &Path, url: &str) -> Result<()> { if git_in(repo_path, &["remote", "get-url", "origin"]).is_ok() { return Ok(()); } + // `git remote add` already configures the default fetch refspec. git_in(repo_path, &["remote", "add", "origin", url])?; - git_in( - repo_path, - &[ - "config", - "remote.origin.fetch", - "+refs/heads/*:refs/remotes/origin/*", - ], - )?; Ok(()) } @@ -550,38 +510,19 @@ pub fn set_origin(repo_path: &Path, url: &str) -> Result<()> { /// /// Never touches the checked-out refs or the worktree. pub fn fetch_repo_refs(repo_path: &Path, urls: &[String], refspec: &str) -> Result<()> { - let mut last_err: Option = None; + try_each_url(urls, "fetch", |url| { + let url = transport_url(url); - for url in urls { - let url = url - .strip_prefix("grasp://") - .map(|rest| format!("https://{rest}")) - .unwrap_or_else(|| url.to_owned()); - - let output = Command::new("git") - .arg("-C") - .arg(repo_path) - .args(["fetch"]) - .arg(&url) - .arg(refspec) - .env("GIT_TERMINAL_PROMPT", "0") - .stderr(Stdio::piped()) - .output() - .context("failed to spawn `git fetch`")?; + let output = git_output(repo_path, &["fetch", &url, refspec], "git fetch")?; if output.status.success() { return Ok(()); } - last_err = Some(anyhow::anyhow!( + bail!( "git fetch from {url} failed: {}", String::from_utf8_lossy(&output.stderr).trim() - )); - } - - match last_err { - Some(e) => Err(e).context("failed to fetch from any mirror"), - None => bail!("no clone URLs provided"), - } + ) + }) } /// Full ref names under `prefix`, sorted lexicographically, like `git for-each-ref`. @@ -592,14 +533,11 @@ pub fn refs_with_prefix(repo_path: &Path, prefix: &str) -> Result> { // `for-each-ref` patterns match whole path components. // A trailing slash would silently change what is matched. let pattern = prefix.trim_end_matches('/'); - let output = Command::new("git") - .arg("-C") - .arg(repo_path) - .args(["for-each-ref", "--format=%(refname)", pattern]) - .env("GIT_TERMINAL_PROMPT", "0") - .stderr(Stdio::piped()) - .output() - .context("failed to spawn `git for-each-ref`")?; + let output = git_output( + repo_path, + &["for-each-ref", "--format=%(refname)", pattern], + "git for-each-ref", + )?; if !output.status.success() { bail!( @@ -659,14 +597,11 @@ pub fn delete_refs_with_prefix(repo_path: &Path, prefix: &str) -> Result<()> { /// /// `None` when it has no `origin` yet. pub fn origin_url(workdir: &Path) -> Result> { - let output = Command::new("git") - .arg("-C") - .arg(workdir) - .args(["remote", "get-url", "origin"]) - .env("GIT_TERMINAL_PROMPT", "0") - .stderr(Stdio::piped()) - .output() - .context("failed to spawn `git remote get-url`")?; + let output = git_output( + workdir, + &["remote", "get-url", "origin"], + "git remote get-url", + )?; if !output.status.success() { return Ok(None); @@ -717,18 +652,25 @@ pub fn fast_forward_branches(workdir: &Path) -> Result { Ok(moved) } -/// Run a git command in `dir`, returning trimmed stdout. +/// Run `git -C dir args`, disabling the terminal prompt and capturing stderr. /// -/// The terminal prompt is disabled so a credential request fails instead of hanging. -fn git_in(dir: &Path, args: &[&str]) -> Result { - let output = Command::new("git") +/// `what` names the command in the spawn error. +fn git_output(dir: &Path, args: &[&str], what: &str) -> Result { + Command::new("git") .arg("-C") .arg(dir) .args(args) .env("GIT_TERMINAL_PROMPT", "0") .stderr(Stdio::piped()) .output() - .context("failed to spawn `git`")?; + .with_context(|| format!("failed to spawn `{what}`")) +} + +/// Run a git command in `dir`, returning trimmed stdout. +/// +/// The terminal prompt is disabled so a credential request fails instead of hanging. +fn git_in(dir: &Path, args: &[&str]) -> Result { + let output = git_output(dir, args, "git")?; if !output.status.success() { bail!( @@ -1631,55 +1573,18 @@ fn take_quoted(input: &str) -> Option<(&str, &str)> { } /// Undo git's C-style path quoting, `\NNN` octal escapes, `\"` and `\\`. +/// +/// Delegates to gitoxide's C-style quote implementation, `gix::quote::ansi_c::undo`. +/// It expects the surrounding double quotes, which are re-added around the interior. fn unquote_path(path: &str) -> Result { if !path.contains('\\') { return Ok(path.to_owned()); } - let mut out = Vec::with_capacity(path.len()); - let mut bytes = path.as_bytes(); - while let Some((&b, rest)) = bytes.split_first() { - bytes = rest; - if b == b'\\' { - match bytes.split_first() { - Some((&b'"', rest)) | Some((&b'\\', rest)) => { - out.push(b); - bytes = rest; - } - Some((&b'n', rest)) => { - out.push(b'\n'); - bytes = rest; - } - Some((&b't', rest)) => { - out.push(b'\t'); - bytes = rest; - } - Some((&d1, rest)) if (b'0'..=b'7').contains(&d1) => { - let Some((&d2, rest)) = rest.split_first() else { - bail!("malformed octal escape in quoted path"); - }; - let Some((&d3, rest)) = rest.split_first() else { - bail!("malformed octal escape in quoted path"); - }; - if !(b'0'..=b'7').contains(&d2) || !(b'0'..=b'7').contains(&d3) { - bail!("malformed octal escape in quoted path"); - } - let code = - (d1 - b'0') as u16 * 64 + (d2 - b'0') as u16 * 8 + (d3 - b'0') as u16; - if code > u8::MAX as u16 { - bail!("octal escape out of range in quoted path"); - } - out.push(code as u8); - bytes = rest; - } - _ => bail!("malformed escape in quoted path"), - } - } else { - out.push(b); - } - } - - String::from_utf8(out).context("invalid UTF-8 in quoted path") + let quoted = format!("\"{path}\""); + let (unquoted, _) = gix::quote::ansi_c::undo(gix::bstr::BStr::new(quoted.as_bytes())) + .map_err(|e| anyhow::anyhow!("malformed quoted path: {e}"))?; + String::from_utf8(unquoted.into_owned().to_vec()).context("invalid UTF-8 in quoted path") } /// Collects the hunks of one blob diff while tracking per-line numbers. @@ -1807,11 +1712,6 @@ pub fn worktree_branches(workdir: &Path) -> Result> { repo_branches(&open_with_cache(workdir)?) } -/// Short names of tags, `refs/tags/*`, sorted alphabetically. -pub fn worktree_tags(workdir: &Path) -> Result> { - repo_tags(&open_with_cache(workdir)?) -} - /// Short name of the branch HEAD points to, or `None` when detached. /// /// Detached after checking out a tag or a commit directly. @@ -2291,38 +2191,6 @@ mod tests { 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()); - // 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. @@ -2977,7 +2845,7 @@ mod tests { assert!(branches.windows(2).all(|pair| pair[0] <= pair[1]), "sorted"); assert_eq!( - worktree_tags(dir).expect("tags"), + repo_tags(&repo).expect("tags"), vec!["v0.9".to_string(), "v1.0".to_string()] ); } diff --git a/crates/signed_nostr/Cargo.toml b/crates/signed_nostr/Cargo.toml index 4ee5f32..5308c8e 100644 --- a/crates/signed_nostr/Cargo.toml +++ b/crates/signed_nostr/Cargo.toml @@ -5,9 +5,6 @@ edition.workspace = true publish.workspace = true [dependencies] -signed_core = { path = "../signed_core" } - -nostr.workspace = true nostr-sdk.workspace = true nostr-connect.workspace = true nostr-gossip-memory.workspace = true diff --git a/crates/signed_nostr/src/update.rs b/crates/signed_nostr/src/update.rs index 5784d21..b5a070f 100644 --- a/crates/signed_nostr/src/update.rs +++ b/crates/signed_nostr/src/update.rs @@ -7,7 +7,6 @@ pub struct Update { /// First `a` tag value of the event, if any, for example the repository coordinate. pub coordinate: Option, pub author: PublicKey, - pub event_id: EventId, } impl Update { @@ -19,7 +18,6 @@ impl Update { kind: event.kind, coordinate, author: event.pubkey, - event_id: event.id, } } } diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 4735668..464eb9e 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1,5 +1,6 @@ use std::collections::hash_map::DefaultHasher; use std::collections::{HashMap, HashSet}; +use std::future::Future; use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -45,8 +46,6 @@ pub enum BackendEvent { PassphraseRequired, /// The signer changed on login, logout or account switch. SignerChanged, - /// Relay bootstrap finished. - Connected, /// A new event was received from a relay and stored in the database. NostrUpdate(Update), /// A negentropy sync completed. @@ -80,7 +79,6 @@ pub struct Backend { client: Client, signer: UniversalSigner, current_user: Option, - connected: bool, sync_progress: Option<(u64, u64)>, /// True when the stored credential is NIP-49 encrypted. passphrase_required: bool, @@ -151,7 +149,6 @@ impl Backend { client, signer, current_user: None, - connected: false, sync_progress: None, passphrase_required: false, recent_fetches: HashMap::new(), @@ -186,11 +183,7 @@ impl Backend { self.tasks.push(cx.spawn(async move |this, cx| { match task.await { Ok(()) => { - this.update(cx, |this, cx| { - this.connected = true; - cx.emit(BackendEvent::Connected); - cx.notify(); - })?; + this.update(cx, |_this, cx| cx.notify())?; } Err(e) => { this.update(cx, |_this, cx| cx.emit(BackendEvent::error(e.to_string())))?; @@ -993,22 +986,14 @@ impl Backend { self.tasks.push(cx.spawn(async move |this, cx| { let result = async { - let events = client.fetch_events(filters::grasp_list(public_key)).await?; - - let urls: Vec = events + let events: Vec = client + .fetch_events(filters::grasp_list(public_key)) + .await? .into_iter() - .max_by_key(|e| e.created_at) - .map(|e| { - e.tags - .iter() - .filter(|t| t.kind() == "g") - .filter_map(|t| t.content().map(str::to_owned)) - .collect() - }) - .unwrap_or_default(); + .collect(); - for url in urls { - client.add_relay(&url).await.ok(); + for url in latest_grasp_list_servers(events) { + client.add_relay(url.as_str()).await.ok(); } client.connect().await; @@ -1049,11 +1034,6 @@ impl Backend { cx.emit(BackendEvent::error(message)); } - /// Whether the relay bootstrap has completed. - pub fn is_connected(&self) -> bool { - self.connected - } - /// Progress of the in-flight negentropy sync, if any. pub fn sync_progress(&self) -> Option<(u64, u64)> { self.sync_progress @@ -1106,11 +1086,7 @@ impl Backend { self.tasks.push(cx.spawn(async move |this, cx| { match task.await { Ok(()) => { - this.update(cx, |this, cx| { - this.connected = true; - cx.emit(BackendEvent::Connected); - cx.notify(); - })?; + this.update(cx, |_this, cx| cx.notify())?; } Err(e) => { this.update(cx, |_this, cx| cx.emit(BackendEvent::error(e.to_string())))?; @@ -1120,43 +1096,6 @@ impl Backend { })); } - /// Add discovery-only relays, e.g. NIP-65 indexers, and connect to them. - pub fn add_discovery_relays(&mut self, urls: Vec, cx: &mut Context) { - let client = self.client.clone(); - - let task = cx.background_spawn(async move { - for url in urls { - client - .add_relay(&url) - .capabilities(RelayCapabilities::DISCOVERY) - .await?; - } - client.connect().await; - Ok::<(), Error>(()) - }); - - self.tasks.push(cx.spawn(async move |this, cx| { - if let Err(e) = task.await { - this.update(cx, |_this, cx| cx.emit(BackendEvent::error(e.to_string())))?; - } - Ok(()) - })); - } - - /// Start a persistent subscription. - pub fn subscribe(&mut self, filter: Filter, cx: &mut Context) { - let client = self.client.clone(); - - let task = cx.background_spawn(async move { client.subscribe(filter).await.map(|_| ()) }); - - self.tasks.push(cx.spawn(async move |this, cx| { - if let Err(e) = task.await { - this.update(cx, |_this, cx| cx.emit(BackendEvent::error(e.to_string())))?; - } - Ok(()) - })); - } - /// Whether an identical fetch started within [`FETCH_DEDUP_WINDOW`] is still recent. /// /// Records the fingerprint when returning `false`, pruning expired entries first. @@ -1298,44 +1237,11 @@ impl Backend { let client = self.client.clone(); let signer = self.signer.clone(); - cx.spawn(async move |this, cx| { + self.publish_task(cx, async move { // Sign with the current signer, broadcast and save locally. // The event is immediately visible to database queries. - let work = cx.background_spawn(async move { - let event = builder.finalize_async(&signer).await?; - 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) - }); - - 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 + let event = builder.finalize_async(&signer).await?; + broadcast_event(&client, &event).await }) } @@ -1346,25 +1252,17 @@ impl Backend { cx: &mut Context, ) -> Task> { let client = self.client.clone(); + self.publish_task(cx, async move { broadcast_event(&client, &event).await }) + } + /// Run `work` in the background, then emit its outcome as a [`BackendEvent`]. + fn publish_task( + &mut self, + cx: &mut Context, + work: impl Future> + 'static + Send, + ) -> Task> { 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; + let result = cx.background_spawn(work).await; match &result { Ok(event) => { @@ -1385,15 +1283,6 @@ impl Backend { }) } - /// Publish a NIP-34 repository announcement, kind 30617, with the current signer. - pub fn publish_announcement( - &mut self, - announcement: GitRepositoryAnnouncement, - cx: &mut Context, - ) -> Task> { - self.send(announcement.into_event_builder(), cx) - } - /// Sign, broadcast and store an event without awaiting the result. fn send_fire_and_forget(&mut self, builder: EventBuilder, cx: &mut Context) { let task = self.send(builder, cx); @@ -1433,6 +1322,25 @@ impl Backend { } } +/// Broadcast an event and fail when no relay accepted it. +/// +/// The client stores accepted events locally, visible to database queries. +async fn broadcast_event(client: &Client, event: &Event) -> Result { + 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()) +} + /// Fingerprint of a relay and filter set, for fetch dedup. /// /// Relays and filters are sorted first, so the fingerprint is order-independent. @@ -1603,8 +1511,8 @@ fn latest_grasp_list_servers(events: Vec) -> Vec { .unwrap_or_default() } -/// Resolve the user's published grasp servers. -pub(crate) async fn user_grasp_list_servers( +/// Resolve the user's published grasp servers from the local database. +pub async fn user_grasp_list_servers( client: Client, user: PublicKey, ) -> Result, Error> { diff --git a/crates/signed_state/src/checkouts.rs b/crates/signed_state/src/checkouts.rs index ce858eb..25bbb46 100644 --- a/crates/signed_state/src/checkouts.rs +++ b/crates/signed_state/src/checkouts.rs @@ -13,6 +13,7 @@ use signed_core::{Announcement, RepoAddr}; use crate::backend::{Backend, BackendEvent}; use crate::git_store::GitStore; use crate::local_repos::LocalReposStore; +use crate::refresh::{RefreshGate, RefreshRequest}; use crate::repo_list::RepoListStore; /// Delay between a refresh request and the actual re-computation. @@ -83,10 +84,8 @@ pub struct CheckoutsStore { /// /// A recompute defaults the base the same way. requested_head: HashMap>, - refreshing: bool, - refresh_dirty: bool, - /// A refresh is waiting out [`REFRESH_DEBOUNCE`]. - debouncing: bool, + /// Refresh coalescing, see [`RefreshGate`]. + refresh: RefreshGate, _subscriptions: Vec, tasks: Vec>>, } @@ -144,9 +143,7 @@ impl CheckoutsStore { push_requested: HashSet::new(), push_statuses: Arc::new(HashMap::new()), requested_head: HashMap::new(), - refreshing: false, - refresh_dirty: false, - debouncing: false, + refresh: RefreshGate::default(), _subscriptions: subscriptions, tasks: Vec::new(), }; @@ -236,22 +233,14 @@ impl CheckoutsStore { /// /// Requests arriving while a pass runs fold into a follow-up. pub fn refresh(&mut self, cx: &mut Context) { - if self.refreshing { - self.refresh_dirty = true; + if self.refresh.request() != RefreshRequest::Schedule { return; } - if self.debouncing { - return; - } - self.debouncing = true; let task = cx.spawn(async move |this, cx| { cx.background_executor().timer(REFRESH_DEBOUNCE).await; - this.update(cx, |this, cx| { - this.debouncing = false; - this.run_refresh(cx); - }) + this.update(cx, |this, cx| this.run_refresh(cx)) }); self.tasks.push(task); @@ -259,7 +248,7 @@ impl CheckoutsStore { /// One resolve and apply cycle, the debounced entry point. fn run_refresh(&mut self, cx: &mut Context) { - self.refreshing = true; + self.refresh.begin(); // Inputs snapshot, all cheap shared reads. let records = { @@ -362,7 +351,7 @@ impl CheckoutsStore { Err(_) => { // Git reads are best-effort, keep the last results. return this.update(cx, |this, _cx| { - this.refreshing = false; + this.refresh.abort(); }); } }; @@ -373,13 +362,7 @@ impl CheckoutsStore { this.push_statuses = Arc::new(push_statuses); cx.notify(); - this.refreshing = false; - if this.refresh_dirty { - this.refresh_dirty = false; - true - } else { - false - } + this.refresh.finish() })?; if again { @@ -388,8 +371,8 @@ impl CheckoutsStore { // Keep the statuses current while any repository panel is open. this.update(cx, |this, cx| { - if poll && !this.debouncing && !this.refreshing { - this.debouncing = true; + if poll && this.refresh.idle() { + this.refresh.debounce(); // Open panels get the fast cadence. // Each cycle fetches every watched checkout's remote. let delay = if this.status_requested.is_empty() { @@ -400,10 +383,7 @@ impl CheckoutsStore { let task = cx.spawn(async move |this, cx| { cx.background_executor().timer(delay).await; - this.update(cx, |this, cx| { - this.debouncing = false; - this.run_refresh(cx); - }) + this.update(cx, |this, cx| this.run_refresh(cx)) }); this.tasks.push(task); diff --git a/crates/signed_state/src/lib.rs b/crates/signed_state/src/lib.rs index 28e46e9..4b1fd57 100644 --- a/crates/signed_state/src/lib.rs +++ b/crates/signed_state/src/lib.rs @@ -3,12 +3,13 @@ mod checkouts; mod git_store; mod local_repos; mod profile; +mod refresh; mod repo; mod repo_list; use std::path::{Path, PathBuf}; -pub use backend::{Backend, BackendEvent}; +pub use backend::{Backend, BackendEvent, user_grasp_list_servers}; pub use checkouts::{CheckoutStatus, CheckoutsStore, pr_proposes_checkout}; pub use git_store::GitStore; use gpui::{App, AppContext, Entity}; @@ -18,7 +19,6 @@ pub use profile::{Profile, ProfileStore}; pub use repo::RepoStore; pub use repo_list::{RepoActivityCounts, RepoListStore}; use signed_nostr::new_backend; -pub use utils::shorten_pubkey; /// Initialize the backend and stores, and install them as globals. /// Call once at startup, before opening any window that uses the stores. diff --git a/crates/signed_state/src/refresh.rs b/crates/signed_state/src/refresh.rs new file mode 100644 index 0000000..c1cae3e --- /dev/null +++ b/crates/signed_state/src/refresh.rs @@ -0,0 +1,79 @@ +/// Refresh coalescing shared by the event stores. +/// +/// [`crate::RepoStore`], [`crate::RepoListStore`] and [`crate::CheckoutsStore`] +/// re-query their inputs on a debounce timer with the same policy: +/// a request arriving while a run is in flight is folded into a follow-up run, +/// a request arriving while the debounce timer is pending is dropped by it. +#[derive(Debug, Default)] +pub struct RefreshGate { + /// A run is in flight. + running: bool, + /// A request arrived while a run was in flight. + dirty: bool, + /// The debounce timer is pending. + debouncing: bool, +} + +/// What a refresh request decided. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RefreshRequest { + /// No run or timer covers the request, start the debounce timer. + Schedule, + /// A run or pending timer already covers the request. + Fold, +} + +impl RefreshGate { + /// Whether a run is in flight. + pub fn running(&self) -> bool { + self.running + } + + /// Whether the debounce timer is pending. + pub fn debouncing(&self) -> bool { + self.debouncing + } + + /// Whether no run is in flight and no timer is pending. + pub fn idle(&self) -> bool { + !self.running && !self.debouncing + } + + /// A new refresh request arrived. + /// + /// Folded into a follow-up run while one is in flight, dropped while the + /// debounce timer is pending, otherwise starts the timer. + pub fn request(&mut self) -> RefreshRequest { + if self.running { + self.dirty = true; + RefreshRequest::Fold + } else if self.debouncing { + RefreshRequest::Fold + } else { + self.debouncing = true; + RefreshRequest::Schedule + } + } + + /// A timer was started without a request, e.g. a poll cycle. + pub fn debounce(&mut self) { + self.debouncing = true; + } + + /// The debounce timer fired and the run starts now. + pub fn begin(&mut self) { + self.debouncing = false; + self.running = true; + } + + /// The run ended. Whether a request arrived while it ran. + pub fn finish(&mut self) -> bool { + self.running = false; + std::mem::take(&mut self.dirty) + } + + /// The run was abandoned, e.g. on error. Pending follow-up requests survive. + pub fn abort(&mut self) { + self.running = false; + } +} diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 096af7f..7778010 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -9,15 +9,15 @@ use gpui::{AppContext, AsyncApp, Context, SharedString, Subscription, Task, Weak 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, pull_request_patches, - subject_override, + Announcement, Deletions, RepoAddr, RepoStatus, filters, parse_state, pull_request_patch, + pull_request_patches, }; use crate::backend::{ Backend, BackendEvent, grasp_base_url, grasp06_prs_url, pr_clone_urls, user_grasp_list_servers, }; use crate::git_store::GitStore; +use crate::refresh::{RefreshGate, RefreshRequest}; /// Delay between a refresh request and the actual re-query. const REFRESH_DEBOUNCE: Duration = Duration::from_millis(300); @@ -34,8 +34,6 @@ const MAX_PATCH_EVENT_BYTES: usize = 60 * 1024; pub struct RepoStore { addr: RepoAddr, pub announcement: Option, - /// `(refname, commit-id)` pairs from the latest state announcement. - pub refs: Vec<(String, String)>, /// Branch pointed to by `HEAD` in the latest state announcement. pub head: Option, pub issues: Vec, @@ -49,11 +47,6 @@ pub struct RepoStore { /// Computed with [`Self::status_by_root`] on every refresh. open_issue_count: usize, open_pr_count: usize, - /// Kind-1624 cover notes and kind-1985 label events. - /// - /// They reference this repository's roots, used by ngit and GitWorkshop. - cover_notes: Vec, - labels: Vec, /// Incremented on every applied refresh. /// /// Views key their derived-data caches to it instead of recomputing on every render. @@ -71,12 +64,9 @@ pub struct RepoStore { /// Root events, issues, patches and PRs, already fetched per root. /// /// The per-root fetches cover NIP-22 comments and statuses without an `a` tag. - /// Also kind-1624 cover notes and kind-1985 labels. root_fetches: HashSet, - refreshing: bool, - refresh_dirty: bool, - /// A refresh is waiting out [`REFRESH_DEBOUNCE`]. - debouncing: bool, + /// Refresh coalescing, see [`RefreshGate`]. + refresh: RefreshGate, tasks: Vec>>, _subscription: Subscription, } @@ -101,10 +91,8 @@ impl RepoStore { // Status events may omit their `a` tag, NIP-34. // Any status event may reference a root of this repository. let status = RepoStatus::from_kind(update.kind).is_some(); - // Cover notes and labels carry no `a` tag either. - let annotation = update.kind == COVER_NOTE_KIND || update.kind == Kind::Label; - deletion || coordinate || (author && kind) || comment || status || annotation + deletion || coordinate || (author && kind) || comment || status } BackendEvent::Published(event) => { let kind = event.kind == Kind::GitRepoAnnouncement; @@ -128,7 +116,6 @@ impl RepoStore { let mut store = Self { addr, announcement: None, - refs: Vec::new(), head: None, issues: Vec::new(), patches: Vec::new(), @@ -137,16 +124,12 @@ impl RepoStore { status_by_root: HashMap::new(), open_issue_count: 0, open_pr_count: 0, - cover_notes: Vec::new(), - labels: Vec::new(), version: 0, last_error: None, last_warning: None, repo_relays: HashSet::new(), root_fetches: HashSet::new(), - refreshing: false, - refresh_dirty: false, - debouncing: false, + refresh: RefreshGate::default(), _subscription: subscription, tasks: Vec::new(), }; @@ -225,22 +208,14 @@ impl RepoStore { /// Re-query the local database and update all fields. pub fn refresh(&mut self, cx: &mut Context) { - if self.refreshing { - self.refresh_dirty = true; + if self.refresh.request() != RefreshRequest::Schedule { return; } - if self.debouncing { - return; - } - self.debouncing = true; let task = cx.spawn(async move |this, cx| { cx.background_executor().timer(REFRESH_DEBOUNCE).await; - this.update(cx, |this, cx| { - this.debouncing = false; - this.run_refresh(cx); - }) + this.update(cx, |this, cx| this.run_refresh(cx)) }); self.tasks.retain(|task| !task.is_ready()); @@ -248,7 +223,7 @@ impl RepoStore { } fn run_refresh(&mut self, cx: &mut Context) { - self.refreshing = true; + self.refresh.begin(); let backend = Backend::global(cx); let client = backend.read(cx).client(); @@ -282,7 +257,6 @@ impl RepoStore { let (mut issues, mut patches, mut pull_requests, mut statuses, mut comments) = (Vec::new(), Vec::new(), Vec::new(), Vec::new(), Vec::new()); - let (mut cover_notes, mut labels): (Vec, Vec) = (Vec::new(), Vec::new()); for event in activity { if deletions.is_deleted(&event) { @@ -338,35 +312,11 @@ impl RepoStore { // Cover notes, 1624, and label events, 1985, carry no `a` tag. // 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) { - continue; - } - if event.kind == COVER_NOTE_KIND && seen_cover_notes.insert(event.id) { - cover_notes.push(event); - } else if event.kind == Kind::Label && seen_labels.insert(event.id) { - labels.push(event); - } - } - } - + // The events are only stored for interop and nothing displays them. sort_newest_first(&mut issues); sort_newest_first(&mut patches); sort_newest_first(&mut pull_requests); sort_oldest_first(&mut comments); - sort_newest_first(&mut cover_notes); - sort_newest_first(&mut labels); // Resolve every root's status once here. // Render paths do HashMap lookups instead of per-root status scans. @@ -402,8 +352,6 @@ impl RepoStore { open_issue_count, open_pr_count, comments, - cover_notes, - labels, )) }); @@ -420,13 +368,11 @@ impl RepoStore { open_issue_count, open_pr_count, comments, - cover_notes, - labels, ) = match work.await { Ok(data) => data, Err(e) => { return this.update(cx, |this, cx| { - this.refreshing = false; + this.refresh.abort(); this.last_error = Some(e.to_string()); cx.notify(); }); @@ -445,8 +391,7 @@ impl RepoStore { .unwrap_or_default(); this.connect_announced_relays(&relays, cx); - if let Some((refs, head)) = state { - this.refs = refs; + if let Some((_, head)) = state { this.head = head; } @@ -457,11 +402,9 @@ impl RepoStore { this.status_by_root = status_by_root; this.open_issue_count = open_issue_count; this.open_pr_count = open_pr_count; - this.cover_notes = cover_notes; - this.labels = labels; this.version = this.version.wrapping_add(1); - // Comments, statuses without an `a` tag, cover notes and labels. + // Comments and statuses without an `a` tag. // None are addressed to the repository. // Fetch them by the root events they reference. // Use the bootstrap relays and the relays this repository announced. @@ -482,11 +425,9 @@ impl RepoStore { if !new_roots.is_empty() { this.root_fetches.extend(new_roots.iter().copied()); // Batch the per-root filters. - // One statuses filter and one annotations filter cover all new roots. // One filter per root costs a negentropy reconciliation per relay. 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); @@ -498,13 +439,7 @@ impl RepoStore { cx.notify(); - this.refreshing = false; - if this.refresh_dirty { - this.refresh_dirty = false; - true - } else { - false - } + this.refresh.finish() })?; // Requests that arrived while the refresh was running. @@ -528,40 +463,6 @@ impl RepoStore { self.version } - /// The effective cover note of `root`, kind 1624, if any. - pub fn cover_note_of(&self, root: &Event) -> Option<&Event> { - let maintainers = self - .announcement - .as_ref() - .map(Announcement::effective_maintainers) - .unwrap_or_default(); - - cover_note(root, &self.cover_notes, &maintainers) - } - - /// The effective hashtag labels of `root`. - pub fn labels_of(&self, root: &Event) -> Vec { - let maintainers = self - .announcement - .as_ref() - .map(Announcement::effective_maintainers) - .unwrap_or_default(); - - let (labels, _) = labels_and_subject(root, &self.labels, &maintainers); - labels - } - - /// The effective subject or title override of `root`, if any. - pub fn subject_of(&self, root: &Event) -> Option { - let maintainers = self - .announcement - .as_ref() - .map(Announcement::effective_maintainers) - .unwrap_or_default(); - - subject_override(root, &self.labels, &maintainers) - } - /// Number of open issues. /// /// Issues whose resolved status is [`RepoStatus::Open`]. @@ -1067,58 +968,6 @@ impl RepoStore { self.send(builder, cx); } - /// Publish a repository state announcement - pub fn publish_state(&mut self, 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 publish repository state".into()); - cx.notify(); - return; - }; - - if !self.is_author(&user) { - self.last_error = Some("Only the repository owner can publish state".into()); - cx.notify(); - return; - } - - let cache = GitStore::global(cx).cache().clone(); - let addr = self.addr.clone(); - let clone_urls: Vec = self - .announcement - .as_ref() - .map(|a| a.clone.iter().map(ToString::to_string).collect()) - .unwrap_or_default(); - - let work = cx.background_spawn(async move { - let repo = cache.ensure_clone(&addr, &clone_urls)?; - signed_git::repo_ref_state(&repo) - }); - - self.tasks.push(cx.spawn(async move |this, cx| { - let state = match work.await { - Ok(state) => state, - Err(e) => { - return this.update(cx, |this, cx| { - this.last_error = Some(e.to_string()); - cx.notify(); - }); - } - }; - - this.update(cx, |this, cx| { - let builder = - build_state(&this.addr.identifier, &state.refs, state.head.as_deref()); - this.send(builder, cx); - })?; - - Ok(()) - })); - } - /// Merge a pull request. pub fn merge_pull_request(&mut self, root: &Event, cx: &mut Context) { self.last_error = None; diff --git a/crates/signed_state/src/repo_list.rs b/crates/signed_state/src/repo_list.rs index 3aa6861..56a3e8a 100644 --- a/crates/signed_state/src/repo_list.rs +++ b/crates/signed_state/src/repo_list.rs @@ -8,6 +8,7 @@ use nostr_sdk::prelude::*; use signed_core::{Announcement, Deletions, RepoAddr, filters, repo_addr}; use crate::backend::{Backend, BackendEvent}; +use crate::refresh::{RefreshGate, RefreshRequest}; /// Delay between a refresh request and the actual re-query. /// @@ -53,10 +54,8 @@ pub struct RepoListStore { /// Used for the Popular ranking of the explore list. pub counts: Arc>, author: Option, - refreshing: bool, - refresh_dirty: bool, - /// A refresh is waiting out [`REFRESH_DEBOUNCE`]. - debouncing: bool, + /// Refresh coalescing, see [`RefreshGate`]. + refresh: RefreshGate, tasks: Vec>>, _subscription: Subscription, } @@ -118,9 +117,7 @@ impl RepoListStore { last_activity: Arc::new(HashMap::new()), counts: Arc::new(HashMap::new()), author, - refreshing: false, - refresh_dirty: false, - debouncing: false, + refresh: RefreshGate::default(), _subscription: subscription, tasks: Vec::new(), }; @@ -132,13 +129,6 @@ impl RepoListStore { store } - /// Scope the list to an author, or clear the scope with `None`. - pub fn set_author(&mut self, author: Option, cx: &mut Context) { - self.author = author; - self.subscribe_remote(cx); - self.refresh(cx); - } - /// Negentropy-sync announcements with the bootstrap relays. fn subscribe_remote(&mut self, cx: &mut Context) { let backend = Backend::global(cx); @@ -160,9 +150,9 @@ impl RepoListStore { /// Query the local database immediately, no debounce. /// Stored announcements appear as soon as the app opens. fn refresh_initial(&mut self, cx: &mut Context) { - debug_assert!(!self.debouncing); - if self.refreshing { - self.refresh_dirty = true; + debug_assert!(!self.refresh.debouncing()); + if self.refresh.running() { + self.refresh.request(); return; } self.run_refresh(cx); @@ -170,22 +160,14 @@ impl RepoListStore { /// Re-query the local database. pub fn refresh(&mut self, cx: &mut Context) { - if self.refreshing { - self.refresh_dirty = true; + if self.refresh.request() != RefreshRequest::Schedule { return; } - if self.debouncing { - return; - } - self.debouncing = true; let task = cx.spawn(async move |this, cx| { cx.background_executor().timer(REFRESH_DEBOUNCE).await; - this.update(cx, |this, cx| { - this.debouncing = false; - this.run_refresh(cx); - }) + this.update(cx, |this, cx| this.run_refresh(cx)) }); self.tasks.push(task); @@ -193,7 +175,7 @@ impl RepoListStore { /// One query and apply cycle, the debounced entry point. fn run_refresh(&mut self, cx: &mut Context) { - self.refreshing = true; + self.refresh.begin(); let backend = Backend::global(cx); let client = backend.read(cx).client(); @@ -315,7 +297,7 @@ impl RepoListStore { // Database errors are transient, keep the last list. Err(_) => { return this.update(cx, |this, _cx| { - this.refreshing = false; + this.refresh.abort(); }); } }; @@ -326,13 +308,7 @@ impl RepoListStore { this.counts = Arc::new(counts); cx.notify(); - this.refreshing = false; - if this.refresh_dirty { - this.refresh_dirty = false; - true - } else { - false - } + this.refresh.finish() })?; // Requests that arrived while the refresh was running. diff --git a/crates/signed_ui/src/image_cache.rs b/crates/signed_ui/src/image_cache.rs deleted file mode 100644 index 26da318..0000000 --- a/crates/signed_ui/src/image_cache.rs +++ /dev/null @@ -1,139 +0,0 @@ -use std::collections::{HashMap, VecDeque}; -use std::mem::take; - -use futures::FutureExt; -use gpui::{ - App, AppContext, Asset, AssetLogger, ElementId, Entity, ImageAssetLoader, ImageCache, - ImageCacheItem, ImageCacheProvider, ImageSource, Resource, hash, -}; - -/// Default number of images each view's cache retains. -/// Loading a new image evicts the least recently used entry once this is reached. -pub const MAX_IMAGES: usize = 128; - -pub fn image_cache(id: impl Into, max_items: usize) -> AppImageCacheProvider { - AppImageCacheProvider { - id: id.into(), - max_items, - } -} - -pub struct AppImageCacheProvider { - id: ElementId, - max_items: usize, -} - -impl ImageCacheProvider for AppImageCacheProvider { - fn provide(&mut self, window: &mut gpui::Window, cx: &mut App) -> gpui::AnyImageCache { - window - .with_global_id(self.id.clone(), |id, window| { - window.with_element_state(id, |cache, _| { - let cache = cache.unwrap_or_else(|| AppImageCache::new(self.max_items, cx)); - (cache.clone(), cache) - }) - }) - .into() - } -} - -pub struct AppImageCache { - max_items: usize, - usage_list: VecDeque, - cache: HashMap, -} - -impl AppImageCache { - pub fn new(max_items: usize, cx: &mut App) -> Entity { - cx.new(|cx| { - log::info!("Creating AppImageCacheProvider"); - cx.on_release(|this: &mut Self, cx| { - for (ix, (mut image, resource)) in take(&mut this.cache) { - if let Some(Ok(image)) = image.get() { - log::info!("Dropping image {ix}"); - cx.drop_image(image, None); - } - ImageSource::Resource(resource).remove_asset(cx); - } - }) - .detach(); - - AppImageCache { - max_items, - usage_list: VecDeque::with_capacity(max_items), - cache: HashMap::with_capacity(max_items), - } - }) - } -} - -impl ImageCache for AppImageCache { - fn load( - &mut self, - resource: &Resource, - window: &mut gpui::Window, - cx: &mut gpui::App, - ) -> Option, gpui::ImageCacheError>> { - let hash = hash(resource); - - if let Some(item) = self.cache.get_mut(&hash) { - let current_idx = self - .usage_list - .iter() - .position(|item| *item == hash) - .expect("cache has an item usage_list doesn't"); - - self.usage_list.remove(current_idx); - self.usage_list.push_front(hash); - - return item.0.get(); - } - - let load_future = AssetLogger::::load(resource.clone(), cx); - let task = cx.background_executor().spawn(load_future).shared(); - - if self.usage_list.len() >= self.max_items { - log::info!("Image cache is full, evicting oldest item"); - - if let Some(oldest) = self.usage_list.pop_back() { - let mut image = self - .cache - .remove(&oldest) - .expect("usage_list has an item cache doesn't"); - - if let Some(Ok(image)) = image.0.get() { - log::info!("requesting image to be dropped"); - cx.drop_image(image, Some(window)); - } - - ImageSource::Resource(image.1).remove_asset(cx); - } - } - - self.cache.insert( - hash, - ( - gpui::ImageCacheItem::Loading(task.clone()), - resource.clone(), - ), - ); - self.usage_list.push_front(hash); - - let entity = window.current_view(); - - window - .spawn(cx, async move |cx| { - let result = task.await; - - if let Err(err) = result { - log::error!("error loading image into cache: {:?}", err); - } - - cx.on_next_frame(move |_, cx| { - cx.notify(entity); - }); - }) - .detach(); - - None - } -} diff --git a/crates/signed_ui/src/lib.rs b/crates/signed_ui/src/lib.rs index f9cfafe..7f63693 100644 --- a/crates/signed_ui/src/lib.rs +++ b/crates/signed_ui/src/lib.rs @@ -10,12 +10,10 @@ mod tree_row; mod user_avatar; pub mod copy_row; -pub mod image_cache; pub mod util; pub use copy_row::{copy_row, menu_copy_row}; pub use dropdown_button::DropdownButton; -pub use image_cache::{MAX_IMAGES, image_cache}; pub use nav_item::NavItem; pub use pixel_avatar::PixelAvatar; pub use placeholder::placeholder; diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index f5a2fc3..59d09f5 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -23,6 +23,5 @@ gix.workspace = true nostr.workspace = true anyhow.workspace = true -chrono.workspace = true futures.workspace = true log.workspace = true diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index 59ed328..e475053 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -3,7 +3,6 @@ mod workspace; use gpui::{App, AppContext, Entity, Window}; use gpui_component::Root; -pub use signed_ui::image_cache; pub use views::{RepoListView, SidebarPanel}; pub use workspace::Workspace; diff --git a/crates/workspace/src/views/dialog_state.rs b/crates/workspace/src/views/dialog_state.rs new file mode 100644 index 0000000..b303490 --- /dev/null +++ b/crates/workspace/src/views/dialog_state.rs @@ -0,0 +1,36 @@ +use gpui::prelude::*; +use gpui::{AnyElement, App, SharedString, div}; +use gpui_component::ActiveTheme; + +/// Progress of an async dialog action: a busy flag disabling the form, +/// and an error line shown under it. +#[derive(Debug, Default)] +pub struct DialogProgress { + pub busy: bool, + pub error: Option, +} + +impl DialogProgress { + /// An action started, disable the form and clear the previous error. + pub fn begin(&mut self) { + self.busy = true; + self.error = None; + } + + /// An action failed, re-enable the form and surface `message`. + pub fn fail(&mut self, message: impl Into) { + self.busy = false; + self.error = Some(message.into()); + } +} + +/// The shared error line under a dialog form, `None` when there is no error. +pub fn error_row(error: &Option, cx: &App) -> Option { + error.as_ref().map(|message| { + div() + .text_sm() + .text_color(cx.theme().danger) + .child(message.clone()) + .into_any_element() + }) +} diff --git a/crates/workspace/src/views/mod.rs b/crates/workspace/src/views/mod.rs index 1ec23e7..830a84a 100644 --- a/crates/workspace/src/views/mod.rs +++ b/crates/workspace/src/views/mod.rs @@ -1,3 +1,4 @@ +mod dialog_state; mod repo_detail; mod repo_list; pub(crate) mod sidebar; diff --git a/crates/workspace/src/views/repo_detail/diff.rs b/crates/workspace/src/views/repo_detail/diff.rs index 201b913..a03da9b 100644 --- a/crates/workspace/src/views/repo_detail/diff.rs +++ b/crates/workspace/src/views/repo_detail/diff.rs @@ -325,7 +325,7 @@ pub struct CommitDiffView { error: Option, /// Changed-files explorer and per-file diff, also used by the new PR panel's compare view. pane: Entity, - /// In-flight tasks, pruned on every push, see [`helpers::track`]. + /// In-flight tasks, pruned on every push. tasks: Vec>>, } diff --git a/crates/workspace/src/views/repo_detail/helpers.rs b/crates/workspace/src/views/repo_detail/helpers.rs index 2e96102..bf0cefd 100644 --- a/crates/workspace/src/views/repo_detail/helpers.rs +++ b/crates/workspace/src/views/repo_detail/helpers.rs @@ -1,15 +1,22 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; +use assets::CustomIconName; use gpui::prelude::*; -use gpui::{AnyElement, App, SharedString, div, px}; +use gpui::{AnyElement, App, Entity, SharedString, div, px}; +use gpui_component::button::{Button, ButtonVariants}; +use gpui_component::input::{Textarea, TextareaState}; use gpui_component::menu::PopupMenu; +use gpui_component::tag::Tag; use gpui_component::tree::TreeItem; -use gpui_component::{ActiveTheme, h_flex}; +use gpui_component::{ActiveTheme, Icon, Sizable, StyledExt, h_flex, v_flex}; use nostr::nips::nip19::{Nip19Coordinate, ToBech32}; +use nostr::prelude::{Event, EventId, PublicKey}; use signed_core::Announcement; use signed_git::{DiffHunk, DiffLine, DiffLineKind, FileDiff}; -use signed_ui::{menu_copy_row, middle_truncate}; +use signed_state::{ProfileStore, RepoStore}; +use signed_ui::{UserAvatar, menu_copy_row, middle_truncate}; +use utils::relative_time; pub(super) struct TreeItemSeed { /// Path of the node, relative to the worktree root. @@ -348,6 +355,229 @@ pub(super) fn find_item<'a>(items: &'a [TreeItem], id: Option<&str>) -> Option<& }) } +/// The root issue events of a repo store, for the shared detail sections. +pub(super) fn issue_roots(store: &RepoStore) -> &[Event] { + &store.issues +} + +/// The root pull request events of a repo store, for the shared detail sections. +pub(super) fn pr_roots(store: &RepoStore) -> &[Event] { + &store.pull_requests +} + +/// Section heading of a detail sidebar, shared by the issue and PR panels. +pub(super) fn sidebar_title(text: &str, cx: &App) -> AnyElement { + div() + .text_xs() + .font_semibold() + .text_color(cx.theme().muted_foreground) + .child(text.to_string()) + .into_any_element() +} + +/// Right sidebar with participants and labels of a root event, issue or PR. +pub(super) fn sidebar_section( + store: &Entity, + id: EventId, + roots: fn(&RepoStore) -> &[Event], + top_gap: bool, + cx: &App, +) -> AnyElement { + let store = store.read(cx); + let Some(root) = roots(store).iter().find(|event| event.id == id) else { + // The caller bails out when the root is missing. + return div().into_any_element(); + }; + let profile_store = ProfileStore::global(cx); + + // Participants, the root author plus everyone who commented. + let mut participants: Vec = vec![root.pubkey]; + participants.extend(store.comments_of(&root.id).map(|comment| comment.pubkey)); + participants.sort_by_key(PublicKey::to_hex); + participants.dedup(); + + // Labels are NIP-34 `t` hashtag tags on the event. + let labels: Vec = root.tags.hashtags().map(|tag| tag.to_string()).collect(); + + v_flex() + .w(px(240.)) + .h_full() + .flex_none() + .px_4() + .gap_4() + .border_l(px(1.)) + .border_color(cx.theme().sidebar_border) + .child( + v_flex() + .when(top_gap, |this| this.mt_4()) + .gap_2() + .child(sidebar_title("Participants", cx)) + .children(participants.iter().map(|pubkey| { + let profile = profile_store.read(cx).get(pubkey); + let name = profile.name(); + let picture = profile.picture(); + + h_flex() + .gap_1() + .items_center() + .child(UserAvatar::new(name.clone()).picture(picture)) + .child(div().text_sm().truncate().text_ellipsis().child(name)) + .into_any_element() + })), + ) + .child( + v_flex() + .gap_2() + .child(sidebar_title("Labels", cx)) + .map(|this| { + if labels.is_empty() { + this.child( + div() + .text_sm() + .text_color(cx.theme().muted_foreground) + .child("None yet."), + ) + } else { + this.child(h_flex().gap_1().children({ + let mut items = vec![]; + + for label in labels.iter() { + items.push( + Tag::secondary() + .outline() + .xsmall() + .child(SharedString::from(label)), + ); + } + + items + })) + } + }), + ) + .into_any_element() +} + +/// The comments on a root event, issue or PR, one card per comment. +/// +/// Comment bodies become shared strings once per comment, not per render. +pub(super) fn comments_section( + store: &Entity, + root: EventId, + contents: &mut HashMap, + cx: &App, +) -> AnyElement { + let store = store.read(cx); + let comments: Vec<&Event> = store.comments_of(&root).collect(); + let title = SharedString::from(format!("Discussions {}", comments.len())); + + v_flex() + .gap_4() + .child(div().text_xs().font_semibold().child(title)) + .children(comments.iter().map(|comment| { + let profile = ProfileStore::global(cx).read(cx).get(&comment.pubkey); + let author = profile.name(); + let picture = profile.picture(); + let age = relative_time(comment.created_at); + let content = contents + .entry(comment.id) + .or_insert_with(|| SharedString::from(comment.content.clone())) + .clone(); + + v_flex() + .gap_1() + .p_3() + .border_1() + .border_color(cx.theme().border) + .rounded(cx.theme().radius) + .child( + h_flex() + .gap_2() + .text_sm() + .child( + h_flex() + .gap_1() + .child(UserAvatar::new(author.clone()).picture(picture)) + .child(author), + ) + .child( + div() + .text_color(cx.theme().muted_foreground) + .child("commented"), + ) + .child( + div() + .text_color(cx.theme().muted_foreground) + .child(SharedString::from(age)), + ), + ) + .child(div().text_sm().child(content)) + })) + .into_any_element() +} + +/// The comment form posting to an issue or PR root event. +/// +/// `roots` selects the root's list within the store, issues or pull requests. +pub(super) fn comment_form( + store: &Entity, + root: EventId, + roots: fn(&RepoStore) -> &[Event], + comment_input: &Entity, + button_id: &'static str, + cx: &App, +) -> AnyElement { + let comment_input = comment_input.clone(); + let store = store.clone(); + + v_flex() + .gap_2() + .child( + Textarea::new(&comment_input) + .h_24() + .text_color(cx.theme().muted_foreground) + .bg(cx.theme().muted), + ) + .child( + h_flex() + .justify_between() + .child( + h_flex() + .gap_1() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(Icon::new(CustomIconName::Markdown).small()) + .child("Markdown is supported"), + ) + .child( + Button::new(button_id) + .primary() + .label("Comment") + .tooltip("Post comment") + .on_click(move |_event, window, cx| { + let content = comment_input.read(cx).value().trim().to_string(); + if content.is_empty() { + return; + } + let Some(root) = roots(store.read(cx)) + .iter() + .find(|event| event.id == root) + .cloned() + else { + return; + }; + store.update(cx, |store, cx| { + store.comment(&root, content, cx); + }); + comment_input.update(cx, |input, cx| { + input.set_value("", window, cx); + }); + }), + ), + ) + .into_any_element() +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/workspace/src/views/repo_detail/init_dialog.rs b/crates/workspace/src/views/repo_detail/init_dialog.rs index b98a478..02083bf 100644 --- a/crates/workspace/src/views/repo_detail/init_dialog.rs +++ b/crates/workspace/src/views/repo_detail/init_dialog.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use assets::CustomIconName; use gpui::prelude::*; -use gpui::{App, Entity, SharedString, WeakEntity, Window, div, px}; +use gpui::{App, Entity, WeakEntity, Window, px}; use gpui_base::h_flex; use gpui_base::input::TextareaState; use gpui_component::button::{Button, ButtonVariants}; @@ -14,16 +14,13 @@ use settings::SettingsStore; use signed_state::Backend; use super::RepoDetailView; +use crate::views::dialog_state::{DialogProgress, error_row}; use crate::views::sidebar::grasp_servers::{ GraspServersState, grasp_servers_field, load_user_grasp_servers, }; /// Shared state for the Init dialog, so async results can be rendered. -#[derive(Default)] -pub struct InitRepoState { - pub busy: bool, - pub error: Option, -} +pub type InitRepoState = DialogProgress; /// Open the Init dialog for the local repository at `local_path`. pub fn open( @@ -113,9 +110,7 @@ pub fn open( ) .child(grasp_servers_field(&grasp_state, &relay_input, cx)), ) - .children(error.map(|message| { - div().text_sm().text_color(cx.theme().danger).child(message) - })) + .children(error_row(&error, cx)) .child( DialogFooter::new().justify_end().child( Button::new("init") @@ -169,23 +164,16 @@ fn init_repository( let servers = grasp_state.read(cx).grasp_servers.clone(); if name.is_empty() { - state.update(cx, |state, _| { - state.error = Some("Repository name is required".into()); - }); + state.update(cx, |state, _| state.fail("Repository name is required")); return; } if servers.is_empty() { - state.update(cx, |state, _| { - state.error = Some("Add at least one grasp server".into()); - }); + state.update(cx, |state, _| state.fail("Add at least one grasp server")); return; } - state.update(cx, |state, _| { - state.busy = true; - state.error = None; - }); + state.update(cx, |state, _| state.begin()); let backend = Backend::global(cx); let task = backend.update(cx, |backend, cx| { @@ -210,10 +198,7 @@ fn init_repository( } Err(e) => { cx.update_window(handle, |_, _window, cx| { - state.update(cx, |state, _| { - state.busy = false; - state.error = Some(e.to_string().into()); - }); + state.update(cx, |state, _| state.fail(e.to_string())); }) .ok(); } diff --git a/crates/workspace/src/views/repo_detail/issue_detail.rs b/crates/workspace/src/views/repo_detail/issue_detail.rs index 972b87f..ebe12d3 100644 --- a/crates/workspace/src/views/repo_detail/issue_detail.rs +++ b/crates/workspace/src/views/repo_detail/issue_detail.rs @@ -1,24 +1,22 @@ use std::collections::HashMap; -use assets::CustomIconName; use dock::{BasePanel, Panel, PanelEvent}; use gpui::prelude::*; use gpui::{ - AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Render, SharedString, - Window, div, px, relative, + App, Context, Entity, EventEmitter, FocusHandle, Focusable, Render, SharedString, Window, div, + relative, }; -use gpui_component::button::{Button, ButtonVariants}; -use gpui_component::input::{Textarea, TextareaState}; +use gpui_component::input::TextareaState; use gpui_component::scroll::ScrollableElement; -use gpui_component::tag::Tag; -use gpui_component::{ActiveTheme, Icon, Sizable, StyledExt, h_flex, v_flex}; -use nostr::prelude::{Event, EventId, PublicKey}; +use gpui_component::{ActiveTheme, StyledExt, h_flex, v_flex}; +use nostr::prelude::EventId; use signed_core::activity_subject; use signed_state::{ProfileStore, RepoStore}; -use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{UserAvatar, placeholder, status_badge}; use utils::relative_time; +use super::helpers::{comment_form, comments_section, issue_roots, sidebar_section}; + /// Detail panel of a single issue. pub struct IssueDetailView { /// Repo store holding the issues and their statuses. @@ -48,189 +46,6 @@ impl IssueDetailView { contents: HashMap::new(), } } - - fn render_sidebar(&self, cx: &mut Context) -> impl IntoElement { - let profile_store = ProfileStore::global(cx); - let store = self.store.read(cx); - - let Some(issue) = store.issues.iter().find(|issue| issue.id == self.issue_id) else { - // `render` already bails out when the issue is missing. - return div().into_any_element(); - }; - - // Participants, the issue author plus everyone who commented. - let mut participants: Vec = vec![issue.pubkey]; - participants.extend(store.comments_of(&issue.id).map(|comment| comment.pubkey)); - participants.sort_by_key(PublicKey::to_hex); - participants.dedup(); - - // Issue labels are NIP-34 `t` hashtag tags on the event. - let labels: Vec = issue.tags.hashtags().map(|tag| tag.to_string()).collect(); - - v_flex() - .w(px(240.)) - .h_full() - .flex_none() - .px_4() - .gap_4() - .border_l(px(1.)) - .border_color(cx.theme().sidebar_border) - .child( - v_flex() - .gap_2() - .child(sidebar_title("Participants", cx)) - .children(participants.iter().map(|pubkey| { - let profile = profile_store.read(cx).get(pubkey); - let name = profile.name(); - let picture = profile.picture(); - - h_flex() - .gap_1() - .items_center() - .child(UserAvatar::new(name.clone()).picture(picture)) - .child(div().text_sm().truncate().text_ellipsis().child(name)) - .into_any_element() - })), - ) - .child( - v_flex() - .gap_2() - .child(sidebar_title("Labels", cx)) - .map(|this| { - if labels.is_empty() { - this.child( - div() - .text_sm() - .text_color(cx.theme().muted_foreground) - .child("None yet."), - ) - } else { - this.child(h_flex().gap_1().children({ - let mut items = vec![]; - - for label in labels.iter() { - items.push( - Tag::secondary() - .outline() - .xsmall() - .child(SharedString::from(label)), - ); - } - - items - })) - } - }), - ) - .into_any_element() - } - - fn render_comments(&mut self, id: &EventId, cx: &mut Context) -> impl IntoElement { - let store = self.store.read(cx); - let comments: Vec<&Event> = store.comments_of(id).collect(); - let title = SharedString::from(format!("Discussions {}", comments.len())); - - v_flex() - .gap_4() - .child(div().text_xs().font_semibold().child(title)) - .children(comments.iter().map(|comment| { - let profile = ProfileStore::global(cx).read(cx).get(&comment.pubkey); - let author = profile.name(); - let picture = profile.picture(); - let age = relative_time(comment.created_at); - // Comment bodies become shared strings once per comment, not per render. - let content = self - .contents - .entry(comment.id) - .or_insert_with(|| SharedString::from(comment.content.clone())) - .clone(); - - v_flex() - .gap_1() - .p_3() - .border_1() - .border_color(cx.theme().border) - .rounded(cx.theme().radius) - .child( - h_flex() - .gap_2() - .text_sm() - .child( - h_flex() - .gap_1() - .child(UserAvatar::new(author.clone()).picture(picture)) - .child(author), - ) - .child( - div() - .text_color(cx.theme().muted_foreground) - .child("commented"), - ) - .child( - div() - .text_color(cx.theme().muted_foreground) - .child(SharedString::from(age)), - ), - ) - .child(div().text_sm().child(content)) - })) - .into_any_element() - } - - fn render_form(&mut self, id: &EventId, cx: &mut Context) -> impl IntoElement { - let comment_input = self.comment_input.clone(); - let store = self.store.clone(); - let id = id.to_owned(); - - v_flex() - .gap_2() - .child( - Textarea::new(&self.comment_input) - .h_24() - .text_color(cx.theme().muted_foreground) - .bg(cx.theme().muted), - ) - .child( - h_flex() - .justify_between() - .child( - h_flex() - .gap_1() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(Icon::new(CustomIconName::Markdown).small()) - .child("Markdown is supported"), - ) - .child( - Button::new("comment") - .primary() - .label("Comment") - .tooltip("Post comment") - .on_click(move |_event, window, cx| { - let content = comment_input.read(cx).value().trim().to_string(); - if content.is_empty() { - return; - } - let Some(root) = store - .read(cx) - .issues - .iter() - .find(|issue| issue.id == id) - .cloned() - else { - return; - }; - store.update(cx, |store, cx| { - store.comment(&root, content, cx); - }); - comment_input.update(cx, |input, cx| { - input.set_value("", window, cx); - }); - }), - ), - ) - .into_any_element() - } } impl BasePanel for IssueDetailView { @@ -300,7 +115,7 @@ impl Render for IssueDetailView { }; h_flex() - .image_cache(image_cache("issue-detail", MAX_IMAGES)) + .image_cache(gpui::retain_all("issue-detail")) .id("issue-detail") .size_full() .child( @@ -357,20 +172,29 @@ impl Render for IssueDetailView { ) .child(div().text_sm().child(content)), ) - .child(self.render_comments(&issue_id, cx)) - .child(self.render_form(&issue_id, cx)), + .child(comments_section( + &self.store, + issue_id, + &mut self.contents, + cx, + )) + .child(comment_form( + &self.store, + issue_id, + issue_roots, + &self.comment_input, + "comment", + cx, + )), ), ) - .child(self.render_sidebar(cx)) + .child(sidebar_section( + &self.store, + issue_id, + issue_roots, + false, + cx, + )) .into_any_element() } } - -fn sidebar_title(text: &str, cx: &App) -> AnyElement { - div() - .text_xs() - .font_semibold() - .text_color(cx.theme().muted_foreground) - .child(text.to_string()) - .into_any_element() -} diff --git a/crates/workspace/src/views/repo_detail/issues.rs b/crates/workspace/src/views/repo_detail/issues.rs index 3886b3c..210d315 100644 --- a/crates/workspace/src/views/repo_detail/issues.rs +++ b/crates/workspace/src/views/repo_detail/issues.rs @@ -18,7 +18,6 @@ use gpui_component::{ use nostr::prelude::EventId; 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 utils::relative_time; @@ -363,7 +362,7 @@ impl Render for IssuesView { v_flex() .size_full() - .image_cache(image_cache("issues", MAX_IMAGES)) + .image_cache(gpui::retain_all("issues")) .child(self.render_header(cx)) .child( v_flex() diff --git a/crates/workspace/src/views/repo_detail/mod.rs b/crates/workspace/src/views/repo_detail/mod.rs index 4749d34..ec17c75 100644 --- a/crates/workspace/src/views/repo_detail/mod.rs +++ b/crates/workspace/src/views/repo_detail/mod.rs @@ -33,7 +33,6 @@ use signed_state::{ Backend, CheckoutStatus, CheckoutsStore, GitStore, LocalReposStore, ProfileStore, RepoListStore, RepoStore, pr_proposes_checkout, }; -use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{DropdownButton, PixelAvatar, UserAvatar, copy_row}; mod about; @@ -2345,7 +2344,7 @@ impl Render for RepoDetailView { .or_else(|| self.render_push_banner(cx)); v_flex() - .image_cache(image_cache("repo", MAX_IMAGES)) + .image_cache(gpui::retain_all("repo")) .id("repo") .size_full() .child(self.render_header(cx)) 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 5126948..c906848 100644 --- a/crates/workspace/src/views/repo_detail/pull_request_detail.rs +++ b/crates/workspace/src/views/repo_detail/pull_request_detail.rs @@ -7,38 +7,30 @@ use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; use gpui::prelude::*; use gpui::{ AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, - ScrollStrategy, SharedString, Size, Task, WeakEntity, Window, div, px, relative, size, + SharedString, Size, Task, WeakEntity, Window, div, px, relative, size, }; 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}; use gpui_component::spinner::Spinner; 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, WindowExt, h_flex, v_flex, + ActiveTheme, Sizable, StyledExt, VirtualListScrollHandle, WindowExt, h_flex, v_flex, v_virtual_list, }; -use nostr::prelude::{Event, EventId, Kind, Nip34Tag, PublicKey}; +use nostr::prelude::{Event, EventId, Kind, Nip34Tag}; use signed_core::{activity_subject, pull_request_patch}; -use signed_git::{CommitDiff, FileCommit, FileDiff, patch_commits, patch_diffs}; +use signed_git::{FileCommit, patch_commits, patch_diffs}; 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 signed_ui::{UserAvatar, placeholder, status_badge}; use utils::{relative_time, relative_time_secs}; -use super::diff::CommitDiffView; -use super::helpers::{ - DIFF_ROW_HEIGHT, DiffRow, build_tree_items, diff_rows, find_item, render_diff_row, tree_items, -}; - -/// Width of the changed-files column. -const TREE_WIDTH: f32 = 260.; +use super::diff::{CommitDiffView, DiffPane}; +use super::helpers::{comment_form, comments_section, pr_roots, sidebar_section}; /// Height of one commit row in the commits tab's virtual list. const ROW_HEIGHT: f32 = 37.; @@ -65,23 +57,13 @@ pub struct PullRequestDetailView { current_commit: Option, /// Commits of the patch series, in patch order, oldest first. commits: Vec, - /// Parsed file changes of the patch, `None` while loading or on failure. - diff: Option, /// The patch is being parsed on a background task. loading: bool, error: Option, /// Active header tab, 0 = Discussion, 1 = Files, 2 = Commits. active_tab: usize, - /// Changed-files explorer state. - tree_state: Entity, - /// Path of the file whose diff is shown in the detail column. - selected_file: Option, - /// Rows of the selected file's diff, hunk headers and lines. - rows: Vec, - /// Per-row heights of [`Self::rows`]. - item_sizes: Rc>>, - /// Virtual list state of the diff rows. - scroll_handle: VirtualListScrollHandle, + /// Changed-files explorer and per-file diff, like the commit and compare views. + pane: Entity, /// Per-row heights of the commits tab's virtual list, built when the patch series loads. commit_item_sizes: Rc>>, /// Virtual list state of the commits tab. @@ -101,7 +83,7 @@ impl PullRequestDetailView { cx: &mut Context, ) -> Self { let repo_name = store.read(cx).name(); - let tree_state = cx.new(|cx| TreeState::new(cx)); + let pane = cx.new(DiffPane::new); let comment_input = cx.new(|cx| TextareaState::new(window, cx).placeholder("Leave a comment...")); @@ -122,15 +104,10 @@ impl PullRequestDetailView { description: SharedString::default(), current_commit: None, commits: Vec::new(), - diff: None, loading: true, error: None, active_tab: 0, - tree_state, - selected_file: None, - rows: Vec::new(), - item_sizes: Rc::new(Vec::new()), - scroll_handle: VirtualListScrollHandle::new(), + pane, commit_item_sizes: Rc::new(Vec::new()), commit_scroll_handle: VirtualListScrollHandle::new(), contents: HashMap::new(), @@ -270,27 +247,7 @@ impl PullRequestDetailView { 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()); - } + this.pane.update(cx, |pane, cx| pane.set_diff(diff, cx)); } Err(error) => { this.error = Some(error.to_string().into()); @@ -307,26 +264,6 @@ impl PullRequestDetailView { self.tasks.push(task); } - /// Show the diff of the file at `path`, selected in the tree. - fn select_file(&mut self, path: &str, cx: &mut Context) { - self.selected_file = Some(path.into()); - self.set_diff_rows(path); - cx.notify(); - } - - /// Rebuild the virtual list state for the file at `path` and scroll back to the top. - fn set_diff_rows(&mut self, path: &str) { - let Some(diff) = self.diff.as_ref() else { - return; - }; - let Some(file) = diff.files.iter().find(|file| file.path == path) else { - return; - }; - self.rows = diff_rows(file); - self.item_sizes = Rc::new(vec![size(px(0.), px(DIFF_ROW_HEIGHT)); self.rows.len()]); - self.scroll_handle.scroll_to_item(0, ScrollStrategy::Top); - } - /// Open the diff of `commit_id` in the bottom dock of the area. fn open_commit_diff( &mut self, @@ -354,206 +291,9 @@ impl PullRequestDetailView { }); } - /// One row of the changed-files tree, icon and name, indented by depth. - fn render_tree_item( - ix: usize, - entry: &TreeEntry, - selected: bool, - view: &WeakEntity, - ) -> ListItem { - let view = view.clone(); - let id = entry.item().id.clone(); - - tree_row(ix, entry, selected, move |_window, cx| { - if let Some(view) = view.upgrade() { - view.update(cx, |this, cx| this.select_file(&id, cx)); - } - }) - } - - fn render_tree_column(&self, cx: &mut Context) -> AnyElement { - let tree_state = self.tree_state.clone(); - let view = cx.entity().downgrade(); - - v_flex() - .h_full() - .w(px(TREE_WIDTH)) - .flex_none() - .border_r_1() - .border_color(cx.theme().border) - .child( - div() - .flex_1() - .min_h_0() - .when(self.diff.is_some(), |this| { - this.child( - tree(&tree_state, move |ix, entry, selected, _window, _cx| { - Self::render_tree_item(ix, entry, selected, &view) - }) - .p_2(), - ) - }) - .when(self.diff.is_none() && !self.loading, |this| { - this.child(placeholder("Failed to load diff", cx)) - }), - ) - .into_any_element() - } - - fn render_detail_column(&self, cx: &mut Context) -> AnyElement { - if self.loading { - return v_flex() - .size_full() - .items_center() - .justify_center() - .child(Spinner::new().small()) - .into_any_element(); - } - - if let Some(error) = self.error.clone() { - return placeholder(&error, cx); - } - - let Some(diff) = self.diff.as_ref() else { - return placeholder("Failed to load diff", cx); - }; - - let Some(path) = self.selected_file.clone() else { - return if diff.files.is_empty() { - placeholder("No files changed in this pull request", cx) - } else { - placeholder("Select a file", cx) - }; - }; - - let Some(file) = diff.files.iter().find(|file| file.path == path.as_ref()) else { - return placeholder("File not found", cx); - }; - self.render_file_diff(file, cx.entity(), cx) - } - - fn render_file_diff(&self, file: &FileDiff, view: Entity, cx: &App) -> AnyElement { - let status_label = match file.status { - signed_git::DiffStatus::Added => "A", - signed_git::DiffStatus::Modified => "M", - signed_git::DiffStatus::Deleted => "D", - signed_git::DiffStatus::Renamed => "R", - signed_git::DiffStatus::Copied => "C", - }; - - let status_color = match file.status { - signed_git::DiffStatus::Added => cx.theme().success, - signed_git::DiffStatus::Modified => cx.theme().info, - signed_git::DiffStatus::Deleted => cx.theme().danger, - signed_git::DiffStatus::Renamed | signed_git::DiffStatus::Copied => { - cx.theme().muted_foreground - } - }; - - let title = match &file.old_path { - Some(old) => format!("{old} → {}", file.path), - None => file.path.clone(), - }; - - let body: AnyElement = if file.binary { - placeholder("Diff not available", cx) - } else if file.hunks.is_empty() { - placeholder("No content changes", cx) - } else { - let sizes = self.item_sizes.clone(); - let scroll_handle = self.scroll_handle.clone(); - - v_flex() - .size_full() - .relative() - .child( - v_virtual_list( - view, - "pr-diff-rows", - sizes, - move |this, range, _window, cx| { - let Some(diff) = this.diff.as_ref() else { - return Vec::new(); - }; - let Some(path) = this.selected_file.as_deref() else { - return Vec::new(); - }; - let Some(file) = diff.files.iter().find(|file| file.path == path) - else { - return Vec::new(); - }; - range - .map(|ix| render_diff_row(&file.hunks, this.rows[ix], cx)) - .collect() - }, - ) - .track_scroll(&scroll_handle) - .size_full(), - ) - .child( - div() - .absolute() - .top_0() - .left_0() - .right_0() - .bottom_0() - .child(Scrollbar::vertical(&scroll_handle)), - ) - .into_any_element() - }; - - v_flex() - .flex_1() - .min_w_0() - .h_full() - .child( - h_flex() - .px_3() - .h_9() - .gap_2() - .items_center() - .child( - div() - .text_xs() - .font_semibold() - .text_color(status_color) - .child(status_label), - ) - .child( - div() - .flex_1() - .min_w_0() - .text_xs() - .font_semibold() - .text_ellipsis() - .whitespace_nowrap() - .child(title), - ) - .when(!file.binary, |this| { - this.child( - h_flex() - .gap_2() - .text_xs() - .child( - div() - .text_color(cx.theme().success) - .child(format!("+{}", file.insertions)), - ) - .child( - div() - .text_color(cx.theme().danger) - .child(format!("-{}", file.deletions)), - ), - ) - }), - ) - .child(div().id("pr-diff-body").flex_1().min_h_0().child(body)) - .into_any_element() - } - fn render_tabs(&self, cx: &mut Context) -> AnyElement { let active = self.active_tab; - let files_count = self.diff.as_ref().map(|diff| diff.files.len()); + let files_count = self.pane.read(cx).diff().map(|diff| diff.files.len()); let commits_count = if self.commits.is_empty() { None } else { @@ -670,104 +410,48 @@ impl PullRequestDetailView { this.child(div().text_sm().child(self.description.clone())) }), ) - .child(self.render_comments(&root_id, cx)) - .child(self.render_form(&root_id, cx)), + .child(comments_section( + &self.store, + root_id, + &mut self.contents, + cx, + )) + .child(comment_form( + &self.store, + root_id, + pr_roots, + &self.comment_input, + "pr-comment", + cx, + )), ), ) - .child(self.render_sidebar(cx)) - .into_any_element() - } - - /// Right sidebar with participants and labels, like the issue panel. - fn render_sidebar(&self, cx: &mut Context) -> AnyElement { - let profile_store = ProfileStore::global(cx); - let store = self.store.read(cx); - - let Some(root) = store - .pull_requests - .iter() - .find(|pr| pr.id == self.pr_id && pr.kind == Kind::GitPullRequest) - else { - // `render_discussion` already bails out when the PR is missing. - return div().into_any_element(); - }; - - // Participants, the PR author plus everyone who commented. - let mut participants: Vec = vec![root.pubkey]; - participants.extend(store.comments_of(&root.id).map(|comment| comment.pubkey)); - participants.sort_by_key(PublicKey::to_hex); - participants.dedup(); - - // PR labels are NIP-34 `t` hashtag tags on the event. - let labels: Vec = root.tags.hashtags().map(|tag| tag.to_string()).collect(); - - v_flex() - .w(px(240.)) - .h_full() - .flex_none() - .px_4() - .gap_4() - .border_l(px(1.)) - .border_color(cx.theme().sidebar_border) - .child( - v_flex() - .mt_4() - .gap_2() - .child(sidebar_title("Participants", cx)) - .children(participants.iter().map(|pubkey| { - let profile = profile_store.read(cx).get(pubkey); - let name = profile.name(); - let picture = profile.picture(); - - h_flex() - .gap_1() - .items_center() - .child(UserAvatar::new(name.clone()).picture(picture)) - .child(div().text_sm().truncate().text_ellipsis().child(name)) - .into_any_element() - })), - ) - .child( - v_flex() - .gap_2() - .child(sidebar_title("Labels", cx)) - .map(|this| { - if labels.is_empty() { - this.child( - div() - .text_sm() - .text_color(cx.theme().muted_foreground) - .child("None yet."), - ) - } else { - this.child(h_flex().gap_1().children({ - let mut items = vec![]; - - for label in labels.iter() { - items.push( - Tag::secondary() - .outline() - .xsmall() - .child(SharedString::from(label)), - ); - } - - items - })) - } - }), - ) + .child(sidebar_section(&self.store, root_id, pr_roots, true, cx)) .into_any_element() } fn render_files_tab(&self, cx: &mut Context) -> AnyElement { + if self.loading { + return v_flex() + .flex_1() + .w_full() + .min_h_0() + .items_center() + .justify_center() + .child(Spinner::new().small()) + .into_any_element(); + } + + if let Some(error) = self.error.clone() { + return placeholder(&error, cx); + } + h_flex() .flex_1() .w_full() .min_h_0() .overflow_hidden() - .child(self.render_tree_column(cx)) - .child(self.render_detail_column(cx)) + .child(self.pane.clone()) .into_any_element() } @@ -876,114 +560,6 @@ impl PullRequestDetailView { .into_any_element() } - /// One comment card, same design as the issue panel. - fn render_comments(&mut self, id: &EventId, cx: &mut Context) -> AnyElement { - let store = self.store.read(cx); - let comments: Vec<&Event> = store.comments_of(id).collect(); - let title = SharedString::from(format!("Discussions {}", comments.len())); - - v_flex() - .gap_4() - .child(div().text_xs().font_semibold().child(title)) - .children(comments.iter().map(|comment| { - let profile = ProfileStore::global(cx).read(cx).get(&comment.pubkey); - let author = profile.name(); - let picture = profile.picture(); - let age = relative_time(comment.created_at); - // Comment bodies become shared strings once per comment, not per render. - let content = self - .contents - .entry(comment.id) - .or_insert_with(|| SharedString::from(comment.content.clone())) - .clone(); - - v_flex() - .gap_1() - .p_3() - .border_1() - .border_color(cx.theme().border) - .rounded(cx.theme().radius) - .child( - h_flex() - .gap_2() - .text_sm() - .child( - h_flex() - .gap_1() - .child(UserAvatar::new(author.clone()).picture(picture)) - .child(author), - ) - .child( - div() - .text_color(cx.theme().muted_foreground) - .child("commented"), - ) - .child( - div() - .text_color(cx.theme().muted_foreground) - .child(SharedString::from(age)), - ), - ) - .child(div().text_sm().child(content)) - })) - .into_any_element() - } - - fn render_form(&mut self, id: &EventId, cx: &mut Context) -> AnyElement { - let comment_input = self.comment_input.clone(); - let store = self.store.clone(); - let id = id.to_owned(); - - v_flex() - .gap_2() - .child( - Textarea::new(&self.comment_input) - .h_24() - .text_color(cx.theme().muted_foreground) - .bg(cx.theme().muted), - ) - .child( - h_flex() - .justify_between() - .child( - h_flex() - .gap_1() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(Icon::new(CustomIconName::Markdown).small()) - .child("Markdown is supported"), - ) - .child( - Button::new("pr-comment") - .primary() - .label("Comment") - .tooltip("Post comment") - .on_click(move |_event, window, cx| { - let content = comment_input.read(cx).value().trim().to_string(); - if content.is_empty() { - return; - } - let Some(root) = store - .read(cx) - .pull_requests - .iter() - .find(|pr| pr.id == id) - .cloned() - else { - return; - }; - store.update(cx, |store, cx| { - store.comment(&root, content, cx); - }); - comment_input.update(cx, |input, cx| { - input.set_value("", window, cx); - }); - }), - ), - ) - .into_any_element() - } - /// Always-visible header with a status badge and title, like the issue panel. fn render_header(&self, cx: &mut Context) -> AnyElement { let current_commit = self.current_commit.clone(); @@ -1147,20 +723,9 @@ fn open_update_pull_request_dialog( }); } -/// One sidebar section title. -fn sidebar_title(text: &str, cx: &App) -> AnyElement { - div() - .text_xs() - .font_semibold() - .text_color(cx.theme().muted_foreground) - .child(text.to_string()) - .into_any_element() -} - -/// The `c` tag of a PR event, the tip of the proposed branch, as hex. -fn current_commit_of(event: &Event) -> Option { - event - .tags +/// The `c` tag of a PR event, the commit the proposal points at. +fn current_commit_of(root: &Event) -> Option { + root.tags .iter() .find_map(|tag| match Nip34Tag::parse(tag.as_slice()) { Ok(Nip34Tag::CurrentCommit(commit)) => Some(commit.to_string()), @@ -1263,7 +828,7 @@ impl Focusable for PullRequestDetailView { impl Render for PullRequestDetailView { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { v_flex() - .image_cache(image_cache("pull-request-detail", MAX_IMAGES)) + .image_cache(gpui::retain_all("pull-request-detail")) .id("pull-request-detail") .size_full() .min_h_0() diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index bea44bc..3e252fb 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -16,7 +16,6 @@ use gpui_component::{ 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::{DropdownButton, SegmentButton, UserAvatar, placeholder, status_badge}; use utils::relative_time; @@ -387,7 +386,7 @@ impl Render for PullRequestsView { v_flex() .size_full() - .image_cache(image_cache("pull-requests", MAX_IMAGES)) + .image_cache(gpui::retain_all("pull-requests")) .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); diff --git a/crates/workspace/src/views/repo_list.rs b/crates/workspace/src/views/repo_list.rs index 3d69fa2..6876c46 100644 --- a/crates/workspace/src/views/repo_list.rs +++ b/crates/workspace/src/views/repo_list.rs @@ -15,7 +15,6 @@ use gpui_component::{ }; use signed_core::Announcement; use signed_state::{ProfileStore, RepoListStore, Timestamp}; -use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{SegmentButton, UserAvatar}; use utils::relative_time; @@ -397,7 +396,7 @@ impl Render for RepoListView { v_flex() .relative() - .image_cache(image_cache("repos", MAX_IMAGES)) + .image_cache(gpui::retain_all("repos")) .size_full() .child(self.render_header(count, cx)) .when(!has_repos, |this| { diff --git a/crates/workspace/src/views/sidebar/create_repo_dialog.rs b/crates/workspace/src/views/sidebar/create_repo_dialog.rs index c34807b..a4723f0 100644 --- a/crates/workspace/src/views/sidebar/create_repo_dialog.rs +++ b/crates/workspace/src/views/sidebar/create_repo_dialog.rs @@ -2,26 +2,23 @@ use std::path::PathBuf; use dock::DockArea; use gpui::prelude::*; -use gpui::{App, Entity, PathPromptOptions, SharedString, WeakEntity, Window, div, px}; +use gpui::{App, Entity, PathPromptOptions, WeakEntity, Window, div, px}; use gpui_base::input::TextareaState; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputState, Textarea}; -use gpui_component::{ActiveTheme, Disableable, IconName, WindowExt, h_flex}; +use gpui_component::{Disableable, IconName, WindowExt, h_flex}; use settings::SettingsStore; use signed_core::Announcement; use signed_state::{Backend, CheckoutsStore}; use super::super::open_repo_panel; use super::grasp_servers::{GraspServersState, grasp_servers_field, load_user_grasp_servers}; +use crate::views::dialog_state::{DialogProgress, error_row}; /// Shared state for the Create Repository dialog, so async results can be rendered. -#[derive(Default)] -pub struct CreateRepoState { - pub busy: bool, - pub error: Option, -} +pub type CreateRepoState = DialogProgress; /// Open the Create Repository dialog. pub fn open(dock_area: WeakEntity, window: &mut Window, cx: &mut App) { @@ -117,9 +114,7 @@ pub fn open(dock_area: WeakEntity, window: &mut Window, cx: &mut App) ) .child(grasp_servers_field(&grasp_state, &relay_input, cx)), ) - .children(error.map(|message| { - div().text_sm().text_color(cx.theme().danger).child(message) - })) + .children(error_row(&error, cx)) .child( DialogFooter::new().justify_end().child( Button::new("create") @@ -211,22 +206,15 @@ fn create_repository( let servers = grasp_state.read(cx).grasp_servers.clone(); if name.is_empty() { - state.update(cx, |state, _| { - state.error = Some("Repository name is required".into()); - }); + state.update(cx, |state, _| state.fail("Repository name is required")); return; } if servers.is_empty() { - state.update(cx, |state, _| { - state.error = Some("Add at least one grasp server".into()); - }); + state.update(cx, |state, _| state.fail("Add at least one grasp server")); return; } - state.update(cx, |state, _| { - state.busy = true; - state.error = None; - }); + state.update(cx, |state, _| state.begin()); let backend = Backend::global(cx); let task = backend.update(cx, |backend, cx| { @@ -254,10 +242,7 @@ fn create_repository( } Err(e) => { cx.update_window(handle, |_, _window, cx| { - state.update(cx, |state, _| { - state.busy = false; - state.error = Some(e.to_string().into()); - }); + state.update(cx, |state, _| state.fail(e.to_string())); }) .ok(); } diff --git a/crates/workspace/src/views/sidebar/grasp_servers.rs b/crates/workspace/src/views/sidebar/grasp_servers.rs index c469376..cd5a2a2 100644 --- a/crates/workspace/src/views/sidebar/grasp_servers.rs +++ b/crates/workspace/src/views/sidebar/grasp_servers.rs @@ -6,7 +6,6 @@ use gpui_component::input::{Input, InputState}; use gpui_component::{ActiveTheme, IconName, Sizable, h_flex, v_flex}; use nostr::prelude::*; use settings::{DEFAULT_GRASP_SERVERS, GraspServersSettings}; -use signed_core::filters; use signed_state::Backend; /// State of the grasp-server section of a publish dialog, so async results can be rendered. @@ -238,30 +237,7 @@ pub fn load_user_grasp_servers( let handle = window.window_handle(); cx.spawn(async move |cx| { - let result: anyhow::Result> = async { - let mut events: Vec = client - .database() - .query(filters::grasp_list(user)) - .await? - .into_iter() - .collect(); - events.sort_by_key(|event| event.created_at); - - Ok(events - .into_iter() - .last() - .map(|event| { - event - .tags - .iter() - .filter(|tag| tag.kind() == "g") - .filter_map(|tag| tag.content()) - .filter_map(|url| RelayUrl::parse(url).ok()) - .collect() - }) - .unwrap_or_default()) - } - .await; + let result = signed_state::user_grasp_list_servers(client, user).await; let _ = cx.update_window(handle, |_, _window, cx| { state.update(cx, |state, _| { diff --git a/crates/workspace/src/views/sidebar/mod.rs b/crates/workspace/src/views/sidebar/mod.rs index 6d4004b..6a595e0 100644 --- a/crates/workspace/src/views/sidebar/mod.rs +++ b/crates/workspace/src/views/sidebar/mod.rs @@ -18,7 +18,6 @@ use signed_core::{Announcement, identifier_from_name}; use signed_state::{ Backend, BackendEvent, CheckoutsStore, LocalReposStore, Profile, ProfileStore, RepoListStore, }; -use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{CountBadge, NavItem, PixelAvatar, UserAvatar, title_bar_drag_handlers}; use super::{RepoDetailView, RepoListView, open_repo_panel}; @@ -559,7 +558,7 @@ impl Render for SidebarPanel { v_flex() .size_full() .justify_between() - .image_cache(image_cache("sidebar", MAX_IMAGES)) + .image_cache(gpui::retain_all("sidebar")) .bg(cx.theme().sidebar) .text_color(cx.theme().sidebar_foreground) .child( diff --git a/crates/workspace/src/views/sidebar/onboarding_dialog.rs b/crates/workspace/src/views/sidebar/onboarding_dialog.rs index 9e70619..8f32e51 100644 --- a/crates/workspace/src/views/sidebar/onboarding_dialog.rs +++ b/crates/workspace/src/views/sidebar/onboarding_dialog.rs @@ -1,18 +1,16 @@ use gpui::prelude::*; -use gpui::{App, Entity, SharedString, Window, div, px}; +use gpui::{App, Entity, Window, px}; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputState}; -use gpui_component::{ActiveTheme, Disableable, WindowExt}; +use gpui_component::{Disableable, WindowExt}; use signed_state::Backend; +use crate::views::dialog_state::{DialogProgress, error_row}; + /// Shared state for the Onboarding dialog, so async results can be rendered. -#[derive(Default)] -pub struct OnboardingState { - pub busy: bool, - pub error: Option, -} +pub type OnboardingState = DialogProgress; /// Open the Onboarding dialog for creating a new identity. pub fn open( @@ -62,9 +60,7 @@ pub fn open( ) .child(field().required(true).child(Input::new(&repass_input))), ) - .children(error.map(|message| { - div().text_sm().text_color(cx.theme().danger).child(message) - })) + .children(error_row(&error, cx)) .child( DialogFooter::new().justify_end().child( Button::new("continue") @@ -87,17 +83,12 @@ pub fn open( if pass != repass { state.update(cx, |state, _| { - state.busy = false; - state.error = - Some("Passphrases do not match".into()); + state.fail("Passphrases do not match"); }); return; } - state.update(cx, |state, _| { - state.busy = true; - state.error = None; - }); + state.update(cx, |state, _| state.begin()); let task = backend.update(cx, |backend, cx| { backend.create_identity(&name, &pass, cx) @@ -115,8 +106,7 @@ pub fn open( Err(e) => { cx.update_window(handle, |_, _window, cx| { state.update(cx, |state, _| { - state.busy = false; - state.error = Some(e.to_string().into()); + state.fail(e.to_string()); }); }) .ok(); diff --git a/crates/workspace/src/views/sidebar/passphrase_dialog.rs b/crates/workspace/src/views/sidebar/passphrase_dialog.rs index 3e540ca..a076352 100644 --- a/crates/workspace/src/views/sidebar/passphrase_dialog.rs +++ b/crates/workspace/src/views/sidebar/passphrase_dialog.rs @@ -1,18 +1,20 @@ use assets::CustomIconName; use gpui::prelude::*; -use gpui::{AnyWindowHandle, App, Entity, SharedString, Subscription, Window, div}; +use gpui::{AnyWindowHandle, App, Entity, Subscription, Window}; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputEvent, InputState}; -use gpui_component::{ActiveTheme, Disableable, WindowExt}; +use gpui_component::{Disableable, WindowExt}; use signed_state::Backend; +use crate::views::dialog_state::{DialogProgress, error_row}; + /// Shared state for the passphrase dialog, so async results can be rendered. #[derive(Default)] pub struct PassphraseState { - pub busy: bool, - pub error: Option, + /// Progress of the unlock flow. + pub progress: DialogProgress, /// Keeps the Enter-to-submit subscription alive while the dialog is open. _enter_subscription: Option, } @@ -50,8 +52,8 @@ pub fn open(window: &mut Window, cx: &mut App) { .overlay_closable(false) .keyboard(false) .content(move |content, _window, cx| { - let busy = state.read(cx).busy; - let error = state.read(cx).error.clone(); + let busy = state.read(cx).progress.busy; + let error = state.read(cx).progress.error.clone(); content .child( @@ -70,9 +72,7 @@ pub fn open(window: &mut Window, cx: &mut App) { .child(Input::new(&pass_input)), ), ) - .children(error.map(|message| { - div().text_sm().text_color(cx.theme().danger).child(message) - })) + .children(error_row(&error, cx)) .child( DialogFooter::new().justify_end().child( Button::new("unlock") @@ -107,15 +107,12 @@ fn unlock( if pass.is_empty() { state.update(cx, |state, _| { - state.error = Some("Passphrase must not be empty".into()); + state.progress.fail("Passphrase must not be empty"); }); return; } - state.update(cx, |state, _| { - state.busy = true; - state.error = None; - }); + state.update(cx, |state, _| state.progress.begin()); let task = backend.update(cx, |backend, cx| backend.restore_with_passphrase(&pass, cx)); let handle = *handle; @@ -130,10 +127,7 @@ fn unlock( } Err(e) => { cx.update_window(handle, |_this, _window, cx| { - state.update(cx, |state, _| { - state.busy = false; - state.error = Some(e.to_string().into()); - }); + state.update(cx, |state, _| state.progress.fail(e.to_string())); }) .ok(); } diff --git a/desktop/Cargo.toml b/desktop/Cargo.toml index d8e2d7e..ced69db 100644 --- a/desktop/Cargo.toml +++ b/desktop/Cargo.toml @@ -17,12 +17,8 @@ workspace = { path = "../crates/workspace" } gpui.workspace = true gpui_platform.workspace = true -gpui_linux.workspace = true -gpui_windows.workspace = true -gpui_macos.workspace = true dock = { workspace = true } gpui-component.workspace = true reqwest_client.workspace = true -log.workspace = true tracing.workspace = true tracing-subscriber.workspace = true diff --git a/docs/PLAN.md b/docs/PLAN.md index 546347e..a2483ad 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -1,657 +1,229 @@ -# PLAN — PR contribution flows: fork compare + GRASP-06 hosting + checkout suggestions +# PLAN — Codebase audit: over-engineering, dead code & simplification -> **Status (2026-09-03): implemented.** Steps 1-9 and 11 are done on -> `feat/fork`; step 10 (sidebar "Ready to contribute" group) remains -> deferred as planned (v2, optional). See `docs/PR_FLOW.md` for the -> resulting flow; the per-step sections below record what shipped and -> where the plan was refined during implementation. - -Combined implementation plan for three coordinated improvements to Signed's pull -request experience: - -- **A. Fork-aware compare** — the New PR panel's compare side can come from an - announced fork repository's branch (fetched into the base repo's GitCache - mirror), instead of only from a user-picked local checkout. -- **B. GRASP-06 hosting** — PR tips are pushed to the *author's own* grasp - servers under `/prs//.git` and advertised in the PR's - `clone` tag, so contributing to someone else's project never depends on their - servers accepting anything from you. -- **C. Checkout associations & suggestions** — remember/derive which local - folders are checkouts of which announced repos, auto-prefill the New PR panel - (no folder picker for the common case), and suggest creating a PR when a - branch is ahead with no open PR (GitHub-like nudge, NIP-34-native dedupe). - -Guiding principles (agreed): follow nostr + grasp + ngit, not GitHub; -NIP-34/GRASP-06 event surface stays untouched (no new tags/kinds, forks never -appear in events); every flow keeps the patch series as the source of truth; -no over-engineering — reuse existing stores, patterns and git helpers. +> **Status (2026-09-04): proposed.** Full-repo audit (~27k lines, 13 crates). +> Every finding was cross-checked against consumers and verified against the +> locked library sources (gpui-component `18922d6`, rust-nostr `472c883`, +> gix 0.87.1). Pick the steps you want before processing; ordered +> safest-first. Estimated total: **~3,000+ lines removable (~12%)**. --- -## 1. Protocol grounding (what we may and may not do) +## Step 1 — Zero-risk deletions (~700 L, pure removals) -### 1.1 NIP-34 facts used by this plan +1. **Unused Cargo deps** (verified by grep): + - `desktop`: `gpui_linux`, `gpui_windows`, `gpui_macos` (only + `gpui_platform` is used — it's the meta-crate that picks the backend), + `log` + - `workspace`: `chrono` + - `signed_nostr`: `signed_core`, `nostr` + - `assets`: `log` +2. **`signed_core::clone_url` module** (whole file) — only self-tests + reference it; `repo_detail/mod.rs:2418` *produces* the format but never + parses it. (`clone_url.rs` + `lib.rs:13`) <- acceptable +3. **`signed_core::comments` module** (`CommentThread`, `comment_threads`) — + no consumers; SDK has `nip22::extract_parent` anyway. + (`comments.rs` + `lib.rs:14`) +4. **`paths` dead accessors**: `cache_dir()`, `logs_dir()`, `keymap_file()`, + `set_custom_data_dir()` + `CUSTOM_DATA_DIR`/`OnceLock` machinery + (`paths/src/lib.rs:13,50-61,112-169`). Used: `desktop_dir`, + `documents_dir`, `settings_file`, `nostr_dir`, `repos_dir`. +5. **`signed_git::patch_applies`** (+test) — own TODO says superseded by the + live compare view (`lib.rs:252-277, 2294-2324`). +6. **`signed_git::worktree_tags`** — both UI sites call `repo_tags` directly + (`lib.rs:1811-1813`). +7. **Unused re-exports**: `workspace::image_cache` (`workspace/src/lib.rs:6`); + `signed_state::lib`'s `pub use utils::shorten_pubkey` + (`signed_state/src/lib.rs:21`). +8. **Sidebar placeholder nav items** "Inbox", "Search", "Guide" — all three + just open the Explore panel (`sidebar/mod.rs:578-618`). <- acceptable +9. **`import_dialog.rs`** — opens an empty 400px dialog; the "Import identity" + sidebar button is a dead end (`sidebar/mod.rs:405-408,493-504`). <- acceptable +10. **All `wasm32` cfg paths + `nostr-memory` dep** — GPUI has no wasm + backend; the target cannot link (`signed_state/src/lib.rs:52-63`, + `checkouts.rs:108-165`, `signed_nostr/src/backend.rs:9-29`, + `signed_nostr/Cargo.toml`). <- acceptable, note: GPUI have support for wasm via gpui_web, updated your memory or check before make changes +11. **`dock::t()` unreachable `"Dock.Unnamed"` arm** (`dock/src/lib.rs:21-30`). +12. Two handler-less `Button`s ("user" with fake `dropdown_caret`, + "maintainers") — render as plain `h_flex` or wire real menus + (`sidebar/mod.rs:426-434`, `repo_detail/mod.rs:2289-2307`). <- acceptable +13. Stale doc references to nonexistent `helpers::track` + (`diff.rs:328`, `mod.rs:201-203`). -- A PR (kind-1618) is addressed to the **base** repo coordinate (`a` tag) and - carries `c` (tip), `merge-base` (common ancestor with the target branch), - `branch-name`, `clone` (≥1 URL where the tip commit can be downloaded), - `e` → root patch event, `r` (EUC), `p` (base owner). Patches are - NIP-10-chained kind-1617 events, ≤60 KB each. Statuses 1630–1633 resolve the - PR. -- Repository announcements (30617): `u` tag marks a subordinate fork - (`30617::` coordinate or git URL); the `r`/`euc` tag identifies - the earliest unique commit, shared by every repo of the same project family - (forks, mirrors). Both are **read-only inputs** for discovery. -- Kind-10317 is the user grasp list (`g` tags, in preference order) — read-only - input for hosting. -- Anybody may open a PR on any announced repo; only the author may update it - (1619); only the author or a maintainer may set status; merge is the - maintainer's action. -- "Patches and PRs to a repository SHOULD be sent to the relays specified in - that repository's announcement" — i.e. the **base** repo's relays, always. +## Step 2 — Dead feature removal (~500 L) -### 1.2 GRASP-06 facts (as ngit implements it — verified against ngit-cli) +1. **Login/logout API** — `login`, `login_with_new_identity`, + `login_with_nsec`, `login_with_bunker`, `logout`, `with_master_key` + (`backend.rs:879-988,1537-1542`). UI only uses `create_identity` + keyring + restore. ⚠️ `login_with_new_identity` stores unencrypted nsec — security + footgun. **Keep** `extract_master_key` + the `bunker://` branch of + `restore_session` (services older keyring entries). <- acceptable +2. **`RepoStore::merge_pull_request` + `publish_applied_status`** — no merge + action exists in the UI (`repo.rs:1122-1256`). <- acceptable +3. **`RepoStore::publish_state`** — superseded by `Backend::push_repo_from` + (`repo.rs:1070-1120`). +4. **Annotation machinery** — `cover_note_of`/`labels_of`/`subject_of` (no + callers) + `cover_notes`/`labels` fields + per-root DB query loop + + `annotations_for` relay fetch. **Every kind-1624/1985 event currently + triggers a full refresh of every open RepoStore for data nothing + displays** (`repo.rs:52-56,339-362,489,531-563`). +5. **`BackendEvent::Error` variant + `emit_error` + ~16 emission sites** — + matched nowhere (consumers only match `SignerChanged`, `SignerRequired`, + `PassphraseRequired`, `NostrUpdate`, `Synced`, `SyncProgress`, + `Published`) (`backend.rs:64,67-74,1047-1050`). <- acceptable +6. **`BackendEvent::Connected` + `connected` field + `is_connected()`** — + emitted twice, never consumed (`backend.rs:49,83,154,1110-1111,1052-1055`). +7. **`sync_progress` field + getter + `SyncProgress::channel` watch-loop + math** — payload discarded; used only as a dumb refresh tick + (`backend.rs:84,1057-1060,1227-1258`). <- acceptable +8. **`Backend::subscribe`, `add_discovery_relays`, `publish_announcement`** — + zero callers (`backend.rs:1123-1158,1388-1395`). +9. **Write-only fields**: `RepoStore::refs` (`repo.rs:37-38,449`), + `RepoListStore::set_author` (`repo_list.rs:135-140`), + `Update::event_id` (`signed_nostr/src/update.rs:10`). +10. **NIP-44 half of `UniversalSigner`** (`AsyncNip44` bounds, + encrypt/decrypt plumbing, ~60 of 200 L) — app never touches DMs + (`signed_nostr/src/signer.rs`). <- acceptable -- GRASP-06 servers expose a contributor namespace: - `http(s):///prs//.git` (input - `ws://`/`wss://` base URLs normalize to `http(s)://`; npub in the URL, hex - on the server's disk — a server detail). Anyone can push there; no - announcement, no maintainer rights, no fork repo required. -- The author's server is tried **first**; the base repo's announcement grasps - still receive the same `refs/nostr/` push as redundancy. -- The PR event shape is unchanged; only *which URLs the `clone` tag lists* - differs. +## Step 3 — Library swaps (~600 L) -### 1.3 Consequences (locked decisions) +1. **`signed_ui::DropdownButton` → `gpui_component::button::DropdownButton`** + — present in the locked revision, same `new/button/dropdown_menu` surface + (the local doc even says it matches). Migrate 3 call sites + (`pull_requests.rs`, `repo_detail/mod.rs`), delete the ~200 L file. <- acceptable +2. **`wire_number_input` → `SettingField::number_input`** + (`settings_dialog.rs:691-761`, ~90 L) — you already import + `NumberFieldOptions` from gpui-component's setting module. <- acceptable +3. **Custom C-unquoting → `gix::quote::ansi_c::undo`** + (`signed_git/src/lib.rs:1610-1683` + call sites, ~100 L) — already in the + dep tree, octal/escape semantics identical. Keep tests as regression tests. +4. **`ensure_origin` redundant refspec write** (`signed_git/src/lib.rs:523-530`) + — `git remote add` creates `remote.origin.fetch` by default. +5. **Two hand-rolled tab bars → gpui-component `TabBar`** + (`repo_detail/mod.rs:2120-2265`, `new_pull_request.rs:1236-1305`, ~110 L) — + `pull_request_detail.rs` already uses `TabBar` correctly (proves the fit). <- acceptable +6. **CLI `merge_base` → `gix::Repository::merge_base`** + (`signed_git/src/lib.rs:201-222`) — the codebase already uses the gix one + in `pull_request_detail.rs:244`. +7. **`image_cache` → `gpui::retain_all` or single-map LRU** — vendored copy + with a `max_items` param every call site passes `MAX_IMAGES=128` to + (`signed_ui/src/image_cache.rs`, 139 L). Delete the param + dual-structure + LRU or use upstream unbounded cache. -- Publishing keeps today's event set and tag semantics. The fork changes only - where the patch series is generated from; GRASP-06 changes only where the tip - is pushed and advertised; suggestions change nothing on the wire. -- We never publish a reference to the fork or to `/prs/` hosting beyond legal - `clone` URLs. -- The PR `clone` tag is fixed before signing (the `refs/nostr/` ref - name embeds the event id), so it carries the full candidate URL set - (author `/prs/` URLs + base announcement clone URLs). Dead URLs are inert — - patch events remain the truth — and ngit readers fail over across URLs. - (ngit instead rebuilds the event per server to keep a single clone URL; we - deliberately do not copy that.) +## Step 4 — Dedup passes (~1,200 L) + +1. **`PullRequestDetailView` → hold `Entity`** instead of its ~200 L + inline copy of `diff.rs` (`pull_request_detail.rs:310-552`). The other two + consumers already do this. +2. **Issue/PR detail shared sections** (~300 L): comments list, comment form, + participants/labels sidebar, `sidebar_title` — extract into `helpers.rs` + (`issue_detail.rs` vs `pull_request_detail.rs`). +3. **Dialog scaffolding helper** (~150-200 L): 4 copies of `{busy, error}` + state structs, verbatim error rows, identical + `cx.spawn → close_dialog / show error` plumbing + (`onboarding_dialog.rs`, `passphrase_dialog.rs`, `create_repo_dialog.rs`, + `init_dialog.rs`, +2 more sites). +4. **Triplicated debounce state machine → one helper** — `refreshing` / + `refresh_dirty` / `debouncing` trio copied into `repo.rs`, `checkouts.rs`, + `repo_list.rs` (~105 L). +5. **signed_git helpers**: `push_main`/`push_all` twin bodies → one + `push_refspecs`; git-CLI spawn boilerplate → one `git_output`; + grasp→https rewrite → one fn; "try each mirror URL" loop → one helper. +6. **grasp-list parsing ×3 → one helper** — `backend.rs:996-1012`, + `backend.rs:1586-1618`, `grasp_servers.rs:240-265`. +7. Smaller copies: ref-selector trigger ×2, count badge ×3 (use + `signed_ui::CountBadge`), grasp-server editor duplicated in + `settings_dialog.rs:408-614` vs `grasp_servers.rs` (~90 L), folder-picker + prompts ×4, "add panel to dock Center" ×10, fork-label upstream lookup ×2, + avatar+name row ×9 (→ one `user_row` helper in signed_ui). +8. **`Backend::send` vs `publish_event`** — copy-pasted bodies differing only + in `finalize_async` (`backend.rs:1293-1386`). + +## Step 5 — Structural (do deliberately) + +1. **`UniversalSigner` → enum** — verified: nostr-sdk 0.45 `Client` has no + signer slot (external `SignerAuthenticator` by value at build time), so a + swap-in-place wrapper IS needed — but only `Keys`/`NostrConnect` ever + occur. Replace 200-L vtable (`InnerSigner` trait + `InnerSignerImpl` + + custom error + boxed futures) with + `enum Signer { Keys(Keys), Connect(NostrConnect) }` in + `Arc>` (~40 L). <- acceptable +2. **`GitStore` single install** — `signed_state::init` installs empty root + (`lib.rs:44`), immediately replaced by `desktop/main.rs:81`. Pass the root + into `init`. +3. **Error handling in signed_git**: `map_err(|e| anyhow!("{e}"))` → plain + `?` (preserves source chain; `lib.rs:1787,1798,1845,1852`); blanket + `.ok()`/`.unwrap_or_default()` → matched cases + (`lib.rs:919,995,1761,364,682,1779`). +4. **`ProfileStore` second flume channel → `WeakEntity` + `update()`** + (~25 L; the results channel only exists to get back to main thread). +5. **Unbounded `tasks` Vec growth** — only `repo.rs` prunes; `backend.rs` + (~18 push sites), `checkouts.rs` (grows every 15-60 s poll cycle), + `profile.rs` (2 per metadata event) accumulate finished handles forever. + One-line `retain(|t| !t.is_ready())` per store. +6. **Redundant `observe → cx.notify()` subscriptions** + (`sidebar/mod.rs:81-88`, `repo_detail/mod.rs:354-355,1914-1921`) — this + gpui revision auto-tracks entities read during render, so re-render-only + observers are belt-and-suspenders. ⚠️ Verify before deleting; see Step 7.2. +7. **`open_upstream` sleep-poll → `cx.observe`** (`repo_detail/mod.rs:1113-1152`) + — 60×250 ms race-prone loop re-implementing the store's notify mechanism. +8. **Checkouts map `Arc` removal** — accessors deep-clone anyway + (`checkouts.rs:69-81`). +9. Minor: `CheckoutRecord` manual `Default` → derive; `dock` re-export trim + (27 items, ~10 used); `PixelAvatar.size` field with no setter → const; + `SCAN_SKIPPED_DIRS` 1-element array → direct compare; `RepoAction` menu + indirection → `on_click`; `Announcement::from_event` round-trip after + building from typed data. + +## Step 6 — Over-optimization (optional, unmeasured machinery) + +1. **`HeaderCache`** (`repo_detail/mod.rs:113-123,1426-1461`) — keyed + invalidation + 3 `Rc` layers memoizing 2 bech32 encodes + 2 `format!`s + (~45 L). Compute inline. +2. **Comment-body memoization** `contents: HashMap` + (`issue_detail.rs`, `pull_request_detail.rs`) — caches one small alloc per + render; convert inline. +3. **`OBJECT_CACHE_BYTES = 64 MiB`** indiscriminately applied — scope to + history-walk entry points or drop (`signed_git/src/lib.rs:770,859-863`). +4. **`file_commit(…, include_description: bool)`** — boolean flag saving one + alloc; split into named constructors or always include. +5. **`signed_core` could drop its `gpui` dep** if `Announcement`'s + `SharedString` fields became `String` (`model.rs:9-35`). +6. **`build_state`** raw `Tag::parse(...).expect()` → typed `Nip34Tag::to_tag` + (`signed_core/src/state.rs:7-18`). +7. **`PixelAvatar` FNV/RNG stack** (~60 L) — deterministic-hashing requirement + is legit; reconsider if `DefaultHasher` stability is acceptable. + +## Step 7 — Clean verdicts + one correctness note (no action) + +1. **Confirmed clean, keep as-is**: `signed_core` correctly uses + `Nip34Tag::parse` / SDK builders (only the `u` tag is hand-parsed — SDK + doesn't model it); `dock` crate is a genuine thin skin over + `gpui_base::dock` renderer traits (keep; only trim re-exports); LMDB is + justified (instant startup lists); `signed_git` test suite + `tempfile` + healthy. Gossip/NIP-65 machinery serves only one login query — *consider* + dropping if NIP-65 routing isn't on the roadmap. +2. **Correctness note**: `IssuesView` / `PullRequestsView` / + `IssueDetailView` / `PullRequestDetailView` never observe their + `RepoStore` — refresh only works because this gpui revision auto-tracks + render reads. Accidental; pinning a different gpui breaks them silently. + Relevant to Step 5.6. --- -## 2. Workstream A — Fork-aware compare - -### 2.1 Model - -The panel keeps today's behavior as the default source and adds a second: - -- **Checkout** (existing): both selectors list a user-picked local checkout's - branches; git ops + tip push run in the checkout. -- **Fork** (new): the flow runs against the **base repo's GitCache mirror** - `P_base = GitStore::global(cx).cache().repo_path(&base_addr)` (ensured via - `GitCache::ensure_clone(&base_addr, &base_clone_urls)` + `fetch_all`): - - "Merge Into" lists `P_base` branches (`refs/remotes/origin/*`); - - "Pull From" lists the chosen fork's branches, imported into `P_base`; - - `merge-base`, range commits/diff, `format-patch`, and the tip push all run - against `P_base` — both histories share one object store, and commit-diff - rows work because fork commits live there. - -### 2.2 Git mechanics (import namespace) - -Fork heads are fetched into `P_base` under a private namespace: - -``` -git -C P_base fetch '+refs/heads/*:refs/fork///*' -``` - -- `refs/fork/…` keeps imported refs away from `refs/remotes/*` and - `refs/heads/*`, so the repo browser, `repo_branches` and DWIM checkout never - see them. -- Fetch tries each announced `clone` URL until one works (`grasp://` → - `https://` rewrite, `GIT_TERMINAL_PROMPT=0`), like `clone_repo` / - `push_commit_ref`. -- Switching fork or refreshing: prune the old `refs/fork///*` - prefix first (`git update-ref --stdin` fed by `for-each-ref`), then - re-import. All-heads import in one fetch; subsequent branch switches within - the same fork are offline. -- Range work uses full refs: `merge_base(P_base, - "refs/remotes/origin/", "refs/fork/…/")`, then the existing - `worktree_commit_range_commits`/`worktree_commit_range_diff` / - `format_patch_between`. None of these touch the checkout state. -- Base `main` and fork `main` are different refs: the "choose different - branches" guard compares full refs, display names stay short. - -### 2.3 Fork discovery - -Candidates = `RepoListStore::global(cx).read(cx).announcements` (already -deletion-filtered, latest-wins) where -`Announcement::is_fork_of(base_addr, base_euc)`: - -- `upstream.addr == Some(base_addr)` (the `u` tag — also covers permanent - forks whose EUC changed), **or** -- `euc == base announcement's euc` (shared earliest-unique-commit family), - excluding the base repo itself. - -Ordering (identity-coherent, ngit-style): **your own forks first** (30617 -owner == signed-in user), then other authors' related repos (same mechanics, -marked, niche). Announcements without `clone` URLs are excluded (unfetchable). -Restricting to your own forks only later is a one-line ownership filter. - -### 2.4 Panel behavior - -- Defaults mirror `apply_checkout`: base = announced `store.head` if present in - mirror branches, else `main`, else first; compare = fork's `main`, else - first fork branch. -- `submit`: `format_patch_between(P_base, merge_base, compare_ref)`; published - `branch-name` = compare short name; `push_from = Some(P_base)` (fork objects - are there after import). Publishing itself is workstream B. -- `open_commit_diff` uses `P_base` in fork mode. -- Errors: no common ancestor → existing message; unreachable base mirror or - fork → inline error; empty range → existing "no commits to propose". - ---- - -## 3. Workstream B — GRASP-06 author hosting - -Applies to **every** PR publish from a repo path that has the objects — -checkout mode and fork mode alike. `RepoStore::open_pull_request` keeps its -signature; internals change: - -1. **Resolve author grasp servers** (new shared helper): latest kind-10317 - grasp list of the signed-in user from the local DB (`filters::grasp_list`, - `g` tags in order) → **fallback to settings defaults** - (`GraspServersSettings.default_servers` / `DEFAULT_GRASP_SERVERS`, the same - source the create-repo dialogs use) when no list is published. -2. **Build `/prs/` URLs**: `grasp_base_url(server) + "/prs/" + user_npub + - "/" + base_repo_id + ".git"` (npub form, like ngit; `grasp_base_url` maps - wss→https, ws→http). -3. **`clone` tag** = dedup of `/prs/` URLs plus the current base-announcement - clone URLs (order: `/prs/` first — the author's servers are the most likely - to be alive and author-controlled). -4. **Push loop** = author `/prs/` servers first (guaranteed writable — the - point of GRASP-06), then the base announcement grasp servers (existing - behavior), all `refs/nostr/` from `push_from`. Best-effort; - zero successes → existing `last_warning` banner; publishing always - proceeds. - -Effect: a repo announced with relays but no reachable grasp hosting still gets -a downloadable tip (on the author's own hosting), and git-native clients -(ngit, `git-remote-nostr`) can fetch Signed PR tips from the `clone` URL. - -**1619 updates are out of scope for v1**: the update dialog is paste-only, so -no repo path holds the new tip's objects. Deferred until the existing -"local-checkout generation for the update-PR dialog" TODO lands; then push the -new tip to the same `/prs/` set under the PR's stable ref -(`refs/nostr/`, advanced per revision — convention to verify -against ngit-grasp first). - ---- - -## 4. Workstream C — Checkout associations & suggestions - -Three tiers: **Remember → Auto-pick → Suggest**. - -### 4.1 Remember (associations) - -A local folder ↔ announced repo association comes from two sources: - -- **Explicit** (persistent settings records `{path, addr, last_used}`): - recorded when the repo header **Clone** action succeeds (addr known) and - when a folder pick succeeds in the New PR panel (store addr known). -- **Implicit** (derived, no persistence): among `LocalReposStore` scan results - (settings `local_repos.scan_paths`), a repo whose - - `origin` URL matches an announcement `clone` URL (compare host+path, - ignoring scheme: ws/wss/http/https/grasp are equivalent transports of the - same grasp URL), or - - root commit equals the announcement EUC - is a checkout of that announced repo. - -Resolution order per repo: remembered (freshest first) ∪ scanned-matched, -deduplicated by path, skipping missing directories. - -### 4.2 Auto-pick (New PR panel prefill) - -`open_new_pull_panel(…)` gains a suggested-checkout parameter, resolved by the -caller from the association store: - -- **Exactly one** checkout → auto-apply it: selectors populate, base = - announced HEAD, compare = current branch, diff loads. The folder button - becomes "Change…". -- **Several** → a small folder combobox instead of the modal folder picker. -- **None** → today's flow unchanged. -- Successful manual folder picks are recorded back (learning). - -### 4.3 Suggest (status + surfaces) - -A small checkout-status computation (part of the association store), scoped to -the bounded set of associated checkouts, on background threads: - -- Triggers: app open, window focus (debounced ~5 s), `LocalReposStore` rescan, - `BackendEvent::Synced`. -- Per checkout: current branch; commits ahead of the base branch (announced - HEAD name if present locally, else `main`, else first local branch — the - same rule as `apply_checkout`), via `rev-list --count`; whether the user has - an **open** PR from that branch on the target repo (author == me, - `branch-name` tag == branch, fallback: tip `c` tag == local HEAD). -- Result states: `ReadyToCreate { target, branch, ahead, base }` / - `HasOpenPr { … }` / `Idle`. -- Noise rules: only when ahead > 0 and branch ≠ base; nothing for dirty - worktrees; one entry per target repo. - -Surfaces: - -| Surface | Shows | Dedupe data source | Scope | -|---|---|---|---| -| Repo **PR list** banner (`PullRequestsView`) | "branch `feature` is 3 commits ahead of `main` — Create pull request →" (opens prefilled New PR) | live open `RepoStore` (precise) | v1 | -| **Sidebar** "Ready to contribute" group | row per `ReadyToCreate`: target repo, branch ↑N → opens target repo + prefilled New PR | v1: only targets with a live open store, else a local-DB query refreshed after a lazy per-target bootstrap activity sync; if the repo has no data yet, the group omits it (no false "ready") | v2 (after v1 proves out) | -| Repo detail header chip | tiny `feature ↑3` on the repo whose checkout is ahead | as PR-list banner | optional | - -The PullRequestsView banner reuses the existing dismissible `Alert` banner -pattern already used for store errors/warnings. - ---- - -## 5. Combined flow (fork mode, end to end) - -```mermaid -sequenceDiagram - participant U as User (New PR panel) - participant P as Base mirror (GitCache) - participant F as Fork grasp server - participant A as Author grasp (GRASP-06 /prs/) - participant B as Base repo grasps - participant R as Nostr relays - - U->>U: pick fork repo (u/EUC relation, yours first) + branch - U->>P: ensure_clone(base) + fetch_all - P-->>F: fetch +refs/heads/*:refs/fork///* - P-->>U: base branches (origin/*) + fork branches (refs/fork/…) - U->>P: merge-base, range commits, range diff (Files/Commits tabs) - U->>P: submit: format-patch merge-base..fork-ref - U->>R: publish kind-1617 series (root + NIP-10 chain, ≤60 KB each) - U->>R: sign kind-1618 (a=base, c=fork tip, merge-base, branch-name, clone=[/prs/…, base clone URLs], e=root patch, r=EUC) - U->>A: push tip → refs/nostr/ (author servers, first) - U->>B: push tip → refs/nostr/ (best-effort redundancy) - U->>R: publish kind-1618 - Note over R: zero successful pushes → last_warning banner only -``` - -Checkout mode is identical except the fork-import step; suggestions (workstream -C) only add entry-point shortcuts into this flow. - ---- - -## 6. Step-by-step implementation - -Phases are ordered so each step lands on green: foundations first, then the -publish-side change (benefits the existing checkout flow immediately), then -the fork UI, then the UX layer. Every step compiles, passes its tests, and -keeps existing behavior unchanged. - -### Phase 0 — Foundations - -#### Step 1 — `signed_core`: fork relation predicate - -- File: `crates/signed_core/src/model.rs`. -- Add `Announcement::is_fork_of(&self, base: &RepoAddr, base_euc: - Option<&str>) -> bool`: - `upstream.addr == Some(base)` OR (`base_euc` present AND `self.euc == - base_euc`), excluding self (same owner + id). -- Tests: u-tag coordinate match; shared EUC match; permanent fork with - different EUC matched via `u`; no-match; base-self exclusion. -- Done when: predicate + tests green; used by Step 5. - -#### Step 2 — `signed_git`: mirror/import primitives - -- File: `crates/signed_git/src/lib.rs`. -- `fetch_repo_refs(repo_path: &Path, urls: &[String], refspec: &str) -> - Result<()>` — CLI `git fetch `, grasp:// → https rewrite, - `GIT_TERMINAL_PROMPT=0`, try each URL until one works (last-error on all - failing, like `clone_repo`). -- `refs_with_prefix(repo_path: &Path, prefix: &str) -> Result>` — - full refnames under `prefix` (`git for-each-ref --format=%(refname)`), - sorted. -- `delete_refs_with_prefix(repo_path: &Path, prefix: &str) -> Result<()>` — - collect via `for-each-ref`, delete via `git update-ref --stdin` lines. -- `origin_url(workdir: &Path) -> Result>` (used by Step 7). -- Tests (existing `file://` fixture infra): import under a target prefix; - URL fallback to the working server; prefix listing; prefix deletion; - origin URL read. -- Done when: helpers + tests green. - -### Phase 1 — GRASP-06 hosting (publish side) - -#### Step 3 — author grasp-server resolution - -- File: `crates/signed_state/src/backend.rs` (or a small new module). -- `resolve_user_grasp_servers(cx, user) -> Vec`: latest kind-10317 - of `user` from the local DB (`filters::grasp_list`; latest event wins; `g` - tags in order) → fallback to settings `GraspServersSettings` - defaults/`DEFAULT_GRASP_SERVERS` when the user has no grasp list. -- Refactor the create-repo/init dialogs to share it (optional, keeps one - resolution path). -- Tests: latest-wins selection; missing list falls back; `g` order preserved. -- Done when: helper + tests green. - -#### Step 4 — `open_pull_request` hosting - -- File: `crates/signed_state/src/repo.rs` (`open_pull_request`, ~L657). -- Pure helpers (unit-testable): `grasp06_prs_url(base_url: &str, npub: - &str, repo_id: &str) -> String`; push-target assembly (author `/prs/` - URLs first, then announcement grasp URLs; dedup). -- `clone` tag = `/prs/` URLs (author npub from `Backend::current_user()`, - repo id from `self.addr().identifier`) + base announcement clone URLs. -- Push loop extended: existing per-relay loop stays; author servers push to - the `/prs/` URL instead of the repo URL. Warning semantics unchanged. -- Tests: URL building; target order; dedup. Behavioral coverage of the full - publish is manual/e2e (see §8) until a harness exists. -- Done when: checkout-mode PRs push to the author's grasp list - (`/prs//.git`) first and the `clone` tag carries those URLs; - all-servers-fail still publishes with a warning. - -### Phase 2 — Fork-aware compare - -#### Step 5 — fork candidates - -- File: `crates/workspace/src/views/repo_detail/new_pull_request.rs` (helper) - or `crates/signed_state`. -- `fork_candidates(cx) -> Vec`: filter - `RepoListStore::global().announcements` with `is_fork_of` (Step 1); exclude - empty `clone`; sort own forks (owner == current user) first, then others, - each group by recency/name. Re-read each time the picker opens. -- Done when: helper returns the expected ordering for a mixed list. - -#### Step 6 — New PR panel fork mode - -- File: `crates/workspace/src/views/repo_detail/new_pull_request.rs`. -- State: `CompareSource { Checkout, Fork { announcement } }`; per-mode item - sets for both selectors; `mirror_path`; fork branch list; full-ref - base/compare tracking (display keeps short names). -- `choose_fork(announcement)` + `prepare_fork` (mirror `choose_checkout` / - `apply_checkout` async shape, `compare_generation` guard): ensure base - mirror (`ensure_clone` + `fetch_all`), prune previous `refs/fork/…` - prefix, import fork heads (`fetch_repo_refs`), list both ref sets - (`refs_with_prefix`), populate selectors with defaults, `reload_compare`. -- `reload_compare` / `submit` / `open_commit_diff` become mode-aware (path + - base ref + compare ref resolution; `format_patch_between` and - `push_from` on `P_base`; published `branch-name` = short name). -- UI (compare bar): source control next to "Pull From" (local checkout / - announced fork), fork-repo combobox (grouped, own forks first), refresh - affordance, "Change…" back to checkout; loading spinner; inline errors. -- Done when: checkout mode is byte-identical in behavior; fork mode shows - base/fork selectors, Files/Commits tabs, commit diffs, and publishes with - the correct tags (manual §8). - -### Phase 3 — Checkout associations & suggestions - -#### Step 7 — association store - -- Files: `crates/settings` (extend the settings model like - `local_repos.scan_paths` with remembered checkouts - `{path, addr, last_used}`); new `crates/signed_state/src/checkouts.rs` - (global store, `Arc` + debounce pattern from `LocalReposStore`/ - `RepoListStore`). -- API: `associations_for(addr) -> Vec` (remembered freshest-first ∪ - scanned-matched by origin URL/EUC via Step 2's `origin_url` + - `signed_git::root_commit`; scheme-insensitive URL compare; dedup; skip - missing dirs); `record(path, addr)`. -- Recording hooks: repo header `clone_to_folder` success - (`repo_detail/mod.rs`) and `choose_checkout` success (panel). -- Tests: matching by origin URL (scheme variants), by EUC, no match; dedup - and ordering. -- Done when: associations resolve correctly and persist. - -#### Step 8 — New PR prefill - -- Files: `new_pull_request.rs` (`open_new_pull_panel` + `new`); callers - `repo_detail/mod.rs` header and `pull_requests.rs`. -- Entry param `suggested_checkout: Option` (default `None`); - panel applies it on construction when the folder still exists, else falls - back to the empty state. When several candidates exist the caller passes - the freshest and the panel offers the others through a folder combobox - (new small control next to the source button). -- Done when: opening New PR on a repo with a remembered/matched checkout - never shows the folder dialog; manual picks get remembered. - -#### Step 9 — status computation + PR-list banner - -- Files: `checkouts.rs` (status states + triggers + debounce), workspace - `pull_requests.rs` (banner), `new_pull_request.rs` (accepts the banner's - "create" click by opening prefilled). -- Status rules from §4.3; banner dedupe against the live open `RepoStore` - (author + `branch-name`, fallback tip match, open status only). -- Done when: after committing on an associated checkout and opening the - target repo's PR list, the banner appears exactly when ahead > 0 and no - open PR exists, and disappears after creating/merging/evening. - -#### Step 10 — sidebar "Ready to contribute" group (v2, optional) - -- File: `crates/workspace/src/views/sidebar/mod.rs` (+ `checkouts.rs` - support). -- Only list targets with reliable dedupe data (live open store, else a - local-DB activity query refreshed after a lazy per-target bootstrap - activity sync); omit everything uncertain. Clicking a row opens the target - repo (`open_repo_panel`) + prefilled New PR. -- Done when: rows appear without false "ready" entries (dedupe-uncertain - targets omitted). - -### Phase 4 — Docs & validation - -#### Step 11 — documentation and final validation - -- Update `docs/PR_FLOW.md`: fork compare path, GRASP-06 server set + clone - tag, suggestion surfaces; the mermaid sequence in §5. -- Update `docs/TODO.md`: tick "Fork-aware compare…", "GRASP-06 …"; add the - checkout-suggestions item; keep deferred items (1619 update push, sidebar - group, reading-side clone-URL fetch) explicit. -- Run the manual validation checklist (§8) end to end. - ---- - -## 7. Error handling & edge cases (all inline or warnings, as today) - -- Base mirror unreachable / no base `clone` URLs → panel error in fork mode; - checkout mode unaffected. -- Fork unreachable / without `clone` URLs (excluded from candidates) → panel - error. -- No common ancestor → existing error (range flow needs shared history; Send - Patch remains the fallback). -- Author has no 10317 list and no default servers → GRASP-06 adds nothing; - today's warning stands. -- Author's grasp server does not implement `/prs/` → its push fails silently - in the loop; its URL in `clone` is inert; base grasps still tried. -- Fork branch deleted upstream / fork switched → prune prefix + re-import; - generation guard discards stale compares. -- Concurrency on `P_base` with the repo browser: we never checkout; git ref - locks make overlapping fetches safe (same class as today's browser refresh). -- Multiple checkouts of one repo → freshest first, "Change…"/combobox for the - rest. -- Branch renamed after a PR → dedupe falls back to tip-commit matching; - otherwise a duplicate suggestion may appear once (accepted v1 tradeoff). -- Suggestions never block UI; results arrive as `Arc` swaps. - -## 8. Non-goals / deferred (explicitly out of scope) - -- 1619 update hosting (depends on the local-checkout update-dialog TODO). -- Paste/Send-Patch flow keeps no git push (no object store; patches = truth; - no scratch-apply resurrection). -- Reading side: fetching other clients' PR tips from `clone` URLs into the - mirror (`ngit pr checkout` analog) — only needed for patch-less PRs. -- Fork creation UI (Signed still cannot announce forks; they come from ngit or - by publishing a clone) — fork candidates simply won't include non-existent - ones. -- GitHub-isms rejected: no fork-network browser, no per-fork PR pages, no - fork identity in events, no "compare across forks" for strangers' branches - beyond what is listed above. -- The panel never auto-submits anything; suggestions only navigate and - prefill. - -## 9. Validation checklist (manual e2e) - -1. Checkout mode regression: clone a repo to disk, branch + commit (external - git), New PR → choose folder → diff/commits → Create → PR appears on the - target repo's PR list; tip pushed to the author's `/prs/` server(s) from - the 10317 list (fallback: defaults); `clone` tag lists `/prs/` URLs first. -2. All grasp servers down/absent → PR still publishes; warning banner shows. -3. Fork mode: with a fork announcement related to the base (own fork first, - other author's fork listed), pick repo + branch → selectors, Files/Commits - tabs, commit-diff rows correct; published 1618 carries `a` = base - coordinate, `c` = fork tip, `merge-base` = fork point, `branch-name` = - fork branch; tip fetchable from the advertised `/prs/` URL via a plain - `git fetch`. -4. Prefill: reopen New PR for the same repo → folder auto-chosen, selectors - populated; "Change…" works. -5. Banner: with the repo's PR list open and an associated checkout ahead with - no open PR → banner appears; disappears after publishing a PR, after - merging, and when the branch is even. -6. Interop: an ngit/git client fetches a Signed PR's tip from the `/prs/` - clone URL (requires a GRASP-06-enabled server). - -## 10. Implementation log (2026-09-03, branch `feat/fork`) - -All steps below landed with unit tests; `cargo test` across `signed_core` -(44), `signed_git` (61), `signed_state` (15), `workspace` (14) and -`settings` (9) is green, and `cargo check` on the whole workspace passes. -The manual e2e checklist above still needs a real GRASP-06 server run. - -- **Step 1** — `Announcement::is_fork_of` (`signed_core::model`) + 4 tests. -- **Step 2** — `signed_git`: `fetch_repo_refs`, `refs_with_prefix`, - `delete_refs_with_prefix`, `origin_url`, `GitCache::root()` + 4 tests - (import/list/prune against `file://` fixtures incl. URL fallback). -- **Step 3** — backend grasp-list resolution: `grasp_list_servers`, - `latest_grasp_list_servers`, `user_grasp_list_servers` (DB query, latest - wins) + `grasp06_prs_url` and `pr_clone_urls` (author-first, dedup) + 4 - tests. `signed_state` gained a `settings` dependency for the defaults - fallback. -- **Step 4** — `RepoStore::open_pull_request` (signature unchanged): - resolves the author's grasp servers (10317 → settings defaults) inside - the publish task, builds the `clone` tag from `/prs/` URLs first, pushes - author `/prs/` targets before the base announcement's servers, deduped; - all-fail keeps the `last_warning` banner. 1619 updates untouched - (deferred, as planned). -- **Step 5** — `fork_candidates` ordering helper + 2 tests (own forks - first; base/unrelated/no-clone excluded; EUC-less base still matches via - `u`). -- **Step 6** — New PR panel fork mode: `ForkCompare` state, `choose_fork`/ - `apply_fork` (mirror `ensure_clone` → prune `refs/fork` → import → list - both ref sets), mode-aware `base_ref`/`compare_ref`/`work_path` used by - `reload_compare`/`submit`/`open_commit_diff`, stale-result guard, - refresh-by-re-picking + refresh button, and a "Source" picker menu - (checkout rows + forks) replacing the folder button. Checkout mode stays - byte-identical in behavior. Deviations from the plan: selectors and the - source picker keep one shared layout (no separate fork-repo combobox — - the source menu lists forks grouped own-first, matching the ordering - requirement); `IconName::GitBranch` does not exist upstream so fork rows - use the project's `CustomIconName::GitBranch`. -- **Step 7** — settings `CheckoutRecord`/`CheckoutsSettings` group + new - `signed_state::checkouts::CheckoutsStore` global (observe settings / - local scan / announcements; debounced, coalesced, Arc-swapped): - scheme-insensitive `same_repo_url`, `resolve_associations` (remembered - freshest-first ∪ scanned origin/EUC matches, dedup, mirror-cache paths - excluded), `record()`, per-repo `request_statuses`/`statuses_of` with - `CheckoutStatus` (branch/head/base/ahead; dirty and detached checkouts - never suggested; 15 s poll while any PR list is open). 7 tests. - Deviations: settings records store the address as a string (the settings - crate stays free of nostr types); mirror exclusion uses the cache root - (new `GitCache::root()`); statuses are computed per requested repo with - the announced HEAD supplied by the open list panel rather than from a - 30618 DB query. -- **Step 8** — New PR panel prefills the freshest associated checkout on - construction (no folder dialog); `apply_folder_path` applies a given - path; successful folder picks and header clones are recorded back; the - Source menu lists associated checkouts (checked when applied) plus - "Choose another folder…". Deviations: instead of a separate folder - combobox, the alternatives live in the Source menu (fewer controls, same - outcome); `open_new_pull_panel` needed no signature change because the - panel reads the association store itself. -- **Step 9** — "ready to contribute" banner: `RepoDetailView` requests - the statuses while the repository panel is open (re-requested when the - announced HEAD lands or changes) and renders the banner under the repo - header; the first ready checkout not covered by an open PR of the - signed-in user (`branch-name`, fallback `c`-tag tip) and not dismissed - (per-panel dismissal set) is offered with a Create button opening the - prefilled panel. The dedupe predicate is the tested - `pr_proposes_checkout` in `signed_state::checkouts`. Deviation from the - plan: the surface is the repository panel (not the PR-list panel, as the - user requested after v1; the PR list keeps only its error/warning - banners), and there is no window-focus trigger (no precedent in the - codebase; the 15 s poll plus open/rescan/settings triggers cover the - plan's "done when" cases). -- **Step 10** — deferred (v2, optional), per plan; the banner is the v1 - surface. -- **Step 11** — `docs/PR_FLOW.md` rewritten for the current panel flow - (fork import, GRASP-06 hosting, suggestions, deferred items explicit); - `docs/TODO.md` updated; this log added. Manual e2e (§9) not yet run - against a live GRASP-06 server. -- **Fix (after e2e, user report)** — creating a repository left the - project only inside the app's GitCache mirror: the announcement, state - event and push happened, but the folder chosen in the Create Repository - dialog was just remembered as a settings default. `Backend:: - create_repository` now also materializes a working copy at - `/` (cloned from the mirror via a `file://` URL - so it shares the announced history exactly, then `origin` re-pointed at - the first grasp server through the new `signed_git::set_origin`), and - the dialog records it as a checkout (`CheckoutsStore`, so the New PR - panel pre-fills it), opens it in the system file manager and opens the - repository panel. Materialization runs before any event is published, - so a failure aborts creation cleanly with nothing announced. Two new - `signed_git` tests (`set_origin_creates_or_replaces_the_remote`, - `working_copy_cloned_from_the_mirror_matches_head_and_origin`). -- **Add (after e2e, user request)** — "ready to push" watch for the - user's own repositories: local commits made in a checkout (external - git) surface as a **sidebar badge** on the repository row (a - `CountBadge` with the unpushed commit count) and, when the repository - panel is open, as an info **banner with a Push button**. The - `CheckoutsStore` gains a second status family (`request_push_statuses`/ - `push_statuses_of`): per checked-out branch it refreshes the remote - view (`git fetch` of the checkout's origin, offline-tolerant) and - counts `origin/..` (`origin/HEAD` for branches the - remote does not have yet); dirty/detached checkouts are skipped like - the PR suggestions. Poll cadence: 15 s while a repository panel is - open (`status_requested`), 60 s for the sidebar-only background watch; - request sets are cleared on signer change. The repo panel's - ready-to-contribute banner now applies only to repositories of other - authors — owned repositories get the push banner instead, whose Push - action calls the new `Backend::push_checkout` (shared body with the - existing mirror-based `push_repository`): publishes a fresh 30618 - state event (keeping the announced `HEAD` branch when the checkout is - on a side branch) then pushes every branch and tag to the announced - grasp servers. New test - `checkout_push_status_counts_unpushed_commits_only`. The mirror's file - browser stays a snapshot (new commits appear after a branch switch), - like the rest of the browser. -- **Fix (after e2e, user report)** — a push warning "cannot lock ref - 'refs/heads/main': is at X but expected Y" (server-side compare-and- - swap rejection, `incorrect old value provided`). Reproduced locally: - two concurrent plain pushes of the *same* ref from the same base make - the loser fail exactly this way — the app can race itself when two - push sources for one repository run at once (two panels of the same - repo, or the banner Push racing the header's Republish; each guard was - per-view only). Fix: pushes are now single-flight per repository in - `Backend::push_repo_from` via an `Arc>>` guard - (`PushGuard`, RAII: the lock is released on completion, on error and on - task cancellation alike); a second concurrent push fails fast with - "A push to this repository is already in progress" instead of racing. - Racing an external `git push` against the same server remains possible - (benign: the ref converges; the loser logs a warning only). -- **Fix (after e2e, user report)** — after a successful push the - repository panel's commit list stayed on the old commit (even across - restarts): the browser reads the GitCache mirror, and a fetch never - moves a mirror's *local* branches — `origin/main` advanced while local - `main` (what the commit list walks) stayed behind. ngit/nak never hit - this because they operate on real clones the user `git pull`s; nak also - publishes the updated 30618 state *before* each push, which Signed - already did. Fixes, mirroring a `git pull --ff-only` on the browser - clone: new `signed_git::fast_forward_branches(workdir)` (per local - branch, when it is an ancestor of its `refs/remotes/origin/*` - counterpart: the checked-out branch is merged so its worktree follows, - dirty worktrees and local-only commits are never touched; returns - whether anything moved); `RepoDetailView::load_repo`'s background - refresh fast-forwards after `fetch_all` and rebuilds the explorer, - previews and commit list (`reload_worktree`) when anything moved; - `push_unpushed_checkout` reloads the mirror on success so an owned - repo's pushed commit appears immediately; `ensure_origin` now also - configures the standard `remote.origin.fetch` refspec (create-flow - mirrors otherwise never map heads on fetch). New test - `fast_forward_branches_moves_the_mirror_and_keeps_local_work`; the - remote-only-branch limitation stays (a branch the mirror has never - checked out is not listed), as documented. +## Suggested order of operations + +1. Step 1 + Step 2 first — pure deletions, verify with `cargo check` + + `cargo test`. +2. Step 3 (library swaps) next — mechanical, mostly call-site migrations. +3. Step 4 (dedup) — largest line savings. +4. Step 5 (structural) — deliberate, one at a time. +5. Step 6 (over-optimization) — optional, only if you want the extra ~110 L. + +Each step below is self-contained; tick what you want done. + +- [ ] Step 1 — zero-risk deletions +- [ ] Step 2 — dead feature removal +- [ ] Step 3 — library swaps +- [ ] Step 4 — dedup passes +- [ ] Step 5 — structural changes +- [ ] Step 6 — over-optimization cleanup