-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Specify defaults in terms of the underlying type rather than strings #1694
Comments
Could you provide a small example so we are on the same page? Thanks |
e.g. enum Switch {
Magic,
MoreMagic,
}
impl FromStr for Switch {
// ...
}
#[derive(Clap)]
struct Opts {
#[clap(default_value(Switch::MoreMagic))]
switch: Switch,
} Currently you have to do e.g. |
If your Full example: use std::fmt;
use clap::Clap;
#[derive(Clap, Debug, PartialEq, Copy, Clone)]
enum Magic {
Little,
Lots,
None,
}
impl Default for Magic {
fn default() -> Self {
Magic::Little
}
}
impl fmt::Display for Magic {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{:?}", self)
}
}
#[derive(Clap)]
struct Ctx {
/// How much magic do you want?
#[clap(long, arg_enum, default_value)]
magic: Magic,
}
fn main() {
let ctx = Ctx::parse();
} |
@emilazy it's not 100% what you asked for, but in your example it's at least part of the way there. I need to re-familiarize myself with the derive macros we made, but I think the underlying issue is that it just calls |
Implementation wise, this would just adding one more method to the ArgEnum trait. |
We would also potentially need to impl |
This would still be useful for cases where the underlying default of the application differs from the default of the type: use clap::Clap;
use std::env;
use std::path::PathBuf;
#[derive(Clap, Debug)]
#[clap(author, version)]
pub struct App {
#[clap(name = "REPO", default = "default_repo_path", parse(from_os_str))]
pub repo_path: PathBuf,
}
fn default_repo_path() -> PathBuf {
env::current_dir().expect("Failed to find current working directory!")
} This is currently IIRC not possible. The default of |
A similar change would also be useful for use clap::{arg_enum, App, Arg};
arg_enum! {
#[derive(Debug, PartialEq)]
enum EnumOpt {
OptionA,
OptionB,
}
}
fn main() {
let matches = App::new("test")
.arg(
Arg::with_name("enum")
.long("enum")
.takes_value(true)
.required(true) .possible_values(&EnumOpt::variants())
.case_insensitive(true),
)
.arg(
Arg::with_name("dependent")
.long("dependent")
.required_if("enum", "OptionB"),
)
.get_matches();
println!("Matches = {:?}", matches);
} If this is run as |
#2612 adds |
Also wanted to add
We'll do that extra parsing anyways because we store the default in |
Another way of solving this problem is we change the attributes to:
StructOpt porting work
Benefits
|
Right now - `default_value="something"` is a raw method - `default_value` uses native types This commit splits the meanings - `default_value="something"` is a raw method - `default_value_t` uses `T::default()` - `default_value_t=expr` uses an expression that evaluates to `T` This is meant to mirror the `value_of` / `value_of_t` API. At the moment, this is limited to `T: Display` to work with clap's default system. Something we can look at in the future is a way to loosen that restriction. One quick win is to specialize when `arg_enum` is set. The main downside is complicating the processing of attributes because it then means we need some processed before others. Since this builds on `clap`s existing default system, this also means users do not get any performance gains out of using `default_value_t`, since we still need to parse it but we also need to convert it to a string. Fixes clap-rs#1694
Right now - `default_value="something"` is a raw method - `default_value` uses native types This commit splits the meanings - `default_value="something"` is a raw method - `default_value_t` uses `T::default()` - `default_value_t=expr` uses an expression that evaluates to `T` This is meant to mirror the `value_of` / `value_of_t` API. At the moment, this is limited to `T: Display` to work with clap's default system. Something we can look at in the future is a way to loosen that restriction. One quick win is to specialize when `arg_enum` is set. The main downside is complicating the processing of attributes because it then means we need some processed before others. Since this builds on `clap`s existing default system, this also means users do not get any performance gains out of using `default_value_t`, since we still need to parse it but we also need to convert it to a string. Fixes clap-rs#1694
Right now - `default_value="something"` is a raw method - `default_value` uses native types This commit splits the meanings - `default_value="something"` is a raw method - `default_value_t` uses `T::default()` - `default_value_t=expr` uses an expression that evaluates to `T` This is meant to mirror the `value_of` / `value_of_t` API. At the moment, this is limited to `T: Display` to work with clap's default system. Something we can look at in the future is a way to loosen that restriction. One quick win is to specialize when `arg_enum` is set. The main downside is complicating the processing of attributes because it then means we need some processed before others. Since this builds on `clap`s existing default system, this also means users do not get any performance gains out of using `default_value_t`, since we still need to parse it but we also need to convert it to a string. Fixes clap-rs#1694
Right now - `default_value="something"` is a raw method - `default_value` uses native types This commit splits the meanings - `default_value="something"` is a raw method - `default_value_t` uses `T::default()` - `default_value_t=expr` uses an expression that evaluates to `T` This is meant to mirror the `value_of` / `value_of_t` API. At the moment, this is limited to `T: Display` to work with clap's default system. Something we can look at in the future is a way to loosen that restriction. One quick win is to specialize when `arg_enum` is set. The main downside is complicating the processing of attributes because it then means we need some processed before others. Since this builds on `clap`s existing default system, this also means users do not get any performance gains out of using `default_value_t`, since we still need to parse it but we also need to convert it to a string. Fixes clap-rs#1694
I really like how the new clap v3 is shaping up! Now that structopt is integrated, it'd be great if defaults could be specified in terms of the default resulting value they produce rather than as a string. Other argument parsing libraries like Python's argparse work this way.
The text was updated successfully, but these errors were encountered: