ls: match GNU's error for a bad --format/--sort/--time/etc. value - #14313
Closed
arbelonson-source wants to merge 1 commit into
Closed
ls: match GNU's error for a bad --format/--sort/--time/etc. value#14313arbelonson-source wants to merge 1 commit into
arbelonson-source wants to merge 1 commit into
Conversation
Eight options here validate an enumerated choice with a ShortcutValueParser: --format, --hyperlink, --quoting-style, --time, --sort, --color, --indicator-style, and --classify. An unrecognized or ambiguous value on any of them produces clap's own generic wording instead of GNU's grouped-alias one. Unlike the other options fixed in this series (uutils#14293, uutils#14303-uutils#14312), none of these are resolved by hand: each keeps its ShortcutValueParser completely unchanged, since that's still the source of truth clap uses both to validate the value and to render --format's/etc.'s own --help text. Instead, clap errors are now parsed through a small wrapper (get_matches) that, on an ErrorKind::InvalidValue naming one of these eight options, reconstructs GNU's wording from a metadata table (CHOICE_OPTIONS) describing each option's own choice groups in GNU's display order; every other clap error -- unknown flags, --help, --version, etc. -- still goes through the exact same shared formatter as before, unchanged. Two of these options have their own uutils-only choice not present in GNU's real --format/--sort at all: --format=columns (GNU's own 'vertical' choice has no separate word for it) is listed as its own group, same as du's 'creation'/'birth' precedent (uutils#14306) -- --quoting-style is unusual in the other direction: GNU lists each of its choices, including 'c' and 'c-maybe' (a real alias pair), on its own separate line rather than grouping aliases together the way every other option in this series does. AI-assisted-by: Claude Opus 5, via Claude Code
|
GNU testsuite comparison: |
Contributor
|
We don't need to show all equivalent aliases on help. Such big code is really no worth to maintain. Please close this PR. |
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.
What
Eight options in
lsvalidate an enumerated choice with aShortcutValueParser:--format,--hyperlink,--quoting-style,--time,--sort,--color,--indicator-style, and--classify.An unrecognized or ambiguous value on any of them produces clap's own
generic wording instead of GNU's grouped-alias one, e.g.:
Fix
Unlike the other options fixed in this series (#14293, #14303-#14312),
none of these eight are resolved by hand: each keeps its
ShortcutValueParsercompletely unchanged, since that's still thesource of truth clap uses both to validate the value and to render
e.g.
--quoting-style's own--helptext (several of these alreadycarry real synonym handling clap needs to keep doing correctly).
Instead, clap errors now go through a small wrapper (
get_matches)that, on an
ErrorKind::InvalidValuenaming one of these eightoptions, reconstructs GNU's wording from a small metadata table
(
CHOICE_OPTIONS) describing each option's own choice groups in GNU'sdisplay order. Every other clap error -- unknown flags,
--help,--version, etc. -- still goes through the exact same sharedformatter as before, unchanged. This mirrors the approach already
used for
tee --output-error(#14308), which has the samehelp-text constraint.
Two things specific to these eight, found only by diffing exhaustively
against real GNU rather than just checking one bad value per option:
--format=columnsis this implementation's own choice; GNU's real--formathas no separate word for what it callsverticalanddoes not accept
columnsat all. Listed as its own group, sameprecedent as du's
creation/birth(du: match GNU's error for a bad --time value, fix --time-style's help hint #14306) -- and, as with thatone, it creates a real ambiguity GNU doesn't have (e.g.
--format=coresolves tocommason GNU, but is ambiguous herebetween
commasandcolumns).--quoting-styleis unusual in the other direction: GNU listseach of its choices, including
candc-maybe-- a real aliaspair -- on its own separate line, rather than grouping aliases
together the way every other option in this series does.
Testing
cargo test -p uu_ls/ fulltests/by-util/test_ls.rssuite: 189 passed, 0 failed, 1 pre-existing ignore.test_localized_possible_valuestest (which asserted clap's own wording, localized, as the expected behavior) to assert GNU's wording instead, in both English and French.--classify=WHENparsing itself still works.LC_ALL=C(including cases specific to each option's own alias structure, e.g.--time=aunambiguous vs--format=vambiguous).-la,--color=auto,--time-style=...),--help,-h,--version, and an unrelated error (unknown flag, still exit code 2) are all unaffected.cargo clippy -p uu_ls --all-targets -- -D warningsandcargo fmt --check: clean.This PR was written with AI assistance (Claude Opus 5, via Claude Code). I've tested the changes but please review the code carefully.