Fix bug with sort_by.

The bug was an over-eager `panic!` to catch a case that shouldn't happen.
In fact, it's OK if it does happen. Add a regression test.

2 files changed, 39 insertions, 17 deletions

diff --git a/src/lib.rs b/src/lib.rs
index 012cac6..6b6176b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -89,7 +89,7 @@ for entry in walker.filter_entry(|e| !is_hidden(e)) {
 #[cfg(test)] extern crate quickcheck;
 #[cfg(test)] extern crate rand;
 
-use std::cmp::min;
+use std::cmp::{Ordering, min};
 use std::error;
 use std::fmt;
 use std::fs::{self, FileType, ReadDir};
@@ -191,7 +191,7 @@ struct WalkDirOptions {
     max_open: usize,
     min_depth: usize,
     max_depth: usize,
-    sorter: Option<Box<FnMut(&OsString,&OsString) -> std::cmp::Ordering>>,
+    sorter: Option<Box<FnMut(&OsString,&OsString) -> Ordering + 'static>>,
 }
 
 impl WalkDir {
@@ -290,11 +290,11 @@ impl WalkDir {
     }
 
     /// Set a function for sorting directory entries.
-    /// If a compare function is set, WalkDir will return all paths in sorted
-    /// order. The compare function will be called to compare names from entries
-    /// from the same directory. Just the file_name() part of the paths is
-    /// passed to the compare function.
-    /// If no function is set, the entries will not be sorted.
+    ///
+    /// If a compare function is set, the resulting iterator will return all
+    /// paths in sorted order. The compare function will be called to compare
+    /// names from entries from the same directory using only the name of the
+    /// entry.
     ///
     /// ```rust,no-run
     /// use std::cmp;
@@ -303,7 +303,8 @@ impl WalkDir {
     ///
     /// WalkDir::new("foo").sort_by(|a,b| a.cmp(b));
     /// ```
-    pub fn sort_by<F: FnMut(&OsString,&OsString) -> std::cmp::Ordering + 'static>(mut self, cmp: F) -> Self {
+    pub fn sort_by<F>(mut self, cmp: F) -> Self
+            where F: FnMut(&OsString, &OsString) -> Ordering + 'static {
         self.opts.sorter = Some(Box::new(cmp));
         self
     }
@@ -569,17 +570,18 @@ impl Iter {
         });
         let mut list = DirList::Opened { depth: self.depth, it: rd };
         if let Some(ref mut cmp) = self.opts.sorter {
-            let mut entries = list.collect::<Vec<_>>();
+            let mut entries: Vec<_> = list.collect();
             entries.sort_by(|a, b| {
                 match (a, b) {
-                    (&Ok(ref a), &Ok(ref b))
-                        => cmp(&a.file_name(), &b.file_name()),
-                    (&Err(_), &Err(_)) => std::cmp::Ordering::Equal,
-                    (&Ok(_), &Err(_)) => std::cmp::Ordering::Greater,
-                    (&Err(_), &Ok(_)) => std::cmp::Ordering::Less,
+                    (&Ok(ref a), &Ok(ref b)) => {
+                        cmp(&a.file_name(), &b.file_name())
+                    }
+                    (&Err(_), &Err(_)) => Ordering::Equal,
+                    (&Ok(_), &Err(_)) => Ordering::Greater,
+                    (&Err(_), &Ok(_)) => Ordering::Less,
                 }
             });
-            list = DirList::Closed ( entries.into_iter() );
+            list = DirList::Closed(entries.into_iter());
         }
         self.stack_list.push(list);
         if self.opts.follow_links {
@@ -637,8 +639,6 @@ impl DirList {
     fn close(&mut self) {
         if let DirList::Opened { .. } = *self {
             *self = DirList::Closed(self.collect::<Vec<_>>().into_iter());
-        } else {
-            unreachable!("BUG: entry already closed");
         }
     }
 }
diff --git a/src/tests.rs b/src/tests.rs
index 03955f4..5b43f27 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -659,3 +659,25 @@ fn walk_dir_sort() {
     assert_eq!(got,
         ["", "/foo", "/foo/abc", "/foo/abc/fit", "/foo/bar", "/foo/faz"]);
 }
+
+#[test]
+fn walk_dir_sort_small_fd_max() {
+    let exp = td("foo", vec![
+        tf("bar"),
+        td("abc", vec![tf("fit")]),
+        tf("faz"),
+    ]);
+    let tmp = tmpdir();
+    let tmp_path = tmp.path();
+    let tmp_len = tmp_path.to_str().unwrap().len();
+    exp.create_in(tmp_path).unwrap();
+    let it =
+        WalkDir::new(tmp_path).max_open(1).sort_by(|a,b| a.cmp(b)).into_iter();
+    let got = it.map(|d| {
+        let path = d.unwrap();
+        let path = &path.path().to_str().unwrap()[tmp_len..];
+        path.replace("\\", "/")
+    }).collect::<Vec<String>>();
+    assert_eq!(got,
+        ["", "/foo", "/foo/abc", "/foo/abc/fit", "/foo/bar", "/foo/faz"]);
+}