Add locale option to filesize display

crates/nu-command/src/formats/to/text.rs

@@ -171,7 +171,7 @@ fn local_into_string(
         Value::Bool { val, .. } => val.to_string(),
         Value::Int { val, .. } => val.to_string(),
         Value::Float { val, .. } => val.to_string(),
-        Value::Filesize { val, .. } => config.filesize.display(val).to_string(),
+        Value::Filesize { val, .. } => config.filesize.format(val).to_string(),
         Value::Duration { val, .. } => format_duration(val),
         Value::Date { val, .. } => {
             format!("{} ({})", val.to_rfc2822(), HumanTime::from(val))

crates/nu-command/src/random/binary.rs

@@ -46,7 +46,7 @@ impl Command for SubCommand {
             Value::Filesize { val, .. } => {
                 usize::try_from(val).map_err(|_| ShellError::InvalidValue {
                     valid: "a non-negative int or filesize".into(),
-                    actual: engine_state.get_config().filesize.display(val).to_string(),
+                    actual: engine_state.get_config().filesize.format(val).to_string(),
                     span: length_val.span(),
                 })
             }

crates/nu-command/src/random/chars.rs

@@ -83,7 +83,7 @@ fn chars(
             Value::Filesize { val, .. } => {
                 usize::try_from(val).map_err(|_| ShellError::InvalidValue {
                     valid: "a non-negative int or filesize".into(),
-                    actual: engine_state.get_config().filesize.display(val).to_string(),
+                    actual: engine_state.get_config().filesize.format(val).to_string(),
                     span: length_val.span(),
                 })
             }

crates/nu-command/src/strings/format/filesize.rs

@@ -1,9 +1,9 @@
 use nu_cmd_base::input_handler::{operate, CmdArgument};
 use nu_engine::command_prelude::*;
-use nu_protocol::{engine::StateWorkingSet, FilesizeUnit};
+use nu_protocol::{engine::StateWorkingSet, FilesizeFormat, FilesizeUnit};
 
 struct Arguments {
-    format: FilesizeUnit,
+    unit: FilesizeUnit,
     cell_paths: Option<Vec<CellPath>>,
 }
 
@@ -61,10 +61,10 @@ impl Command for FormatFilesize {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let format = parse_filesize_unit(call.req::<Spanned<String>>(engine_state, stack, 0)?)?;
+        let unit = parse_filesize_unit(call.req::<Spanned<String>>(engine_state, stack, 0)?)?;
         let cell_paths: Vec<CellPath> = call.rest(engine_state, stack, 1)?;
         let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
-        let arg = Arguments { format, cell_paths };
+        let arg = Arguments { unit, cell_paths };
         operate(
             format_value_impl,
             arg,
@@ -80,10 +80,10 @@ impl Command for FormatFilesize {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let format = parse_filesize_unit(call.req_const::<Spanned<String>>(working_set, 0)?)?;
+        let unit = parse_filesize_unit(call.req_const::<Spanned<String>>(working_set, 0)?)?;
         let cell_paths: Vec<CellPath> = call.rest_const(working_set, 1)?;
         let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
-        let arg = Arguments { format, cell_paths };
+        let arg = Arguments { unit, cell_paths };
         operate(
             format_value_impl,
             arg,
@@ -127,7 +127,13 @@ fn parse_filesize_unit(format: Spanned<String>) -> Result<FilesizeUnit, ShellErr
 fn format_value_impl(val: &Value, arg: &Arguments, span: Span) -> Value {
     let value_span = val.span();
     match val {
-        Value::Filesize { val, .. } => Value::string(val.display(arg.format).to_string(), span),
+        Value::Filesize { val, .. } => Value::string(
+            FilesizeFormat::new()
+                .unit(arg.unit)
+                .format(*val)
+                .to_string(),
+            span,
+        ),
         Value::Error { .. } => val.clone(),
         _ => Value::error(
             ShellError::OnlySupportsThisInputType {

crates/nu-protocol/src/config/filesize.rs

@@ -1,43 +1,12 @@
-use super::{config_update_string_enum, prelude::*};
+use super::prelude::*;
-use crate::{DisplayFilesize, Filesize, FilesizeUnit};
+use crate::{Filesize, FilesizeFormat, FilesizeUnitFormat, FormattedFilesize};
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
+impl IntoValue for FilesizeUnitFormat {
-pub enum FilesizeFormatUnit {
-    Metric,
-    Binary,
-    Unit(FilesizeUnit),
-}
-
-impl From<FilesizeUnit> for FilesizeFormatUnit {
-    fn from(unit: FilesizeUnit) -> Self {
-        Self::Unit(unit)
-    }
-}
-
-impl FromStr for FilesizeFormatUnit {
-    type Err = &'static str;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s {
-            "metric" => Ok(Self::Metric),
-            "binary" => Ok(Self::Binary),
-            _ => {
-                if let Ok(unit) = s.parse() {
-                    Ok(Self::Unit(unit))
-                } else {
-                    Err("'metric', 'binary', 'B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', or 'EiB'")
-                }
-            }
-        }
-    }
-}
-
-impl IntoValue for FilesizeFormatUnit {
     fn into_value(self, span: Span) -> Value {
         match self {
-            FilesizeFormatUnit::Metric => "metric",
+            FilesizeUnitFormat::Metric => "metric",
-            FilesizeFormatUnit::Binary => "binary",
+            FilesizeUnitFormat::Binary => "binary",
-            FilesizeFormatUnit::Unit(unit) => unit.as_str(),
+            FilesizeUnitFormat::Unit(unit) => unit.as_str(),
         }
         .into_value(span)
     }
@@ -45,30 +14,37 @@ impl IntoValue for FilesizeFormatUnit {
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
 pub struct FilesizeConfig {
-    pub unit: FilesizeFormatUnit,
+    pub unit: FilesizeUnitFormat,
     pub precision: Option<usize>,
 }
 
 impl FilesizeConfig {
-    pub fn display(&self, filesize: Filesize) -> DisplayFilesize {
+    pub fn as_filesize_format(&self) -> FilesizeFormat {
-        let unit = match self.unit {
+        FilesizeFormat::new()
-            FilesizeFormatUnit::Metric => filesize.largest_metric_unit(),
+            .unit(self.unit)
-            FilesizeFormatUnit::Binary => filesize.largest_binary_unit(),
+            .precision(self.precision)
-            FilesizeFormatUnit::Unit(unit) => unit,
+    }
-        };
+
-        filesize.display(unit).precision(self.precision)
+    pub fn format(&self, filesize: Filesize) -> FormattedFilesize {
+        self.as_filesize_format().format(filesize)
     }
 }
 
 impl Default for FilesizeConfig {
     fn default() -> Self {
         Self {
-            unit: FilesizeFormatUnit::Metric,
+            unit: FilesizeUnitFormat::Metric,
             precision: Some(1),
         }
     }
 }
 
+impl From<FilesizeConfig> for FilesizeFormat {
+    fn from(config: FilesizeConfig) -> Self {
+        config.as_filesize_format()
+    }
+}
+
 impl UpdateFromValue for FilesizeConfig {
     fn update<'a>(
         &mut self,
@@ -84,17 +60,22 @@ impl UpdateFromValue for FilesizeConfig {
         for (col, val) in record.iter() {
             let path = &mut path.push(col);
             match col.as_str() {
-                "unit" => config_update_string_enum(&mut self.unit, val, path, errors),
+                "unit" => {
+                    if let Ok(str) = val.as_str() {
+                        match str.parse() {
+                            Ok(unit) => self.unit = unit,
+                            Err(_) => errors.invalid_value(path, "'metric', 'binary', 'B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', or 'EiB'", val),
+                        }
+                    } else {
+                        errors.type_mismatch(path, Type::String, val)
+                    }
+                }
                 "precision" => match *val {
                     Value::Nothing { .. } => self.precision = None,
                     Value::Int { val, .. } if val >= 0 => self.precision = Some(val as usize),
                     Value::Int { .. } => errors.invalid_value(path, "a non-negative integer", val),
                     _ => errors.type_mismatch(path, Type::custom("int or nothing"), val),
                 },
-                "format" | "metric" => {
-                    // TODO: remove after next release
-                    errors.deprecated_option(path, "set $env.config.filesize.unit", val.span())
-                }
                 _ => errors.unknown_option(path, val),
             }
         }

crates/nu-protocol/src/value/filesize.rs

@@ -1,7 +1,9 @@
 use crate::{FromValue, IntoValue, ShellError, Span, Type, Value};
+use num_format::{Locale, WriteFormatted};
 use serde::{Deserialize, Serialize};
 use std::{
-    fmt,
+    char,
+    fmt::{self, Write},
     iter::Sum,
     ops::{Add, Mul, Neg, Sub},
     str::FromStr,
@@ -138,33 +140,29 @@ impl Filesize {
         }
     }
 
-    /// Returns a struct that can be used to display a [`Filesize`] scaled to the given
+    // /// Returns a struct that can be used to display a [`Filesize`] scaled to the given
-    /// [`FilesizeUnit`].
+    // /// [`FilesizeUnit`].
-    ///
+    // ///
-    /// You can use [`largest_binary_unit`](Filesize::largest_binary_unit) or
+    // /// You can use [`largest_binary_unit`](Filesize::largest_binary_unit) or
-    /// [`largest_metric_unit`](Filesize::largest_metric_unit) to automatically determine a
+    // /// [`largest_metric_unit`](Filesize::largest_metric_unit) to automatically determine a
-    /// [`FilesizeUnit`] of appropriate scale for a specific [`Filesize`].
+    // /// [`FilesizeUnit`] of appropriate scale for a specific [`Filesize`].
-    ///
+    // ///
-    /// The default [`Display`](fmt::Display) implementation for [`Filesize`] is
+    // /// The default [`Display`](fmt::Display) implementation for [`Filesize`] is
-    /// `self.display(self.largest_metric_unit())`.
+    // /// `self.display(self.largest_metric_unit())`.
-    ///
+    // ///
-    /// # Examples
+    // /// # Examples
-    /// ```
+    // /// ```
-    /// # use nu_protocol::{Filesize, FilesizeUnit};
+    // /// # use nu_protocol::{Filesize, FilesizeUnit};
-    /// let filesize = Filesize::from_unit(4, FilesizeUnit::KiB).unwrap();
+    // /// let filesize = Filesize::from_unit(4, FilesizeUnit::KiB).unwrap();
-    ///
+    // ///
-    /// assert_eq!(filesize.display(FilesizeUnit::B).to_string(), "4096 B");
+    // /// assert_eq!(filesize.display(FilesizeUnit::B).to_string(), "4096 B");
-    /// assert_eq!(filesize.display(FilesizeUnit::KiB).to_string(), "4 KiB");
+    // /// assert_eq!(filesize.display(FilesizeUnit::KiB).to_string(), "4 KiB");
-    /// assert_eq!(filesize.display(filesize.largest_binary_unit()).to_string(), "4 KiB");
+    // /// assert_eq!(filesize.display(filesize.largest_binary_unit()).to_string(), "4 KiB");
-    /// assert_eq!(filesize.display(filesize.largest_metric_unit()).to_string(), "4.096 kB");
+    // /// assert_eq!(filesize.display(filesize.largest_metric_unit()).to_string(), "4.096 kB");
-    /// ```
+    // /// ```
-    pub fn display(&self, unit: FilesizeUnit) -> DisplayFilesize {
+    // pub fn display(&self, options: &DisplayFilesizeOptions) -> DisplayFilesize {
-        DisplayFilesize {
+    //     options.display(*self)
-            filesize: *self,
+    // }
-            unit,
-            precision: None,
-        }
-    }
 }
 
 impl From<i64> for Filesize {
@@ -359,7 +357,7 @@ impl Sum<Filesize> for Option<Filesize> {
 
 impl fmt::Display for Filesize {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.display(self.largest_metric_unit()))
+        write!(f, "{}", FilesizeFormat::new().format(*self))
     }
 }
 
@@ -370,7 +368,7 @@ impl fmt::Display for Filesize {
 ///
 /// The number of bytes in a [`FilesizeUnit`] can be obtained using
 /// [`as_bytes`](FilesizeUnit::as_bytes).
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum FilesizeUnit {
     /// One byte
     B,
@@ -522,91 +520,200 @@ impl FromStr for FilesizeUnit {
 
 impl fmt::Display for FilesizeUnit {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.as_str().fmt(f)
+        f.write_str(self.as_str())
     }
 }
 
-#[derive(Debug)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
-pub struct DisplayFilesize {
+pub enum FilesizeUnitFormat {
-    filesize: Filesize,
+    Metric,
-    unit: FilesizeUnit,
+    Binary,
-    precision: Option<usize>,
+    Unit(FilesizeUnit),
 }
 
-impl DisplayFilesize {
+impl FilesizeUnitFormat {
+    pub const fn unit(unit: FilesizeUnit) -> Self {
+        Self::Unit(unit)
+    }
+
+    pub const fn as_str(&self) -> &'static str {
+        match self {
+            Self::Metric => "metric",
+            Self::Binary => "binary",
+            Self::Unit(unit) => unit.as_str(),
+        }
+    }
+
+    /// Returns `true` for [`DisplayFilesizeUnit::Metric`] or if the underlying [`FilesizeUnit`] is metric according to [`FilesizeUnit::is_metric`].
+    ///
+    /// Note that this returns `true` for [`FilesizeUnit::B`] as well.
+    pub const fn is_metric(&self) -> bool {
+        match self {
+            Self::Metric => true,
+            Self::Binary => false,
+            Self::Unit(unit) => unit.is_metric(),
+        }
+    }
+
+    /// Returns `true` for [`DisplayFilesizeUnit::Binary`] or if the underlying [`FilesizeUnit`] is binary according to [`FilesizeUnit::is_binary`].
+    ///
+    /// Note that this returns `true` for [`FilesizeUnit::B`] as well.
+    pub const fn is_binary(&self) -> bool {
+        match self {
+            Self::Metric => false,
+            Self::Binary => true,
+            Self::Unit(unit) => unit.is_binary(),
+        }
+    }
+}
+
+impl From<FilesizeUnit> for FilesizeUnitFormat {
+    fn from(unit: FilesizeUnit) -> Self {
+        Self::unit(unit)
+    }
+}
+
+/// The error returned when failing to parse a [`DisplayFilesizeUnit`].
+///
+/// This occurs when the string being parsed does not exactly match any of:
+/// - `metric`
+/// - `binary`
+/// - the name of any of the enum cases in [`FilesizeUnit`].
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Error)]
+pub struct ParseFilesizeUnitFormatError(());
+
+impl fmt::Display for ParseFilesizeUnitFormatError {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(fmt, "invalid file size unit format")
+    }
+}
+
+impl FromStr for FilesizeUnitFormat {
+    type Err = ParseFilesizeUnitFormatError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s {
+            "metric" => Self::Metric,
+            "binary" => Self::Binary,
+            s => Self::unit(s.parse().map_err(|_| ParseFilesizeUnitFormatError(()))?),
+        })
+    }
+}
+
+impl fmt::Display for FilesizeUnitFormat {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self.as_str())
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+pub struct FilesizeFormat {
+    unit: FilesizeUnitFormat,
+    precision: Option<usize>,
+    locale: Locale,
+}
+
+impl FilesizeFormat {
+    pub fn new() -> Self {
+        FilesizeFormat {
+            unit: FilesizeUnitFormat::Metric,
+            precision: None,
+            locale: Locale::en_US_POSIX,
+        }
+    }
+
+    pub fn unit(mut self, unit: impl Into<FilesizeUnitFormat>) -> Self {
+        self.unit = unit.into();
+        self
+    }
+
     pub fn precision(mut self, precision: impl Into<Option<usize>>) -> Self {
         self.precision = precision.into();
         self
     }
+
+    pub fn locale(mut self, locale: Locale) -> Self {
+        self.locale = locale;
+        self
+    }
+
+    pub fn format(&self, filesize: Filesize) -> FormattedFilesize {
+        FormattedFilesize {
+            format: *self,
+            filesize,
+        }
+    }
 }
 
-impl fmt::Display for DisplayFilesize {
+impl Default for FilesizeFormat {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn default() -> Self {
-        let Self {
+        Self::new()
-            filesize: Filesize(filesize),
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct FormattedFilesize {
+    format: FilesizeFormat,
+    filesize: Filesize,
+}
+
+impl fmt::Display for FormattedFilesize {
+    fn fmt(&self, mut f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Self { filesize, format } = *self;
+        let FilesizeFormat {
             unit,
             precision,
-        } = *self;
+            locale,
+        } = format;
+        let unit = match unit {
+            FilesizeUnitFormat::Metric => filesize.largest_metric_unit(),
+            FilesizeUnitFormat::Binary => filesize.largest_binary_unit(),
+            FilesizeUnitFormat::Unit(unit) => unit,
+        };
+        let Filesize(filesize) = filesize;
         let precision = precision.or(f.precision());
-        match unit {
+
-            FilesizeUnit::B => write!(f, "{filesize} B"),
+        // Format an exact, possibly fractional, string representation of `filesize`.
-            FilesizeUnit::KiB
+        let bytes = unit.as_bytes() as i64;
-            | FilesizeUnit::MiB
+        let whole = filesize / bytes;
-            | FilesizeUnit::GiB
+        let fract = (filesize % bytes).unsigned_abs();
-            | FilesizeUnit::TiB
+
-            | FilesizeUnit::PiB
+        f.write_formatted(&whole, &locale)
-            | FilesizeUnit::EiB => {
+            .map_err(|_| std::fmt::Error)?;
-                // This won't give exact results for large filesizes and/or units.
+
-                let val = filesize as f64 / unit.as_bytes() as f64;
+        if unit != FilesizeUnit::B && precision != Some(0) && !(precision.is_none() && fract == 0) {
-                if let Some(precision) = precision {
+            f.write_str(locale.decimal())?;
-                    write!(f, "{val:.precision$} {unit}")
+
-                } else {
+            let bytes = unit.as_bytes();
-                    write!(f, "{val} {unit}")
+            let mut fract = fract * 10;
+            let mut i = 0;
+            loop {
+                let q = fract / bytes;
+                let r = fract % bytes;
+                // Quick soundness proof:
+                // r <= bytes                by definition of remainder `%`
+                // => 10 * r <= 10 * bytes
+                // => fract <= 10 * bytes    before next iteration, fract = r * 10
+                // => fract / bytes <= 10
+                // => q <= 10                next iteration, q = fract / bytes
+                debug_assert!(q <= 10);
+                f.write_char(char::from_digit(q as u32, 10).expect("q <= 10"))?;
+                i += 1;
+                if r == 0 || precision.is_some_and(|p| i >= p) {
+                    break;
                 }
+                fract = r * 10;
             }
-            FilesizeUnit::KB
-            | FilesizeUnit::GB
-            | FilesizeUnit::MB
-            | FilesizeUnit::TB
-            | FilesizeUnit::PB
-            | FilesizeUnit::EB => {
-                // Format an exact, possibly fractional, string representation of `filesize`.
-                let bytes = unit.as_bytes() as i64;
-                let whole = filesize / bytes;
-                let fract = (filesize % bytes).unsigned_abs();
-                if precision == Some(0) || (precision.is_none() && fract == 0) {
-                    write!(f, "{whole} {unit}")
-                } else {
-                    // fract <= bytes by nature of `%` and bytes <= EB = 10 ^ 18
-                    // So, the longest string for the fractional portion can be 18 characters.
-                    let buf = &mut [b'0'; 18];
-                    let stop = precision.unwrap_or(usize::MAX).min(buf.len());
-                    let bytes = bytes.unsigned_abs();
-                    let mut fract = fract * 10;
-                    let mut i = 0;
-                    loop {
-                        let q = fract / bytes;
-                        let r = fract % bytes;
-                        debug_assert!(q < 10);
-                        buf[i] += q as u8;
-                        i += 1;
-                        if r == 0 || i >= stop {
-                            break;
-                        }
-                        fract = r * 10;
-                    }
 
-                    // Safety: all the characters in `buf` are valid UTF-8.
+            if let Some(precision) = precision {
-                    let fract = unsafe { std::str::from_utf8_unchecked(&buf[..i]) };
+                for _ in 0..(precision - i) {
-
+                    f.write_char('0')?;
-                    if let Some(p) = precision {
-                        write!(f, "{whole}.{fract:0<p$} {unit}")
-                    } else {
-                        write!(f, "{whole}.{fract} {unit}")
-                    }
                 }
             }
         }
+
+        write!(f, " {unit}")?;
+        Ok(())
     }
 }
 
@@ -622,7 +729,13 @@ mod tests {
     #[case(3_000_000, FilesizeUnit::MB, "3 MB")]
     #[case(3_000_000, FilesizeUnit::KB, "3000 kB")]
     fn display_unit(#[case] bytes: i64, #[case] unit: FilesizeUnit, #[case] exp: &str) {
-        assert_eq!(exp, Filesize::new(bytes).display(unit).to_string());
+        assert_eq!(
+            exp,
+            FilesizeFormat::new()
+                .unit(unit)
+                .format(Filesize::new(bytes))
+                .to_string()
+        );
     }
 
     #[rstest]
@@ -630,10 +743,12 @@ mod tests {
     #[case(1024, "1 KiB")]
     #[case(1025, "1.0009765625 KiB")]
     fn display_auto_binary(#[case] val: i64, #[case] exp: &str) {
-        let filesize = Filesize::new(val);
         assert_eq!(
             exp,
-            filesize.display(filesize.largest_binary_unit()).to_string(),
+            FilesizeFormat::new()
+                .unit(FilesizeUnitFormat::Binary)
+                .format(Filesize::new(val))
+                .to_string()
         );
     }
 
@@ -642,10 +757,12 @@ mod tests {
     #[case(1000, "1 kB")]
     #[case(1024, "1.024 kB")]
     fn display_auto_metric(#[case] val: i64, #[case] exp: &str) {
-        let filesize = Filesize::new(val);
         assert_eq!(
             exp,
-            filesize.display(filesize.largest_metric_unit()).to_string(),
+            FilesizeFormat::new()
+                .unit(FilesizeUnitFormat::Metric)
+                .format(Filesize::new(val))
+                .to_string()
         );
     }
 }

crates/nu-protocol/src/value/mod.rs

@@ -936,7 +936,7 @@ impl Value {
             Value::Bool { val, .. } => val.to_string(),
             Value::Int { val, .. } => val.to_string(),
             Value::Float { val, .. } => val.to_string(),
-            Value::Filesize { val, .. } => config.filesize.display(*val).to_string(),
+            Value::Filesize { val, .. } => config.filesize.format(*val).to_string(),
             Value::Duration { val, .. } => format_duration(*val),
             Value::Date { val, .. } => match &config.datetime_format.normal {
                 Some(format) => self.format_datetime(val, format),