From adcf08325d1030bd8b1784e0a55429c480716c40 Mon Sep 17 00:00:00 2001 From: ariasuni Date: Wed, 5 May 2021 18:46:16 +0200 Subject: [PATCH] Make sort more consistent accross uses and flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some cases, files were sorted twice, which in most cases is harmless. But when two files are determined equal, their order is unchanged (because the sort is stable). With `--reverse`, equal elements order, which wasn’t changed by the sort, were reversed twice (unchanged). Also, command lines arguments are now sorted by their path. --- src/fs/filter.rs | 13 ++++++++++--- src/main.rs | 26 ++++++++++++++------------ src/output/details.rs | 13 +++++++------ src/output/grid.rs | 4 ++-- src/output/grid_details.rs | 5 +++-- src/output/lines.rs | 4 ++-- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/fs/filter.rs b/src/fs/filter.rs index 7a9fc7f9..db4ab6d6 100644 --- a/src/fs/filter.rs +++ b/src/fs/filter.rs @@ -88,11 +88,11 @@ impl FileFilter { } /// Sort the files in the given vector based on the sort field option. - pub fn sort_files<'a, F>(&self, files: &mut Vec) + pub fn sort_files<'a, F>(&self, files: &mut Vec, maybe_different_parents: bool) where F: AsRef> { files.sort_by(|a, b| { - self.sort_field.compare_files(a.as_ref(), b.as_ref()) + self.sort_field.compare_files(a.as_ref(), b.as_ref(), maybe_different_parents) }); if self.reverse { @@ -213,9 +213,16 @@ impl SortField { /// into groups between letters and numbers, and then sorts those blocks /// together, so `file10` will sort after `file9`, instead of before it /// because of the `1`. - pub fn compare_files(self, a: &File<'_>, b: &File<'_>) -> Ordering { + pub fn compare_files(self, a: &File<'_>, b: &File<'_>, maybe_different_parents: bool) -> Ordering { use self::SortCase::{ABCabc, AaBbCc}; + if self != Self::Unsorted && maybe_different_parents { + match a.path.parent().cmp(&b.path.parent()) { + Ordering::Equal => {} + ord @ _ => return ord + } + } + match self { Self::Unsorted => Ordering::Equal, diff --git a/src/main.rs b/src/main.rs index b8d1b211..50090da5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -195,7 +195,7 @@ impl<'args> Exa<'args> { let is_only_dir = dirs.len() == 1 && no_files; self.options.filter.filter_argument_files(&mut files); - self.print_files(None, files)?; + self.print_files(None, files, true)?; self.print_dirs(dirs, no_files, is_only_dir, exit_status) } @@ -204,7 +204,7 @@ impl<'args> Exa<'args> { for dir in dir_files { // Put a gap between directories, or between the list of files and - // the first directory. + // the first directory when recursing. if first { first = false; } @@ -226,13 +226,15 @@ impl<'args> Exa<'args> { Err((path, e)) => writeln!(io::stderr(), "[{}: {}]", path.display(), e)?, } }; - self.options.filter.filter_child_files(&mut children); - self.options.filter.sort_files(&mut children); + if children.is_empty() { + continue; + } if let Some(recurse_opts) = self.options.dir_action.recurse_options() { let depth = dir.path.components().filter(|&c| c != Component::CurDir).count() + 1; if ! recurse_opts.tree && ! recurse_opts.is_too_deep(depth) { + self.options.filter.sort_files(&mut children, false); let mut child_dirs = Vec::new(); for child_dir in children.iter().filter(|f| f.is_directory() && ! f.is_all_all) { @@ -242,7 +244,7 @@ impl<'args> Exa<'args> { } } - self.print_files(Some(&dir), children)?; + self.print_files(Some(&dir), children, false)?; match self.print_dirs(child_dirs, false, false, exit_status) { Ok(_) => (), Err(e) => return Err(e), @@ -251,14 +253,14 @@ impl<'args> Exa<'args> { } } - self.print_files(Some(&dir), children)?; + self.print_files(Some(&dir), children, false)?; } Ok(exit_status) } /// Prints the list of files using whichever view is selected. - fn print_files(&mut self, dir: Option<&Dir>, files: Vec>) -> io::Result<()> { + fn print_files(&mut self, dir: Option<&Dir>, files: Vec>, maybe_different_parents: bool) -> io::Result<()> { if files.is_empty() { return Ok(()); } @@ -270,14 +272,14 @@ impl<'args> Exa<'args> { (Mode::Grid(ref opts), Some(console_width)) => { let filter = &self.options.filter; let r = grid::Render { files, theme, file_style, opts, console_width, filter }; - r.render(&mut self.writer) + r.render(&mut self.writer, maybe_different_parents) } (Mode::Grid(_), None) | (Mode::Lines, _) => { let filter = &self.options.filter; let r = lines::Render { files, theme, file_style, filter }; - r.render(&mut self.writer) + r.render(&mut self.writer, maybe_different_parents) } (Mode::Details(ref opts), _) => { @@ -287,7 +289,7 @@ impl<'args> Exa<'args> { let git_ignoring = self.options.filter.git_ignore == GitIgnore::CheckAndIgnore; let git = self.git.as_ref(); let r = details::Render { dir, files, theme, file_style, opts, recurse, filter, git_ignoring, git }; - r.render(&mut self.writer) + r.render(&mut self.writer, maybe_different_parents) } (Mode::GridDetails(ref opts), Some(console_width)) => { @@ -300,7 +302,7 @@ impl<'args> Exa<'args> { let git = self.git.as_ref(); let r = grid_details::Render { dir, files, theme, file_style, grid, details, filter, row_threshold, git_ignoring, git, console_width }; - r.render(&mut self.writer) + r.render(&mut self.writer, maybe_different_parents) } (Mode::GridDetails(ref opts), None) => { @@ -311,7 +313,7 @@ impl<'args> Exa<'args> { let git = self.git.as_ref(); let r = details::Render { dir, files, theme, file_style, opts, recurse, filter, git_ignoring, git }; - r.render(&mut self.writer) + r.render(&mut self.writer, maybe_different_parents) } } } diff --git a/src/output/details.rs b/src/output/details.rs index 9dca7d40..712a74cf 100644 --- a/src/output/details.rs +++ b/src/output/details.rs @@ -146,7 +146,7 @@ impl<'a> AsRef> for Egg<'a> { impl<'a> Render<'a> { - pub fn render(mut self, w: &mut W) -> io::Result<()> { + pub fn render(mut self, w: &mut W, first: bool) -> io::Result<()> { let mut pool = Pool::new(num_cpus::get() as u32); let mut rows = Vec::new(); @@ -168,14 +168,14 @@ impl<'a> Render<'a> { // This is weird, but I can’t find a way around it: // https://internals.rust-lang.org/t/should-option-mut-t-implement-copy/3715/6 let mut table = Some(table); - self.add_files_to_table(&mut pool, &mut table, &mut rows, &self.files, TreeDepth::root()); + self.add_files_to_table(&mut pool, &mut table, &mut rows, &self.files, TreeDepth::root(), first); for row in self.iterate_with_table(table.unwrap(), rows) { writeln!(w, "{}", row.strings())? } } else { - self.add_files_to_table(&mut pool, &mut None, &mut rows, &self.files, TreeDepth::root()); + self.add_files_to_table(&mut pool, &mut None, &mut rows, &self.files, TreeDepth::root(), first); for row in self.iterate(rows) { writeln!(w, "{}", row.strings())? @@ -187,7 +187,7 @@ impl<'a> Render<'a> { /// Adds files to the table, possibly recursively. This is easily /// parallelisable, and uses a pool of threads. - fn add_files_to_table<'dir>(&self, pool: &mut Pool, table: &mut Option>, rows: &mut Vec, src: &[File<'dir>], depth: TreeDepth) { + fn add_files_to_table<'dir>(&self, pool: &mut Pool, table: &mut Option>, rows: &mut Vec, src: &[File<'dir>], depth: TreeDepth, first: bool) { use std::sync::{Arc, Mutex}; use log::*; use crate::fs::feature::xattr; @@ -272,7 +272,8 @@ impl<'a> Render<'a> { // this is safe because all entries have been initialized above let mut file_eggs = unsafe { std::mem::transmute::<_, Vec>>(file_eggs) }; - self.filter.sort_files(&mut file_eggs); + + self.filter.sort_files(&mut file_eggs, first); for (tree_params, egg) in depth.iterate_over(file_eggs.into_iter()) { let mut files = Vec::new(); @@ -318,7 +319,7 @@ impl<'a> Render<'a> { rows.push(self.render_error(&error, TreeParams::new(depth.deeper(), false), path)); } - self.add_files_to_table(pool, table, rows, &files, depth.deeper()); + self.add_files_to_table(pool, table, rows, &files, depth.deeper(), false); continue; } } diff --git a/src/output/grid.rs b/src/output/grid.rs index 290ee8b3..e6cfa47e 100644 --- a/src/output/grid.rs +++ b/src/output/grid.rs @@ -31,7 +31,7 @@ pub struct Render<'a> { } impl<'a> Render<'a> { - pub fn render(mut self, w: &mut W) -> io::Result<()> { + pub fn render(mut self, w: &mut W, maybe_different_parents: bool) -> io::Result<()> { let mut grid = tg::Grid::new(tg::GridOptions { direction: self.opts.direction(), filling: tg::Filling::Spaces(2), @@ -39,7 +39,7 @@ impl<'a> Render<'a> { grid.reserve(self.files.len()); - self.filter.sort_files(&mut self.files); + self.filter.sort_files(&mut self.files, maybe_different_parents); for file in &self.files { let filename = self.file_style.for_file(file, self.theme).paint(); diff --git a/src/output/grid_details.rs b/src/output/grid_details.rs index 35ff0236..705c9f83 100644 --- a/src/output/grid_details.rs +++ b/src/output/grid_details.rs @@ -133,12 +133,13 @@ impl<'a> Render<'a> { // This doesn’t take an IgnoreCache even though the details one does // because grid-details has no tree view. - pub fn render(mut self, w: &mut W) -> io::Result<()> { + pub fn render(mut self, w: &mut W, maybe_different_parents: bool) -> io::Result<()> { + self.filter.sort_files(&mut self.files, maybe_different_parents); if let Some((grid, width)) = self.find_fitting_grid() { write!(w, "{}", grid.fit_into_columns(width)) } else { - self.give_up().render(w) + self.give_up().render(w, maybe_different_parents) } } diff --git a/src/output/lines.rs b/src/output/lines.rs index 2343ce50..da2852ae 100644 --- a/src/output/lines.rs +++ b/src/output/lines.rs @@ -18,8 +18,8 @@ pub struct Render<'a> { } impl<'a> Render<'a> { - pub fn render(mut self, w: &mut W) -> io::Result<()> { - self.filter.sort_files(&mut self.files); + pub fn render(mut self, w: &mut W, maybe_different_parents: bool) -> io::Result<()> { + self.filter.sort_files(&mut self.files, maybe_different_parents); for file in &self.files { let name_cell = self.render_file(file); writeln!(w, "{}", ANSIStrings(&name_cell))?;