From 67cbf15e4566cbf41214ecc919f5694e180cd542 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 30 Nov 2023 17:56:01 +0000 Subject: [PATCH] cli: unify label and unlabel Make the CLI easier by unifying the label and unlabel, using `--add` and `--remove` options instead. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-cli/examples/rad-issue.md | 2 +- radicle-cli/examples/rad-patch.md | 2 +- radicle-cli/src/commands/issue.rs | 90 ++++++++++++------------- radicle-cli/src/commands/patch.rs | 72 ++++++++++---------- radicle-cli/src/commands/patch/label.rs | 25 ++----- 5 files changed, 86 insertions(+), 105 deletions(-) diff --git a/radicle-cli/examples/rad-issue.md b/radicle-cli/examples/rad-issue.md index 19cd78d0..15ba8ca4 100644 --- a/radicle-cli/examples/rad-issue.md +++ b/radicle-cli/examples/rad-issue.md @@ -52,7 +52,7 @@ duplicated. While we're at it, let's add a label. ``` $ rad issue assign d185ee1 --add did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi -$ rad issue label d185ee1 -l good-first-issue +$ rad issue label d185ee1 --add good-first-issue ``` It will now show in the list of issues assigned to us, along with the new label. diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md index d5d7947d..896675c1 100644 --- a/radicle-cli/examples/rad-patch.md +++ b/radicle-cli/examples/rad-patch.md @@ -93,7 +93,7 @@ We can also label patches as well as assign DIDs to the patch to help organise your workflow: ``` -$ rad patch label 6ff4f09 -l fun +$ rad patch label 6ff4f09 --add fun $ rad patch assign 6ff4f09 --add did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi $ rad patch show 6ff4f09 ╭────────────────────────────────────────────────────╮ diff --git a/radicle-cli/src/commands/issue.rs b/radicle-cli/src/commands/issue.rs index 22e6b6f7..65eb306b 100644 --- a/radicle-cli/src/commands/issue.rs +++ b/radicle-cli/src/commands/issue.rs @@ -4,7 +4,6 @@ use std::str::FromStr; use anyhow::{anyhow, Context as _}; -use nonempty::NonEmpty; use radicle::cob::common::{Label, Reaction}; use radicle::cob::issue; use radicle::cob::issue::{CloseReason, Issues, State}; @@ -40,7 +39,7 @@ Usage rad issue open [--title ] [--description <text>] [--label <label>] [<option>...] rad issue react <issue-id> [--emoji <char>] [--to <comment>] [<option>...] rad issue assign <issue-id> [--add <did>] [--remove <did>] [<option>...] - rad issue label <issue-id> --label <label> [<option>...] + rad issue label <issue-id> [--add <label>] [--remove <did>] [<option>...] rad issue unlabel <issue-id> --label <label> [<option>...] rad issue comment <issue-id> [--message <message>] [--reply-to <comment-id>] [<option>...] rad issue show <issue-id> [<option>...] @@ -48,26 +47,26 @@ Usage Assign options - -a, --add <did> Add an assignee to the issue (may be specified multiple times). - Note: --add will take precedence over --remove + -a, --add <did> Add an assignee to the issue (may be specified multiple times). + Note: --add will take precedence over --remove - -r, --remove <did> Remove an assignee from the issue (may be specified multiple times). - Note: --add will take precedence over --remove + -r, --remove <did> Remove an assignee from the issue (may be specified multiple times). + Note: --add will take precedence over --remove Label options - -l, --label Label the issue with the provided label (may be specified multiple times) + -a, --add <label> Add a label to the issue (may be specified multiple times). + Note: --add will take precedence over --remove -Unlabel options - - -l, --label Remove the provided label from the issue (may be specified multiple times) + -r, --remove <label> Remove a label from the issue (may be specified multiple times). + Note: --add will take precedence over --remove Options - --no-announce Don't announce issue to peers - --header Show only the issue header, hiding the comments - --quiet, -q Don't print anything - --help Print help + --no-announce Don't announce issue to peers + --header Show only the issue header, hiding the comments + --quiet, -q Don't print anything + --help Print help "#, }; @@ -84,7 +83,6 @@ pub enum OperationName { React, Show, State, - Unlabel, } /// Command line Peer argument. @@ -135,11 +133,7 @@ pub enum Operation { }, Label { id: Rev, - labels: NonEmpty<Label>, - }, - Unlabel { - id: Rev, - labels: NonEmpty<Label>, + opts: LabelOptions, }, List { assigned: Option<Assigned>, @@ -153,6 +147,12 @@ pub struct AssignOptions { pub remove: BTreeSet<Did>, } +#[derive(Debug, Default, PartialEq, Eq)] +pub struct LabelOptions { + pub add: BTreeSet<Label>, + pub remove: BTreeSet<Label>, +} + #[derive(Debug)] pub struct Options { pub op: Operation, @@ -181,6 +181,7 @@ impl Args for Options { let mut announce = true; let mut quiet = false; let mut assign_opts = AssignOptions::default(); + let mut label_opts = LabelOptions::default(); while let Some(arg) = parser.next()? { match arg { @@ -206,12 +207,7 @@ impl Args for Options { Long("title") if op == Some(OperationName::Open) => { title = Some(parser.value()?.to_string_lossy().into()); } - Short('l') | Long("label") - if matches!( - op, - Some(OperationName::Open | OperationName::Label | OperationName::Unlabel) - ) => - { + Short('l') | Long("label") if matches!(op, Some(OperationName::Open)) => { let val = parser.value()?; let name = term::args::string(&val); let label = Label::new(name)?; @@ -294,6 +290,22 @@ impl Args for Options { .insert(term::args::did(&parser.value()?)?); } + // Label options + Short('a') | Long("add") if matches!(op, Some(OperationName::Label)) => { + let val = parser.value()?; + let name = term::args::string(&val); + let label = Label::new(name)?; + + label_opts.add.insert(label); + } + Short('r') | Long("remove") if matches!(op, Some(OperationName::Label)) => { + let val = parser.value()?; + let name = term::args::string(&val); + let label = Label::new(name)?; + + label_opts.remove.insert(label); + } + Long("quiet") | Short('q') => { quiet = true; } @@ -308,7 +320,6 @@ impl Args for Options { "s" | "state" => op = Some(OperationName::State), "assign" => op = Some(OperationName::Assign), "label" => op = Some(OperationName::Label), - "unlabel" => op = Some(OperationName::Unlabel), unknown => anyhow::bail!("unknown operation '{}'", unknown), }, @@ -361,13 +372,7 @@ impl Args for Options { }, OperationName::Label => Operation::Label { id: id.ok_or_else(|| anyhow!("an issue to label must be provided"))?, - labels: NonEmpty::from_vec(labels) - .ok_or_else(|| anyhow!("at least one label must be specified"))?, - }, - OperationName::Unlabel => Operation::Unlabel { - id: id.ok_or_else(|| anyhow!("an issue to label must be provided"))?, - labels: NonEmpty::from_vec(labels) - .ok_or_else(|| anyhow!("at least one label must be specified"))?, + opts: label_opts, }, OperationName::List => Operation::List { assigned, state }, }; @@ -397,7 +402,6 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { | Operation::Delete { .. } | Operation::Assign { .. } | Operation::Label { .. } - | Operation::Unlabel { .. } ); let mut node = Node::new(profile.socket()); @@ -501,22 +505,18 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { .collect::<Vec<_>>(); issue.assign(assignees, &signer)?; } - Operation::Label { id, labels } => { - let id = id.resolve(&repo.backend)?; - let Ok(mut issue) = issues.get_mut(&id) else { - anyhow::bail!("Issue `{id}` not found"); - }; - let labels = issue.labels().cloned().chain(labels).collect::<Vec<_>>(); - issue.label(labels, &signer)?; - } - Operation::Unlabel { id, labels } => { + Operation::Label { + id, + opts: LabelOptions { add, remove }, + } => { let id = id.resolve(&repo.backend)?; let Ok(mut issue) = issues.get_mut(&id) else { anyhow::bail!("Issue `{id}` not found"); }; let labels = issue .labels() - .filter(|&l| !labels.contains(l)) + .filter(|did| !remove.contains(did)) + .chain(add.iter()) .cloned() .collect::<Vec<_>>(); issue.label(labels, &signer)?; diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index e67ae44d..7026fab9 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -30,7 +30,6 @@ use std::ffi::OsString; use anyhow::anyhow; -use nonempty::NonEmpty; use radicle::cob::patch::PatchId; use radicle::cob::{patch, Label}; use radicle::prelude::*; @@ -57,8 +56,7 @@ Usage rad patch delete <patch-id> [<option>...] rad patch redact <revision-id> [<option>...] rad patch assign <revision-id> [--add <did>] [--remove <did>] [<option>...] - rad patch label <revision-id> --label <label> [<option>...] - rad patch unlabel <revision-id> --label <label> [<option>...] + rad patch label <revision-id> [--add <label>] [--remove <label>] [<option>...] rad patch ready <patch-id> [--undo] [<option>...] rad patch edit <patch-id> [<option>...] rad patch set <patch-id> [<option>...] @@ -79,19 +77,19 @@ Edit options Assign options - -a, --add <did> Add an assignee to the issue (may be specified multiple times). - Note: --add will take precedence over --remove + -a, --add <did> Add an assignee to the issue (may be specified multiple times). + Note: --add will take precedence over --remove - -r, --remove <did> Remove an assignee from the issue (may be specified multiple times). - Note: --add will take precedence over --remove + -r, --remove <did> Remove an assignee from the issue (may be specified multiple times). + Note: --add will take precedence over --remove Label options - -l, --label Label the patch with the provided label (may be specified multiple times) + -a, --add <label> Add an assignee to the issue (may be specified multiple times). + Note: --add will take precedence over --remove -Unlabel options - - -l, --label Remove the provided label from the patch (may be specified multiple times) + -r, --remove <label> Remove an assignee from the issue (may be specified multiple times). + Note: --add will take precedence over --remove Update options @@ -137,7 +135,6 @@ pub enum OperationName { Comment, Ready, Label, - Unlabel, #[default] List, Edit, @@ -151,6 +148,12 @@ pub struct AssignOptions { pub remove: BTreeSet<Did>, } +#[derive(Debug, Default, PartialEq, Eq)] +pub struct LabelOptions { + pub add: BTreeSet<Label>, + pub remove: BTreeSet<Label>, +} + pub struct Filter(fn(&patch::State) -> bool); impl Filter { @@ -208,11 +211,7 @@ pub enum Operation { }, Label { patch_id: Rev, - labels: NonEmpty<Label>, - }, - Unlabel { - patch_id: Rev, - labels: NonEmpty<Label>, + opts: LabelOptions, }, List { filter: Filter, @@ -260,8 +259,8 @@ impl Args for Options { let mut undo = false; let mut reply_to: Option<Rev> = None; let mut checkout_opts = checkout::Options::default(); - let mut labels = Vec::new(); let mut assign_opts = AssignOptions::default(); + let mut label_opts = LabelOptions::default(); while let Some(arg) = parser.next()? { match arg { @@ -343,15 +342,21 @@ impl Args for Options { .insert(term::args::did(&parser.value()?)?); } - // Un/Label options. - Short('l') | Long("label") - if matches!(op, Some(OperationName::Label | OperationName::Unlabel)) => - { + // Label options. + Short('a') | Long("add") if matches!(op, Some(OperationName::Label)) => { let val = parser.value()?; let name = term::args::string(&val); let label = Label::new(name)?; - labels.push(label); + label_opts.add.insert(label); + } + + Short('r') | Long("remove") if matches!(op, Some(OperationName::Label)) => { + let val = parser.value()?; + let name = term::args::string(&val); + let label = Label::new(name)?; + + label_opts.remove.insert(label); } // List options. @@ -403,7 +408,6 @@ impl Args for Options { "r" | "redact" => op = Some(OperationName::Redact), "assign" => op = Some(OperationName::Assign), "label" => op = Some(OperationName::Label), - "unlabel" => op = Some(OperationName::Unlabel), "comment" => op = Some(OperationName::Comment), "set" => op = Some(OperationName::Set), unknown => anyhow::bail!("unknown operation '{}'", unknown), @@ -426,7 +430,6 @@ impl Args for Options { Some(OperationName::Set), Some(OperationName::Assign), Some(OperationName::Label), - Some(OperationName::Unlabel), ] .contains(&op) => { @@ -481,13 +484,7 @@ impl Args for Options { }, OperationName::Label => Operation::Label { patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, - labels: NonEmpty::from_vec(labels) - .ok_or_else(|| anyhow!("at least one label must be specified"))?, - }, - OperationName::Unlabel => Operation::Unlabel { - patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, - labels: NonEmpty::from_vec(labels) - .ok_or_else(|| anyhow!("at least one label must be specified"))?, + opts: label_opts, }, OperationName::Set => Operation::Set { patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, @@ -602,13 +599,12 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { let patch_id = patch_id.resolve(&repository.backend)?; assign::run(&patch_id, add, remove, &profile, &repository)?; } - Operation::Label { patch_id, labels } => { + Operation::Label { + patch_id, + opts: LabelOptions { add, remove }, + } => { let patch_id = patch_id.resolve(&repository.backend)?; - label::add(&patch_id, labels, &profile, &repository)?; - } - Operation::Unlabel { patch_id, labels } => { - let patch_id = patch_id.resolve(&repository.backend)?; - label::remove(&patch_id, labels, &profile, &repository)?; + label::run(&patch_id, add, remove, &profile, &repository)?; } Operation::Set { patch_id } => { let patches = radicle::cob::patch::Patches::open(&repository)?; diff --git a/radicle-cli/src/commands/patch/label.rs b/radicle-cli/src/commands/patch/label.rs index 1b6e6eb0..c4fd8a80 100644 --- a/radicle-cli/src/commands/patch/label.rs +++ b/radicle-cli/src/commands/patch/label.rs @@ -1,29 +1,13 @@ use super::*; -use nonempty::NonEmpty; use radicle::storage::git::Repository; use crate::terminal as term; -pub fn add( +pub fn run( patch_id: &PatchId, - labels: NonEmpty<Label>, - profile: &Profile, - repository: &Repository, -) -> anyhow::Result<()> { - let signer = term::signer(profile)?; - let mut patches = radicle::cob::patch::Patches::open(repository)?; - let Ok(mut patch) = patches.get_mut(patch_id) else { - anyhow::bail!("Patch `{patch_id}` not found"); - }; - let labels = patch.labels().cloned().chain(labels).collect::<Vec<_>>(); - patch.label(labels, &signer)?; - Ok(()) -} - -pub fn remove( - patch_id: &PatchId, - labels: NonEmpty<Label>, + add: BTreeSet<Label>, + remove: BTreeSet<Label>, profile: &Profile, repository: &Repository, ) -> anyhow::Result<()> { @@ -34,7 +18,8 @@ pub fn remove( }; let labels = patch .labels() - .filter(|&l| !labels.contains(l)) + .filter(|l| !remove.contains(l)) + .chain(add.iter()) .cloned() .collect::<Vec<_>>(); patch.label(labels, &signer)?;