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 #333

Open
epage opened this issue Jan 16, 2020 · 12 comments
Open

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

epage opened this issue Jan 16, 2020 · 12 comments
Assignees
Labels
bug This is a BUG. The fix may be released in a patch version even if considered breaking

Comments

@epage
Copy link
Contributor

epage commented Jan 16, 2020

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 #333 (comment)

@epage
Copy link
Contributor Author

epage commented Jan 16, 2020

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

@CreepySkeleton
Copy link
Collaborator

CreepySkeleton commented Jan 16, 2020

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
Contributor Author

epage commented Jan 16, 2020

/// 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

@CreepySkeleton CreepySkeleton added the bug This is a BUG. The fix may be released in a patch version even if considered breaking label Jan 16, 2020
@CreepySkeleton CreepySkeleton self-assigned this Jan 16, 2020
@CreepySkeleton
Copy link
Collaborator

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

@ssokolow
Copy link

ssokolow commented Jun 22, 2020

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.

@pizzamig
Copy link

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

@CreepySkeleton
Copy link
Collaborator

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

@vorner
Copy link

vorner commented Oct 17, 2020

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.

@TeXitoi
Copy link
Owner

TeXitoi commented Oct 17, 2020

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

@vorner
Copy link

vorner commented Oct 17, 2020

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?

vorner added a commit to vorner/spirit that referenced this issue Oct 19, 2020
Work around for the issue where doc comments „sneak in“ while flattening
into the program description

TeXitoi/structopt#333

Simply enable the doc comments only when building the docs or doc
comments.
@vorner
Copy link

vorner commented Oct 19, 2020

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)

vorner added a commit to vorner/spirit that referenced this issue Oct 19, 2020
Work around for the issue where doc comments „sneak in“ while flattening
into the program description

TeXitoi/structopt#333

Simply enable the doc comments only when building the docs or doc
comments.
@TeXitoi
Copy link
Owner

TeXitoi commented Oct 19, 2020

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

fimmind added a commit to fimmind/dotmake that referenced this issue Feb 12, 2021
The problem is described in these issues:
TeXitoi/structopt#333
TeXitoi/structopt#391

The is easiest workaround is to remove doc comments from `Options` and
`Subcommand`
mdekstrand added a commit to mdekstrand/happylog that referenced this issue Apr 26, 2021
gakonst pushed a commit to foundry-rs/foundry that referenced this issue Jan 5, 2022
notmandatory added a commit to notmandatory/bdk-cli that referenced this issue Jan 9, 2022
Was showing proxy opts description instead of repl description.
This is a workaround for `structopt` issue #333, #391, #418;
see TeXitoi/structopt#333 (comment)
notmandatory added a commit to notmandatory/bdk-cli that referenced this issue Jan 9, 2022
Was showing proxy opts description instead of repl description.
This is a workaround for `structopt` issue #333, #391, #418;
see TeXitoi/structopt#333 (comment)
notmandatory added a commit to notmandatory/bdk-cli that referenced this issue Jan 9, 2022
Was showing proxy opts description instead of repl description.
This is a workaround for `structopt` issue #333, #391, #418;
see TeXitoi/structopt#333 (comment)
notmandatory added a commit to notmandatory/bdk-cli that referenced this issue Jan 9, 2022
Was showing proxy opts description instead of repl description.
This is a workaround for `structopt` issue #333, #391, #418;
see TeXitoi/structopt#333 (comment)
notmandatory added a commit to notmandatory/bdk-cli that referenced this issue Jan 9, 2022
This fixes the help docs for the repl and wallet subcommands. The repl subcommand was showing the proxy options docs instead, and the wallet subcommand was showing the description from the WalletSubCommand enum.
This is a workaround for structopt issue #333, #391, #418.
see TeXitoi/structopt#333 (comment)
notmandatory added a commit to notmandatory/bdk-cli that referenced this issue Feb 11, 2022
This fixes the help docs for the repl and wallet subcommands. The repl subcommand was showing the proxy options docs instead, and the wallet subcommand was showing the description from the WalletSubCommand enum.
This is a workaround for structopt issue #333, #391, #418.
see TeXitoi/structopt#333 (comment)
notmandatory added a commit to bitcoindevkit/bdk-cli that referenced this issue Feb 12, 2022
3f9b892 Pin fd-lock version to 3.0.2 (Steve Myers)
4a02a9f Manual docs formatting fix (Steve Myers)
c82fd90 Update Cargo.lock (Steve Myers)
81652b1 Fix repl and wallet help docs (Steve Myers)

Pull request description:

  ### Description

  This fixes the help docs for the `repl` and `wallet` subcommands. The `repl` subcommand was showing the proxy options docs instead, and the `wallet` subcommand was showing the description from the `WalletSubCommand` enum.

  This is a workaround for `structopt` issue #333, #391, #418; see TeXitoi/structopt#333 (comment)

  ### Notes to the reviewers

  Before this PR this was showing:
  ```shell
  $ cargo run --features esplora-ureq,compiler -- --help
  ...
  SUBCOMMANDS:
      compile    Compile a miniscript policy to an output descriptor
      help       Prints this message or the help of the given subcommand(s)
      key        Key management sub-commands
      repl       Proxy Server options
      wallet     Wallet sub-commands
  ```

  After this PR we get:
  ```shell
  $ cargo run --features esplora-ureq,compiler -- --help
  ...
  SUBCOMMANDS:
      compile    Compile a miniscript policy to an output descriptor
      help       Prints this message or the help of the given subcommand(s)
      key        Key management sub-commands
      repl       Enter REPL command loop mode
      wallet     Wallet options and sub-commands
  ```

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [ ] I'm linking the issue being fixed by this PR

Top commit has no ACKs.

Tree-SHA512: 8d708720f60940edd40826a159388ced261c91d23e074b51164f8c49a2e673cbaecee0241e52cb02090e40ff1f989a980aee0122230cee211f4c3284cad4483c
facebook-github-bot pushed a commit to facebook/buck2 that referenced this issue Jun 22, 2022
Summary: This workaround seems to be unnecessary since we migrated to newer clap.

Reviewed By: stepancheg

Differential Revision: D37334350

fbshipit-source-id: 3b5c116ec492ea8e6be2fe61cbdd975a115573ba
charisma98 added a commit to charisma98/foundry that referenced this issue Mar 4, 2023
0129general added a commit to 0129general/FoundryProject that referenced this issue May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a BUG. The fix may be released in a patch version even if considered breaking
Projects
None yet
Development

No branches or pull requests

6 participants