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

Option arg cannot have None as default value #5078

Closed
2 tasks done
justinlovinger opened this issue Aug 17, 2023 · 7 comments
Closed
2 tasks done

Option arg cannot have None as default value #5078

justinlovinger opened this issue Aug 17, 2023 · 7 comments
Labels
C-bug Category: Updating dependencies

Comments

@justinlovinger
Copy link

Please complete the following tasks

Rust Version

rustc 1.70.0 (90c541806 2023-05-31) (built from a source tarball)

Clap Version

4.3.21

Minimal reproducible code

#[derive(Parser)]
struct Args {
    #[arg(long, num_args = 0..=1, default_value = "[]")]
    foo: Option<usize>,
}

fn main() {
    let args = Args::parse();
}

Steps to reproduce the bug with the above code

cargo run

Actual Behaviour

The program exits with error: invalid value '[]' for '--foo [<FOO>]': invalid digit found in string.

Expected Behaviour

The program should run with args.foo set to None.

Additional Context

num_args = 0..=1 disables the implied required = false behavior of Option<T>.

--foo [] is valid if entered from the command line. It is only not valid when used as a default value.

default_value_t = None fails because Option does not implement Display.

Debug Output

[clap_builder::builder::command]Command::_do_parse
[clap_builder::builder::command]Command::_build: name="owm"
[clap_builder::builder::command]Command::_propagate:owm
[clap_builder::builder::command]Command::_check_help_and_version:owm expand_help_tree=false
[clap_builder::builder::command]Command::long_help_exists
[clap_builder::builder::command]Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]Command::_propagate_global_args:owm
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:foo
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]Command::_verify_positionals
[clap_builder::parser::parser]Parser::get_matches_with
[clap_builder::parser::parser]Parser::add_defaults
[clap_builder::parser::parser]Parser::add_defaults:iter:foo:
[clap_builder::parser::parser]Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]Parser::add_default_value:iter:foo: has default vals
[clap_builder::parser::parser]Parser::add_default_value:iter:foo: wasn't used
[clap_builder::parser::parser]Parser::react action=Set, identifier=None, source=DefaultValue
[clap_builder::parser::arg_matcher]ArgMatcher::start_custom_arg: id="foo", source=DefaultValue
[clap_builder::parser::parser]Parser::push_arg_values: ["[]"]
[clap_builder::parser::parser]Parser::add_single_val_to_arg: cur_idx:=1
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
error: invalid value '[]' for '--foo []': invalid digit found in string

@justinlovinger justinlovinger added the C-bug Category: Updating dependencies label Aug 17, 2023
@epage
Copy link
Member

epage commented Aug 18, 2023

#[derive(Parser)]
struct Args {
    #[arg(long, num_args = 0..=1, default_value = "[]")]
    foo: Option<usize>,
}

says to parse [] as a usize

#[derive(Parser)]
struct Args {
    #[arg(long, num_args = 0..=1, default_value_t = None)]
    foo: Option<usize>,
}

Recognizes usize as the core type and expects default_value_t to be for the core type. There are other advanced features you can use to override that to make it None which is why this makes sense.

#[derive(Parser)]
struct Args {
    #[arg(long, num_args = 0..=1, default_value_t)]
    foo: Option<usize>,
}

Says to use `Default::default

--foo [] is valid if entered from the command line. It is only not valid when used as a default value.

Not for me:

$ ./clap-5078.rs --foo []
warning: `package.edition` is unspecified, defaulting to `2021`
warning: unused variable: `args`
  --> clap-5078.rs:19:9
   |
19 |     let args = Args::parse();
   |         ^^^^ help: if this is intentional, prefix it with an underscore: `_args`
   |
   = note: `#[warn(unused_variables)]` on by default

warning: `clap-5078` (bin "clap-5078") generated 1 warning (run `cargo fix --bin "clap-5078"` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `/home/epage/.cargo/target/92/58a7b79897d359/debug/clap-5078 --foo '[]'`
error: invalid value '[]' for '--foo [<FOO>]': invalid digit found in string

For more information, try '--help'.

You might be running into shell specific logic where [] becomes "". If you do '[]', then you will likely bypass shell processing and see what is actually happening when it is passed into clap.

num_args = 0..=1 disables the implied required = false behavior of Option.

I'm not sure what this is referring to. It doesn't do that for me. I removed any defaulting and it runs as I would expect unless I'm missing something about what you are trying to accomplish.

@justinlovinger
Copy link
Author

num_args = 0..=1 disables the implied required = false behavior of Option.

I'm not sure what this is referring to. It doesn't do that for me. I removed any defaulting and it runs as I would expect unless I'm missing something about what you are trying to accomplish.

This is referring to overriding the "assumed intent", https://docs.rs/clap/latest/clap/_derive/index.html#arg-types. From my testing, it makes the core type Option<usize> instead of it being an optional usize. It is like Option<Option<usize>>, but without the extra Option.

You might be running into shell specific logic where [] becomes "". If you do '[]', then you will likely bypass shell processing and see what is actually happening when it is passed into clap.

You may be right about this being shell-specific behavior. It appears equivalent to --foo. In which case, the core issue may be how to pass an explicit None value to an Option arg. --foo "" is not valid.

P.S. Vec<T> may have the same issue as I cannot find any way to pass an empty vector when using value_delimiter = ','.

@epage
Copy link
Member

epage commented Aug 18, 2023

It might help if you stepped back and explained what you are trying to accomplish. What behavior do you want out of --foo?

@justinlovinger
Copy link
Author

I want --foo to either take some value, like --foo 10, or a "none" value, like --foo None or --foo "".

For context, my application wraps a function that takes max_width: Option<usize> and max_height: Option<usize>. These can either be Some(value), in which case the function will run with a max width or height, or None, in which case it will run with no max width or height. These values can be set with the CLI, and I am trying to reflect this behavior in the CLI.

Additionally, by default, the CLI sets a max width, 1920, and no max height. The user should be able to set max width to None and set a value for max height. I want these clearly reflected in --help, so a user can look at the --max-height default, see it is set to "no max height", understand that as a possibility, and understand they can pass the same value to --max-width to get the same behavior for width.

@epage
Copy link
Member

epage commented Aug 18, 2023

So you want to opt-out of special Option handling and have the value_parser return an Option<usize>. Currently special handling is purely textual. You can use std::option::Option or type OptionUsize = Option<usize> to work around this (see also #4626).

You can then do:

fn parser(raw: &str) -> Result<OptionUsize, String> {
    if raw == "None" || raw.is_empty() {
        Ok(None)
    } else {
        raw.parse::<usize>().map(Some)
    }
}


...

#[derive(Parser)]
struct Args {
    #[arg(long, value_parser = parser)]
    foo: OptionUsize,
}

@justinlovinger
Copy link
Author

That work for my case. I suppose the question then is, why does Clap not support --foo "" for Option types out of the box?

@epage
Copy link
Member

epage commented Aug 20, 2023

We don't know what the intention is for an empty string.

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants