diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index ee692d8f0..f0f1bf739 100644 --- a/src/uu/numfmt/src/format.rs +++ b/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)> { 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)> { if with_i { iter.next_back(); } - let suffix: Option = 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)> { let number = s[..s.len() - suffix_len] .parse::() - .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, u: &Unit) -> Result { } } -fn transform_from(s: &str, opts: &Transform) -> Result { +fn transform_from(s: &str, opts: &Unit) -> Result { 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 { /// 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)> { +fn consider_suffix(n: f64, u: &Unit, round_method: RoundMethod) -> Result<(f64, Option)> { use crate::units::RawSuffix::*; let abs_n = n.abs(); @@ -190,7 +187,7 @@ fn consider_suffix(n: f64, u: &Unit) -> Result<(f64, Option)> { _ => 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)> { } } -fn transform_to(s: f64, opts: &Transform) -> Result { - let (i2, s) = consider_suffix(s, &opts.unit)?; +fn transform_to(s: f64, opts: &Unit, round_method: RoundMethod) -> Result { + 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!("{: Result { 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::().map_err(|err| err.to_string()), @@ -114,17 +111,16 @@ fn parse_options(args: &ArgMatches) -> Result { 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 { } })?; + // 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); diff --git a/src/uu/numfmt/src/options.rs b/src/uu/numfmt/src/options.rs index 17f0a6fbe..59bf9d8d3 100644 --- a/src/uu/numfmt/src/options.rs +++ b/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, pub delimiter: Option, + 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(), + } + } } diff --git a/src/uu/numfmt/src/units.rs b/src/uu/numfmt/src/units.rs index 5f9907bdf..8a2895ab7 100644 --- a/src/uu/numfmt/src/units.rs +++ b/src/uu/numfmt/src/units.rs @@ -24,10 +24,6 @@ pub enum Unit { None, } -pub struct Transform { - pub unit: Unit, -} - pub type Result = std::result::Result; #[derive(Clone, Copy, Debug)] diff --git a/tests/by-util/test_numfmt.rs b/tests/by-util/test_numfmt.rs index bb29d431e..336b0f7cd 100644 --- a/tests/by-util/test_numfmt.rs +++ b/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"); + } +}