ls: add -T support and fix --classify output #7616
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("aaaaaaaa"); | ||
| at.touch("bbbb"); | ||
| at.touch("cccc"); | ||
| 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: |
| @@ -4380,22 +4380,22 @@ fn test_tabsize_formatting() { | |||
|
|
|||
| 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: |
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