-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
chore: clap v3 #13266
chore: clap v3 #13266
Conversation
# Conflicts: # cli/Cargo.toml # cli/flags.rs
# Conflicts: # Cargo.lock # cli/Cargo.toml # cli/flags.rs
# Conflicts: # Cargo.lock # cli/Cargo.toml # cli/flags.rs
# Conflicts: # Cargo.lock # cli/flags.rs
# Conflicts: # Cargo.lock # cli/Cargo.toml # cli/flags.rs
# Conflicts: # Cargo.lock # cli/Cargo.toml # cli/flags.rs
Also closes #12118 |
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.
LGTM
commit message should probably be "upgrade: clap v3"
checksum = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" | ||
dependencies = [ | ||
"winapi 0.3.9", | ||
] |
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.
Nice!
DO NOT MERGE. discussed with @bartlomieju and we came to the conclusion that this PR should wait for 2.0, as there are some fundamental changes to clap's parsing, which would likely be a breaking change in various cases |
@crowlKats Can you give an example of what changed? |
I have tried to keep behaviour same as current, but i cant be certain that it actually fully is. one test was failing with a weird case and had to change what we were doing, but that change maybe would need to be applied to other subcommands, or maybe not. its a bit hairy. also, cargo is being wary of updating as well rust-lang/cargo#10253 (comment) (and subsequent comments) |
@crowlKats and I have done a little experimentation and decided that if this does introduce bugs, they are very not common, it works in all the instances we tried. I suggest landing. LGTM |
# Conflicts: # Cargo.lock
Closes #8475
Closes #12118