Use clap builder instead of derive, restructure a bit to look like cargo #772
Conversation
cassaundra
force-pushed
the
merge-rm-cli
branch
from
ea840e9
to
13c2494
Compare
| let dev = args.contains_id("dev"); | ||
| let build = args.contains_id("build"); | ||
| let target = args.get_one("target").cloned(); | ||
| let manifest_path = args.get_one("manifest_path").cloned(); | ||
| let pkg_id = args.get_one("pkg_id"); | ||
| let dry_run = args.contains_id("dry_run"); | ||
| let quiet = args.contains_id("quiet"); |
There was a problem hiding this comment.
Please either use is_present or update the flags to ArgAction::SetTrue. Part of the intention of renaming is_present to contains_id is so that people can easily identify presence-check calls to update when porting from the old behavior to the new behavior
crates/cargo-rm/tests/testsuite/cargo_rm/invalid_arg/stderr.log
Outdated
Show resolved
Hide resolved
cassaundra
force-pushed
the
merge-rm-cli
branch
from
13c2494
to
4372325
Compare
|
I was going to push these fixes in a separate commit, but since they were so minor I just amended them to the last. Here are the latest changes as requested:
|
Since we already have a value name DEP_ID, we should use a similar convention for PKG_ID.
This is expected. iirc cargo has a help for it. The builder API is lower level than the derive, so I made the trade off for accessing bools to be clunky in favor of making them consistent with how everything else operates, letting them participate in all other clap features. |
This PR converts rm's arg parsing to clap builder with minimal changes to functionality. After this is merged, I will start working on using more cargo internals in the argument parsing, which will have more noticeable effects.