Skip to content

touch: match GNU's error for a bad --time value - #14311

Closed
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/touch-time-invalid-choice
Closed

touch: match GNU's error for a bad --time value#14311
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/touch-time-invalid-choice

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

What

--time validates its value with a plain ShortcutValueParser, so an
unrecognized or ambiguous value produces clap's own generic wording
instead of GNU's grouped-alias wording:

$ touch --time=bogus f

# GNU
touch: invalid argument 'bogus' for '--time'
Valid arguments are:
  - 'atime', 'access', 'use'
  - 'mtime', 'modify'
Try 'touch --help' for more information.

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

  [possible values: atime, mtime]

For more information, try '--help'.

Fix

Same fix already shipped for the same option name on other utilities,
and for other options generally, across this series (#14293, #14303-
#14310): resolve the value against the option's own alias groups by
hand, accepting any unambiguous abbreviation the way GNU does --
including one ambiguous only between aliases of the same choice,
e.g. a among atime/access/use -- and report GNU's wording when
it does not resolve.

determine_atime_mtime_change now returns a UResult<ChangeTimes>
instead of a bare ChangeTimes to propagate that; the existing unit
test for it (test_determine_atime_mtime_change) is updated to
.unwrap() accordingly.

Testing

  • cargo test -p uu_touch (lib + integration): 6 + 65 passed, 0 failed.
  • Added a regression test for the exact GNU wording on an invalid value.
  • Manually diffed every --time value (all real choices/aliases/abbreviations, invalid, ambiguous, empty) against GNU touch 9.11 under LC_ALL=C.
  • Manually confirmed --time=atime/--time=mtime still update the correct timestamp (not just the error path).
  • cargo clippy -p uu_touch --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.

--time validated its value with a plain ShortcutValueParser, so an
unrecognized or ambiguous value produced clap's own generic wording
instead of GNU's grouped-alias wording ('invalid argument ... for
--time / Valid arguments are: / - 'atime', 'access', 'use' / ...').

Same fix already shipped for the same option name on wc, date, du,
sort, uniq, shred, tee, rm (uutils#14293, uutils#14303-uutils#14310): resolve the value
against the option's own alias groups by hand, accepting any
unambiguous abbreviation the way GNU does -- including one ambiguous
only between aliases of the same choice, e.g. 'a' among
'atime'/'access'/'use' -- and report GNU's wording when it does not
resolve. determine_atime_mtime_change now returns a UResult to
propagate that.

AI-assisted-by: Claude Opus 5, via Claude Code
@oech3

oech3 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

We don't need to cover all equivalent aliases in the error message since there is

For more information, try '--help'.

@arbelonson-source Would you stop aligning error message with GNU if it does not cause actual misleading?

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

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)

@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