diff --git a/src/uu/ls/BENCHMARKING.md b/src/uu/ls/BENCHMARKING.md index 852220496..b009b703a 100644 --- a/src/uu/ls/BENCHMARKING.md +++ b/src/uu/ls/BENCHMARKING.md @@ -36,6 +36,8 @@ args="$@" hyperfine "ls $args" "target/release/coreutils ls $args" ``` +**Note**: No localization is currently implemented. This means that the comparison above is not really fair. We can fix this by setting `LC_ALL=C`, so GNU `ls` can ignore localization. + ## Checking system call count - Another thing to look at would be system calls count using strace (on linux) or equivalent on other operating systems. diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 381612189..0e2754f07 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1120,14 +1120,21 @@ impl PathData { fn new( p_buf: PathBuf, file_type: Option>, + file_name: Option, config: &Config, command_line: bool, ) -> Self { - let name = p_buf - .file_name() - .unwrap_or_else(|| p_buf.iter().next_back().unwrap()) - .to_string_lossy() - .into_owned(); + // We cannot use `Path::ends_with` or `Path::Components`, because they remove occurrences of '.' + // For '..', the filename is None + let name = if let Some(name) = file_name { + name + } else { + p_buf + .file_name() + .unwrap_or_else(|| p_buf.iter().next_back().unwrap()) + .to_string_lossy() + .into_owned() + }; let must_dereference = match &config.dereference { Dereference::All => true, Dereference::Args => command_line, @@ -1190,7 +1197,7 @@ fn list(locs: Vec, config: Config) -> i32 { continue; } - let path_data = PathData::new(p, None, &config, true); + let path_data = PathData::new(p, None, None, &config, true); let show_dir_contents = if let Some(ft) = path_data.file_type() { !config.directory && ft.is_dir() @@ -1235,14 +1242,8 @@ fn sort_entries(entries: &mut Vec, config: &Config) { entries.sort_by_key(|k| Reverse(k.md().as_ref().map(|md| md.len()).unwrap_or(0))) } // The default sort in GNU ls is case insensitive - Sort::Name => entries.sort_by_cached_key(|k| { - let has_dot: bool = k.file_name.starts_with('.'); - let filename_nodot: &str = &k.file_name[if has_dot { 1 } else { 0 }..]; - // We want hidden files to appear before regular files of the same - // name, so we need to negate the "has_dot" variable. - (filename_nodot.to_lowercase(), !has_dot) - }), - Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)), + Sort::Name => entries.sort_by(|a, b| a.file_name.cmp(&b.file_name)), + Sort::Version => entries.sort_by(|a, b| version_cmp::version_cmp(&a.p_buf, &b.p_buf)), Sort::Extension => entries.sort_by(|a, b| { a.p_buf .extension() @@ -1281,8 +1282,14 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) { let mut entries: Vec<_> = if config.files == Files::All { vec![ - PathData::new(dir.p_buf.join("."), None, config, false), - PathData::new(dir.p_buf.join(".."), None, config, false), + PathData::new( + dir.p_buf.clone(), + Some(Ok(*dir.file_type().unwrap())), + Some(".".into()), + config, + false, + ), + PathData::new(dir.p_buf.join(".."), None, Some("..".into()), config, false), ] } else { vec![] @@ -1291,7 +1298,7 @@ fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter) let mut temp: Vec<_> = safe_unwrap!(fs::read_dir(&dir.p_buf)) .map(|res| safe_unwrap!(res)) .filter(|e| should_display(e, config)) - .map(|e| PathData::new(DirEntry::path(&e), Some(e.file_type()), config, false)) + .map(|e| PathData::new(DirEntry::path(&e), Some(e.file_type()), None, config, false)) .collect(); sort_entries(&mut temp, config); diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 79e26f200..110764aa5 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -43,23 +43,74 @@ fn test_ls_a() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; at.touch(".test-1"); + at.mkdir("some-dir"); + at.touch( + Path::new("some-dir") + .join(".test-2") + .as_os_str() + .to_str() + .unwrap(), + ); - let result = scene.ucmd().succeeds(); - let stdout = result.stdout_str(); - assert!(!stdout.contains(".test-1")); - assert!(!stdout.contains("..")); + let re_pwd = Regex::new(r"^\.\n").unwrap(); + + // Using the present working directory + scene + .ucmd() + .arg("-1") + .succeeds() + .stdout_does_not_contain(".test-1") + .stdout_does_not_contain("..") + .stdout_does_not_match(&re_pwd); scene .ucmd() .arg("-a") + .arg("-1") .succeeds() .stdout_contains(&".test-1") - .stdout_contains(&".."); + .stdout_contains(&"..") + .stdout_matches(&re_pwd); - let result = scene.ucmd().arg("-A").succeeds(); - result.stdout_contains(".test-1"); - let stdout = result.stdout_str(); - assert!(!stdout.contains("..")); + scene + .ucmd() + .arg("-A") + .arg("-1") + .succeeds() + .stdout_contains(".test-1") + .stdout_does_not_contain("..") + .stdout_does_not_match(&re_pwd); + + // Using a subdirectory + scene + .ucmd() + .arg("-1") + .arg("some-dir") + .succeeds() + .stdout_does_not_contain(".test-2") + .stdout_does_not_contain("..") + .stdout_does_not_match(&re_pwd); + + scene + .ucmd() + .arg("-a") + .arg("-1") + .arg("some-dir") + .succeeds() + .stdout_contains(&".test-2") + .stdout_contains(&"..") + .no_stderr() + .stdout_matches(&re_pwd); + + scene + .ucmd() + .arg("-A") + .arg("-1") + .arg("some-dir") + .succeeds() + .stdout_contains(".test-2") + .stdout_does_not_contain("..") + .stdout_does_not_match(&re_pwd); } #[test] @@ -482,7 +533,6 @@ fn test_ls_sort_name() { .succeeds() .stdout_is(["test-1", "test-2", "test-3\n"].join(sep)); - // Order of a named sort ignores leading dots. let scene_dot = TestScenario::new(util_name!()); let at = &scene_dot.fixtures; at.touch(".a"); @@ -495,7 +545,7 @@ fn test_ls_sort_name() { .arg("--sort=name") .arg("-A") .succeeds() - .stdout_is([".a", "a", ".b", "b\n"].join(sep)); + .stdout_is([".a", ".b", "a", "b\n"].join(sep)); } #[test]