5 files changed, 71 insertions, 57 deletions

diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs
index 07c6f59..f361272 100644
--- a/src/bin/git_remote_nostr/list.rs
+++ b/src/bin/git_remote_nostr/list.rs
@@ -149,10 +149,10 @@ async fn get_open_proposals_state(
     let current_user = get_curent_user(git_repo)?;
     for (_, (proposal, patches)) in open_proposals {
         if let Ok(cl) = event_to_cover_letter(&proposal) {
-            if let Ok(mut branch_name) = cl.get_branch_name() {
+            if let Ok(mut branch_name) = cl.get_branch_name_with_pr_prefix_and_shorthand_id() {
                 branch_name = if let Some(public_key) = current_user {
                     if proposal.pubkey.eq(&public_key) {
-                        format!("pr/{}", cl.branch_name)
+                        format!("pr/{}", cl.branch_name_without_id_or_prefix)
                     } else {
                         branch_name
                     }
diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs
index 05c8fc2..922808c 100644
--- a/src/bin/git_remote_nostr/push.rs
+++ b/src/bin/git_remote_nostr/push.rs
@@ -1184,7 +1184,8 @@ async fn create_merge_events(
             term.write_line(
                 format!(
                     "applied commits from proposal: create nostr proposal status event for {}",
-                    event_to_cover_letter(&proposal)?.get_branch_name()?,
+                    event_to_cover_letter(&proposal)?
+                        .get_branch_name_with_pr_prefix_and_shorthand_id()?,
                 )
                 .as_str(),
             )?;
@@ -1192,7 +1193,8 @@ async fn create_merge_events(
             term.write_line(
                 format!(
                     "fast-forward merge: create nostr proposal status event for {}",
-                    event_to_cover_letter(&proposal)?.get_branch_name()?,
+                    event_to_cover_letter(&proposal)?
+                        .get_branch_name_with_pr_prefix_and_shorthand_id()?,
                 )
                 .as_str(),
             )?;
diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs
index 15ebb10..316fedb 100644
--- a/src/bin/git_remote_nostr/utils.rs
+++ b/src/bin/git_remote_nostr/utils.rs
@@ -184,10 +184,10 @@ pub async fn get_all_proposals(
 
 pub fn find_proposal_and_patches_by_branch_name<'a>(
     refstr: &'a str,
-    open_proposals: &'a HashMap<EventId, (Event, Vec<Event>)>,
+    proposals: &'a HashMap<EventId, (Event, Vec<Event>)>,
     current_user: Option<&PublicKey>,
 ) -> Option<(&'a EventId, &'a (Event, Vec<Event>))> {
-    open_proposals.iter().find(|(_, (proposal, _))| {
+    proposals.iter().find(|(_, (proposal, _))| {
         is_event_proposal_root_for_branch(proposal, refstr, current_user).unwrap_or(false)
     })
 }
diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs
index 9b84c1b..e8d2e97 100644
--- a/src/bin/ngit/sub_commands/list.rs
+++ b/src/bin/ngit/sub_commands/list.rs
@@ -261,11 +261,15 @@ pub async fn launch() -> Result<()> {
             .get_local_branch_names()
             .context("gitlib2 will not show a list of local branch names")?
             .iter()
-            .any(|n| n.eq(&cover_letter.get_branch_name().unwrap()));
+            .any(|n| {
+                n.eq(&cover_letter
+                    .get_branch_name_with_pr_prefix_and_shorthand_id()
+                    .unwrap())
+            });
 
         let checked_out_proposal_branch = git_repo
             .get_checked_out_branch_name()?
-            .eq(&cover_letter.get_branch_name()?);
+            .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(
@@ -327,14 +331,14 @@ pub async fn launch() -> Result<()> {
                     check_clean(&git_repo)?;
                     let _ = git_repo
                         .apply_patch_chain(
-                            &cover_letter.get_branch_name()?,
+                            &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
                             most_recent_proposal_patch_chain,
                         )
                         .context("failed to apply patch chain")?;
 
                     println!(
                         "checked out proposal as '{}' branch",
-                        cover_letter.get_branch_name()?
+                        cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?
                     );
                     Ok(())
                 }
@@ -347,7 +351,8 @@ pub async fn launch() -> Result<()> {
             };
         }
 
-        let local_branch_tip = git_repo.get_tip_of_branch(&cover_letter.get_branch_name()?)?;
+        let local_branch_tip = git_repo
+            .get_tip_of_branch(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?)?;
 
         // up-to-date
         if proposal_tip.eq(&local_branch_tip) {
@@ -382,10 +387,12 @@ pub async fn launch() -> Result<()> {
             )? {
                 0 => {
                     check_clean(&git_repo)?;
-                    git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                    git_repo.checkout(
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
+                    )?;
                     println!(
                         "checked out proposal as '{}' branch",
-                        cover_letter.get_branch_name()?
+                        cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?
                     );
                     Ok(())
                 }
@@ -419,10 +426,12 @@ pub async fn launch() -> Result<()> {
             )? {
                 0 => {
                     check_clean(&git_repo)?;
-                    git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                    git_repo.checkout(
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
+                    )?;
                     let _ = git_repo
                         .apply_patch_chain(
-                            &cover_letter.get_branch_name()?,
+                            &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
                             most_recent_proposal_patch_chain,
                         )
                         .context("failed to apply patch chain")?;
@@ -472,14 +481,16 @@ pub async fn launch() -> Result<()> {
                 0 => {
                     check_clean(&git_repo)?;
                     git_repo.create_branch_at_commit(
-                        &cover_letter.get_branch_name()?,
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
                         &proposal_base_commit.to_string(),
                     )?;
-                    git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                    git_repo.checkout(
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
+                    )?;
                     let chain_length = most_recent_proposal_patch_chain.len();
                     let _ = git_repo
                         .apply_patch_chain(
-                            &cover_letter.get_branch_name()?,
+                            &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
                             most_recent_proposal_patch_chain,
                         )
                         .context("failed to apply patch chain")?;
@@ -494,7 +505,9 @@ pub async fn launch() -> Result<()> {
                 }
                 1 => {
                     check_clean(&git_repo)?;
-                    git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                    git_repo.checkout(
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
+                    )?;
                     println!(
                         "checked out old proposal in existing branch ({} ahead {} behind '{main_branch_name}')",
                         local_ahead_of_main.len(),
@@ -537,7 +550,9 @@ pub async fn launch() -> Result<()> {
                     ]),
             )? {
                 0 => {
-                    git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                    git_repo.checkout(
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
+                    )?;
                     println!(
                         "checked out proposal branch with {} unpublished commits ({} ahead {} behind '{main_branch_name}')",
                         local_ahead_of_proposal.len(),
@@ -604,7 +619,8 @@ pub async fn launch() -> Result<()> {
         )? {
             0 => {
                 check_clean(&git_repo)?;
-                git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                git_repo
+                    .checkout(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?)?;
                 println!(
                     "checked out old proposal in existing branch ({} ahead {} behind '{main_branch_name}')",
                     local_ahead_of_main.len(),
@@ -615,18 +631,19 @@ pub async fn launch() -> Result<()> {
             1 => {
                 check_clean(&git_repo)?;
                 git_repo.create_branch_at_commit(
-                    &cover_letter.get_branch_name()?,
+                    &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 _ = git_repo
                     .apply_patch_chain(
-                        &cover_letter.get_branch_name()?,
+                        &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?,
                         most_recent_proposal_patch_chain,
                     )
                     .context("failed to apply patch chain")?;
 
-                git_repo.checkout(&cover_letter.get_branch_name()?)?;
+                git_repo
+                    .checkout(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?)?;
                 println!(
                     "checked out latest version of proposal ({} ahead {} behind '{main_branch_name}'), replacing unpublished version ({} ahead {} behind '{main_branch_name}')",
                     chain_length,
diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs
index af469d3..559155a 100644
--- a/src/lib/git_events.rs
+++ b/src/lib/git_events.rs
@@ -453,15 +453,15 @@ pub async fn generate_cover_letter_and_patch_events(
 pub struct CoverLetter {
     pub title: String,
     pub description: String,
-    pub branch_name: String,
+    pub branch_name_without_id_or_prefix: String,
     pub event_id: Option<nostr::EventId>,
 }
 
 impl CoverLetter {
-    pub fn get_branch_name(&self) -> Result<String> {
+    pub fn get_branch_name_with_pr_prefix_and_shorthand_id(&self) -> Result<String> {
         Ok(format!(
             "pr/{}({})",
-            self.branch_name,
+            self.branch_name_without_id_or_prefix,
             &self
                 .event_id
                 .context("proposal root event_id must be know to get it's branch name")?
@@ -520,39 +520,33 @@ pub fn event_to_cover_letter(event: &nostr::Event) -> Result<CoverLetter> {
     Ok(CoverLetter {
         title: title.clone(),
         description,
-        // TODO should this be prefixed by format!("{}-"e.id.to_string()[..5]?)
-        branch_name: if let Ok(name) = match tag_value(event, "branch-name") {
-            Ok(name) => {
-                if !name.eq("main") && !name.eq("master") {
-                    Ok(name.chars().take(60).collect::<String>())
-                } else {
-                    Err(())
-                }
+        branch_name_without_id_or_prefix: if let Ok(name) = tag_value(event, "branch-name") {
+            if !name.eq("main") && !name.eq("master") {
+                safe_branch_name_for_pr(&name)
+            } else {
+                safe_branch_name_for_pr(&title)
             }
-            _ => Err(()),
-        } {
-            name
         } else {
-            let s = title
-                .replace(' ', "-")
-                .chars()
-                .map(|c| {
-                    if c.is_ascii_alphanumeric() || c.eq(&'/') {
-                        c
-                    } else {
-                        '-'
-                    }
-                })
-                .collect();
-            s
-        }
-        .chars()
-        .take(60)
-        .collect(),
+            safe_branch_name_for_pr(&title)
+        },
         event_id: Some(event.id),
     })
 }
 
+fn safe_branch_name_for_pr(s: &str) -> String {
+    s.replace(' ', "-")
+        .chars()
+        .map(|c| {
+            if c.is_ascii_alphanumeric() || c.eq(&'/') {
+                c
+            } else {
+                '-'
+            }
+        })
+        .take(60)
+        .collect()
+}
+
 pub fn get_most_recent_patch_with_ancestors(
     mut patches: Vec<nostr::Event>,
 ) -> Result<Vec<nostr::Event>> {
@@ -622,9 +616,10 @@ pub fn is_event_proposal_root_for_branch(
     let branch_name = branch_name_or_refstr.replace("refs/heads/", "");
     Ok(event_to_cover_letter(e).is_ok_and(|cl| {
         (logged_in_user.is_some_and(|public_key| e.pubkey.eq(public_key))
-            && (branch_name.eq(&format!("pr/{}", cl.branch_name))
-                || cl.branch_name.eq(&branch_name)))
-            || cl.get_branch_name().is_ok_and(|s| s.eq(&branch_name))
+            && branch_name.eq(&format!("pr/{}", cl.branch_name_without_id_or_prefix)))
+            || cl
+                .get_branch_name_with_pr_prefix_and_shorthand_id()
+                .is_ok_and(|s| s.eq(&branch_name))
     }) && !event_is_revision_root(e))
 }