cli/node: Use clap

Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>

crates/radicle-cli/src/commands/help.rs

@@ -84,7 +84,10 @@ const COMMANDS: &[CommandItem] = &[
         name: "ls",
         about: crate::commands::ls::ABOUT,
     },
-    CommandItem::Lexopt(crate::commands::node::HELP),
+    CommandItem::Clap {
+        name: "node",
+        about: crate::commands::node::ABOUT,
+    },
     CommandItem::Lexopt(crate::commands::patch::HELP),
     CommandItem::Clap {
         name: "path",

crates/radicle-cli/src/commands/node.rs

@@ -1,310 +1,52 @@
-use std::ffi::OsString;
+mod args;
-use std::path::PathBuf;
-use std::str::FromStr;
-use std::{process, time};
-
-use anyhow::anyhow;
-
-use radicle::node::address::Store as AddressStore;
-use radicle::node::config::ConnectAddress;
-use radicle::node::routing::Store;
-use radicle::node::Handle as _;
-use radicle::node::{Address, Node, NodeId, PeerAddr};
-use radicle::prelude::RepoId;
-
-use crate::terminal as term;
-use crate::terminal::args::{Args, Error, Help};
-use crate::terminal::Element as _;
-
 mod commands;
 pub mod control;
 mod events;
 mod logs;
 pub mod routing;
 
-pub const HELP: Help = Help {
+use std::{process, time};
-    name: "node",
-    description: "Control and query the Radicle Node",
-    version: env!("RADICLE_VERSION"),
-    usage: r#"
-Usage
 
-    rad node status [<option>...]
+use radicle::node::address::Store as AddressStore;
-    rad node start [--foreground] [--verbose] [<option>...] [-- <node-option>...]
+use radicle::node::config::ConnectAddress;
-    rad node stop [<option>...]
+use radicle::node::routing::Store;
-    rad node logs [-n <lines>]
+use radicle::node::Handle as _;
-    rad node debug [<option>...]
+use radicle::node::Node;
-    rad node connect <nid>[@<addr>] [<option>...]
-    rad node routing [--rid <rid>] [--nid <nid>] [--json] [<option>...]
-    rad node inventory [--nid <nid>] [<option>...]
-    rad node events [--timeout <secs>] [-n <count>] [<option>...]
-    rad node config [--addresses]
-    rad node db <command> [<option>..]
 
-    For `<node-option>` see `radicle-node --help`.
+use crate::commands::node::args::Only;
+use crate::terminal as term;
+use crate::terminal::Element as _;
+use crate::warning;
 
-Start options
+pub use args::Args;
+pub(crate) use args::ABOUT;
+use args::{Addr, Command};
 
-    --foreground         Start the node in the foreground
+pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
-    --path <path>        Start node binary at path (default: radicle-node)
-    --verbose, -v        Verbose output
-
-Routing options
-
-    --rid <rid>          Show the routing table entries for the given RID
-    --nid <nid>          Show the routing table entries for the given NID
-    --json               Output the routing table as json
-
-Inventory options
-
-    --nid <nid>          List the inventory of the given NID (default: self)
-
-Events options
-
-    --timeout <secs>     How long to wait to receive an event before giving up
-    --count, -n <count>  Exit after <count> events
-
-Status options
-
-    --only nid           If node is running, only print the Node ID and exit,
-                         otherwise exit with a non-zero exit status.
-
-General options
-
-    --help               Print help
-"#,
-};
-
-pub struct Options {
-    op: Operation,
-}
-
-/// Address used for the [`Operation::Connect`]
-pub enum Addr {
-    /// Fully-specified address of the form `<NID>@<Address>`
-    Peer(PeerAddr<NodeId, Address>),
-    /// Just the `NID`, to be used for address lookups.
-    Node(NodeId),
-}
-
-impl FromStr for Addr {
-    type Err = anyhow::Error;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if s.contains("@") {
-            PeerAddr::from_str(s)
-                .map(Self::Peer)
-                .map_err(|e| anyhow!("expected <nid> or <nid>@<addr>: {e}"))
-        } else {
-            NodeId::from_str(s)
-                .map(Self::Node)
-                .map_err(|e| anyhow!("expected <nid> or <nid>@<addr>: {e}"))
-        }
-    }
-}
-
-pub enum Operation {
-    Connect {
-        addr: Addr,
-        timeout: time::Duration,
-    },
-    Config {
-        addresses: bool,
-    },
-    Db {
-        args: Vec<OsString>,
-    },
-    Events {
-        timeout: time::Duration,
-        count: usize,
-    },
-    Routing {
-        json: bool,
-        rid: Option<RepoId>,
-        nid: Option<NodeId>,
-    },
-    Start {
-        foreground: bool,
-        verbose: bool,
-        path: PathBuf,
-        options: Vec<OsString>,
-    },
-    Logs {
-        lines: usize,
-    },
-    Status {
-        only_nid: bool,
-    },
-    Inventory {
-        nid: Option<NodeId>,
-    },
-    Debug,
-    Sessions,
-    Stop,
-}
-
-#[derive(Default, PartialEq, Eq)]
-pub enum OperationName {
-    Connect,
-    Config,
-    Db,
-    Events,
-    Routing,
-    Logs,
-    Start,
-    #[default]
-    Status,
-    Inventory,
-    Debug,
-    Sessions,
-    Stop,
-}
-
-impl Args for Options {
-    fn from_args(args: Vec<OsString>) -> anyhow::Result<(Self, Vec<OsString>)> {
-        use lexopt::prelude::*;
-
-        let mut foreground = false;
-        let mut options = vec![];
-        let mut parser = lexopt::Parser::from_args(args);
-        let mut op: Option<OperationName> = None;
-        let mut nid: Option<NodeId> = None;
-        let mut rid: Option<RepoId> = None;
-        let mut json: bool = false;
-        let mut addr: Option<Addr> = None;
-        let mut lines: usize = 60;
-        let mut count: usize = usize::MAX;
-        let mut timeout = time::Duration::MAX;
-        let mut addresses = false;
-        let mut path = None;
-        let mut verbose = false;
-        let mut only_nid = false;
-
-        while let Some(arg) = parser.next()? {
-            match arg {
-                Long("help") | Short('h') => {
-                    return Err(Error::Help.into());
-                }
-                Value(val) if op.is_none() => match val.to_string_lossy().as_ref() {
-                    "connect" => op = Some(OperationName::Connect),
-                    "db" => op = Some(OperationName::Db),
-                    "events" => op = Some(OperationName::Events),
-                    "logs" => op = Some(OperationName::Logs),
-                    "config" => op = Some(OperationName::Config),
-                    "routing" => op = Some(OperationName::Routing),
-                    "inventory" => op = Some(OperationName::Inventory),
-                    "start" => op = Some(OperationName::Start),
-                    "status" => op = Some(OperationName::Status),
-                    "stop" => op = Some(OperationName::Stop),
-                    "sessions" => op = Some(OperationName::Sessions),
-                    "debug" => op = Some(OperationName::Debug),
-
-                    unknown => anyhow::bail!("unknown operation '{}'", unknown),
-                },
-                Value(val) if matches!(op, Some(OperationName::Connect)) => {
-                    addr = Some(val.parse()?);
-                }
-                Long("rid") if matches!(op, Some(OperationName::Routing)) => {
-                    let val = parser.value()?;
-                    rid = term::args::rid(&val).ok();
-                }
-                Long("nid")
-                    if matches!(op, Some(OperationName::Routing))
-                        || matches!(op, Some(OperationName::Inventory)) =>
-                {
-                    let val = parser.value()?;
-                    nid = term::args::nid(&val).ok();
-                }
-                Long("only") if matches!(op, Some(OperationName::Status)) => {
-                    if &parser.value()? == "nid" {
-                        only_nid = true;
-                    } else {
-                        anyhow::bail!("unknown argument to --only");
-                    }
-                }
-                Long("json") if matches!(op, Some(OperationName::Routing)) => json = true,
-                Long("timeout")
-                    if op == Some(OperationName::Events) || op == Some(OperationName::Connect) =>
-                {
-                    let val = parser.value()?;
-                    timeout = term::args::seconds(&val)?;
-                }
-                Long("count") | Short('n') if matches!(op, Some(OperationName::Events)) => {
-                    let val = parser.value()?;
-                    count = term::args::number(&val)?;
-                }
-                Long("foreground") if matches!(op, Some(OperationName::Start)) => {
-                    foreground = true;
-                }
-                Long("addresses") if matches!(op, Some(OperationName::Config)) => {
-                    addresses = true;
-                }
-                Long("verbose") | Short('v') if matches!(op, Some(OperationName::Start)) => {
-                    verbose = true;
-                }
-                Long("path") if matches!(op, Some(OperationName::Start)) => {
-                    let val = parser.value()?;
-                    path = Some(PathBuf::from(val));
-                }
-                Short('n') if matches!(op, Some(OperationName::Logs)) => {
-                    lines = parser.value()?.parse()?;
-                }
-                Value(val) if matches!(op, Some(OperationName::Start)) => {
-                    options.push(val);
-                }
-                Value(val) if matches!(op, Some(OperationName::Db)) => {
-                    options.push(val);
-                }
-                _ => return Err(anyhow!(arg.unexpected())),
-            }
-        }
-
-        let op = match op.unwrap_or_default() {
-            OperationName::Connect => Operation::Connect {
-                addr: addr.ok_or_else(|| {
-                    anyhow!("an `<nid>` or an address of the form `<nid>@<host>:<port>` must be provided")
-                })?,
-                timeout,
-            },
-            OperationName::Config => Operation::Config { addresses },
-            OperationName::Db => Operation::Db { args: options },
-            OperationName::Events => Operation::Events { timeout, count },
-            OperationName::Routing => Operation::Routing { rid, nid, json },
-            OperationName::Logs => Operation::Logs { lines },
-            OperationName::Start => Operation::Start {
-                foreground,
-                verbose,
-                options,
-                path: path.unwrap_or(PathBuf::from("radicle-node")),
-            },
-            OperationName::Inventory => Operation::Inventory { nid },
-            OperationName::Status => Operation::Status { only_nid },
-            OperationName::Debug => Operation::Debug,
-            OperationName::Sessions => Operation::Sessions,
-            OperationName::Stop => Operation::Stop,
-        };
-        Ok((Options { op }, vec![]))
-    }
-}
-
-pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
     let mut node = Node::new(profile.socket());
 
