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

@@ -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<String> = 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(),

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,
+            },
         ]
     }
 }

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.

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<Path>) -> 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<Path>) -> bool {
-    match dir.as_ref().read_dir() {
-        Err(_) => true,
-        Ok(mut s) => s.next().is_none(),
-    }
-}