Merge pull request #3191 from jfinkels/df-block-size-heading

df: fix block size header for multiples of 1024

src/uu/df/src/blocks.rs

@@ -0,0 +1,197 @@
+//  * This file is part of the uutils coreutils package.
+//  *
+//  * For the full copyright and license information, please view the LICENSE
+//  * file that was distributed with this source code.
+//! Types for representing and displaying block sizes.
+use crate::{OPT_BLOCKSIZE, OPT_HUMAN_READABLE, OPT_HUMAN_READABLE_2};
+use clap::ArgMatches;
+use std::fmt;
+use std::num::ParseIntError;
+
+/// The first ten powers of 1024.
+const IEC_BASES: [u128; 10] = [
+    1,
+    1_024,
+    1_048_576,
+    1_073_741_824,
+    1_099_511_627_776,
+    1_125_899_906_842_624,
+    1_152_921_504_606_846_976,
+    1_180_591_620_717_411_303_424,
+    1_208_925_819_614_629_174_706_176,
+    1_237_940_039_285_380_274_899_124_224,
+];
+
+/// Suffixes for the first nine multi-byte unit suffixes.
+const SUFFIXES: [char; 9] = ['B', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y'];
+
+/// Convert a multiple of 1024 into a string like "12K" or "34M".
+///
+/// # Examples
+///
+/// Powers of 1024 become "1K", "1M", "1G", etc.
+///
+/// ```rust,ignore
+/// assert_eq!(to_magnitude_and_suffix_1024(1024).unwrap(), "1K");
+/// assert_eq!(to_magnitude_and_suffix_1024(1024 * 1024).unwrap(), "1M");
+/// assert_eq!(to_magnitude_and_suffix_1024(1024 * 1024 * 1024).unwrap(), "1G");
+/// ```
+///
+/// Multiples of those powers affect the magnitude part of the
+/// returned string:
+///
+/// ```rust,ignore
+/// assert_eq!(to_magnitude_and_suffix_1024(123 * 1024).unwrap(), "123K");
+/// assert_eq!(to_magnitude_and_suffix_1024(456 * 1024 * 1024).unwrap(), "456M");
+/// assert_eq!(to_magnitude_and_suffix_1024(789 * 1024 * 1024 * 1024).unwrap(), "789G");
+/// ```
+fn to_magnitude_and_suffix_1024(n: u128) -> Result<String, ()> {
+    // Find the smallest power of 1024 that is larger than `n`. That
+    // number indicates which units and suffix to use.
+    for i in 0..IEC_BASES.len() - 1 {
+        if n < IEC_BASES[i + 1] {
+            return Ok(format!("{}{}", n / IEC_BASES[i], SUFFIXES[i]));
+        }
+    }
+    Err(())
+}
+
+/// Convert a number into a magnitude and a multi-byte unit suffix.
+///
+/// # Errors
+///
+/// If the number is too large to represent.
+fn to_magnitude_and_suffix(n: u128) -> Result<String, ()> {
+    if n % 1024 == 0 {
+        to_magnitude_and_suffix_1024(n)
+    } else {
+        // TODO Implement this, probably using code from `numfmt`.
+        Ok("1kB".into())
+    }
+}
+
+/// A block size to use in condensing the display of a large number of bytes.
+///
+/// The [`BlockSize::Bytes`] variant represents a static block
+/// size. The [`BlockSize::HumanReadableDecimal`] and
+/// [`BlockSize::HumanReadableBinary`] variants represent dynamic
+/// block sizes: as the number of bytes increases, the divisor
+/// increases as well (for example, from 1 to 1,000 to 1,000,000 and
+/// so on in the case of [`BlockSize::HumanReadableDecimal`]).
+///
+/// The default variant is `Bytes(1024)`.
+pub(crate) enum BlockSize {
+    /// A fixed number of bytes.
+    ///
+    /// The number must be positive.
+    Bytes(u64),
+
+    /// Use the largest divisor corresponding to a unit, like B, K, M, G, etc.
+    ///
+    /// This variant represents powers of 1,000. Contrast with
+    /// [`BlockSize::HumanReadableBinary`], which represents powers of
+    /// 1,024.
+    HumanReadableDecimal,
+
+    /// Use the largest divisor corresponding to a unit, like B, K, M, G, etc.
+    ///
+    /// This variant represents powers of 1,024. Contrast with
+    /// [`BlockSize::HumanReadableDecimal`], which represents powers
+    /// of 1,000.
+    HumanReadableBinary,
+}
+
+impl Default for BlockSize {
+    fn default() -> Self {
+        Self::Bytes(1024)
+    }
+}
+
+pub(crate) fn block_size_from_matches(matches: &ArgMatches) -> Result<BlockSize, ParseIntError> {
+    if matches.is_present(OPT_HUMAN_READABLE) {
+        Ok(BlockSize::HumanReadableBinary)
+    } else if matches.is_present(OPT_HUMAN_READABLE_2) {
+        Ok(BlockSize::HumanReadableDecimal)
+    } else if matches.is_present(OPT_BLOCKSIZE) {
+        let s = matches.value_of(OPT_BLOCKSIZE).unwrap();
+        Ok(BlockSize::Bytes(s.parse()?))
+    } else {
+        Ok(Default::default())
+    }
+}
+
+impl fmt::Display for BlockSize {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::HumanReadableBinary => write!(f, "Size"),
+            Self::HumanReadableDecimal => write!(f, "Size"),
+            Self::Bytes(n) => match to_magnitude_and_suffix(*n as u128) {
+                Ok(s) => write!(f, "{}-blocks", s),
+                Err(_) => Err(fmt::Error),
+            },
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+
+    use crate::blocks::{to_magnitude_and_suffix, BlockSize};
+
+    #[test]
+    fn test_to_magnitude_and_suffix_powers_of_1024() {
+        assert_eq!(to_magnitude_and_suffix(1024).unwrap(), "1K");
+        assert_eq!(to_magnitude_and_suffix(2048).unwrap(), "2K");
+        assert_eq!(to_magnitude_and_suffix(4096).unwrap(), "4K");
+        assert_eq!(to_magnitude_and_suffix(1024 * 1024).unwrap(), "1M");
+        assert_eq!(to_magnitude_and_suffix(2 * 1024 * 1024).unwrap(), "2M");
+        assert_eq!(to_magnitude_and_suffix(1024 * 1024 * 1024).unwrap(), "1G");
+        assert_eq!(
+            to_magnitude_and_suffix(34 * 1024 * 1024 * 1024).unwrap(),
+            "34G"
+        );
+    }
+
+    // TODO We have not yet implemented this behavior, but when we do,
+    // uncomment this test.
+
+    // #[test]
+    // fn test_to_magnitude_and_suffix_not_powers_of_1024() {
+    //     assert_eq!(to_magnitude_and_suffix(1).unwrap(), "1B");
+    //     assert_eq!(to_magnitude_and_suffix(999).unwrap(), "999B");
+
+    //     assert_eq!(to_magnitude_and_suffix(1000).unwrap(), "1kB");
+    //     assert_eq!(to_magnitude_and_suffix(1001).unwrap(), "1.1kB");
+    //     assert_eq!(to_magnitude_and_suffix(1023).unwrap(), "1.1kB");
+    //     assert_eq!(to_magnitude_and_suffix(1025).unwrap(), "1.1kB");
+    //     assert_eq!(to_magnitude_and_suffix(999_000).unwrap(), "999kB");
+
+    //     assert_eq!(to_magnitude_and_suffix(999_001).unwrap(), "1MB");
+    //     assert_eq!(to_magnitude_and_suffix(999_999).unwrap(), "1MB");
+    //     assert_eq!(to_magnitude_and_suffix(1_000_000).unwrap(), "1MB");
+    //     assert_eq!(to_magnitude_and_suffix(1_000_001).unwrap(), "1.1MB");
+    //     assert_eq!(to_magnitude_and_suffix(1_100_000).unwrap(), "1.1MB");
+    //     assert_eq!(to_magnitude_and_suffix(1_100_001).unwrap(), "1.2MB");
+    //     assert_eq!(to_magnitude_and_suffix(1_900_000).unwrap(), "1.9MB");
+    //     assert_eq!(to_magnitude_and_suffix(1_900_001).unwrap(), "2MB");
+    //     assert_eq!(to_magnitude_and_suffix(9_900_000).unwrap(), "9.9MB");
+    //     assert_eq!(to_magnitude_and_suffix(9_900_001).unwrap(), "10MB");
+    //     assert_eq!(to_magnitude_and_suffix(999_000_000).unwrap(), "999MB");
+
+    //     assert_eq!(to_magnitude_and_suffix(999_000_001).unwrap(), "1GB");
+    //     assert_eq!(to_magnitude_and_suffix(1_000_000_000).unwrap(), "1GB");
+    //     // etc.
+    // }
+
+    #[test]
+    fn test_block_size_display() {
+        assert_eq!(format!("{}", BlockSize::HumanReadableBinary), "Size");
+        assert_eq!(format!("{}", BlockSize::HumanReadableDecimal), "Size");
+        assert_eq!(format!("{}", BlockSize::Bytes(1024)), "1K-blocks");
+        assert_eq!(format!("{}", BlockSize::Bytes(2 * 1024)), "2K-blocks");
+        assert_eq!(
+            format!("{}", BlockSize::Bytes(3 * 1024 * 1024)),
+            "3M-blocks"
+        );
+    }
+}

src/uu/df/src/df.rs

@@ -6,21 +6,25 @@
 // For the full copyright and license information, please view the LICENSE file
 // that was distributed with this source code.
 // spell-checker:ignore itotal iused iavail ipcent pcent tmpfs squashfs
+mod blocks;
 mod table;
 
+use uucore::error::{UResult, USimpleError};
+use uucore::format_usage;
 #[cfg(unix)]
 use uucore::fsext::statfs;
 use uucore::fsext::{read_fs_list, FsUsage, MountInfo};
-use uucore::{error::UResult, format_usage};
 
 use clap::{crate_version, App, AppSettings, Arg, ArgMatches};
 
 use std::collections::HashSet;
+use std::fmt;
 use std::iter::FromIterator;
 
 #[cfg(windows)]
 use std::path::Path;
 
+use crate::blocks::{block_size_from_matches, BlockSize};
 use crate::table::{DisplayRow, Header, Row};
 
 static ABOUT: &str = "Show information about the file system on which each FILE resides,\n\
@@ -57,55 +61,6 @@ struct FsSelector {
     exclude: HashSet<String>,
 }
 
