feat(send): select commits from a list

when since_or_range isn't specified

adds resilience as assuming master..HEAD can cause some issues
eg when master is not up-to-date with origin/master

9 files changed, 438 insertions, 391 deletions

diff --git a/src/cli_interactor.rs b/src/cli_interactor.rs
index dc15c87..4cf6357 100644
--- a/src/cli_interactor.rs
+++ b/src/cli_interactor.rs
@@ -14,6 +14,7 @@ pub trait InteractorPrompt {
     fn password(&self, parms: PromptPasswordParms) -> Result<String>;
     fn confirm(&self, params: PromptConfirmParms) -> Result<bool>;
     fn choice(&self, params: PromptChoiceParms) -> Result<usize>;
+    fn multi_choice(&self, params: PromptMultiChoiceParms) -> Result<Vec<usize>>;
 }
 impl InteractorPrompt for Interactor {
     fn input(&self, parms: PromptInputParms) -> Result<String> {
@@ -53,6 +54,18 @@ impl InteractorPrompt for Interactor {
         }
         choice.interact().context("failed to get choice")
     }
+    fn multi_choice(&self, parms: PromptMultiChoiceParms) -> Result<Vec<usize>> {
+        // the colorful theme is not very clear so falling back to default
+        let mut choice = dialoguer::MultiSelect::default();
+        choice
+            .with_prompt(parms.prompt)
+            .report(parms.report)
+            .items(&parms.choices);
+        if let Some(defaults) = parms.defaults {
+            choice.defaults(&defaults);
+        }
+        choice.interact().context("failed to get choice")
+    }
 }
 
 #[derive(Default)]
@@ -140,3 +153,34 @@ impl PromptChoiceParms {
         self
     }
 }
