From eae124a4608d0357b50dc53a300f3e4a415ca308 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 3 Jun 2026 14:14:52 +0200 Subject: [PATCH 1/4] feat(#1141): pr-reviews list mode shows inline review comments --- hive-forge/src/verbs/pr_reviews.rs | 106 ++++++++++++++++++++++++----- 1 file changed, 88 insertions(+), 18 deletions(-) diff --git a/hive-forge/src/verbs/pr_reviews.rs b/hive-forge/src/verbs/pr_reviews.rs index 8755f218..f9eff091 100644 --- a/hive-forge/src/verbs/pr_reviews.rs +++ b/hive-forge/src/verbs/pr_reviews.rs @@ -65,24 +65,94 @@ pub fn run(client: &Client, args: Args) -> Result<()> { if args.body.is_some() { bail!("--body requires one of --approve / --request-changes / --comment"); } - // List mode (original behaviour). - let v = client.get_json(&format!("/repos/{repo}/pulls/{}/reviews", args.number))?; - let trimmed: Vec = v - .as_array() - .map(|a| { - a.iter() - .map(|r| { - json!({ - "id": r.get("id"), - "state": r.get("state"), - "user": r.get("user").and_then(|u| u.get("login")), - "body": r.get("body"), - "comments_count": r.get("comments_count"), - }) + // List mode: fetch reviews, then fetch inline comments for each review + // so the full review content is visible without curl fallbacks. + let v = + client.get_json(&format!("/repos/{repo}/pulls/{}/reviews", args.number))?; + let reviews = v.as_array().cloned().unwrap_or_default(); + + if client.json_mode() { + let trimmed: Vec = reviews + .iter() + .map(|r| { + let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); + let inline = if id > 0 { + client + .get_json(&format!( + "/repos/{repo}/pulls/{}/reviews/{id}/comments", + args.number + )) + .ok() + .and_then(|v| v.as_array().cloned()) + .map(|comments| { + comments + .iter() + .map(|c| { + json!({ + "id": c.get("id"), + "path": c.get("path"), + "line": c.get("line"), + "body": c.get("body"), + }) + }) + .collect::>() + }) + .unwrap_or_default() + } else { + vec![] + }; + json!({ + "id": r.get("id"), + "state": r.get("state"), + "user": r.get("user").and_then(|u| u.get("login")), + "body": r.get("body"), + "comments_count": r.get("comments_count"), + "comments": inline, }) - .collect() - }) - .unwrap_or_default(); - print_json(&Value::Array(trimmed)) + }) + .collect(); + print_json(&Value::Array(trimmed)) + } else { + if reviews.is_empty() { + println!("(no reviews)"); + return Ok(()); + } + for r in &reviews { + let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); + let user = r + .get("user") + .and_then(|u| u.get("login")) + .and_then(Value::as_str) + .unwrap_or("?"); + let state = r.get("state").and_then(Value::as_str).unwrap_or("?"); + let body = r.get("body").and_then(Value::as_str).unwrap_or("").trim(); + println!("### review by {user} ({state})"); + if !body.is_empty() { + println!("{body}"); + } + // Fetch and print inline comments for this review. + if id > 0 { + if let Ok(ic) = client.get_json(&format!( + "/repos/{repo}/pulls/{}/reviews/{id}/comments", + args.number + )) { + let inline = ic.as_array().cloned().unwrap_or_default(); + for c in &inline { + let path = c.get("path").and_then(Value::as_str).unwrap_or("?"); + let line = c + .get("line") + .and_then(Value::as_u64) + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".to_string()); + let cbody = + c.get("body").and_then(Value::as_str).unwrap_or("").trim(); + println!(" [{path}:{line}] {cbody}"); + } + } + } + println!(); + } + Ok(()) + } } } From b39967eb7afa66b5c76a8192bf8ebcdaa92f7e38 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 3 Jun 2026 16:00:35 +0200 Subject: [PATCH 2/4] refactor(#1141): extract submit_review, list_reviews, fetch_inline_comments helpers --- hive-forge/src/verbs/pr_reviews.rs | 203 +++++++++++++++-------------- 1 file changed, 103 insertions(+), 100 deletions(-) diff --git a/hive-forge/src/verbs/pr_reviews.rs b/hive-forge/src/verbs/pr_reviews.rs index f9eff091..e2ab8086 100644 --- a/hive-forge/src/verbs/pr_reviews.rs +++ b/hive-forge/src/verbs/pr_reviews.rs @@ -32,8 +32,6 @@ pub struct Args { } pub fn run(client: &Client, args: Args) -> Result<()> { - let repo = client.repo(); - let event = if args.approve { Some("APPROVED") } else if args.request_changes { @@ -45,114 +43,119 @@ pub fn run(client: &Client, args: Args) -> Result<()> { }; if let Some(ev) = event { - // Submit a review. - let payload = json!({ - "event": ev, - "body": args.body.unwrap_or_default(), - }); - let v = client.post_json( - &format!("/repos/{repo}/pulls/{}/reviews", args.number), - &payload, - )?; - // Print a compact summary rather than the full review blob. - let summary = json!({ - "id": v.get("id"), - "state": v.get("state"), - "user": v.get("user").and_then(|u| u.get("login")), - }); - print_json(&summary) + submit_review(client, args.number, ev, args.body) } else { if args.body.is_some() { bail!("--body requires one of --approve / --request-changes / --comment"); } - // List mode: fetch reviews, then fetch inline comments for each review - // so the full review content is visible without curl fallbacks. - let v = - client.get_json(&format!("/repos/{repo}/pulls/{}/reviews", args.number))?; - let reviews = v.as_array().cloned().unwrap_or_default(); + list_reviews(client, args.number) + } +} - if client.json_mode() { - let trimmed: Vec = reviews - .iter() - .map(|r| { - let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); - let inline = if id > 0 { - client - .get_json(&format!( - "/repos/{repo}/pulls/{}/reviews/{id}/comments", - args.number - )) - .ok() - .and_then(|v| v.as_array().cloned()) - .map(|comments| { - comments - .iter() - .map(|c| { - json!({ - "id": c.get("id"), - "path": c.get("path"), - "line": c.get("line"), - "body": c.get("body"), - }) - }) - .collect::>() +/// Submit a review event (APPROVED / REQUEST_CHANGES / COMMENT) and print +/// a compact summary of the created review. +fn submit_review( + client: &Client, + number: u64, + event: &str, + body: Option, +) -> Result<()> { + let repo = client.repo(); + let payload = json!({ + "event": event, + "body": body.unwrap_or_default(), + }); + let v = client.post_json( + &format!("/repos/{repo}/pulls/{number}/reviews"), + &payload, + )?; + print_json(&json!({ + "id": v.get("id"), + "state": v.get("state"), + "user": v.get("user").and_then(|u| u.get("login")), + })) +} + +/// Fetch inline diff comments for a single review. Returns an empty vec on +/// any error (missing review, network failure) so callers can degrade +/// gracefully. +fn fetch_inline_comments(client: &Client, repo: &str, pr: u64, review_id: u64) -> Vec { + client + .get_json(&format!("/repos/{repo}/pulls/{pr}/reviews/{review_id}/comments")) + .ok() + .and_then(|v| v.as_array().cloned()) + .unwrap_or_default() +} + +/// List all reviews for a PR, including inline diff comments per review. +fn list_reviews(client: &Client, number: u64) -> Result<()> { + let repo = client.repo(); + let v = client.get_json(&format!("/repos/{repo}/pulls/{number}/reviews"))?; + let reviews = v.as_array().cloned().unwrap_or_default(); + + if client.json_mode() { + let trimmed: Vec = reviews + .iter() + .map(|r| { + let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); + let inline: Vec = if id > 0 { + fetch_inline_comments(client, repo, number, id) + .iter() + .map(|c| { + json!({ + "id": c.get("id"), + "path": c.get("path"), + "line": c.get("line"), + "body": c.get("body"), }) - .unwrap_or_default() - } else { - vec![] - }; - json!({ - "id": r.get("id"), - "state": r.get("state"), - "user": r.get("user").and_then(|u| u.get("login")), - "body": r.get("body"), - "comments_count": r.get("comments_count"), - "comments": inline, - }) + }) + .collect() + } else { + vec![] + }; + json!({ + "id": r.get("id"), + "state": r.get("state"), + "user": r.get("user").and_then(|u| u.get("login")), + "body": r.get("body"), + "comments_count": r.get("comments_count"), + "comments": inline, }) - .collect(); - print_json(&Value::Array(trimmed)) - } else { - if reviews.is_empty() { - println!("(no reviews)"); - return Ok(()); + }) + .collect(); + print_json(&Value::Array(trimmed)) + } else { + if reviews.is_empty() { + println!("(no reviews)"); + return Ok(()); + } + for r in &reviews { + let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); + let user = r + .get("user") + .and_then(|u| u.get("login")) + .and_then(Value::as_str) + .unwrap_or("?"); + let state = r.get("state").and_then(Value::as_str).unwrap_or("?"); + let body = r.get("body").and_then(Value::as_str).unwrap_or("").trim(); + println!("### review by {user} ({state})"); + if !body.is_empty() { + println!("{body}"); } - for r in &reviews { - let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); - let user = r - .get("user") - .and_then(|u| u.get("login")) - .and_then(Value::as_str) - .unwrap_or("?"); - let state = r.get("state").and_then(Value::as_str).unwrap_or("?"); - let body = r.get("body").and_then(Value::as_str).unwrap_or("").trim(); - println!("### review by {user} ({state})"); - if !body.is_empty() { - println!("{body}"); - } - // Fetch and print inline comments for this review. - if id > 0 { - if let Ok(ic) = client.get_json(&format!( - "/repos/{repo}/pulls/{}/reviews/{id}/comments", - args.number - )) { - let inline = ic.as_array().cloned().unwrap_or_default(); - for c in &inline { - let path = c.get("path").and_then(Value::as_str).unwrap_or("?"); - let line = c - .get("line") - .and_then(Value::as_u64) - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".to_string()); - let cbody = - c.get("body").and_then(Value::as_str).unwrap_or("").trim(); - println!(" [{path}:{line}] {cbody}"); - } - } + if id > 0 { + for c in &fetch_inline_comments(client, repo, number, id) { + let path = c.get("path").and_then(Value::as_str).unwrap_or("?"); + let line = c + .get("line") + .and_then(Value::as_u64) + .map(|n| n.to_string()) + .unwrap_or_else(|| "?".to_string()); + let cbody = c.get("body").and_then(Value::as_str).unwrap_or("").trim(); + println!(" [{path}:{line}] {cbody}"); } - println!(); } - Ok(()) + println!(); } + Ok(()) } } From 374e53d13ed319ac589c8cde2924463c7ecf64e1 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 3 Jun 2026 16:01:32 +0200 Subject: [PATCH 3/4] nit(#1141): suppress :line in text output when comment has no line number --- hive-forge/src/verbs/pr_reviews.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hive-forge/src/verbs/pr_reviews.rs b/hive-forge/src/verbs/pr_reviews.rs index e2ab8086..91b9d2f8 100644 --- a/hive-forge/src/verbs/pr_reviews.rs +++ b/hive-forge/src/verbs/pr_reviews.rs @@ -145,13 +145,12 @@ fn list_reviews(client: &Client, number: u64) -> Result<()> { if id > 0 { for c in &fetch_inline_comments(client, repo, number, id) { let path = c.get("path").and_then(Value::as_str).unwrap_or("?"); - let line = c - .get("line") - .and_then(Value::as_u64) - .map(|n| n.to_string()) - .unwrap_or_else(|| "?".to_string()); let cbody = c.get("body").and_then(Value::as_str).unwrap_or("").trim(); - println!(" [{path}:{line}] {cbody}"); + // PR-level comments have no line; omit `:line` when absent. + match c.get("line").and_then(Value::as_u64) { + Some(line) => println!(" [{path}:{line}] {cbody}"), + None => println!(" [{path}] {cbody}"), + } } } println!(); From aa91d0d30d255ce9bf066ccd1c99835e49885356 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 3 Jun 2026 16:04:07 +0200 Subject: [PATCH 4/4] refactor(#1141): split list_reviews into list_reviews_json + list_reviews_text --- hive-forge/src/verbs/pr_reviews.rs | 146 ++++++++++++++++------------- 1 file changed, 83 insertions(+), 63 deletions(-) diff --git a/hive-forge/src/verbs/pr_reviews.rs b/hive-forge/src/verbs/pr_reviews.rs index 91b9d2f8..4533f468 100644 --- a/hive-forge/src/verbs/pr_reviews.rs +++ b/hive-forge/src/verbs/pr_reviews.rs @@ -87,74 +87,94 @@ fn fetch_inline_comments(client: &Client, repo: &str, pr: u64, review_id: u64) - .unwrap_or_default() } -/// List all reviews for a PR, including inline diff comments per review. +/// List all reviews for a PR, dispatching to the appropriate output mode. fn list_reviews(client: &Client, number: u64) -> Result<()> { let repo = client.repo(); let v = client.get_json(&format!("/repos/{repo}/pulls/{number}/reviews"))?; let reviews = v.as_array().cloned().unwrap_or_default(); - if client.json_mode() { - let trimmed: Vec = reviews - .iter() - .map(|r| { - let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); - let inline: Vec = if id > 0 { - fetch_inline_comments(client, repo, number, id) - .iter() - .map(|c| { - json!({ - "id": c.get("id"), - "path": c.get("path"), - "line": c.get("line"), - "body": c.get("body"), - }) - }) - .collect() - } else { - vec![] - }; - json!({ - "id": r.get("id"), - "state": r.get("state"), - "user": r.get("user").and_then(|u| u.get("login")), - "body": r.get("body"), - "comments_count": r.get("comments_count"), - "comments": inline, - }) - }) - .collect(); - print_json(&Value::Array(trimmed)) + list_reviews_json(client, repo, number, &reviews) } else { - if reviews.is_empty() { - println!("(no reviews)"); - return Ok(()); - } - for r in &reviews { - let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); - let user = r - .get("user") - .and_then(|u| u.get("login")) - .and_then(Value::as_str) - .unwrap_or("?"); - let state = r.get("state").and_then(Value::as_str).unwrap_or("?"); - let body = r.get("body").and_then(Value::as_str).unwrap_or("").trim(); - println!("### review by {user} ({state})"); - if !body.is_empty() { - println!("{body}"); - } - if id > 0 { - for c in &fetch_inline_comments(client, repo, number, id) { - let path = c.get("path").and_then(Value::as_str).unwrap_or("?"); - let cbody = c.get("body").and_then(Value::as_str).unwrap_or("").trim(); - // PR-level comments have no line; omit `:line` when absent. - match c.get("line").and_then(Value::as_u64) { - Some(line) => println!(" [{path}:{line}] {cbody}"), - None => println!(" [{path}] {cbody}"), - } - } - } - println!(); - } - Ok(()) + list_reviews_text(client, repo, number, &reviews) } } + +/// JSON output: one object per review, with an inline `comments` array. +fn list_reviews_json( + client: &Client, + repo: &str, + number: u64, + reviews: &[Value], +) -> Result<()> { + let trimmed: Vec = reviews + .iter() + .map(|r| { + let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); + let inline: Vec = if id > 0 { + fetch_inline_comments(client, repo, number, id) + .iter() + .map(|c| { + json!({ + "id": c.get("id"), + "path": c.get("path"), + "line": c.get("line"), + "body": c.get("body"), + }) + }) + .collect() + } else { + vec![] + }; + json!({ + "id": r.get("id"), + "state": r.get("state"), + "user": r.get("user").and_then(|u| u.get("login")), + "body": r.get("body"), + "comments_count": r.get("comments_count"), + "comments": inline, + }) + }) + .collect(); + print_json(&Value::Array(trimmed)) +} + +/// Human-readable output: Markdown-style heading per review, inline +/// comments as `[path:line] body` (line omitted for PR-level comments). +fn list_reviews_text( + client: &Client, + repo: &str, + number: u64, + reviews: &[Value], +) -> Result<()> { + if reviews.is_empty() { + println!("(no reviews)"); + return Ok(()); + } + for r in reviews { + let id = r.get("id").and_then(Value::as_u64).unwrap_or(0); + let user = r + .get("user") + .and_then(|u| u.get("login")) + .and_then(Value::as_str) + .unwrap_or("?"); + let state = r.get("state").and_then(Value::as_str).unwrap_or("?"); + let body = r.get("body").and_then(Value::as_str).unwrap_or("").trim(); + println!("### review by {user} ({state})"); + if !body.is_empty() { + println!("{body}"); + } + if id > 0 { + for c in &fetch_inline_comments(client, repo, number, id) { + let path = c.get("path").and_then(Value::as_str).unwrap_or("?"); + let cbody = c.get("body").and_then(Value::as_str).unwrap_or("").trim(); + // PR-level comments have no line; omit `:line` when absent. + match c.get("line").and_then(Value::as_u64) { + Some(line) => println!(" [{path}:{line}] {cbody}"), + None => println!(" [{path}] {cbody}"), + } + } + } + println!(); + } + Ok(()) +}