fix: Use roff bullet lists for possible_values

clap_mangen/src/render.rs

@@ -1,4 +1,6 @@
 use clap::AppSettings;
+use std::vec;
+
 use roff::{bold, italic, roman, Inline, Roff};
 
 pub(crate) fn subcommand_heading(cmd: &clap::Command) -> String {
@@ -114,27 +116,53 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) {
             body.push(roman(help));
         }
 
+        roff.control("TP", []);
+        roff.text(header);
+        roff.text(body);
+
         let possibles: Vec<clap::builder::PossibleValue> = opt
             .get_value_parser()
             .possible_values()
             .map(|pvs| pvs.collect())
             .unwrap_or_default();
-
+        let possibles: Vec<&clap::builder::PossibleValue> =
+            possibles.iter().filter(|pos| !pos.is_hide_set()).collect();
 
         if !(possibles.is_empty() || opt.is_hide_possible_values_set()) {
             if help_written {
                 // It looks nice to have a separation between the help and the values
-                body.push(Inline::LineBreak);
+                roff.text([Inline::LineBreak]);
+            }
+            // with help for each possible value
+
+            if possibles.iter().any(|p| p.get_help().is_some()) {
+                roff.text([Inline::LineBreak, italic("Possible values:")]);
+
+                // Need to indent twice to get it to look right,
+                // because .TP heading indents, but that indent doesn't
+                // Carry over to the .IP for the bullets.
+                // The standard shift size is 7 for terminal devices
+                roff.control("RS", ["14"]);
+                for line in format_possible_values(&possibles) {
+                    roff.control("IP", ["\\(bu", "2"]);
+                    roff.text([roman(line)]);
+                }
+                roff.control("RE", []);
             }
 
-            let possible_vals = possibles.iter().filter(|pos| !pos.is_hide_set()).collect();
-            body.append(&mut format_possible_values(possible_vals));
+            // without help for each possible value
+            else {
+                let possible_list = format_possible_values(&possibles);
+                let possible_value_text: Vec<Inline> = vec![
+                    Inline::LineBreak,
+                    italic("[possible values: "),
+                    roman(possible_list.join(", ")),
+                    roman("]"),
+                ];
+                roff.text(possible_value_text);
+            }
         }
 
-        roff.control("TP", []);
-        roff.text(header);
-        roff.text(body);
-
         if let Some(env) = option_environment(opt) {
             roff.control("RS", []);
             roff.text(env);
@@ -260,40 +288,18 @@ fn option_default_values(opt: &clap::Arg) -> Option<String> {
     None
 }
 
-/// Generates a Vector of Inline Commands to push to the roff
-/// to properly format possible values that an option can take.
-fn format_possible_values(values: Vec<&clap::builder::PossibleValue>) -> Vec<Inline> {
-    let mut formats: Vec<Inline> = vec![];
-    // With Help
-    if values.iter().any(|p| p.get_help().is_some()) {
-        formats.push(Inline::LineBreak);
-        formats.push(roman("Possible values:"));
-        formats.push(Inline::LineBreak);
-        for value in values {
-            formats.push(roman("  - "));
-            formats.push(roman(value.get_name()));
+fn format_possible_values(possibles: &Vec<&clap::builder::PossibleValue>) -> Vec<String> {
+    let mut lines = vec![];
+    if possibles.iter().any(|p| p.get_help().is_some()) {
+        for value in possibles {
+            let val_name = value.get_name();
             match value.get_help() {
-                Some(help) => {
-                    formats.push(roman(": "));
-                    formats.push(roman(help));
-                }
-                None => {}
+                Some(help) => lines.push(format!("{}: {}", val_name, help)),
+                None => lines.push(val_name.to_string()),
             }
-            formats.push(Inline::LineBreak);
         }
+    } else {
+        lines.append(&mut possibles.iter().map(|p| p.get_name().to_string()).collect());
     }
-    // Without help
-    else {
-        formats.push(Inline::LineBreak);
-        formats.push(roman("[possible values: "));
-        formats.push(italic(
-            values
-                .iter()
-                .map(|p| p.get_name())
-                .collect::<Vec<&str>>()
-                .join(", "),
-        ));
-        formats.push(roman("]"));
-    }
-    formats
+    lines
 }

clap_mangen/tests/common.rs

@@ -1,4 +1,7 @@
+use clap::builder::PossibleValue;
+
 pub fn basic_command(name: &'static str) -> clap::Command<'static> {
+
     clap::Command::new(name)
         .arg(clap::Arg::new("config").short('c').global(true))
         .arg(clap::Arg::new("v").short('v').conflicts_with("config"))
@@ -252,3 +255,24 @@ pub fn assert_matches_path(expected_path: impl AsRef<std::path::Path>, cmd: clap
         .action_env("SNAPSHOTS")
         .matches_path(expected_path, buf);
 }
+
+pub fn possible_values_command(name: &'static str) -> clap::Command<'static> {
+    clap::Command::new(name)
+        .trailing_var_arg(true)
+        .arg(
+            clap::Arg::new("choice")
+                .long("choice")
+                .action(clap::ArgAction::Set)
+                .value_parser(["bash", "fish", "zsh"]),
+        )
+        .arg(
+            clap::Arg::new("method")
+            .long("method")
+            .action(clap::ArgAction::Set)
+            .value_parser([
+                PossibleValue::new("fast").help("use the Fast method"),
+                PossibleValue::new("slow").help("use the slow method"),
+                PossibleValue::new("normal").help("use normal mode").hide(true)
+            ])
+        )
+}

clap_mangen/tests/roff.rs

@@ -62,3 +62,10 @@ fn value_env() {
     let cmd = common::env_value_command(name);
     common::assert_matches_path("tests/snapshots/value_env.bash.roff", cmd);
 }
+
+#[test]
+fn possible_values() {
+    let name = "my-app";
+    let cmd = common::possible_values_command(name);
+    common::assert_matches_path("tests/snapshots/possible_values.bash.roff", cmd);
+}

clap_mangen/tests/snapshots/possible_values.bash.roff

@@ -0,0 +1,28 @@
+.ie /n(.g .ds Aq /(aq
+.el .ds Aq '
+.TH my-app 1  "my-app " 
+.SH NAME
+my/-app
+.SH SYNOPSIS
+/fBmy/-app/fR [/fB/-h/fR|/fB/-/-help/fR] [/fB/-/-choice/fR] [/fB/-/-method/fR] 
+.SH DESCRIPTION
+.SH OPTIONS
+.TP
+/fB/-h/fR, /fB/-/-help/fR
+Print help information
+.TP
+/fB/-/-choice/fR
+
+.br
+/fI[possible values: /fRbash, fish, zsh]
+.TP
+/fB/-/-method/fR
+
+.br
+/fIPossible values:/fR
+.RS 14
+.IP /(bu 2
+fast: use the Fast method
+.IP /(bu 2
+slow: use the slow method
+.RE

clap_mangen/tests/snapshots/value_hint.bash.roff

@@ -12,8 +12,9 @@ my/-app
 Print help information
 .TP
 /fB/-/-choice/fR
+
 .br
-[possible values: /fIbash, fish, zsh/fR]
+/fI[possible values: /fRbash, fish, zsh]
 .TP
 /fB/-/-unknown/fR