From b98513875a2da2042a6b1d6546f78c27a49ffd30 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 23 Apr 2026 08:00:02 -0400 Subject: [PATCH] simplify: return derived context from load_pr_record `load_pr_record` built a `with_target`-derived context internally and threw it away, forcing callers to re-derive or fall back to `base_ctx`. Return the context alongside the record and let close/merge/view use it directly for printer/json access and github-credentials lookup. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-cli/src/commands/pr/close.rs | 15 ++++++++++----- lib/crates/fabro-cli/src/commands/pr/merge.rs | 15 ++++++++++----- lib/crates/fabro-cli/src/commands/pr/mod.rs | 4 ++-- lib/crates/fabro-cli/src/commands/pr/view.rs | 9 +++++---- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/pr/close.rs b/lib/crates/fabro-cli/src/commands/pr/close.rs index bf584b45c..034467526 100644 --- a/lib/crates/fabro-cli/src/commands/pr/close.rs +++ b/lib/crates/fabro-cli/src/commands/pr/close.rs @@ -6,10 +6,10 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(super) async fn close_command(args: PrCloseArgs, base_ctx: &CommandContext) -> Result<()> { - let printer = base_ctx.printer(); - let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; + let (ctx, record, _run_id) = + super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; - let creds = super::load_github_credentials_required(base_ctx)?; + let creds = super::load_github_credentials_required(&ctx)?; fabro_github::close_pull_request( &creds, @@ -22,13 +22,18 @@ pub(super) async fn close_command(args: PrCloseArgs, base_ctx: &CommandContext) .map_err(|err| anyhow::anyhow!("{err}"))?; info!(number = record.number, owner = %record.owner, repo = %record.repo, "Closed pull request"); - if base_ctx.json_output() { + if ctx.json_output() { print_json_pretty(&serde_json::json!({ "number": record.number, "html_url": record.html_url, }))?; } else { - fabro_util::printout!(printer, "Closed #{} ({})", record.number, record.html_url); + fabro_util::printout!( + ctx.printer(), + "Closed #{} ({})", + record.number, + record.html_url + ); } Ok(()) diff --git a/lib/crates/fabro-cli/src/commands/pr/merge.rs b/lib/crates/fabro-cli/src/commands/pr/merge.rs index 2c7de4a0a..c34d5b4fc 100644 --- a/lib/crates/fabro-cli/src/commands/pr/merge.rs +++ b/lib/crates/fabro-cli/src/commands/pr/merge.rs @@ -6,10 +6,10 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(super) async fn merge_command(args: PrMergeArgs, base_ctx: &CommandContext) -> Result<()> { - let printer = base_ctx.printer(); - let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; + let (ctx, record, _run_id) = + super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; - let creds = super::load_github_credentials_required(base_ctx)?; + let creds = super::load_github_credentials_required(&ctx)?; fabro_github::merge_pull_request( &creds, @@ -23,14 +23,19 @@ pub(super) async fn merge_command(args: PrMergeArgs, base_ctx: &CommandContext) .map_err(|err| anyhow::anyhow!("{err}"))?; info!(number = record.number, owner = %record.owner, repo = %record.repo, method = %args.method, "Merged pull request"); - if base_ctx.json_output() { + if ctx.json_output() { print_json_pretty(&serde_json::json!({ "number": record.number, "html_url": record.html_url, "method": args.method, }))?; } else { - fabro_util::printout!(printer, "Merged #{} ({})", record.number, record.html_url); + fabro_util::printout!( + ctx.printer(), + "Merged #{} ({})", + record.number, + record.html_url + ); } Ok(()) diff --git a/lib/crates/fabro-cli/src/commands/pr/mod.rs b/lib/crates/fabro-cli/src/commands/pr/mod.rs index a0c83c21b..d5fd6c7e7 100644 --- a/lib/crates/fabro-cli/src/commands/pr/mod.rs +++ b/lib/crates/fabro-cli/src/commands/pr/mod.rs @@ -58,7 +58,7 @@ pub(crate) async fn load_pr_record( server: &ServerTargetArgs, run_id: &str, base_ctx: &CommandContext, -) -> Result<(PullRequestRecord, fabro_types::RunId)> { +) -> Result<(CommandContext, PullRequestRecord, fabro_types::RunId)> { let ctx = base_ctx.with_target(server)?; let client = ctx.server().await?; let run_id = client.resolve_run(run_id).await?.run_id; @@ -66,5 +66,5 @@ pub(crate) async fn load_pr_record( let record = state.pull_request.with_context(|| { format!("No pull request found in store. Create one first with: fabro pr create {run_id}") })?; - Ok((record, run_id)) + Ok((ctx, record, run_id)) } diff --git a/lib/crates/fabro-cli/src/commands/pr/view.rs b/lib/crates/fabro-cli/src/commands/pr/view.rs index e861e19d1..0fcf343a6 100644 --- a/lib/crates/fabro-cli/src/commands/pr/view.rs +++ b/lib/crates/fabro-cli/src/commands/pr/view.rs @@ -6,10 +6,10 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(super) async fn view_command(args: PrViewArgs, base_ctx: &CommandContext) -> Result<()> { - let printer = base_ctx.printer(); - let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; + let (ctx, record, _run_id) = + super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; - let creds = super::load_github_credentials_required(base_ctx)?; + let creds = super::load_github_credentials_required(&ctx)?; let detail = fabro_github::get_pull_request( &creds, @@ -23,11 +23,12 @@ pub(super) async fn view_command(args: PrViewArgs, base_ctx: &CommandContext) -> info!(number = detail.number, owner = %record.owner, repo = %record.repo, "Viewing pull request"); - if base_ctx.json_output() { + if ctx.json_output() { print_json_pretty(&detail)?; return Ok(()); } + let printer = ctx.printer(); fabro_util::printout!(printer, "#{} {}", detail.number, detail.title); let state_display = if detail.draft { "draft" } else { &detail.state }; fabro_util::printout!(printer, "State: {state_display}");