hive-forge: dedup the --json refusal into refuse_json(), test it, cover attachment-get/artifact-get too

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<i64> {
 /// 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!(

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)?;

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()

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,

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()

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<Value> {
 
 #[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() {

hive-forge/src/verbs/tree_sha.rs

@@ -1,11 +1,12 @@
 //! `tree-sha <ref> [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".

hive-forge/src/verbs/view.rs

@@ -1,13 +1,13 @@
 //! `view <number> [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.