-    match options.op {
+    let command = args.command.unwrap_or_default();
-        Operation::Connect { addr, timeout } => match addr {
+
-            Addr::Peer(addr) => control::connect(&mut node, addr.id, addr.addr, timeout)?,
+    match command {
-            Addr::Node(nid) => {
+        Command::Connect { addr, timeout } => {
-                let db = profile.database()?;
+            let timeout = timeout
-                let addresses = db
+                .map(time::Duration::from_secs)
-                    .addresses_of(&nid)?
+                .unwrap_or(time::Duration::MAX);
-                    .into_iter()
+            match addr {
-                    .map(|ka| ka.addr)
+                Addr::Peer(addr) => control::connect(&mut node, addr.id, addr.addr, timeout)?,
-                    .collect();
+                Addr::Node(nid) => {
-                control::connect_many(&mut node, nid, addresses, timeout)?;
+                    let db = profile.database()?;
+                    let addresses = db
+                        .addresses_of(&nid)?
+                        .into_iter()
+                        .map(|ka| ka.addr)
+                        .collect();
+                    control::connect_many(&mut node, nid, addresses, timeout)?;
+                }
             }
-        },
+        }
-        Operation::Config { addresses } => {
+        Command::Config { addresses } => {
             if addresses {
                 let cfg = node.config()?;
                 for addr in cfg.external_addresses {
@@ -314,27 +56,33 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
                 control::config(&node)?;
             }
         }
-        Operation::Db { args } => {
+        Command::Db(op) => {
-            commands::db(&profile, args)?;
+            commands::db(&profile, op)?;
         }
-        Operation::Debug => {
+        Command::Debug => {
             control::debug(&mut node)?;
         }
-        Operation::Sessions => {
+        Command::Sessions => {
+            warning::deprecated("rad node sessions", "rad node status");
             let sessions = control::sessions(&node)?;
             if let Some(table) = sessions {
                 table.print();
             }
         }
-        Operation::Events { timeout, count } => {
+        Command::Events { timeout, count } => {
+            let count = count.unwrap_or(usize::MAX);
+            let timeout = timeout
+                .map(time::Duration::from_secs)
+                .unwrap_or(time::Duration::MAX);
+
             events::run(node, count, timeout)?;
         }
-        Operation::Routing { rid, nid, json } => {
+        Command::Routing { rid, nid, json } => {
             let store = profile.database()?;
             routing::run(&store, rid, nid, json)?;
         }
-        Operation::Logs { lines } => control::logs(lines, Some(time::Duration::MAX), &profile)?,
+        Command::Logs { lines } => control::logs(lines, Some(time::Duration::MAX), &profile)?,
-        Operation::Start {
+        Command::Start {
             foreground,
             options,
             path,
@@ -342,23 +90,25 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
         } => {
             control::start(node, !foreground, verbose, options, &path, &profile)?;
         }
-        Operation::Inventory { nid } => {
+        Command::Inventory { nid } => {
             let nid = nid.as_ref().unwrap_or(profile.id());
             for rid in profile.routing()?.get_inventory(nid)? {
                 println!("{}", term::format::tertiary(rid));
             }
         }
-        Operation::Status { only_nid: false } => {
+        Command::Status {
-            control::status(&node, &profile)?;
+            only: Some(Only::Nid),
-        }
+        } => {
-        Operation::Status { only_nid: true } => {
             if node.is_running() {
                 term::print(term::format::node_id_human(&node.nid()?));
             } else {
                 process::exit(2);
             }
         }
-        Operation::Stop => {
+        Command::Status { only: None } => {
+            control::status(&node, &profile)?;
+        }
+        Command::Stop => {
             control::stop(node, &profile);
         }
     }

crates/radicle-cli/src/commands/node/args.rs

@@ -0,0 +1,231 @@
+use std::ffi::OsString;
+use std::fmt::Debug;
+use std::path::PathBuf;
+use std::str::FromStr;
+
+use thiserror::Error;
+
+use clap::{Parser, Subcommand};
+
+use radicle::crypto::{PublicKey, PublicKeyError};
+use radicle::node::{Address, NodeId, PeerAddr, PeerAddrParseError};
+use radicle::prelude::RepoId;
+
+pub(crate) const ABOUT: &str = "Control and query the Radicle Node";
+
+#[derive(Parser, Debug)]
+#[command(about = ABOUT, long_about, disable_version_flag = true)]
+pub struct Args {
+    #[command(subcommand)]
+    pub(super) command: Option<Command>,
+}
+
+/// Address used for the [`Operation::Connect`]
+#[derive(Clone, Debug)]
+pub(super) enum Addr {
+    /// Fully-specified address of the form `<NID>@<ADDR>`
+    Peer(PeerAddr<NodeId, Address>),
+    /// Just the `NID`, to be used for address lookups.
+    Node(NodeId),
+}
+
+#[derive(Error, Debug)]
+pub(super) enum AddrParseError {
+    #[error("{0}, expected <NID> or <NID>@<ADDR>")]
+    PeerAddr(#[from] PeerAddrParseError<PublicKey>),
+    #[error(transparent)]
+    NodeId(#[from] PublicKeyError),
+}
+
+impl FromStr for Addr {
+    type Err = AddrParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.contains("@") {
+            PeerAddr::from_str(s)
+                .map(Self::Peer)
+                .map_err(AddrParseError::PeerAddr)
+        } else {
+            NodeId::from_str(s)
+                .map(Self::Node)
+                .map_err(AddrParseError::NodeId)
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub enum Only {
+    Nid,
+}
+
+#[derive(Error, Debug)]
+#[error("could not parse value `{0}`")]
+pub struct OnlyParseError(String);
+
+impl FromStr for Only {
+    type Err = OnlyParseError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        match value {
+            "nid" => Ok(Only::Nid),
+            _ => Err(OnlyParseError(value.to_string())),
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+struct OnlyParser;
+
+impl clap::builder::TypedValueParser for OnlyParser {
+    type Value = Only;
+
+    fn parse_ref(
+        &self,
+        cmd: &clap::Command,
+        arg: Option<&clap::Arg>,
+        value: &std::ffi::OsStr,
+    ) -> Result<Self::Value, clap::Error> {
+        <Only as std::str::FromStr>::from_str.parse_ref(cmd, arg, value)
+    }
+
+    fn possible_values(
+        &self,
+    ) -> Option<Box<dyn Iterator<Item = clap::builder::PossibleValue> + '_>> {
+        use clap::builder::PossibleValue;
+        Some(Box::new([PossibleValue::new("nid")].into_iter()))
+    }
+}
+
+#[derive(Subcommand, Debug)]
+pub(super) enum Command {
+    /// Instruct the node to connect to another node
+    Connect {
+        /// The Node ID, and optionally the address and port, of the node to connect to
+        #[arg(value_name = "NID[@ADDR]")]
+        addr: Addr,
+
+        /// How long to wait for the connection to be established
+        #[arg(long, value_name = "SECS")]
+        timeout: Option<u64>,
+    },
+
+    /// Show the config
+    Config {
+        /// Only show external addresses from the node's config
+        #[arg(long)]
+        addresses: bool,
+    },
+
+    /// Interact with the node database
+    #[command(subcommand, hide = true)]
+    Db(DbOperation),
+
+    /// Watch and print events.
+    ///
+    /// This command will connect to the node and print events to
+    /// standard output as they occur.
+    ///
+    /// If no timeout or count is specified, it will run indefinitely.
+    Events {
+        /// How long to wait to receive an event before giving up
+        #[arg(long, value_name = "SECS")]
+        timeout: Option<u64>,
+
+        /// Exit after <COUNT> events
+        #[arg(long, short = 'n')]
+        count: Option<usize>,
+    },
+
+    /// Show the routing table
+    Routing {
+        /// Output the routing table as json
+        #[arg(long)]
+        json: bool,
+
+        /// Show the routing table entries for the given RID
+        #[arg(long)]
+        rid: Option<RepoId>,
+
+        /// Show the routing table entries for the given NID
+        #[arg(long)]
+        nid: Option<NodeId>,
+    },
+
+    /// Start the node
+    Start {
+        /// Start the node in the foreground
+        #[arg(long)]
+        foreground: bool,
+
+        /// Verbose output
+        #[arg(long, short)]
+        verbose: bool,
+
+        /// Start node binary at path
+        #[arg(long, default_value = "radicle-node")]
+        path: PathBuf,
+
+        /// Additional options to pass to the binary
+        ///
+        /// See `radicle-node --help` for additional options
+        #[arg(value_name = "NODE_OPTIONS", last = true, num_args = 1..)]
+        options: Vec<OsString>,
+    },
+
+    /// Show the log
+    Logs {
+        /// Only show <COUNT> lines of the log
+        #[arg(long, value_name = "COUNT", default_value_t = 60)]
+        lines: usize,
+    },
+
+    /// Show the status
+    Status {
+        /// If node is running, only print the Node ID and exit, otherwise exit with a non-zero exit status.
+        #[arg(long, value_parser = OnlyParser)]
+        only: Option<Only>,
+    },
+
+    /// Manage the inventory
+    Inventory {
+        /// List the inventory of the given NID, defaults to `self`
+        #[arg(long)]
+        nid: Option<NodeId>,
+    },
+
+    /// Show debug information related to the running node.
+    ///
+    /// This includes metrics fetching, peer connections, rate limiting, etc.
+    Debug,
+
+    /// Show the active sessions of the running node.
+    ///
+    /// Deprecated, use `status` instead.
+    #[command(hide = true)]
+    Sessions,
+
+    /// Stop the node
+    Stop,
+}
+
+impl Default for Command {
+    fn default() -> Self {
+        Command::Status { only: None }
+    }
+}
+
+/// Operations related to the [`Command::Db`]
+#[derive(Debug, Subcommand)]
+pub(super) enum DbOperation {
+    /// Execute an SQL operation on the local node database.
+    ///
+    /// The command only returns the number of rows that are affected by the
+    /// query. This means that `SELECT` queries will not return their output.
+    ///
+    /// The command should only be used for executing queries given you know
+    /// what you are doing.
+    Exec {
+        #[arg(value_name = "SQL")]
+        query: String,
+    },
+}

crates/radicle-cli/src/commands/node/commands.rs

@@ -1,39 +1,11 @@
-use std::ffi::OsString;
-
-use anyhow::anyhow;
 use radicle::Profile;
 use radicle_term as term;
 
-#[derive(PartialEq, Eq)]
+use super::args::DbOperation;
-pub enum Operation {
-    Exec { query: String },
-}
 
-pub fn db(profile: &Profile, args: Vec<OsString>) -> anyhow::Result<()> {
+pub fn db(profile: &Profile, op: DbOperation) -> anyhow::Result<()> {
-    use lexopt::prelude::*;
+    match op {
-
+        DbOperation::Exec { query } => {
-    let mut parser = lexopt::Parser::from_args(args);
-    let mut op: Option<Operation> = None;
-
-    while let Some(arg) = parser.next()? {
-        match arg {
-            Value(cmd) if op.is_none() => match cmd.to_string_lossy().as_ref() {
-                "exec" => {
-                    let val = parser
-                        .value()
-                        .map_err(|_| anyhow!("a query to execute must be provided for `exec`"))?;
-                    op = Some(Operation::Exec {
-                        query: val.to_string_lossy().to_string(),
-                    });
-                }
-                unknown => anyhow::bail!("unknown operation '{unknown}'"),
-            },
-            _ => return Err(anyhow!(arg.unexpected())),
-        }
-    }
-
-    match op.ok_or_else(|| anyhow!("a command must be provided, eg. `rad node db exec`"))? {
-        Operation::Exec { query } => {
             let db = profile.database_mut()?;
             db.execute(query)?;
 

crates/radicle-cli/src/main.rs

@@ -72,6 +72,7 @@ enum Commands {
     Init(init::Args),
     Issue(issue::Args),
     Ls(ls::Args),
+    Node(node::Args),
     Path(path::Args),
     Publish(publish::Args),
     Remote(remote::Args),
@@ -292,7 +293,9 @@ pub(crate) fn run_other(exe: &str, args: &[OsString]) -> Result<(), Option<anyho
             }
         }
         "node" => {
-            term::run_command_args::<node::Options, _>(node::HELP, node::run, args.to_vec());
+            if let Some(Commands::Node(args)) = CliArgs::parse().command {
+                term::run_command_fn(node::run, args);
+            }
         }
         "patch" => {
             term::run_command_args::<patch::Options, _>(patch::HELP, patch::run, args.to_vec());