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

ls: Implement --zero flag. (#2929) #3746

Merged
merged 4 commits into from
Jul 31, 2022
Merged

Conversation

pimzero
Copy link
Contributor

@pimzero pimzero commented Jul 26, 2022

Fixes #2929

(Full disclosure, I am very new to rust, and contributing mainly as a learning exercise. But I hope this change can be useful).

This flag can be used to provide a easy machine parseable output from
ls, as discussed in the GNU bug report
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49716.

There are some peculiarities with this flag:

  • Current implementation of GNU ls of the --zero flag implies some
    other flags. Those can be overridden by setting those flags after
    --zero in the command line.
  • This flag is not compatible with --dired. This patch is not 100%
    compliant with GNU ls: GNU ls --zero will fail if --dired and
    -l are set, while with this patch only --dired is needed for the
    command to fail.

We also add --dired flag to the parser, with no additional behaviour
change.

Testing done:

$ bash util/build-gnu.sh
 [...]
$ bash util/run-gnu-test.sh tests/ls/zero-option.sh
 [...]
 PASS: tests/ls/zero-option.sh
 ============================================================================
 Testsuite summary for GNU coreutils 9.1.36-8ec11
 ============================================================================
 # TOTAL: 1
 # PASS:  1
 # SKIP:  0
 # XFAIL: 0
 # FAIL:  0
 # XPASS: 0
 # ERROR: 0
 ============================================================================

@sylvestre
Copy link
Sponsor Contributor

Cool for a first contrib.
Fails on Windows with:


---- test_ls::test_ls_zero stdout ----
current_directory_resolved: 
mkdir: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpVlGog3\0-test-zero
touch: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpVlGog3\1
test-zero
thread 'test_ls::test_ls_zero' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 123, kind: Uncategorized, message: "The filename, directory name, or volume label syntax is incorrect." }', tests\common\util.rs:656:40

@pimzero
Copy link
Contributor Author

pimzero commented Jul 26, 2022

Thanks, I suppose this issue is caused by Windows filesystem not supporting newline in filenames, I updated the test to only run with the \n in filenames in unix. Also I fixed the lint & spelling issues the CI found

@sylvestre
Copy link
Sponsor Contributor

Well done:
Warning: Congrats! The gnu test tests/ls/zero-option is no longer failing!

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

Cool! Nice work on all the edge cases.

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved
src/uu/ls/src/ls.rs Show resolved Hide resolved
pimzero and others added 4 commits July 30, 2022 15:39
This flag can be used to provide a easy machine parseable output from
ls, as discussed in the GNU bug report
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49716.

There are some peculiarities with this flag:

 - Current implementation of GNU ls of the `--zero` flag implies some
   other flags. Those can be overridden by setting those flags after
   `--zero` in the command line.
 - This flag is not compatible with `--dired`. This patch is not 100%
   compliant with GNU ls: GNU ls `--zero` will fail if `--dired` and
   `-l` are set, while with this patch only `--dired` is needed for the
   command to fail.

We also add `--dired` flag to the parser, with no additional behaviour
change.

Testing done:
```
$ bash util/build-gnu.sh
 [...]
$ bash util/run-gnu-test.sh tests/ls/zero-option.sh
 [...]
 PASS: tests/ls/zero-option.sh
 ============================================================================
 Testsuite summary for GNU coreutils 9.1.36-8ec11
 ============================================================================
 # TOTAL: 1
 # PASS:  1
 # SKIP:  0
 # XFAIL: 0
 # FAIL:  0
 # XPASS: 0
 # ERROR: 0
 ============================================================================
```
Also, allow multiple --zero flags, as this is possible with GNU ls
command. Only the last one is taken into account.
@sylvestre sylvestre merged commit 4ad0d5c into uutils:main Jul 31, 2022
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.

ls: add support for --zero option
3 participants