diff --git a/proxmox-router/Cargo.toml b/proxmox-router/Cargo.toml index 8630aa7c..aae38194 100644 --- a/proxmox-router/Cargo.toml +++ b/proxmox-router/Cargo.toml @@ -9,6 +9,12 @@ description = "proxmox API Router and CLI utilities" exclude.workspace = true +[[test]] +name = "docs" +path = "tests/docs.rs" +test = true +required-features = [ "cli" ] + [dependencies] anyhow.workspace = true env_logger = { workspace = true, optional = true } diff --git a/proxmox-router/src/cli/command.rs b/proxmox-router/src/cli/command.rs index 36fe829c..36d28ebe 100644 --- a/proxmox-router/src/cli/command.rs +++ b/proxmox-router/src/cli/command.rs @@ -1,4 +1,4 @@ -use anyhow::{format_err, Error}; +use anyhow::{bail, format_err, Error}; use serde_json::Value; use std::cell::RefCell; use std::sync::Arc; @@ -9,8 +9,8 @@ use proxmox_schema::*; use super::environment::CliEnvironment; use super::getopts; use super::{ - generate_nested_usage, generate_usage_str, print_help, print_nested_usage_error, - print_simple_usage_error, CliCommand, CliCommandMap, CommandLineInterface, + generate_nested_usage, generate_usage_str_do, print_help, print_nested_usage_error, + print_simple_usage_error_do, CliCommand, CliCommandMap, CommandLineInterface, }; use crate::{ApiFuture, ApiHandler, ApiMethod, RpcEnvironment}; @@ -28,7 +28,12 @@ pub const OUTPUT_FORMAT: Schema = StringSchema::new("Output format.") ])) .schema(); -fn parse_arguments(prefix: &str, cli_cmd: &CliCommand, args: Vec) -> Result { +fn parse_arguments( + prefix: &str, + cli_cmd: &CliCommand, + args: Vec, + global_options_iter: impl Iterator, +) -> Result { let (params, remaining) = match getopts::parse_arguments( &args, cli_cmd.arg_param, @@ -38,14 +43,14 @@ fn parse_arguments(prefix: &str, cli_cmd: &CliCommand, args: Vec) -> Res Ok((p, r)) => (p, r), Err(err) => { let err_msg = err.to_string(); - print_simple_usage_error(prefix, cli_cmd, &err_msg); + print_simple_usage_error_do(prefix, cli_cmd, &err_msg, global_options_iter); return Err(format_err!("{}", err_msg)); } }; if !remaining.is_empty() { let err_msg = format!("got additional arguments: {:?}", remaining); - print_simple_usage_error(prefix, cli_cmd, &err_msg); + print_simple_usage_error_do(prefix, cli_cmd, &err_msg, global_options_iter); return Err(format_err!("{}", err_msg)); } @@ -58,7 +63,7 @@ async fn handle_simple_command_future( args: Vec, mut rpcenv: CliEnvironment, ) -> Result<(), Error> { - let params = parse_arguments(prefix, cli_cmd, args)?; + let params = parse_arguments(prefix, cli_cmd, args, [].into_iter())?; let result = match cli_cmd.info.handler { ApiHandler::Sync(handler) => (handler)(params, cli_cmd.info, &mut rpcenv), @@ -70,9 +75,7 @@ async fn handle_simple_command_future( .and_then(|r| r.to_value().map_err(Error::from)), #[cfg(feature = "server")] ApiHandler::AsyncHttp(_) => { - let err_msg = "CliHandler does not support ApiHandler::AsyncHttp - internal error"; - print_simple_usage_error(prefix, cli_cmd, err_msg); - return Err(format_err!("{}", err_msg)); + bail!("CliHandler does not support ApiHandler::AsyncHttp - internal error") } }; @@ -97,35 +100,28 @@ pub(crate) fn handle_simple_command( args: Vec, rpcenv: &mut CliEnvironment, run: Option Result>, + global_options_iter: impl Iterator, ) -> Result<(), Error> { - let params = parse_arguments(prefix, cli_cmd, args)?; + let params = parse_arguments(prefix, cli_cmd, args, global_options_iter)?; let result = match cli_cmd.info.handler { ApiHandler::Sync(handler) => (handler)(params, cli_cmd.info, rpcenv), ApiHandler::StreamingSync(handler) => { (handler)(params, cli_cmd.info, rpcenv).and_then(|r| r.to_value().map_err(Error::from)) } - ApiHandler::Async(handler) => match run { - Some(run) => { - let future = (handler)(params, cli_cmd.info, rpcenv); - (run)(future) - } - None => { - let err_msg = "CliHandler does not support ApiHandler::Async - internal error"; - print_simple_usage_error(prefix, cli_cmd, err_msg); - return Err(format_err!("{}", err_msg)); - } - }, + ApiHandler::Async(handler) => { + let run = run.ok_or_else(|| { + format_err!("CliHandler does not support ApiHandler::Async - internal error") + })?; + let future = (handler)(params, cli_cmd.info, rpcenv); + (run)(future) + } ApiHandler::StreamingAsync(_handler) => { - let err_msg = "CliHandler does not support ApiHandler::StreamingAsync - internal error"; - print_simple_usage_error(prefix, cli_cmd, err_msg); - return Err(format_err!("{}", err_msg)); + bail!("CliHandler does not support ApiHandler::StreamingAsync - internal error"); } #[cfg(feature = "server")] ApiHandler::AsyncHttp(_) => { - let err_msg = "CliHandler does not support ApiHandler::AsyncHttp - internal error"; - print_simple_usage_error(prefix, cli_cmd, err_msg); - return Err(format_err!("{}", err_msg)); + bail!("CliHandler does not support ApiHandler::AsyncHttp - internal error"); } }; @@ -325,12 +321,12 @@ pub fn handle_command( let result = match &*def { CommandLineInterface::Simple(ref cli_cmd) => { - handle_simple_command(prefix, cli_cmd, args, &mut rpcenv, run) + handle_simple_command(prefix, cli_cmd, args, &mut rpcenv, run, [].into_iter()) } CommandLineInterface::Nested(ref map) => { let mut prefix = prefix.to_string(); let cli_cmd = parse_nested_command(&mut prefix, map, &mut args)?; - handle_simple_command(&prefix, cli_cmd, args, &mut rpcenv, run) + handle_simple_command(&prefix, cli_cmd, args, &mut rpcenv, run, [].into_iter()) } }; @@ -359,9 +355,14 @@ where if args[0] == "printdoc" { let usage = match def { - CommandLineInterface::Simple(cli_cmd) => { - generate_usage_str(&prefix, cli_cmd, DocumentationFormat::ReST, "", &[]) - } + CommandLineInterface::Simple(cli_cmd) => generate_usage_str_do( + &prefix, + cli_cmd, + DocumentationFormat::ReST, + "", + &[], + [].into_iter(), + ), CommandLineInterface::Nested(map) => { generate_nested_usage(&prefix, map, DocumentationFormat::ReST) } diff --git a/proxmox-router/src/cli/format.rs b/proxmox-router/src/cli/format.rs index 65743597..823d7b70 100644 --- a/proxmox-router/src/cli/format.rs +++ b/proxmox-router/src/cli/format.rs @@ -1,7 +1,8 @@ #![allow(clippy::match_bool)] // just no... -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; +use anyhow::{bail, Error}; use serde::Serialize; use serde_json::Value; @@ -11,7 +12,7 @@ use proxmox_schema::format::{ use proxmox_schema::*; use super::{value_to_text, TableFormatOptions}; -use super::{CliCommand, CliCommandMap, CommandLineInterface}; +use super::{CliCommand, CliCommandMap, CommandLineInterface, GlobalOptions}; /// Helper function to format and print result. /// @@ -56,6 +57,7 @@ pub fn format_and_print_result_full( } } +#[deprecated = "to be removed, not meant as a public interface"] /// Helper to generate command usage text for simple commands. pub fn generate_usage_str( prefix: &str, @@ -63,6 +65,24 @@ pub fn generate_usage_str( format: DocumentationFormat, indent: &str, skip_options: &[&str], +) -> String { + generate_usage_str_do( + prefix, + cli_cmd, + format, + indent, + skip_options, + [].into_iter(), + ) +} + +pub(crate) fn generate_usage_str_do( + prefix: &str, + cli_cmd: &CliCommand, + format: DocumentationFormat, + indent: &str, + skip_options: &[&str], + global_options_iter: impl Iterator, ) -> String { let arg_param = cli_cmd.arg_param; let fixed_param = &cli_cmd.fixed_param; @@ -180,12 +200,51 @@ pub fn generate_usage_str( text.push_str("Optional parameters:\n\n"); text.push_str(&options); } + + let mut global_options = String::new(); + let mut separator = ""; + for opt in global_options_iter { + use std::fmt::Write as _; + + if done_hash.contains(opt) { + continue; + } + let _ = match format { + DocumentationFormat::ReST => writeln!(global_options, "{separator}``--{opt}``"), + _ => writeln!(global_options, "--{opt}"), + }; + separator = "\n"; + } + + if !global_options.is_empty() { + text.push_str("Inherited group parameters:\n\n"); + text.push_str(&global_options); + } + text } +#[deprecated = "will be removed, not meant to be a public interface"] /// Print command usage for simple commands to ``stderr``. pub fn print_simple_usage_error(prefix: &str, cli_cmd: &CliCommand, err_msg: &str) { - let usage = generate_usage_str(prefix, cli_cmd, DocumentationFormat::Long, "", &[]); + print_simple_usage_error_do(prefix, cli_cmd, err_msg, [].into_iter()) +} + +/// Print command usage for simple commands to ``stderr``. +pub(crate) fn print_simple_usage_error_do( + prefix: &str, + cli_cmd: &CliCommand, + err_msg: &str, + global_options_iter: impl Iterator, +) { + let usage = generate_usage_str_do( + prefix, + cli_cmd, + DocumentationFormat::Long, + "", + &[], + global_options_iter, + ); eprint!("Error: {}\nUsage: {}", err_msg, usage); } @@ -195,12 +254,89 @@ pub fn print_nested_usage_error(prefix: &str, def: &CliCommandMap, err_msg: &str eprintln!("Error: {}\n\nUsage:\n\n{}", err_msg, usage); } +/// While going through nested commands, this keeps track of the available global options. +#[derive(Default)] +struct UsageState { + global_options: Vec>, +} + +impl UsageState { + fn push_global_options(&mut self, options: &HashMap) { + self.global_options + .push(options.values().map(|o| o.schema).collect()); + } + + fn pop_global_options(&mut self) { + self.global_options.pop(); + } + + fn describe_current(&self, prefix: &str, format: DocumentationFormat) -> String { + use std::fmt::Write as _; + + let mut out = String::new(); + + let Some(opts) = self.global_options.last() else { + return out; + }; + + if opts.is_empty() { + return out; + } + + if !matches!( + format, + DocumentationFormat::ReST | DocumentationFormat::Full + ) { + return out; + } + + if format == DocumentationFormat::ReST { + let _ = write!(out, "----\n\n"); + } + let _ = write!(out, "Options available for command group ``{prefix}``:\n\n"); + for opt in opts { + for (name, _optional, schema) in opt + .any_object() + .expect("non-object schema in global optiosn") + .properties() + { + let _ = write!( + out, + "{}", + get_property_description(name, schema, ParameterDisplayStyle::Arg, format) + ); + } + } + + out + } + + 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) + } +} + /// Helper to generate command usage text for nested commands. pub fn generate_nested_usage( prefix: &str, def: &CliCommandMap, format: DocumentationFormat, ) -> String { + generate_nested_usage_do(&mut UsageState::default(), prefix, def, format) +} + +fn generate_nested_usage_do( + state: &mut UsageState, + prefix: &str, + def: &CliCommandMap, + format: DocumentationFormat, +) -> String { + state.push_global_options(&def.global_options); + let mut cmds: Vec<&String> = def.commands.keys().collect(); cmds.sort(); @@ -208,6 +344,11 @@ pub fn generate_nested_usage( let mut usage = String::new(); + let globals = state.describe_current(prefix, format); + if !globals.is_empty() { + usage.push_str(&globals); + } + for cmd in cmds { let new_prefix = if prefix.is_empty() { String::from(cmd) @@ -220,34 +361,55 @@ pub fn generate_nested_usage( if !usage.is_empty() && format == DocumentationFormat::ReST { usage.push_str("----\n\n"); } - usage.push_str(&generate_usage_str( + usage.push_str(&generate_usage_str_do( &new_prefix, cli_cmd, format, "", skip_options, + state.global_options_iter(), )); } CommandLineInterface::Nested(map) => { - usage.push_str(&generate_nested_usage(&new_prefix, map, format)); + usage.push_str(&generate_nested_usage_do(state, &new_prefix, map, format)); } } } + state.pop_global_options(); + usage } /// Print help text to ``stderr``. pub fn print_help( + top_def: &CommandLineInterface, + prefix: String, + args: &[String], + verbose: Option, +) { + let mut message = String::new(); + match print_help_to(top_def, prefix, args, verbose, &mut message) { + Ok(()) => print!("{message}"), + Err(err) => eprintln!("{err}"), + } +} + +pub fn print_help_to( top_def: &CommandLineInterface, mut prefix: String, args: &[String], mut verbose: Option, -) { + mut to: impl std::fmt::Write, +) -> Result<(), Error> { let mut iface = top_def; + let mut usage_state = UsageState::default(); + for cmd in args { if let CommandLineInterface::Nested(map) = iface { + usage_state.push_global_options(&map.global_options); + if let Some((full_name, subcmd)) = map.find_command(cmd) { iface = subcmd; if !prefix.is_empty() { @@ -258,11 +420,10 @@ pub fn print_help( } } if prefix.is_empty() { - eprintln!("no such command '{}'", cmd); + bail!("no such command '{}'", cmd); } else { - eprintln!("no such command '{} {}'", prefix, cmd); + bail!("no such command '{} {}'", prefix, cmd); } - return; } if verbose.is_none() { @@ -278,13 +439,27 @@ pub fn print_help( match iface { CommandLineInterface::Nested(map) => { - println!("Usage:\n\n{}", generate_nested_usage(&prefix, map, format)); + write!( + to, + "Usage:\n\n{}", + generate_nested_usage_do(&mut usage_state, &prefix, map, format) + )?; } CommandLineInterface::Simple(cli_cmd) => { - println!( + write!( + to, "Usage: {}", - generate_usage_str(&prefix, cli_cmd, format, "", &[]) - ); + generate_usage_str_do( + &prefix, + cli_cmd, + format, + "", + &[], + usage_state.global_options_iter() + ) + )?; } } + + Ok(()) } diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs index 37dc73c2..82eae27a 100644 --- a/proxmox-router/src/cli/mod.rs +++ b/proxmox-router/src/cli/mod.rs @@ -544,8 +544,14 @@ impl<'cli> CommandLineParseState<'cli> { Ok(Invocation { call: Box::new(move |rpcenv| { command::set_help_context(Some(interface)); - let out = - command::handle_simple_command(&self.prefix, cli, args, rpcenv, self.async_run); + let out = command::handle_simple_command( + &self.prefix, + cli, + args, + rpcenv, + self.async_run, + self.global_option_schemas.keys().copied(), + ); command::set_help_context(None); out }), diff --git a/proxmox-router/tests/docs.rs b/proxmox-router/tests/docs.rs new file mode 100644 index 00000000..aa833ec2 --- /dev/null +++ b/proxmox-router/tests/docs.rs @@ -0,0 +1,316 @@ +use anyhow::Error; +use serde_json::Value; + +use proxmox_router::cli::{CliCommand, CliCommandMap, CommandLineInterface, GlobalOptions}; +use proxmox_router::{ApiHandler, ApiMethod, RpcEnvironment}; +use proxmox_schema::format::DocumentationFormat; +use proxmox_schema::{ + ApiStringFormat, ApiType, BooleanSchema, EnumEntry, ObjectSchema, Schema, StringSchema, +}; + +fn dummy_method( + _param: Value, + _info: &ApiMethod, + _rpcenv: &mut dyn RpcEnvironment, +) -> Result { + Ok(Value::Null) +} + +const API_METHOD_SIMPLE1: ApiMethod = ApiMethod::new( + &ApiHandler::Sync(&dummy_method), + &ObjectSchema::new( + "Simple API method with one required and one optional argument.", + &[ + ( + "optional-arg", + true, + &BooleanSchema::new("Optional boolean argument.") + .default(false) + .schema(), + ), + ( + "required-arg", + false, + &StringSchema::new("Required string argument.").schema(), + ), + ( + "another-required-arg", + false, + &StringSchema::new("A second required string argument.").schema(), + ), + ], + ), +); + +#[allow(dead_code)] +struct GlobalOpts { + global: String, +} + +impl<'de> serde::Deserialize<'de> for GlobalOpts { + fn deserialize(_deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + unreachable!("not used in tests, implemented to satisfy `.global_option` constraint"); + } +} + +impl ApiType for GlobalOpts { + const API_SCHEMA: Schema = ObjectSchema::new( + "Global options.", + &[ + ( + "global1", + true, + &StringSchema::new("A global option.") + .format(&ApiStringFormat::Enum(&[ + EnumEntry::new("one", "Option one."), + EnumEntry::new("two", "Option two."), + ])) + .schema(), + ), + ( + "global2", + true, + &StringSchema::new("A second global option.").schema(), + ), + ], + ) + .schema(); +} + +/// Generates the following: +/// +/// ```text +/// clicmd l0c1 --required-arg --another-required-arg [--optional-arg] +/// clicmd l0c2 --another-required-arg [--optional-arg] +/// clicmd l0sub l1c1 --required-arg --another-required-arg [--optional-arg] +/// clicmd l0sub l1c2 --required-arg --another-required-arg [--optional-arg] +/// ``` +fn get_complex_test_cmddef() -> CliCommandMap { + let sub_def = CliCommandMap::new() + .global_option(GlobalOptions::of::()) + .insert("l1c1", CliCommand::new(&API_METHOD_SIMPLE1)) + .insert("l1c2", CliCommand::new(&API_METHOD_SIMPLE1)); + + CliCommandMap::new() + .insert_help() + .insert("l0sub", CommandLineInterface::Nested(sub_def)) + .insert("l0c1", CliCommand::new(&API_METHOD_SIMPLE1)) + .insert( + "l0c2", + CliCommand::new(&API_METHOD_SIMPLE1).arg_param(&["required-arg"]), + ) +} + +fn expected_toplevel_help_text() -> &'static str { + r##" +Usage: + +clicmd help [{}] [OPTIONS] +clicmd l0c1 --required-arg --another-required-arg [OPTIONS] +clicmd l0c2 --another-required-arg [OPTIONS] +clicmd l0sub l1c1 --required-arg --another-required-arg [OPTIONS] +clicmd l0sub l1c2 --required-arg --another-required-arg [OPTIONS] +"## + .trim_start() +} + +fn expected_group_help_text() -> &'static str { + r##" +Usage: clicmd l0sub l1c1 --required-arg --another-required-arg [OPTIONS] + +Simple API method with one required and one optional argument. + + --required-arg + Required string argument. + + --another-required-arg + A second required string argument. + +Optional parameters: + + --optional-arg (default=false) + Optional boolean argument. + +Inherited group parameters: + +--global1 +--global2 +"## + .trim_start() +} + +fn expected_nested_usage_text() -> &'static str { + r##" +``clicmd help [{}] [OPTIONS]`` + +Get help about specified command (or sub-command). + +```` : ```` + Command. This may be a list in order to spefify nested sub-commands. Can be + specified more than once. + + +Optional parameters: + +``--verbose`` ```` + Verbose help. + + +---- + +``clicmd l0c1 --required-arg --another-required-arg [OPTIONS]`` + +Simple API method with one required and one optional argument. + +``--required-arg`` ```` + Required string argument. + + +``--another-required-arg`` ```` + A second required string argument. + + +Optional parameters: + +``--optional-arg`` `` (default=false)`` + Optional boolean argument. + + +---- + +``clicmd l0c2 --another-required-arg [OPTIONS]`` + +Simple API method with one required and one optional argument. + +```` : ```` + Required string argument. + + +``--another-required-arg`` ```` + A second required string argument. + + +Optional parameters: + +``--optional-arg`` `` (default=false)`` + Optional boolean argument. + + +---- + +Options available for command group ``clicmd l0sub``: + +``--global1`` ``one|two`` + A global option. + + +``--global2`` ```` + A second global option. + + +---- + +``clicmd l0sub l1c1 --required-arg --another-required-arg [OPTIONS]`` + +Simple API method with one required and one optional argument. + +``--required-arg`` ```` + Required string argument. + + +``--another-required-arg`` ```` + A second required string argument. + + +Optional parameters: + +``--optional-arg`` `` (default=false)`` + Optional boolean argument. + + +Inherited group parameters: + +``--global1`` + +``--global2`` + + + +---- + +``clicmd l0sub l1c2 --required-arg --another-required-arg [OPTIONS]`` + +Simple API method with one required and one optional argument. + +``--required-arg`` ```` + Required string argument. + + +``--another-required-arg`` ```` + A second required string argument. + + +Optional parameters: + +``--optional-arg`` `` (default=false)`` + Optional boolean argument. + + +Inherited group parameters: + +``--global1`` + +``--global2`` +"## + .trim_start() +} + +#[test] +fn test_nested_usage() { + let doc = proxmox_router::cli::generate_nested_usage( + "clicmd", + &get_complex_test_cmddef(), + DocumentationFormat::ReST, + ); + println!("--- BEGIN EXPECTED DOC OUTPUT ---"); + println!("{doc}"); + println!("--- END EXPECTED DOC OUTPUT ---"); + assert_eq!(doc, expected_nested_usage_text()); +} + +#[test] +fn test_toplevel_help() { + let mut help = String::new(); + proxmox_router::cli::print_help_to( + &get_complex_test_cmddef().into(), + "clicmd".to_string(), + &[], + None, + &mut help, + ) + .expect("failed to format help string"); + // println!("--- BEGIN EXPECTED DOC OUTPUT ---"); + // println!("{help}"); + // println!("--- END EXPECTED DOC OUTPUT ---"); + assert_eq!(help, expected_toplevel_help_text()); +} + +#[test] +fn test_group_help() { + let mut help = String::new(); + proxmox_router::cli::print_help_to( + &get_complex_test_cmddef().into(), + "clicmd".to_string(), + &["l0sub".to_string(), "l1c1".to_string()], + None, + &mut help, + ) + .expect("failed to format help string"); + // println!("--- BEGIN EXPECTED DOC OUTPUT ---"); + // println!("{help}"); + // println!("--- END EXPECTED DOC OUTPUT ---"); + assert_eq!(help, expected_group_help_text()); +}