Merge pull request #2439 from tertsdiepraam/numfmt/round-and-c-locale

`numfmt`: add `--round` and other minor improvements

Cargo.toml

@@ -63,6 +63,7 @@ feat_common_core = [
     "more",
     "mv",
     "nl",
+    "numfmt",
     "od",
     "paste",
     "pr",
@@ -160,7 +161,6 @@ feat_require_unix = [
     "mkfifo",
     "mknod",
     "nice",
-    "numfmt",
     "nohup",
     "pathchk",
     "stat",

src/uu/numfmt/src/format.rs

@@ -1,7 +1,5 @@
-use crate::options::NumfmtOptions;
-use crate::units::{
-    DisplayableSuffix, RawSuffix, Result, Suffix, Transform, Unit, IEC_BASES, SI_BASES,
-};
+use crate::options::{NumfmtOptions, RoundMethod};
+use crate::units::{DisplayableSuffix, RawSuffix, Result, Suffix, Unit, IEC_BASES, SI_BASES};
 
 /// Iterate over a line's fields, where each field is a contiguous sequence of
 /// non-whitespace, optionally prefixed with one or more characters of leading
@@ -70,18 +68,18 @@ fn parse_suffix(s: &str) -> Result<(f64, Option<Suffix>)> {
     if with_i {
         iter.next_back();
     }
-    let suffix: Option<Suffix> = match iter.next_back() {
-        Some('K') => Ok(Some((RawSuffix::K, with_i))),
-        Some('M') => Ok(Some((RawSuffix::M, with_i))),
-        Some('G') => Ok(Some((RawSuffix::G, with_i))),
-        Some('T') => Ok(Some((RawSuffix::T, with_i))),
-        Some('P') => Ok(Some((RawSuffix::P, with_i))),
-        Some('E') => Ok(Some((RawSuffix::E, with_i))),
-        Some('Z') => Ok(Some((RawSuffix::Z, with_i))),
-        Some('Y') => Ok(Some((RawSuffix::Y, with_i))),
-        Some('0'..='9') => Ok(None),
-        _ => Err(format!("invalid suffix in input: '{}'", s)),
-    }?;
+    let suffix = match iter.next_back() {
+        Some('K') => Some((RawSuffix::K, with_i)),
+        Some('M') => Some((RawSuffix::M, with_i)),
+        Some('G') => Some((RawSuffix::G, with_i)),
+        Some('T') => Some((RawSuffix::T, with_i)),
+        Some('P') => Some((RawSuffix::P, with_i)),
+        Some('E') => Some((RawSuffix::E, with_i)),
+        Some('Z') => Some((RawSuffix::Z, with_i)),
+        Some('Y') => Some((RawSuffix::Y, with_i)),
+        Some('0'..='9') => None,
+        _ => return Err(format!("invalid suffix in input: '{}'", s)),
+    };  
 
     let suffix_len = match suffix {
         None => 0,
@@ -127,44 +125,50 @@ fn remove_suffix(i: f64, s: Option<Suffix>, u: &Unit) -> Result<f64> {
     }
 }
 
-fn transform_from(s: &str, opts: &Transform) -> Result<f64> {
+fn transform_from(s: &str, opts: &Unit) -> Result<f64> {
     let (i, suffix) = parse_suffix(s)?;
 
-    remove_suffix(i, suffix, &opts.unit).map(|n| if n < 0.0 { -n.abs().ceil() } else { n.ceil() })
+    remove_suffix(i, suffix, opts).map(|n| if n < 0.0 { -n.abs().ceil() } else { n.ceil() })
 }
 
-/// Divide numerator by denominator, with ceiling.
+/// Divide numerator by denominator, with rounding.
 ///
-/// If the result of the division is less than 10.0, truncate the result
-/// to the next highest tenth.
+/// If the result of the division is less than 10.0, round to one decimal point.
 ///
-/// Otherwise, truncate the result to the next highest whole number.
+/// Otherwise, round to an integer.
 ///
 /// # Examples:
 ///
 /// ```
-/// use uu_numfmt::format::div_ceil;
+/// use uu_numfmt::format::div_round;
+/// use uu_numfmt::options::RoundMethod;
 ///
-/// assert_eq!(div_ceil(1.01, 1.0), 1.1);
-/// assert_eq!(div_ceil(999.1, 1000.), 1.0);
-/// assert_eq!(div_ceil(1001., 10.), 101.);
-/// assert_eq!(div_ceil(9991., 10.), 1000.);
-/// assert_eq!(div_ceil(-12.34, 1.0), -13.0);
-/// assert_eq!(div_ceil(1000.0, -3.14), -319.0);
-/// assert_eq!(div_ceil(-271828.0, -271.0), 1004.0);
+/// // Rounding methods:
+/// assert_eq!(div_round(1.01, 1.0, RoundMethod::FromZero), 1.1);
+/// assert_eq!(div_round(1.01, 1.0, RoundMethod::TowardsZero), 1.0);
+/// assert_eq!(div_round(1.01, 1.0, RoundMethod::Up), 1.1);
+/// assert_eq!(div_round(1.01, 1.0, RoundMethod::Down), 1.0);
+/// assert_eq!(div_round(1.01, 1.0, RoundMethod::Nearest), 1.0);
+///
+/// // Division:
+/// assert_eq!(div_round(999.1, 1000.0, RoundMethod::FromZero), 1.0);
+/// assert_eq!(div_round(1001., 10., RoundMethod::FromZero), 101.);
+/// assert_eq!(div_round(9991., 10., RoundMethod::FromZero), 1000.);
+/// assert_eq!(div_round(-12.34, 1.0, RoundMethod::FromZero), -13.0);
+/// assert_eq!(div_round(1000.0, -3.14, RoundMethod::FromZero), -319.0);
+/// assert_eq!(div_round(-271828.0, -271.0, RoundMethod::FromZero), 1004.0);
 /// ```
-pub fn div_ceil(n: f64, d: f64) -> f64 {
-    let v = n / (d / 10.0);
-    let (v, sign) = if v < 0.0 { (v.abs(), -1.0) } else { (v, 1.0) };
+pub fn div_round(n: f64, d: f64, method: RoundMethod) -> f64 {
+    let v = n / d;
 
-    if v < 100.0 {
-        v.ceil() / 10.0 * sign
+    if v.abs() < 10.0 {
+        method.round(10.0 * v) / 10.0
     } else {
-        (v / 10.0).ceil() * sign
+        method.round(v)
     }
 }
 
-fn consider_suffix(n: f64, u: &Unit) -> Result<(f64, Option<Suffix>)> {
+fn consider_suffix(n: f64, u: &Unit, round_method: RoundMethod) -> Result<(f64, Option<Suffix>)> {
     use crate::units::RawSuffix::*;
 
     let abs_n = n.abs();
@@ -190,7 +194,7 @@ fn consider_suffix(n: f64, u: &Unit) -> Result<(f64, Option<Suffix>)> {
         _ => return Err("Number is too big and unsupported".to_string()),
     };
 
-    let v = div_ceil(n, bases[i]);
+    let v = div_round(n, bases[i], round_method);
 
     // check if rounding pushed us into the next base
     if v.abs() >= bases[1] {
@@ -200,8 +204,8 @@ fn consider_suffix(n: f64, u: &Unit) -> Result<(f64, Option<Suffix>)> {
     }
 }
 
-fn transform_to(s: f64, opts: &Transform) -> Result<String> {
-    let (i2, s) = consider_suffix(s, &opts.unit)?;
+fn transform_to(s: f64, opts: &Unit, round_method: RoundMethod) -> Result<String> {
+    let (i2, s) = consider_suffix(s, opts, round_method)?;
     Ok(match s {
         None => format!("{}", i2),
         Some(s) if i2.abs() < 10.0 => format!("{:.1}{}", i2, DisplayableSuffix(s)),
@@ -217,10 +221,11 @@ fn format_string(
     let number = transform_to(
         transform_from(source, &options.transform.from)?,
         &options.transform.to,
+        options.round,
     )?;
 
     Ok(match implicit_padding.unwrap_or(options.padding) {
-        p if p == 0 => number,
+        0 => number,
         p if p > 0 => format!("{:>padding$}", number, padding = p as usize),
         p => format!("{:<padding$}", number, padding = p.abs() as usize),
     })

src/uu/numfmt/src/numfmt.rs

@@ -5,18 +5,20 @@
 //  * For the full copyright and license information, please view the LICENSE
 //  * file that was distributed with this source code.
 
+// spell-checker:ignore N'th M'th
+
 #[macro_use]
 extern crate uucore;
 
 use crate::format::format_and_print;
 use crate::options::*;
-use crate::units::{Result, Transform, Unit};
+use crate::units::{Result, Unit};
 use clap::{crate_version, App, AppSettings, Arg, ArgMatches};
 use std::io::{BufRead, Write};
 use uucore::ranges::Range;
 
 pub mod format;
-mod options;
+pub mod options;
 mod units;
 
 static ABOUT: &str = "Convert numbers from/to human-readable strings";
@@ -92,10 +94,7 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
     let from = parse_unit(args.value_of(options::FROM).unwrap())?;
     let to = parse_unit(args.value_of(options::TO).unwrap())?;
 
-    let transform = TransformOptions {
-        from: Transform { unit: from },
-        to: Transform { unit: to },
-    };
+    let transform = TransformOptions { from, to };
 
     let padding = match args.value_of(options::PADDING) {
         Some(s) => s.parse::<isize>().map_err(|err| err.to_string()),
@@ -118,13 +117,12 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
         }
     }?;
 
-    let fields = match args.value_of(options::FIELD) {
-        Some("-") => vec![Range {
+    let fields = match args.value_of(options::FIELD).unwrap() {
+        "-" => vec![Range {
             low: 1,
             high: std::usize::MAX,
         }],
-        Some(v) => Range::from_list(v)?,
-        None => unreachable!(),
+        v => Range::from_list(v)?,
     };
 
     let delimiter = args.value_of(options::DELIMITER).map_or(Ok(None), |arg| {
@@ -135,12 +133,23 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
         }
     })?;
 
+    // unwrap is fine because the argument has a default value
+    let round = match args.value_of(options::ROUND).unwrap() {
+        "up" => RoundMethod::Up,
+        "down" => RoundMethod::Down,
+        "from-zero" => RoundMethod::FromZero,
+        "towards-zero" => RoundMethod::TowardsZero,
+        "nearest" => RoundMethod::Nearest,
+        _ => unreachable!("Should be restricted by clap"),
+    };
+
     Ok(NumfmtOptions {
         transform,
         padding,
         header,
         fields,
         delimiter,
+        round,
     })
 }
 
@@ -203,6 +212,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .default_value(options::HEADER_DEFAULT)
                 .hide_default_value(true),
         )
+        .arg(
+            Arg::with_name(options::ROUND)
+                .long(options::ROUND)
+                .help(
+                    "use METHOD for rounding when scaling; METHOD can be: up,\
+                    down, from-zero (default), towards-zero, nearest",
+                )
+                .value_name("METHOD")
+                .default_value("from-zero")
+                .possible_values(&["up", "down", "from-zero", "towards-zero", "nearest"]),
+        )
         .arg(Arg::with_name(options::NUMBER).hidden(true).multiple(true))
         .get_matches_from(args);
 

src/uu/numfmt/src/options.rs

@@ -1,4 +1,4 @@
-use crate::units::Transform;
+use crate::units::Unit;
 use uucore::ranges::Range;
 
 pub const DELIMITER: &str = "delimiter";
@@ -10,12 +10,13 @@ pub const HEADER: &str = "header";
 pub const HEADER_DEFAULT: &str = "1";
 pub const NUMBER: &str = "NUMBER";
 pub const PADDING: &str = "padding";
+pub const ROUND: &str = "round";
 pub const TO: &str = "to";
 pub const TO_DEFAULT: &str = "none";
 
 pub struct TransformOptions {
-    pub from: Transform,
-    pub to: Transform,
+    pub from: Unit,
+    pub to: Unit,
 }
 
 pub struct NumfmtOptions {
@@ -24,4 +25,38 @@ pub struct NumfmtOptions {
     pub header: usize,
     pub fields: Vec<Range>,
     pub delimiter: Option<String>,
+    pub round: RoundMethod,
+}
+
+#[derive(Clone, Copy)]
+pub enum RoundMethod {
+    Up,
+    Down,
+    FromZero,
+    TowardsZero,
+    Nearest,
+}
+
+impl RoundMethod {
+    pub fn round(&self, f: f64) -> f64 {
+        match self {
+            RoundMethod::Up => f.ceil(),
+            RoundMethod::Down => f.floor(),
+            RoundMethod::FromZero => {
+                if f < 0.0 {
+                    f.floor()
+                } else {
+                    f.ceil()
+                }
+            }
+            RoundMethod::TowardsZero => {
+                if f < 0.0 {
+                    f.ceil()
+                } else {
+                    f.floor()
+                }
+            }
+            RoundMethod::Nearest => f.round(),
+        }
+    }
 }

src/uu/numfmt/src/units.rs

@@ -24,10 +24,6 @@ pub enum Unit {
     None,
 }
 
-pub struct Transform {
-    pub unit: Unit,
-}
-
 pub type Result<T> = std::result::Result<T, String>;
 
 #[derive(Clone, Copy, Debug)]

tests/by-util/test_numfmt.rs

@@ -481,3 +481,27 @@ fn test_delimiter_with_padding_and_fields() {
         .succeeds()
         .stdout_only(" 1.0K| 2.0K\n");
 }
+
+#[test]
+fn test_round() {
+    for (method, exp) in &[
+        ("from-zero", ["9.1K", "-9.1K", "9.1K", "-9.1K"]),
+        ("towards-zero", ["9.0K", "-9.0K", "9.0K", "-9.0K"]),
+        ("up", ["9.1K", "-9.0K", "9.1K", "-9.0K"]),
+        ("down", ["9.0K", "-9.1K", "9.0K", "-9.1K"]),
+        ("nearest", ["9.0K", "-9.0K", "9.1K", "-9.1K"]),
+    ] {
+        new_ucmd!()
+            .args(&[
+                "--to=si",
+                &format!("--round={}", method),
+                "--",
+                "9001",
+                "-9001",
+                "9099",
+                "-9099",
+            ])
+            .succeeds()
+            .stdout_only(exp.join("\n") + "\n");
+    }
+}