-
Notifications
You must be signed in to change notification settings - Fork 319
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
Bump clap to version 4 #679
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me! The changes are mostly mechanical. Just one question.
src/lib.rs
Outdated
if let Some(color) = matches.get_one("color").map(String::as_str) { | ||
if color != "auto" { | ||
eprintln!("Warning: --color will be ignored when running with cargo-criterion. Use `cargo criterion --color {} -- <args>` instead.", color); | ||
} | ||
} | ||
if matches.is_present("verbose") { | ||
if matches.get_flag("verbose") { | ||
eprintln!("Warning: --verbose will be ignored when running with cargo-criterion. Use `cargo criterion --output-format verbose -- <args>` instead."); | ||
} | ||
if matches.is_present("noplot") { | ||
if matches.get_flag("noplot") { | ||
eprintln!("Warning: --noplot will be ignored when running with cargo-criterion. Use `cargo criterion --plotting-backend disabled -- <args>` instead."); | ||
} | ||
if let Some(backend) = matches.value_of("plotting-backend") { | ||
if let Some(backend) = matches.get_one::<String>("plotting-backend") { | ||
eprintln!("Warning: --plotting-backend will be ignored when running with cargo-criterion. Use `cargo criterion --plotting-backend {} -- <args>` instead.", backend); | ||
} | ||
if let Some(format) = matches.value_of("output-format") { | ||
if let Some(format) = matches.get_one::<String>("output-format") { | ||
if format != "criterion" { | ||
eprintln!("Warning: --output-format will be ignored when running with cargo-criterion. Use `cargo criterion --output-format {} -- <args>` instead.", format); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is there a reason for the different ways in which get_one
is called here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When used in an if
statement where the type needs to be comparable to a &str
, it ought to be
if let Some(…) = matches.get_one::<String>("…") {
When used in an if
statement where the type needs to be exactly a &str
, it ought to be
match matches.get_one("…").map(String::as_str) {
I've fixed one case on line 930. Thanks for catching that style nit!
src/lib.rs
Outdated
@@ -920,50 +927,51 @@ https://bheisler.github.io/criterion.rs/book/faq.html | |||
.get_matches(); | |||
|
|||
if self.connection.is_some() { | |||
if let Some(color) = matches.value_of("color") { | |||
if let Some(color) = matches.get_one("color").map(String::as_str) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunshowers This line was the odd one out.
I've changed
if let Some(color) = matches.get_one("color").map(String::as_str) {
to
if let Some(color) = matches.get_one::<String>("color") {
Thank you for merging! |
I see that the last stable release was version 0.4.0 on 2022-09-10. No rush, but might there be a stable release on the horizon that includes this change? |
@lemmih Thank you for the releases, David! |
Bump
clap
to version 4.Bump MSRV to 1.64 for
clap
.value_parser
for numeric value typespossible_values
tovalue_parser
hidden(true)
tohide(true)
value_of
andvalue_of_t_or_exit
toget_one
is_present
toget_flag
orcontains_id
takes_value(true)
, sincenum_args(1)
is the defaultnum_args(0)
where neededSee https://github.com/clap-rs/clap/releases/tag/v3.2.0