ls: refactor out padding calculations (#3072)

* Refactor padding calculations into a function

* Propagate all write and (most) flush errors

src/uu/ls/src/ls.rs

@@ -324,16 +324,16 @@ struct LongFormat {
 
 struct PaddingCollection {
     #[cfg(unix)]
-    longest_inode_len: usize,
+    inode: usize,
-    longest_link_count_len: usize,
+    link_count: usize,
-    longest_uname_len: usize,
+    uname: usize,
-    longest_group_len: usize,
+    group: usize,
-    longest_context_len: usize,
+    context: usize,
-    longest_size_len: usize,
+    size: usize,
     #[cfg(unix)]
-    longest_major_len: usize,
+    major: usize,
     #[cfg(unix)]
-    longest_minor_len: usize,
+    minor: usize,
 }
 
 impl Config {
@@ -1305,9 +1305,9 @@ only ignore '.' and '..'.",
         )
 }
 
-/// Represents a Path along with it's associated data
+/// Represents a Path along with it's associated data.
-/// Any data that will be reused several times makes sense to be added to this structure
+/// Any data that will be reused several times makes sense to be added to this structure.
-/// Caching data here helps eliminate redundant syscalls to fetch same information
+/// Caching data here helps eliminate redundant syscalls to fetch same information.
 #[derive(Debug)]
 struct PathData {
     // Result<MetaData> got from symlink_metadata() or metadata() based on config
@@ -1708,92 +1708,10 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
     // option, print the security context to the left of the size column.
 
     if config.format == Format::Long {
-        #[cfg(unix)]
+        let padding_collection = calculate_padding_collection(items, config, out);
-        let (
-            mut longest_inode_len,
-            mut longest_link_count_len,
-            mut longest_uname_len,
-            mut longest_group_len,
-            mut longest_context_len,
-            mut longest_size_len,
-            mut longest_major_len,
-            mut longest_minor_len,
-        ) = (1, 1, 1, 1, 1, 1, 1, 1);
-
-        #[cfg(not(unix))]
-        let (
-            mut longest_link_count_len,
-            mut longest_uname_len,
-            mut longest_group_len,
-            mut longest_context_len,
-            mut longest_size_len,
-        ) = (1, 1, 1, 1, 1);
-
-        #[cfg(unix)]
-        for item in items {
-            let context_len = item.security_context.len();
-            let (link_count_len, uname_len, group_len, size_len, major_len, minor_len, inode_len) =
-                display_dir_entry_size(item, config, out);
-            longest_inode_len = inode_len.max(longest_inode_len);
-            longest_link_count_len = link_count_len.max(longest_link_count_len);
-            longest_uname_len = uname_len.max(longest_uname_len);
-            longest_group_len = group_len.max(longest_group_len);
-            if config.context {
-                longest_context_len = context_len.max(longest_context_len);
-            }
-            if items.len() == 1usize {
-                longest_size_len = 0usize;
-                longest_major_len = 0usize;
-                longest_minor_len = 0usize;
-            } else {
-                longest_major_len = major_len.max(longest_major_len);
-                longest_minor_len = minor_len.max(longest_minor_len);
-                longest_size_len = size_len
-                    .max(longest_size_len)
-                    .max(longest_major_len + longest_minor_len + 2usize);
-            }
-        }
-
-        #[cfg(not(unix))]
-        for item in items {
-            let context_len = item.security_context.len();
-            let (
-                link_count_len,
-                uname_len,
-                group_len,
-                size_len,
-                _major_len,
-                _minor_len,
-                _inode_len,
-            ) = display_dir_entry_size(item, config, out);
-            longest_link_count_len = link_count_len.max(longest_link_count_len);
-            longest_uname_len = uname_len.max(longest_uname_len);
-            longest_group_len = group_len.max(longest_group_len);
-            if config.context {
-                longest_context_len = context_len.max(longest_context_len);
-            }
-            longest_size_len = size_len.max(longest_size_len);
-        }
 
         for item in items {
-            display_item_long(
+            display_item_long(item, &padding_collection, config, out);
-                item,
-                &PaddingCollection {
-                    #[cfg(unix)]
-                    longest_inode_len,
-                    longest_link_count_len,
-                    longest_uname_len,
-                    longest_group_len,
-                    longest_context_len,
-                    longest_size_len,
-                    #[cfg(unix)]
-                    longest_major_len,
-                    #[cfg(unix)]
-                    longest_minor_len,
-                },
-                config,
-                out,
-            );
         }
     } else {
         let mut longest_context_len = 1;
@@ -1966,11 +1884,7 @@ fn display_item_long(
         #[cfg(unix)]
         {
             if config.inode {
-                let _ = write!(
+                let _ = write!(out, "{} ", pad_left(&get_inode(md), padding.inode),);
-                    out,
-                    "{} ",
-                    pad_left(&get_inode(md), padding.longest_inode_len),
-                );
             }
         }
 
@@ -1985,14 +1899,14 @@ fn display_item_long(
             } else {
                 ""
             },
-            pad_left(&display_symlink_count(md), padding.longest_link_count_len),
+            pad_left(&display_symlink_count(md), padding.link_count),
         );
 
         if config.long.owner {
             let _ = write!(
                 out,
                 " {}",
-                pad_right(&display_uname(md, config), padding.longest_uname_len),
+                pad_right(&display_uname(md, config), padding.uname),
             );
         }
 
@@ -2000,7 +1914,7 @@ fn display_item_long(
             let _ = write!(
                 out,
                 " {}",
-                pad_right(&display_group(md, config), padding.longest_group_len),
+                pad_right(&display_group(md, config), padding.group),
             );
         }
 
@@ -2008,7 +1922,7 @@ fn display_item_long(
             let _ = write!(
                 out,
                 " {}",
-                pad_right(&item.security_context, padding.longest_context_len),
+                pad_right(&item.security_context, padding.context),
             );
         }
 
@@ -2018,13 +1932,13 @@ fn display_item_long(
             let _ = write!(
                 out,
                 " {}",
-                pad_right(&display_uname(md, config), padding.longest_uname_len),
+                pad_right(&display_uname(md, config), padding.uname),
             );
         }
 
         match display_size_or_rdev(md, config) {
             SizeOrDeviceId::Size(size) => {
-                let _ = write!(out, " {}", pad_left(&size, padding.longest_size_len),);
+                let _ = write!(out, " {}", pad_left(&size, padding.size),);
             }
             SizeOrDeviceId::Device(major, minor) => {
                 let _ = write!(
@@ -2035,10 +1949,10 @@ fn display_item_long(
                         #[cfg(not(unix))]
                         0usize,
                         #[cfg(unix)]
-                        padding.longest_major_len.max(
+                        padding.major.max(
                             padding
-                                .longest_size_len
+                                .size
-                                .saturating_sub(padding.longest_minor_len.saturating_add(2usize))
+                                .saturating_sub(padding.minor.saturating_add(2usize))
                         )
                     ),
                     pad_left(
@@ -2046,7 +1960,7 @@ fn display_item_long(
                         #[cfg(not(unix))]
                         0usize,
                         #[cfg(unix)]
-                        padding.longest_minor_len,
+                        padding.minor,
                     ),
                 );
             }
@@ -2060,7 +1974,7 @@ fn display_item_long(
         #[cfg(unix)]
         {
             if config.inode {
-                let _ = write!(out, "{} ", pad_left("?", padding.longest_inode_len),);
+                let _ = write!(out, "{} ", pad_left("?", padding.inode),);
             }
         }
 
@@ -2108,29 +2022,29 @@ fn display_item_long(
             } else {
                 ""
             },
-            pad_left("?", padding.longest_link_count_len),
+            pad_left("?", padding.link_count),
         );
 
         if config.long.owner {
-            let _ = write!(out, " {}", pad_right("?", padding.longest_uname_len));
+            let _ = write!(out, " {}", pad_right("?", padding.uname));
         }
 
         if config.long.group {
-            let _ = write!(out, " {}", pad_right("?", padding.longest_group_len));
+            let _ = write!(out, " {}", pad_right("?", padding.group));
         }
 
         if config.context {
             let _ = write!(
                 out,
                 " {}",
-                pad_right(&item.security_context, padding.longest_context_len)
+                pad_right(&item.security_context, padding.context)
             );
         }
 
         // Author is only different from owner on GNU/Hurd, so we reuse
         // the owner, since GNU/Hurd is not currently supported by Rust.
         if config.long.author {
-            let _ = write!(out, " {}", pad_right("?", padding.longest_uname_len));
+            let _ = write!(out, " {}", pad_right("?", padding.uname));
         }
 
         let dfn = display_file_name(item, config, None, 0, out).contents;
@@ -2139,7 +2053,7 @@ fn display_item_long(
         let _ = writeln!(
             out,
             " {} {} {}",
-            pad_left("?", padding.longest_size_len),
+            pad_left("?", padding.size),
             pad_left("?", date_len),
             dfn,
         );
@@ -2594,3 +2508,77 @@ fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) -
         substitute_string
     }
 }
+
+#[cfg(unix)]
+fn calculate_padding_collection(
+    items: &[PathData],
+    config: &Config,
+    out: &mut BufWriter<Stdout>,
+) -> PaddingCollection {
+    let mut padding_collections = PaddingCollection {
+        inode: 1,
+        link_count: 1,
+        uname: 1,
+        group: 1,
+        context: 1,
+        size: 1,
+        major: 1,
+        minor: 1,
+    };
+
+    for item in items {
+        let context_len = item.security_context.len();
+        let (link_count_len, uname_len, group_len, size_len, major_len, minor_len, inode_len) =
+            display_dir_entry_size(item, config, out);
+        padding_collections.inode = inode_len.max(padding_collections.inode);
+        padding_collections.link_count = link_count_len.max(padding_collections.link_count);
+        padding_collections.uname = uname_len.max(padding_collections.uname);
+        padding_collections.group = group_len.max(padding_collections.group);
+        if config.context {
+            padding_collections.context = context_len.max(padding_collections.context);
+        }
+        if items.len() == 1usize {
+            padding_collections.size = 0usize;
+            padding_collections.major = 0usize;
+            padding_collections.minor = 0usize;
+        } else {
+            padding_collections.major = major_len.max(padding_collections.major);
+            padding_collections.minor = minor_len.max(padding_collections.minor);
+            padding_collections.size = size_len
+                .max(padding_collections.size)
+                .max(padding_collections.major + padding_collections.minor + 2usize);
+        }
+    }
+
+    padding_collections
+}
+
+#[cfg(not(unix))]
+fn calculate_padding_collection(
+    items: &[PathData],
+    config: &Config,
+    out: &mut BufWriter<Stdout>,
+) -> PaddingCollection {
+    let mut padding_collections = PaddingCollection {
+        link_count: 1,
+        uname: 1,
+        group: 1,
+        context: 1,
+        size: 1,
+    };
+
+    for item in items {
+        let context_len = item.security_context.len();
+        let (link_count_len, uname_len, group_len, size_len, _major_len, _minor_len, _inode_len) =
+            display_dir_entry_size(item, config, out);
+        padding_collections.link_count = link_count_len.max(padding_collections.link_count);
+        padding_collections.uname = uname_len.max(padding_collections.uname);
+        padding_collections.group = group_len.max(padding_collections.group);
+        if config.context {
+            padding_collections.context = context_len.max(padding_collections.context);
+        }
+        padding_collections.size = size_len.max(padding_collections.size);
+    }
+
+    padding_collections
+}