Move `ls` back to last-known-good state (#6175)0.66.2

* revert the recent ls changes

* cargo fmt

2 files changed, 130 insertions, 308 deletions

diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs
index 0fa94e1cc..02263cc7a 100644
--- a/crates/nu-command/src/filesystem/ls.rs
+++ b/crates/nu-command/src/filesystem/ls.rs
@@ -1,17 +1,15 @@
 use crate::DirBuilder;
 use crate::DirInfo;
 use chrono::{DateTime, Local, LocalResult, TimeZone, Utc};
-use itertools::Itertools;
 use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_glob::MatchOptions;
 use nu_path::expand_to_real_path;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::IntoPipelineData;
 use nu_protocol::{
-    Category, DataSource, Example, IntoInterruptiblePipelineData, PipelineData, PipelineMetadata,
-    ShellError, Signature, Span, Spanned, SyntaxShape, Value,
+    Category, DataSource, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData,
+    PipelineMetadata, ShellError, Signature, Span, Spanned, SyntaxShape, Value,
 };
 use pathdiff::diff_paths;
 
@@ -40,11 +38,7 @@ impl Command for Ls {
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("ls")
             // Using a string instead of a glob pattern shape so it won't auto-expand
-            .rest(
-                "pattern(s)",
-                SyntaxShape::String,
-                "the glob pattern(s) to use",
-            )
+            .optional("pattern", SyntaxShape::String, "the glob pattern to use")
             .switch("all", "Show hidden files", Some('a'))
             .switch(
                 "long",
@@ -87,23 +81,19 @@ impl Command for Ls {
         let call_span = call.head;
         let cwd = current_dir(engine_state, stack)?;
 
-        let mut shell_errors: Vec<ShellError> = vec![];
-        let pattern_args: Vec<Spanned<String>> = call.rest(engine_state, stack, 0)?;
-        let glob_results = if !pattern_args.is_empty() {
-            pattern_args
-                .into_iter()
-                .flat_map(|pattern_arg| {
-                    let mut path = expand_to_real_path(pattern_arg.clone().item);
-                    let p_tag = pattern_arg.span;
-                    let cwd = cwd.clone();
-                    let ctrl_c = ctrl_c.clone();
-
-                    let expanded = nu_path::expand_path_with(&path, &cwd);
-                    // Avoid checking and pushing "*" to the path when directory (do not show contents) flag is true
-                    if !directory && expanded.is_dir() {
-                        if permission_denied(&path) {
-                            #[cfg(unix)]
-                            let error_msg = format!(
+        let pattern_arg: Option<Spanned<String>> = call.opt(engine_state, stack, 0)?;
+
+        let (path, p_tag, absolute_path) = match pattern_arg {
+            Some(p) => {
+                let p_tag = p.span;
+                let mut p = expand_to_real_path(p.item);
+
+                let expanded = nu_path::expand_path_with(&p, &cwd);
+                // 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!(
                             "The permissions of {:o} do not allow access for this user",
                             expanded
                                 .metadata()
@@ -114,225 +104,94 @@ impl Command for Ls {
                                 .mode()
                                 & 0o0777
                         );
-                            #[cfg(not(unix))]
-                            let error_msg = String::from("Permission denied");
-                            shell_errors.push(ShellError::GenericError(
-                                "Permission denied".to_string(),
-                                error_msg,
-                                Some(p_tag),
-                                None,
-                                Vec::new(),
-                            ));
-                        }
-                        if is_empty_dir(&expanded) {
-                            return Vec::from([Value::nothing(call_span)]).into_iter();
-                        }
-                        path.push("*");
-                    }
-
-                    let absolute_path = path.is_absolute();
-
-                    let hidden_dir_specified = is_hidden_dir(&path);
-
-                    let glob_path = Spanned {
-                        item: path.display().to_string(),
-                        span: p_tag,
-                    };
-
-                    let glob_options = if all {
-                        None
-                    } else {
-                        let mut glob_options = MatchOptions::new();
-                        glob_options.recursive_match_hidden_dir = false;
-                        Some(glob_options)
-                    };
-                    let (prefix, paths) =
-                        match nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options) {
-                            Ok((prefix, paths)) => (prefix, paths),
-                            Err(e) => {
-                                shell_errors.push(e);
-                                return Vec::from([Value::nothing(call_span)]).into_iter();
-                            }
-                        };
-                    let mut paths_peek = paths.peekable();
-                    if paths_peek.peek().is_none() {
-                        shell_errors.push(ShellError::GenericError(
-                            format!("No matches found for {}", &path.display().to_string()),
-                            "".to_string(),
+                        #[cfg(not(unix))]
+                        let error_msg = String::from("Permission denied");
+                        return Err(ShellError::GenericError(
+                            "Permission denied".to_string(),
+                            error_msg,
+                            Some(p_tag),
                             None,
-                            Some("no matches found".to_string()),
                             Vec::new(),
                         ));
                     }
+                    if is_empty_dir(&expanded) {
+                        return Ok(Value::nothing(call_span).into_pipeline_data());
+                    }
+                    p.push("*");
+                }
+                let absolute_path = p.is_absolute();
+                (p, p_tag, absolute_path)
+            }
+            None => {
+                // 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)
+                }
+            }
+        };
 
-                    let mut hidden_dirs = vec![];
-
-                    paths_peek
-                        .into_iter()
-                        .filter_map(move |x| match x {
-                            Ok(path) => {
-                                let metadata = match std::fs::symlink_metadata(&path) {
-                                    Ok(metadata) => Some(metadata),
-                                    Err(_) => None,
-                                };
-                                if path_contains_hidden_folder(&path, &hidden_dirs) {
-                                    return None;
-                                }
-
-                                if !all && !hidden_dir_specified && is_hidden_dir(&path) {
-                                    if path.is_dir() {
-                                        hidden_dirs.push(path);
-                                    }
-                                    return None;
-                                }
-
-                                let display_name = if short_names {
-                                    path.file_name().map(|os| os.to_string_lossy().to_string())
-                                } else if full_paths || absolute_path {
-                                    Some(path.to_string_lossy().to_string())
-                                } else if let Some(prefix) = &prefix {
-                                    if let Ok(remainder) = path.strip_prefix(&prefix) {
-                                        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(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())
-                                    }
-                                } else {
-                                    Some(path.to_string_lossy().to_string())
-                                }
-                                .ok_or_else(|| {
-                                    ShellError::GenericError(
-                                        format!("Invalid file name: {:}", path.to_string_lossy()),
-                                        "invalid file name".into(),
-                                        Some(call_span),
-                                        None,
-                                        Vec::new(),
-                                    )
-                                });
-
-                                match display_name {
-                                    Ok(name) => {
-                                        let entry = dir_entry_dict(
-                                            &path,
-                                            &name,
-                                            metadata.as_ref(),
-                                            call_span,
-                                            long,
-                                            du,
-                                            ctrl_c.clone(),
-                                        );
-                                        match entry {
-                                            Ok(value) => Some(value),
-                                            Err(err) => Some(Value::Error { error: err }),
-                                        }
-                                    }
-                                    Err(err) => Some(Value::Error { error: err }),
-                                }
-                            }
-                            _ => Some(Value::Nothing { span: call_span }),
-                        })
-                        .collect_vec()
-                        .into_iter()
-                })
-                .collect_vec()
-        } else {
-            let (path, p_tag, absolute_path) = 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)
-            };
-
-            let hidden_dir_specified = is_hidden_dir(&path);
+        let hidden_dir_specified = is_hidden_dir(&path);
 
-            let glob_path = Spanned {
-                item: path.display().to_string(),
-                span: p_tag,
-            };
+        let glob_path = Spanned {
+            item: path.display().to_string(),
+            span: p_tag,
+        };
 
-            let glob_options = if all {
-                None
-            } else {
-                let mut glob_options = MatchOptions::new();
-                glob_options.recursive_match_hidden_dir = false;
-                Some(glob_options)
-            };
-            let (prefix, paths) = nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options)?;
-
-            let mut paths_peek = paths.peekable();
-            if paths_peek.peek().is_none() {
-                return Err(ShellError::GenericError(
-                    format!("No matches found for {}", &path.display().to_string()),
-                    "".to_string(),
-                    None,
-                    Some("no matches found".to_string()),
-                    Vec::new(),
-                ));
-            }
+        let glob_options = if all {
+            None
+        } else {
+            let mut glob_options = MatchOptions::new();
+            glob_options.recursive_match_hidden_dir = false;
+            Some(glob_options)
+        };
+        let (prefix, paths) = nu_engine::glob_from(&glob_path, &cwd, call_span, glob_options)?;
+
+        let mut paths_peek = paths.peekable();
+        if paths_peek.peek().is_none() {
+            return Err(ShellError::GenericError(
+                format!("No matches found for {}", &path.display().to_string()),
+                "".to_string(),
+                None,
+                Some("no matches found".to_string()),
+                Vec::new(),
+            ));
+        }
 
-            let mut hidden_dirs = vec![];
+        let mut hidden_dirs = vec![];
 
-            paths_peek
-                .into_iter()
-                .filter_map(move |x| match x {
-                    Ok(path) => {
-                        let metadata = match std::fs::symlink_metadata(&path) {
-                            Ok(metadata) => Some(metadata),
-                            Err(_) => None,
-                        };
-                        if path_contains_hidden_folder(&path, &hidden_dirs) {
-                            return None;
-                        }
+        Ok(paths_peek
+            .into_iter()
+            .filter_map(move |x| match x {
+                Ok(path) => {
+                    let metadata = match std::fs::symlink_metadata(&path) {
+                        Ok(metadata) => Some(metadata),
+                        Err(_) => None,
+                    };
+                    if path_contains_hidden_folder(&path, &hidden_dirs) {
+                        return None;
+                    }
 
-                        if !all && !hidden_dir_specified && is_hidden_dir(&path) {
-                            if path.is_dir() {
-                                hidden_dirs.push(path);
-                            }
-                            return None;
+                    if !all && !hidden_dir_specified && is_hidden_dir(&path) {
+                        if path.is_dir() {
+                            hidden_dirs.push(path);
                         }
+                        return None;
+                    }
 
-                        let display_name = if short_names {
-                            path.file_name().map(|os| os.to_string_lossy().to_string())
-                        } else if full_paths || absolute_path {
-                            Some(path.to_string_lossy().to_string())
-                        } else if let Some(prefix) = &prefix {
-                            if let Ok(remainder) = path.strip_prefix(&prefix) {
-                                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 display_name = if short_names {
+                        path.file_name().map(|os| os.to_string_lossy().to_string())
+                    } else if full_paths || absolute_path {
+                        Some(path.to_string_lossy().to_string())
+                    } else if let Some(prefix) = &prefix {
+                        if let Ok(remainder) = path.strip_prefix(&prefix) {
+                            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()
@@ -343,63 +202,53 @@ impl Command for Ls {
                                         path.to_string_lossy().to_string()
                                     };
 
-                                    Some(path_diff)
+                                Some(path_diff)
+                            } else {
+                                let new_prefix = if let Some(pfx) = diff_paths(&prefix, &cwd) {
+                                    pfx
                                 } else {
-                                    let new_prefix = if let Some(pfx) = diff_paths(&prefix, &cwd) {
-                                        pfx
-                                    } else {
-                                        prefix.to_path_buf()
-                                    };
+                                    prefix.to_path_buf()
+                                };
 
-                                    Some(new_prefix.join(remainder).to_string_lossy().to_string())
-                                }
-                            } else {
-                                Some(path.to_string_lossy().to_string())
+                                Some(new_prefix.join(remainder).to_string_lossy().to_string())
                             }
                         } else {
                             Some(path.to_string_lossy().to_string())
                         }
-                        .ok_or_else(|| {
-                            ShellError::GenericError(
-                                format!("Invalid file name: {:}", path.to_string_lossy()),
-                                "invalid file name".into(),
-                                Some(call_span),
-                                None,
-                                Vec::new(),
-                            )
-                        });
-
-                        match display_name {
-                            Ok(name) => {
-                                let entry = dir_entry_dict(
-                                    &path,
-                                    &name,
-                                    metadata.as_ref(),
-                                    call_span,
-                                    long,
-                                    du,
-                                    ctrl_c.clone(),
-                                );
-                                match entry {
-                                    Ok(value) => Some(value),
-                                    Err(err) => Some(Value::Error { error: err }),
-                                }
+                    } else {
+                        Some(path.to_string_lossy().to_string())
+                    }
+                    .ok_or_else(|| {
+                        ShellError::GenericError(
+                            format!("Invalid file name: {:}", path.to_string_lossy()),
+                            "invalid file name".into(),
+                            Some(call_span),
+                            None,
+                            Vec::new(),
+                        )
+                    });
+
+                    match display_name {
+                        Ok(name) => {
+                            let entry = dir_entry_dict(
+                                &path,
+                                &name,
+                                metadata.as_ref(),
+                                call_span,
+                                long,
+                                du,
+                                ctrl_c.clone(),
+                            );
+                            match entry {
+                                Ok(value) => Some(value),
+                                Err(err) => Some(Value::Error { error: err }),
                             }
-                            Err(err) => Some(Value::Error { error: err }),
                         }
+                        Err(err) => Some(Value::Error { error: err }),
                     }
-                    _ => Some(Value::Nothing { span: call_span }),
-                })
-                .collect_vec()
-        };
-
-        if !shell_errors.is_empty() {
-            return Err(shell_errors.pop().expect("Vec pop error"));
-        }
-
-        Ok(glob_results
-            .into_iter()
-            .filter(|result| !matches!(result, Value::Nothing { .. }))
+                }
+                _ => Some(Value::Nothing { span: call_span }),
+            })
             .into_pipeline_data_with_metadata(
                 PipelineMetadata {
                     data_source: DataSource::Ls,
@@ -431,11 +280,6 @@ impl Command for Ls {
                 result: None,
             },
             Example {
-                description: "List all rust files and all toml files",
-                example: "ls *.rs *.toml",
-                result: None,
-            },
-            Example {
                 description: "List all files and directories whose name do not contain 'bar'",
                 example: "ls -s | where name !~ bar",
                 result: None,
diff --git a/crates/nu-command/tests/commands/ls.rs b/crates/nu-command/tests/commands/ls.rs
index 93d09ea9e..f5708df52 100644
--- a/crates/nu-command/tests/commands/ls.rs
+++ b/crates/nu-command/tests/commands/ls.rs
@@ -336,28 +336,6 @@ fn lists_files_including_starting_with_dot() {
 }
 
 #[test]
-fn lists_regular_files_using_multiple_asterisk_wildcards() {
-    Playground::setup("ls_test_10", |dirs, sandbox| {
-        sandbox.with_files(vec![
-            EmptyFile("los.txt"),
-            EmptyFile("tres.txt"),
-            EmptyFile("amigos.txt"),
-            EmptyFile("arepas.clu"),
-        ]);
-
-        let actual = nu!(
-            cwd: dirs.test(), pipeline(
-            r#"
-                ls *.txt *.clu
-                | length
-            "#
-        ));
-
-        assert_eq!(actual.out, "4");
-    })
-}
-
-#[test]
 fn list_all_columns() {
     Playground::setup("ls_test_all_columns", |dirs, sandbox| {
         sandbox.with_files(vec![