Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flatten causes the wrong doc-comment to be respected #3127

Closed
epage opened this issue Dec 9, 2021 · 14 comments
Closed

flatten causes the wrong doc-comment to be respected #3127

epage opened this issue Dec 9, 2021 · 14 comments
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies

Comments

@epage
Copy link
Member

epage commented Dec 9, 2021

Issue by epage
Thursday Jan 16, 2020 at 13:46 GMT
Originally opened as TeXitoi/structopt#333


With clap-verbosity-flag "3.0.0"

use structopt::StructOpt;
use clap_verbosity_flag::Verbosity;

#[derive(Debug, StructOpt)]
/// Foo
struct Cli {
    #[structopt(flatten)]
    verbose: Verbosity,
}

fn main() {
    Cli::from_args();
}

will cause clap-verbosity-flag's documentation to be used for the help instead of the user-specified one

❯ ./target/debug/examples/simple --help
clap-verbosity-flag 0.3.0
Easily add a `--verbose` flag to CLIs using Structopt

# Examples

```rust use structopt::StructOpt; use clap_verbosity_flag::Verbosity;

/// Le CLI #[derive(Debug, StructOpt)] struct Cli { #[structopt(flatten)] verbose: Verbosity, } # # fn main() {} ```

USAGE:
    simple [FLAGS]

FLAGS:
    -h, --help
            Prints help information

    -q, --quiet
            Pass many times for less log output

    -V, --version
            Prints version information

    -v, --verbose
            Pass many times for more log output

            By default, it'll only report errors. Passing `-v` one time also prints warnings, `-vv` enables info
            logging, `-vvv` debug, and `-vvvv` trace.

I am working around it in clap-verbosity-flag by. not providing a doc comment (clap-rs/clap-verbosity-flag#21).

A workaround is explained in TeXitoi/structopt#333 (comment)

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by epage
Thursday Jan 16, 2020 at 13:46 GMT


Based on reports (clap-rs/clap-verbosity-flag#20) I'm guessing TeXitoi/structopt@e2270de broke this

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by CreepySkeleton
Thursday Jan 16, 2020 at 15:23 GMT


will cause clap-verbosity-flag's documentation to be used for the help instead of the user-specified one

But there's no user specified help here as far as I can see 😕

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by epage
Thursday Jan 16, 2020 at 15:24 GMT


/// Foo
struct Cli {

The user's doc-comment is being overridden by the doc-comment from the chain. Instead of "Foo", we're getting library documentation

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by CreepySkeleton
Wednesday Jan 22, 2020 at 13:30 GMT


This is caused by #290 since doc comments on top of struct/enum are top level attributes too :(

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by ssokolow
Monday Jun 22, 2020 at 21:31 GMT


I just noticed that this also causes the doc comment on my boilerplate opts (which get flattened into the main struct) to override the about attribute on the main struct.

I'm really starting to feel like I need to write a regression/integration suite on my --help output just to feel safe using --help generation.

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by pizzamig
Wednesday Jul 22, 2020 at 19:29 GMT


This is caused by #290 since doc comments on top of struct/enum are top level attributes too :(

Is there any update or way to fix this?
Currently I cannot document a struct and re-use it flattened and the workaround to move documentation to the lib doesn't work for me now, it would require quite some changes in my code.

The complexity of structopt is too high for me right now to try to find a fix.

thanks in advance

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by CreepySkeleton
Wednesday Jul 22, 2020 at 20:49 GMT


I tried to fix it. I couldn't. I'll take another round a bit later.

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by vorner
Saturday Oct 17, 2020 at 09:22 GMT


Hello

I guess the „proper“ way to fix it will be hard and will take time (I'm not sure if reordering the calls inside the StructOptInternal::augment_clap to call these after descending into flattened fields instead of at the beginning would work ‒ it seems version is called at the end, so possibly the .about and similar could too?).

But if it is hard, would it make sense to add some kind of #[no_about] parameter that would skip all the auto-generated application information, like about and version? I guess most people know that a struct will be used as flattened field and could annotate them.

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by TeXitoi
Saturday Oct 17, 2020 at 10:30 GMT


By default, there is no about, but doc comment are about, so the workaround would be no doc comment on these structs.

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by vorner
Saturday Oct 17, 2020 at 10:40 GMT


I've figured that, but it is kind of bad for structs provided by libraries. I want to have doc comment to render on docs.rs, I also tend to #[warn(missing_docs)]. But then structdoc tries to interpret it and hijacks it if someone flattens the structure into their top-level Opts structure, which is bad.

But as the author of the library I know my struct should not be used directly and that it should never provide the about and --version. I want to mark it as just a source of more fields, not to do anything with the App at all.

https://docs.rs/spirit-daemonize/0.3.2/spirit_daemonize/struct.Opts.html

Maybe the right name would be #[ignore_doc_comment] or #[flattenable] or like that? Or is this orthogonal to this bug?

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by vorner
Monday Oct 19, 2020 at 15:59 GMT


Just for the record, I've figured a workaround that keeps them for the docs.rs, but doesn't hijacks them for the about:

#[cfg_attr(not(doc), allow(missing_docs))]
#[cfg_attr(doc, doc = r#"
The doc comment goes here

Following the doc comment
"#)]

It simply enables the doc comment only for the documentation generation (and seems to work with doc tests too)

@epage
Copy link
Member Author

epage commented Dec 9, 2021

Comment by TeXitoi
Monday Oct 19, 2020 at 16:36 GMT


I've put a link to your workaround in the description of the issue for ease of access.

@epage epage added A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies labels Dec 9, 2021
@epage
Copy link
Member Author

epage commented Dec 10, 2021

Assuming this is a valid reproduction case, this is fixed

use clap::StructOpt;

#[derive(Debug, StructOpt)]
#[structopt(name = "structopt-bug", about = "Show a bug in Structopt")]
struct Opt {
    #[structopt(flatten)]
    v: Verbose,
}

/// Re-usable Verbose flag
#[derive(Debug, StructOpt)]
struct Verbose {
    /// Enable the verbosity messages
    #[structopt(short)]
    verbose: bool,
}
fn main() {
    let opt = Opt::from_args();
    println!("{:?}", opt);
}
structopt-bug 

Show a bug in Structopt

USAGE:
    test-clap [OPTIONS]

OPTIONS:
    -h, --help    Print help information
    -v            Enable the verbosity messages

@epage epage closed this as completed Dec 10, 2021
@epage
Copy link
Member Author

epage commented Dec 10, 2021

To add, we still have #2983 which is about about/long_about interactions being weird

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

1 participant