8000 df: -h -H shouldn't cause an error #3366 by gmnsii · Pull Request #3414 · uutils/coreutils · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

df: -h -H shouldn't cause an error #3366 #3414

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 18, 2022
Merged

df: -h -H shouldn't cause an error #3366 #3414

merged 2 commits into from
Apr 18, 2022

Conversation

gmnsii
Copy link
Contributor
@gmnsii gmnsii commented Apr 16, 2022 •
edited
Loading

Solve issue #3366. Make -h and -H override each other (the latest take the priority)

< 8000 /include-fragment>
@tertsdiepraam
Copy link
Member

Looks good, but could you maybe extend this to all arguments? Then we don't have to go back and fix those later. All arguments should override themselves and the arguments they now conflict with. For example,

df --sync --sync

should work and also

df --block-size=3K -k

should override each other.

@tertsdiepraam tertsdiepraam linked an issue Apr 17, 2022 that may be closed by this pull request
df: -h -H shouldn't cause an error #3366
Closed
@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 17, 2022 •
edited
Loading

No problem. By the way, df --block-size=3Kdoesn't seem to work (It work on GNU df)

$ ./target/debug/coreutils df --block-size=3K     
df: invalid --block-size argument

@tertsdiepraam
Copy link
Member

Thanks for fixing this! I'll open an issue for the --block-size parsing :)

@tertsdiepraam tertsdiepraam mentioned this pull request Apr 17, 2022
Closed
@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 17, 2022

I finished. --output still conflicts with -i, -P and -T because it is the same in the GNU version

@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 17, 2022 •
edited
Loading

But if we want all utils to work exactly like their GNU couterpart we need to do this with all of them:

GNU:

$ ls -i -i -i
786491 bin   921616 etc   786937 media	     1 proc  786947 sbin  921373 tmp
786560 boot  786730 home  786938 mnt	786941 root  787009 srv   921417 usr
     1 dev   921413 lib   786939 opt	786944 run        1 sys   921364 var

uutils:

$ ./target/debug/coreutils ls -i -i -i
error: The argument '--inode' was provided more than once, but cannot be used multiple times

USAGE:
    ./target/debug/coreutils ls [OPTION]... [FILE]...

For more information try --help

@sylvestre
Copy link
Contributor

not a big deal but please try to avoid screenshots: search isn't able to parse the content :)

@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 17, 2022

Sorry about that

@sylvestre
Copy link
Contributor

don't be, it isn't a big deal :)

@tertsdiepraam
Copy link
Member

But if we want all utils to work exactly like their GNU couterpart we need to do this with all of them

Yep, it's an ongoing effort :) Feel free to do this for more utils if you want!

@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 17, 2022 •
edited
Loading

A test for sort is failing the CI, which is weird since I didn't touch anything related to it.

failures:
    test_sort::test_numeric_floats_and_ints

test result: FAILED. 1661 passed; 1 failed; 28 ignored; 0 measured; 0 filtered out; finished in 407.86s

---- test_sort::test_numeric_floats_and_ints stderr ----
thread 'main' panicked at 'Command was expected to succeed.
stdout = 0
.02

 stderr = ', tests\common\util.rs:174:9

@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 18, 2022

CI is now passing

@tertsdiepraam
Copy link
Member

Almost ready to merge, but there's a chance one of the tests (test_df_output_overridden) might fail due to another PR I just merged. I'll rerun the checks to be sure.

@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 18, 2022

Ok

@tertsdiepraam
Copy link
Member

CI is now passing

Yeah, some tests are flaky. We reran the failed checks.

@gmnsii
Copy link
Contributor Author
gmnsii commented Apr 18, 2022

The FreeBSD testsuite failed while installing packages

[5/8] Installing gettext-runtime-0.21...
[5/8] Extracting gettext-runtime-0.21: .........Connection to localhost closed by remote host.
Error: The process '/usr/bin/ssh' failed with exit code 255

@sylvestre sylvestre merged commit d7cf3e7 into uutils:main Apr 18, 2022
@sylvestre
Copy link
Contributor

not your fault, thanks

@sylvestre
Copy link
Contributor

Cool, seems that it fixed a GNU test:

Congrats! The gnu test tests/df/df-symlink is no longer failing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

df: -h -H shouldn't cause an error
3 participants
@gmnsii @tertsdiepraam @sylvestre
0