From 3eb2354edb8e76428625d5645e110c30aa1ccc2a Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 18 Jul 2025 11:56:15 +0100 Subject: feat(pr): list PR and PR updates remote will list the refs under `pr/*` namespace. `ngit list` will display in the list of open / draft proposals. it won't yet fetch the related oids to enable fetching or checking out the branch. --- src/bin/ngit/sub_commands/list.rs | 157 ++++++++++++++++++++++++++------------ 1 file changed, 107 insertions(+), 50 deletions(-) (limited to 'src/bin/ngit/sub_commands/list.rs') diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index 0330be1..a90b28e 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -3,10 +3,12 @@ use std::{io::Write, ops::Add}; use anyhow::{Context, Result, bail}; use ngit::{ client::{ - Params, get_all_proposal_patch_events_from_cache, get_proposals_and_revisions_from_cache, + Params, get_all_proposal_patch_pr_pr_update_events_from_cache, + get_proposals_and_revisions_from_cache, }, git_events::{ - get_commit_id_from_patch, get_most_recent_patch_with_ancestors, status_kinds, tag_value, + KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, get_commit_id_from_patch, + get_pr_tip_event_or_most_recent_patch_with_ancestors, status_kinds, tag_value, }, }; use nostr_sdk::Kind; @@ -184,21 +186,22 @@ pub async fn launch() -> Result<()> { let cover_letter = event_to_cover_letter(proposals_for_status[selected_index]) .context("failed to extract proposal details from proposal root event")?; - let commits_events: Vec = get_all_proposal_patch_events_from_cache( - git_repo_path, - &repo_ref, - &proposals_for_status[selected_index].id, - ) - .await?; + let commits_events: Vec = + get_all_proposal_patch_pr_pr_update_events_from_cache( + git_repo_path, + &repo_ref, + &proposals_for_status[selected_index].id, + ) + .await?; - let Ok(most_recent_proposal_patch_chain) = - get_most_recent_patch_with_ancestors(commits_events.clone()) + let Ok(most_recent_proposal_patch_chain_or_pr_or_pr_update) = + get_pr_tip_event_or_most_recent_patch_with_ancestors(commits_events.clone()) else { if Interactor::default().confirm( PromptConfirmParms::default() .with_default(true) .with_prompt( - "failed to find any patches on this proposal. choose another proposal?", + "failed to find any PR or patch events on this proposal. choose another proposal?", ), )? { continue; @@ -208,15 +211,37 @@ pub async fn launch() -> Result<()> { // for commit in &most_recent_proposal_patch_chain { // println!("recent_event: {:?}", commit.as_json()); // } + if most_recent_proposal_patch_chain_or_pr_or_pr_update + .iter() + .any(|e| [KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains(&e.kind)) + { + match Interactor::default().choice( + PromptChoiceParms::default() + .with_prompt("this is new PR event kind which ngit doesnt yet support") + .with_default(0) + .with_choices(vec!["back to proposals".to_string()]), + )? { + 0 => continue, + _ => { + bail!("unexpected choice") + } + }; + } - let binding_patch_text_ref = format!("{} commits", most_recent_proposal_patch_chain.len()); - let patch_text_ref = if most_recent_proposal_patch_chain.len().gt(&1) { + let binding_patch_text_ref = format!( + "{} commits", + most_recent_proposal_patch_chain_or_pr_or_pr_update.len() + ); + let patch_text_ref = if most_recent_proposal_patch_chain_or_pr_or_pr_update + .len() + .gt(&1) + { binding_patch_text_ref.as_str() } else { "1 commit" }; - let no_support_for_patches_as_branch = most_recent_proposal_patch_chain + let no_support_for_patches_as_branch = most_recent_proposal_patch_chain_or_pr_or_pr_update .iter() .any(|event| !patch_supports_commit_ids(event)); @@ -253,8 +278,13 @@ pub async fn launch() -> Result<()> { )?; continue; } - 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 2 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 1 => { + launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update) + } + 2 => save_patches_to_dir( + most_recent_proposal_patch_chain_or_pr_or_pr_update, + &git_repo, + ), 3 => continue, _ => { bail!("unexpected choice") @@ -277,9 +307,11 @@ pub async fn launch() -> Result<()> { .eq(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?); let proposal_base_commit = str_to_sha1(&tag_value( - most_recent_proposal_patch_chain.last().context( - "there should be at least one patch as we have already checked for this", - )?, + most_recent_proposal_patch_chain_or_pr_or_pr_update + .last() + .context( + "there should be at least one patch as we have already checked for this", + )?, "parent-commit", )?) .context("failed to get valid parent commit id from patch")?; @@ -300,8 +332,8 @@ pub async fn launch() -> Result<()> { ], ))? { 0 | 3 => continue, - 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 2 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update), + 2 => save_patches_to_dir(most_recent_proposal_patch_chain_or_pr_or_pr_update, &git_repo), _ => { bail!("unexpected choice") } @@ -309,9 +341,13 @@ pub async fn launch() -> Result<()> { } let proposal_tip = str_to_sha1( - &get_commit_id_from_patch(most_recent_proposal_patch_chain.first().context( - "there should be at least one patch as we have already checked for this", - )?) + &get_commit_id_from_patch( + most_recent_proposal_patch_chain_or_pr_or_pr_update + .first() + .context( + "there should be at least one patch as we have already checked for this", + )?, + ) .context("failed to get valid commit_id from patch")?, ) .context("failed to get valid commit_id from patch")?; @@ -325,7 +361,7 @@ pub async fn launch() -> Result<()> { .choice(PromptChoiceParms::default().with_default(0).with_choices(vec![ format!( "create and checkout proposal branch ({} ahead {} behind '{main_branch_name}')", - most_recent_proposal_patch_chain.len(), + most_recent_proposal_patch_chain_or_pr_or_pr_update.len(), proposal_behind_main.len(), ), format!("apply to current branch with `git am`"), @@ -337,7 +373,7 @@ pub async fn launch() -> Result<()> { let _ = git_repo .apply_patch_chain( &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, - most_recent_proposal_patch_chain, + most_recent_proposal_patch_chain_or_pr_or_pr_update, ) .context("failed to apply patch chain")?; @@ -347,8 +383,8 @@ pub async fn launch() -> Result<()> { ); Ok(()) } - 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 2 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update), + 2 => save_patches_to_dir(most_recent_proposal_patch_chain_or_pr_or_pr_update, &git_repo), 3 => continue, _ => { bail!("unexpected choice") @@ -382,7 +418,7 @@ pub async fn launch() -> Result<()> { .with_choices(vec![ format!( "checkout proposal branch ({} ahead {} behind '{main_branch_name}')", - most_recent_proposal_patch_chain.len(), + most_recent_proposal_patch_chain_or_pr_or_pr_update.len(), proposal_behind_main.len(), ), format!("apply to current branch with `git am`"), @@ -401,8 +437,13 @@ pub async fn launch() -> Result<()> { ); Ok(()) } - 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 2 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 1 => { + launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update) + } + 2 => save_patches_to_dir( + most_recent_proposal_patch_chain_or_pr_or_pr_update, + &git_repo, + ), 3 => continue, _ => { bail!("unexpected choice") @@ -414,11 +455,14 @@ pub async fn launch() -> Result<()> { git_repo.get_commits_ahead_behind(&master_tip, &local_branch_tip)?; // new appendments to proposal - if let Some(index) = most_recent_proposal_patch_chain.iter().position(|patch| { - get_commit_id_from_patch(patch) - .unwrap_or_default() - .eq(&local_branch_tip.to_string()) - }) { + if let Some(index) = most_recent_proposal_patch_chain_or_pr_or_pr_update + .iter() + .position(|patch| { + get_commit_id_from_patch(patch) + .unwrap_or_default() + .eq(&local_branch_tip.to_string()) + }) + { return match Interactor::default().choice( PromptChoiceParms::default() .with_default(0) @@ -437,7 +481,7 @@ pub async fn launch() -> Result<()> { let _ = git_repo .apply_patch_chain( &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, - most_recent_proposal_patch_chain, + most_recent_proposal_patch_chain_or_pr_or_pr_update, ) .context("failed to apply patch chain")?; println!( @@ -448,8 +492,13 @@ pub async fn launch() -> Result<()> { ); Ok(()) } - 1 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 2 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 1 => { + launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update) + } + 2 => save_patches_to_dir( + most_recent_proposal_patch_chain_or_pr_or_pr_update, + &git_repo, + ), 3 => continue, _ => { bail!("unexpected choice") @@ -467,7 +516,7 @@ pub async fn launch() -> Result<()> { }) { println!( "updated proposal available ({} ahead {} behind '{main_branch_name}'). existing version is {} ahead {} behind '{main_branch_name}'", - most_recent_proposal_patch_chain.len(), + most_recent_proposal_patch_chain_or_pr_or_pr_update.len(), proposal_behind_main.len(), local_ahead_of_main.len(), local_beind_main.len(), @@ -492,11 +541,11 @@ pub async fn launch() -> Result<()> { git_repo.checkout( &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, )?; - let chain_length = most_recent_proposal_patch_chain.len(); + let chain_length = most_recent_proposal_patch_chain_or_pr_or_pr_update.len(); let _ = git_repo .apply_patch_chain( &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, - most_recent_proposal_patch_chain, + most_recent_proposal_patch_chain_or_pr_or_pr_update, ) .context("failed to apply patch chain")?; println!( @@ -520,8 +569,13 @@ pub async fn launch() -> Result<()> { ); Ok(()) } - 2 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 3 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 2 => { + launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update) + } + 3 => save_patches_to_dir( + most_recent_proposal_patch_chain_or_pr_or_pr_update, + &git_repo, + ), 4 => continue, _ => { bail!("unexpected choice") @@ -581,7 +635,7 @@ pub async fn launch() -> Result<()> { if git_repo.does_commit_exist(&proposal_tip.to_string())? { println!( "you have previously applied the latest version of the proposal ({} ahead {} behind '{main_branch_name}') but your local proposal branch has amended or rebased it ({} ahead {} behind '{main_branch_name}')", - most_recent_proposal_patch_chain.len(), + most_recent_proposal_patch_chain_or_pr_or_pr_update.len(), proposal_behind_main.len(), local_ahead_of_main.len(), local_beind_main.len(), @@ -594,7 +648,7 @@ pub async fn launch() -> Result<()> { "your local proposal branch ({} ahead {} behind '{main_branch_name}') has conflicting changes with the latest published proposal ({} ahead {} behind '{main_branch_name}')", local_ahead_of_main.len(), local_beind_main.len(), - most_recent_proposal_patch_chain.len(), + most_recent_proposal_patch_chain_or_pr_or_pr_update.len(), proposal_behind_main.len(), ); @@ -639,11 +693,11 @@ pub async fn launch() -> Result<()> { &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, &proposal_base_commit.to_string(), )?; - let chain_length = most_recent_proposal_patch_chain.len(); + let chain_length = most_recent_proposal_patch_chain_or_pr_or_pr_update.len(); let _ = git_repo .apply_patch_chain( &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, - most_recent_proposal_patch_chain, + most_recent_proposal_patch_chain_or_pr_or_pr_update, ) .context("failed to apply patch chain")?; @@ -658,8 +712,11 @@ pub async fn launch() -> Result<()> { ); Ok(()) } - 2 => launch_git_am_with_patches(most_recent_proposal_patch_chain), - 3 => save_patches_to_dir(most_recent_proposal_patch_chain, &git_repo), + 2 => launch_git_am_with_patches(most_recent_proposal_patch_chain_or_pr_or_pr_update), + 3 => save_patches_to_dir( + most_recent_proposal_patch_chain_or_pr_or_pr_update, + &git_repo, + ), 4 => continue, _ => { bail!("unexpected choice") -- cgit v1.2.3 From a3d4c8eaa263f4adb174ac81c4248fa200e1857e Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 18 Jul 2025 17:33:11 +0100 Subject: feat(pr): fetch pr and pr updates from clone urls we try and get them from clone urls of repo and fallback to those specified by contributor --- src/bin/git_remote_nostr/fetch.rs | 105 +++++++++++++++++++++++++++++++++----- src/bin/ngit/sub_commands/list.rs | 6 ++- src/lib/client.rs | 10 +++- src/lib/git_events.rs | 13 +++++ 4 files changed, 118 insertions(+), 16 deletions(-) (limited to 'src/bin/ngit/sub_commands/list.rs') diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs index b191850..f3d4362 100644 --- a/src/bin/git_remote_nostr/fetch.rs +++ b/src/bin/git_remote_nostr/fetch.rs @@ -1,6 +1,6 @@ use core::str; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, io::Stdin, sync::{Arc, Mutex}, time::Instant, @@ -16,7 +16,7 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, utils::check_ssh_keys, }, - git_events::tag_value, + git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, tag_value}, login::get_curent_user, repo_ref::{RepoRef, is_grasp_server}, }; @@ -37,38 +37,78 @@ pub async fn run_fetch( ) -> Result<()> { let mut fetch_batch = get_oids_from_fetch_batch(stdin, oid, refstr)?; - let oids_from_git_servers = fetch_batch + let oids_from_state = fetch_batch .iter() .filter(|(refstr, _)| !refstr.contains("refs/heads/pr/")) .map(|(_, oid)| oid.clone()) .collect::>(); + let pr_oid_clone_url_map = identify_clone_urls_for_oids_from_pr_pr_update_events( + fetch_batch.values().collect::>(), + git_repo, + repo_ref, + ) + .await?; + + let oids_to_fetch_from_git_servers = [ + oids_from_state.clone(), + pr_oid_clone_url_map + .keys() + .cloned() + .collect::>(), + ] + .concat(); + + let git_servers = { + let mut seen: HashSet = HashSet::new(); + let mut out: Vec = vec![]; + for server in &repo_ref.git_server { + if seen.insert(server.clone()) { + out.push(server.clone()); + } + } + for url in pr_oid_clone_url_map.values().flatten() { + if seen.insert(url.clone()) { + out.push(url.clone()); + } + } + out + }; + let mut errors = vec![]; let term = console::Term::stderr(); - for git_server_url in &repo_ref.git_server { + for git_server_url in &git_servers { + let oids_to_fetch_from_server = oids_to_fetch_from_git_servers + .clone() + .into_iter() + .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) + .collect::>(); + + if oids_to_fetch_from_server.is_empty() { + continue; + } + let term = console::Term::stderr(); if let Err(error) = fetch_from_git_server( git_repo, - &oids_from_git_servers, + &oids_from_state, git_server_url, &repo_ref.to_nostr_git_url(&None), &term, is_grasp_server(git_server_url, &repo_ref.grasp_servers()), ) { errors.push(error); - } else { - break; } } - if oids_from_git_servers + if oids_from_state .iter() .any(|oid| !git_repo.does_commit_exist(oid).unwrap()) && !errors.is_empty() { bail!( - "fetch: failed to fetch objects in nostr state event from:\r\n{}", + "fetch: failed to fetch objects from:\r\n{}", errors .iter() .map(|e| format!(" - {e}")) @@ -79,12 +119,43 @@ pub async fn run_fetch( fetch_batch.retain(|refstr, _| refstr.contains("refs/heads/pr/")); - fetch_open_or_draft_proposals(git_repo, &term, repo_ref, &fetch_batch).await?; + fetch_open_or_draft_proposals_from_patches(git_repo, &term, repo_ref, &fetch_batch).await?; + // TODO fetch_open_or_draft_proposals just needs to do it for patches term.flush()?; println!(); Ok(()) } +async fn identify_clone_urls_for_oids_from_pr_pr_update_events( + oids: Vec<&String>, + git_repo: &Repo, + repo_ref: &RepoRef, +) -> Result>> { + let mut map: HashMap> = HashMap::new(); + + let open_and_draft_proposals = get_open_or_draft_proposals(git_repo, repo_ref).await?; + + for (_, (_, events)) in open_and_draft_proposals { + for event in events { + if [KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains(&event.kind) { + if let Ok(c) = tag_value(&event, "c") { + if oids.contains(&&c) { + for tag in event.tags.as_slice() { + if tag.kind().eq(&nostr::event::TagKind::Clone) { + for clone_url in tag.as_slice().iter().skip(1) { + map.entry(c.clone()).or_default().push(clone_url.clone()); + } + } + } + } + } + } + } + } + + Ok(map) +} + pub fn make_commits_for_proposal( git_repo: &Repo, repo_ref: &RepoRef, @@ -128,7 +199,7 @@ pub fn make_commits_for_proposal( Ok(tip_commit_id) } -async fn fetch_open_or_draft_proposals( +async fn fetch_open_or_draft_proposals_from_patches( git_repo: &Repo, term: &console::Term, repo_ref: &RepoRef, @@ -140,12 +211,19 @@ async fn fetch_open_or_draft_proposals( let current_user = get_curent_user(git_repo)?; for refstr in proposal_refs.keys() { - if let Some((_, (_, patches))) = find_proposal_and_patches_by_branch_name( + if let Some((_, (_, events_to_apply))) = find_proposal_and_patches_by_branch_name( refstr, &open_and_draft_proposals, current_user.as_ref(), ) { - if let Err(error) = make_commits_for_proposal(git_repo, repo_ref, patches) { + if events_to_apply + .iter() + .any(|e| e.kind.eq(&KIND_PULL_REQUEST) || e.kind.eq(&KIND_PULL_REQUEST_UPDATE)) + { + // do nothing - we fetch these oids as part of run_fetch + } else if let Err(error) = + make_commits_for_proposal(git_repo, repo_ref, events_to_apply) + { term.write_line( format!("WARNING: failed to create branch for {refstr}, error: {error}",) .as_str(), @@ -429,6 +507,7 @@ fn fetch_from_git_server_url( remote_callbacks.credentials(auth.credentials(&git_config)); } fetch_options.remote_callbacks(remote_callbacks); + git_server_remote.download(oids, Some(&mut fetch_options))?; git_server_remote.disconnect()?; diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index a90b28e..9e35b33 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -219,7 +219,11 @@ pub async fn launch() -> Result<()> { PromptChoiceParms::default() .with_prompt("this is new PR event kind which ngit doesnt yet support") .with_default(0) - .with_choices(vec!["back to proposals".to_string()]), + .with_choices(vec![ + // TODO enable checkout by fetching oids, creating / updating branch and + // checking out + "back to proposals".to_string(), + ]), )? { 0 => continue, _ => { diff --git a/src/lib/client.rs b/src/lib/client.rs index 1f3b08c..091d68d 100644 --- a/src/lib/client.rs +++ b/src/lib/client.rs @@ -49,7 +49,8 @@ use crate::{ git::{Repo, RepoActions, get_git_config_item}, git_events::{ KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, event_is_cover_letter, - event_is_patch_set_root, event_is_revision_root, status_kinds, + event_is_patch_set_root, event_is_revision_root, event_is_valid_pr_or_pr_update, + status_kinds, }, login::{get_likely_logged_in_user, user::get_user_ref_from_cache}, repo_ref::RepoRef, @@ -1824,6 +1825,7 @@ pub async fn get_proposals_and_revisions_from_cache( .await? .iter() .filter(|e| event_is_patch_set_root(e) || e.kind.eq(&KIND_PULL_REQUEST)) + .filter(|e| e.kind.eq(&Kind::GitPatch) || event_is_valid_pr_or_pr_update(e)) .cloned() .collect::>(); proposals.sort_by_key(|e| e.created_at); @@ -1874,7 +1876,11 @@ pub async fn get_all_proposal_patch_pr_pr_update_events_from_cache( .iter() .copied() .collect(); - commit_events.retain(|e| permissioned_users.contains(&e.pubkey)); + + commit_events.retain(|e| { + permissioned_users.contains(&e.pubkey) + && (e.kind.eq(&Kind::GitPatch) || event_is_valid_pr_or_pr_update(e)) + }); let revision_roots: HashSet = commit_events .iter() diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 7b25daf..09ec040 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -90,6 +90,19 @@ pub fn patch_supports_commit_ids(event: &Event) -> bool { .any(|t| !t.as_slice().is_empty() && t.as_slice()[0].eq("commit-pgp-sig")) } +pub fn event_is_valid_pr_or_pr_update(event: &Event) -> bool { + [KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains(&event.kind) + && event.tags.iter().any(|t| { + t.as_slice().len().gt(&1) + && t.as_slice()[0].eq("c") + && git2::Oid::from_str(&t.as_slice()[1]).is_ok() + }) + && event + .tags + .iter() + .any(|t| t.as_slice().len().gt(&1) && t.as_slice()[0].eq("clone")) +} + #[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_lines)] pub async fn generate_patch_event( -- cgit v1.2.3 From 9357b62b9824299be6fc85b09f57d93d3902f79a Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 25 Jul 2025 11:48:22 +0100 Subject: refactor: abstract `get_status` for use by `ngit list` --- src/bin/git_remote_nostr/utils.rs | 58 ++++++++------------------------------- src/bin/ngit/sub_commands/list.rs | 5 +++- src/lib/git_events.rs | 56 ++++++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 48 deletions(-) (limited to 'src/bin/ngit/sub_commands/list.rs') diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs index 8433967..2cb85bf 100644 --- a/src/bin/git_remote_nostr/utils.rs +++ b/src/bin/git_remote_nostr/utils.rs @@ -18,9 +18,8 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, git_events::{ - KIND_PULL_REQUEST, event_is_revision_root, - get_pr_tip_event_or_most_recent_patch_with_ancestors, is_event_proposal_root_for_branch, - status_kinds, + event_is_revision_root, get_pr_tip_event_or_most_recent_patch_with_ancestors, get_status, + is_event_proposal_root_for_branch, status_kinds, }, repo_ref::RepoRef, }; @@ -104,7 +103,10 @@ pub async fn get_open_or_draft_proposals( get_proposals_and_revisions_from_cache(git_repo_path, repo_ref.coordinates()) .await? .iter() - .filter(|e| !event_is_revision_root(e)) + .filter(|e| + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e)) .cloned() .collect(); @@ -124,48 +126,9 @@ pub async fn get_open_or_draft_proposals( }; let mut open_or_draft_proposals = HashMap::new(); - let get_status = |proposal: &Event| { - if let Some(e) = statuses - .iter() - .filter(|e| { - status_kinds().contains(&e.kind) - && e.tags.iter().any(|t| { - t.as_slice().len() > 1 && t.as_slice()[1].eq(&proposal.id.to_string()) - }) - && (proposal.pubkey.eq(&e.pubkey) || repo_ref.maintainers.contains(&e.pubkey)) - }) - .collect::>() - .first() - { - e.kind - } else { - Kind::GitStatusOpen - } - }; - - let is_proposal_pr_revision_of_patch = |proposal: &Event, patch: &Event| { - proposal.kind.eq(&KIND_PULL_REQUEST) - && proposal.tags.clone().into_iter().any(|t| { - t.as_slice().len() > 1 - && t.as_slice()[0].eq("e") - && t.as_slice()[1].eq(&patch.id.to_string()) - && [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&get_status(proposal)) - }) - }; - for proposal in &proposals { - let status = get_status(proposal); - if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) - || // or patch has been revised as a PR which is open or draft - ( - status.eq(&Kind::GitStatusClosed) && - proposal.kind.eq(&Kind::GitPatch) && - proposals.iter() - .any(|p| { - is_proposal_pr_revision_of_patch(p, proposal) - }) - ) - { + let status = get_status(proposal, repo_ref, &statuses, &proposals); + if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) { if let Ok(commits_events) = get_all_proposal_patch_pr_pr_update_events_from_cache( git_repo_path, repo_ref, @@ -196,7 +159,10 @@ pub async fn get_all_proposals( get_proposals_and_revisions_from_cache(git_repo_path, repo_ref.coordinates()) .await? .iter() - .filter(|e| !event_is_revision_root(e)) + .filter(|e| + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e)) .cloned() .collect(); diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index 9e35b33..95e17f3 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -72,7 +72,10 @@ pub async fn launch() -> Result<()> { let proposals: Vec = proposals_and_revisions .iter() - .filter(|e| !event_is_revision_root(e)) + .filter(|e| + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e)) .cloned() .collect(); diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 7cd64ab..2e1f215 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -835,7 +835,61 @@ pub fn is_event_proposal_root_for_branch( || cl .get_branch_name_with_pr_prefix_and_shorthand_id() .is_ok_and(|s| s.eq(&branch_name)) - }) && !event_is_revision_root(e)) + }) && ( + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // Note: whilst this the the case elsewhere event_is_revision_root is used, there is more to + // think about here? + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e) + )) +} + +pub fn get_status( + proposal: &Event, + repo_ref: &RepoRef, + all_status_in_repo: &[Event], + all_pr_roots_in_repo: &[Event], +) -> Kind { + let get_direct_status = |proposal: &Event| { + if let Some(e) = all_status_in_repo + .iter() + .filter(|e| { + status_kinds().contains(&e.kind) + && e.tags.iter().any(|t| { + t.as_slice().len() > 1 && t.as_slice()[1].eq(&proposal.id.to_string()) + }) + && (proposal.pubkey.eq(&e.pubkey) || repo_ref.maintainers.contains(&e.pubkey)) + }) + .collect::>() + .first() + { + e.kind + } else { + Kind::GitStatusOpen + } + }; + let is_proposal_pr_revision_of_patch = |proposal: &Event, patch: &Event| { + proposal.kind.eq(&KIND_PULL_REQUEST) + && proposal.tags.clone().into_iter().any(|t| { + t.as_slice().len() > 1 + && t.as_slice()[0].eq("e") + && t.as_slice()[1].eq(&patch.id.to_string()) + }) + }; + + let direct_status = get_direct_status(proposal); + if direct_status.eq(&Kind::GitStatusClosed) && proposal.kind.eq(&Kind::GitPatch) { + if let Some(pr_revision) = all_pr_roots_in_repo + .iter() + .find(|p| is_proposal_pr_revision_of_patch(p, proposal)) + { + get_direct_status(pr_revision) + } else { + direct_status + } + } else { + direct_status + } } #[cfg(test)] -- cgit v1.2.3 From e0f6d034ce5b85924d31238e0def42edd241d2f5 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 25 Jul 2025 12:23:53 +0100 Subject: feat(list): fix status for pr as patch revision using the recently abstracted `get_status` function --- src/bin/ngit/sub_commands/list.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) (limited to 'src/bin/ngit/sub_commands/list.rs') diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index 95e17f3..1fe5b49 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -8,7 +8,7 @@ use ngit::{ }, git_events::{ KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, get_commit_id_from_patch, - get_pr_tip_event_or_most_recent_patch_with_ancestors, status_kinds, tag_value, + get_pr_tip_event_or_most_recent_patch_with_ancestors, get_status, status_kinds, tag_value, }, }; use nostr_sdk::Kind; @@ -80,21 +80,7 @@ pub async fn launch() -> Result<()> { .collect(); for proposal in &proposals { - let status = if let Some(e) = statuses - .iter() - .filter(|e| { - status_kinds().contains(&e.kind) - && e.tags.iter().any(|t| { - t.as_slice().len() > 1 && t.as_slice()[1].eq(&proposal.id.to_string()) - }) - }) - .collect::>() - .first() - { - e.kind - } else { - Kind::GitStatusOpen - }; + let status = get_status(proposal, &repo_ref, &statuses, &proposals); if status.eq(&Kind::GitStatusOpen) { open_proposals.push(proposal); } else if status.eq(&Kind::GitStatusClosed) { -- cgit v1.2.3 From 27cdea120e195b68d998764519ff3a472641c79b Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 25 Jul 2025 12:42:37 +0100 Subject: fix: update help text for patches without parent adjust the help text to reflect availablity of PR event for when a patch is selected that doesnt list a parent commit id --- src/bin/ngit/sub_commands/list.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/bin/ngit/sub_commands/list.rs') diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index 1fe5b49..c3c8c71 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -244,16 +244,16 @@ pub async fn launch() -> Result<()> { PromptChoiceParms::default() .with_default(0) .with_choices(vec![ - "learn why 'patch only' proposals can't be checked out".to_string(), + "learn why this proposals can't be checked out".to_string(), format!("apply to current branch with `git am`"), format!("download to ./patches"), "back".to_string(), ]), )? { 0 => { - println!("Some proposals are posted as 'patch only'\n"); + println!("Some proposals are posted as patch without listing a parent commit\n"); println!( - "they are not anchored against a particular state of the code base like a standard proposal or a GitHub Pull Request can be\n" + "they are not anchored against a particular state of the code base like a standard patch or a pull request can be\n" ); println!( "they are designed to reviewed by studying the diff (in a tool like gitworkshop.dev) and if acceptable by a maintainer, applied to the latest version of master with any conflicts resolved as the do so\n" @@ -262,7 +262,7 @@ pub async fn launch() -> Result<()> { "this has proven to be a smoother workflow for large scale projects with a high frequency of changes, even when patches are exchanged via email\n" ); println!( - "by default ngit posts proposals that support both the branch and patch model so either workflow can be used" + "by default ngit posts proposals with a parent commit so either workflow can be used" ); Interactor::default().choice( PromptChoiceParms::default() -- cgit v1.2.3 From 0cad465dd3f78bd6c680067d12d396d4782829bf Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 25 Jul 2025 15:52:19 +0100 Subject: fix(list): improve pr unsupport text and show a more helpful message when proposal can be checked out using the remote --- src/bin/ngit/sub_commands/list.rs | 35 ++++++++++++++++++++++++++++------- src/lib/git/mod.rs | 20 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) (limited to 'src/bin/ngit/sub_commands/list.rs') diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index c3c8c71..0083c91 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -206,13 +206,32 @@ pub async fn launch() -> Result<()> { { match Interactor::default().choice( PromptChoiceParms::default() - .with_prompt("this is new PR event kind which ngit doesnt yet support") + .with_prompt( + "this is new PR event kind which isn't supported in `ngit list` yet", + ) .with_default(0) - .with_choices(vec![ - // TODO enable checkout by fetching oids, creating / updating branch and - // checking out - "back to proposals".to_string(), - ]), + .with_choices( + if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&selected_status) + && git_repo + .get_first_nostr_remote_when_in_ngit_binary() + .await + .is_ok_and(|r| r.is_some()) + { + vec![ + format!( + "I'll manually checkout the proposal at remote branch '{}'", + cover_letter + .get_branch_name_with_pr_prefix_and_shorthand_id() + .unwrap() + ), + // TODO fetch oids and follow similar logic for dealing with + // conflcts as with patches below + "back to proposals".to_string(), + ] + } else { + vec!["back to proposals".to_string()] + }, + ), )? { 0 => continue, _ => { @@ -251,7 +270,9 @@ pub async fn launch() -> Result<()> { ]), )? { 0 => { - println!("Some proposals are posted as patch without listing a parent commit\n"); + println!( + "Some proposals are posted as patch without listing a parent commit\n" + ); println!( "they are not anchored against a particular state of the code base like a standard patch or a pull request can be\n" ); diff --git a/src/lib/git/mod.rs b/src/lib/git/mod.rs index d4bf2f5..b275b49 100644 --- a/src/lib/git/mod.rs +++ b/src/lib/git/mod.rs @@ -10,6 +10,7 @@ use nostr_sdk::{ Tags, hashes::{Hash, sha1::Hash as Sha1Hash}, }; +use nostr_url::NostrUrlDecoded; use crate::git_events::{get_commit_id_from_patch, tag_value}; pub mod identify_ahead_behind; @@ -92,6 +93,10 @@ pub trait RepoActions { fn get_git_config_item(&self, item: &str, global: Option) -> Result>; fn save_git_config_item(&self, item: &str, value: &str, global: bool) -> Result<()>; fn remove_git_config_item(&self, item: &str, global: bool) -> Result; + #[allow(async_fn_in_trait)] + async fn get_first_nostr_remote_when_in_ngit_binary( + &self, + ) -> Result>; } impl RepoActions for Repo { @@ -796,6 +801,21 @@ impl RepoActions for Repo { Ok(true) } } + + async fn get_first_nostr_remote_when_in_ngit_binary( + &self, + ) -> Result> { + for remote_name in self.git_repo.remotes()?.iter().flatten() { + if let Some(remote_url) = self.git_repo.find_remote(remote_name)?.url() { + if let Ok(nostr_url_decoded) = + NostrUrlDecoded::parse_and_resolve(remote_url, &Some(self)).await + { + return Ok(Some((remote_name.to_string(), nostr_url_decoded))); + } + } + } + Ok(None) + } } fn oid_to_u8_20_bytes(oid: &Oid) -> [u8; 20] { -- cgit v1.2.3