diff --git a/hive-forge/src/verbs/artifact_get.rs b/hive-forge/src/verbs/artifact_get.rs index 4de16d8c..ba78cc8d 100644 --- a/hive-forge/src/verbs/artifact_get.rs +++ b/hive-forge/src/verbs/artifact_get.rs @@ -26,6 +26,7 @@ use clap::Args as ClapArgs; use forgejo_api::structs::ListActionRunsQuery; use crate::client::Client; +use crate::verbs::refuse_json; #[derive(ClapArgs)] pub struct Args { @@ -82,6 +83,11 @@ fn resolve_run_id(client: &Client, repo: &str, run_number: u64) -> Result { /// download fails (network, or a non-2xx status such as `404` for an /// unknown artifact name), or if the output path can't be written. pub fn run(client: &Client, args: Args) -> Result<()> { + refuse_json( + client.json_mode(), + "artifact-get", + "it downloads the raw artifact zip, there is no JSON form", + )?; let repo = client.repo()?; let run_id = resolve_run_id(client, repo, args.run)?; let url = client.web_url(&format!( diff --git a/hive-forge/src/verbs/attachment_get.rs b/hive-forge/src/verbs/attachment_get.rs index 8b781050..21d0ac34 100644 --- a/hive-forge/src/verbs/attachment_get.rs +++ b/hive-forge/src/verbs/attachment_get.rs @@ -14,6 +14,7 @@ use anyhow::{Result, bail}; use clap::Args as ClapArgs; use crate::client::Client; +use crate::verbs::refuse_json; #[derive(ClapArgs)] pub struct Args { @@ -34,6 +35,11 @@ pub struct Args { /// the input, if the download fails (network, 4xx/5xx), or if the /// output path cannot be written. pub fn run(client: &Client, args: Args) -> Result<()> { + refuse_json( + client.json_mode(), + "attachment-get", + "it downloads the raw attachment bytes, there is no JSON form", + )?; let uuid = extract_uuid(&args.attachment)?; let url = client.attachment_url(&uuid); let bytes = client.get_bytes_raw(&url)?; diff --git a/hive-forge/src/verbs/branches.rs b/hive-forge/src/verbs/branches.rs index 849088ca..0b4ec3fd 100644 --- a/hive-forge/src/verbs/branches.rs +++ b/hive-forge/src/verbs/branches.rs @@ -1,9 +1,10 @@ //! `branches [pattern] [repo]` — list branches, optionally filtered. -use anyhow::{Result, bail}; +use anyhow::Result; use clap::Args as ClapArgs; use crate::client::Client; +use crate::verbs::refuse_json; #[derive(ClapArgs)] pub struct Args { @@ -12,11 +13,7 @@ pub struct Args { } pub fn run(client: &Client, args: Args) -> Result<()> { - if client.json_mode() { - bail!( - "hive-forge branches: --json is not supported for this verb, only the plain name list" - ); - } + refuse_json(client.json_mode(), "branches", "only the plain name list")?; let (owner, name) = client.owner_repo()?; let (_, branches) = client .api() diff --git a/hive-forge/src/verbs/clone.rs b/hive-forge/src/verbs/clone.rs index c597eee2..7a7b4684 100644 --- a/hive-forge/src/verbs/clone.rs +++ b/hive-forge/src/verbs/clone.rs @@ -20,6 +20,7 @@ use anyhow::{Context, Result, bail}; use clap::Args as ClapArgs; use crate::client::Client; +use crate::verbs::refuse_json; #[derive(ClapArgs)] pub struct Args { @@ -39,9 +40,7 @@ pub struct Args { /// Returns an error if the `git clone` shellout fails (bad repo, auth /// rejected, network) or the destination can't be derived. pub fn run(client: &Client, args: Args) -> Result<()> { - if client.json_mode() { - bail!("hive-forge clone: --json is not supported for this verb, only the destination path"); - } + refuse_json(client.json_mode(), "clone", "only the destination path")?; let repo = client.repo()?; let dest = match args.dest { Some(d) => d, diff --git a/hive-forge/src/verbs/diff.rs b/hive-forge/src/verbs/diff.rs index 6a30104c..37979765 100644 --- a/hive-forge/src/verbs/diff.rs +++ b/hive-forge/src/verbs/diff.rs @@ -13,11 +13,12 @@ //! lockfile without any information the reader can act on. //! Pass `--full` to dump the unfiltered diff. -use anyhow::{Result, bail}; +use anyhow::Result; use clap::Args as ClapArgs; use forgejo_api::structs::RepoDownloadPullDiffOrPatchQuery; use crate::client::{Client, index}; +use crate::verbs::refuse_json; #[derive(ClapArgs)] pub struct Args { @@ -31,9 +32,7 @@ pub struct Args { } pub fn run(client: &Client, args: Args) -> Result<()> { - if client.json_mode() { - bail!("hive-forge diff: --json is not supported for this verb — a diff is text by nature"); - } + refuse_json(client.json_mode(), "diff", "a diff is text by nature")?; let (owner, name) = client.owner_repo()?; let diff = client .api() diff --git a/hive-forge/src/verbs/mod.rs b/hive-forge/src/verbs/mod.rs index 3c8b750e..491a152e 100644 --- a/hive-forge/src/verbs/mod.rs +++ b/hive-forge/src/verbs/mod.rs @@ -50,7 +50,7 @@ pub mod view; use std::fmt::Write as _; -use anyhow::Result; +use anyhow::{Result, bail}; use forgejo_api::structs::{Attachment, Reaction}; use serde::Deserialize; use serde_json::{Value, json}; @@ -67,6 +67,19 @@ pub(crate) fn print_json(v: &Value) -> Result<()> { Ok(()) } +/// Error out when the global `--json` flag is set on a verb that has no +/// structured output to give it — a silently-ignored flag reads as "this is +/// JSON" when it is actually prose or raw bytes, which is worse than a loud +/// refusal. `why` names what the verb prints instead, e.g. `"only the plain +/// name list"`; `verb` is the subcommand name as the operator typed it +/// (`"branches"`, `"pr diff"`, …), for the error message. +pub(crate) fn refuse_json(json_mode: bool, verb: &str, why: &str) -> Result<()> { + if json_mode { + bail!("hive-forge {verb}: --json is not supported for this verb — {why}"); + } + Ok(()) +} + /// Format an optional timestamp as its RFC 3339 string — the shape the /// raw API emitted, so output stays stable across the typed-client /// port. `None` (and the never-in-practice unformattable timestamp) @@ -580,7 +593,20 @@ pub(crate) fn attachment_json(assets: Option<&[Attachment]>) -> Vec { #[cfg(test)] mod tests { - use super::{NullableVec, pct_encode, reviewed_older_head}; + use super::{NullableVec, pct_encode, refuse_json, reviewed_older_head}; + + #[test] + fn refuse_json_bails_with_the_verb_and_reason() { + let err = refuse_json(true, "branches", "only the plain name list").unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("branches"), "{msg}"); + assert!(msg.contains("only the plain name list"), "{msg}"); + } + + #[test] + fn refuse_json_passes_through_when_json_mode_is_off() { + assert!(refuse_json(false, "branches", "only the plain name list").is_ok()); + } #[test] fn nullable_vec_treats_a_json_null_as_empty() { diff --git a/hive-forge/src/verbs/tree_sha.rs b/hive-forge/src/verbs/tree_sha.rs index 38641893..06aa6631 100644 --- a/hive-forge/src/verbs/tree_sha.rs +++ b/hive-forge/src/verbs/tree_sha.rs @@ -1,11 +1,12 @@ //! `tree-sha [repo]` — print the tree SHA of the commit at a //! branch name or commit SHA. -use anyhow::{Result, bail}; +use anyhow::Result; use clap::Args as ClapArgs; use forgejo_api::structs::RepoGetSingleCommitQuery; use crate::client::Client; +use crate::verbs::refuse_json; #[derive(ClapArgs)] pub struct Args { @@ -14,9 +15,7 @@ pub struct Args { } pub fn run(client: &Client, args: Args) -> Result<()> { - if client.json_mode() { - bail!("hive-forge tree-sha: --json is not supported for this verb, only the plain sha"); - } + refuse_json(client.json_mode(), "tree-sha", "only the plain sha")?; let (owner, name) = client.owner_repo()?; // Try branch first; the bash helper silently treats failure as // "not a branch, use the reference as a commit sha directly". diff --git a/hive-forge/src/verbs/view.rs b/hive-forge/src/verbs/view.rs index 396b180c..5672c524 100644 --- a/hive-forge/src/verbs/view.rs +++ b/hive-forge/src/verbs/view.rs @@ -1,13 +1,13 @@ //! `view [repo]` — dump title + body + all comments as //! markdown. -use anyhow::{Result, bail}; +use anyhow::Result; use clap::Args as ClapArgs; use forgejo_api::structs::{IssueGetCommentsQuery, StateType}; use crate::client::{Client, index}; use crate::notify; -use crate::verbs::{attachment_line, issue_reactions, reaction_summary}; +use crate::verbs::{attachment_line, issue_reactions, reaction_summary, refuse_json}; #[derive(ClapArgs)] pub struct Args { @@ -16,12 +16,11 @@ pub struct Args { } pub fn run(client: &Client, args: Args) -> Result<()> { - if client.json_mode() { - bail!( - "hive-forge view: --json is not supported for this verb — \ - use `issue show`/`pr show` for structured fields" - ); - } + refuse_json( + client.json_mode(), + "view", + "use `issue show`/`pr show` for structured fields", + )?; let repo = client.repo()?; // Reading the thread clears its unread notification so the // read-before-comment guard (in `comment`) lets a reply through.