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

comm: Handle duplicated flags and output-delimiter correctly #6112

Merged
merged 4 commits into from
Mar 24, 2024

Conversation

BenWiederhake
Copy link
Collaborator

@BenWiederhake BenWiederhake commented Mar 23, 2024

This PR implements and tests correct handling of repeated arguments and flags.

This was non-trivial because GNU comm behaves a bit weirdly around repeated arguments for the output-delimiter:

$ comm -123 --out=X --out=X a b  # Succeeds
$ comm -123 --out=X --out=Y a b  # Fails
comm: multiple output delimiters specified
[$? = 1]
$

This PR mostly mirrors this behavior, however it cannot be done perfectly so, as indicated by the new (and ignored) test output_delimiter_multiple_different_prevents_help: Preventing --help later in the command-line because some arguments are semantically contradictory seems like a bad idea.

Note that this quirk cannot be reproduced (sensibly) with uutils-args, because Options::apply cannot indicate failure.

However, I think this difference is firmly in the territory of "irrelevant for compatibility".

This is work towards #5998.

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • Just rebased on current main. Looks like I created the PR while the repo was in the middle of a release cycle, which caused some weird inconsistent version numbers.

Copy link

GNU testsuite comparison:

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

Copy link

GNU testsuite comparison:

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

@cakebaker cakebaker merged commit 6f07bf1 into uutils:main Mar 24, 2024
62 checks passed
@cakebaker
Copy link
Contributor

Good work, thanks :)

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.

2 participants