-/// A block size to use in condensing the display of a large number of bytes.
-///
-/// The [`BlockSize::Bytes`] variant represents a static block
-/// size. The [`BlockSize::HumanReadableDecimal`] and
-/// [`BlockSize::HumanReadableBinary`] variants represent dynamic
-/// block sizes: as the number of bytes increases, the divisor
-/// increases as well (for example, from 1 to 1,000 to 1,000,000 and
-/// so on in the case of [`BlockSize::HumanReadableDecimal`]).
-///
-/// The default variant is `Bytes(1024)`.
-enum BlockSize {
-    /// A fixed number of bytes.
-    ///
-    /// The number must be positive.
-    Bytes(u64),
-
-    /// Use the largest divisor corresponding to a unit, like B, K, M, G, etc.
-    ///
-    /// This variant represents powers of 1,000. Contrast with
-    /// [`BlockSize::HumanReadableBinary`], which represents powers of
-    /// 1,024.
-    HumanReadableDecimal,
-
-    /// Use the largest divisor corresponding to a unit, like B, K, M, G, etc.
-    ///
-    /// This variant represents powers of 1,024. Contrast with
-    /// [`BlockSize::HumanReadableDecimal`], which represents powers
-    /// of 1,000.
-    HumanReadableBinary,
-}
-
-impl Default for BlockSize {
-    fn default() -> Self {
-        Self::Bytes(1024)
-    }
-}
-
-impl From<&ArgMatches> for BlockSize {
-    fn from(matches: &ArgMatches) -> Self {
-        if matches.is_present(OPT_HUMAN_READABLE) {
-            Self::HumanReadableBinary
-        } else if matches.is_present(OPT_HUMAN_READABLE_2) {
-            Self::HumanReadableDecimal
-        } else {
-            Self::default()
-        }
-    }
-}
-
 /// Parameters that control the behavior of `df`.
 ///
 /// Most of these parameters control which rows and which columns are
