Upgrade the clap CLI parsing crate to 4.5 #22088
Conversation
benjyw
added
the
release-notes:not-required
|
This was largely mechanical but also somewhat annoying to write and presumably to review. Sorry! |
| @@ -6,7 +6,7 @@ authors = ["Pants Build <[email protected]>"] | |||
| publish = false | |||
|
|
|||
| [dependencies] | |||
| clap = { workspace = true } | |||
| clap = { workspace = true, features = ["string"] } | |||
There was a problem hiding this comment.
The "string" feature allows default_value to take dynamic strings, which we use in a couple of places. Otherwise they must be &'static str...
| .long("local-store-path") | ||
| .default_value_os(default_store_path.as_ref()) | ||
| .default_value(Store::default_path().to_str().unwrap().to_string()) |
There was a problem hiding this comment.
This is an example of a dynamic, owned default.
| .required(false), | ||
| ).arg( | ||
| Arg::new("server-address") | ||
| .takes_value(true) | ||
| .num_args(1) |
There was a problem hiding this comment.
| .long("batch-api-size-limit") | ||
| .required(false) | ||
| .default_value("4194304") | ||
| ) | ||
| .get_matches(); | ||
|
|
||
| let mount_path = args.value_of("mount-path").unwrap(); | ||
| let store_path = args.value_of("local-store-path").unwrap(); | ||
| let mount_path = args.get_one::<String>("mount-path").unwrap(); |
There was a problem hiding this comment.
| tls_config, | ||
| headers, | ||
| chunk_size_bytes: 4 * 1024 * 1024, | ||
| timeout: std::time::Duration::from_secs(5 * 60), | ||
| retries: 1, | ||
| concurrency_limit: args | ||
| .value_of_t::<usize>("rpc-concurrency-limit") | ||
| .expect("Bad rpc-concurrency-limit flag"), |
There was a problem hiding this comment.
These .expects are no longer needed - get_one() will panic appropriately already, with more information than this message provides...
| @@ -86,8 +86,12 @@ async fn main() { | |||
| "Ingest a file by path, which allows it to be used in Directories/Snapshots. \ | |||
| Outputs a fingerprint of its contents and its size in bytes, separated by a space.", | |||
| ) | |||
| .arg(Arg::new("path").required(true).takes_value(true)) | |||
| .arg(Arg::new("output-mode").long("output-mode").possible_values(["json", "simple"]).default_value("simple").multiple_occurrences(false).takes_value(true).help( | |||
There was a problem hiding this comment.
multiple_occurrences is the default (or rather, it corresponds to ArgAction::Set, which is the default).
|
Note that we use both the programmatic and declarative APIs, in different |
Looks good to me. Thanks for doing this! |
We were stuck on v3 for a long time, and were using
deprecatedAPIs removed in v4.
This allows us to finally upgrade the crate and hopefully
stay current with it going forward.