From 221f36ca65a75f50c3886814db10916ae3f68dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=98=E5=8F=AF=E6=80=9D?= Date: Sat, 9 Jul 2022 03:15:34 +0800 Subject: [PATCH] Add --directory (-D) flag to ls, list the directory itself instead of its contents (#5970) * Avoid extending the directory without globs in `nu_engine::glob_from` * avoid joining a `*` to the directory without globs * remove checks on directory permission and whether it is empty The previous implemention of `nu_engine::glob_from` will extend the given directory even if it containes no glob pattern. This commit overcomes lack of consistency with the function `nu_glob::glob`. * Add flag -D to ls, to list the directory itself instead of its contents * add --directory (-d) flag to ls * correct the difference between the given path and the cwd * set default path to `.` instead of `./*` when --directory (-d) flag is true * add comments * add an example * add tests * fmt --- crates/nu-cli/tests/flag_completions.rs | 4 +- crates/nu-command/src/filesystem/ls.rs | 49 +++++++++++++++--- crates/nu-command/tests/commands/ls.rs | 68 +++++++++++++++++++++++++ crates/nu-engine/src/glob_from.rs | 50 +----------------- 4 files changed, 113 insertions(+), 58 deletions(-) diff --git a/crates/nu-cli/tests/flag_completions.rs b/crates/nu-cli/tests/flag_completions.rs index c83144429..6e816bb4b 100644 --- a/crates/nu-cli/tests/flag_completions.rs +++ b/crates/nu-cli/tests/flag_completions.rs @@ -14,15 +14,17 @@ fn flag_completions() { // Test completions for the 'ls' flags let suggestions = completer.complete("ls -", 4); - assert_eq!(12, suggestions.len()); + assert_eq!(14, suggestions.len()); let expected: Vec = vec![ "--all".into(), + "--directory".into(), "--du".into(), "--full-paths".into(), "--help".into(), "--long".into(), "--short-names".into(), + "-D".into(), "-a".into(), "-d".into(), "-f".into(), diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index d1f91a933..02263cc7a 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -56,6 +56,11 @@ impl Command for Ls { "Display the apparent directory size in place of the directory metadata size", Some('d'), ) + .switch( + "directory", + "List the specified directory itself instead of its contents", + Some('D'), + ) .category(Category::FileSystem) } @@ -71,6 +76,7 @@ impl Command for Ls { let short_names = call.has_flag("short-names"); let full_paths = call.has_flag("full-paths"); let du = call.has_flag("du"); + let directory = call.has_flag("directory"); let ctrl_c = engine_state.ctrlc.clone(); let call_span = call.head; let cwd = current_dir(engine_state, stack)?; @@ -83,7 +89,8 @@ impl Command for Ls { let mut p = expand_to_real_path(p.item); let expanded = nu_path::expand_path_with(&p, &cwd); - if expanded.is_dir() { + // Avoid checking and pushing "*" to the path when directory (do not show contents) flag is true + if !directory && expanded.is_dir() { if permission_denied(&p) { #[cfg(unix)] let error_msg = format!( @@ -116,7 +123,10 @@ impl Command for Ls { (p, p_tag, absolute_path) } None => { - if is_empty_dir(current_dir(engine_state, stack)?) { + // Avoid pushing "*" to the default path when directory (do not show contents) flag is true + if directory { + (PathBuf::from("."), call_span, false) + } else if is_empty_dir(current_dir(engine_state, stack)?) { return Ok(Value::nothing(call_span).into_pipeline_data()); } else { (PathBuf::from("./*"), call_span, false) @@ -178,13 +188,30 @@ impl Command for Ls { Some(path.to_string_lossy().to_string()) } else if let Some(prefix) = &prefix { if let Ok(remainder) = path.strip_prefix(&prefix) { - let new_prefix = if let Some(pfx) = diff_paths(&prefix, &cwd) { - pfx - } else { - prefix.to_path_buf() - }; + if directory { + // When the path is the same as the cwd, path_diff should be "." + let path_diff = + if let Some(path_diff_not_dot) = diff_paths(&path, &cwd) { + let path_diff_not_dot = path_diff_not_dot.to_string_lossy(); + if path_diff_not_dot.is_empty() { + ".".to_string() + } else { + path_diff_not_dot.to_string() + } + } else { + path.to_string_lossy().to_string() + }; - Some(new_prefix.join(remainder).to_string_lossy().to_string()) + Some(path_diff) + } else { + let new_prefix = if let Some(pfx) = diff_paths(&prefix, &cwd) { + pfx + } else { + prefix.to_path_buf() + }; + + Some(new_prefix.join(remainder).to_string_lossy().to_string()) + } } else { Some(path.to_string_lossy().to_string()) } @@ -268,6 +295,12 @@ impl Command for Ls { example: "ls -s ~ | where type == dir && modified < ((date now) - 7day)", result: None, }, + Example { + description: "List given paths, show directories themselves", + example: + "['/path/to/directory' '/path/to/file'] | each { |it| ls -D $it } | flatten", + result: None, + }, ] } } diff --git a/crates/nu-command/tests/commands/ls.rs b/crates/nu-command/tests/commands/ls.rs index 4f907478d..cc41e4728 100644 --- a/crates/nu-command/tests/commands/ls.rs +++ b/crates/nu-command/tests/commands/ls.rs @@ -392,6 +392,74 @@ fn list_all_columns() { }); } +#[test] +fn lists_with_directory_flag() { + Playground::setup("ls_test_flag_directory_1", |dirs, sandbox| { + sandbox + .within("dir_files") + .with_files(vec![EmptyFile("nushell.json")]) + .within("dir_empty"); + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + cd dir_empty; + ['.' '././.' '..' '../dir_files' '../dir_files/*'] + | each { |it| ls --directory $it } + | flatten + | get name + | to text + "# + )); + let expected = [".", ".", "..", "../dir_files", "../dir_files/nushell.json"].join(""); + #[cfg(windows)] + let expected = expected.replace("/", "\\"); + assert_eq!( + actual.out, expected, + "column names are incorrect for ls --directory (-D)" + ); + }); +} + +#[test] +fn lists_with_directory_flag_without_argument() { + Playground::setup("ls_test_flag_directory_2", |dirs, sandbox| { + sandbox + .within("dir_files") + .with_files(vec![EmptyFile("nushell.json")]) + .within("dir_empty"); + // Test if there are some files in the current directory + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + cd dir_files; + ls --directory + | get name + | to text + "# + )); + let expected = "."; + assert_eq!( + actual.out, expected, + "column names are incorrect for ls --directory (-D)" + ); + // Test if there is no file in the current directory + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + cd dir_empty; + ls -D + | get name + | to text + "# + )); + let expected = "."; + assert_eq!( + actual.out, expected, + "column names are incorrect for ls --directory (-D)" + ); + }); +} + /// Rust's fs::metadata function is unable to read info for certain system files on Windows, /// like the `C:\Windows\System32\Configuration` folder. https://github.com/rust-lang/rust/issues/96980 /// This test confirms that Nu can work around this successfully. diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index 2025c1d8a..4cd744f99 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -1,5 +1,3 @@ -#[cfg(unix)] -use std::os::unix::fs::PermissionsExt; use std::path::{Component, Path, PathBuf}; use nu_glob::MatchOptions; @@ -48,39 +46,7 @@ pub fn glob_from( } else { return Err(ShellError::DirectoryNotFound(pattern.span, None)); }; - - if path.is_dir() { - if permission_denied(&path) { - #[cfg(unix)] - let error_msg = format!( - "The permissions of {:o} do not allow access for this user", - path.metadata() - .expect("this shouldn't be called since we already know there is a dir") - .permissions() - .mode() - & 0o0777 - ); - - #[cfg(not(unix))] - let error_msg = String::from("Permission denied"); - - return Err(ShellError::GenericError( - "Permission denied".into(), - error_msg, - Some(pattern.span), - None, - Vec::new(), - )); - } - - if is_empty_dir(&path) { - return Ok((Some(path), Box::new(vec![].into_iter()))); - } - - (Some(path.clone()), path.join("*")) - } else { - (path.parent().map(|parent| parent.to_path_buf()), path) - } + (path.parent().map(|parent| parent.to_path_buf()), path) }; let pattern = pattern.to_string_lossy().to_string(); @@ -110,17 +76,3 @@ pub fn glob_from( })), )) } - -fn permission_denied(dir: impl AsRef) -> bool { - match dir.as_ref().read_dir() { - Err(e) => matches!(e.kind(), std::io::ErrorKind::PermissionDenied), - Ok(_) => false, - } -} - -fn is_empty_dir(dir: impl AsRef) -> bool { - match dir.as_ref().read_dir() { - Err(_) => true, - Ok(mut s) => s.next().is_none(), - } -}