@@ -125,19 +80,36 @@ struct Options {
     show_total: bool,
 }
 
+enum OptionsError {
+    InvalidBlockSize,
+}
+
+impl fmt::Display for OptionsError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            // TODO This should include the raw string provided as the argument.
+            //
+            // TODO This needs to vary based on whether `--block-size`
+            // or `-B` were provided.
+            Self::InvalidBlockSize => write!(f, "invalid --block-size argument"),
+        }
+    }
+}
+
 impl Options {
     /// Convert command-line arguments into [`Options`].
-    fn from(matches: &ArgMatches) -> Self {
-        Self {
+    fn from(matches: &ArgMatches) -> Result<Self, OptionsError> {
+        Ok(Self {
             show_local_fs: matches.is_present(OPT_LOCAL),
             show_all_fs: matches.is_present(OPT_ALL),
             show_listed_fs: false,
             show_fs_type: matches.is_present(OPT_PRINT_TYPE),
             show_inode_instead: matches.is_present(OPT_INODES),
-            block_size: BlockSize::from(matches),
+            block_size: block_size_from_matches(matches)
+                .map_err(|_| OptionsError::InvalidBlockSize)?,
             fs_selector: FsSelector::from(matches),
             show_total: matches.is_present(OPT_TOTAL),
-        }
+        })
     }
 }
 
@@ -307,7 +279,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         }
     }
 
