From 6cfed884bf37cba1e0d8e97fa8b0e94df4a04b1f Mon Sep 17 00:00:00 2001
From: Fintan Halpenny <fintan.halpenny@gmail.com>
Date: Mon, 20 Oct 2025 15:45:01 +0100
Subject: [PATCH] cli/remote: migrate to clap

---
 crates/radicle-cli/src/commands/help.rs       |   5 +-
 crates/radicle-cli/src/commands/remote.rs     | 189 ++----------------
 .../radicle-cli/src/commands/remote/args.rs   | 161 +++++++++++++++
 crates/radicle-cli/src/main.rs                |   5 +-
 4 files changed, 186 insertions(+), 174 deletions(-)
 create mode 100644 crates/radicle-cli/src/commands/remote/args.rs

diff --git a/crates/radicle-cli/src/commands/help.rs b/crates/radicle-cli/src/commands/help.rs
index 5c012032..56275003 100644
--- a/crates/radicle-cli/src/commands/help.rs
+++ b/crates/radicle-cli/src/commands/help.rs
@@ -122,7 +122,10 @@ const COMMANDS: &[CommandItem] = &[
         name: "unseed",
         about: crate::commands::unseed::ABOUT,
     },
-    CommandItem::Lexopt(crate::commands::remote::HELP),
+    CommandItem::Clap {
+        name: "remote",
+        about: crate::commands::remote::ABOUT,
+    },
     CommandItem::Clap {
         name: "stats",
         about: crate::commands::stats::ABOUT,
diff --git a/crates/radicle-cli/src/commands/remote.rs b/crates/radicle-cli/src/commands/remote.rs
index 77705470..16013bba 100644
--- a/crates/radicle-cli/src/commands/remote.rs
+++ b/crates/radicle-cli/src/commands/remote.rs
@@ -4,203 +4,48 @@ pub mod add;
 pub mod list;
 pub mod rm;
 
-use std::ffi::OsString;
+mod args;
 
 use anyhow::anyhow;
 
-use radicle::git::fmt::RefString;
-use radicle::prelude::NodeId;
 use radicle::storage::ReadStorage;
 
 use crate::terminal as term;
-use crate::terminal::args;
-use crate::terminal::{Args, Context, Help};
+use crate::terminal::Context;
 
-pub const HELP: Help = Help {
-    name: "remote",
-    description: "Manage a repository's remotes",
-    version: env!("RADICLE_VERSION"),
-    usage: r#"
-Usage
+pub use args::Args;
+pub(crate) use args::ABOUT;
+use args::{Command, ListOption};
 
-    rad remote [<option>...]
-    rad remote list [--tracked | --untracked | --all] [<option>...]
-    rad remote add (<did> | <nid>) [--name <string>] [<option>...]
-    rad remote rm <name> [<option>...]
-
-List options
-
-    --tracked     Show all remotes that are listed in the working copy
-    --untracked   Show all remotes that are listed in the Radicle storage
-    --all         Show all remotes in both the Radicle storage and the working copy
-
-Add options
-
-    --name        Override the name of the remote that by default is set to the node alias
-    --[no-]fetch  Fetch the remote from local storage (default: fetch)
-    --[no-]sync   Sync the remote refs from the network (default: sync)
-
-Options
-
-    --help        Print help
-"#,
-};
-
-#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
-pub enum OperationName {
-    Add,
-    Rm,
-    #[default]
-    List,
-}
-
-#[derive(Debug)]
-pub enum Operation {
-    Add {
-        id: NodeId,
-        name: Option<RefString>,
-        fetch: bool,
-        sync: bool,
-    },
-    Rm {
-        name: RefString,
-    },
-    List {
-        option: ListOption,
-    },
-}
-
-#[derive(Debug, Default)]
-pub enum ListOption {
-    All,
-    #[default]
-    Tracked,
-    Untracked,
-}
-
-#[derive(Debug)]
-pub struct Options {
-    pub op: Operation,
-}
-
-impl Args for Options {
-    fn from_args(args: Vec<OsString>) -> anyhow::Result<(Self, Vec<OsString>)> {
-        use lexopt::prelude::*;
-
-        let mut parser = lexopt::Parser::from_args(args);
-        let mut op: Option<OperationName> = None;
-        let mut id: Option<NodeId> = None;
-        let mut name: Option<RefString> = None;
-        let mut list_op: ListOption = ListOption::default();
-        let mut fetch = true;
-        let mut sync = true;
-
-        while let Some(arg) = parser.next()? {
-            match arg {
-                Long("help") | Short('h') => {
-                    return Err(args::Error::Help.into());
-                }
-                Long("name") | Short('n') => {
-                    let value = parser.value()?;
-                    let value = args::refstring("name", value)?;
-
-                    name = Some(value);
-                }
-                Value(val) if op.is_none() => match val.to_string_lossy().as_ref() {
-                    "a" | "add" => op = Some(OperationName::Add),
-                    "l" | "list" => op = Some(OperationName::List),
-                    "r" | "rm" => op = Some(OperationName::Rm),
-                    unknown => anyhow::bail!("unknown operation '{}'", unknown),
-                },
-
-                // List options
-                Long("all") if op.unwrap_or_default() == OperationName::List => {
-                    list_op = ListOption::All;
-                }
-                Long("tracked") if op.unwrap_or_default() == OperationName::List => {
-                    list_op = ListOption::Tracked;
-                }
-                Long("untracked") if op.unwrap_or_default() == OperationName::List => {
-                    list_op = ListOption::Untracked;
-                }
-
-                // Add options
-                Long("sync") if op == Some(OperationName::Add) => {
-                    sync = true;
-                }
-                Long("no-sync") if op == Some(OperationName::Add) => {
-                    sync = false;
-                }
-                Long("fetch") if op == Some(OperationName::Add) => {
-                    fetch = true;
-                }
-                Long("no-fetch") if op == Some(OperationName::Add) => {
-                    fetch = false;
-                }
-                Value(val) if op == Some(OperationName::Add) && id.is_none() => {
-                    let nid = args::pubkey(&val)?;
-                    id = Some(nid);
-                }
-
-                // Remove options
-                Value(val) if op == Some(OperationName::Rm) && name.is_none() => {
-                    let val = args::string(&val);
-                    let val = RefString::try_from(val)
-                        .map_err(|e| anyhow!("invalid remote name specified: {e}"))?;
-
-                    name = Some(val);
-                }
-                _ => anyhow::bail!(arg.unexpected()),
-            }
-        }
-
-        let op = match op.unwrap_or_default() {
-            OperationName::Add => Operation::Add {
-                id: id.ok_or(anyhow!(
-                    "`DID` required, try running `rad remote add <did>`"
-                ))?,
-                name,
-                fetch,
-                sync,
-            },
-            OperationName::List => Operation::List { option: list_op },
-            OperationName::Rm => Operation::Rm {
-                name: name.ok_or(anyhow!("name required, see `rad remote`"))?,
-            },
-        };
-
-        Ok((Options { op }, vec![]))
-    }
-}
-
-pub fn run(options: Options, ctx: impl Context) -> anyhow::Result<()> {
+pub fn run(args: Args, ctx: impl Context) -> anyhow::Result<()> {
     let (working, rid) = radicle::rad::cwd()
         .map_err(|_| anyhow!("this command must be run in the context of a repository"))?;
     let profile = ctx.profile()?;
-
-    match options.op {
-        Operation::Add {
-            ref id,
+    let command = args
+        .command
+        .unwrap_or_else(|| Command::List(args.empty.into()));
+    match command {
+        Command::Add {
+            nid,
             name,
             fetch,
             sync,
         } => {
             let proj = profile.storage.repository(rid)?.project()?;
             let branch = proj.default_branch();
-
             self::add::run(
                 rid,
-                id,
+                &nid,
                 name,
                 Some(branch.clone()),
                 &profile,
                 &working,
-                fetch,
-                sync,
+                fetch.should_fetch(),
+                sync.should_sync(),
             )?
         }
-        Operation::Rm { ref name } => self::rm::run(name, &working)?,
-        Operation::List { option } => match option {
+        Command::Rm { ref name } => self::rm::run(name, &working)?,
+        Command::List(args) => match ListOption::from(args) {
             ListOption::All => {
                 let tracked = list::tracked(&working)?;
                 let untracked = list::untracked(rid, &profile, tracked.iter())?;
diff --git a/crates/radicle-cli/src/commands/remote/args.rs b/crates/radicle-cli/src/commands/remote/args.rs
new file mode 100644
index 00000000..69871851
--- /dev/null
+++ b/crates/radicle-cli/src/commands/remote/args.rs
@@ -0,0 +1,161 @@
+use clap::{Parser, Subcommand};
+
+use radicle::git;
+use radicle::git::fmt::RefString;
+use radicle::node::NodeId;
+
+use crate::terminal as term;
+
+pub(crate) const ABOUT: &str = "Manage a repository's remotes";
+
+#[derive(Parser, Debug)]
+#[command(about = ABOUT, disable_version_flag = true)]
+pub struct Args {
+    #[command(subcommand)]
+    pub(super) command: Option<Command>,
+
+    /// Arguments for the empty subcommand.
+    /// Will fall back to [`Command::List`].
+    #[clap(flatten)]
+    pub(super) empty: EmptyArgs,
+}
+
+#[derive(Subcommand, Debug)]
+pub(super) enum Command {
+    /// Add a Git remote for the provided NID
+    #[clap(alias = "a")]
+    Add {
+        /// The DID or NID of the remote to add
+        #[arg(value_parser = term::args::parse_nid)]
+        nid: NodeId,
+
+        /// Override the name of the Git remote
+        ///
+        /// [default: <ALIAS>@<NID>]
+        #[arg(long, short, value_name = "REMOTE", value_parser = parse_refstr)]
+        name: Option<RefString>,
+
+        #[clap(flatten)]
+        fetch: FetchArgs,
+
+        #[clap(flatten)]
+        sync: SyncArgs,
+    },
+    /// Remove the Git remote identified by REMOTE
+    #[clap(alias = "r")]
+    Rm {
+        /// The name of the remote to delete
+        #[arg(value_name = "REMOTE", value_parser = parse_refstr)]
+        name: RefString,
+    },
+    /// List the stored remotes
+    ///
+    /// Filter the listed remotes using the provided options
+    #[clap(alias = "l")]
+    List(ListArgs),
+}
+
+#[derive(Parser, Debug)]
+pub(super) struct FetchArgs {
+    /// Fetch the remote from local storage (default)
+    #[arg(long, conflicts_with = "no_fetch")]
+    fetch: bool,
+
+    /// Do not fetch the remote from local storage
+    #[arg(long)]
+    no_fetch: bool,
+}
+
+impl FetchArgs {
+    pub(super) fn should_fetch(&self) -> bool {
+        let Self { fetch, no_fetch } = self;
+        *fetch || !no_fetch
+    }
+}
+
+#[derive(Parser, Debug)]
+pub(super) struct SyncArgs {
+    /// Sync the remote refs from the network (default)
+    #[arg(long, conflicts_with = "no_sync")]
+    sync: bool,
+
+    /// Do not sync the remote refs from the network
+    #[arg(long)]
+    no_sync: bool,
+}
+
+impl SyncArgs {
+    pub(super) fn should_sync(&self) -> bool {
+        let Self { sync, no_sync } = self;
+        *sync || !no_sync
+    }
+}
+
+#[derive(Parser, Clone, Copy, Debug)]
+#[group(multiple = false)]
+pub struct ListArgs {
+    /// Show all remotes in both the Radicle storage and the working copy
+    #[arg(long)]
+    all: bool,
+
+    /// Show all remotes that are listed in the working copy
+    #[arg(long)]
+    tracked: bool,
+
+    /// Show all remotes that are listed in the Radicle storage
+    #[arg(long)]
+    untracked: bool,
+}
+
+impl From<ListArgs> for ListOption {
+    fn from(
+        ListArgs {
+            all,
+            tracked,
+            untracked,
+        }: ListArgs,
+    ) -> Self {
+        match (all, tracked, untracked) {
+            (true, false, false) => Self::All,
+            (false, true, false) | (false, false, false) => Self::Tracked,
+            (false, false, true) => Self::Untracked,
+            _ => unreachable!(),
+        }
+    }
+}
+
+pub(super) enum ListOption {
+    /// Show all remotes in both the Radicle storage and the working copy
+    All,
+    /// Show all remotes that are listed in the working copy
+    Tracked,
+    /// Show all remotes that are listed in the Radicle storage
+    Untracked,
+}
+
+#[derive(Parser, Clone, Copy, Debug)]
+#[group(multiple = false)]
+pub(super) struct EmptyArgs {
+    #[arg(long, hide = true)]
+    all: bool,
+
+    #[arg(long, hide = true)]
+    tracked: bool,
+
+    #[arg(long, hide = true)]
+    untracked: bool,
+}
+
+impl From<EmptyArgs> for ListArgs {
+    fn from(args: EmptyArgs) -> Self {
+        Self {
+            all: args.all,
+            tracked: args.tracked,
+            untracked: args.untracked,
+        }
+    }
+}
+
+fn parse_refstr(refstr: &str) -> Result<RefString, git::fmt::Error> {
+    RefString::try_from(refstr)
+}
diff --git a/crates/radicle-cli/src/main.rs b/crates/radicle-cli/src/main.rs
index b3520c35..5b61332f 100644
--- a/crates/radicle-cli/src/main.rs
+++ b/crates/radicle-cli/src/main.rs
@@ -74,6 +74,7 @@ enum Commands {
     Ls(ls::Args),
     Path(path::Args),
     Publish(publish::Args),
+    Remote(remote::Args),
     Seed(seed::Args),
     #[command(name = "self")]
     RadSelf(rad_self::Args),
@@ -340,7 +341,9 @@ pub(crate) fn run_other(exe: &str, args: &[OsString]) -> Result<(), Option<anyho
             }
         }
         "remote" => {
-            term::run_command_args::<remote::Options, _>(remote::HELP, remote::run, args.to_vec())
+            if let Some(Commands::Remote(args)) = CliArgs::parse().command {
+                term::run_command_fn(remote::run, args);
+            }
         }
         "stats" => {
             if let Some(Commands::Stats(args)) = CliArgs::parse().command {