From 3112576195aef212622d27ad9164336796c1953e Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 13 Feb 2024 06:27:34 +0000 Subject: feat(prs-create)!: pr to nip34-like cover letter up the pr event type to a nip34-like cover letter format this sets the building blocks in place to enable simplier clients to use the 'cover letter' feature in `git format-patch` to create the experience as a pr event --- src/git.rs | 3 +- src/sub_commands/prs/create.rs | 240 ++++++++++++++++++++++++++++++++++++----- src/sub_commands/prs/list.rs | 28 +++-- src/sub_commands/pull.rs | 10 +- src/sub_commands/push.rs | 13 ++- 5 files changed, 250 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/src/git.rs b/src/git.rs index 067ce24..24afe76 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1260,7 +1260,7 @@ mod tests { &git_repo, &git_repo.get_root_commit()?, &oid_to_sha1(&original_oid), - nostr::EventId::all_zeros(), + Some(nostr::EventId::all_zeros()), &TEST_KEY_1_KEYS, &RepoRef::try_from(generate_repo_ref_event()).unwrap(), None, @@ -1418,6 +1418,7 @@ mod tests { let git_repo = Repo::from_path(&original_repo.dir)?; let mut events = generate_pr_and_patch_events( + // Some(("test".to_string(), "test".to_string())), "title", "description", &git_repo, diff --git a/src/sub_commands/prs/create.rs b/src/sub_commands/prs/create.rs index 8506303..83a3942 100644 --- a/src/sub_commands/prs/create.rs +++ b/src/sub_commands/prs/create.rs @@ -21,10 +21,10 @@ use crate::{ #[derive(Debug, clap::Args)] pub struct SubCommandArgs { #[clap(short, long)] - /// title of pull request (defaults to first line of first commit) + /// optional cover letter title title: Option, #[clap(short, long)] - /// optional description + /// optional cover letter description description: Option, #[clap(long)] /// branch to get changes from (defaults to head) @@ -34,6 +34,7 @@ pub struct SubCommandArgs { to_branch: Option, } +#[allow(clippy::too_many_lines)] pub async fn launch( cli_args: &Cli, _pr_args: &super::SubCommandArgs, @@ -91,6 +92,21 @@ pub async fn launch( .input(PromptInputParms::default().with_prompt("description (Optional)"))?, }; + // let cover_letter_title_description = if let Some(title) = title { + // Some(( + // title, + // if let Some(t) = &args.description { + // t.clone() + // } else { + // Interactor::default() + // .input(PromptInputParms::default().with_prompt("cover letter + // description"))? .clone() + // }, + // )) + // } else { + // None + // }; + #[cfg(not(test))] let mut client = Client::default(); #[cfg(test)] @@ -111,8 +127,15 @@ pub async fn launch( ) .await?; - let events = - generate_pr_and_patch_events(&title, &description, &git_repo, &ahead, &keys, &repo_ref)?; + let events = generate_pr_and_patch_events( + // cover_letter_title_description, + &title, + &description, + &git_repo, + &ahead, + &keys, + &repo_ref, + )?; println!( "posting 1 pull request with {} commits...", @@ -308,6 +331,7 @@ pub static PATCH_KIND: u64 = 1617; pub fn generate_pr_and_patch_events( title: &str, description: &str, + // cover_letter_title_description: Option<(String, String)>, git_repo: &Repo, commits: &Vec, keys: &nostr::Keys, @@ -317,40 +341,57 @@ pub fn generate_pr_and_patch_events( .get_root_commit() .context("failed to get root commit of the repository")?; - let mut pr_tags = vec![ - Tag::Reference(format!("r-{root_commit}")), - Tag::Name(title.to_string()), - Tag::Description(description.to_string()), - ]; - - if let Ok(branch_name) = git_repo.get_checked_out_branch_name() { - pr_tags.push(Tag::Generic( - TagKind::Custom("branch-name".to_string()), - vec![branch_name], - )); - } + let mut events = vec![]; - let pr_event = EventBuilder::new( + // if let Some((title, description)) = cover_letter_title_description { + if !title.is_empty() { + events.push(EventBuilder::new( nostr::event::Kind::Custom(PR_KIND), - format!("{title}\r\n\r\n{description}"), - pr_tags, - // TODO: add Repo event as root - // TODO: people tag maintainers - // TODO: add relay tags + format!( + "From {} Mon Sep 17 00:00:00 2001\nSubject: [PATCH 0/{}] {title}\n\n{description}", + commits.last().unwrap(), + commits.len() + ), + [ + vec![ + // TODO: why not tag all maintainer identifiers? + Tag::A { + kind: nostr::Kind::Custom(REPO_REF_KIND), + public_key: *repo_ref.maintainers.first() + .context("repo reference should always have at least one maintainer - the issuer of the repo event") + ?, + identifier: repo_ref.identifier.to_string(), + relay_url: repo_ref.relays.first().map(nostr::UncheckedUrl::from).clone(), + }, + Tag::Reference(format!("{root_commit}")), + Tag::Hashtag("cover-letter".to_string()), + Tag::Hashtag("root".to_string()), + ], + if let Ok(branch_name) = git_repo.get_checked_out_branch_name() { + vec![Tag::Generic( + TagKind::Custom("branch-name".to_string()), + vec![branch_name], + )] + } else { + vec![] + }, + repo_ref.maintainers + .iter() + .map(|pk| Tag::public_key(*pk)) + .collect(), + ].concat(), ) .to_event(keys) - .context("failed to create pr event")?; - - let pr_event_id = pr_event.id; + .context("failed to create cover-letter event")?); + } - let mut events = vec![pr_event]; for (i, commit) in commits.iter().enumerate() { events.push( generate_patch_event( git_repo, &root_commit, commit, - pr_event_id, + events.first().map(|event| event.id), keys, repo_ref, events.last().map(nostr::Event::id), @@ -366,12 +407,45 @@ pub fn generate_pr_and_patch_events( Ok(events) } +pub struct CoverLetter { + pub title: String, + pub description: String, + pub branch_name: Option, +} + +fn event_is_cover_letter(event: &nostr::Event) -> bool { + event.kind.as_u64().eq(&PR_KIND) && event.iter_tags().any(|t| t.as_vec()[1].eq("cover-letter")) +} +pub fn event_to_cover_letter(event: &nostr::Event) -> Result { + if !event_is_cover_letter(event) { + bail!("event is not a cover letter") + } + let title_index = event + .content + .find("] ") + .context("event is not formatted as a cover letter patch")? + + 2; + let description_index = event.content[title_index..] + .find('\n') + .unwrap_or(event.content.len() - 1 - title_index) + + title_index; + + Ok(CoverLetter { + title: event.content[title_index..description_index].to_string(), + description: event.content[description_index..].trim().to_string(), + branch_name: event + .iter_tags() + .find(|t| t.as_vec()[0].eq("branch-name")) + .map(|tag| tag.as_vec()[1].clone()), + }) +} + #[allow(clippy::too_many_arguments)] pub fn generate_patch_event( git_repo: &Repo, root_commit: &Sha1Hash, commit: &Sha1Hash, - thread_event_id: nostr::EventId, + thread_event_id: Option, keys: &nostr::Keys, repo_ref: &RepoRef, parent_patch_event_id: Option, @@ -404,10 +478,13 @@ pub fn generate_patch_event( // the commit id is correct Tag::Reference(commit.to_string()), - Tag::Event { + if let Some(thread_event_id) = thread_event_id { Tag::Event { event_id: thread_event_id, relay_url: relay_hint.clone(), marker: Some(Marker::Root), + } } + else { + Tag::Hashtag("root".to_string()) }, ], if let Some(id) = parent_patch_event_id { @@ -686,4 +763,109 @@ mod tests { Ok(()) } } + + mod event_to_cover_letter { + use super::*; + + fn generate_cover_letter(title: &str, description: &str) -> Result { + Ok(nostr::event::EventBuilder::new( + nostr::event::Kind::Custom(PR_KIND), + format!("From ea897e987ea9a7a98e7a987e97987ea98e7a3334 Mon Sep 17 00:00:00 2001\nSubject: [PATCH 0/2] {title}\n\n{description}"), + [ + Tag::Hashtag("cover-letter".to_string()), + Tag::Hashtag("root".to_string()), + ], + ) + .to_event(&nostr::Keys::generate())?) + } + + #[test] + fn basic_title() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter("the title", "description here")?)? + .title, + "the title", + ); + Ok(()) + } + + #[test] + fn basic_description() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter("the title", "description here")?)? + .description, + "description here", + ); + Ok(()) + } + + #[test] + fn description_trimmed() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter( + "the title", + " \n \ndescription here\n\n " + )?)? + .description, + "description here", + ); + Ok(()) + } + + #[test] + fn multi_line_description() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter( + "the title", + "description here\n\nmore here\nmore" + )?)? + .description, + "description here\n\nmore here\nmore", + ); + Ok(()) + } + + #[test] + fn new_lines_in_title_forms_part_of_description() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter( + "the title\nwith new line", + "description here\n\nmore here\nmore" + )?)? + .title, + "the title", + ); + assert_eq!( + event_to_cover_letter(&generate_cover_letter( + "the title\nwith new line", + "description here\n\nmore here\nmore" + )?)? + .description, + "with new line\n\ndescription here\n\nmore here\nmore", + ); + Ok(()) + } + + mod blank_description { + use super::*; + + #[test] + fn title_correct() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter("the title", "")?)?.title, + "the title", + ); + Ok(()) + } + + #[test] + fn description_is_empty_string() -> Result<()> { + assert_eq!( + event_to_cover_letter(&generate_cover_letter("the title", "")?)?.description, + "", + ); + Ok(()) + } + } + } } diff --git a/src/sub_commands/prs/list.rs b/src/sub_commands/prs/list.rs index 88b325b..bc85eed 100644 --- a/src/sub_commands/prs/list.rs +++ b/src/sub_commands/prs/list.rs @@ -8,8 +8,8 @@ use crate::{ cli_interactor::{Interactor, InteractorPrompt, PromptChoiceParms, PromptConfirmParms}, client::Connect, git::{Repo, RepoActions}, - repo_ref, - sub_commands::prs::create::{PATCH_KIND, PR_KIND}, + repo_ref::{self}, + sub_commands::prs::create::{event_to_cover_letter, PATCH_KIND, PR_KIND}, Cli, }; @@ -20,6 +20,7 @@ pub struct SubCommandArgs { open_only: bool, } +#[allow(clippy::too_many_lines)] pub async fn launch( _cli_args: &Cli, _pr_args: &super::SubCommandArgs, @@ -56,7 +57,7 @@ pub async fn launch( vec![ nostr::Filter::default() .kind(nostr::Kind::Custom(PR_KIND)) - .reference(format!("r-{root_commit}")), + .reference(format!("{root_commit}")), ], ) .await? @@ -65,7 +66,7 @@ pub async fn launch( e.kind.as_u64() == PR_KIND && e.tags .iter() - .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("r-{root_commit}"))) + .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("{root_commit}"))) }) .map(std::borrow::ToOwned::to_owned) .collect(); @@ -92,8 +93,8 @@ pub async fn launch( pr_events .iter() .map(|e| { - if let Ok(name) = tag_value(e, "name") { - name + if let Ok(cl) = event_to_cover_letter(e) { + cl.title } else { e.id.to_string() } @@ -131,7 +132,19 @@ pub async fn launch( let most_recent_pr_patch_chain = get_most_recent_patch_with_ancestors(commits_events) .context("cannot get most recent patch for PR")?; - let branch_name = tag_value(&pr_events[selected_index], "branch-name")?; + let branch_name: String = if let Ok(cl) = event_to_cover_letter(&pr_events[selected_index]) { + if let Some(name) = cl.branch_name { + name + } else { + cl.title + .replace(' ', "-") + .chars() + .filter(|c| c.is_ascii_alphanumeric() || c.eq(&'/')) + .collect() + } + } else { + bail!("Placeholder not a cover letter") + }; let applied = git_repo .apply_patch_chain(&branch_name, most_recent_pr_patch_chain) @@ -145,7 +158,6 @@ pub async fn launch( applied.len(), ); } - Ok(()) } diff --git a/src/sub_commands/pull.rs b/src/sub_commands/pull.rs index c426510..f3ae81f 100644 --- a/src/sub_commands/pull.rs +++ b/src/sub_commands/pull.rs @@ -8,6 +8,7 @@ use crate::{ client::Connect, git::{Repo, RepoActions}, repo_ref, + repo_ref::REPO_REF_KIND, sub_commands::prs::{ create::{PATCH_KIND, PR_KIND}, list::{get_most_recent_patch_with_ancestors, tag_value}, @@ -53,7 +54,12 @@ pub async fn launch() -> Result<()> { vec![ nostr::Filter::default() .kind(nostr::Kind::Custom(PR_KIND)) - .reference(format!("r-{root_commit}")), + .identifiers( + repo_ref + .maintainers + .iter() + .map(|m| format!("{REPO_REF_KIND}:{m}:{}", repo_ref.identifier)), + ), ], ) .await? @@ -62,7 +68,7 @@ pub async fn launch() -> Result<()> { e.kind.as_u64() == PR_KIND && e.tags .iter() - .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("r-{root_commit}"))) + .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("{root_commit}"))) && tag_value(e, "branch-name") .unwrap_or_default() .eq(&branch_name) diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs index 037d512..61d5d46 100644 --- a/src/sub_commands/push.rs +++ b/src/sub_commands/push.rs @@ -9,7 +9,7 @@ use crate::{ client::Connect, git::{str_to_sha1, Repo, RepoActions}, login, - repo_ref::{self, RepoRef}, + repo_ref::{self, RepoRef, REPO_REF_KIND}, sub_commands::prs::{ create::{generate_patch_event, send_events, PATCH_KIND, PR_KIND}, list::{get_most_recent_patch_with_ancestors, tag_value}, @@ -106,7 +106,7 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { &git_repo, &root_commit, commit, - pr_event.id, + Some(pr_event.id), &keys, &repo_ref, patch_events.last().map(nostr::Event::id), @@ -146,7 +146,12 @@ async fn fetch_pr_and_most_recent_patch_chain( vec![ nostr::Filter::default() .kind(nostr::Kind::Custom(PR_KIND)) - .reference(format!("r-{root_commit}")), + .identifiers( + repo_ref + .maintainers + .iter() + .map(|m| format!("{REPO_REF_KIND}:{m}:{}", repo_ref.identifier)), + ), ], ) .await? @@ -155,7 +160,7 @@ async fn fetch_pr_and_most_recent_patch_chain( e.kind.as_u64() == PR_KIND && e.tags .iter() - .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("r-{root_commit}"))) + .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("{root_commit}"))) && tag_value(e, "branch-name") .unwrap_or_default() .eq(branch_name) -- cgit v1.2.3