From 9bcdd353c90f42a159b19045d02671198670a05a Mon Sep 17 00:00:00 2001
From: Sebastian Martinez <me@sebastinez.dev>
Date: Mon, 20 Oct 2025 10:59:53 +0200
Subject: [PATCH] cli/inbox: Use clap

---
 crates/radicle-cli/src/commands/help.rs       |   5 +-
 crates/radicle-cli/src/commands/inbox.rs      | 295 +++++-------------
 crates/radicle-cli/src/commands/inbox/args.rs | 224 +++++++++++++
 crates/radicle-cli/src/main.rs                |   5 +-
 4 files changed, 315 insertions(+), 214 deletions(-)
 create mode 100644 crates/radicle-cli/src/commands/inbox/args.rs

diff --git a/crates/radicle-cli/src/commands/help.rs b/crates/radicle-cli/src/commands/help.rs
index bfcca935..4f806c2f 100644
--- a/crates/radicle-cli/src/commands/help.rs
+++ b/crates/radicle-cli/src/commands/help.rs
@@ -74,7 +74,10 @@ const COMMANDS: &[CommandItem] = &[
         name: "init",
         about: crate::commands::init::ABOUT,
     },
-    CommandItem::Lexopt(crate::commands::inbox::HELP),
+    CommandItem::Clap {
+        name: "inbox",
+        about: crate::commands::inbox::ABOUT,
+    },
     CommandItem::Clap {
         name: "inspect",
         about: crate::commands::inspect::ABOUT,
diff --git a/crates/radicle-cli/src/commands/inbox.rs b/crates/radicle-cli/src/commands/inbox.rs
index 81a7bd67..bd021518 100644
--- a/crates/radicle-cli/src/commands/inbox.rs
+++ b/crates/radicle-cli/src/commands/inbox.rs
@@ -1,4 +1,8 @@
-use std::ffi::OsString;
+mod args;
+
+pub use args::Args;
+pub(crate) use args::ABOUT;
+
 use std::path::Path;
 use std::process;
 
@@ -20,210 +24,77 @@ use radicle::{cob, git, Storage};
 use term::Element as _;
 
 use crate::terminal as term;
-use crate::terminal::args;
-use crate::terminal::args::{Args, Error, Help};
+use args::{ClearMode, Command, ListMode, SortBy};
 
-pub const HELP: Help = Help {
-    name: "inbox",
-    description: "Manage your Radicle notifications",
-    version: env!("RADICLE_VERSION"),
-    usage: r#"
-Usage
-
-    rad inbox [<option>...]
-    rad inbox list [<option>...]
-    rad inbox show <id> [<option>...]
-    rad inbox clear <id...> [<option>...]
-
-    By default, this command lists all items in your inbox.
-    If your working directory is a Radicle repository, it only shows item
-    belonging to this repository, unless `--all` is used.
-
-    The `rad inbox show` command takes a notification ID (which can be found in
-    the `list` command) and displays the information related to that
-    notification. This will mark the notification as read.
-
-    The `rad inbox clear` command will delete all notifications by their passed id
-    or all notifications if no ids were passed.
-
-Options
-
-    --all                Operate on all repositories
-    --repo <rid>         Operate on the given repository (default: rad .)
-    --sort-by <field>    Sort by `id` or `timestamp` (default: timestamp)
-    --reverse, -r        Reverse the list
-    --show-unknown       Show any updates that were not recognized
-    --help               Print help
-"#,
-};
-
-#[derive(Debug, Default, PartialEq, Eq)]
-enum Operation {
-    #[default]
-    List,
-    Show,
-    Clear,
-}
-
-#[derive(Default, Debug)]
-enum Mode {
-    #[default]
-    Contextual,
-    All,
-    ById(Vec<NotificationId>),
-    ByRepo(RepoId),
-}
-
-#[derive(Clone, Copy, Debug)]
-struct SortBy {
-    reverse: bool,
-    field: &'static str,
-}
-
-pub struct Options {
-    op: Operation,
-    mode: Mode,
-    sort_by: SortBy,
-    show_unknown: bool,
-}
-
-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<Operation> = None;
-        let mut mode = None;
-        let mut ids = Vec::new();
-        let mut reverse = None;
-        let mut field = None;
-        let mut show_unknown = false;
-
-        while let Some(arg) = parser.next()? {
-            match arg {
-                Long("help") | Short('h') => {
-                    return Err(Error::Help.into());
-                }
-                Long("all") | Short('a') if mode.is_none() => {
-                    mode = Some(Mode::All);
-                }
-                Long("reverse") | Short('r') => {
-                    reverse = Some(true);
-                }
-                Long("show-unknown") => {
-                    show_unknown = true;
-                }
-                Long("sort-by") => {
-                    let val = parser.value()?;
-
-                    match term::args::string(&val).as_str() {
-                        "timestamp" => field = Some("timestamp"),
-                        "id" => field = Some("rowid"),
-                        other => {
-                            return Err(anyhow!(
-                                "unknown sorting field `{other}`, see `rad inbox --help`"
-                            ))
-                        }
-                    }
-                }
-                Long("repo") if mode.is_none() => {
-                    let val = parser.value()?;
-                    let repo = args::rid(&val)?;
-
-                    mode = Some(Mode::ByRepo(repo));
-                }
-                Value(val) if op.is_none() => match val.to_string_lossy().as_ref() {
-                    "list" => op = Some(Operation::List),
-                    "show" => op = Some(Operation::Show),
-                    "clear" => op = Some(Operation::Clear),
-                    cmd => return Err(anyhow!("unknown command `{cmd}`, see `rad inbox --help`")),
-                },
-                Value(val) if op.is_some() && mode.is_none() => {
-                    let id = term::args::number(&val)? as NotificationId;
-                    ids.push(id);
-                }
-                _ => anyhow::bail!(arg.unexpected()),
-            }
-        }
-        let mode = if ids.is_empty() {
-            mode.unwrap_or_default()
-        } else {
-            Mode::ById(ids)
-        };
-        let op = op.unwrap_or_default();
-
-        let sort_by = if let Some(field) = field {
-            SortBy {
-                field,
-                reverse: reverse.unwrap_or(false),
-            }
-        } else {
-            SortBy {
-                field: "timestamp",
-                reverse: true,
-            }
-        };
-
-        Ok((
-            Options {
-                op,
-                mode,
-                sort_by,
-                show_unknown,
-            },
-            vec![],
-        ))
-    }
-}
-
-pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
+pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
     let profile = ctx.profile()?;
     let storage = &profile.storage;
     let mut notifs = profile.notifications_mut()?;
-    let Options {
-        op,
-        mode,
-        sort_by,
-        show_unknown,
-    } = options;
+    let command = args
+        .clone()
+        .command
+        .unwrap_or_else(|| Command::List(args.empty.into()));
 
-    match op {
-        Operation::List => list(
-            mode,
-            sort_by,
-            show_unknown,
-            &notifs.read_only(),
-            storage,
-            &profile,
-        ),
-        Operation::Clear => clear(mode, &mut notifs),
-        Operation::Show => show(mode, &mut notifs, storage, &profile),
+    match command {
+        Command::List(args) => {
+            let show_unknown = args.show_unknown;
+            let sort_by = args.sort_by;
+            let reverse = args.reverse;
+
+            list(
+                &notifs.read_only(),
+                args.into(),
+                sort_by,
+                reverse,
+                show_unknown,
+                storage,
+                &profile,
+            )
+        }
+        Command::Clear(args) => clear(&mut notifs, args.into()),
+        Command::Show { id } => show(&mut notifs, id, storage, &profile),
     }
 }
 
 fn list(
-    mode: Mode,
-    sort_by: SortBy,
-    show_unknown: bool,
     notifs: &notifications::StoreReader,
+    mode: ListMode,
+    sort_by: SortBy,
+    reverse: bool,
+    show_unknown: bool,
     storage: &Storage,
     profile: &Profile,
 ) -> anyhow::Result<()> {
     let repos: Vec<term::VStack<'_>> = match mode {
-        Mode::Contextual => {
+        ListMode::Contextual => {
             if let Ok((_, rid)) = radicle::rad::cwd() {
-                list_repo(rid, sort_by, show_unknown, notifs, storage, profile)?
-                    .into_iter()
-                    .collect()
+                list_repo(
+                    notifs,
+                    rid,
+                    sort_by,
+                    reverse,
+                    show_unknown,
+                    storage,
+                    profile,
+                )?
+                .into_iter()
+                .collect()
             } else {
-                list_all(sort_by, show_unknown, notifs, storage, profile)?
+                list_all(notifs, sort_by, reverse, show_unknown, storage, profile)?
             }
         }
-        Mode::ByRepo(rid) => list_repo(rid, sort_by, show_unknown, notifs, storage, profile)?
-            .into_iter()
-            .collect(),
-        Mode::All => list_all(sort_by, show_unknown, notifs, storage, profile)?,
-        Mode::ById(_) => anyhow::bail!("the `list` command does not take IDs"),
+        ListMode::All => list_all(notifs, sort_by, reverse, show_unknown, storage, profile)?,
+        ListMode::ByRepo(rid) => list_repo(
+            notifs,
+            rid,
+            sort_by,
+            reverse,
+            show_unknown,
+            storage,
+            profile,
+        )?
+        .into_iter()
+        .collect(),
     };
 
     if repos.is_empty() {
@@ -237,9 +108,10 @@ fn list(
 }
 
 fn list_all<'a>(
-    sort_by: SortBy,
-    show_unknown: bool,
     notifs: &notifications::StoreReader,
+    sort_by: SortBy,
+    reverse: bool,
+    show_unknown: bool,
     storage: &Storage,
     profile: &Profile,
 ) -> anyhow::Result<Vec<term::VStack<'a>>> {
@@ -248,17 +120,26 @@ fn list_all<'a>(
 
     let mut vstacks = Vec::new();
     for repo in repos {
-        let vstack = list_repo(repo.rid, sort_by, show_unknown, notifs, storage, profile)?;
+        let vstack = list_repo(
+            notifs,
+            repo.rid,
+            sort_by,
+            reverse,
+            show_unknown,
+            storage,
+            profile,
+        )?;
         vstacks.extend(vstack.into_iter());
     }
     Ok(vstacks)
 }
 
 fn list_repo<'a, R: ReadStorage>(
