shadow-core, tools: de-duplicate the clap-error boilerplate (#181) - #190
Merged
Conversation
Every tool's uumain carried the same 10-line clap-error match plus a per-enum AlreadyPrinted(i32) sentinel variant with identical Display and code() arms. Hoist all of it into shadow-core: - shadow_core::cli::AlreadyPrinted(i32): shared UError whose Display is empty (the message was already printed), carrying only the exit code. - shadow_core::cli::parse_args(cmd, args, code): parses argv, prints the clap error, returns Ok(None) for --help/--version, and maps real parse failures through the tool's code function. Each of the 12 tools now opens with a single let-else line; the AlreadyPrinted variant and its Display/code() arms are deleted from all 12 error enums, and non-clap uses of the sentinel switch to the shared type. grpck/pwck (which use plain '?') are unchanged. Behavior is preserved and verified: --help/--version exit 0 with identical output; bad flags exit 1 (chfn/chpasswd/chsh/newgrp), 2 (chage/group*/user*), and passwd keeps its kind-dependent 2/6 mapping. useradd's clap-error match had two arms both returning 2; it collapses to the constant. userdel's remaining variants all share the Cant prefix, tripping clippy::enum_variant_names; allowed with a comment since the names mirror userdel(8)'s documented exit-code names. Closes #181.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes the previously copy‑pasted clap parse-error handling used across the shadow-rs tool crates into shadow_core::cli, replacing each tool’s local AlreadyPrinted(i32) sentinel and try_get_matches_from boilerplate with a shared parse_args helper while preserving exit-code behavior.
Changes:
- Added
shadow_core::cli::{AlreadyPrinted, parse_args}(with unit tests) to standardize clap error printing and exit-code mapping. - Updated the affected tools to use
parse_args(...)and the sharedAlreadyPrinted(...)instead of per-tool sentinel variants. - Added
clapanduucoreasshadow-coredependencies to support the shared CLI plumbing.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/uu/usermod/src/usermod.rs | Replaces local clap error boilerplate and sentinel with shadow_core::cli::parse_args / shared AlreadyPrinted. |
| src/uu/userdel/src/userdel.rs | Same refactor; also adds a clippy allow explaining intentional Cant* variant naming. |
| src/uu/useradd/src/useradd.rs | Collapses clap parsing boilerplate into parse_args and switches non-clap sentinel uses to shared AlreadyPrinted. |
| src/uu/passwd/src/passwd.rs | Uses parse_args with per-error-kind exit code mapping (2 vs 6), preserving GNU passwd behavior. |
| src/uu/newgrp/src/newgrp.rs | Replaces local clap boilerplate with shared parse_args and removes per-tool sentinel variant. |
| src/uu/groupmod/src/groupmod.rs | Replaces local clap boilerplate and sentinel with shared parse_args / AlreadyPrinted. |
| src/uu/groupdel/src/groupdel.rs | Replaces local clap boilerplate and sentinel with shared parse_args / AlreadyPrinted. |
| src/uu/groupadd/src/groupadd.rs | Replaces local clap boilerplate and sentinel with shared parse_args / AlreadyPrinted. |
| src/uu/chsh/src/chsh.rs | Replaces local clap boilerplate and sentinel with shared parse_args / AlreadyPrinted. |
| src/uu/chpasswd/src/chpasswd.rs | Replaces local clap boilerplate and sentinel; updates unit tests to assert shared AlreadyPrinted behavior. |
| src/uu/chfn/src/chfn.rs | Replaces local clap boilerplate and sentinel with shared parse_args / AlreadyPrinted. |
| src/uu/chage/src/chage.rs | Replaces local clap boilerplate and sentinel; updates unit tests to assert shared AlreadyPrinted behavior. |
| src/shadow-core/src/cli.rs | Adds shared AlreadyPrinted and parse_args plus unit tests; expands module docs to cover shared CLI plumbing. |
| src/shadow-core/Cargo.toml | Adds clap and uucore dependencies needed by the new shared CLI utilities. |
| Cargo.lock | Records the new shadow-core dependency edges on clap and uucore. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #181: hoists the copy-pasted clap-error handling out of all 12 tool crates into
shadow-core.New shared plumbing (
shadow_core::cli, unit-tested):AlreadyPrinted(i32)— sharedUErrorwhoseDisplayis empty (message already printed); carries only the exit code.parse_args(cmd, args, code)— parses argv, prints the clap error, returnsOk(None)for--help/--version(exit 0), and maps real parse failures through the tool'scodefunction.Per tool (chage, chfn, chpasswd, chsh, groupadd, groupdel, groupmod, newgrp, passwd, useradd, userdel, usermod):
try_get_matches_frommatch collapses to onelet Some(matches) = parse_args(...)? else { return Ok(()); }.AlreadyPrinted(i32)variant and its identicalDisplay/code()arms are deleted from every enum; non-clap uses of the sentinel switch to the shared type.grpck/pwck(plain?) are unchanged. Net: −180/+153 lines with 12 enums simplified.Behavior preserved (verified, not assumed)
Ran the built binaries before/after:
passwd --help/--versionpasswd (uutils shadow-rs) 0.2.0)passwd --boguspasswd -l -u(conflict)chpasswd --bogusgroupadd --bogus/ missing argNotes:
useradd's clap match had two arms that both returned 2 — collapsed to the constant (no behavior change).userdel's remaining variants all share theCantprefix, trippingclippy::enum_variant_names; allowed with a comment since the names mirroruserdel(8)'s documented exit-code names.Test plan
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace— green on Debiancargo test --workspace— green on Alpine (musl) and Fedora (59 passing suites each; matrix is push-gated on PRs)AlreadyPrinted(empty Display, code passthrough) andparse_args(success / error mapping /--version→Ok(None))Closes #181.