From a6d59b3c0eeafe2d6fdff4b58e067c087c552a71 Mon Sep 17 00:00:00 2001
From: Slack Coder <slackcoder@server.ky>
Date: Wed, 21 Dec 2022 08:47:01 -0500
Subject: [PATCH] cli: alter use subcommands for patch

Make `rad patch` accept subcommands for specifying its operation in
place of the '--command' notation.

For instance `rad patch update` versus `rad patch --update`.  Also make
the default operation list active patches, replacing patch creation.
Patch creation is now done via `rad patch open`.

Make these changes in a way consistent with other commands like `rad
delegate`.

Add '--no-confirm' to patch to support this subcommand's documentation
test.

Signed-off-by: Slack Coder <slackcoder@server.ky>
---
 radicle-cli/examples/rad-patch.md        |  65 +++++++++++
 radicle-cli/src/commands/patch.rs        | 139 ++++++++++++++++-------
 radicle-cli/src/commands/patch/create.rs |  38 ++++---
 radicle-cli/tests/commands.rs            |  18 +++
 4 files changed, 204 insertions(+), 56 deletions(-)
 create mode 100644 radicle-cli/examples/rad-patch.md

diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md
new file mode 100644
index 00000000..d6a6f19a
--- /dev/null
+++ b/radicle-cli/examples/rad-patch.md
@@ -0,0 +1,65 @@
+When contributing to another's project, it is common for the contribution to be
+of many commits and involve a discussion with the project's maintainer.  This is supported
+via Radicle's patches.
+
+Here we give a brief overview for using patches in our hypothetical car
+scenario.  It turns out instructions containing the power requirements were
+missing from the project.
+
+```
+$ git checkout -b flux-capacitor-power
+$ touch README.md
+```
+
+Here the instructions are added to the project's README for 1.21 gigawatts and
+commit the changes to git.
+
+```
+$ git add README.md
+$ git commit -m "Define power requirements"
+[flux-capacitor-power 7939a9e] Define power requirements
+ 1 file changed, 0 insertions(+), 0 deletions(-)
+ create mode 100644 README.md
+```
+
+Once the code is ready, we open (or create) a patch with our changes for the project.
+
+```
+$ rad patch open --message "define power requirements" --no-confirm
+
+🌱 Creating patch for heartwood
+
+ok Pushing HEAD to storage...
+ok Analyzing remotes...
+
+z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master (cdf76ce) <- z6MknSL…StBU8Vi/flux-capacitor-power (7939a9e)
+1 commit(s) ahead, 0 commit(s) behind
+
+7939a9e Define power requirements
+
+
+╭─ define power requirements ───────
+
+No description provided.
+
+╰───────────────────────────────────
+
+
+ok Patch 3b1f58414e51266d7621203554a63eaee242b744 created 🌱
+```
+
+It will now be listed as one of the project's open patches.
+
+```
+$ rad patch
+
+ YOU PROPOSED
+
+define power requirements 3b1f58414e5 R0 7939a9e (flux-capacitor-power) ahead 1, behind 0
+└─ * opened by z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) [..]
+
+ OTHERS PROPOSED
+
+Nothing to show.
+
+```
diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs
index b9282b1c..4bb644ef 100644
--- a/radicle-cli/src/commands/patch.rs
+++ b/radicle-cli/src/commands/patch.rs
@@ -24,11 +24,13 @@ pub const HELP: Help = Help {
     usage: r#"
 Usage
 
-    rad patch [<option>...]
+    rad patch
+    rad patch open [<option>...]
+    rad patch update <id> [<option>...]
 
-Create options
+Create/Update options
 
-    -u, --update [<id>]        Update an existing patch (default: no)
+        --[no-]confirm         Don't ask for confirmation during clone
         --[no-]sync            Sync patch to seed (default: sync)
         --[no-]push            Push patch head to storage (default: true)
     -m, --message [<string>]   Provide a comment message to the patch or revision (default: prompt)
@@ -36,32 +38,45 @@ Create options
 
 Options
 
-    -l, --list                 List all patches (default: false)
         --help                 Print help
 "#,
 };
 
-#[derive(Debug)]
-pub enum Update {
-    No,
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+pub enum OptPatch {
+    #[default]
     Any,
+    None,
     Patch(PatchId),
 }
 
-impl Default for Update {
-    fn default() -> Self {
-        Self::No
-    }
+#[derive(Debug, Default, PartialEq, Eq)]
+pub enum OperationName {
+    Open,
+    Update,
+    #[default]
+    List,
 }
 
-#[derive(Default, Debug)]
+#[derive(Debug)]
+pub enum Operation {
+    Open {
+        message: Comment,
+    },
+    Update {
+        patch_id: OptPatch,
+        message: Comment,
+    },
+    List,
+}
+
+#[derive(Debug)]
 pub struct Options {
-    pub list: bool,
-    pub verbose: bool,
+    pub op: Operation,
+    pub confirm: bool,
     pub sync: bool,
     pub push: bool,
-    pub update: Update,
-    pub message: Comment,
+    pub verbose: bool,
 }
 
 impl Args for Options {
@@ -69,34 +84,23 @@ impl Args for Options {
         use lexopt::prelude::*;
 
         let mut parser = lexopt::Parser::from_args(args);
-        let mut list = false;
+        let mut confirm = true;
+        let mut op: Option<OperationName> = None;
         let mut verbose = false;
         let mut sync = true;
+        let mut id = OptPatch::default();
         let mut message = Comment::default();
         let mut push = true;
-        let mut update = Update::default();
 
         while let Some(arg) = parser.next()? {
             match arg {
-                // Operations.
-                Long("list") | Short('l') => {
-                    list = true;
-                }
-                Long("update") | Short('u') => {
-                    if let Ok(val) = parser.value() {
-                        let val = val
-                            .to_str()
-                            .ok_or_else(|| anyhow!("patch id specified is not UTF-8"))?;
-                        let id = PatchId::from_str(val)
-                            .map_err(|_| anyhow!("invalid patch id '{}'", val))?;
-
-                        update = Update::Patch(id);
-                    } else {
-                        update = Update::Any;
-                    }
-                }
-
                 // Options.
+                Long("confirm") => {
+                    confirm = true;
+                }
+                Long("no-confirm") => {
+                    confirm = false;
+                }
                 Long("message") | Short('m') => {
                     if message != Comment::Blank {
                         // We skip this code when `no-message` is specified.
@@ -129,17 +133,43 @@ impl Args for Options {
                 Long("help") => {
                     return Err(Error::Help.into());
                 }
+
+                Value(val) if op.is_none() => match val.to_string_lossy().as_ref() {
+                    "l" | "list" => op = Some(OperationName::List),
+                    "o" | "open" => op = Some(OperationName::Open),
+                    "u" | "update" => op = Some(OperationName::Update),
+
+                    unknown => anyhow::bail!("unknown operation '{}'", unknown),
+                },
+                Value(val) if op == Some(OperationName::Update) && id == OptPatch::Any => {
+                    let val = val
+                        .to_str()
+                        .ok_or_else(|| anyhow!("patch id specified is not UTF-8"))?;
+
+                    id = OptPatch::Patch(
+                        PatchId::from_str(val)
+                            .map_err(|_| anyhow!("invalid patch id '{}'", val))?,
+                    );
+                }
                 _ => return Err(anyhow::anyhow!(arg.unexpected())),
             }
         }
 
+        let op = match op.unwrap_or_default() {
+            OperationName::Open => Operation::Open { message },
+            OperationName::List => Operation::List,
+            OperationName::Update => Operation::Update {
+                patch_id: id,
+                message,
+            },
+        };
+
         Ok((
             Options {
-                list,
+                op,
+                confirm,
                 sync,
-                message,
                 push,
-                update,
                 verbose,
             },
             vec![],
@@ -154,10 +184,33 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
     let storage = profile.storage.repository(id)?;
 
-    if options.list {
-        list::run(&storage, &profile, Some(workdir), options)?;
-    } else {
-        create::run(&storage, &profile, &workdir, options)?;
+    match options.op {
+        Operation::Open { ref message } => {
+            create::run(
+                &storage,
+                &profile,
+                &workdir,
+                OptPatch::None,
+                message.clone(),
+                options,
+            )?;
+        }
+        Operation::List => {
+            list::run(&storage, &profile, Some(workdir), options)?;
+        }
+        Operation::Update {
+            ref patch_id,
+            ref message,
+        } => {
+            create::run(
+                &storage,
+                &profile,
+                &workdir,
+                *patch_id,
+                message.clone(),
+                options,
+            )?;
+        }
     }
     Ok(())
 }
diff --git a/radicle-cli/src/commands/patch/create.rs b/radicle-cli/src/commands/patch/create.rs
index 08366847..875c39de 100644
--- a/radicle-cli/src/commands/patch/create.rs
+++ b/radicle-cli/src/commands/patch/create.rs
@@ -1,3 +1,4 @@
+use std::fmt;
 use std::path::Path;
 
 use anyhow::{anyhow, Context};
@@ -10,9 +11,10 @@ use radicle::storage::git::Repository;
 
 use crate::terminal as term;
 use crate::terminal::args::Error;
+use crate::terminal::patch;
 
 use super::common;
-use super::{Options, Update};
+use super::{OptPatch, Options};
 
 const PATCH_MSG: &str = r#"
 <!--
@@ -33,11 +35,18 @@ blank is also okay.
 -->
 "#;
 
+#[inline]
+fn confirm<D: fmt::Display>(prompt: D, options: &Options) -> bool {
+    !options.confirm || term::confirm(prompt)
+}
+
 /// Run patch creation.
 pub fn run(
     storage: &Repository,
     profile: &Profile,
     workdir: &git::raw::Repository,
+    patch_id: OptPatch,
+    message: patch::Comment,
     options: Options,
 ) -> anyhow::Result<()> {
     let project = storage.project_of(&profile.public_key).context(format!(
@@ -118,9 +127,9 @@ pub fn run(
     let base_oid = workdir.merge_base((*target_oid).into(), head_oid)?;
     let commits = common::patch_commits(workdir, &base_oid, &head_oid)?;
 
-    let patch = match &options.update {
-        Update::No => None,
-        Update::Any => {
+    let patch = match &patch_id {
+        OptPatch::None => None,
+        OptPatch::Any => {
             let mut spinner = term::spinner("Finding patches to update...");
             let mut result = common::find_unmerged_with_base(
                 head_oid,
@@ -152,7 +161,7 @@ pub fn run(
                 anyhow::bail!("No patches found that share a base, please create a new patch or specify the patch id manually");
             }
         }
-        Update::Patch(id) => {
+        OptPatch::Patch(id) => {
             if let Ok(patch) = patches.get_mut(id) {
                 Some((*id, patch))
             } else {
@@ -162,10 +171,12 @@ pub fn run(
     };
 
     if let Some((id, patch)) = patch {
-        if term::confirm("Update?") {
+        if confirm("Update?", &options) {
             term::blank();
 
-            return update(patch, id, &base_oid, &head_oid, workdir, options, &signer);
+            return update(
+                patch, id, &base_oid, &head_oid, workdir, options, message, &signer,
+            );
         } else {
             anyhow::bail!("Patch update aborted by user");
         }
@@ -194,14 +205,14 @@ pub fn run(
     term::patch::list_commits(&commits)?;
     term::blank();
 
-    if !term::confirm("Continue?") {
+    if !confirm("Continue?", &options) {
         anyhow::bail!("patch proposal aborted by user");
     }
 
-    let message = head_commit
+    let commit_message = head_commit
         .message()
         .ok_or(anyhow!("commit summary is not valid UTF-8; aborting"))?;
-    let message = options.message.get(&format!("{}{}", message, PATCH_MSG));
+    let message = message.get(&format!("{}{}", commit_message, PATCH_MSG));
     let (title, description) = message.split_once("\n\n").unwrap_or((&message, ""));
     let (title, description) = (title.trim(), description.trim());
     let description = description.replace(PATCH_MSG.trim(), ""); // Delete help message.
@@ -229,7 +240,7 @@ pub fn run(
     )));
     term::blank();
 
-    if !term::confirm("Create patch?") {
+    if !confirm("Create patch?", &options) {
         anyhow::bail!("patch proposal aborted by user");
     }
 
@@ -261,6 +272,7 @@ fn update<G: Signer>(
     head: &Oid,
     workdir: &git::raw::Repository,
     options: Options,
+    message: patch::Comment,
     signer: &G,
 ) -> anyhow::Result<()> {
     // TODO(cloudhead): Handle error.
@@ -280,13 +292,13 @@ fn update<G: Signer>(
         term::format::dim(format!("R{}", current_version + 1)),
         term::format::secondary(term::format::oid(*head)),
     );
-    let message = options.message.get(REVISION_MSG);
+    let message = message.get(REVISION_MSG);
 
     // Difference between the two revisions.
     term::patch::print_commits_ahead_behind(workdir, *head, *current_revision.oid)?;
     term::blank();
 
-    if !term::confirm("Continue?") {
+    if !confirm("Continue?", &options) {
         anyhow::bail!("patch update aborted by user");
     }
     patch.update(message, *base, *head, signer)?;
diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs
index 06124950..4391bc4e 100644
--- a/radicle-cli/tests/commands.rs
+++ b/radicle-cli/tests/commands.rs
@@ -84,3 +84,21 @@ fn rad_delegate() {
     test("examples/rad-init.md", working.path(), Some(&profile)).unwrap();
     test("examples/rad-delegate.md", working.path(), Some(&profile)).unwrap();
 }
+
+#[test]
+fn rad_patch() {
+    let home = tempfile::tempdir().unwrap();
+    let working = tempfile::tempdir().unwrap();
+    let profile = profile(home.path());
+
+    // Setup a test repository.
+    fixtures::repository(working.path());
+    // Set a fixed commit time.
+    env::set_var(radicle_cob::git::RAD_COMMIT_TIME, "1671125284");
+    env::set_var("GIT_COMMITTER_DATE", "1671125284");
+    env::set_var("GIT_AUTHOR_DATE", "1671125284");
+
+    test("examples/rad-init.md", working.path(), Some(&profile)).unwrap();
+    test("examples/rad-issue.md", working.path(), Some(&profile)).unwrap();
+    test("examples/rad-patch.md", working.path(), Some(&profile)).unwrap();
+}