From bbcca3eefd98c06984cd22b016513dd6c10435e7 Mon Sep 17 00:00:00 2001 From: Alessandro Stoltenberg Date: Mon, 29 Mar 2021 22:26:55 +0200 Subject: [PATCH 1/4] ls: Implements https://github.com/uutils/coreutils/issues/1880 extension sorting. --- src/uu/ls/src/ls.rs | 32 ++++++++++++++++++++++-- tests/by-util/test_ls.rs | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 0351227eb..9f2c2d993 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -74,6 +74,7 @@ pub mod options { pub static TIME: &str = "t"; pub static NONE: &str = "U"; pub static VERSION: &str = "v"; + pub static EXTENSION: &str = "X"; } pub mod time { pub static ACCESS: &str = "u"; @@ -134,6 +135,7 @@ enum Sort { Size, Time, Version, + Extension, } enum SizeFormat { @@ -277,6 +279,7 @@ impl Config { "time" => Sort::Time, "size" => Sort::Size, "version" => Sort::Version, + "extension" => Sort::Extension, // below should never happen as clap already restricts the values. _ => unreachable!("Invalid field for --sort"), } @@ -288,6 +291,8 @@ impl Config { Sort::None } else if options.is_present(options::sort::VERSION) { Sort::Version + } else if options.is_present(options::sort::EXTENSION) { + Sort::Extension } else { Sort::Name }; @@ -752,10 +757,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .arg( Arg::with_name(options::SORT) .long(options::SORT) - .help("Sort by : name, none (-U), time (-t) or size (-S)") + .help("Sort by : name, none (-U), time (-t), size (-S) or extension (-X)") .value_name("field") .takes_value(true) - .possible_values(&["name", "none", "time", "size", "version"]) + .possible_values(&["name", "none", "time", "size", "version", "extension"]) .require_equals(true) .overrides_with_all(&[ options::SORT, @@ -763,6 +768,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { options::sort::TIME, options::sort::NONE, options::sort::VERSION, + options::sort::EXTENSION, ]) ) .arg( @@ -775,6 +781,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { options::sort::TIME, options::sort::NONE, options::sort::VERSION, + options::sort::EXTENSION, ]) ) .arg( @@ -787,6 +794,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { options::sort::TIME, options::sort::NONE, options::sort::VERSION, + options::sort::EXTENSION, ]) ) .arg( @@ -799,6 +807,20 @@ pub fn uumain(args: impl uucore::Args) -> i32 { options::sort::TIME, options::sort::NONE, options::sort::VERSION, + options::sort::EXTENSION, + ]) + ) + .arg( + Arg::with_name(options::sort::EXTENSION) + .short(options::sort::EXTENSION) + .help("Sort alphabetically by entry extension.") + .overrides_with_all(&[ + options::SORT, + options::sort::SIZE, + options::sort::TIME, + options::sort::NONE, + options::sort::VERSION, + options::sort::EXTENSION, ]) ) .arg( @@ -813,6 +835,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { options::sort::TIME, options::sort::NONE, options::sort::VERSION, + options::sort::EXTENSION, ]) ) @@ -1141,6 +1164,11 @@ fn sort_entries(entries: &mut Vec, config: &Config) { // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by_cached_key(|k| k.file_name.to_lowercase()), Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)), + Sort::Extension => entries.sort_by(|a, b| { + a.extension() + .cmp(&b.extension()) + .then(a.to_string_lossy().cmp(&b.to_string_lossy())) + }), Sort::None => {} } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 09e02f264..364a71d47 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -1681,3 +1681,57 @@ fn test_ls_deref_command_line_dir() { assert!(!result.stdout_str().ends_with("sym_dir")); } + +#[test] +fn test_ls_sort_extension() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + for filename in &[ + "file1", + "file2", + "anotherFile", + ".hidden", + ".file.1", + ".file.2", + "file.1", + "file.2", + "anotherFile.1", + "anotherFile.2", + "file.ext", + "file.debug", + "anotherFile.ext", + "anotherFile.debug", + ] { + at.touch(filename); + } + + let expected = vec![ + ".", + "..", + ".hidden", + "anotherFile", + "file1", + "file2", + ".file.1", + "anotherFile.1", + "file.1", + ".file.2", + "anotherFile.2", + "file.2", + "anotherFile.debug", + "file.debug", + "anotherFile.ext", + "file.ext", + "", // because of '\n' at the end of the output + ]; + + let result = scene.ucmd().arg("-1aX").run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert_eq!(result.stdout.split('\n').collect::>(), expected,); + + let result = scene.ucmd().arg("-1a").arg("--sort=extension").run(); + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + assert_eq!(result.stdout.split('\n').collect::>(), expected,); +} \ No newline at end of file From 9c221148a86ff7b2003c1913aaebcb1a6be7007c Mon Sep 17 00:00:00 2001 From: Alessandro Stoltenberg Date: Tue, 30 Mar 2021 22:54:06 +0200 Subject: [PATCH 2/4] ls: Extension sorting, use file_stem() instead of to_string_lossy() --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 9f2c2d993..145ed0428 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1167,7 +1167,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { Sort::Extension => entries.sort_by(|a, b| { a.extension() .cmp(&b.extension()) - .then(a.to_string_lossy().cmp(&b.to_string_lossy())) + .then(a.file_stem().cmp(&b.file_stem())) }), Sort::None => {} } From 43f3f7e01c0707d6617ec5239f683d2f6790a498 Mon Sep 17 00:00:00 2001 From: Alessandro Stoltenberg Date: Sun, 25 Apr 2021 20:13:42 +0200 Subject: [PATCH 3/4] feat2: Rebased on current master and incorporated changes done to the filetype handling. --- src/uu/ls/src/ls.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 145ed0428..3cd1e037d 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1165,9 +1165,10 @@ fn sort_entries(entries: &mut Vec, config: &Config) { Sort::Name => entries.sort_by_cached_key(|k| k.file_name.to_lowercase()), Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)), Sort::Extension => entries.sort_by(|a, b| { - a.extension() - .cmp(&b.extension()) - .then(a.file_stem().cmp(&b.file_stem())) + a.p_buf + .extension() + .cmp(&b.p_buf.extension()) + .then(a.p_buf.file_stem().cmp(&b.p_buf.file_stem())) }), Sort::None => {} } From f8e7fe4c27b9f9c74218fe02ab24d5ecc5b7a8f2 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 25 Apr 2021 23:33:19 +0200 Subject: [PATCH 4/4] Update to use stdout_str() --- tests/by-util/test_ls.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 364a71d47..b819ef159 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -1726,12 +1726,8 @@ fn test_ls_sort_extension() { ]; let result = scene.ucmd().arg("-1aX").run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - assert_eq!(result.stdout.split('\n').collect::>(), expected,); + assert_eq!(result.stdout_str().split('\n').collect::>(), expected,); let result = scene.ucmd().arg("-1a").arg("--sort=extension").run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - assert_eq!(result.stdout.split('\n').collect::>(), expected,); -} \ No newline at end of file + assert_eq!(result.stdout_str().split('\n').collect::>(), expected,); +}