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 <w.bumiller@proxmox.com>

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(&param_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}``"),
+            DocumentationFormat::ReST => write!(global_options, "``--{opt}``"),
-            _ => writeln!(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 mut out = String::new();
-            let _ = write!(out, "----\n\n");
+        let _ = write!(out, "Options available for command group ``{prefix}``:");
-        }
-        let _ = write!(out, "Options available for command group ``{prefix}``:\n\n");
         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(

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`` ``<boolean>``
   Verbose help.
 
-
 ----
 
 ``clicmd l0c1 --required-arg <string> --another-required-arg <string> [OPTIONS]``
@@ -168,17 +166,14 @@ Simple API method with one required and one optional argument.
 ``--required-arg`` ``<string>``
   Required string argument.
 
-
 ``--another-required-arg`` ``<string>``
   A second required string argument.
 
-
 Optional parameters:
 
 ``--optional-arg`` ``<boolean>   (default=false)``
   Optional boolean argument.
 
-
 ----
 
 ``clicmd l0c2 <required-arg> --another-required-arg <string> [OPTIONS]``
@@ -188,17 +183,14 @@ Simple API method with one required and one optional argument.
 ``<required-arg>`` : ``<string>``
   Required string argument.
 
-
 ``--another-required-arg`` ``<string>``
   A second required string argument.
 
-
 Optional parameters:
 
 ``--optional-arg`` ``<boolean>   (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`` ``<string>``
   A second global option.
 
-
 ----
 
 ``clicmd l0sub l1c1 --required-arg <string> --another-required-arg <string> [OPTIONS]``
@@ -220,25 +210,20 @@ Simple API method with one required and one optional argument.
 ``--required-arg`` ``<string>``
   Required string argument.
 
-
 ``--another-required-arg`` ``<string>``
   A second required string argument.
 
-
 Optional parameters:
 
 ``--optional-arg`` ``<boolean>   (default=false)``
   Optional boolean argument.
 
-
 Inherited group parameters:
 
 ``--global1``
 
 ``--global2``
 
-
-
 ----
 
 ``clicmd l0sub l1c2 --required-arg <string> --another-required-arg <string> [OPTIONS]``
@@ -248,23 +233,19 @@ Simple API method with one required and one optional argument.
 ``--required-arg`` ``<string>``
   Required string argument.
 
-
 ``--another-required-arg`` ``<string>``
   A second required string argument.
 
-
 Optional parameters:
 
 ``--optional-arg`` ``<boolean>   (default=false)``
   Optional boolean argument.
 
-
 Inherited group parameters:
 
 ``--global1``
 
-``--global2``
+``--global2``"##
-"##
         .trim_start()
 }
 
@@ -275,9 +256,9 @@ fn test_nested_usage() {
         &get_complex_test_cmddef(),
         DocumentationFormat::ReST,
     );
-    println!("--- BEGIN EXPECTED DOC OUTPUT ---");
+    // println!("--- BEGIN EXPECTED DOC OUTPUT ---");
-    println!("{doc}");
+    // print!("{doc}");
-    println!("--- END EXPECTED DOC OUTPUT ---");
+    // 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());
 }