-    let opt = Options::from(&matches);
+    let opt = Options::from(&matches).map_err(|e| USimpleError::new(1, format!("{}", e)))?;
 
     let mounts = read_fs_list();
     let data: Vec<Row> = filter_mount_list(mounts, &paths, &opt)

src/uu/df/src/table.rs

@@ -306,13 +306,12 @@ impl fmt::Display for Header<'_> {
             write!(f, "{0: >12} ", "IFree")?;
             write!(f, "{0: >5} ", "IUse%")?;
         } else {
-            // TODO Support arbitrary positive scaling factors (from
-            // the `--block-size` command-line argument).
-            if let BlockSize::Bytes(_) = self.options.block_size {
-                write!(f, "{0: >12} ", "1k-blocks")?;
-            } else {
-                write!(f, "{0: >12} ", "Size")?;
-            };
+            // `Display` is implemented for `BlockSize`, but `Display`
+            // only works when formatting an object into an empty
+            // format, `{}`. So we use `format!()` first to create the
+            // string, then use `write!()` to align the string and pad
+            // with spaces.
+            write!(f, "{0: >12} ", format!("{}", self.options.block_size))?;
             write!(f, "{0: >12} ", "Used")?;
             write!(f, "{0: >12} ", "Available")?;
             #[cfg(target_os = "macos")]
@@ -335,7 +334,7 @@ mod tests {
         let options = Default::default();
         assert_eq!(
             Header::new(&options).to_string(),
-            "Filesystem          1k-blocks         Used    Available  Use% Mounted on       "
+            "Filesystem          1K-blocks         Used    Available  Use% Mounted on       "
         );
     }
 
@@ -347,7 +346,7 @@ mod tests {
         };
         assert_eq!(
             Header::new(&options).to_string(),
-            "Filesystem       Type     1k-blocks         Used    Available  Use% Mounted on       "
+            "Filesystem       Type     1K-blocks         Used    Available  Use% Mounted on       "
         );
     }
 
@@ -363,6 +362,18 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_header_display_block_size_1024() {
+        let options = Options {
+            block_size: BlockSize::Bytes(3 * 1024),
+            ..Default::default()
+        };
+        assert_eq!(
+            Header::new(&options).to_string(),
+            "Filesystem          3K-blocks         Used    Available  Use% Mounted on       "
+        );
+    }
+
     #[test]
     fn test_header_display_human_readable_binary() {
         let options = Options {