+
+#[derive(Default)]
+pub struct PromptMultiChoiceParms {
+    pub prompt: String,
+    pub choices: Vec<String>,
+    pub defaults: Option<Vec<bool>>,
+    pub report: bool,
+}
+
+impl PromptMultiChoiceParms {
+    pub fn with_prompt<S: Into<String>>(mut self, prompt: S) -> Self {
+        self.prompt = prompt.into();
+        self.report = true;
+        self
+    }
+
+    pub fn dont_report(mut self) -> Self {
+        self.report = false;
+        self
+    }
+
+    pub fn with_choices(mut self, choices: Vec<String>) -> Self {
+        self.choices = choices;
+        self
+    }
+
+    pub fn with_defaults(mut self, defaults: Vec<bool>) -> Self {
+        self.defaults = Some(defaults);
+        self
+    }
+}
diff --git a/src/git.rs b/src/git.rs
index ef14ab1..42297be 100644
--- a/src/git.rs
+++ b/src/git.rs
@@ -41,6 +41,7 @@ pub trait RepoActions {
     fn get_head_commit(&self) -> Result<Sha1Hash>;
     fn get_commit_parent(&self, commit: &Sha1Hash) -> Result<Sha1Hash>;
     fn get_commit_message(&self, commit: &Sha1Hash) -> Result<String>;
+    fn get_commit_message_summary(&self, commit: &Sha1Hash) -> Result<String>;
     /// returns vector ["name", "email", "unixtime", "offset"]
     /// eg ["joe bloggs", "joe@pm.me", "12176","-300"]
     fn get_commit_author(&self, commit: &Sha1Hash) -> Result<Vec<String>>;
@@ -52,6 +53,7 @@ pub trait RepoActions {
         base_commit: &Sha1Hash,
         latest_commit: &Sha1Hash,
     ) -> Result<(Vec<Sha1Hash>, Vec<Sha1Hash>)>;
+    fn get_refs(&self, commit: &Sha1Hash) -> Result<Vec<String>>;
     // including (un)staged changes and (un)tracked files
     fn has_outstanding_changes(&self) -> Result<bool>;
     fn make_patch_from_commit(
@@ -199,6 +201,22 @@ impl RepoActions for Repo {
             .to_string())
     }
 
+    fn get_commit_message_summary(&self, commit: &Sha1Hash) -> Result<String> {
+        Ok(self
+            .git_repo
+            .find_commit(sha1_to_oid(commit)?)
+            .context(format!("could not find commit {commit}"))?
+            .message_raw()
+            .context("commit message has unusual characters in (not valid utf-8)")?
+            .split('\r')
+            .collect::<Vec<&str>>()[0]
+            .split('\n')
+            .collect::<Vec<&str>>()[0]
+            .to_string()
+            .trim()
+            .to_string())
+    }
+
     fn get_commit_author(&self, commit: &Sha1Hash) -> Result<Vec<String>> {
         let commit = self
             .git_repo
@@ -217,6 +235,25 @@ impl RepoActions for Repo {
         Ok(git_sig_to_tag_vec(&sig))
     }
 
+    fn get_refs(&self, commit: &Sha1Hash) -> Result<Vec<String>> {
+        Ok(self
+            .git_repo
+            .references()?
+            .filter(|r| {
+                if let Ok(r) = r {
+                    if let Ok(ref_tip) = r.peel_to_commit() {
+                        ref_tip.id().to_string().eq(&commit.to_string())
+                    } else {
+                        false
+                    }
+                } else {
+                    false
+                }
+            })
+            .map(|r| r.unwrap().shorthand().unwrap().to_string())
+            .collect::<Vec<String>>())
+    }
+
     fn make_patch_from_commit(
         &self,
         commit: &Sha1Hash,
@@ -744,6 +781,43 @@ mod tests {
         }
     }
 
+    mod get_commit_message_summary {
+        use super::*;
+        fn run(message: &str, summary: &str) -> Result<()> {
+            let test_repo = GitTestRepo::default();
+            test_repo.populate()?;
+            std::fs::write(test_repo.dir.join("t100.md"), "some content")?;
+            let oid = test_repo.stage_and_commit(message)?;
+
+            let git_repo = Repo::from_path(&test_repo.dir)?;
+
+            assert_eq!(
+                summary,
+                git_repo.get_commit_message_summary(&oid_to_sha1(&oid))?,
+            );
+            Ok(())
+        }
+        #[test]
+        fn one_liner() -> Result<()> {
+            run("add t100.md", "add t100.md")
+        }
+
+        #[test]
+        fn multiline() -> Result<()> {
+            run("add t100.md\r\nanother line\r\nthird line", "add t100.md")
+        }
+
+        #[test]
+        fn trailing_newlines() -> Result<()> {
+            run("add t100.md\r\n\r\n\r\n\r\n\r\n\r\n", "add t100.md")
+        }
+
+        #[test]
+        fn unicode_characters() -> Result<()> {
+            run("add t100.md ❤️", "add t100.md ❤️")
+        }
+    }
+
     mod get_commit_author {
         use super::*;
 
diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs
index bcac178..fdaab8e 100644
--- a/src/sub_commands/push.rs
+++ b/src/sub_commands/push.rs
@@ -107,7 +107,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> {
         sub_commands::send::launch(
             cli_args,
             &sub_commands::send::SubCommandArgs {
-                since_or_revision_range: String::new(),
+                since_or_range: String::new(),
                 in_reply_to: Some(proposal_root_event.id.to_string()),
                 title: None,
                 description: None,
diff --git a/src/sub_commands/send.rs b/src/sub_commands/send.rs
index 16f10c4..9b44cc3 100644
--- a/src/sub_commands/send.rs
+++ b/src/sub_commands/send.rs
@@ -1,6 +1,7 @@
 use std::{str::FromStr, time::Duration};
 
 use anyhow::{bail, Context, Result};
+use console::Style;
 use futures::future::join_all;
 use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
 use nostr::{
@@ -14,7 +15,9 @@ use crate::client::Client;
 #[cfg(test)]
 use crate::client::MockConnect;
 use crate::{
-    cli_interactor::{Interactor, InteractorPrompt, PromptConfirmParms, PromptInputParms},
+    cli_interactor::{
+        Interactor, InteractorPrompt, PromptConfirmParms, PromptInputParms, PromptMultiChoiceParms,
+    },
     client::Connect,
     git::{Repo, RepoActions},
     login,
@@ -24,9 +27,9 @@ use crate::{
 
 #[derive(Debug, clap::Args)]
 pub struct SubCommandArgs {
-    #[arg(default_value = "master..HEAD")]
-    /// commits to send as proposal; like in `git format-patch`
-    pub(crate) since_or_revision_range: String,
+    #[arg(default_value = "")]
+    /// commits to send as proposal; like in `git format-patch` eg. HEAD~2
+    pub(crate) since_or_range: String,
     #[clap(long)]
     /// nevent or event id of an existing proposal for which this is a new
     /// version
@@ -46,67 +49,83 @@ pub struct SubCommandArgs {
 pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> {
     let git_repo = Repo::discover().context("cannot find a git repository")?;
 
+    if let Some(id) = &args.in_reply_to {
+        println!("creating proposal revision for: {id}");
+    }
+
     let mut commits: Vec<Sha1Hash> = {
-        if args.since_or_revision_range.is_empty()
-            || args.since_or_revision_range.eq("master..HEAD")
-        {
+        if args.since_or_range.is_empty() {
             let branch_name = git_repo.get_checked_out_branch_name()?;
             let (main_branch_name, main_tip) = git_repo
                 .get_main_or_master_branch()
                 .context("the default branches (main or master) do not exist")?;
-            if branch_name.eq(main_branch_name) {
-                println!("creating 1 patch from latest commit");
+
+            let proposed_commits = if branch_name.eq(main_branch_name) {
                 vec![main_tip]
             } else {
-                let (from_branch, to_branch, ahead, behind) =
-                    identify_ahead_behind(&git_repo, &None, &None)?;
-
-                if ahead.is_empty() {
-                    bail!(format!(
-                        "'{from_branch}' is 0 commits ahead of '{to_branch}' so no patches were created"
-                    ));
-                }
-
-                if behind.is_empty() {
-                    println!(
-                        "creating patch for {} commits from '{from_branch}' that can be merged into '{to_branch}'",
-                        ahead.len(),
-                    );
-                } else {
-                    if !Interactor::default().confirm(
-                    PromptConfirmParms::default()
-                        .with_prompt(
-                            format!(
-                                "'{from_branch}' is {} commits behind '{to_branch}' and {} ahead. Consider rebasing before sending patches. Proceed anyway?",
-                                behind.len(),
-                                ahead.len(),
-                            )
-                        )
-                        .with_default(false)
-                ).context("failed to get confirmation response from interactor confirm")? {
-                    bail!("aborting so branch can be rebased");
-                }
-                    println!(
-                        "creating patch for {} commit{} from '{from_branch}' that {} {} behind '{to_branch}'",
-                        ahead.len(),
-                        if ahead.len() > 1 { "s" } else { "" },
-                        if ahead.len() > 1 { "are" } else { "is" },
-                        behind.len(),
-                    );
-                }
+                let (_, _, ahead, _) = identify_ahead_behind(&git_repo, &None, &None)?;
                 ahead
-            }
+            };
+            choose_commits(&git_repo, proposed_commits)?
         } else {
-            let ahead = git_repo
-                .parse_starting_commits(&args.since_or_revision_range)
-                .context("cannot parse specified starting commit or range")?;
-            println!("creating patch for {} commits", ahead.len(),);
-            ahead
+            git_repo
+                .parse_starting_commits(&args.since_or_range)
+                .context("cannot parse specified starting commit or range")?
         }
     };
 
-    if let Some(id) = &args.in_reply_to {
-        println!("as a revision to proposal: {id}");
+    if commits.is_empty() {
+        bail!("no commits selected");
+    }
+    println!("creating proposal from {} commits:", commits.len());
+
+    let dim = Style::new().color256(247);
+    for commit in &commits {
+        println!(
+            "{} {}",
+            dim.apply_to(commit.to_string().chars().take(7).collect::<String>()),
+            git_repo.get_commit_message_summary(commit)?
+        );
+    }
+
+    let (main_branch_name, main_tip) = git_repo
+        .get_main_or_master_branch()
+        .context("the default branches (main or master) do not exist")?;
+    let (first_commit_ahead, behind) =
+        git_repo.get_commits_ahead_behind(&main_tip, commits.last().context("no commits")?)?;
+
+    // check proposal ahead of origin/main
+    if first_commit_ahead.len().gt(&1) && !Interactor::default().confirm(
+            PromptConfirmParms::default()
+                .with_prompt(
+                    format!("proposal builds on a commit {} ahead of '{main_branch_name}' - do you want to continue?", first_commit_ahead.len() - 1)
+                )
+                .with_default(false)
+        ).context("failed to get confirmation response from interactor confirm")? {
+        bail!("aborting because selected commits were ahead of origin/master");
+    }
+
+    // check if a selected commit is already in origin
+    if commits.iter().any(|c| c.eq(&main_tip)) {
+        if !Interactor::default().confirm(
+            PromptConfirmParms::default()
+                .with_prompt(
+                    format!("proposal contains commit(s) already in  '{main_branch_name}'. proceed anyway?")
+                )
+                .with_default(false)
+        ).context("failed to get confirmation response from interactor confirm")? {
+            bail!("aborting as proposal contains commit(s) already in '{main_branch_name}'");
+        }
+    }
+    // check proposal isn't behind origin/main
+    else if !behind.is_empty() && !Interactor::default().confirm(
+            PromptConfirmParms::default()
+                .with_prompt(
+                    format!("proposal is {} behind '{main_branch_name}'. consider rebasing before submission. proceed anyway?", behind.len())
+                )
+                .with_default(false)
+        ).context("failed to get confirmation response from interactor confirm")? {
+        bail!("aborting so commits can be rebased");
     }
 
     let title = if args.no_cover_letter {
@@ -359,6 +378,97 @@ where
     (vec1_u, vec2_u, dup, all)
 }
 
+fn choose_commits(git_repo: &Repo, proposed_commits: Vec<Sha1Hash>) -> Result<Vec<Sha1Hash>> {
+    let mut proposed_commits = if proposed_commits.len().gt(&10) {
+        vec![]
+    } else {
+        proposed_commits
+    };
+
+    let tip_of_head = git_repo.get_tip_of_local_branch(&git_repo.get_checked_out_branch_name()?)?;
+    let most_recent_commit = proposed_commits.first().unwrap_or(&tip_of_head);
+
+    let mut last_15_commits = vec![*most_recent_commit];
+
+    while last_15_commits.len().lt(&15) {
+        if let Ok(parent_commit) = git_repo.get_commit_parent(last_15_commits.last().unwrap()) {
+            last_15_commits.push(parent_commit);
+        } else {
+            break;
+        }
+    }
+
+    let term = console::Term::stderr();
+    let mut printed_error_line = false;
+
+    let selected_commits = 'outer: loop {
+        let selected = Interactor::default().multi_choice(
+            PromptMultiChoiceParms::default()
+                .with_prompt("select commits for proposal")
+                .dont_report()
+                .with_choices(
+                    last_15_commits
+                        .iter()
+                        .map(|h| summarise_commit_for_selection(git_repo, h).unwrap())
+                        .collect(),
+                )
+                .with_defaults(
+                    last_15_commits
+                        .iter()
+                        .map(|h| proposed_commits.iter().any(|c| c.eq(h)))
+                        .collect(),
+                ),
+        )?;
+        proposed_commits = selected.iter().map(|i| last_15_commits[*i]).collect();
+
+        if printed_error_line {
+            term.clear_last_lines(1)?;
+        }
+
+        if proposed_commits.is_empty() {
+            term.write_line("no commits selected")?;
+            printed_error_line = true;
+            continue;
+        }
+        for (i, selected_i) in selected.iter().enumerate() {
+            if i.gt(&0) && selected_i.ne(&(selected[i - 1] + 1)) {
+                term.write_line("commits must be consecutive. try again.")?;
+                printed_error_line = true;
+                continue 'outer;
+            }
+        }
+
+        break proposed_commits;
+    };
+    Ok(selected_commits)
+}
+
+fn summarise_commit_for_selection(git_repo: &Repo, commit: &Sha1Hash) -> Result<String> {
+    let references = git_repo.get_refs(commit)?;
+    let dim = Style::new().color256(247);
+    let prefix = format!("({})", git_repo.get_commit_author(commit)?[0],);
+    let references_string = if references.is_empty() {
+        String::new()
+    } else {
+        format!(
+            " {}",
+            references
+                .iter()
+                .map(|r| format!("[{r}]"))
+                .collect::<Vec<String>>()
+                .join(" ")
+        )
+    };
+
+    Ok(format!(
+        "{} {}{} {}",
+        dim.apply_to(prefix),
+        git_repo.get_commit_message_summary(commit)?,
+        Style::new().magenta().apply_to(references_string),
+        dim.apply_to(commit.to_string().chars().take(7).collect::<String>(),),
+    ))
+}
+
 mod tests_unique_and_duplicate {
 
     #[test]
diff --git a/test_utils/src/lib.rs b/test_utils/src/lib.rs
index b4f0360..2edbc60 100644
--- a/test_utils/src/lib.rs
+++ b/test_utils/src/lib.rs
@@ -1,6 +1,6 @@
 use std::{ffi::OsStr, path::PathBuf};
 
-use anyhow::{ensure, Context, Result};
+use anyhow::{bail, ensure, Context, Result};
 use dialoguer::theme::{ColorfulTheme, Theme};
 use directories::ProjectDirs;
 use nostr::{self, prelude::FromSkStr, Kind, Tag};
@@ -259,6 +259,20 @@ impl CliTester {
         i.prompt(false).context("initial confirm prompt")?;
         Ok(i)
     }
+
+    pub fn expect_multi_select(
+        &mut self,
+        prompt: &str,
+        choices: Vec<String>,
+    ) -> Result<CliTesterMultiSelectPrompt> {
+        let mut i = CliTesterMultiSelectPrompt {
+            tester: self,
+            prompt: prompt.to_string(),
+            choices,
+        };
+        i.prompt(false).context("initial confirm prompt")?;
+        Ok(i)
+    }
 }
 
 pub struct CliTesterInputPrompt<'a> {
@@ -448,6 +462,75 @@ impl CliTesterConfirmPrompt<'_> {
     }
 }
 
+pub struct CliTesterMultiSelectPrompt<'a> {
+    tester: &'a mut CliTester,
+    prompt: String,
+    choices: Vec<String>,
+}
+
+impl CliTesterMultiSelectPrompt<'_> {
+    fn prompt(&mut self, eventually: bool) -> Result<&mut Self> {
+        if eventually {
+            self.tester
+                .expect_eventually(format!("{}:\r\n", self.prompt))
+                .context("expect multi-select prompt eventually")?;
+        } else {
+            self.tester
+                .expect(format!("{}:\r\n", self.prompt))
+                .context("expect multi-select prompt")?;
+        }
+        Ok(self)
+    }
+
+    pub fn succeeds_with(
+        &mut self,
+        chosen_indexes: Vec<usize>,
+        report: bool,
+        default_indexes: Vec<usize>,
+    ) -> Result<&mut Self> {
+        if report {
+            bail!("TODO: add support for report")
+        }
+
+        fn show_options(
+            tester: &mut CliTester,
+            choices: &[String],
+            active_index: usize,
+            selected_indexes: &[usize],
+        ) -> Result<()> {
+            for (index, item) in choices.iter().enumerate() {
+                tester.expect(format!(
+                    "{}{}{}\r\n",
+                    if active_index.eq(&index) { "> " } else { "  " },
+                    if selected_indexes.iter().any(|i| i.eq(&index)) {
+                        "[x] "
+                    } else {
+                        "[ ] "
+                    },
+                    item,
+                ))?;
+            }
+            Ok(())
+        }
+
+        show_options(self.tester, &self.choices, 0, &default_indexes)?;
+
+        if default_indexes.eq(&chosen_indexes) {
+            self.tester.send("\r\n")?;
+        } else {
+            bail!("TODO: add support changing options");
+        }
+
+        for _ in self.choices.iter() {
+            self.tester.expect("\r")?;
+        }
+        // one for removing prompt maybe?
+        self.tester.expect("\r")?;
+
+        Ok(self)
+    }
+}
+
 pub struct CliTesterChoicePrompt<'a> {
     tester: &'a mut CliTester,
     prompt: String,
diff --git a/tests/list.rs b/tests/list.rs
index 7e12dc1..4f55645 100644
--- a/tests/list.rs
+++ b/tests/list.rs
@@ -83,6 +83,7 @@ fn cli_tester_create_proposal(
                 TEST_PASSWORD,
                 "--disable-cli-spinners",
                 "send",
+                "HEAD~2",
                 "--no-cover-letter",
                 "--in-reply-to",
                 in_reply_to.as_str(),
@@ -99,6 +100,7 @@ fn cli_tester_create_proposal(
                 TEST_PASSWORD,
                 "--disable-cli-spinners",
                 "send",
+                "HEAD~2",
                 "--title",
                 format!("\"{title}\"").as_str(),
                 "--description",
@@ -116,6 +118,7 @@ fn cli_tester_create_proposal(
                 TEST_PASSWORD,
                 "--disable-cli-spinners",
                 "send",
+                "HEAD~2",
                 "--no-cover-letter",
             ],
         );
diff --git a/tests/pull.rs b/tests/pull.rs
index 77ff87b..50dddbf 100644
--- a/tests/pull.rs
+++ b/tests/pull.rs
@@ -81,6 +81,7 @@ fn cli_tester_create_proposal(
                 TEST_PASSWORD,
                 "--disable-cli-spinners",
                 "send",
+                "HEAD~2",
                 "--no-cover-letter",
                 "--in-reply-to",
                 in_reply_to.as_str(),
@@ -97,6 +98,7 @@ fn cli_tester_create_proposal(
                 TEST_PASSWORD,
                 "--disable-cli-spinners",
                 "send",
+                "HEAD~2",
                 "--title",
                 format!("\"{title}\"").as_str(),
                 "--description",
@@ -114,6 +116,7 @@ fn cli_tester_create_proposal(
                 TEST_PASSWORD,
                 "--disable-cli-spinners",
                 "send",
+                "HEAD~2",
                 "--no-cover-letter",
             ],
         );
diff --git a/tests/push.rs b/tests/push.rs
index db7a8b8..5fe1f15 100644
--- a/tests/push.rs
+++ b/tests/push.rs
@@ -80,6 +80,7 @@ fn cli_tester_create_proposal(
             TEST_PASSWORD,
             "--disable-cli-spinners",
             "send",
+            "HEAD~2",
             "--title",
             format!("\"{title}\"").as_str(),
             "--description",
@@ -571,13 +572,26 @@ mod when_branch_is_checked_out {
                         p.expect("finding proposal root event...\r\n")?;
                         p.expect("found proposal root event. finding commits...\r\n")?;
                         p.expect("preparing to force push proposal revision...\r\n")?;
-
                         // standard output from `ngit send`
-                        p.expect(format!("creating patch for 2 commits from '{FEATURE_BRANCH_NAME_1}' that can be merged into 'main'\r\n"))?;
-                        p.expect("as a revision to proposal: ")?;
+                        p.expect("creating proposal revision for: ")?;
                         // proposal id will be printed in this gap
                         p.expect_eventually("\r\n")?;
-
+                        let mut selector = p.expect_multi_select(
+                            "select commits for proposal",
+                            vec![
+                                "(Joe Bloggs) add a4.md [feature-example-t] 355bdf1".to_string(),
+                                "(Joe Bloggs) add a3.md dbd1115".to_string(),
+                                "(Joe Bloggs) commit for rebasing on top of [main] 1aa2cfe"
+                                    .to_string(),
+                                "(Joe Bloggs) add t2.md 431b84e".to_string(),
+                                "(Joe Bloggs) add t1.md af474d8".to_string(),
+                                "(Joe Bloggs) Initial commit 9ee507f".to_string(),
+                            ],
+                        )?;
+                        selector.succeeds_with(vec![0, 1], false, vec![0, 1])?;
+                        p.expect("creating proposal from 2 commits:\r\n")?;
+                        p.expect("355bdf1 add a4.md\r\n")?;
+                        p.expect("dbd1115 add a3.md\r\n")?;
                         p.expect("searching for profile and relay updates...\r\n")?;
                         p.expect("\r")?;
                         p.expect("logged in as fred\r\n")?;
diff --git a/tests/send.rs b/tests/send.rs
index 3c619a4..538f38a 100644
--- a/tests/send.rs
+++ b/tests/send.rs
@@ -12,19 +12,8 @@ fn when_no_main_or_master_branch_return_error() -> Result<()> {
     Ok(())
 }
 
-#[test]
-fn when_no_commits_ahead_of_main_return_error() -> Result<()> {
-    let test_repo = GitTestRepo::default();
-    test_repo.populate()?;
-    // create feature branch with 1 commit ahead
-    test_repo.create_branch("feature")?;
-    test_repo.checkout("feature")?;
-
-    let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
-    p.expect("Error: 'feature' is 0 commits ahead of 'main' so no patches were created")?;
-    Ok(())
-}
-
+// TODO when commits ahead of origin/master - test ask to proceed
+// TODO when commits in origin/master - test ask to proceed
 mod when_commits_behind_ask_to_proceed {
     use super::*;
 
@@ -46,16 +35,13 @@ mod when_commits_behind_ask_to_proceed {
         test_repo.checkout("feature")?;
         Ok(test_repo)
     }
-    static BEHIND_LEN: u8 = 1;
-    static AHEAD_LEN: u8 = 2;
-
-    fn expect_confirm_prompt(
-        p: &mut CliTester,
-        behind: u8,
-        ahead: u8,
-    ) -> Result<CliTesterConfirmPrompt> {
+
+    fn expect_confirm_prompt(p: &mut CliTester) -> Result<CliTesterConfirmPrompt> {
+        p.expect("creating proposal from 2 commits:\r\n")?;
+        p.expect("fe973a8 add t4.md\r\n")?;
+        p.expect("232efb3 add t3.md\r\n")?;
         p.expect_confirm(
-            format!("'feature' is {behind} commits behind 'main' and {ahead} ahead. Consider rebasing before sending patches. Proceed anyway?").as_str(),
+            "proposal is 1 behind 'main'. consider rebasing before submission. proceed anyway?",
             Some(false),
         )
     }
@@ -64,8 +50,8 @@ mod when_commits_behind_ask_to_proceed {
     fn asked_with_default_no() -> Result<()> {
         let test_repo = prep_test_repo()?;
 
-        let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
-        expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?;
+        let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]);
+        expect_confirm_prompt(&mut p)?;
         p.exit()?;
         Ok(())
     }
@@ -74,11 +60,11 @@ mod when_commits_behind_ask_to_proceed {
     fn when_response_is_false_aborts() -> Result<()> {
         let test_repo = prep_test_repo()?;
 
-        let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
+        let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]);
 
-        expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?.succeeds_with(Some(false))?;
+        expect_confirm_prompt(&mut p)?.succeeds_with(Some(false))?;
 
-        p.expect_end_with("Error: aborting so branch can be rebased\r\n")?;
+        p.expect_end_with("Error: aborting so commits can be rebased\r\n")?;
 
         Ok(())
     }
@@ -87,37 +73,14 @@ mod when_commits_behind_ask_to_proceed {
     fn when_response_is_true_proceeds() -> Result<()> {
         let test_repo = prep_test_repo()?;
 
-        let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
-        expect_confirm_prompt(&mut p, BEHIND_LEN, AHEAD_LEN)?.succeeds_with(Some(true))?;
-        p.expect(
-            format!("creating patch for {AHEAD_LEN} commits from 'feature' that are {BEHIND_LEN} behind 'main'",)
-                .as_str(),
-        )?;
+        let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]);
+        expect_confirm_prompt(&mut p)?.succeeds_with(Some(true))?;
+        p.expect("? include cover letter")?;
         p.exit()?;
         Ok(())
     }
 }
 
-#[test]
-#[serial]
-fn cli_message_creating_patches() -> Result<()> {
-    let test_repo = GitTestRepo::default();
-    test_repo.populate()?;
-    // create feature branch with 2 commit ahead
-    test_repo.create_branch("feature")?;
-    test_repo.checkout("feature")?;
-    std::fs::write(test_repo.dir.join("t3.md"), "some content")?;
-    test_repo.stage_and_commit("add t3.md")?;
-    std::fs::write(test_repo.dir.join("t4.md"), "some content")?;
-    test_repo.stage_and_commit("add t4.md")?;
-
-    let mut p = CliTester::new_from_dir(&test_repo.dir, ["send"]);
-
-    p.expect("creating patch for 2 commits from 'feature' that can be merged into 'main'")?;
-    p.exit()?;
-    Ok(())
-}
-
 fn is_cover_letter(event: &nostr::Event) -> bool {
     event.kind.as_u64().eq(&PATCH_KIND)
         && event.iter_tags().any(|t| t.as_vec()[1].eq("cover-letter"))
@@ -149,6 +112,7 @@ fn cli_tester_create_proposal(git_repo: &GitTestRepo, include_cover_letter: bool
         TEST_PASSWORD,
         "--disable-cli-spinners",
         "send",
+        "HEAD~2",
     ];
     if include_cover_letter {
         for arg in [
@@ -166,7 +130,9 @@ fn cli_tester_create_proposal(git_repo: &GitTestRepo, include_cover_letter: bool
 }
 
 fn expect_msgs_first(p: &mut CliTester, include_cover_letter: bool) -> Result<()> {
-    p.expect("creating patch for 2 commits from 'feature' that can be merged into 'main'\r\n")?;
+    p.expect("creating proposal from 2 commits:\r\n")?;
+    p.expect("fe973a8 add t4.md\r\n")?;
+    p.expect("232efb3 add t3.md\r\n")?;
     p.expect("searching for profile and relay updates...\r\n")?;
     p.expect("\r")?;
     p.expect("logged in as fred\r\n")?;
@@ -247,7 +213,7 @@ async fn prep_run_create_proposal(
     Ok((r51, r52, r53, r55, r56))
 }
 
-mod sends_cover_letter_and_2_patches_to_3_relays {
+mod when_cover_letter_details_specified_with_range_of_head_2_sends_cover_letter_and_2_patches_to_3_relays {
 
     use super::*;
     #[tokio::test]
@@ -937,7 +903,7 @@ mod sends_cover_letter_and_2_patches_to_3_relays {
     }
 }
 
-mod sends_2_patches_without_cover_letter {
+mod when_no_cover_letter_flag_set_with_range_of_head_2_sends_2_patches_without_cover_letter {
     use super::*;
 
     mod cli_ouput {
@@ -1118,206 +1084,9 @@ mod sends_2_patches_without_cover_letter {
     }
 }
 
-mod when_on_main_branch_defaults_to_last_commit {
-    use super::*;
-
-    fn prep_git_repo() -> Result<GitTestRepo> {
-        let test_repo = GitTestRepo::default();
-        test_repo.populate()?;
-        // dont checkout feature branch
-        std::fs::write(test_repo.dir.join("t3.md"), "some content")?;
-        test_repo.stage_and_commit("add t3.md")?;
-        std::fs::write(test_repo.dir.join("t4.md"), "some content")?;
-        test_repo.stage_and_commit("add t4.md")?;
-        Ok(test_repo)
-    }
-
-    fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester {
-        let args = vec![
-            "--nsec",
-            TEST_KEY_1_NSEC,
-            "--password",
-            TEST_PASSWORD,
-            "--disable-cli-spinners",
-            "send",
-            "--no-cover-letter",
-        ];
-        CliTester::new_from_dir(&git_repo.dir, args)
-    }
-    fn expect_msgs_first(p: &mut CliTester) -> Result<()> {
-        p.expect("creating 1 patch from latest commit\r\n")?;
-        p.expect("searching for profile and relay updates...\r\n")?;
-        p.expect("\r")?;
-        p.expect("logged in as fred\r\n")?;
-        p.expect("posting 1 patch without a covering letter...\r\n")?;
-        Ok(())
-    }
-    async fn prep_run_create_proposal() -> Result<(
-        Relay<'static>,
-        Relay<'static>,
-        Relay<'static>,
-        Relay<'static>,
-        Relay<'static>,
-    )> {
-        let git_repo = prep_git_repo()?;
-
-        // fallback (51,52) user write (53, 55) repo (55, 56)
-        let (mut r51, mut r52, mut r53, mut r55, mut r56) = (
-            Relay::new(
-                8051,
-                None,
-                Some(&|relay, client_id, subscription_id, _| -> Result<()> {
-                    relay.respond_events(
-                        client_id,
-                        &subscription_id,
-                        &vec![
-                            generate_test_key_1_metadata_event("fred"),
-                            generate_test_key_1_relay_list_event(),
-                        ],
-                    )?;
-                    Ok(())
-                }),
-            ),
-            Relay::new(8052, None, None),
-            Relay::new(8053, None, None),
-            Relay::new(
-                8055,
-                None,
-                Some(&|relay, client_id, subscription_id, _| -> Result<()> {
-                    relay.respond_events(
-                        client_id,
-                        &subscription_id,
-                        &vec![generate_repo_ref_event()],
-                    )?;
-                    Ok(())
-                }),
-            ),
-            Relay::new(8056, None, None),
-        );
-
-        // // check relay had the right number of events
-        let cli_tester_handle = std::thread::spawn(move || -> Result<()> {
-            let mut p = cli_tester_create_proposal(&git_repo);
-            p.expect_end_eventually()?;
-            for p in [51, 52, 53, 55, 56] {
-                relay::shutdown_relay(8000 + p)?;
-            }
-            Ok(())
-        });
-
-        // launch relay
-        let _ = join!(
-            r51.listen_until_close(),
-            r52.listen_until_close(),
-            r53.listen_until_close(),
-            r55.listen_until_close(),
-            r56.listen_until_close(),
-        );
-        cli_tester_handle.join().unwrap()?;
-        Ok((r51, r52, r53, r55, r56))
-    }
-    mod cli_ouput {
-        use super::*;
-
-        #[tokio::test]
-        #[serial]
-        async fn check_cli_output() -> Result<()> {
-            let git_repo = prep_git_repo()?;
-
-            let (mut r51, mut r52, mut r53, mut r55, mut r56) = (
-                Relay::new(
-                    8051,
-                    None,
-                    Some(&|relay, client_id, subscription_id, _| -> Result<()> {
-                        relay.respond_events(
-                            client_id,
-                            &subscription_id,
-                            &vec![
-                                generate_test_key_1_metadata_event("fred"),
-                                generate_test_key_1_relay_list_event(),
-                            ],
-                        )?;
-                        Ok(())
-                    }),
-                ),
-                Relay::new(8052, None, None),
-                Relay::new(8053, None, None),
-                Relay::new(
-                    8055,
-                    None,
-                    Some(&|relay, client_id, subscription_id, _| -> Result<()> {
-                        relay.respond_events(
-                            client_id,
-                            &subscription_id,
-                            &vec![generate_repo_ref_event()],
-                        )?;
-                        Ok(())
-                    }),
-                ),
-                Relay::new(8056, None, None),
-            );
-
-            // // check relay had the right number of events
-            let cli_tester_handle = std::thread::spawn(move || -> Result<()> {
-                let mut p = cli_tester_create_proposal(&git_repo);
-
-                expect_msgs_first(&mut p)?;
-                relay::expect_send_with_progress(
-                    &mut p,
-                    vec![
-                        (" [my-relay] [repo-relay] ws://localhost:8055", true, ""),
-                        (" [my-relay] ws://localhost:8053", true, ""),
-                        (" [repo-relay] ws://localhost:8056", true, ""),
-                        (" [default] ws://localhost:8051", true, ""),
-                        (" [default] ws://localhost:8052", true, ""),
-                    ],
-                    1,
-                )?;
-                p.expect_end_with_whitespace()?;
-                for p in [51, 52, 53, 55, 56] {
-                    relay::shutdown_relay(8000 + p)?;
-                }
-                Ok(())
-            });
-
-            // launch relay
-            let _ = join!(
-                r51.listen_until_close(),
-                r52.listen_until_close(),
-                r53.listen_until_close(),
-                r55.listen_until_close(),
-                r56.listen_until_close(),
-            );
-            cli_tester_handle.join().unwrap()?;
-            Ok(())
-        }
-    }
-
-    #[tokio::test]
-    #[serial]
-    async fn one_patch_event_sent() -> Result<()> {
-        let (_, _, r53, r55, r56) = prep_run_create_proposal().await?;
-        for relay in [&r53, &r55, &r56] {
-            assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 1);
-        }
-        Ok(())
-    }
-}
-
-mod specify_starting_commits_whist_on_main_branch {
+mod when_range_ommited_prompts_for_selection_defaulting_ahead_of_main {
     use super::*;
 
-    fn prep_git_repo() -> Result<GitTestRepo> {
-        let test_repo = GitTestRepo::default();
-        test_repo.populate()?;
-        // dont checkout feature branch
-        std::fs::write(test_repo.dir.join("t3.md"), "some content")?;
-        test_repo.stage_and_commit("add t3.md")?;
-        std::fs::write(test_repo.dir.join("t4.md"), "some content")?;
-        test_repo.stage_and_commit("add t4.md")?;
-        Ok(test_repo)
-    }
-
     fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester {
         let args = vec![
             "--nsec",
@@ -1326,17 +1095,29 @@ mod specify_starting_commits_whist_on_main_branch {
             TEST_PASSWORD,
             "--disable-cli-spinners",
             "send",
-            "HEAD~3",
             "--no-cover-letter",
         ];
         CliTester::new_from_dir(&git_repo.dir, args)
     }
     fn expect_msgs_first(p: &mut CliTester) -> Result<()> {
-        p.expect("creating patch for 3 commits\r\n")?;
+        let mut selector = p.expect_multi_select(
+            "select commits for proposal",
+            vec![
+                "(Joe Bloggs) add t4.md [feature] fe973a8".to_string(),
+                "(Joe Bloggs) add t3.md 232efb3".to_string(),
+                "(Joe Bloggs) add t2.md [main] 431b84e".to_string(),
+                "(Joe Bloggs) add t1.md af474d8".to_string(),
+                "(Joe Bloggs) Initial commit 9ee507f".to_string(),
+            ],
+        )?;
+        selector.succeeds_with(vec![0, 1], false, vec![0, 1])?;
+        p.expect("creating proposal from 2 commits:\r\n")?;
+        p.expect("fe973a8 add t4.md\r\n")?;
+        p.expect("232efb3 add t3.md\r\n")?;
         p.expect("searching for profile and relay updates...\r\n")?;
         p.expect("\r")?;
         p.expect("logged in as fred\r\n")?;
-        p.expect("posting 3 patches without a covering letter...\r\n")?;
+        p.expect("posting 2 patches without a covering letter...\r\n")?;
         Ok(())
     }
     async fn prep_run_create_proposal() -> Result<(
@@ -1385,6 +1166,7 @@ mod specify_starting_commits_whist_on_main_branch {
         // // check relay had the right number of events
         let cli_tester_handle = std::thread::spawn(move || -> Result<()> {
             let mut p = cli_tester_create_proposal(&git_repo);
+            expect_msgs_first(&mut p)?;
             p.expect_end_eventually()?;
             for p in [51, 52, 53, 55, 56] {
                 relay::shutdown_relay(8000 + p)?;
@@ -1444,7 +1226,6 @@ mod specify_starting_commits_whist_on_main_branch {
                 Relay::new(8056, None, None),
             );
 
-            // // check relay had the right number of events
             let cli_tester_handle = std::thread::spawn(move || -> Result<()> {
                 let mut p = cli_tester_create_proposal(&git_repo);
 
@@ -1458,7 +1239,7 @@ mod specify_starting_commits_whist_on_main_branch {
                         (" [default] ws://localhost:8051", true, ""),
                         (" [default] ws://localhost:8052", true, ""),
                     ],
-                    3,
+                    2,
                 )?;
                 p.expect_end_with_whitespace()?;
                 for p in [51, 52, 53, 55, 56] {
@@ -1482,84 +1263,16 @@ mod specify_starting_commits_whist_on_main_branch {
 
     #[tokio::test]
     #[serial]
-    async fn three_patch_events() -> Result<()> {
+    async fn two_patch_events_sent() -> Result<()> {
         let (_, _, r53, r55, r56) = prep_run_create_proposal().await?;
         for relay in [&r53, &r55, &r56] {
-            assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 3);
-        }
-        Ok(())
-    }
-
-    #[tokio::test]
-    #[serial]
-    async fn root_patch_doesnt_have_a_branch_name_tag() -> Result<()> {
-        let (_, _, r53, r55, r56) = prep_run_create_proposal().await?;
-        for relay in [&r53, &r55, &r56] {
-            let patch_events = relay
-                .events
-                .iter()
-                .filter(|e| is_patch(e))
-                .collect::<Vec<&nostr::Event>>();
-
-            assert!(
-                !patch_events[0]
-                    .iter_tags()
-                    .any(|t| t.as_vec()[0].eq("branch-name"))
-            );
-        }
-        Ok(())
-    }
-
-    #[tokio::test]
-    #[serial]
-    async fn first_patch_is_ancestor_and_root_others_in_correct_order() -> Result<()> {
-        let (_, _, r53, r55, r56) = prep_run_create_proposal().await?;
-        for relay in [&r53, &r55, &r56] {
-            let patch_events = relay
-                .events
-                .iter()
-                .filter(|e| is_patch(e))
-                .collect::<Vec<&nostr::Event>>();
-
-            // first patch tagged as root
-            assert!(
-                patch_events[0]
-                    .iter_tags()
-                    .any(|t| t.as_vec()[0].eq("t") && t.as_vec()[1].eq("root"))
-            );
-            // first patch is ancestor
-            assert_eq!(
-                patch_events[0]
-                    .iter_tags()
-                    .find(|t| t.as_vec()[0].eq("commit"))
-                    .unwrap()
-                    .as_vec()[1],
-                "431b84edc0d2fa118d63faa3c2db9c73d630a5ae"
-            );
-            // second patch not tagged as root
-            assert_eq!(
-                patch_events[1]
-                    .iter_tags()
-                    .find(|t| t.as_vec()[0].eq("commit"))
-                    .unwrap()
-                    .as_vec()[1],
-                "232efb37ebc67692c9e9ff58b83c0d3d63971a0a"
-            );
-            // second patch not tagged as root
-            assert_eq!(
-                patch_events[2]
-                    .iter_tags()
-                    .find(|t| t.as_vec()[0].eq("commit"))
-                    .unwrap()
-                    .as_vec()[1],
-                "fe973a840fba2a8ab37dd505c154854a69a6505c"
-            );
+            assert_eq!(relay.events.iter().filter(|e| is_patch(e)).count(), 2);
         }
         Ok(())
     }
 }
 
-mod specify_in_reply_to {
+mod in_reply_to_specified_with_range_of_head_2_and_cover_letter_details_specified {
     use super::*;
     fn cli_tester_create_proposal(git_repo: &GitTestRepo) -> CliTester {
         let args = vec![
@@ -1569,6 +1282,7 @@ mod specify_in_reply_to {
             TEST_PASSWORD,
             "--disable-cli-spinners",
             "send",
+            "HEAD~2",
             "--in-reply-to",
             "nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam",
             "--title",
@@ -1579,8 +1293,10 @@ mod specify_in_reply_to {
         CliTester::new_from_dir(&git_repo.dir, args)
     }
     fn expect_msgs_first(p: &mut CliTester, include_cover_letter: bool) -> Result<()> {
-        p.expect("creating patch for 2 commits from 'feature' that can be merged into 'main'\r\n")?;
-        p.expect("as a revision to proposal: nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam\r\n")?;
+        p.expect("creating proposal revision for: nevent1qqsypm62fzw7qynvlc4gjl3tr0jw4vmh659nvr2cc5qyhdg92a5yy0qzypumuen7l8wthtz45p3ftn58pvrs9xlumvkuu2xet8egzkcklqtesxygzam\r\n")?;
+        p.expect("creating proposal from 2 commits:\r\n")?;
+        p.expect("fe973a8 add t4.md\r\n")?;
+        p.expect("232efb3 add t3.md\r\n")?;
         p.expect("searching for profile and relay updates...\r\n")?;
         p.expect("\r")?;
         p.expect("logged in as fred\r\n")?;