ls: ignore leading period when sorting by name #2112
Conversation
|
Every check except |
src/uu/ls/src/ls.rs
Outdated
| Sort::Name => entries.sort_by_key(|k| k.file_name.to_lowercase()), | ||
| Sort::Name => entries.sort_by_key(|k| { | ||
| let has_dot: bool = k.file_name.starts_with('.'); | ||
| let filename_nodot: String = if has_dot { |
There was a problem hiding this comment.
Names are usually short so this maybe a nitpick, but we could work with something like this that does not involve clone / collect:
let filename_nodot = &k.file_name[if has_dot { 1 } else { 0 }..];
There was a problem hiding this comment.
And please add a test to cover this case :)
There was a problem hiding this comment.
I think I did the clone/collect thing because I was thinking about the general case of "characters in Rust can be up to four bytes long", but I guess if we know the first character is an ASCII ., then we're free to slice it.
Also, the test case for the "Dot / No Dot" sorting is in test_ls.rs (inside the test_ls_sort_name function). Should I add more tests than the simple test case I have right now? (Nevermind. Just saw that these two lines aren't covered... I can try to change that).
| @@ -158,7 +158,7 @@ pub fn base_conv_float(src: &[u8], radix_src: u8, _radix_dest: u8) -> f64 { | |||
| // to implement this for arbitrary string input. | |||
| // until then, the below operates as an outline | |||
| // of how it would work. | |||
| let result: Vec<u8> = vec![0]; | |||
| let _result: Vec<u8> = vec![0]; | |||
There was a problem hiding this comment.
please remove this unrelated change :)
src/uu/ls/src/ls.rs
Outdated
| Sort::Name => entries.sort_by_key(|k| k.file_name.to_lowercase()), | ||
| Sort::Name => entries.sort_by_key(|k| { | ||
| let has_dot: bool = k.file_name.starts_with('.'); | ||
| let filename_nodot: String = if has_dot { |
There was a problem hiding this comment.
And please add a test to cover this case :)
ls now behaves like GNU ls with respect to sorting files by ignoring leading periods when sorting by main. Added tests to ensure "touch a .a b .b ; ls" returns ".a a .b b"
8f31505 to
f0aefd9
Compare
ls now behaves like GNU ls with respect to sorting files by ignoring
leading periods when sorting by main.
Added tests to ensure "touch a .a b .b ; ls" returns ".a a .b b"
Note: The non
src/ls/files that were modified were modified because of clippy warnings that wouldn't let me commit otherwise.