numfmt: add round and use C locale style for errors

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
@@ -62,7 +60,7 @@ impl<'a> Iterator for WhitespaceSplitter<'a> {
 
 fn parse_suffix(s: &str) -> Result<(f64, Option<Suffix>)> {
     if s.is_empty() {
-        return Err("invalid number: ‘’".to_string());
+        return Err("invalid number: ''".to_string());
     }
 
     let with_i = s.ends_with('i');
@@ -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,
@@ -91,7 +89,7 @@ fn parse_suffix(s: &str) -> Result<(f64, Option<Suffix>)> {
 
     let number = s[..s.len() - suffix_len]
         .parse::<f64>()
-        .map_err(|_| format!("invalid number: ‘{}’", s))?;
+        .map_err(|_| format!("invalid number: '{}'", s))?;
 
     Ok((number, suffix))
 }
@@ -127,10 +125,10 @@ 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.
@@ -153,18 +151,17 @@ fn transform_from(s: &str, opts: &Transform) -> Result<f64> {
 /// assert_eq!(div_ceil(1000.0, -3.14), -319.0);
 /// assert_eq!(div_ceil(-271828.0, -271.0), 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 +187,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 +197,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 +214,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

@@ -10,7 +10,7 @@ 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;
@@ -92,10 +92,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()),
@@ -114,17 +111,16 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
                     0 => Err(value),
                     _ => Ok(n),
                 })
-                .map_err(|value| format!("invalid header value ‘{}’", value))
+                .map_err(|value| format!("invalid header value '{}'", value))
         }
     }?;
 
-    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 +131,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 +210,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

@@ -35,7 +35,7 @@ fn test_from_iec_i_requires_suffix() {
     new_ucmd!()
         .args(&["--from=iec-i", "1024"])
         .fails()
-        .stderr_is("numfmt: missing 'i' suffix in input: ‘1024’ (e.g Ki/Mi/Gi)");
+        .stderr_is("numfmt: missing 'i' suffix in input: '1024' (e.g Ki/Mi/Gi)");
 }
 
 #[test]
@@ -123,7 +123,7 @@ fn test_header_error_if_non_numeric() {
     new_ucmd!()
         .args(&["--header=two"])
         .run()
-        .stderr_is("numfmt: invalid header value ‘two’");
+        .stderr_is("numfmt: invalid header value 'two'");
 }
 
 #[test]
@@ -131,7 +131,7 @@ fn test_header_error_if_0() {
     new_ucmd!()
         .args(&["--header=0"])
         .run()
-        .stderr_is("numfmt: invalid header value ‘0’");
+        .stderr_is("numfmt: invalid header value '0'");
 }
 
 #[test]
@@ -139,7 +139,7 @@ fn test_header_error_if_negative() {
     new_ucmd!()
         .args(&["--header=-3"])
         .run()
-        .stderr_is("numfmt: invalid header value ‘-3’");
+        .stderr_is("numfmt: invalid header value '-3'");
 }
 
 #[test]
@@ -187,7 +187,7 @@ fn test_should_report_invalid_empty_number_on_empty_stdin() {
         .args(&["--from=auto"])
         .pipe_in("\n")
         .run()
-        .stderr_is("numfmt: invalid number: ‘’\n");
+        .stderr_is("numfmt: invalid number: ''\n");
 }
 
 #[test]
@@ -196,7 +196,7 @@ fn test_should_report_invalid_empty_number_on_blank_stdin() {
         .args(&["--from=auto"])
         .pipe_in("  \t  \n")
         .run()
-        .stderr_is("numfmt: invalid number: ‘’\n");
+        .stderr_is("numfmt: invalid number: ''\n");
 }
 
 #[test]
@@ -205,14 +205,14 @@ fn test_should_report_invalid_suffix_on_stdin() {
         .args(&["--from=auto"])
         .pipe_in("1k")
         .run()
-        .stderr_is("numfmt: invalid suffix in input: ‘1k’\n");
+        .stderr_is("numfmt: invalid suffix in input: '1k'\n");
 
     // GNU numfmt reports this one as “invalid number”
     new_ucmd!()
         .args(&["--from=auto"])
         .pipe_in("NaN")
         .run()
-        .stderr_is("numfmt: invalid suffix in input: ‘NaN’\n");
+        .stderr_is("numfmt: invalid suffix in input: 'NaN'\n");
 }
 
 #[test]
@@ -222,7 +222,7 @@ fn test_should_report_invalid_number_with_interior_junk() {
         .args(&["--from=auto"])
         .pipe_in("1x0K")
         .run()
-        .stderr_is("numfmt: invalid number: ‘1x0K’\n");
+        .stderr_is("numfmt: invalid number: '1x0K'\n");
 }
 
 #[test]
@@ -461,7 +461,7 @@ fn test_delimiter_overrides_whitespace_separator() {
         .args(&["-d,"])
         .pipe_in("1 234,56")
         .fails()
-        .stderr_is("numfmt: invalid number: ‘1 234’\n");
+        .stderr_is("numfmt: invalid number: '1 234'\n");
 }
 
 #[test]
@@ -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");
+    }
+}