Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: Accept shortcuts for stringly-enum options #6174

Merged

Conversation

BenWiederhake
Copy link
Collaborator

We already infer long arguments, like wc --tot being equivalent to wc --total. This PR introduces the same functionality for values, just like the GNU tools. In particular, this means the following calls are equivalent:

ls -l --time=a
ls -l --time=atime
ls -l --time=access

This example also demonstrates that alias resolution does not interfere with value inference: "a" is lexically ambiguous (it might mean "atime" or "access"), but has only one meaning (because "atime" and "access" are the same thing).

The implementation to do that was already there, but was only used in date(1) for some reason.

This PR is completely different from the following PRs:

@BenWiederhake BenWiederhake marked this pull request as draft April 1, 2024 18:24
@BenWiederhake
Copy link
Collaborator Author

(Whoops, sorry for the review request, that was too early.)

@BenWiederhake BenWiederhake marked this pull request as ready for review April 1, 2024 19:39
@BenWiederhake BenWiederhake force-pushed the dev-all-partial-stringly-options branch from e28d7be to 2cc5ec6 Compare April 1, 2024 19:40
@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • Actually added the implementation for wc. Somehow I forgot to commit it, I'm a silly goose.

Copy link

github-actions bot commented Apr 1, 2024

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

@BenWiederhake BenWiederhake force-pushed the dev-all-partial-stringly-options branch from 2cc5ec6 to 91679fc Compare April 14, 2024 13:39
@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • Rebased to current main, to show that it still works

Is there anything else needed?

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/timeout/timeout is no longer failing!

@sylvestre
Copy link
Contributor

LGTM but I would like @tertsdiepraam to comment as he often has great opinions on such subjects

@@ -66,7 +66,7 @@ impl TypedValueParser for ShortcutValueParser {
let matched_values: Vec<_> = self
.0
.iter()
.filter(|x| x.get_name().starts_with(value))
.filter(|x| x.get_name_and_aliases().any(|name| name.starts_with(value)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clever and I love it :)

@tertsdiepraam
Copy link
Member

Excellent! Let's get this merged indeed!

@tertsdiepraam tertsdiepraam merged commit 652c65f into uutils:main Apr 14, 2024
64 checks passed
@BenWiederhake BenWiederhake deleted the dev-all-partial-stringly-options branch April 14, 2024 20:21
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