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>
2022-12-21 08:47:01 -05:00 · 2022-12-21 08:47:01 -05:00 · a6d59b3c0e
parent 6d420813f5
commit a6d59b3c0e
4 changed files with 204 additions and 56 deletions
--- a/radicle-cli/examples/rad-patch.md
+++ 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.
+
+```
--- 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(())
 }
--- 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)?;
--- 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();
+}