Repository: clap-rs/clap (language: rust)

Issue:
Generated man pages still claim that flag options take values
### Please complete the following tasks

- [X] I have searched the [discussions](https://github.com/clap-rs/clap/discussions)
- [X] I have searched the [open](https://github.com/clap-rs/clap/issues) and [rejected](https://github.com/clap-rs/clap/issues?q=is%3Aissue+label%3AS-wont-fix+is%3Aclosed) issues

### Rust Version

rustc 1.64.0 (Fedora 1.64.0-1.fc36)

### Clap Version

4.0.18, clap_mangen 0.2.4

### Minimal reproducible code

```rust
use clap::{CommandFactory, Parser};

#[derive(Parser)]
/// Command to do things
pub struct Cmd {
    /// Do thing
    #[clap(long)]
    thing: bool,
}

fn main() {
    Cmd::parse();
    clap_mangen::Man::new(Cmd::command())
        .render(&mut std::io::stdout())
        .unwrap();
}
```

Cargo.toml:

```toml
[package]
name = "clap-man"
version = "0.0.0"
edition = "2021"

[dependencies]
clap = { version = "4", features = ["derive"] }
clap_mangen = "0.2"
```

### Steps to reproduce the bug with the above code

```command
cargo run | man -l -
```

### Actual Behaviour

The option description for `--thing` claims that it takes an argument:

```
clap-man(1)                 General Commands Manual                clap-man(1)

NAME
       clap-man - Command to do things

SYNOPSIS
       clap-man [--thing] [-h|--help]

DESCRIPTION
       Command to do things

OPTIONS
       --thing=THING
              Do thing

       -h, --help
              Print help information

                                   clap-man                        clap-man(1)
```

It doesn't:

```command
$ cargo run -- --thing=true
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/clap-man --thing=true`
error: The value 'true' was provided to '--thing' but it wasn't expecting any more values

Usage: clap-man --thing

For more information try '--help'
```

### Expected Behaviour

The man page lists the option without any argument, as it did in clap 3.2 / clap_mangen 0.1:

```
clap-man(1)                 General Commands Manual                clap-man(1)

NAME
       clap-man - Command to do things

SYNOPSIS
       clap-man [--thing] [-h|--help]

DESCRIPTION
       Command to do things

OPTIONS
       --thing
              Do thing

       -h, --help
              Print help information

                                   clap-man                        clap-man(1)
```

### Additional Context

This is a followup to https://github.com/clap-rs/clap/issues/4410.  clap_mangen 0.2.4 improved the situation but didn't fix it.

### Debug Output

_No response_


Discussion / hints:
As pointed out in #4506, #4432 highlights output where this is working and we have tests highlighting this working.

@Calder-Ty pointed out regarding #4506 

> It's strange because the change definitely does work on the example provided in the bug report
I want to clarify that it is the proposed change in #4506 that brought a fix for the given examples above. However as pointed out, in the PR the change doesn't affect existing tests, which we should expect it to. I'm still trying to dig up what changed between #4432 and now, as i'm able to replicate the issue in the report.
The one big difference I notice this Issue is using the derive API and the existing tests are using the builder API.  Since #4506 fixes this, it implies that a value name is being set for flags in the derive.

It looks like we always set value_name in the derive: https://github.com/clap-rs/clap/blob/master/clap_derive/src/derives/args.rs#L232

Normally, we verify the relationship between num_args and value_names but apparently we skip it for empty value names to make the derive easier: https://github.com/clap-rs/clap/blob/master/src/builder/debug_asserts.rs#L757

So the question is if we should fix this in the derive or in the callers.

I will point out that clap_mangen is a bit simple in its rendering of values; `--help` has a bit more complex of rendering: https://github.com/clap-rs/clap/blob/master/src/builder/arg.rs#L4148
One option would be for us to expose an `Arg::render` function that calls `Arg::stylized(None)`.  The only public way to access the styling is with `StyledStr::ansi().to_string()` and it will remain that way.  I've been considering writing an ansi code to roff converter.  

If someone wants to step up and write the ansi to roff converter as I won't be getting to it immediately, that'd be great.  I have notes to help prepare for it in the issues at https://github.com/epage/anstyle/issues.

However, that is a big ask and people likely don't want to wait that long.  I can understand just duplicating the code into clap_mangen until then.
@epage, I don't mind working getting a workaround you describe setup for now. I don't know if i'm up to the ANSI conversion, would have to look into that more. 
@epage Sorry for leaving this alone for so long. I've been looking into ways to make a quick workaround
for this in a way that is robust. After looking into it I think the best solution is just to go forwards
with creating the ANSI-Roff converter.

Adding an Arg::Render function is feasible, but it also generates styles that would be noticeably different
from the current manpage styling. I'm not sure how "in stone" those need to be, but want to err on
the conservative side.

Copying the code into clap_mangen is a lot of reduplication, as there are a lot of private dependencies
that would need to be copied over as well.

---

I wouldn't mind working on writing an ANSI to Roff converter, but I would like to have
a better idea of what you are imagining that looking like, perhaps in another discussion. For
now I have added some tests to #4506 that demonstrate some parity between Derive and Builder commands,
and that they both show flag options appropriately.

As an aside, when testing these different API's I found some other cosmetic disparities between
Derive and Builder API's that I will write up another ticket for. However it probably would be better
to get the ansi-roff converter done first as that will probably clear up the differences.


> I wouldn't mind working on writing an ANSI to Roff converter, but I would like to have
a better idea of what you are imagining that looking like

In https://github.com/epage/anstyle/issues/4, I contrast different ANSI parsing libraries.  For a first round, feel free to use what is easiest as long as it as a good license.  I'm less concerned about `clap_mangen`s build times.  We can then iterate on it as clap needs to adopt an ANSI parser.

anstyle is where I'm housing these different integrations, e.g. you can see what it supports today in the [README](https://github.com/epage/anstyle#readme).

> Adding an Arg::Render function is feasible, but it also generates styles that would be noticeably different
from the current manpage styling. I'm not sure how "in stone" those need to be, but want to err on
the conservative side.

`clap_mangen` is in its very early stages; nothing is set in stone.  I wasn't going to get started on it until we could reuse more help generation logic but some others were wanting it sooner and wrote most of the code.  It still has a ways to go.

Fix patch:
diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs
index ab9c9ec75c1..ea3dffbe3e7 100644
--- a/clap_mangen/src/render.rs
+++ b/clap_mangen/src/render.rs
@@ -99,9 +99,11 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) {
             (None, None) => vec![],
         };
 
-        if let Some(value) = &opt.get_value_names() {
-            header.push(roman("="));
-            header.push(italic(value.join(" ")));
+        if opt.get_action().takes_values() {
+            if let Some(value) = &opt.get_value_names() {
+                header.push(roman("="));
+                header.push(italic(value.join(" ")));
+            }
         }
 
         if let Some(defs) = option_default_values(opt) {


Test patch:
diff --git a/clap_mangen/tests/common.rs b/clap_mangen/tests/common.rs
index ca9f1f9b4fb..029098c0956 100644
--- a/clap_mangen/tests/common.rs
+++ b/clap_mangen/tests/common.rs
@@ -312,3 +312,12 @@ pub fn possible_values_command(name: &'static str) -> clap::Command {
                 ]),
         )
 }
+
+pub fn value_name_without_arg(name: &'static str) -> clap::Command {
+    clap::Command::new(name).arg(
+        clap::Arg::new("flag")
+            .long("flag")
+            .value_name("SPURIOUS")
+            .action(clap::ArgAction::SetTrue),
+    )
+}
diff --git a/clap_mangen/tests/roff.rs b/clap_mangen/tests/roff.rs
index 223196c3a44..38d1aef11ba 100644
--- a/clap_mangen/tests/roff.rs
+++ b/clap_mangen/tests/roff.rs
@@ -83,3 +83,10 @@ fn sub_subcommands_help() {
         common::assert_matches_path("tests/snapshots/sub_subcommand_help.roff", cmd.clone());
     }
 }
+
+#[test]
+fn value_name_without_arg() {
+    let name = "my-app";
+    let cmd = common::value_name_without_arg(name);
+    common::assert_matches_path("tests/snapshots/value_name_without_arg.bash.roff", cmd);
+}
diff --git a/clap_mangen/tests/snapshots/value_name_without_arg.bash.roff b/clap_mangen/tests/snapshots/value_name_without_arg.bash.roff
new file mode 100644
index 00000000000..2ffbe0210fa
--- /dev/null
+++ b/clap_mangen/tests/snapshots/value_name_without_arg.bash.roff
@@ -0,0 +1,15 @@
+.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/-/-flag/fR] [/fB/-h/fR|/fB/-/-help/fR] 
+.SH DESCRIPTION
+.SH OPTIONS
+.TP
+/fB/-/-flag/fR
+
+.TP
+/fB/-h/fR, /fB/-/-help/fR
+Print help