hive-forge: pr review-reply answers an inline review comment in its conversation
This commit is contained in:
parent
bef9d401a4
commit
66837e7c10
4 changed files with 192 additions and 3 deletions
|
|
@ -85,7 +85,7 @@ enum Verb {
|
|||
View(verbs::view::Args),
|
||||
/// Issue-scoped commands: `issue <show|create|edit|view|comment|comments|close|reopen|labels|assign|dependency|reaction|timeline> …`.
|
||||
Issue(verbs::issue_cmd::Args),
|
||||
/// PR-scoped commands: `pr <show|status|create|merge|reviews|assign-reviewer|commits|diff|view|edit|comment|comments|close|reopen|labels|assign-committer|dependency|reaction|timeline> …`.
|
||||
/// PR-scoped commands: `pr <show|status|create|merge|reviews|review-reply|assign-reviewer|commits|diff|view|edit|comment|comments|close|reopen|labels|assign-committer|dependency|reaction|timeline> …`.
|
||||
Pr(verbs::pr_cmd::Args),
|
||||
/// Post a comment on an issue or PR.
|
||||
#[command(hide = true)]
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ pub mod pr_commits;
|
|||
pub mod pr_create;
|
||||
pub mod pr_edit;
|
||||
pub mod pr_merge;
|
||||
pub mod pr_review_reply;
|
||||
pub mod pr_reviews;
|
||||
pub mod pr_status;
|
||||
pub mod reaction;
|
||||
|
|
|
|||
|
|
@ -33,6 +33,8 @@ enum Cmd {
|
|||
Merge(verbs::pr_merge::Args),
|
||||
/// List a PR's reviews, or submit one.
|
||||
Reviews(verbs::pr_reviews::Args),
|
||||
/// Answer an inline review comment, in its conversation.
|
||||
ReviewReply(verbs::pr_review_reply::Args),
|
||||
/// Request (or withdraw with `--remove`) a review from a user.
|
||||
AssignReviewer(verbs::pr_assign_reviewer::Args),
|
||||
/// Print the PR's unified diff.
|
||||
|
|
@ -73,6 +75,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
Cmd::Status(a) => verbs::pr_status::run(client, a),
|
||||
Cmd::Merge(a) => verbs::pr_merge::run(client, a),
|
||||
Cmd::Reviews(a) => verbs::pr_reviews::run(client, a),
|
||||
Cmd::ReviewReply(a) => verbs::pr_review_reply::run(client, a),
|
||||
Cmd::AssignReviewer(a) => verbs::pr_assign_reviewer::run(client, a),
|
||||
Cmd::Diff(a) => verbs::diff::run(client, a),
|
||||
// Generics shared with `issue` — verify the number is a PR first.
|
||||
|
|
|
|||
165
hive-forge/src/verbs/pr_review_reply.rs
Normal file
165
hive-forge/src/verbs/pr_review_reply.rs
Normal file
|
|
@ -0,0 +1,165 @@
|
|||
//! `pr review-reply <number> <comment-id>` - answer an inline review comment.
|
||||
//!
|
||||
//! Forgejo's API has no reply endpoint, so the reply is a one-comment
|
||||
//! `COMMENT` review anchored at the same path and line as the comment it
|
||||
//! answers; the forge shows comments with one anchor as one conversation.
|
||||
|
||||
use anyhow::{Result, bail};
|
||||
use clap::Args as ClapArgs;
|
||||
use forgejo_api::structs::{CreatePullReviewComment, CreatePullReviewOptions};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::body;
|
||||
use crate::client::{Client, index};
|
||||
use crate::verbs::pr_reviews::{comment_location, fetch_inline_comments};
|
||||
use crate::verbs::{all_pages, print_json};
|
||||
|
||||
#[derive(ClapArgs)]
|
||||
pub struct Args {
|
||||
/// PR number.
|
||||
number: u64,
|
||||
/// Id of the inline comment to answer: the `#id` that `pr reviews` shows.
|
||||
comment_id: i64,
|
||||
/// Inline body text.
|
||||
#[arg(long, conflicts_with = "body_file")]
|
||||
body: Option<String>,
|
||||
/// Read body from a file. `-` means stdin.
|
||||
#[arg(long = "body-file")]
|
||||
body_file: Option<String>,
|
||||
}
|
||||
|
||||
/// Where a reply has to be anchored to land in the same conversation.
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
struct Anchor {
|
||||
path: String,
|
||||
line: i64,
|
||||
extra_lines: i64,
|
||||
}
|
||||
|
||||
/// The anchor of `comment`, or an error when it has none to reuse.
|
||||
///
|
||||
/// Forgejo drops `position` when a push moved the line out of the diff, and
|
||||
/// then there is nothing to anchor a reply to.
|
||||
fn anchor_of(comment: &Value) -> Result<Anchor> {
|
||||
let line = comment
|
||||
.get("position")
|
||||
.and_then(Value::as_i64)
|
||||
.filter(|&l| l > 0);
|
||||
let path = comment
|
||||
.get("path")
|
||||
.and_then(Value::as_str)
|
||||
.filter(|p| !p.is_empty());
|
||||
let (Some(line), Some(path)) = (line, path) else {
|
||||
bail!(
|
||||
"comment at {} has no line position (a push moved it out of the diff), so a reply \
|
||||
cannot be anchored to it; post a normal PR comment instead",
|
||||
comment_location(comment)
|
||||
);
|
||||
};
|
||||
Ok(Anchor {
|
||||
path: path.to_owned(),
|
||||
line,
|
||||
extra_lines: comment
|
||||
.get("extra_lines_count")
|
||||
.and_then(Value::as_i64)
|
||||
.unwrap_or(0),
|
||||
})
|
||||
}
|
||||
|
||||
/// Find the inline comment with `id` among the PR's reviews.
|
||||
fn find_comment(client: &Client, number: u64, id: i64) -> Result<Value> {
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let reviews = all_pages(|page, size| {
|
||||
let (_, batch) = client
|
||||
.api()
|
||||
.repo_list_pull_reviews(owner, name, index(number)?)
|
||||
.page(page)
|
||||
.page_size(size)
|
||||
.send()?;
|
||||
Ok(batch)
|
||||
})?;
|
||||
for review in reviews {
|
||||
let Some(review_id) = review.id.filter(|&r| r > 0) else {
|
||||
continue;
|
||||
};
|
||||
let found = fetch_inline_comments(client, number, review_id)
|
||||
.into_iter()
|
||||
.find(|c| c.get("id").and_then(Value::as_i64) == Some(id));
|
||||
if let Some(c) = found {
|
||||
return Ok(c);
|
||||
}
|
||||
}
|
||||
bail!("no inline review comment #{id} on PR #{number}; `pr reviews {number}` lists their ids")
|
||||
}
|
||||
|
||||
/// # Errors
|
||||
///
|
||||
/// Fails when the comment is not on this PR, has no line position to anchor
|
||||
/// to, or the body is empty; propagates transport errors from the forge.
|
||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||
let text = body::resolve_required(
|
||||
args.body.as_deref(),
|
||||
args.body_file.as_deref(),
|
||||
"pr review-reply",
|
||||
)?;
|
||||
let target = find_comment(client, args.number, args.comment_id)?;
|
||||
let anchor = anchor_of(&target)?;
|
||||
let (owner, name) = client.owner_repo()?;
|
||||
let review = client
|
||||
.api()
|
||||
.repo_create_pull_review(
|
||||
owner,
|
||||
name,
|
||||
index(args.number)?,
|
||||
CreatePullReviewOptions {
|
||||
body: Some(String::new()),
|
||||
comments: Some(vec![CreatePullReviewComment {
|
||||
body: Some(text),
|
||||
extra_lines_count: Some(anchor.extra_lines),
|
||||
new_position: Some(anchor.line),
|
||||
old_position: Some(0),
|
||||
path: Some(anchor.path),
|
||||
}]),
|
||||
commit_id: target
|
||||
.get("commit_id")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_owned),
|
||||
event: Some("COMMENT".to_owned()),
|
||||
},
|
||||
)
|
||||
.send()?;
|
||||
print_json(&json!({
|
||||
"review": review.id,
|
||||
"in_reply_to": args.comment_id,
|
||||
"at": comment_location(&target),
|
||||
}))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn anchor_reuses_path_line_and_span() {
|
||||
let c = json!({"path": "src/a.rs", "position": 42, "extra_lines_count": 2});
|
||||
assert_eq!(
|
||||
anchor_of(&c).unwrap(),
|
||||
Anchor {
|
||||
path: "src/a.rs".into(),
|
||||
line: 42,
|
||||
extra_lines: 2
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn anchor_refuses_a_comment_without_position() {
|
||||
let c = json!({"path": "src/a.rs", "position": 0});
|
||||
let err = anchor_of(&c).unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("src/a.rs") && err.contains("no line position"),
|
||||
"{err}"
|
||||
);
|
||||
assert!(anchor_of(&json!({"position": 3})).is_err());
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue