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

try_update_from returns an Error MissingRequiredArgument for argument with existing values #4617

Closed
2 tasks done
nh13 opened this issue Jan 9, 2023 · 1 comment · Fixed by #4618
Closed
2 tasks done
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much

Comments

@nh13
Copy link
Contributor

nh13 commented Jan 9, 2023

Please complete the following tasks

Rust Version

1.66.0

Clap Version

4.0.32

Minimal reproducible code

use clap::Parser;

#[derive(Parser, Debug, Clone)]
#[clap(name = "tool", version = "0.0.1", about="short", long_about="long", term_width=0)]

pub struct Opts {
  #[clap(long, short='f', required=true)]
  pub foo: usize,
  #[clap(long, short='b', required=true)]
  pub bar: usize,
}

pub fn main() {
  let mut opts = Opts {
      foo: 0,
      bar: 1,
  };
  println!("initial opts: {:?}", opts);
  
  let argv = vec!["tool", "--bar", "2"];
  
  match opts.try_update_from(argv.into_iter()) {
      Ok(()) => println!("OK: {:?}", opts),
      Err(err) => println!("ERR: {:?}", err)
  }
}

Steps to reproduce the bug with the above code

Please see: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=43678edf67d7e39b179c141c55d2036d

Actual Behaviour

try_update_from returns an error of kind MissingRequiredArgument

Expected Behaviour

try_update_from should succeed as foo already set in opt

Additional Context

No response

Debug Output

No response

@nh13 nh13 added the C-bug Category: Updating dependencies label Jan 9, 2023
@epage
Copy link
Member

epage commented Jan 9, 2023

In update_from, we skip applying our implicit required = true but we are not overriding the users explicit required = true which we should

@epage epage added E-easy Call for participation: Experience needed to fix: Easy / not much A-derive Area: #[derive]` macro API labels Jan 9, 2023
epage added a commit to epage/clap that referenced this issue Jan 10, 2023
nh13 added a commit to Singular-Genomics/singular-demux that referenced this issue Jan 10, 2023
The fastqs clap arugment had `required = True` for `fastqs`.  However,
when fastqs is provided on the command line and there are also command
line arguments in the sample sheet, a missing argument error is
encountered due to "fastqs not being provided".
This occurs when trying to use the `try_update_from` on `Opts`.  A
reproducible example independent of this code base can be found here:
clap-rs/clap#4617
nh13 added a commit to Singular-Genomics/singular-demux that referenced this issue Jan 10, 2023
…so in a sample sheet (#33)

* [bugfix] do not require FASTQs on the command line

The fastqs clap arugment had `required = True` for `fastqs`.  However,
when fastqs is provided on the command line and there are also command
line arguments in the sample sheet, a missing argument error is
encountered due to "fastqs not being provided".
This occurs when trying to use the `try_update_from` on `Opts`.  A
reproducible example independent of this code base can be found here:
clap-rs/clap#4617

* [internal] minor doc fixup in sample metadata validate_samples

* add unit test for pre-specified FASTQs with sample sheet
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 E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants