From 07d4aee67ce1411e286753ae81f65c7df1580418 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 23 Apr 2026 07:28:54 -0400 Subject: [PATCH] simplify: drop redundant CommandContext derivations and guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ssh/graph: remove `explicit_json_requested() &&` guard before `require_no_json_override()` (the call already no-ops without --json). - pr close/merge/view: drop the outer `with_target` derivation that was used only for `printer()` and the output format — both match base_ctx, so the derivation was an unused disk-read + settings re-merge. - command_context tests: collapse `synthetic_context` to delegate to `synthetic_context_with_settings`, removing duplicated struct literals. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-cli/src/command_context.rs | 34 ++++++++----------- lib/crates/fabro-cli/src/commands/graph.rs | 2 +- lib/crates/fabro-cli/src/commands/pr/close.rs | 5 ++- lib/crates/fabro-cli/src/commands/pr/merge.rs | 5 ++- lib/crates/fabro-cli/src/commands/pr/view.rs | 5 ++- lib/crates/fabro-cli/src/commands/run/ssh.rs | 2 +- 6 files changed, 23 insertions(+), 30 deletions(-) diff --git a/lib/crates/fabro-cli/src/command_context.rs b/lib/crates/fabro-cli/src/command_context.rs index 60348841d..6e3c48dfa 100644 --- a/lib/crates/fabro-cli/src/command_context.rs +++ b/lib/crates/fabro-cli/src/command_context.rs @@ -228,25 +228,6 @@ mod tests { } } - fn synthetic_context(process_local_json: bool, printer: Printer) -> CommandContext { - let cli_layer = cli_layer_with_json_and_verbose(); - let (machine_settings, user_settings) = - merge_settings_layer(parse_settings_layer("_version = 1\n").unwrap(), &cli_layer) - .expect("settings should merge"); - - CommandContext { - printer, - process_local_json, - cwd: PathBuf::from("/tmp/workspace"), - base_config_path: PathBuf::from("/tmp/settings.toml"), - cli_layer, - machine_settings, - user_settings, - server_mode: ServerMode::None, - server: OnceCell::new(), - } - } - fn synthetic_context_with_settings( process_local_json: bool, printer: Printer, @@ -268,6 +249,21 @@ mod tests { } } + fn synthetic_context(process_local_json: bool, printer: Printer) -> CommandContext { + let cli_layer = cli_layer_with_json_and_verbose(); + let (machine_settings, user_settings) = + merge_settings_layer(parse_settings_layer("_version = 1\n").unwrap(), &cli_layer) + .expect("settings should merge"); + synthetic_context_with_settings( + process_local_json, + printer, + cli_layer, + machine_settings, + user_settings, + ServerMode::None, + ) + } + #[test] fn context_exposes_resolved_output_and_explicit_json_state() { let ctx = synthetic_context(true, Printer::Default); diff --git a/lib/crates/fabro-cli/src/commands/graph.rs b/lib/crates/fabro-cli/src/commands/graph.rs index 066d41cad..5848cb845 100644 --- a/lib/crates/fabro-cli/src/commands/graph.rs +++ b/lib/crates/fabro-cli/src/commands/graph.rs @@ -29,7 +29,7 @@ pub(crate) async fn run( styles: &Styles, base_ctx: &CommandContext, ) -> anyhow::Result<()> { - if base_ctx.explicit_json_requested() && args.output.is_none() { + if args.output.is_none() { base_ctx.require_no_json_override()?; } diff --git a/lib/crates/fabro-cli/src/commands/pr/close.rs b/lib/crates/fabro-cli/src/commands/pr/close.rs index cdc9bcba7..2d9975709 100644 --- a/lib/crates/fabro-cli/src/commands/pr/close.rs +++ b/lib/crates/fabro-cli/src/commands/pr/close.rs @@ -7,8 +7,7 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(super) async fn close_command(args: PrCloseArgs, base_ctx: &CommandContext) -> Result<()> { - let ctx = base_ctx.with_target(&args.server)?; - let printer = ctx.printer(); + let printer = base_ctx.printer(); let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; let creds = super::load_github_credentials_required(base_ctx)?; @@ -24,7 +23,7 @@ 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 ctx.user_settings().cli.output.format == OutputFormat::Json { + if base_ctx.user_settings().cli.output.format == OutputFormat::Json { print_json_pretty(&serde_json::json!({ "number": record.number, "html_url": record.html_url, diff --git a/lib/crates/fabro-cli/src/commands/pr/merge.rs b/lib/crates/fabro-cli/src/commands/pr/merge.rs index ac043b654..9966e3169 100644 --- a/lib/crates/fabro-cli/src/commands/pr/merge.rs +++ b/lib/crates/fabro-cli/src/commands/pr/merge.rs @@ -7,8 +7,7 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(super) async fn merge_command(args: PrMergeArgs, base_ctx: &CommandContext) -> Result<()> { - let ctx = base_ctx.with_target(&args.server)?; - let printer = ctx.printer(); + let printer = base_ctx.printer(); let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; let creds = super::load_github_credentials_required(base_ctx)?; @@ -25,7 +24,7 @@ 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 ctx.user_settings().cli.output.format == OutputFormat::Json { + if base_ctx.user_settings().cli.output.format == OutputFormat::Json { print_json_pretty(&serde_json::json!({ "number": record.number, "html_url": record.html_url, diff --git a/lib/crates/fabro-cli/src/commands/pr/view.rs b/lib/crates/fabro-cli/src/commands/pr/view.rs index e0f934d15..8e96729e7 100644 --- a/lib/crates/fabro-cli/src/commands/pr/view.rs +++ b/lib/crates/fabro-cli/src/commands/pr/view.rs @@ -7,8 +7,7 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(super) async fn view_command(args: PrViewArgs, base_ctx: &CommandContext) -> Result<()> { - let ctx = base_ctx.with_target(&args.server)?; - let printer = ctx.printer(); + let printer = base_ctx.printer(); let (record, _run_id) = super::load_pr_record(&args.server, &args.run_id, base_ctx).await?; let creds = super::load_github_credentials_required(base_ctx)?; @@ -25,7 +24,7 @@ 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 ctx.user_settings().cli.output.format == OutputFormat::Json { + if base_ctx.user_settings().cli.output.format == OutputFormat::Json { print_json_pretty(&detail)?; return Ok(()); } diff --git a/lib/crates/fabro-cli/src/commands/run/ssh.rs b/lib/crates/fabro-cli/src/commands/run/ssh.rs index 537c781e9..8ed85380b 100644 --- a/lib/crates/fabro-cli/src/commands/run/ssh.rs +++ b/lib/crates/fabro-cli/src/commands/run/ssh.rs @@ -7,7 +7,7 @@ use crate::command_context::CommandContext; use crate::shared::print_json_pretty; pub(crate) async fn run(args: SshArgs, base_ctx: &CommandContext) -> Result<()> { - if base_ctx.explicit_json_requested() && !args.print { + if !args.print { base_ctx.require_no_json_override()?; }