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

fix tr with delete flag if more than 1 operand given #5945

Merged
merged 5 commits into from
Feb 6, 2024

Conversation

BaherSalama
Copy link
Contributor

fixes the issue #5944 where tr -d flag should not take more than one operand
so its like this now

❯ echo "acsd5c"| ./target/debug/tr -d "c"
asd5

~/sad/coreutils main
❯ echo "acsd5c"| ./target/debug/tr -d "c" "p"
./target/debug/tr: extra operand 'p'
Only one string may be given when deleting without squeezing repeats.
Try './target/debug/tr --help' for more information.

problem tho is that adding more arguments errors out tr when it should just display the same massage

where gnu tr

❯ echo "acsd5c"| tr -d "c" "p" "m"
tr: extra operand ‘p’
Try 'tr --help' for more information.

coreutils one

❯ echo "acsd5c"| ./target/debug/tr -d "c" "p" "m"
error: unexpected value 'm' for '[sets]...' found; no more were expected

Usage: ./target/debug/tr [OPTION]... SET1 [SET2]

For more information, try '--help'.

not sure this is related to #5944 since i could not grep for the error expect in whom test so don't know where to look for that

@cakebaker cakebaker changed the title fix tr with delete flag if more than 1 oprerand given fix tr with delete flag if more than 1 operand given Feb 5, 2024
@cakebaker cakebaker linked an issue Feb 5, 2024 that may be closed by this pull request
@sylvestre
Copy link
Sponsor Contributor

looks good but could you please add a test to make sure we don't regress in the future ?
thanks

@BaherSalama
Copy link
Contributor Author

added a test for 2 operands with -d flag

@cakebaker cakebaker merged commit 5c2ae5b into uutils:main Feb 6, 2024
56 of 59 checks passed
@cakebaker
Copy link
Contributor

Thanks for your PR :)

ysthakur pushed a commit to ysthakur/coreutils that referenced this pull request Feb 27, 2024
* fix tr

* fix

* adding a test

* tr: rename test function

---------

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>
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.

tr -d should reject a second argument
3 participants