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

Arg::to_string panics #5627

Open
2 tasks done
crowlKats opened this issue Aug 6, 2024 · 2 comments
Open
2 tasks done

Arg::to_string panics #5627

crowlKats opened this issue Aug 6, 2024 · 2 comments
Labels
A-builder Area: Builder API C-bug Category: Updating dependencies S-waiting-on-decision Status: Waiting on a go/no-go before implementing

Comments

@crowlKats
Copy link

crowlKats commented Aug 6, 2024

Please complete the following tasks

Rust Version

1.79.0

Clap Version

4.5.13

Minimal reproducible code

fn main() {
    let cmd = clap::Command::new("cargo")
      .bin_name("cargo")
      .arg(clap::Arg::new("manifest-path").long("manifest-path"));

    dbg!(cmd.get_arguments().next().unwrap().to_string());
}

Steps to reproduce the bug with the above code

cargo run

Actual Behaviour

its panicking with
thread 'main' panicked at /Users/crowlkats/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.13/src/builder/arg.rs:4001:29
which is

self.get_num_args().expect(INTERNAL_ERROR_MSG).min_values()

Expected Behaviour

shouldn't panic

Additional Context

Am trying to get the arg usage representation to do a basic JSON output.

Debug Output

No response

@crowlKats crowlKats added the C-bug Category: Updating dependencies label Aug 6, 2024
@epage
Copy link
Member

epage commented Aug 6, 2024

You need to cmd.build() first.

The next question is if our panic message should call that out. I'm unsure. There are a lot of corner cases and am unsure if we should go for "catastrophic failure" or "you didn't do one specific thing" (when you might have and instead something else went wrong).

@crowlKats
Copy link
Author

You need to cmd.build() first.

Ah, that makes sense.

The next question is if our panic message should call that out. I'm unsure. There are a lot of corner cases and am unsure if we should go for "catastrophic failure" or "you didn't do one specific thing" (when you might have and instead something else went wrong).

I tend to be in favour of more documentation, especially if its a behaviour that is here to stay indefinitively and has a reason, but I do understand your point, especially if such cases are common, so not sure either which would be better.

@epage epage added S-waiting-on-decision Status: Waiting on a go/no-go before implementing A-builder Area: Builder API labels Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builder Area: Builder API C-bug Category: Updating dependencies S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Projects
None yet
Development

No branches or pull requests

2 participants