From 11be286fa12bf6f52583c0da2f2bab623fab2dac Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Apr 2026 18:41:00 -0400 Subject: [PATCH] refactor(dev): simplify generated docs tooling Share the real CLI parser with reference generation, reuse option metadata flattening, and centralize dev command execution helpers. --- Cargo.lock | 3 +- docs/reference/cli.mdx | 5 +- lib/crates/fabro-cli/src/args.rs | 29 +++++- lib/crates/fabro-cli/src/lib.rs | 15 +-- lib/crates/fabro-cli/src/main.rs | 33 +------ lib/crates/fabro-config/src/layers/cli.rs | 2 +- lib/crates/fabro-config/src/layers/run.rs | 3 +- lib/crates/fabro-dev/Cargo.toml | 3 +- .../fabro-dev/src/commands/docker_build.rs | 21 +---- .../src/commands/generate_cli_reference.rs | 35 ++++--- .../commands/generate_options_reference.rs | 33 +------ lib/crates/fabro-dev/src/commands/mod.rs | 33 +++++-- .../fabro-dev/src/commands/refresh_spa.rs | 10 +- lib/crates/fabro-dev/src/commands/release.rs | 93 +++++++++---------- .../tests/it/generate_cli_reference.rs | 4 - lib/crates/fabro-dev/tests/it/main.rs | 31 +------ .../fabro-macros/src/options_metadata.rs | 15 ++- lib/crates/fabro-options-metadata/src/lib.rs | 86 ++++++++++++----- 18 files changed, 224 insertions(+), 230 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1cecdb551..f658447b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1784,8 +1784,9 @@ dependencies = [ "fabro-cli", "fabro-config", "fabro-options-metadata", - "regex", + "shlex", "tempfile", + "toml_edit", "tracing-subscriber", "walkdir", ] diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 8685ff392..92efb2c72 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -230,6 +230,7 @@ fabro auth login [OPTIONS] | Option | Description | | --- | --- | +| `--dev-token ` | Log in with a dev-token instead of browser OAuth | | `--no-browser` | Print the browser URL instead of opening it automatically | | `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | | `--timeout ` | Timeout in seconds waiting for the browser flow to complete
Default: `300` | @@ -972,8 +973,8 @@ fabro secret set [OPTIONS] [VALUE] | Option | Description | | --- | --- | -| `--description ` | TODO: add CLI help text. | -| `--type ` | Values: `environment`, `file`
Default: `environment` | +| `--description ` | Optional human-readable description | +| `--type ` | Secret storage type
Values: `environment`, `file`
Default: `environment` | | `--value-stdin` | Read the secret value from stdin | ### `fabro server` diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index e7b27b003..e076f5d20 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -2,7 +2,7 @@ use std::fmt; use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; -use clap::{Args, Subcommand, ValueEnum}; +use clap::{Args, Parser, Subcommand, ValueEnum}; use fabro_agent::cli::AgentArgs; use fabro_config::{CliLayer, CliLoggingLayer, CliOutputLayer, CliUpdatesLayer}; use fabro_server::serve::DEFAULT_TCP_PORT; @@ -21,6 +21,31 @@ pub(crate) const LONG_VERSION: &str = concat!( ")" ); +#[derive(Parser)] +#[command(name = "fabro", version, long_version = LONG_VERSION)] +pub(crate) struct Cli { + #[command(flatten)] + pub(crate) globals: GlobalArgs, + + #[command(subcommand)] + pub(crate) command: Option>, +} + +impl Cli { + pub(crate) fn parse() -> Self { + ::parse() + } + + #[cfg(test)] + pub(crate) fn try_parse_from(args: I) -> Result + where + I: IntoIterator, + T: Into + Clone, + { + ::try_parse_from(args) + } +} + #[derive(Args)] pub(crate) struct GlobalArgs { /// Output as JSON @@ -573,8 +598,10 @@ pub(crate) struct SecretSetArgs { /// Read the secret value from stdin #[arg(long, conflicts_with = "value")] pub(crate) value_stdin: bool, + /// Secret storage type #[arg(long, value_enum, default_value = "environment")] pub(crate) r#type: SecretTypeArg, + /// Optional human-readable description #[arg(long)] pub(crate) description: Option, } diff --git a/lib/crates/fabro-cli/src/lib.rs b/lib/crates/fabro-cli/src/lib.rs index 984557a48..efeecfd0a 100644 --- a/lib/crates/fabro-cli/src/lib.rs +++ b/lib/crates/fabro-cli/src/lib.rs @@ -5,19 +5,8 @@ mod args; -use args::{Commands, GlobalArgs, LONG_VERSION}; -use clap::{Command, CommandFactory, Parser}; - -#[derive(Parser)] -#[command(name = "fabro", version, long_version = LONG_VERSION)] -struct Cli { - #[command(flatten)] - globals: GlobalArgs, - - #[command(subcommand)] - command: Option>, -} +use clap::{Command, CommandFactory}; pub fn command_for_reference() -> Command { - Cli::command() + args::Cli::command() } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 43269ffe8..949ab7b90 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -18,16 +18,14 @@ mod shared; mod sleep_inhibitor; mod user_config; -#[cfg(test)] -use std::ffi::OsString; use std::fmt::{self, Debug, Display}; use anyhow::Result; use args::{ - Commands, GlobalArgs, LONG_VERSION, RunCommands, ServerCommand, ServerNamespace, - global_args_cli_layer, require_no_json_override, + Cli, Commands, RunCommands, ServerCommand, ServerNamespace, global_args_cli_layer, + require_no_json_override, }; -use clap::{CommandFactory, Parser}; +use clap::CommandFactory; use fabro_static::EnvVars; use fabro_telemetry::{git, panic as tel_panic, sanitize, sender}; use fabro_util::exit::ExitClass; @@ -39,31 +37,6 @@ use tracing::debug; use crate::command_context::CommandContext; -#[derive(Parser)] -#[command(name = "fabro", version, long_version = LONG_VERSION)] -struct Cli { - #[command(flatten)] - globals: GlobalArgs, - - #[command(subcommand)] - command: Option>, -} - -impl Cli { - fn parse() -> Self { - ::parse() - } - - #[cfg(test)] - fn try_parse_from(args: I) -> Result - where - I: IntoIterator, - T: Into + Clone, - { - ::try_parse_from(args) - } -} - #[expect(clippy::print_stderr, reason = "fatal error reporting before exit")] #[tokio::main] async fn main() { diff --git a/lib/crates/fabro-config/src/layers/cli.rs b/lib/crates/fabro-config/src/layers/cli.rs index 3476588ba..00a998190 100644 --- a/lib/crates/fabro-config/src/layers/cli.rs +++ b/lib/crates/fabro-config/src/layers/cli.rs @@ -64,7 +64,7 @@ pub struct CliAuthLayer { pub struct CliExecLayer { /// Prevent idle sleep on macOS while an exec run is in flight. #[serde(default, skip_serializing_if = "Option::is_none")] - #[option(name = "prevent_idle_sleep", default = "false", value_type = "boolean")] + #[option(default = "false", value_type = "boolean")] pub prevent_idle_sleep: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub model: Option, diff --git a/lib/crates/fabro-config/src/layers/run.rs b/lib/crates/fabro-config/src/layers/run.rs index 33cfedc6c..f89b23ed3 100644 --- a/lib/crates/fabro-config/src/layers/run.rs +++ b/lib/crates/fabro-config/src/layers/run.rs @@ -535,12 +535,11 @@ pub struct RunPullRequestLayer { /// Enable GitHub auto-merge for created pull requests. Implies `draft = /// false`. #[serde(default, skip_serializing_if = "Option::is_none")] - #[option(name = "auto_merge", default = "false", value_type = "boolean")] + #[option(default = "false", value_type = "boolean")] pub auto_merge: Option, /// Merge method to configure for the pull request. #[serde(default, skip_serializing_if = "Option::is_none")] #[option( - name = "merge_strategy", default = "\"squash\"", value_type = "\"merge\" | \"squash\" | \"rebase\"" )] diff --git a/lib/crates/fabro-dev/Cargo.toml b/lib/crates/fabro-dev/Cargo.toml index 4cac0b339..32558b585 100644 --- a/lib/crates/fabro-dev/Cargo.toml +++ b/lib/crates/fabro-dev/Cargo.toml @@ -20,7 +20,8 @@ clap.workspace = true fabro-cli = { path = "../fabro-cli" } fabro-config = { path = "../fabro-config" } fabro-options-metadata.workspace = true -regex.workspace = true +shlex = "1" +toml_edit.workspace = true tracing-subscriber.workspace = true walkdir.workspace = true diff --git a/lib/crates/fabro-dev/src/commands/docker_build.rs b/lib/crates/fabro-dev/src/commands/docker_build.rs index 6e012b0fd..60a2a1b91 100644 --- a/lib/crates/fabro-dev/src/commands/docker_build.rs +++ b/lib/crates/fabro-dev/src/commands/docker_build.rs @@ -4,7 +4,7 @@ use std::path::PathBuf; use anyhow::{Context, Result, bail}; use clap::{Args, ValueEnum}; -use super::{PlannedCommand, command, shell_arg, workspace_root}; +use super::{PlannedCommand, run_command, shell_arg, workspace_root}; const ZIG_VERSION: &str = "0.13.0"; @@ -103,7 +103,7 @@ impl DockerBuildPlan { self.arch.target() ); let build_command = self.build_command(); - self.run_command(&build_command)?; + run_command(&self.workspace_root, &build_command)?; println!("Extracting binary from builder cache..."); std::fs::create_dir_all(self.context_dir()).with_context(|| { @@ -113,7 +113,7 @@ impl DockerBuildPlan { ) })?; let extract_command = self.extract_command(); - self.run_command(&extract_command)?; + run_command(&self.workspace_root, &extract_command)?; if self.compile_only { println!( @@ -125,7 +125,7 @@ impl DockerBuildPlan { println!("Building Docker image as {}...", self.tag); let image_build_command = self.image_build_command(); - self.run_command(&image_build_command) + run_command(&self.workspace_root, &image_build_command) } fn dry_run_lines(&self) -> Vec { @@ -200,19 +200,6 @@ impl DockerBuildPlan { .arg(".") } - fn run_command(&self, planned: &PlannedCommand) -> Result<()> { - let status = command(planned) - .current_dir(&self.workspace_root) - .status() - .with_context(|| format!("running {}", planned.to_shell_line()))?; - - if !status.success() { - bail!("command failed with {status}: {}", planned.to_shell_line()); - } - - Ok(()) - } - fn context_dir(&self) -> PathBuf { self.workspace_root .join("docker-context") diff --git a/lib/crates/fabro-dev/src/commands/generate_cli_reference.rs b/lib/crates/fabro-dev/src/commands/generate_cli_reference.rs index a097f21af..dc755c8a9 100644 --- a/lib/crates/fabro-dev/src/commands/generate_cli_reference.rs +++ b/lib/crates/fabro-dev/src/commands/generate_cli_reference.rs @@ -30,7 +30,7 @@ pub(crate) fn generate_cli_reference(args: GenerateCliReferenceArgs) -> Result<( let path = root.join(CLI_REFERENCE_PATH); let current = std::fs::read_to_string(&path).with_context(|| format!("reading {}", path.display()))?; - let generated = render_cli_reference(fabro_cli::command_for_reference()); + let generated = render_cli_reference(fabro_cli::command_for_reference())?; let updated = replace_generated_region( ¤t, &generated, @@ -54,17 +54,22 @@ pub(crate) fn generate_cli_reference(args: GenerateCliReferenceArgs) -> Result<( Ok(()) } -fn render_cli_reference(mut command: Command) -> String { +fn render_cli_reference(mut command: Command) -> Result { command.build(); let mut output = String::new(); - render_command(&mut output, &command, &[], 2); - output.trim_end().to_string() + render_command(&mut output, &command, &[], 2)?; + Ok(output.trim_end().to_string()) } -fn render_command(output: &mut String, command: &Command, parents: &[&str], level: usize) { +fn render_command( + output: &mut String, + command: &Command, + parents: &[&str], + level: usize, +) -> Result<()> { if command.is_hide_set() { - return; + return Ok(()); } let path = command_path(command, parents); @@ -91,7 +96,7 @@ fn render_command(output: &mut String, command: &Command, parents: &[&str], leve output.push_str("| `"); output.push_str(&argument_name(arg)); output.push_str("` | "); - output.push_str(&arg_help(arg)); + output.push_str(&arg_help(arg, &path)?); output.push_str(" |\n"); } output.push('\n'); @@ -106,7 +111,7 @@ fn render_command(output: &mut String, command: &Command, parents: &[&str], leve output.push_str("| `"); output.push_str(&option_name(arg)); output.push_str("` | "); - output.push_str(&arg_help(arg)); + output.push_str(&arg_help(arg, &path)?); output.push_str(" |\n"); } output.push('\n'); @@ -135,8 +140,10 @@ fn render_command(output: &mut String, command: &Command, parents: &[&str], leve let mut next_parents = parents.to_vec(); next_parents.push(command.get_name()); for subcommand in visible_subcommands { - render_command(output, subcommand, &next_parents, level + 1); + render_command(output, subcommand, &next_parents, level + 1)?; } + + Ok(()) } fn command_path(command: &Command, parents: &[&str]) -> String { @@ -216,7 +223,7 @@ fn option_takes_value(arg: &Arg) -> bool { arg.get_num_args().is_some_and(|range| range.takes_values()) } -fn arg_help(arg: &Arg) -> String { +fn arg_help(arg: &Arg, command_path: &str) -> Result { let mut parts = Vec::new(); if let Some(help) = arg.get_long_help().or_else(|| arg.get_help()) { let help = help.to_string(); @@ -248,10 +255,12 @@ fn arg_help(arg: &Arg) -> String { } if parts.is_empty() { - "TODO: add CLI help text.".to_string() - } else { - parts.join("
") + bail!( + "{command_path} argument `{}` is missing help text", + arg.get_id() + ) } + Ok(parts.join("
")) } fn is_boolean_switch(arg: &Arg) -> bool { diff --git a/lib/crates/fabro-dev/src/commands/generate_options_reference.rs b/lib/crates/fabro-dev/src/commands/generate_options_reference.rs index 87b308daa..775e75bce 100644 --- a/lib/crates/fabro-dev/src/commands/generate_options_reference.rs +++ b/lib/crates/fabro-dev/src/commands/generate_options_reference.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use std::path::PathBuf; use anyhow::{Context, Result, bail}; -use fabro_options_metadata::{OptionField, OptionSet, Visit}; +use fabro_options_metadata::{OptionField, OptionSet}; use super::{markdown_cell, replace_generated_region, workspace_root}; @@ -159,7 +159,7 @@ fn render_section(output: &mut String, section: &Section) { output.push_str("```toml title=\"settings.toml\"\n"); output.push_str(section.example); output.push_str("\n```\n\n"); - render_field_table(output, collect_fields(section.set)); + render_field_table(output, section.set.fields()); } fn render_field_table(output: &mut String, fields: BTreeMap) { @@ -181,35 +181,6 @@ fn render_field_table(output: &mut String, fields: BTreeMap output.push('\n'); } -fn collect_fields(set: OptionSet) -> BTreeMap { - struct CollectVisitor<'a> { - prefix: String, - entries: &'a mut BTreeMap, - } - - impl Visit for CollectVisitor<'_> { - fn record_field(&mut self, name: &str, field: OptionField) { - self.entries - .insert(format!("{}{}", self.prefix, name), field); - } - - fn record_set(&mut self, name: &str, set: OptionSet) { - let previous = self.prefix.clone(); - self.prefix.push_str(name); - self.prefix.push('.'); - set.record(self); - self.prefix = previous; - } - } - - let mut entries = BTreeMap::new(); - set.record(&mut CollectVisitor { - prefix: String::new(), - entries: &mut entries, - }); - entries -} - fn field_type(field: &OptionField) -> String { if let Some(possible_values) = field .possible_values diff --git a/lib/crates/fabro-dev/src/commands/mod.rs b/lib/crates/fabro-dev/src/commands/mod.rs index 36ac119f1..4fbaef977 100644 --- a/lib/crates/fabro-dev/src/commands/mod.rs +++ b/lib/crates/fabro-dev/src/commands/mod.rs @@ -6,7 +6,7 @@ mod refresh_spa; mod release; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Output}; use anyhow::{Context, Result}; pub(crate) use check_spa_budgets::{CheckSpaBudgetsArgs, check_spa_budgets}; @@ -123,14 +123,29 @@ pub(crate) fn command(planned: &PlannedCommand) -> Command { command } -pub(crate) fn shell_arg(arg: impl AsRef) -> String { - let arg = arg.as_ref(); - if arg - .chars() - .all(|ch| ch.is_ascii_alphanumeric() || "_-./:=@".contains(ch)) - { - return arg.to_string(); +pub(crate) fn run_command(cwd: &Path, planned: &PlannedCommand) -> Result<()> { + let status = command(planned) + .current_dir(cwd) + .status() + .with_context(|| format!("running {}", planned.to_shell_line()))?; + if !status.success() { + anyhow::bail!("command failed with {status}: {}", planned.to_shell_line()); } - format!("'{}'", arg.replace('\'', "'\\''")) + Ok(()) +} + +pub(crate) fn capture_command(cwd: &Path, planned: &PlannedCommand) -> Result { + command(planned) + .current_dir(cwd) + .output() + .with_context(|| format!("running {}", planned.to_shell_line())) +} + +pub(crate) fn shell_arg(arg: impl AsRef) -> String { + let arg = arg.as_ref(); + shlex::try_quote(arg).map_or_else( + |_| format!("'{}'", arg.replace('\'', "'\\''")), + std::borrow::Cow::into_owned, + ) } diff --git a/lib/crates/fabro-dev/src/commands/refresh_spa.rs b/lib/crates/fabro-dev/src/commands/refresh_spa.rs index b6f739027..57aee06ad 100644 --- a/lib/crates/fabro-dev/src/commands/refresh_spa.rs +++ b/lib/crates/fabro-dev/src/commands/refresh_spa.rs @@ -17,17 +17,21 @@ pub(crate) struct RefreshSpaArgs { skip_build: bool, } +pub(crate) fn refresh_spa(args: RefreshSpaArgs) -> Result<()> { + let root = args.root.unwrap_or_else(workspace_root); + refresh_spa_root(&root, args.skip_build) +} + #[expect( clippy::print_stdout, reason = "dev refresh-spa command reports progress directly" )] -pub(crate) fn refresh_spa(args: RefreshSpaArgs) -> Result<()> { - let root = args.root.unwrap_or_else(workspace_root); +pub(super) fn refresh_spa_root(root: &Path, skip_build: bool) -> Result<()> { let web_dir = root.join("apps/fabro-web"); let dist_dir = web_dir.join("dist"); let asset_dir = root.join("lib/crates/fabro-spa/assets"); - if !args.skip_build { + if !skip_build { println!("Running bun run build in apps/fabro-web..."); run_bun_build(&web_dir)?; } diff --git a/lib/crates/fabro-dev/src/commands/release.rs b/lib/crates/fabro-dev/src/commands/release.rs index f6bcb8283..dbbeda0b6 100644 --- a/lib/crates/fabro-dev/src/commands/release.rs +++ b/lib/crates/fabro-dev/src/commands/release.rs @@ -1,12 +1,12 @@ use std::fmt; use std::path::{Path, PathBuf}; -use std::process::Output; use anyhow::{Context, Result, bail}; use chrono::{Local, NaiveDate}; use clap::{Args, ValueEnum}; -use super::{PlannedCommand, command, workspace_root}; +use super::refresh_spa::refresh_spa_root; +use super::{PlannedCommand, capture_command, run_command, workspace_root}; const RELEASE_EPOCH: &str = "2026-01-01"; const RELEASE_TEST_SEGMENT_WRITE_KEY: &str = "fake-for-local-smoke"; @@ -86,26 +86,30 @@ pub(crate) fn release(args: ReleaseArgs) -> Result<()> { update_version(&cargo_toml, ¤t_version, &new_version)?; println!("Updated {}", cargo_toml.display()); - plan.run_command( + run_command( + &plan.root, &PlannedCommand::new("cargo") .arg("update") .arg("--workspace"), )?; println!("Updated Cargo.lock"); - plan.run_command( + run_command( + &plan.root, &PlannedCommand::new("git") .arg("add") .arg("Cargo.toml") .arg("Cargo.lock"), )?; - plan.run_command( + run_command( + &plan.root, &PlannedCommand::new("git") .arg("commit") .arg("-m") .arg(format!("Bump version to {new_version}")), )?; - plan.run_command( + run_command( + &plan.root, &PlannedCommand::new("git") .arg("tag") .arg("-a") @@ -113,7 +117,8 @@ pub(crate) fn release(args: ReleaseArgs) -> Result<()> { .arg("-m") .arg(&tag), )?; - plan.run_command( + run_command( + &plan.root, &PlannedCommand::new("git") .arg("push") .arg("origin") @@ -167,7 +172,8 @@ impl ReleasePlan { } fn ensure_clean_worktree(&self) -> Result<()> { - let output = self.capture_command( + let output = capture_command( + &self.root, &PlannedCommand::new("git") .arg("status") .arg("--porcelain") @@ -187,8 +193,8 @@ impl ReleasePlan { } fn verify_spa_assets(&self) -> Result<()> { - self.run_command(&Self::refresh_spa_command())?; - let output = self.capture_command(&Self::spa_assets_diff_command())?; + refresh_spa_root(&self.root, false)?; + let output = capture_command(&self.root, &Self::spa_assets_diff_command())?; if !output.status.success() { bail!("fabro-spa assets are stale. Commit the refreshed assets before releasing."); } @@ -207,7 +213,7 @@ impl ReleasePlan { } println!("Running release-mode test smoke (SEGMENT_WRITE_KEY baked in)..."); - self.run_command(&Self::release_tests_command()) + run_command(&self.root, &Self::release_tests_command()) } #[expect( @@ -283,7 +289,8 @@ impl ReleasePlan { } fn tag_exists(&self, tag: &str) -> Result { - let output = self.capture_command( + let output = capture_command( + &self.root, &PlannedCommand::new("git") .arg("rev-parse") .arg("--verify") @@ -292,25 +299,6 @@ impl ReleasePlan { )?; Ok(output.status.success()) } - - fn run_command(&self, planned: &PlannedCommand) -> Result<()> { - let status = command(planned) - .current_dir(&self.root) - .status() - .with_context(|| format!("running {}", planned.to_shell_line()))?; - if !status.success() { - bail!("command failed with {status}: {}", planned.to_shell_line()); - } - - Ok(()) - } - - fn capture_command(&self, planned: &PlannedCommand) -> Result { - command(planned) - .current_dir(&self.root) - .output() - .with_context(|| format!("running {}", planned.to_shell_line())) - } } #[expect( @@ -320,20 +308,10 @@ impl ReleasePlan { fn read_current_version(cargo_toml: &Path) -> Result { let contents = std::fs::read_to_string(cargo_toml) .with_context(|| format!("reading {}", cargo_toml.display()))?; - for line in contents.lines().map(str::trim) { - let Some(rest) = line.strip_prefix("version = \"") else { - continue; - }; - let Some(version) = rest.strip_suffix('"') else { - continue; - }; - return Ok(version.to_string()); - } - - bail!( - "could not find workspace package version in {}", - cargo_toml.display() - ) + let manifest = contents + .parse::() + .with_context(|| format!("parsing {}", cargo_toml.display()))?; + workspace_package_version(&manifest, cargo_toml).map(ToOwned::to_owned) } #[expect( @@ -343,15 +321,32 @@ fn read_current_version(cargo_toml: &Path) -> Result { fn update_version(cargo_toml: &Path, current_version: &str, new_version: &str) -> Result<()> { let contents = std::fs::read_to_string(cargo_toml) .with_context(|| format!("reading {}", cargo_toml.display()))?; - let needle = format!("version = \"{current_version}\""); - let replacement = format!("version = \"{new_version}\""); - if !contents.contains(&needle) { + let mut manifest = contents + .parse::() + .with_context(|| format!("parsing {}", cargo_toml.display()))?; + let version = workspace_package_version(&manifest, cargo_toml)?; + if version != current_version { bail!( "could not find current version {current_version} in {}", cargo_toml.display() ); } - std::fs::write(cargo_toml, contents.replacen(&needle, &replacement, 1)) + manifest["workspace"]["package"]["version"] = toml_edit::value(new_version); + std::fs::write(cargo_toml, manifest.to_string()) .with_context(|| format!("writing {}", cargo_toml.display())) } + +fn workspace_package_version<'a>( + manifest: &'a toml_edit::DocumentMut, + cargo_toml: &Path, +) -> Result<&'a str> { + manifest["workspace"]["package"]["version"] + .as_str() + .with_context(|| { + format!( + "could not find [workspace.package] version in {}", + cargo_toml.display() + ) + }) +} diff --git a/lib/crates/fabro-dev/tests/it/generate_cli_reference.rs b/lib/crates/fabro-dev/tests/it/generate_cli_reference.rs index f06597f6a..ab54cab2a 100644 --- a/lib/crates/fabro-dev/tests/it/generate_cli_reference.rs +++ b/lib/crates/fabro-dev/tests/it/generate_cli_reference.rs @@ -47,10 +47,6 @@ Tail copy. contents.contains("### `fabro run`"), "generated output should include subcommand reference:\n{contents}" ); - assert!( - contents.contains("TODO: add CLI help text."), - "undocumented clap args should be visible follow-up work:\n{contents}" - ); assert!( !contents.contains("stale"), "stale generated content should be replaced:\n{contents}" diff --git a/lib/crates/fabro-dev/tests/it/main.rs b/lib/crates/fabro-dev/tests/it/main.rs index c0f3e0375..dc1f83a76 100644 --- a/lib/crates/fabro-dev/tests/it/main.rs +++ b/lib/crates/fabro-dev/tests/it/main.rs @@ -1,5 +1,4 @@ use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; mod docker_build; mod generate_cli_reference; @@ -42,19 +41,6 @@ fn read_file(root: &Path, path: &str) -> String { std::fs::read_to_string(root.join(path)).expect("reading fixture file") } -#[expect( - clippy::disallowed_methods, - reason = "integration test intentionally shells out to Cargo to verify the cargo dev alias" -)] -fn cargo_dev(args: &[&str]) -> Output { - Command::new("cargo") - .arg("dev") - .args(args) - .current_dir(workspace_root()) - .output() - .expect("cargo dev should run") -} - #[test] fn help_lists_scaffolded_commands() { let output = fabro_dev() @@ -81,20 +67,11 @@ fn help_lists_scaffolded_commands() { } #[test] -fn cargo_dev_alias_resolves_to_fabro_dev_help() { - let output = cargo_dev(&["--help"]); - +fn cargo_dev_alias_points_at_fabro_dev() { + let config = read_file(&workspace_root(), ".cargo/config.toml"); assert!( - output.status.success(), - "cargo dev --help failed\nstdout:\n{}\nstderr:\n{}", - output_text(&output.stdout), - output_text(&output.stderr) - ); - - let stdout = output_text(&output.stdout); - assert!( - stdout.contains("docker-build"), - "cargo dev help should come from fabro-dev:\n{stdout}" + config.contains(r#"dev = "run --package fabro-dev --""#), + "cargo dev alias should invoke fabro-dev:\n{config}" ); } diff --git a/lib/crates/fabro-macros/src/options_metadata.rs b/lib/crates/fabro-macros/src/options_metadata.rs index 69fbd0801..79e2a0b14 100644 --- a/lib/crates/fabro-macros/src/options_metadata.rs +++ b/lib/crates/fabro-macros/src/options_metadata.rs @@ -81,7 +81,7 @@ fn handle_option(field: &Field, attr: &Attribute) -> syn::Result { .name .clone() .or(option_long_name(field)?) - .unwrap_or_else(|| ident.to_string().replace('_', "-")); + .unwrap_or_else(|| ident.to_string()); let doc = quote_option_str(doc_string(&field.attrs)?); let default = quote_option_str(attrs.default); let value_type = quote_option_str(attrs.value_type); @@ -409,4 +409,17 @@ mod tests { .contains("unsupported `option` metadata key") ); } + + #[test] + fn option_metadata_defaults_to_field_name_without_clap_long() { + let input: DeriveInput = syn::parse_quote! { + struct Args { + #[option] + prevent_idle_sleep: bool, + } + }; + + let tokens = derive_impl(input).expect("metadata should derive"); + assert!(tokens.to_string().contains("\"prevent_idle_sleep\"")); + } } diff --git a/lib/crates/fabro-options-metadata/src/lib.rs b/lib/crates/fabro-options-metadata/src/lib.rs index fbd3b4657..34739a7a9 100644 --- a/lib/crates/fabro-options-metadata/src/lib.rs +++ b/lib/crates/fabro-options-metadata/src/lib.rs @@ -130,6 +130,36 @@ impl OptionSet { self.record(&mut visitor); visitor.entry } + + /// Returns all field entries flattened by dotted name. + pub fn fields(&self) -> BTreeMap { + struct FieldsVisitor<'a> { + entries: &'a mut BTreeMap, + prefix: String, + } + + impl Visit for FieldsVisitor<'_> { + fn record_field(&mut self, name: &str, field: OptionField) { + self.entries + .insert(format!("{}{}", self.prefix, name), field); + } + + fn record_set(&mut self, name: &str, set: OptionSet) { + let previous = self.prefix.clone(); + self.prefix.push_str(name); + self.prefix.push('.'); + set.record(self); + self.prefix = previous; + } + } + + let mut entries = BTreeMap::new(); + self.record(&mut FieldsVisitor { + entries: &mut entries, + prefix: String::new(), + }); + entries + } } impl PartialEq for OptionSet { @@ -185,31 +215,7 @@ impl Serialize for OptionSet { where S: Serializer, { - struct SerializeVisitor<'a> { - entries: &'a mut BTreeMap, - } - - impl Visit for SerializeVisitor<'_> { - fn record_field(&mut self, name: &str, field: OptionField) { - self.entries.insert(name.to_string(), field); - } - - fn record_set(&mut self, name: &str, set: OptionSet) { - let mut nested = BTreeMap::new(); - set.record(&mut SerializeVisitor { - entries: &mut nested, - }); - for (key, value) in nested { - self.entries.insert(format!("{name}.{key}"), value); - } - } - } - - let mut entries = BTreeMap::new(); - self.record(&mut SerializeVisitor { - entries: &mut entries, - }); - entries.serialize(serializer) + self.fields().serialize(serializer) } } @@ -396,6 +402,36 @@ mod tests { ); } + #[test] + fn option_set_fields_flatten_nested_fields_with_dot_keys() { + struct Root; + struct Nested; + + impl OptionsMetadata for Root { + fn record(visit: &mut dyn Visit) { + visit.record_field("verbose", field(Some("Enable verbose output."))); + visit.record_set("nested", Nested::metadata()); + } + } + + impl OptionsMetadata for Nested { + fn record(visit: &mut dyn Visit) { + visit.record_field("dry-run", field(Some("Preview the work."))); + } + } + + let fields = Root::metadata().fields(); + assert_eq!(fields.len(), 2); + assert_eq!( + fields.get("nested.dry-run"), + Some(&field(Some("Preview the work."))) + ); + assert_eq!( + fields.get("verbose"), + Some(&field(Some("Enable verbose output."))) + ); + } + #[test] fn field_doc_can_be_absent() { struct Root;