Browse Source

refactor: Move total-size calculations to File

Robert Minsk 2 years ago
parent
commit
dedaf17da2

+ 1 - 0
Cargo.lock

@@ -370,6 +370,7 @@ dependencies = [
  "natord",
  "num_cpus",
  "number_prefix",
+ "once_cell",
  "percent-encoding",
  "phf",
  "proc-mounts",

+ 1 - 0
Cargo.toml

@@ -81,6 +81,7 @@ log = "0.4"
 natord = "1.0"
 num_cpus = "1.16"
 number_prefix = "0.4"
+once_cell = "1.18.0"
 percent-encoding = "2.3.0"
 phf = { version = "0.11.2", features = ["macros"] }
 scoped_threadpool = "0.1"

+ 1 - 1
completions/nush/eza.nu

@@ -42,7 +42,7 @@ export extern "eza" [
     --accessed(-u)             # Use the accessed timestamp field
     --created(-U)              # Use the created timestamp field
     --time-style               # How to format timestamps
-    --total-size                # Show recursive directory size
+    --total-size               # Show recursive directory size
     --no-permissions           # Suppress the permissions field
     --octal-permissions(-o)    # List each file's permission in octal format
     --no-filesize              # Suppress the filesize field

+ 21 - 4
src/fs/dir.rs

@@ -51,6 +51,7 @@ impl Dir {
         git: Option<&'ig GitCache>,
         git_ignoring: bool,
         deref_links: bool,
+        total_size: bool,
     ) -> Files<'dir, 'ig> {
         Files {
             inner: self.contents.iter(),
@@ -60,6 +61,7 @@ impl Dir {
             git,
             git_ignoring,
             deref_links,
+            total_size,
         }
     }
 
@@ -95,6 +97,9 @@ pub struct Files<'dir, 'ig> {
 
     /// Whether symbolic links should be dereferenced when querying information.
     deref_links: bool,
+
+    /// Whether to calculate the directory size recursively
+    total_size: bool,
 }
 
 impl<'dir, 'ig> Files<'dir, 'ig> {
@@ -131,8 +136,14 @@ impl<'dir, 'ig> Files<'dir, 'ig> {
                     }
                 }
 
-                let file = File::from_args(path.clone(), self.dir, filename, self.deref_links)
-                    .map_err(|e| (path.clone(), e));
+                let file = File::from_args(
+                    path.clone(),
+                    self.dir,
+                    filename,
+                    self.deref_links,
+                    self.total_size,
+                )
+                .map_err(|e| (path.clone(), e));
 
                 // Windows has its own concept of hidden files, when dotfiles are
                 // hidden Windows hidden files should also be filtered out
@@ -169,12 +180,18 @@ impl<'dir, 'ig> Iterator for Files<'dir, 'ig> {
         match self.dots {
             DotsNext::Dot => {
                 self.dots = DotsNext::DotDot;
-                Some(File::new_aa_current(self.dir).map_err(|e| (Path::new(".").to_path_buf(), e)))
+                Some(
+                    File::new_aa_current(self.dir, self.total_size)
+                        .map_err(|e| (Path::new(".").to_path_buf(), e)),
+                )
             }
 
             DotsNext::DotDot => {
                 self.dots = DotsNext::Files;
-                Some(File::new_aa_parent(self.parent(), self.dir).map_err(|e| (self.parent(), e)))
+                Some(
+                    File::new_aa_parent(self.parent(), self.dir, self.total_size)
+                        .map_err(|e| (self.parent(), e)),
+                )
             }
 
             DotsNext::Files => self.next_visible_file(),

+ 96 - 85
src/fs/file.rs

@@ -1,16 +1,22 @@
 //! Files, and methods and fields to access their metadata.
 
+#[cfg(unix)]
+use std::collections::HashMap;
 use std::io;
 #[cfg(unix)]
 use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt};
 #[cfg(windows)]
 use std::os::windows::fs::MetadataExt;
 use std::path::{Path, PathBuf};
+#[cfg(unix)]
+use std::sync::Mutex;
 use std::sync::OnceLock;
 
 use chrono::prelude::*;
 
 use log::*;
+#[cfg(unix)]
+use once_cell::sync::Lazy;
 
 use crate::fs::dir::Dir;
 use crate::fs::feature::xattr;
@@ -20,6 +26,12 @@ use crate::fs::fields as f;
 use super::mounts::all_mounts;
 use super::mounts::MountedFs;
 
+// Mutex::new is const but HashMap::new is not const requiring us to use lazy
+// initialization. Replace with std::sync::LazyLock when it is stable.
+#[cfg(unix)]
+static DIRECTORY_SIZE_CACHE: Lazy<Mutex<HashMap<(u64, u64), u64>>> =
+    Lazy::new(|| Mutex::new(HashMap::new()));
+
 /// A **File** is a wrapper around one of Rust’s `PathBuf` values, along with
 /// associated data about the file.
 ///
@@ -84,6 +96,11 @@ pub struct File<'dir> {
 
     /// The absolute value of this path, used to look up mount points.
     absolute_path: OnceLock<Option<PathBuf>>,
+
+    pub total_size: bool,
+
+    /// The recursive directory size when total_size is used.
+    recursive_size: Option<u64>,
 }
 
 impl<'dir> File<'dir> {
@@ -92,6 +109,7 @@ impl<'dir> File<'dir> {
         parent_dir: PD,
         filename: FN,
         deref_links: bool,
+        total_size: bool,
     ) -> io::Result<File<'dir>>
     where
         PD: Into<Option<&'dir Dir>>,
@@ -107,7 +125,7 @@ impl<'dir> File<'dir> {
         let extended_attributes = OnceLock::new();
         let absolute_path = OnceLock::new();
 
-        Ok(File {
+        let mut file = File {
             name,
             ext,
             path,
@@ -117,11 +135,23 @@ impl<'dir> File<'dir> {
             deref_links,
             extended_attributes,
             absolute_path,
-        })
+            total_size,
+            recursive_size: None,
+        };
+
+        if total_size {
+            file.recursive_size = file.total_directory_size()
+        }
+
+        Ok(file)
     }
 
-    pub fn new_aa_current(parent_dir: &'dir Dir) -> io::Result<File<'dir>> {
-        let path = parent_dir.path.clone();
+    fn new_aa(
+        path: PathBuf,
+        parent_dir: &'dir Dir,
+        name: &'static str,
+        total_size: bool,
+    ) -> io::Result<File<'dir>> {
         let ext = File::ext(&path);
 
         debug!("Statting file {:?}", &path);
@@ -131,40 +161,37 @@ impl<'dir> File<'dir> {
         let extended_attributes = OnceLock::new();
         let absolute_path = OnceLock::new();
 
-        Ok(File {
+        let mut file = File {
+            name: name.into(),
+            ext,
             path,
-            parent_dir,
             metadata,
-            ext,
-            name: ".".into(),
+            parent_dir,
             is_all_all,
             deref_links: false,
             extended_attributes,
             absolute_path,
-        })
-    }
+            total_size,
+            recursive_size: None,
+        };
 
-    pub fn new_aa_parent(path: PathBuf, parent_dir: &'dir Dir) -> io::Result<File<'dir>> {
-        let ext = File::ext(&path);
+        if total_size {
+            file.recursive_size = file.total_directory_size()
+        }
 
-        debug!("Statting file {:?}", &path);
-        let metadata = std::fs::symlink_metadata(&path)?;
-        let is_all_all = true;
-        let parent_dir = Some(parent_dir);
-        let extended_attributes = OnceLock::new();
-        let absolute_path = OnceLock::new();
+        Ok(file)
+    }
 
-        Ok(File {
-            path,
-            parent_dir,
-            metadata,
-            ext,
-            name: "..".into(),
-            is_all_all,
-            deref_links: false,
-            extended_attributes,
-            absolute_path,
-        })
+    pub fn new_aa_current(parent_dir: &'dir Dir, total_size: bool) -> io::Result<File<'dir>> {
+        File::new_aa(parent_dir.path.clone(), parent_dir, ".", total_size)
+    }
+
+    pub fn new_aa_parent(
+        path: PathBuf,
+        parent_dir: &'dir Dir,
+        total_size: bool,
+    ) -> io::Result<File<'dir>> {
+        File::new_aa(path, parent_dir, "..", total_size)
     }
 
     /// A file’s name is derived from its string. This needs to handle directories
@@ -375,6 +402,8 @@ impl<'dir> File<'dir> {
                     deref_links: self.deref_links,
                     extended_attributes,
                     absolute_path: absolute_path_cell,
+                    total_size: false,
+                    recursive_size: None,
                 };
                 FileTarget::Ok(Box::new(file))
             }
@@ -488,7 +517,8 @@ impl<'dir> File<'dir> {
             }
         }
         if self.is_directory() {
-            f::Size::None
+            self.recursive_size
+                .map_or(f::Size::None, |s| f::Size::Some(s))
         } else if self.is_char_device() || self.is_block_device() {
             let device_id = self.metadata.rdev();
 
@@ -514,59 +544,49 @@ impl<'dir> File<'dir> {
         }
     }
 
-    /// Recursive folder size
+    /// Calculate the total directory size recursively.  If not a directory `None`
+    /// will be returned.  The directory size is cached for recursive directory
+    /// listing.
     #[cfg(unix)]
-    pub fn recursive_size(&self, toplevel: bool) -> f::Size {
-        use crate::fs::RECURSIVE_SIZE_HASHMAP;
+    fn total_directory_size(&self) -> Option<u64> {
         if self.is_directory() {
-            let mut recursive_size: u64 = 0;
-            let Ok(dir) = Dir::read_dir(self.path.clone()) else {
-                return f::Size::None;
-            };
-            let files = dir.files(super::DotFilter::Dotfiles, None, false, false);
-
-            for file in files.flatten() {
-                if file.is_file() {
-                    recursive_size += file.metadata.size();
-                } else {
-                    recursive_size += match file.recursive_size(false) {
-                        f::Size::Some(s) => s,
-                        _ => file.metadata.size(),
-                    };
-                }
-            }
-            if toplevel {
-                RECURSIVE_SIZE_HASHMAP
-                    .lock()
-                    .unwrap()
-                    .insert((self.metadata.dev(), self.metadata.ino()), recursive_size);
+            let key = (self.metadata.dev(), self.metadata.ino());
+            if let Some(size) = DIRECTORY_SIZE_CACHE.lock().unwrap().get(&key) {
+                return Some(*size);
             }
-            f::Size::Some(recursive_size)
-        } else if self.is_char_device() || self.is_block_device() {
-            let device_id = self.metadata.rdev();
-
-            // MacOS and Linux have different arguments and return types for the
-            // functions major and minor.  On Linux the try_into().unwrap() and
-            // the "as u32" cast are not needed.  We turn off the warning to
-            // allow it to compile cleanly on Linux.
-            #[allow(trivial_numeric_casts)]
-            #[allow(clippy::unnecessary_cast)]
-            #[allow(clippy::useless_conversion)]
-            f::Size::DeviceIDs(f::DeviceIDs {
-                // SAFETY: Calling libc function to decompose the device_id
-                major: unsafe { libc::major(device_id.try_into().unwrap()) } as u32,
-                minor: unsafe { libc::minor(device_id.try_into().unwrap()) } as u32,
-            })
-        } else if self.is_link() && self.deref_links {
-            match self.link_target() {
-                FileTarget::Ok(f) => f::Size::Some(f.metadata.size()),
-                _ => f::Size::None,
+            let dir = Dir::read_dir(self.path.clone()).ok()?;
+            let mut size = 0;
+            for file in dir
+                .files(super::DotFilter::Dotfiles, None, false, false, true)
+                .flatten()
+            {
+                size += match file.total_directory_size() {
+                    Some(s) => s,
+                    _ => file.metadata.size(),
+                }
             }
+            DIRECTORY_SIZE_CACHE.lock().unwrap().insert(key, size);
+            Some(size)
         } else {
-            f::Size::Some(self.metadata.len())
+            None
         }
     }
 
+    /// Windows version always returns None.  The metadata for
+    /// `volume_serial_number` and `file_index` are marked unstable so we can
+    /// not cache the sizes.  Without caching we could end up walking the
+    /// directory structure several times.
+    #[cfg(windows)]
+    fn total_directory_size(&self) -> Option<u64> {
+        None
+    }
+
+    /// Returns the same value as `self.metadata.len()` or the recursive size
+    /// of a directory when `total_size` is used.
+    pub fn length(&self) -> u64 {
+        self.recursive_size.unwrap_or_else(|| self.metadata.len())
+    }
+
     /// Returns the size of the file or indicates no size if it's a directory.
     ///
     /// For Windows platforms, the size of directories is not computed and will
@@ -580,15 +600,6 @@ impl<'dir> File<'dir> {
         }
     }
 
-    #[cfg(windows)]
-    pub fn recursive_size(&self, _toplevel: bool) -> f::Size {
-        if self.is_directory() {
-            f::Size::None
-        } else {
-            f::Size::Some(self.metadata.len())
-        }
-    }
-
     /// Determines if the directory is empty or not.
     ///
     /// For Unix platforms, this function first checks the link count to quickly
@@ -643,7 +654,7 @@ impl<'dir> File<'dir> {
         match Dir::read_dir(self.path.clone()) {
             // . & .. are skipped, if the returned iterator has .next(), it's not empty
             Ok(has_files) => has_files
-                .files(super::DotFilter::Dotfiles, None, false, false)
+                .files(super::DotFilter::Dotfiles, None, false, false, false)
                 .next()
                 .is_none(),
             Err(_) => false,

+ 4 - 71
src/fs/filter.rs

@@ -108,14 +108,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 [F], total_size: bool)
+    pub fn sort_files<'a, F>(&self, files: &mut [F])
     where
         F: AsRef<File<'a>>,
     {
-        files.sort_by(|a, b| {
-            self.sort_field
-                .compare_files(a.as_ref(), b.as_ref(), total_size)
-        });
+        files.sort_by(|a, b| self.sort_field.compare_files(a.as_ref(), b.as_ref()));
 
         if self.flags.contains(&FileFilterFlags::Reverse) {
             files.reverse();
@@ -233,10 +230,8 @@ 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`.
-    #[cfg(unix)]
-    pub fn compare_files(self, a: &File<'_>, b: &File<'_>, recursive: bool) -> Ordering {
+    pub fn compare_files(self, a: &File<'_>, b: &File<'_>) -> Ordering {
         use self::SortCase::{ABCabc, AaBbCc};
-        use crate::fs::RECURSIVE_SIZE_HASHMAP;
 
         #[rustfmt::skip]
         return match self {
@@ -245,22 +240,7 @@ impl SortField {
             Self::Name(ABCabc)  => natord::compare(&a.name, &b.name),
             Self::Name(AaBbCc)  => natord::compare_ignore_case(&a.name, &b.name),
 
-            Self::Size => {
-                if recursive {
-                    let recursive_map = RECURSIVE_SIZE_HASHMAP.lock().unwrap();
-                    match recursive_map.get(&(a.metadata.dev(), a.metadata.ino())) {
-                            Some(v) => *v,
-                            _ => a.metadata.len()
-                    }.cmp(
-                        &match recursive_map.get(&(b.metadata.dev(), b.metadata.ino())) {
-                            Some(v) => *v,
-                            _ => b.metadata.len()
-                        }
-                    )
-                } else {
-                    a.metadata.len().cmp(&b.metadata.len())
-                }
-            }
+            Self::Size          => a.length().cmp(&b.length()),
 
             #[cfg(unix)]
             Self::FileInode     => a.metadata.ino().cmp(&b.metadata.ino()),
@@ -302,53 +282,6 @@ impl SortField {
             None => n,
         }
     }
-
-    /// Windows safe version of the above
-    #[cfg(windows)]
-    pub fn compare_files(self, a: &File<'_>, b: &File<'_>, _recursive: bool) -> Ordering {
-        use self::SortCase::{ABCabc, AaBbCc};
-
-        #[rustfmt::skip]
-        return match self {
-            Self::Unsorted  => Ordering::Equal,
-
-            Self::Name(ABCabc)  => natord::compare(&a.name, &b.name),
-            Self::Name(AaBbCc)  => natord::compare_ignore_case(&a.name, &b.name),
-
-            Self::Size => a.metadata.len().cmp(&b.metadata.len()),
-
-            #[cfg(unix)]
-            Self::FileInode     => a.metadata.ino().cmp(&b.metadata.ino()),
-            Self::ModifiedDate  => a.modified_time().cmp(&b.modified_time()),
-            Self::AccessedDate  => a.accessed_time().cmp(&b.accessed_time()),
-            Self::ChangedDate   => a.changed_time().cmp(&b.changed_time()),
-            Self::CreatedDate   => a.created_time().cmp(&b.created_time()),
-            Self::ModifiedAge   => b.modified_time().cmp(&a.modified_time()),  // flip b and a
-            Self::FileType => match a.type_char().cmp(&b.type_char()) { // todo: this recomputes
-                Ordering::Equal  => natord::compare(&a.name, &b.name),
-                order            => order,
-            },
-
-            Self::Extension(ABCabc) => match a.ext.cmp(&b.ext) {
-                Ordering::Equal  => natord::compare(&a.name, &b.name),
-                order            => order,
-            },
-
-            Self::Extension(AaBbCc) => match a.ext.cmp(&b.ext) {
-                Ordering::Equal  => natord::compare_ignore_case(&a.name, &b.name),
-                order            => order,
-            },
-
-            Self::NameMixHidden(ABCabc) => natord::compare(
-                Self::strip_dot(&a.name),
-                Self::strip_dot(&b.name)
-            ),
-            Self::NameMixHidden(AaBbCc) => natord::compare_ignore_case(
-                Self::strip_dot(&a.name),
-                Self::strip_dot(&b.name)
-            ),
-        };
-    }
 }
 
 /// The **ignore patterns** are a list of globs that are tested against

+ 0 - 7
src/fs/mod.rs

@@ -9,10 +9,3 @@ pub mod feature;
 pub mod fields;
 pub mod filter;
 pub mod mounts;
-
-use lazy_static::lazy_static;
-use std::collections::HashMap;
-use std::sync::Mutex;
-lazy_static! {
-    static ref RECURSIVE_SIZE_HASHMAP: Mutex<HashMap<(u64, u64), u64>> = Mutex::new(HashMap::new());
-}

+ 3 - 10
src/main.rs

@@ -187,6 +187,7 @@ impl<'args> Exa<'args> {
                 None,
                 None,
                 self.options.view.deref_links,
+                self.options.view.total_size,
             ) {
                 Err(e) => {
                     exit_status = 2;
@@ -263,6 +264,7 @@ impl<'args> Exa<'args> {
                 self.git.as_ref(),
                 git_ignore,
                 self.options.view.deref_links,
+                self.options.view.total_size,
             ) {
                 match file {
                     Ok(file) => children.push(file),
@@ -271,9 +273,7 @@ impl<'args> Exa<'args> {
             }
 
             self.options.filter.filter_child_files(&mut children);
-            self.options
-                .filter
-                .sort_files(&mut children, self.options.view.total_size);
+            self.options.filter.sort_files(&mut children);
 
             if let Some(recurse_opts) = self.options.dir_action.recurse_options() {
                 let depth = dir
@@ -355,8 +355,6 @@ impl<'args> Exa<'args> {
 
                 let git_ignoring = self.options.filter.git_ignore == GitIgnore::CheckAndIgnore;
                 let git = self.git.as_ref();
-                let total_size = self.options.view.total_size;
-
                 let r = details::Render {
                     dir,
                     files,
@@ -367,7 +365,6 @@ impl<'args> Exa<'args> {
                     filter,
                     git_ignoring,
                     git,
-                    total_size,
                 };
                 r.render(&mut self.writer)
             }
@@ -380,7 +377,6 @@ impl<'args> Exa<'args> {
                 let filter = &self.options.filter;
                 let git_ignoring = self.options.filter.git_ignore == GitIgnore::CheckAndIgnore;
                 let git = self.git.as_ref();
-                let total_size = self.options.view.total_size;
 
                 let r = grid_details::Render {
                     dir,
@@ -394,7 +390,6 @@ impl<'args> Exa<'args> {
                     git_ignoring,
                     git,
                     console_width,
-                    total_size,
                 };
                 r.render(&mut self.writer)
             }
@@ -405,7 +400,6 @@ impl<'args> Exa<'args> {
                 let recurse = self.options.dir_action.recurse_options();
                 let git_ignoring = self.options.filter.git_ignore == GitIgnore::CheckAndIgnore;
                 let git = self.git.as_ref();
-                let total_size = self.options.view.total_size;
 
                 let r = details::Render {
                     dir,
@@ -417,7 +411,6 @@ impl<'args> Exa<'args> {
                     filter,
                     git_ignoring,
                     git,
-                    total_size,
                 };
                 r.render(&mut self.writer)
             }

+ 2 - 2
src/options/flags.rs

@@ -51,7 +51,7 @@ pub static LINKS:       Arg = Arg { short: Some(b'H'), long: "links",       take
 pub static MODIFIED:    Arg = Arg { short: Some(b'm'), long: "modified",    takes_value: TakesValue::Forbidden };
 pub static CHANGED:     Arg = Arg { short: None,       long: "changed",     takes_value: TakesValue::Forbidden };
 pub static BLOCKSIZE:   Arg = Arg { short: Some(b'S'), long: "blocksize",   takes_value: TakesValue::Forbidden };
-pub static TOTALSIZE:   Arg = Arg { short: None,       long: "total-size",  takes_value: TakesValue::Forbidden };
+pub static TOTAL_SIZE:  Arg = Arg { short: None,       long: "total-size",  takes_value: TakesValue::Forbidden };
 pub static TIME:        Arg = Arg { short: Some(b't'), long: "time",        takes_value: TakesValue::Necessary(Some(TIMES)) };
 pub static ACCESSED:    Arg = Arg { short: Some(b'u'), long: "accessed",    takes_value: TakesValue::Forbidden };
 pub static CREATED:     Arg = Arg { short: Some(b'U'), long: "created",     takes_value: TakesValue::Forbidden };
@@ -87,7 +87,7 @@ pub static ALL_ARGS: Args = Args(&[
     &IGNORE_GLOB, &GIT_IGNORE, &ONLY_DIRS, &ONLY_FILES,
 
     &BINARY, &BYTES, &GROUP, &NUMERIC, &HEADER, &ICONS, &INODE, &LINKS, &MODIFIED, &CHANGED,
-    &BLOCKSIZE, &TOTALSIZE, &TIME, &ACCESSED, &CREATED, &TIME_STYLE, &HYPERLINK, &MOUNTS,
+    &BLOCKSIZE, &TOTAL_SIZE, &TIME, &ACCESSED, &CREATED, &TIME_STYLE, &HYPERLINK, &MOUNTS,
     &NO_PERMISSIONS, &NO_FILESIZE, &NO_USER, &NO_TIME, &SMART_GROUP,
 
     &GIT, &NO_GIT, &GIT_REPOS, &GIT_REPOS_NO_STAT,

+ 1 - 1
src/options/help.rs

@@ -65,7 +65,7 @@ LONG VIEW OPTIONS
   -U, --created            use the created timestamp field
   --changed                use the changed timestamp field
   --time-style             how to format timestamps (default, iso, long-iso, full-iso, relative, or a custom style with '+' as prefix. Ex: '+%Y/%m/%d')
-  --total-size              show recursive directory size
+  --total-size             show recursive directory size
   --no-permissions         suppress the permissions field
   -o, --octal-permissions  list each file's permission in octal format
   --no-filesize            suppress the filesize field

+ 1 - 1
src/options/view.rs

@@ -13,7 +13,7 @@ impl View {
     pub fn deduce<V: Vars>(matches: &MatchedFlags<'_>, vars: &V) -> Result<Self, OptionsError> {
         let mode = Mode::deduce(matches, vars)?;
         let deref_links = matches.has(&flags::DEREF_LINKS)?;
-        let total_size = matches.has(&flags::TOTALSIZE)?;
+        let total_size = matches.has(&flags::TOTAL_SIZE)?;
         let width = TerminalWidth::deduce(matches, vars)?;
         let file_style = FileStyle::deduce(matches, vars, width.actual_terminal_width().is_some())?;
         Ok(Self {

+ 3 - 4
src/output/details.rs

@@ -134,8 +134,6 @@ pub struct Render<'a> {
     pub git_ignoring: bool,
 
     pub git: Option<&'a GitCache>,
-
-    pub total_size: bool,
 }
 
 #[rustfmt::skip]
@@ -286,7 +284,7 @@ impl<'a> Render<'a> {
 
                     let table_row = table
                         .as_ref()
-                        .map(|t| t.row_for_file(file, self.show_xattr_hint(file), self.total_size));
+                        .map(|t| t.row_for_file(file, self.show_xattr_hint(file)));
 
                     let mut dir = None;
                     if let Some(r) = self.recurse {
@@ -317,7 +315,7 @@ impl<'a> Render<'a> {
 
         // this is safe because all entries have been initialized above
         let mut file_eggs = unsafe { std::mem::transmute::<_, Vec<Egg<'_>>>(file_eggs) };
-        self.filter.sort_files(&mut file_eggs, self.total_size);
+        self.filter.sort_files(&mut file_eggs);
 
         for (tree_params, egg) in depth.iterate_over(file_eggs.into_iter()) {
             let mut files = Vec::new();
@@ -349,6 +347,7 @@ impl<'a> Render<'a> {
                     self.git,
                     self.git_ignoring,
                     egg.file.deref_links,
+                    egg.file.total_size,
                 ) {
                     match file_to_add {
                         Ok(f) => {

+ 1 - 1
src/output/grid.rs

@@ -41,7 +41,7 @@ impl<'a> Render<'a> {
 
         grid.reserve(self.files.len());
 
-        self.filter.sort_files(&mut self.files, false);
+        self.filter.sort_files(&mut self.files);
         for file in &self.files {
             let filename = self.file_style.for_file(file, self.theme);
 

+ 1 - 7
src/output/grid_details.rs

@@ -85,8 +85,6 @@ pub struct Render<'a> {
     pub git: Option<&'a GitCache>,
 
     pub console_width: usize,
-
-    pub total_size: bool,
 }
 
 impl<'a> Render<'a> {
@@ -108,7 +106,6 @@ impl<'a> Render<'a> {
             filter:        self.filter,
             git_ignoring:  self.git_ignoring,
             git:           self.git,
-            total_size:    self.total_size
         };
     }
 
@@ -128,7 +125,6 @@ impl<'a> Render<'a> {
             filter:        self.filter,
             git_ignoring:  self.git_ignoring,
             git:           self.git,
-            total_size:    self.total_size
         };
     }
 
@@ -157,9 +153,7 @@ impl<'a> Render<'a> {
         let rows = self
             .files
             .iter()
-            .map(|file| {
-                first_table.row_for_file(file, drender.show_xattr_hint(file), self.total_size)
-            })
+            .map(|file| first_table.row_for_file(file, drender.show_xattr_hint(file)))
             .collect::<Vec<_>>();
 
         let file_names = self

+ 1 - 1
src/output/lines.rs

@@ -18,7 +18,7 @@ pub struct Render<'a> {
 
 impl<'a> Render<'a> {
     pub fn render<W: Write>(mut self, w: &mut W) -> io::Result<()> {
-        self.filter.sort_files(&mut self.files, false);
+        self.filter.sort_files(&mut self.files);
         for file in &self.files {
             let name_cell = self.render_file(file);
             writeln!(w, "{}", ANSIStrings(&name_cell))?;

+ 6 - 15
src/output/table.rs

@@ -410,11 +410,11 @@ impl<'a> Table<'a> {
         Row { cells }
     }
 
-    pub fn row_for_file(&self, file: &File<'_>, xattrs: bool, total_size: bool) -> Row {
+    pub fn row_for_file(&self, file: &File<'_>, xattrs: bool) -> Row {
         let cells = self
             .columns
             .iter()
-            .map(|c| self.display(file, *c, xattrs, total_size))
+            .map(|c| self.display(file, *c, xattrs))
             .collect();
 
         Row { cells }
@@ -450,21 +450,12 @@ impl<'a> Table<'a> {
             .map(|p| f::OctalPermissions { permissions: p })
     }
 
-    fn display(&self, file: &File<'_>, column: Column, xattrs: bool, total_size: bool) -> TextCell {
+    fn display(&self, file: &File<'_>, column: Column, xattrs: bool) -> TextCell {
         match column {
             Column::Permissions => self.permissions_plus(file, xattrs).render(self.theme),
-            Column::FileSize => {
-                if total_size {
-                    file.recursive_size(true).render(
-                        self.theme,
-                        self.size_format,
-                        &self.env.numeric,
-                    )
-                } else {
-                    file.size()
-                        .render(self.theme, self.size_format, &self.env.numeric)
-                }
-            }
+            Column::FileSize => file
+                .size()
+                .render(self.theme, self.size_format, &self.env.numeric),
             #[cfg(unix)]
             Column::HardLinks => file.links().render(self.theme, &self.env.numeric),
             #[cfg(unix)]