Skip to content

Commit

Permalink
fix(parser): Simplify the common getter API
Browse files Browse the repository at this point in the history
Clap has focused on reporting development errors through assertions
rather than mixing user errors with development errors.  Sometimes,
developers need to handle things more flexibly so included in clap-rs#3732 was
the reporting of value accessor failures as internal errors with a
distinct type.  I've been going back and forth on whether the extra
error pessimises the usability in the common case vs dealing with the
proliferation of different function combinations.  In working on
deprecating the `value_of` functions, I decided that it was going to be
worth duplicating so long as we can keep the documentation focused.
  • Loading branch information
epage committed May 25, 2022
1 parent 5aea9ad commit eda0ca5
Show file tree
Hide file tree
Showing 25 changed files with 119 additions and 172 deletions.
5 changes: 0 additions & 5 deletions clap_derive/src/derives/args.rs
Expand Up @@ -601,7 +601,6 @@ fn gen_parsers(
Ty::Option => {
quote_spanned! { ty.span()=>
#arg_matches.#get_one(#id)
.expect("unexpected type")
.map(#deref)
.map(#parse)
.transpose()?
Expand All @@ -612,7 +611,6 @@ fn gen_parsers(
if #arg_matches.is_present(#id) {
Some(
#arg_matches.#get_one(#id)
.expect("unexpected type")
.map(#deref)
.map(#parse).transpose()?
)
Expand All @@ -624,7 +622,6 @@ fn gen_parsers(
Ty::OptionVec => quote_spanned! { ty.span()=>
if #arg_matches.is_present(#id) {
Some(#arg_matches.#get_many(#id)
.expect("unexpected type")
.map(|v| v.map(#deref).map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
.transpose()?
.unwrap_or_else(Vec::new))
Expand All @@ -636,7 +633,6 @@ fn gen_parsers(
Ty::Vec => {
quote_spanned! { ty.span()=>
#arg_matches.#get_many(#id)
.expect("unexpected type")
.map(|v| v.map(#deref).map::<::std::result::Result<#convert_type, clap::Error>, _>(#parse).collect::<::std::result::Result<Vec<_>, clap::Error>>())
.transpose()?
.unwrap_or_else(Vec::new)
Expand All @@ -658,7 +654,6 @@ fn gen_parsers(
Ty::Other => {
quote_spanned! { ty.span()=>
#arg_matches.#get_one(#id)
.expect("unexpected type")
.map(#deref)
.ok_or_else(|| clap::Error::raw(clap::ErrorKind::MissingRequiredArgument, format!("The following required argument was not provided: {}", #id)))
.and_then(#parse)?
Expand Down
1 change: 0 additions & 1 deletion clap_derive/src/derives/subcommand.rs
Expand Up @@ -528,7 +528,6 @@ fn gen_from_arg_matches(
.chain(
#sub_arg_matches_var
.remove_many::<#str_ty>("")
.expect("unexpected type")
.into_iter().flatten() // `""` isn't present, bug in `unstable-v4`
.map(#str_ty::from)
)
Expand Down
4 changes: 2 additions & 2 deletions examples/cargo-example.md
Expand Up @@ -37,9 +37,9 @@ OPTIONS:
Then to directly invoke the command, run:
```console
$ cargo-example example
Ok(None)
None

$ cargo-example example --manifest-path Cargo.toml
Ok(Some("Cargo.toml"))
Some("Cargo.toml")

```
9 changes: 2 additions & 7 deletions examples/derive_ref/flatten_hand_args.rs
Expand Up @@ -17,9 +17,7 @@ impl FromArgMatches for CliArgs {
Ok(Self {
foo: matches.is_present("foo"),
bar: matches.is_present("bar"),
quuz: matches
.remove_one::<String>("quuz")
.expect("matches definition"),
quuz: matches.remove_one::<String>("quuz"),
})
}
fn update_from_arg_matches(&mut self, matches: &ArgMatches) -> Result<(), Error> {
Expand All @@ -29,10 +27,7 @@ impl FromArgMatches for CliArgs {
fn update_from_arg_matches_mut(&mut self, matches: &mut ArgMatches) -> Result<(), Error> {
self.foo |= matches.is_present("foo");
self.bar |= matches.is_present("bar");
if let Some(quuz) = matches
.remove_one::<String>("quuz")
.expect("matches definition")
{
if let Some(quuz) = matches.remove_one::<String>("quuz") {
self.quuz = Some(quuz);
}
Ok(())
Expand Down
8 changes: 1 addition & 7 deletions examples/escaped-positional.rs
Expand Up @@ -24,18 +24,12 @@ fn main() {
// -f used: true
println!("-f used: {:?}", matches.is_present("eff"));
// -p's value: Some("bob")
println!(
"-p's value: {:?}",
matches
.get_one::<String>("pea")
.expect("matches definition")
);
println!("-p's value: {:?}", matches.get_one::<String>("pea"));
// 'slops' values: Some(["sloppy", "slop", "slop"])
println!(
"'slops' values: {:?}",
matches
.get_many::<String>("slop")
.expect("matches definition")
.map(|vals| vals.collect::<Vec<_>>())
.unwrap_or_default()
);
Expand Down
24 changes: 5 additions & 19 deletions examples/git.rs
Expand Up @@ -51,25 +51,18 @@ fn main() {
Some(("clone", sub_matches)) => {
println!(
"Cloning {}",
sub_matches
.get_one::<String>("REMOTE")
.expect("matches definition")
.expect("required")
sub_matches.get_one::<String>("REMOTE").expect("required")
);
}
Some(("push", sub_matches)) => {
println!(
"Pushing to {}",
sub_matches
.get_one::<String>("REMOTE")
.expect("matches definition")
.expect("required")
sub_matches.get_one::<String>("REMOTE").expect("required")
);
}
Some(("add", sub_matches)) => {
let paths = sub_matches
.get_many::<PathBuf>("PATH")
.expect("matches definition")
.into_iter()
.flatten()
.collect::<Vec<_>>();
Expand All @@ -79,21 +72,15 @@ fn main() {
let stash_command = sub_matches.subcommand().unwrap_or(("push", sub_matches));
match stash_command {
("apply", sub_matches) => {
let stash = sub_matches
.get_one::<String>("STASH")
.expect("matches definition");
let stash = sub_matches.get_one::<String>("STASH");
println!("Applying {:?}", stash);
}
("pop", sub_matches) => {
let stash = sub_matches
.get_one::<String>("STASH")
.expect("matches definition");
let stash = sub_matches.get_one::<String>("STASH");
println!("Popping {:?}", stash);
}
("push", sub_matches) => {
let message = sub_matches
.get_one::<String>("message")
.expect("matches definition");
let message = sub_matches.get_one::<String>("message");
println!("Pushing {:?}", message);
}
(name, _) => {
Expand All @@ -104,7 +91,6 @@ fn main() {
Some((ext, sub_matches)) => {
let args = sub_matches
.get_many::<OsString>("")
.expect("matches definition")
.into_iter()
.flatten()
.collect::<Vec<_>>();
Expand Down
12 changes: 2 additions & 10 deletions examples/pacman.rs
Expand Up @@ -73,7 +73,6 @@ fn main() {
if sync_matches.is_present("search") {
let packages: Vec<_> = sync_matches
.get_many::<String>("search")
.expect("matches definition")
.expect("is present")
.map(|s| s.as_str())
.collect();
Expand All @@ -84,7 +83,6 @@ fn main() {

let packages: Vec<_> = sync_matches
.get_many::<String>("package")
.expect("matches definition")
.expect("is present")
.map(|s| s.as_str())
.collect();
Expand All @@ -97,16 +95,10 @@ fn main() {
}
}
Some(("query", query_matches)) => {
if let Some(packages) = query_matches
.get_many::<String>("info")
.expect("matches definition")
{
if let Some(packages) = query_matches.get_many::<String>("info") {
let comma_sep = packages.map(|s| s.as_str()).collect::<Vec<_>>().join(", ");
println!("Retrieving info for {}...", comma_sep);
} else if let Some(queries) = query_matches
.get_many::<String>("search")
.expect("matches definition")
{
} else if let Some(queries) = query_matches.get_many::<String>("search") {
let comma_sep = queries.map(|s| s.as_str()).collect::<Vec<_>>().join(", ");
println!("Searching Locally for {}...", comma_sep);
} else {
Expand Down
10 changes: 2 additions & 8 deletions examples/tutorial_builder/01_quick.rs
Expand Up @@ -26,17 +26,11 @@ fn main() {
.get_matches();

// You can check the value provided by positional arguments, or option arguments
if let Some(name) = matches
.get_one::<String>("name")
.expect("matches definition")
{
if let Some(name) = matches.get_one::<String>("name") {
println!("Value for name: {}", name);
}

if let Some(config_path) = matches
.get_one::<PathBuf>("config")
.expect("matches definition")
{
if let Some(config_path) = matches.get_one::<PathBuf>("config") {
println!("Value for config: {}", config_path.display());
}

Expand Down
10 changes: 2 additions & 8 deletions examples/tutorial_builder/02_app_settings.rs
Expand Up @@ -13,16 +13,10 @@ fn main() {

println!(
"two: {:?}",
matches
.get_one::<String>("two")
.expect("matches definition")
.expect("required")
matches.get_one::<String>("two").expect("required")
);
println!(
"one: {:?}",
matches
.get_one::<String>("one")
.expect("matches definition")
.expect("required")
matches.get_one::<String>("one").expect("required")
);
}
10 changes: 2 additions & 8 deletions examples/tutorial_builder/02_apps.rs
Expand Up @@ -11,16 +11,10 @@ fn main() {

println!(
"two: {:?}",
matches
.get_one::<String>("two")
.expect("matches definition")
.expect("required")
matches.get_one::<String>("two").expect("required")
);
println!(
"one: {:?}",
matches
.get_one::<String>("one")
.expect("matches definition")
.expect("required")
matches.get_one::<String>("one").expect("required")
);
}
10 changes: 2 additions & 8 deletions examples/tutorial_builder/02_crate.rs
Expand Up @@ -10,16 +10,10 @@ fn main() {

println!(
"two: {:?}",
matches
.get_one::<String>("two")
.expect("matches definition")
.expect("required")
matches.get_one::<String>("two").expect("required")
);
println!(
"one: {:?}",
matches
.get_one::<String>("one")
.expect("matches definition")
.expect("required")
matches.get_one::<String>("one").expect("required")
);
}
7 changes: 1 addition & 6 deletions examples/tutorial_builder/03_02_option.rs
Expand Up @@ -7,10 +7,5 @@ fn main() {
.arg(arg!(-n --name <NAME>).required(false))
.get_matches();

println!(
"name: {:?}",
matches
.get_one::<String>("name")
.expect("matches definition")
);
println!("name: {:?}", matches.get_one::<String>("name"));
}
7 changes: 1 addition & 6 deletions examples/tutorial_builder/03_03_positional.rs
Expand Up @@ -5,10 +5,5 @@ use clap::{arg, command};
fn main() {
let matches = command!().arg(arg!([NAME])).get_matches();

println!(
"NAME: {:?}",
matches
.get_one::<String>("NAME")
.expect("matches definition")
);
println!("NAME: {:?}", matches.get_one::<String>("NAME"));
}
4 changes: 1 addition & 3 deletions examples/tutorial_builder/03_04_subcommands.rs
Expand Up @@ -17,9 +17,7 @@ fn main() {
match matches.subcommand() {
Some(("add", sub_matches)) => println!(
"'myapp add' was used, name is: {:?}",
sub_matches
.get_one::<String>("NAME")
.expect("matches definition")
sub_matches.get_one::<String>("NAME")
),
_ => unreachable!("Exhausted list of subcommands and subcommand_required prevents `None`"),
}
Expand Down
1 change: 0 additions & 1 deletion examples/tutorial_builder/03_05_default_values.rs
Expand Up @@ -11,7 +11,6 @@ fn main() {
"NAME: {:?}",
matches
.get_one::<String>("NAME")
.expect("matches definition")
.expect("default ensures there is always a value")
);
}
1 change: 0 additions & 1 deletion examples/tutorial_builder/04_01_enum.rs
Expand Up @@ -20,7 +20,6 @@ fn main() {
// Note, it's safe to call unwrap() because the arg is required
match matches
.get_one::<Mode>("MODE")
.expect("matches definition")
.expect("'MODE' is required and parsing will fail if its missing")
{
Mode::Fast => {
Expand Down
1 change: 0 additions & 1 deletion examples/tutorial_builder/04_01_possible.rs
Expand Up @@ -14,7 +14,6 @@ fn main() {
// Note, it's safe to call unwrap() because the arg is required
match matches
.get_one::<String>("MODE")
.expect("matches definition")
.expect("'MODE' is required and parsing will fail if its missing")
.as_str()
{
Expand Down
1 change: 0 additions & 1 deletion examples/tutorial_builder/04_02_parse.rs
Expand Up @@ -14,7 +14,6 @@ fn main() {
// Note, it's safe to call unwrap() because the arg is required
let port: u16 = *matches
.get_one::<u16>("PORT")
.expect("matches definition")
.expect("'PORT' is required and parsing will fail if its missing");
println!("PORT = {}", port);
}
1 change: 0 additions & 1 deletion examples/tutorial_builder/04_02_validate.rs
Expand Up @@ -16,7 +16,6 @@ fn main() {
// Note, it's safe to call unwrap() because the arg is required
let port: u16 = *matches
.get_one::<u16>("PORT")
.expect("matches definition")
.expect("'PORT' is required and parsing will fail if its missing");
println!("PORT = {}", port);
}
Expand Down
19 changes: 3 additions & 16 deletions examples/tutorial_builder/04_03_relations.rs
Expand Up @@ -47,10 +47,7 @@ fn main() {
let mut patch = 3;

// See if --set-ver was used to set the version manually
let version = if let Some(ver) = matches
.get_one::<String>("set-ver")
.expect("matches definition")
{
let version = if let Some(ver) = matches.get_one::<String>("set-ver") {
ver.to_owned()
} else {
// Increment the one requested (in a real program, we'd reset the lower numbers)
Expand All @@ -74,22 +71,12 @@ fn main() {
if matches.is_present("config") {
let input = matches
.get_one::<PathBuf>("INPUT_FILE")
.expect("matches definition")
.unwrap_or_else(|| {
matches
.get_one::<PathBuf>("spec-in")
.expect("matches definition")
.unwrap()
})
.unwrap_or_else(|| matches.get_one::<PathBuf>("spec-in").unwrap())
.display();
println!(
"Doing work using input {} and config {}",
input,
matches
.get_one::<PathBuf>("config")
.expect("matches definition")
.unwrap()
.display()
matches.get_one::<PathBuf>("config").unwrap().display()
);
}
}

0 comments on commit eda0ca5

Please sign in to comment.