From bb97414a2d353270791d3dec924129d97980a1a5 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Wed, 15 Apr 2026 23:35:06 +0200 Subject: [PATCH] cli: Deduplicate RID Argument Handling Multiple commands take an RID as a command line argument. However, the handling, which usually involves falling back to detection of the repository associated with the current directory, is duplicated many times over. Introduce `fn rid_or_cwd` which can be used by many commands. --- crates/radicle-cli/src/commands/fork.rs | 17 ++++--------- crates/radicle-cli/src/commands/id.rs | 9 ++----- crates/radicle-cli/src/commands/inbox.rs | 10 +++----- crates/radicle-cli/src/commands/issue.rs | 8 ++----- crates/radicle-cli/src/commands/patch.rs | 10 ++------ crates/radicle-cli/src/commands/publish.rs | 10 +++----- crates/radicle-cli/src/commands/remote.rs | 10 ++++---- crates/radicle-cli/src/commands/sync.rs | 21 ++++------------ crates/radicle-cli/src/commands/watch.rs | 12 +++------- crates/radicle-cli/src/terminal/args.rs | 28 ++++++++++++++++++++++ 10 files changed, 57 insertions(+), 78 deletions(-) diff --git a/crates/radicle-cli/src/commands/fork.rs b/crates/radicle-cli/src/commands/fork.rs index 83781d36..6cd02c2c 100644 --- a/crates/radicle-cli/src/commands/fork.rs +++ b/crates/radicle-cli/src/commands/fork.rs @@ -1,10 +1,11 @@ mod args; -use anyhow::Context as _; - use radicle::rad; -use crate::{terminal as term, warning}; +use crate::{ + terminal::{self as term, args::rid_or_cwd}, + warning, +}; pub use args::Args; @@ -13,15 +14,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { let profile = ctx.profile()?; let signer = profile.signer()?; let storage = &profile.storage; - - let rid = match args.repo { - Some(rid) => rid, - None => { - let (_, rid) = rad::cwd().context("Current directory is not a Radicle repository")?; - - rid - } - }; + let (_, rid) = rid_or_cwd(args.repo)?; rad::fork(rid, &signer, &storage)?; term::success!("Forked repository {rid} for {}", profile.id()); diff --git a/crates/radicle-cli/src/commands/id.rs b/crates/radicle-cli/src/commands/id.rs index c2e8c994..03f5e6f6 100644 --- a/crates/radicle-cli/src/commands/id.rs +++ b/crates/radicle-cli/src/commands/id.rs @@ -17,7 +17,7 @@ use radicle_term::Element; use crate::git::Rev; use crate::git::unified_diff::Encode as _; use crate::terminal as term; -use crate::terminal::args::Error; +use crate::terminal::args::{Error, rid_or_cwd}; use crate::terminal::format::Author; use crate::terminal::patch::Message; @@ -27,12 +27,7 @@ use args::Command; pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { let profile = ctx.profile()?; let storage = &profile.storage; - let rid = if let Some(rid) = args.repo { - rid - } else { - let (_, rid) = radicle::rad::cwd()?; - rid - }; + let (_, rid) = rid_or_cwd(args.repo)?; let repo = storage .repository(rid) .context(anyhow!("repository `{rid}` not found in local storage"))?; diff --git a/crates/radicle-cli/src/commands/inbox.rs b/crates/radicle-cli/src/commands/inbox.rs index 91f7ec04..f57fa844 100644 --- a/crates/radicle-cli/src/commands/inbox.rs +++ b/crates/radicle-cli/src/commands/inbox.rs @@ -5,8 +5,6 @@ pub use args::Args; use std::path::Path; use std::process; -use anyhow::anyhow; - use localtime::LocalTime; use radicle::cob::TypedId; use radicle::git::BranchName; @@ -23,6 +21,7 @@ use radicle::{Storage, cob, git}; use term::Element as _; use crate::terminal as term; +use crate::terminal::args::rid_or_cwd; use args::{ClearMode, Command, ListMode, SortBy}; pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { @@ -391,11 +390,8 @@ fn clear(notifs: &mut notifications::StoreWriter, mode: ClearMode) -> anyhow::Re 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(anyhow!("not a radicle repository")); - } + let (_, rid) = rid_or_cwd(None)?; + notifs.clear_by_repo(&rid)? } }; if cleared > 0 { diff --git a/crates/radicle-cli/src/commands/issue.rs b/crates/radicle-cli/src/commands/issue.rs index 6717ccd6..9e5c9538 100644 --- a/crates/radicle-cli/src/commands/issue.rs +++ b/crates/radicle-cli/src/commands/issue.rs @@ -26,7 +26,7 @@ use crate::git::Rev; use crate::node; use crate::terminal as term; use crate::terminal::Element; -use crate::terminal::args::Error; +use crate::terminal::args::{Error, rid_or_cwd}; use crate::terminal::format::Author; use crate::terminal::issue::Format; @@ -34,11 +34,7 @@ const ABOUT: &str = "Manage issues"; pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { let profile = ctx.profile()?; - let rid = match args.repo { - Some(rid) => rid, - None => radicle::rad::cwd().map(|(_, rid)| rid)?, - }; - + let (_, rid) = rid_or_cwd(args.repo)?; let repo = profile.storage.repository_mut(rid)?; // Fallback to [`Command::List`] if no subcommand is provided. diff --git a/crates/radicle-cli/src/commands/patch.rs b/crates/radicle-cli/src/commands/patch.rs index 25136f17..23d23f0f 100644 --- a/crates/radicle-cli/src/commands/patch.rs +++ b/crates/radicle-cli/src/commands/patch.rs @@ -30,6 +30,7 @@ use radicle::{Node, prelude::*}; use crate::git::Rev; use crate::node; use crate::terminal as term; +use crate::terminal::args::rid_or_cwd; use crate::terminal::patch::Message; pub use args::Args; @@ -37,14 +38,7 @@ pub use args::Args; use args::{AssignArgs, Command, CommentAction, LabelArgs}; pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { - let (workdir, rid) = if let Some(rid) = args.repo { - (None, rid) - } else { - radicle::rad::cwd() - .map(|(workdir, rid)| (Some(workdir), rid)) - .map_err(|_| anyhow!("this command must be run in the context of a repository"))? - }; - + let (workdir, rid) = rid_or_cwd(args.repo)?; let profile = ctx.profile()?; let repository = profile.storage.repository(rid)?; diff --git a/crates/radicle-cli/src/commands/publish.rs b/crates/radicle-cli/src/commands/publish.rs index 2acf79b0..dc8d491d 100644 --- a/crates/radicle-cli/src/commands/publish.rs +++ b/crates/radicle-cli/src/commands/publish.rs @@ -1,6 +1,6 @@ mod args; -use anyhow::{Context as _, anyhow}; +use anyhow::anyhow; use radicle::cob; use radicle::identity::{Identity, Visibility}; @@ -8,17 +8,13 @@ use radicle::node::Handle as _; use radicle::storage::{SignRepository, ValidateRepository, WriteRepository, WriteStorage}; use crate::terminal as term; +use crate::terminal::args::rid_or_cwd; pub use args::Args; pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { let profile = ctx.profile()?; - let rid = match args.repo { - Some(rid) => rid, - None => radicle::rad::cwd() - .map(|(_, rid)| rid) - .context("Current directory is not a Radicle repository")?, - }; + let (_, rid) = rid_or_cwd(args.repo)?; let repo = profile.storage.repository_mut(rid)?; let signer = profile.signer()?; diff --git a/crates/radicle-cli/src/commands/remote.rs b/crates/radicle-cli/src/commands/remote.rs index 61c59ecb..c7ed3fca 100644 --- a/crates/radicle-cli/src/commands/remote.rs +++ b/crates/radicle-cli/src/commands/remote.rs @@ -6,19 +6,19 @@ pub mod rm; mod args; -use anyhow::anyhow; - use radicle::storage::ReadStorage; -use crate::terminal as term; use crate::terminal::Context; +use crate::terminal::{self as term, args::rid_or_cwd}; pub use args::Args; use args::{Command, ListOption}; 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 (Some(working), rid) = rid_or_cwd(None)? else { + anyhow::bail!("this command must be run in the context of a repository"); + }; + let profile = ctx.profile()?; let command = args .command diff --git a/crates/radicle-cli/src/commands/sync.rs b/crates/radicle-cli/src/commands/sync.rs index 0bbb0b07..bee36e04 100644 --- a/crates/radicle-cli/src/commands/sync.rs +++ b/crates/radicle-cli/src/commands/sync.rs @@ -5,7 +5,7 @@ use std::collections::BTreeMap; use std::collections::HashSet; use std::time; -use anyhow::{Context as _, anyhow}; +use anyhow::anyhow; use radicle::node; use radicle::node::SyncedAt; @@ -22,6 +22,7 @@ use radicle_term::Element; use crate::node::SyncReporting; use crate::node::SyncSettings; use crate::terminal as term; +use crate::terminal::args::rid_or_cwd; use crate::terminal::format::Author; use crate::terminal::{Table, TableOptions}; @@ -41,14 +42,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { match args.command { Some(Command::Status { repo, sort_by }) => { - let rid = match repo { - Some(rid) => rid, - None => { - let (_, rid) = radicle::rad::cwd() - .context("Current directory is not a Radicle repository")?; - rid - } - }; + let (_, rid) = rid_or_cwd(repo)?; sync_status(rid, &mut node, &profile, &sort_by, verbose)?; } None => match SyncMode::from(args.sync) { @@ -57,14 +51,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { settings, direction, } => { - let rid = match repo { - Some(rid) => rid, - None => { - let (_, rid) = radicle::rad::cwd() - .context("Current directory is not a Radicle repository")?; - rid - } - }; + let (_, rid) = rid_or_cwd(repo)?; let settings = settings.clone().with_profile(&profile); if matches!(direction, SyncDirection::Fetch | SyncDirection::Both) { diff --git a/crates/radicle-cli/src/commands/watch.rs b/crates/radicle-cli/src/commands/watch.rs index d1e8b021..e1dfb06b 100644 --- a/crates/radicle-cli/src/commands/watch.rs +++ b/crates/radicle-cli/src/commands/watch.rs @@ -2,7 +2,7 @@ mod args; use std::{thread, time}; -use anyhow::{Context as _, anyhow}; +use anyhow::anyhow; use radicle::git; use radicle::git::raw::ErrorExt as _; @@ -10,6 +10,7 @@ use radicle::prelude::NodeId; use radicle::storage::{ReadRepository, ReadStorage}; use crate::terminal as term; +use crate::terminal::args::rid_or_cwd; pub use args::Args; @@ -21,14 +22,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> { .qualified() .ok_or_else(|| anyhow!("reference must be fully-qualified, eg. 'refs/heads/master'"))?; let nid = args.node.unwrap_or(profile.public_key); - let rid = match args.repo { - Some(rid) => rid, - None => { - let (_, rid) = - radicle::rad::cwd().context("Current directory is not a Radicle repository")?; - rid - } - }; + let (_, rid) = rid_or_cwd(args.repo)?; let repo = storage.repository(rid)?; let now = time::SystemTime::now(); let timeout = args.timeout(); diff --git a/crates/radicle-cli/src/terminal/args.rs b/crates/radicle-cli/src/terminal/args.rs index d1e6e5ab..d47b3140 100644 --- a/crates/radicle-cli/src/terminal/args.rs +++ b/crates/radicle-cli/src/terminal/args.rs @@ -105,6 +105,34 @@ impl TypedValueParser for ScopeParser { } } +/// A wrapper around [`radicle::rad::cwd`] that should be preferred over direct +/// calls in this crate. +/// +/// If the given [`Option`] is `Some`, it is returned as is. +/// No attempt is made to detect whether the current working directory is +/// a Radicle repository (therefore it is also not checked whether the given +/// [`RepoId`] matches the [`RepoId`] associated with the current working +/// directory), and the returned repository is `None`. +/// +/// Otherwise, i.e, if the given [`Option`] is `None`, an attempt is +/// made to detect the [`RepoId`] associated with the current working directory, +/// by calling [`radicle::rad::cwd`]. If this detection fails, an error with +/// context is returned. +pub(crate) fn rid_or_cwd( + rid: Option, +) -> anyhow::Result<(Option, RepoId)> { + match rid { + Some(rid) => Ok((None, rid)), + None => { + use anyhow::Context as _; + + let (repository, rid) = + radicle::rad::cwd().context("Current directory is not a Radicle repository")?; + Ok((Some(repository), rid)) + } + } +} + #[cfg(test)] mod test { use std::str::FromStr;