From d872eb9d7ebadbd8fcc9506eb2c5689632fa89bc Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Thu, 27 Jun 2024 11:22:57 +0200 Subject: [PATCH] router: cli: rework newline handling for doc and help output The rules are as follows: - An "item" by itself does neither start nor end with a newline. - Where items are connected, the appropriate amount of newlines must be inserted, assuming items do not have any trailing newlines. Otherwise this just gets WAY too confusing everywhere! Signed-off-by: Wolfgang Bumiller --- proxmox-router/src/cli/format.rs | 69 ++++++++++++++++++++++---------- proxmox-router/tests/docs.rs | 33 ++++----------- 2 files changed, 54 insertions(+), 48 deletions(-) diff --git a/proxmox-router/src/cli/format.rs b/proxmox-router/src/cli/format.rs index 823d7b70..d8f72eaf 100644 --- a/proxmox-router/src/cli/format.rs +++ b/proxmox-router/src/cli/format.rs @@ -132,6 +132,9 @@ pub(crate) fn generate_usage_str_do( ParameterDisplayStyle::Fixed, format, ); + if !arg_descr.is_empty() { + arg_descr.push_str("\n\n"); + } arg_descr.push_str(¶m_descr); } @@ -151,6 +154,9 @@ pub(crate) fn generate_usage_str_do( get_property_description(prop, param_schema, ParameterDisplayStyle::Arg, format); if *optional { + if !options.is_empty() { + options.push('\n'); + } options.push_str(&prop_descr); } else { args.push_str(" --"); @@ -158,6 +164,9 @@ pub(crate) fn generate_usage_str_do( args.push(' '); args.push_str(&type_text); + if !arg_descr.is_empty() { + arg_descr.push_str("\n\n"); + } arg_descr.push_str(&prop_descr); } @@ -172,11 +181,11 @@ pub(crate) fn generate_usage_str_do( let mut text = match format { DocumentationFormat::Short => { - return format!("{}{}{}{}\n", indent, prefix, args, option_indicator); + return format!("{}{}{}{}", indent, prefix, args, option_indicator); } - DocumentationFormat::Long => format!("{}{}{}{}\n", indent, prefix, args, option_indicator), + DocumentationFormat::Long => format!("{}{}{}{}", indent, prefix, args, option_indicator), DocumentationFormat::Full => format!( - "{}{}{}{}\n\n{}\n\n", + "{}{}{}{}\n\n{}", indent, prefix, args, @@ -184,7 +193,7 @@ pub(crate) fn generate_usage_str_do( schema.description() ), DocumentationFormat::ReST => format!( - "``{}{}{}``\n\n{}\n\n", + "``{}{}{}``\n\n{}", prefix, args, option_indicator, @@ -193,31 +202,37 @@ pub(crate) fn generate_usage_str_do( }; if !arg_descr.is_empty() { + text.push_str("\n\n"); text.push_str(&arg_descr); } if !options.is_empty() { - text.push_str("Optional parameters:\n\n"); + text.push_str("\n\nOptional 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; } + if !global_options.is_empty() { + if matches!(format, DocumentationFormat::ReST) { + global_options.push_str("\n\n"); + } else { + global_options.push('\n'); + } + } let _ = match format { - DocumentationFormat::ReST => writeln!(global_options, "{separator}``--{opt}``"), - _ => writeln!(global_options, "--{opt}"), + DocumentationFormat::ReST => write!(global_options, "``--{opt}``"), + _ => write!(global_options, "--{opt}"), }; - separator = "\n"; } if !global_options.is_empty() { - text.push_str("Inherited group parameters:\n\n"); + text.push_str("\n\nInherited group parameters:\n\n"); text.push_str(&global_options); } @@ -273,27 +288,23 @@ impl UsageState { 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; + return String::new(); }; if opts.is_empty() { - return out; + return String::new(); } if !matches!( format, DocumentationFormat::ReST | DocumentationFormat::Full ) { - return out; + return String::new(); } - if format == DocumentationFormat::ReST { - let _ = write!(out, "----\n\n"); - } - let _ = write!(out, "Options available for command group ``{prefix}``:\n\n"); + let mut out = String::new(); + let _ = write!(out, "Options available for command group ``{prefix}``:"); for opt in opts { for (name, _optional, schema) in opt .any_object() @@ -302,7 +313,7 @@ impl UsageState { { let _ = write!( out, - "{}", + "\n\n{}", get_property_description(name, schema, ParameterDisplayStyle::Arg, format) ); } @@ -346,10 +357,24 @@ fn generate_nested_usage_do( let globals = state.describe_current(prefix, format); if !globals.is_empty() { + if format == DocumentationFormat::ReST { + usage.push_str("----\n\n"); + } usage.push_str(&globals); } for cmd in cmds { + if !usage.is_empty() { + if matches!( + format, + DocumentationFormat::ReST | DocumentationFormat::Long + ) { + usage.push_str("\n\n"); + } else { + usage.push('\n'); + } + } + let new_prefix = if prefix.is_empty() { String::from(cmd) } else { @@ -439,14 +464,14 @@ pub fn print_help_to( match iface { CommandLineInterface::Nested(map) => { - write!( + writeln!( to, "Usage:\n\n{}", generate_nested_usage_do(&mut usage_state, &prefix, map, format) )?; } CommandLineInterface::Simple(cli_cmd) => { - write!( + writeln!( to, "Usage: {}", generate_usage_str_do( diff --git a/proxmox-router/tests/docs.rs b/proxmox-router/tests/docs.rs index aa833ec2..509b29a1 100644 --- a/proxmox-router/tests/docs.rs +++ b/proxmox-router/tests/docs.rs @@ -152,13 +152,11 @@ 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]`` @@ -168,17 +166,14 @@ 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]`` @@ -188,17 +183,14 @@ 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``: @@ -206,11 +198,9 @@ 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]`` @@ -220,25 +210,20 @@ 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]`` @@ -248,24 +233,20 @@ 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() +``--global2``"## + .trim_start() } #[test] @@ -275,9 +256,9 @@ fn test_nested_usage() { &get_complex_test_cmddef(), DocumentationFormat::ReST, ); - println!("--- BEGIN EXPECTED DOC OUTPUT ---"); - println!("{doc}"); - println!("--- END EXPECTED DOC OUTPUT ---"); + // println!("--- BEGIN EXPECTED DOC OUTPUT ---"); + // print!("{doc}"); + // println!("--- END EXPECTED DOC OUTPUT ---"); assert_eq!(doc, expected_nested_usage_text()); } @@ -293,7 +274,7 @@ fn test_toplevel_help() { ) .expect("failed to format help string"); // println!("--- BEGIN EXPECTED DOC OUTPUT ---"); - // println!("{help}"); + // print!("{help}"); // println!("--- END EXPECTED DOC OUTPUT ---"); assert_eq!(help, expected_toplevel_help_text()); } @@ -310,7 +291,7 @@ fn test_group_help() { ) .expect("failed to format help string"); // println!("--- BEGIN EXPECTED DOC OUTPUT ---"); - // println!("{help}"); + // print!("{help}"); // println!("--- END EXPECTED DOC OUTPUT ---"); assert_eq!(help, expected_group_help_text()); }