From 95c0614ccd35dcd38074f053d66dea2c1c7c6644 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Wed, 3 Jul 2024 12:32:52 +0200 Subject: [PATCH] router: cli: improve doc-gen global options handling Passing the &GlobalOptions through is more telling than an opaque Iterator<&str>... Also: actually generate the property descriptions in non-ReST mode for global options as well, so the `help` output for a specific command includes the property documentation instead of only showing the name. Signed-off-by: Wolfgang Bumiller --- proxmox-router/src/cli/command.rs | 10 ++--- proxmox-router/src/cli/format.rs | 71 ++++++++++++++++++++----------- proxmox-router/src/cli/mod.rs | 2 +- proxmox-router/tests/docs.rs | 6 ++- 4 files changed, 55 insertions(+), 34 deletions(-) diff --git a/proxmox-router/src/cli/command.rs b/proxmox-router/src/cli/command.rs index 36d28ebe..b658e055 100644 --- a/proxmox-router/src/cli/command.rs +++ b/proxmox-router/src/cli/command.rs @@ -10,7 +10,7 @@ use super::environment::CliEnvironment; use super::getopts; use super::{ generate_nested_usage, generate_usage_str_do, print_help, print_nested_usage_error, - print_simple_usage_error_do, CliCommand, CliCommandMap, CommandLineInterface, + print_simple_usage_error_do, CliCommand, CliCommandMap, CommandLineInterface, GlobalOptions, }; use crate::{ApiFuture, ApiHandler, ApiMethod, RpcEnvironment}; @@ -28,11 +28,11 @@ pub const OUTPUT_FORMAT: Schema = StringSchema::new("Output format.") ])) .schema(); -fn parse_arguments( +fn parse_arguments<'cli>( prefix: &str, cli_cmd: &CliCommand, args: Vec, - global_options_iter: impl Iterator, + global_options_iter: impl Iterator, ) -> Result { let (params, remaining) = match getopts::parse_arguments( &args, @@ -94,13 +94,13 @@ async fn handle_simple_command_future( Ok(()) } -pub(crate) fn handle_simple_command( +pub(crate) fn handle_simple_command<'cli>( prefix: &str, cli_cmd: &CliCommand, args: Vec, rpcenv: &mut CliEnvironment, run: Option Result>, - global_options_iter: impl Iterator, + global_options_iter: impl Iterator, ) -> Result<(), Error> { let params = parse_arguments(prefix, cli_cmd, args, global_options_iter)?; diff --git a/proxmox-router/src/cli/format.rs b/proxmox-router/src/cli/format.rs index d8f72eaf..b3d2aa75 100644 --- a/proxmox-router/src/cli/format.rs +++ b/proxmox-router/src/cli/format.rs @@ -76,13 +76,13 @@ pub fn generate_usage_str( ) } -pub(crate) fn generate_usage_str_do( +pub(crate) fn generate_usage_str_do<'cli>( prefix: &str, cli_cmd: &CliCommand, format: DocumentationFormat, indent: &str, skip_options: &[&str], - global_options_iter: impl Iterator, + global_options_iter: impl Iterator, ) -> String { let arg_param = cli_cmd.arg_param; let fixed_param = &cli_cmd.fixed_param; @@ -212,23 +212,44 @@ pub(crate) fn generate_usage_str_do( } let mut global_options = String::new(); - for opt in global_options_iter { - use std::fmt::Write as _; - if done_hash.contains(opt) { + for (name, _optional, param_schema) in + global_options_iter.flat_map(|o| o.schema.any_object().unwrap().properties()) + { + if done_hash.contains(name) { continue; } - if !global_options.is_empty() { - if matches!(format, DocumentationFormat::ReST) { + if format == DocumentationFormat::ReST { + use std::fmt::Write as _; + + // In the ReST outputs we don't include the documentation for global options each time + // as this is mostly used for the man page and we have a separate section before + // labeled + // "Options available for command group :" + // which documents them fully. + // + // FIXME: Ideally we'd instead be able to tell the difference between generating the + // entire tree or just a single command's documentation to know whether to print all of + // them? + + if !global_options.is_empty() { global_options.push_str("\n\n"); - } else { + } + + let _ = write!(global_options, "``--{name}``"); + } else { + // For the other ones we use the same generation method as for any other option: + + if !global_options.is_empty() { global_options.push('\n'); } + global_options.push_str(&get_property_description( + name, + param_schema, + ParameterDisplayStyle::Arg, + format, + )); } - let _ = match format { - DocumentationFormat::ReST => write!(global_options, "``--{opt}``"), - _ => write!(global_options, "--{opt}"), - }; } if !global_options.is_empty() { @@ -246,11 +267,11 @@ pub fn print_simple_usage_error(prefix: &str, cli_cmd: &CliCommand, err_msg: &st } /// Print command usage for simple commands to ``stderr``. -pub(crate) fn print_simple_usage_error_do( +pub(crate) fn print_simple_usage_error_do<'cli>( prefix: &str, cli_cmd: &CliCommand, err_msg: &str, - global_options_iter: impl Iterator, + global_options_iter: impl Iterator, ) { let usage = generate_usage_str_do( prefix, @@ -271,14 +292,13 @@ pub fn print_nested_usage_error(prefix: &str, def: &CliCommandMap, err_msg: &str /// While going through nested commands, this keeps track of the available global options. #[derive(Default)] -struct UsageState { - global_options: Vec>, +struct UsageState<'cli> { + global_options: Vec>, } -impl UsageState { - fn push_global_options(&mut self, options: &HashMap) { - self.global_options - .push(options.values().map(|o| o.schema).collect()); +impl<'cli> UsageState<'cli> { + fn push_global_options(&mut self, options: &'cli HashMap) { + self.global_options.push(options.values().collect()); } fn pop_global_options(&mut self) { @@ -307,6 +327,7 @@ impl UsageState { let _ = write!(out, "Options available for command group ``{prefix}``:"); for opt in opts { for (name, _optional, schema) in opt + .schema .any_object() .expect("non-object schema in global optiosn") .properties() @@ -322,12 +343,10 @@ impl UsageState { out } - fn global_options_iter(&self) -> impl Iterator + '_ { + fn global_options_iter(&self) -> impl Iterator + '_ { self.global_options .iter() .flat_map(|list| list.iter().copied()) - .flat_map(|o| o.any_object().unwrap().properties()) - .map(|(name, _optional, _schema)| *name) } } @@ -340,10 +359,10 @@ pub fn generate_nested_usage( generate_nested_usage_do(&mut UsageState::default(), prefix, def, format) } -fn generate_nested_usage_do( - state: &mut UsageState, +fn generate_nested_usage_do<'cli>( + state: &mut UsageState<'cli>, prefix: &str, - def: &CliCommandMap, + def: &'cli CliCommandMap, format: DocumentationFormat, ) -> String { state.push_global_options(&def.global_options); diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs index 82eae27a..14f98b0c 100644 --- a/proxmox-router/src/cli/mod.rs +++ b/proxmox-router/src/cli/mod.rs @@ -550,7 +550,7 @@ impl<'cli> CommandLineParseState<'cli> { args, rpcenv, self.async_run, - self.global_option_schemas.keys().copied(), + self.global_option_types.values().copied(), ); command::set_help_context(None); out diff --git a/proxmox-router/tests/docs.rs b/proxmox-router/tests/docs.rs index 509b29a1..00b5d65a 100644 --- a/proxmox-router/tests/docs.rs +++ b/proxmox-router/tests/docs.rs @@ -136,8 +136,10 @@ Optional parameters: Inherited group parameters: ---global1 ---global2 + --global1 one|two + A global option. + --global2 + A second global option. "## .trim_start() }