ls: add -T support and fix --classify output#7616
ls: add -T support and fix --classify output#7616tertsdiepraam merged 13 commits intouutils:mainfrom
Conversation
|
test_ls_columns was updated with \t since this behavior is on by default |
|
GNU testsuite comparison: |
tests/by-util/test_ls.rs
Outdated
| at.touch("dddddddd"); | ||
|
|
||
| ucmd.args(&["-T", "4"]) | ||
| // Need additional options to simulate columns output. |
There was a problem hiding this comment.
Could you explain this comment? Is it not supported right now by the options we have in clap?
There was a problem hiding this comment.
If I'm not specifically telling to print out the grid ls output gets piped and returns entries on new lines. Or am I wrong here?
There was a problem hiding this comment.
By default yes, but you should be able to force it to column layout with -C, I believe. Some of the tests you've adapted above use that as well.
There was a problem hiding this comment.
And could you remove the comment now? It would also be fine to have each of these tests for -C and -x by the way. More tests is (almost) always better!
| } else { | ||
| Filling::Spaces(2) | ||
| let filling = match tab_size { | ||
| 0 => Filling::Spaces(DEFAULT_SEPARATOR_SIZE), |
There was a problem hiding this comment.
I guess you left this in as an optimization?
There was a problem hiding this comment.
Yes, since -T0 literally means no \t I thought it's gonna be a good idea to add this small check here to omit additional work in grid
There was a problem hiding this comment.
Could you add that as a comment to the code?
|
GNU testsuite comparison: |
| scene | ||
| .ucmd() | ||
| .args(&["-x", "-w18", "-T4"]) | ||
| .args(&["-C", "-w18", "-T4"]) |
There was a problem hiding this comment.
Maybe we should test both -x and -C? The logic is the same in uutils-term-grid I guess, but we shouldn't assume too much about that here.
|
Is it something my test change has broken? Or is it something in CI? |
|
GNU testsuite comparison: |
1 similar comment
|
GNU testsuite comparison: |
since this behavior is on by default
|
GNU testsuite comparison: |
* add -T option parsing * add usage of tab_size in display_grid * fix test_ls_columns with \t since this behavior is on by default * update test_tabsize_formatting * use grid DEFAULT_SEPARATOR_SIZE * update Tabs * fix test with column width * fix cspell * fix linter warning on match bool * add comment for 0 tab_size * update tabsize test with -C * update one of the tabs tests to use -x * remove comment and split tests for both x/C args
This update relies on MR46 in
uutils-term-gridcrate.This update uses new
Filling::Tabsoption instead of explicitly setting text separator with \t.Closes #6897
Closes #7526
Closes #3624