ls: use uucore parse_block_size for block size resolution

Author: sylvestre

src/uu/ls/src/config.rs

@@ -19,8 +19,9 @@ use term_grid::SPACES_IN_TAB;
 
 use uucore::{
     display::Quotable, error::UResult, format::human::SizeFormat, fsext::MetadataTimeField,
-    line_ending::LineEnding, parser::parse_glob, parser::parse_size::parse_size_non_zero_u64,
-    quoting_style::QuotingStyle, show_error, show_warning, time::format, translate,
+    line_ending::LineEnding, parser::parse_block_size, parser::parse_glob,
+    parser::parse_size::parse_size_non_zero_u64, quoting_style::QuotingStyle, show_error,
+    show_warning, time::format, translate,
 };
 
 use crate::{
@@ -123,6 +124,49 @@ const POSIXLY_CORRECT_BLOCK_SIZE: u64 = 512;
 const DEFAULT_BLOCK_SIZE: u64 = 1024;
 const DEFAULT_FILE_SIZE_BLOCK_SIZE: u64 = 1;
 
+/// Resolve `(file_size_block_size, block_size)` from environment variables.
+///
+/// `LS_BLOCK_SIZE` and `BLOCK_SIZE` affect both values.
+/// `BLOCKSIZE` only affects `block_size` (allocation display with `-s`).
+/// `POSIXLY_CORRECT` sets `block_size` to 512 as a last resort.
+/// `-k` (`opt_kb`) forces `block_size` to `DEFAULT_BLOCK_SIZE`.
+fn resolve_block_sizes_from_env(opt_kb: bool) -> (u64, u64) {
+    match parse_block_size::block_size_from_env(&["LS_BLOCK_SIZE", "BLOCK_SIZE"]) {
+        parse_block_size::BlockSizeEnv::Found(size) => {
+            if opt_kb {
+                (size, DEFAULT_BLOCK_SIZE)
+            } else {
+                (size, size)
+            }
+        }
+        parse_block_size::BlockSizeEnv::SetButInvalid => (DEFAULT_BLOCK_SIZE, DEFAULT_BLOCK_SIZE),
+        parse_block_size::BlockSizeEnv::NotSet => {
+            // Neither LS_BLOCK_SIZE nor BLOCK_SIZE was set; check BLOCKSIZE
+            // which only affects allocation display, not file size.
+            match parse_block_size::block_size_from_env(&["BLOCKSIZE"]) {
+                parse_block_size::BlockSizeEnv::Found(size) => {
+                    if opt_kb {
+                        (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
+                    } else {
+                        (DEFAULT_FILE_SIZE_BLOCK_SIZE, size)
+                    }
+                }
+                parse_block_size::BlockSizeEnv::SetButInvalid => {
+                    // BLOCKSIZE was set but invalid: stop lookup, use defaults.
+                    (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
+                }
+                parse_block_size::BlockSizeEnv::NotSet => {
+                    if std::env::var_os("POSIXLY_CORRECT").is_some() && !opt_kb {
+                        (DEFAULT_FILE_SIZE_BLOCK_SIZE, POSIXLY_CORRECT_BLOCK_SIZE)
+                    } else {
+                        (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
+                    }
+                }
+            }
+        }
+    }
+}
+
 pub(crate) enum Dereference {
     None,
     DirArgs,
@@ -704,61 +748,23 @@ impl Config {
             SizeFormat::Bytes
         };
 
-        let env_var_blocksize = std::env::var_os("BLOCKSIZE");
-        let env_var_block_size = std::env::var_os("BLOCK_SIZE");
-        let env_var_ls_block_size = std::env::var_os("LS_BLOCK_SIZE");
-        let env_var_posixly_correct = std::env::var_os("POSIXLY_CORRECT");
-        let mut is_env_var_blocksize = false;
-
-        let raw_block_size = if let Some(opt_block_size) = opt_block_size {
-            OsString::from(opt_block_size)
-        } else if let Some(env_var_ls_block_size) = env_var_ls_block_size {
-            env_var_ls_block_size
-        } else if let Some(env_var_block_size) = env_var_block_size {
-            env_var_block_size
-        } else if let Some(env_var_blocksize) = env_var_blocksize {
-            is_env_var_blocksize = true;
-            env_var_blocksize
-        } else {
-            OsString::from("")
-        };
-
-        let (file_size_block_size, block_size) = if !opt_si && !opt_hr && !raw_block_size.is_empty()
-        {
-            if let Ok(size) = parse_size_non_zero_u64(&raw_block_size.to_string_lossy()) {
-                match (is_env_var_blocksize, opt_kb) {
-                    (true, true) => (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE),
-                    (true, false) => (DEFAULT_FILE_SIZE_BLOCK_SIZE, size),
-                    (false, true) => {
-                        // --block-size overrides -k
-                        if opt_block_size.is_some() {
-                            (size, size)
-                        } else {
-                            (size, DEFAULT_BLOCK_SIZE)
-                        }
-                    }
-                    (false, false) => (size, size),
-                }
-            } else {
-                // only fail if invalid block size was specified with --block-size,
-                // ignore invalid block size from env vars
-                if let Some(invalid_block_size) = opt_block_size {
-                    return Err(Box::new(LsError::BlockSizeParseError(
-                        invalid_block_size.clone(),
-                    )));
-                }
-                if is_env_var_blocksize {
-                    (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
-                } else {
-                    (DEFAULT_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
-                }
-            }
-        } else if env_var_posixly_correct.is_some() {
-            if opt_kb {
+        let (file_size_block_size, block_size) = if let Some(opt_block_size) = opt_block_size {
+            // --block-size command-line argument: parse it, error on invalid
+            // If --block-size=si or --block-size=human-readable, skip numeric parsing
+            if opt_si {
+                (DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000)
+            } else if opt_hr {
                 (DEFAULT_FILE_SIZE_BLOCK_SIZE, DEFAULT_BLOCK_SIZE)
+            } else if let Ok(size) = parse_size_non_zero_u64(opt_block_size) {
+                // --block-size overrides -k
+                (size, size)
             } else {
-                (DEFAULT_FILE_SIZE_BLOCK_SIZE, POSIXLY_CORRECT_BLOCK_SIZE)
+                return Err(Box::new(LsError::BlockSizeParseError(
+                    opt_block_size.clone(),
+                )));
             }
+        } else if !opt_si && !opt_hr {
+            resolve_block_sizes_from_env(opt_kb)
         } else if opt_si {
             (DEFAULT_FILE_SIZE_BLOCK_SIZE, 1000)
         } else {