From b7a76288454e8bbf31a7a6743ef0002e077934dc Mon Sep 17 00:00:00 2001 From: damocles Date: Sat, 15 Aug 2026 12:33:46 +0200 Subject: [PATCH] hive-forge: point to per-line review comments instead of inlining them, fix pr reviews line numbers --- hive-forge/src/verbs/comments.rs | 41 +++++++++++++++++++++--------- hive-forge/src/verbs/pr_reviews.rs | 9 ++++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/hive-forge/src/verbs/comments.rs b/hive-forge/src/verbs/comments.rs index 88cebb71..8066e2d0 100644 --- a/hive-forge/src/verbs/comments.rs +++ b/hive-forge/src/verbs/comments.rs @@ -21,8 +21,9 @@ //! same way (see `crate::verbs::MAX_LIMIT`). //! //! Review *bodies* on PRs are always merged in too (`pulls//reviews` -//! isn't the issues/comments thread, so a plain listing used to miss -//! them), tagged `[review: STATE]`. +//! isn't the issues/comments thread), tagged `[review: STATE]`, with a +//! `(N line comment(s) — see 'pr reviews')` pointer when a review has +//! inline comments — this verb never inlines those, `pr reviews` does. //! //! `--json` output is an object (`{"comments": [...], "more_before": N, //! "more_after": N, "since_more": bool}`), not a bare array, so a @@ -106,6 +107,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> { "url": c.get("html_url"), "kind": c.get("kind").and_then(Value::as_str).unwrap_or("comment"), "state": c.get("state"), + "comments_count": c.get("comments_count"), "attachments": attachment_json_from_value(c), }) }) @@ -128,7 +130,15 @@ pub fn run(client: &Client, args: Args) -> Result<()> { let body = c.get("body").and_then(Value::as_str).unwrap_or(""); if c.get("kind").and_then(Value::as_str) == Some("review") { let state = c.get("state").and_then(Value::as_str).unwrap_or("?"); - println!("**{user} @ {ts}** [review: {state}]: {body}"); + let n = c.get("comments_count").and_then(Value::as_i64).unwrap_or(0); + if n > 0 { + println!( + "**{user} @ {ts}** [review: {state}] ({n} line comment(s) — see `hive-forge pr reviews {}`): {body}", + args.number + ); + } else { + println!("**{user} @ {ts}** [review: {state}]: {body}"); + } } else { println!("**{user} @ {ts}**: {body}"); } @@ -240,14 +250,19 @@ fn to_values(items: Vec) -> Result> { /// /// Review summaries live in the `pulls//reviews` object, not the /// issues/comments thread, so the plain comment listing misses them -/// — the review-body gap this fixes. Best-effort: returns empty on -/// any error — notably when -/// `number` is an issue (no reviews endpoint) — so callers degrade -/// gracefully. Skips PENDING reviews (not yet visible to others) and -/// empty-body reviews (a bare approval adds nothing to the thread). -/// `created_at` is synthesised from the review's `submitted_at` so -/// the chronological merge sorts uniformly; `kind:"review"` + the -/// review `state` tag the entry for display. +/// — the review-body gap this fixes. Best-effort: returns empty on any +/// error — notably when `number` is an issue (no reviews endpoint) — +/// so callers degrade gracefully. Skips PENDING reviews (not yet +/// visible to others) and reviews with neither a body nor line +/// comments (a bare approval adds nothing to the thread). Deliberately +/// does NOT inline the per-line comments themselves: merging them into +/// this glance-first thread would spam anyone who just wants status. +/// `comments_count` carries a pointer to `pr reviews ` instead, +/// which prints the full per-line detail — see that verb +/// (`pr_reviews.rs`) for the exhaustive form. `created_at` +/// is synthesised from the review's `submitted_at` so the chronological +/// merge sorts uniformly; `kind:"review"` + the review `state` tag the +/// entry for display. fn fetch_review_bodies(client: &Client, number: u64) -> Vec { let Ok((owner, name)) = client.owner_repo() else { return Vec::new(); @@ -268,7 +283,8 @@ fn fetch_review_bodies(client: &Client, number: u64) -> Vec { if state == "PENDING" { return None; } - if r.body.as_deref().unwrap_or("").trim().is_empty() { + let comments_count = r.comments_count.unwrap_or(0); + if r.body.as_deref().unwrap_or("").trim().is_empty() && comments_count == 0 { return None; } let submitted = rfc3339(r.submitted_at); @@ -281,6 +297,7 @@ fn fetch_review_bodies(client: &Client, number: u64) -> Vec { "html_url": r.html_url, "kind": "review", "state": state, + "comments_count": comments_count, })) }) .collect() diff --git a/hive-forge/src/verbs/pr_reviews.rs b/hive-forge/src/verbs/pr_reviews.rs index 5280bb5c..152f03dc 100644 --- a/hive-forge/src/verbs/pr_reviews.rs +++ b/hive-forge/src/verbs/pr_reviews.rs @@ -124,7 +124,7 @@ fn list_reviews_json(client: &Client, number: u64, reviews: &[Value]) -> Result< json!({ "id": c.get("id"), "path": c.get("path"), - "line": c.get("line"), + "line": c.get("position"), "body": c.get("body"), }) }) @@ -187,8 +187,11 @@ fn list_reviews_text(client: &Client, number: u64, reviews: &[Value]) { for c in &fetch_inline_comments(client, 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) { + // PR-level comments have no position; omit `:line` when + // absent (also true for a comment whose anchored line has + // since fallen out of the diff — forgejo drops `position` + // in that case too, same display fallback). + match c.get("position").and_then(Value::as_u64) { Some(line) => println!(" [{path}:{line}] {cbody}"), None => println!(" [{path}] {cbody}"), }