+    notifs: &notifications::StoreReader,
     rid: RepoId,
     sort_by: SortBy,
+    reverse: bool,
     show_unknown: bool,
-    notifs: &notifications::StoreReader,
     storage: &R,
     profile: &Profile,
 ) -> anyhow::Result<Option<term::VStack<'a>>>
@@ -272,8 +153,10 @@ where
     let issues = term::cob::issues(profile, &repo)?;
     let patches = term::cob::patches(profile, &repo)?;
 
-    let mut notifs = notifs.by_repo(&rid, sort_by.field)?.collect::<Vec<_>>();
-    if !sort_by.reverse {
+    let mut notifs = notifs
+        .by_repo(&rid, &sort_by.to_string())?
+        .collect::<Vec<_>>();
+    if !reverse {
         // Notifications are returned in descendant order by default.
         notifs.reverse();
     }
@@ -503,20 +386,16 @@ impl NotificationRow {
     }
 }
 
-fn clear(mode: Mode, notifs: &mut notifications::StoreWriter) -> anyhow::Result<()> {
+fn clear(notifs: &mut notifications::StoreWriter, mode: ClearMode) -> anyhow::Result<()> {
     let cleared = match mode {
-        Mode::All => notifs.clear_all()?,
-        Mode::ById(ids) => notifs.clear(&ids)?,
-        Mode::ByRepo(rid) => notifs.clear_by_repo(&rid)?,
-        Mode::Contextual => {
+        ClearMode::ByNotifications(ids) => notifs.clear(&ids)?,
+        ClearMode::ByRepo(rid) => notifs.clear_by_repo(&rid)?,
+        ClearMode::All => notifs.clear_all()?,
+        ClearMode::Contextual => {
             if let Ok((_, rid)) = radicle::rad::cwd() {
                 notifs.clear_by_repo(&rid)?
             } else {
-                return Err(Error::WithHint {
-                    err: anyhow!("not a radicle repository"),
-                    hint: "to clear all repository notifications, use the `--all` flag",
-                }
-                .into());
+                return Err(anyhow!("not a radicle repository"));
             }
         }
     };
@@ -529,19 +408,11 @@ fn clear(mode: Mode, notifs: &mut notifications::StoreWriter) -> anyhow::Result<
 }
 
 fn show(
-    mode: Mode,
     notifs: &mut notifications::StoreWriter,
+    id: NotificationId,
     storage: &Storage,
     profile: &Profile,
 ) -> anyhow::Result<()> {
-    let id = match mode {
-        Mode::ById(ids) => match ids.as_slice() {
-            [id] => *id,
-            [] => anyhow::bail!("a Notification ID must be given"),
-            _ => anyhow::bail!("too many Notification IDs given"),
-        },
-        _ => anyhow::bail!("a Notification ID must be given"),
-    };
     let n = notifs.get(id)?;
     let repo = storage.repository(n.repo)?;
 
diff --git a/crates/radicle-cli/src/commands/inbox/args.rs b/crates/radicle-cli/src/commands/inbox/args.rs
new file mode 100644
index 00000000..b45bab36
--- /dev/null
+++ b/crates/radicle-cli/src/commands/inbox/args.rs
@@ -0,0 +1,224 @@
+use std::{fmt::Display, str::FromStr};
+
+use clap::{Parser, Subcommand, ValueEnum};
+use radicle::{node::notifications::NotificationId, prelude::RepoId};
+
+pub(crate) const ABOUT: &str = "Manage your Radicle notifications";
+
+const LONG_ABOUT: &str = r#"
+By default, this command lists all items in your inbox.
+If your working directory is a Radicle repository, it only shows items
+belonging to this repository, unless `--all` is used.
+
+The `show` subcommand takes a notification ID (which can be found in
+the output of the `list` subcommand) and displays the information related to that
+notification. This will mark the notification as read.
+
+The `clear` subcommand will clear all notifications with given IDs,
+or all notifications if no IDs are given. Cleared notifications are
+deleted and cannot be restored.
+"#;
+
+#[derive(Clone, Debug, Parser)]
+#[command(about = ABOUT, long_about = LONG_ABOUT, disable_version_flag = true)]
+pub struct Args {
+    #[command(subcommand)]
+    pub(super) command: Option<Command>,
+
+    #[clap(flatten)]
+    pub(super) empty: EmptyArgs,
+}
+
+#[derive(Subcommand, Clone, Debug)]
+pub(super) enum Command {
+    /// List all items in your inbox
+    List(ListArgs),
+    /// Show a notification
+    ///
+    /// The NOTIFICATION_ID can be found by listing the items in your inbox
+    ///
+    /// Showing a notification will mark that notification as read
+    Show {
+        /// The notification to display
+        #[arg(value_name = "NOTIFICATION_ID")]
+        id: NotificationId,
+    },
+    /// Clear notifications
+    ///
+    /// This will clear all given notifications
+    ///
+    /// If no notifications are specified then all notifications are cleared
+    Clear(ClearArgs),
+}
+
+#[derive(Parser, Clone, Copy, Debug)]
+pub(super) struct EmptyArgs {
+    /// Sort by column
+    #[arg(long, value_enum, default_value_t, hide = true)]
+    sort_by: SortBy,
+
+    /// Reverse the list
+    #[arg(short, long, hide = true)]
+    reverse: bool,
+
+    /// Show any updates that were not recognized
+    #[arg(long, hide = true)]
+    show_unknown: bool,
+
+    /// Operate on a given repository [default: cwd]
+    #[arg(value_name = "RID")]
+    #[arg(long, hide = true)]
+    repo: Option<RepoId>,
+
+    /// Operate on all repositories
+    #[arg(short, long, conflicts_with = "repo", hide = true)]
+    all: bool,
+}
+
+#[derive(Parser, Clone, Copy, Debug)]
+pub(super) struct ListArgs {
+    /// Sort by column
+    #[arg(long, value_enum, default_value_t)]
+    pub(super) sort_by: SortBy,
+
+    /// Reverse the list
+    #[arg(short, long)]
+    pub(super) reverse: bool,
+
+    /// Show any updates that were not recognized
+    #[arg(long)]
+    pub(super) show_unknown: bool,
+
+    /// Operate on a given repository [default: cwd]
+    #[arg(long, value_name = "RID")]
+    pub(super) repo: Option<RepoId>,
+
+    /// Operate on all repositories
+    #[arg(short, long, conflicts_with = "repo")]
+    pub(super) all: bool,
+}
+
+impl From<ListArgs> for ListMode {
+    fn from(args: ListArgs) -> Self {
+        if args.all {
+            assert!(args.repo.is_none());
+            return Self::All;
+        }
+
+        if let Some(repo) = args.repo {
+            return Self::ByRepo(repo);
+        }
+
+        Self::Contextual
+    }
+}
+
+impl From<EmptyArgs> for ListArgs {
+    fn from(
+        EmptyArgs {
+            sort_by,
+            reverse,
+            show_unknown,
+            repo,
+            all,
+        }: EmptyArgs,
+    ) -> Self {
+        Self {
+            sort_by,
+            reverse,
+            show_unknown,
+            repo,
+            all,
+        }
+    }
+}
+
+#[derive(Parser, Clone, Debug)]
+pub(super) struct ClearArgs {
+    /// Operate on a given repository [default: cwd]
+    #[arg(long, value_name = "RID")]
+    repo: Option<RepoId>,
+
+    /// Operate on all repositories
+    #[arg(short, long, conflicts_with = "repo")]
+    all: bool,
+
+    /// A list of notifications to clear
+    ///
+    /// The --repo or --all options are ignored when the notification ID's are
+    /// specified
+    #[arg(value_name = "NOTIFICATION_ID")]
+    ids: Option<Vec<NotificationId>>,
+}
+
+impl From<ClearArgs> for ClearMode {
+    fn from(ClearArgs { repo, all, ids }: ClearArgs) -> Self {
+        if let Some(ids) = ids {
+            return Self::ByNotifications(ids);
+        }
+
+        if all {
+            assert!(repo.is_none());
+            return Self::All;
+        }
+
+        if let Some(repo) = repo {
+            return Self::ByRepo(repo);
+        }
+
+        Self::Contextual
+    }
+}
+
+#[derive(ValueEnum, Clone, Copy, Default, Debug)]
+pub enum SortBy {
+    Id,
+    #[default]
+    Timestamp,
+}
+
+impl Display for SortBy {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Id => write!(f, "rowid"),
+            Self::Timestamp => write!(f, "timestamp"),
+        }
+    }
+}
+
+impl FromStr for SortBy {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "id" => Ok(Self::Id),
+            "timestamp" => Ok(Self::Timestamp),
+            _ => Err(format!("'{s}' is not a valid sort by column")),
+        }
+    }
+}
+
+pub(super) enum ListMode {
+    /// List the notifications of the current repository, if in a working
+    /// directory, otherwise all the repositories.
+    Contextual,
+    /// List the notifications for a all repositories.
+    All,
+    /// List the notifications for a specific repository.
+    ByRepo(RepoId),
+}
+
+pub(super) enum ClearMode {
+    /// Clear the specified notifications.
+    ///
+    /// Note that this does not require a `RepoId` since the IDs are globally
+    /// unique due to the use of a single sqlite table.
+    ByNotifications(Vec<NotificationId>),
+    /// Clear the notifications of a specific repository.
+    ByRepo(RepoId),
+    /// Clear all notifications of all repositories.
+    All,
+    /// Clear the notifications of the current repository, only if in a working
+    /// directory.
+    Contextual,
+}
diff --git a/crates/radicle-cli/src/main.rs b/crates/radicle-cli/src/main.rs
index 3ac1afbe..9edaa5b0 100644
--- a/crates/radicle-cli/src/main.rs
+++ b/crates/radicle-cli/src/main.rs
@@ -69,6 +69,7 @@ enum Commands {
     Follow(follow::Args),
     Fork(fork::Args),
     Id(id::Args),
+    Inbox(inbox::Args),
     Init(init::Args),
     Inspect(inspect::Args),
     Issue(issue::Args),
@@ -269,7 +270,9 @@ pub(crate) fn run_other(exe: &str, args: &[OsString]) -> Result<(), Option<anyho
             }
         }
         "inbox" => {
-            term::run_command_args::<inbox::Options, _>(inbox::HELP, inbox::run, args.to_vec())
+            if let Some(Commands::Inbox(args)) = CliArgs::parse().command {
+                term::run_command_fn(inbox::run, args)
+            }
         }
         "init" => {
             if let Some(Commands::Init(args)) = CliArgs::parse().command {