From a84d410f117d2247acb2b33452356c5c182572db Mon Sep 17 00:00:00 2001
From: Bark <32035685+userwiths@users.noreply.github.com>
Date: Fri, 15 Nov 2024 01:39:41 +0200
Subject: [PATCH] Fix inconsistency in `ls` sort-order (#13875)

Fixes #13267

As we can see from the bisect done in the comments.
Bisected to https://github.com/nushell/nushell/pull/12625 /
https://github.com/nushell/nushell/commit/460a1c8f874b503cb4140619838be483600f00ce

We can see that this update brought the use of `read_dir` and for it, it
is mentioned in the [rust
docs](https://doc.rust-lang.org/std/fs/fn.read_dir.html#platform-specific-behavior)
that it does **not** provide any specific order of files.
As was the advice there, I went and applied a manual `sort` to the
entries and tested it manually on my local machine.

If required I could probably try and add tests for the order
consistency, would need some time to find my way around them, so I'm
sending the PR first.
---
 crates/nu-command/src/filesystem/ls.rs | 16 +++++++++++++---
 crates/nu-command/tests/commands/ls.rs | 24 ++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs
index 465fae16ff..b1b231a4a5 100644
--- a/crates/nu-command/src/filesystem/ls.rs
+++ b/crates/nu-command/src/filesystem/ls.rs
@@ -339,7 +339,7 @@ fn ls_for_one_pattern(
     let path = pattern_arg.into_spanned(p_tag);
     let (prefix, paths) = if just_read_dir {
         let expanded = nu_path::expand_path_with(path.item.as_ref(), &cwd, path.item.is_expand());
-        let paths = read_dir(&expanded)?;
+        let paths = read_dir(&expanded, use_threads)?;
         // just need to read the directory, so prefix is path itself.
         (Some(expanded), paths)
     } else {
@@ -979,10 +979,20 @@ mod windows_helper {
 #[allow(clippy::type_complexity)]
 fn read_dir(
     f: &Path,
+    use_threads: bool,
 ) -> Result<Box<dyn Iterator<Item = Result<PathBuf, ShellError>> + Send>, ShellError> {
-    let iter = f.read_dir()?.map(|d| {
+    let items = f.read_dir()?.map(|d| {
         d.map(|r| r.path())
             .map_err(|e| ShellError::IOError { msg: e.to_string() })
     });
-    Ok(Box::new(iter))
+    if !use_threads {
+        let mut collected = items.collect::<Vec<_>>();
+        collected.sort_by(|a, b| {
+            let a = a.as_ref().expect("path should be valid");
+            let b = b.as_ref().expect("path should be valid");
+            a.cmp(b)
+        });
+        return Ok(Box::new(collected.into_iter()));
+    }
+    Ok(Box::new(items))
 }
diff --git a/crates/nu-command/tests/commands/ls.rs b/crates/nu-command/tests/commands/ls.rs
index 7ac0063741..4140ea64bc 100644
--- a/crates/nu-command/tests/commands/ls.rs
+++ b/crates/nu-command/tests/commands/ls.rs
@@ -833,3 +833,27 @@ fn list_symlink_with_full_path() {
         );
     })
 }
+
+#[test]
+fn consistent_list_order() {
+    Playground::setup("ls_test_order", |dirs, sandbox| {
+        sandbox.with_files(&[
+            EmptyFile("los.txt"),
+            EmptyFile("tres.txt"),
+            EmptyFile("amigos.txt"),
+            EmptyFile("arepas.clu"),
+        ]);
+
+        let no_arg = nu!(
+            cwd: dirs.test(), pipeline(
+            "ls"
+        ));
+
+        let with_arg = nu!(
+            cwd: dirs.test(), pipeline(
+            "ls ."
+        ));
+
+        assert_eq!(no_arg.out, with_arg.out);
+    })
+}