Avoid new deprecation warnings from clap 3.1.0 #10396
Conversation
|
r? @ehuss (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
cc @epage in case you want to double-check my fixes here. In particular the |
| @@ -281,7 +281,9 @@ pub fn multi_opt(name: &'static str, value_name: &'static str, help: &'static st | |||
| } | |||
|
|
|||
| pub fn subcommand(name: &'static str) -> App { | |||
| App::new(name).setting(AppSettings::DeriveDisplayOrder | AppSettings::DontCollapseArgsInUsage) | |||
| App::new(name) | |||
| .dont_collapse_args_in_usage(true) | |||
There was a problem hiding this comment.
FYI this became a global_setting.
If that is a concern, you can still use the AppSetting until clap v4. In clap v4, my expectation is we'll support subcommands overriding global settings (I think that doesn't work today)
When we made this global, it was under the assumption people would want a uniform look. If this assumption is incorrect, we can open an issue and re-visit this in clap v4.
There was a problem hiding this comment.
Huh, interesting. I honestly couldn't tell you whether that change is okay or not. I feel like it probably is since we're seemingly applying it to all subcommands, but others who know the CLI interface better may need to weigh in here.
|
Thanks for taking care of this! It was sitting in the back of my head as something that needed to be done. |
|
cc #10397 |
|
Brought over a few of the change flavors from #10397 👍 |
| let cmd = e | ||
| .context() | ||
| .find_map(|c| match c { | ||
| (ContextKind::InvalidSubcommand, &ContextValue::String(ref cmd)) => { | ||
| Some(cmd) | ||
| } | ||
| _ => None, | ||
| }) |
There was a problem hiding this comment.
Clap feature request: this would be better as:
let cmd = e.context::<clap::context::InvalidSubcommand>().expect(...);where the signature is:
impl Error {
fn context<Kind: ContextKind>(&self) -> Option<&Kind::ContextValue>;
}It seems clumsy that to access the piece of context you want requires iterating arbitrary other context kinds and having to assert that clap didn't stick your context in there with a type that it wasn't supposed to.
There was a problem hiding this comment.
Yeah, this code made me really sad to write.
The proposed interface may have to be able to deal with multiple values for a given kind, I'm not sure, but worth thinking about for sure 👍
There was a problem hiding this comment.
I started with the most general API, awaiting seeing how it was going to be used to know what would be helpful.
Since fn context() is already in use, we'll just have to come up with a different name for this.
|
The changes look good! |
|
@bors: r=epage |
|
📌 Commit 6cbf913 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Avoid new deprecation warnings from clap 3.1.0 ### What does this PR try to resolve? This fixes a number of new deprecations in [clap 3.1.0](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md#310---2022-02-16) that are currently causing CI to fail for all other PRs due to `-Dwarnings`.
Update cargo 8 commits in ea2a21c994ca1e4d4c49412827b3cf4dcb158b1d..d6cdde584a1f15ea086bae922e20fd27f7165431 2022-02-15 04:24:07 +0000 to 2022-02-22 19:55:51 +0000 - Add common profile validation. (rust-lang/cargo#10411) - Add -Z check-cfg-features to enable compile-time checking of features (rust-lang/cargo#10408) - Remove invalid target-specific dependency example. (rust-lang/cargo#10401) - Fix errors in `cargo fetch` usage guide (rust-lang/cargo#10398) - Fix some broken doc links. (rust-lang/cargo#10404) - Implement "artifact dependencies" (RFC-3028) (rust-lang/cargo#9992) - Print executable name on cargo test --no-run rust-lang/cargo#2 (rust-lang/cargo#10346) - Avoid new deprecation warnings from clap 3.1.0 (rust-lang/cargo#10396)
[beta] Beta backports * No branch protection metadata unless enabled rust-lang#93516 * update auto trait lint for PhantomData rust-lang#94315 * Cargo: * [1.60] Fix term.verbose without quiet, and vice versa (rust-lang/cargo#10436) * [beta] Add common profile validation. (rust-lang/cargo#10413) * Avoid new deprecation warnings from clap 3.1.0 (rust-lang/cargo#10396)
What does this PR try to resolve?
This fixes a number of new deprecations in clap 3.1.0 that are currently causing CI to fail for all other PRs due to
-Dwarnings.