Skip to content

ls: match GNU's error for a bad --format/--sort/--time/etc. value - #14313

Closed
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/ls-choice-options-invalid-value
Closed

ls: match GNU's error for a bad --format/--sort/--time/etc. value#14313
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/ls-choice-options-invalid-value

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

What

Eight options in ls 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, e.g.:

$ ls --time=bogus

# GNU
ls: invalid argument 'bogus' for '--time'
Valid arguments are:
  - 'atime', 'access', 'use'
  - 'ctime', 'status'
  - 'mtime', 'modification'
  - 'birth', 'creation'
Try 'ls --help' for more information.

# uutils, before this PR
error: invalid value 'bogus' for '--time <field>'

  [possible values: atime, ctime, mtime, birth]

For more information, try '--help'.

Fix

Unlike the other options fixed in this series (#14293, #14303-#14312),
none of these eight 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
e.g. --quoting-style's own --help text (several of these already
carry real synonym handling clap needs to keep doing correctly).

Instead, clap errors now go through a small wrapper (get_matches)
that, on an ErrorKind::InvalidValue naming one of these eight
options, reconstructs GNU's wording from a small 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. This mirrors the approach already
used for tee --output-error (#14308), which has the same
help-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=columns is this implementation's own choice; GNU's real
    --format has no separate word for what it calls vertical and
    does not accept columns at all. Listed as its own group, same
    precedent 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 that
    one, it creates a real ambiguity GNU doesn't have (e.g.
    --format=co resolves to commas on GNU, but is ambiguous here
    between commas and columns).
  • --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.

Testing

  • cargo test -p uu_ls / full tests/by-util/test_ls.rs suite: 189 passed, 0 failed, 1 pre-existing ignore.
  • Updated the existing test_localized_possible_values test (which asserted clap's own wording, localized, as the expected behavior) to assert GNU's wording instead, in both English and French.
  • Added 7 new regression tests covering each of the three message shapes (flat, grouped, one choice per line) and confirming --classify=WHEN parsing itself still works.
  • Manually diffed every one of the 8 options across invalid, ambiguous, empty, and valid-abbreviation values against GNU ls 9.11 under LC_ALL=C (including cases specific to each option's own alias structure, e.g. --time=a unambiguous vs --format=v ambiguous).
  • Manually confirmed ordinary usage (-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 warnings and cargo 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.

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
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

@oech3

oech3 commented Aug 31, 2026

Copy link
Copy Markdown
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.

@sylvestre sylvestre closed this Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants