standardize on how to get file size (#2992)

* standardize on how to get file size

* forgot to remove comment

* make specified size lowercase

* fix the test due to precision

* added another test

* Update README.md

add contributors graphic

* clippy - test adjustment

* tweaked matching

README.md

@@ -318,7 +318,6 @@ Thanks to all the people who already contributed!
 <a href="https://github.com/nushell/nushell/graphs/contributors">
   <img src="https://contributors-img.web.app/image?repo=nushell/nushell" />
 </a>
-
 ## License
 
 The project is made available under the MIT license. See the `LICENSE` file for more information.

crates/nu-command/src/commands/format/format_filesize.rs

@@ -1,16 +1,13 @@
 use crate::prelude::*;
-use nu_errors::ShellError;
+use nu_data::base::shape::InlineShape;
-
 use nu_engine::WholeStreamCommand;
+use nu_errors::ShellError;
 use nu_protocol::{
-    ColumnPath, Primitive::Filesize, ReturnSuccess, Signature, SyntaxShape, UntaggedValue,
+    ColumnPath, Primitive::Filesize, ReturnSuccess, Signature, SyntaxShape, UntaggedValue, Value,
-    UntaggedValue::Primitive, Value,
 };
 use nu_source::Tagged;
 use nu_value_ext::get_data_by_column_path;
 
-use num_format::{Locale, ToFormattedString};
-
 pub struct FileSize;
 
 #[derive(Deserialize)]
@@ -71,12 +68,25 @@ async fn process_row(
     Ok({
         let replace_for = get_data_by_column_path(&input, &field, move |_, _, error| error);
         match replace_for {
-            Ok(s) => match convert_bytes_to_string_using_format(s, format) {
+            Ok(s) => {
-                Ok(b) => OutputStream::one(ReturnSuccess::value(
+                if let Value {
-                    input.replace_data_at_column_path(&field, b).expect("Given that the existence check was already done, this shouldn't trigger never"),
+                    value: UntaggedValue::Primitive(Filesize(fs)),
-                )),
+                    ..
-                Err(e) => OutputStream::one(Err(e)),
+                } = s
-            },
+                {
+                    let byte_format = InlineShape::format_bytes(&fs, Some(&format.item));
+                    let byte_value = Value::from(byte_format.1);
+                    OutputStream::one(ReturnSuccess::value(
+                        input.replace_data_at_column_path(&field, byte_value).expect("Given that the existence check was already done, this shouldn't trigger never"),
+                    ))
+                } else {
+                    return Err(ShellError::labeled_error(
+                        "the data in this row is not of the type filesize",
+                        "invalid datatype in row",
+                        input.tag(),
+                    ));
+                }
+            }
             Err(e) => OutputStream::one(Err(e)),
         }
     })
@@ -102,86 +112,6 @@ async fn filesize(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
         .to_output_stream())
 }
 
-fn convert_bytes_to_string_using_format(
-    bytes: Value,
-    format: Tagged<String>,
-) -> Result<Value, ShellError> {
-    match bytes.value {
-        Primitive(Filesize(b)) => {
-            if let Some(value) = b.to_u128() {
-                let byte = byte_unit::Byte::from_bytes(value);
-                let value = match format.item().to_lowercase().as_str() {
-                    "b" => Ok(UntaggedValue::string(
-                        value.to_formatted_string(&Locale::en),
-                    )),
-                    "kb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::KB).to_string(),
-                    )),
-                    "kib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::KiB).to_string(),
-                    )),
-                    "mb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::MB).to_string(),
-                    )),
-                    "mib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::MiB).to_string(),
-                    )),
-                    "gb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::GB).to_string(),
-                    )),
-                    "gib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::GiB).to_string(),
-                    )),
-                    "tb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::TB).to_string(),
-                    )),
-                    "tib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::TiB).to_string(),
-                    )),
-                    "pb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::PB).to_string(),
-                    )),
-                    "pib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::PiB).to_string(),
-                    )),
-                    "eb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::EB).to_string(),
-                    )),
-                    "eib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::EiB).to_string(),
-                    )),
-                    "zb" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::ZB).to_string(),
-                    )),
-                    "zib" => Ok(UntaggedValue::string(
-                        byte.get_adjusted_unit(byte_unit::ByteUnit::ZiB).to_string(),
-                    )),
-                    _ => Err(ShellError::labeled_error(
-                        format!("Invalid format code: {:}", format.item()),
-                        "invalid format",
-                        format.tag(),
-                    )),
-                };
-                match value {
-                    Ok(b) => Ok(Value { value: b, ..bytes }),
-                    Err(e) => Err(e),
-                }
-            } else {
-                Err(ShellError::labeled_error(
-                    "Value too large to fit in 128 bits",
-                    "value too large to fit in format",
-                    format.span(),
-                ))
-            }
-        }
-        _ => Err(ShellError::labeled_error(
-            "the data in this row is not of the type filesize",
-            "invalid row type",
-            bytes.tag(),
-        )),
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::FileSize;

crates/nu-command/src/commands/str_/from.rs

@@ -132,7 +132,7 @@ pub fn action(
             Primitive::Date(a_date) => a_date.format("%c").to_string(),
             Primitive::FilePath(a_filepath) => a_filepath.as_path().display().to_string(),
             Primitive::Filesize(a_filesize) => {
-                let byte_string = InlineShape::format_bytes(a_filesize);
+                let byte_string = InlineShape::format_bytes(a_filesize, None);
                 byte_string.1
             }
             _ => {

crates/nu-command/tests/commands/format.rs

@@ -1,4 +1,4 @@
-use nu_test_support::fs::Stub::EmptyFile;
+use nu_test_support::fs::Stub::{EmptyFile, FileWithContentToBeTrimmed};
 use nu_test_support::playground::Playground;
 use nu_test_support::{nu, pipeline};
 
@@ -61,6 +61,33 @@ fn format_filesize_works() {
             "#
         ));
 
-        assert_eq!(actual.out, "0.01 KB");
+        assert_eq!(actual.out, "0.0 KB");
     })
 }
+
+#[test]
+fn format_filesize_works_with_nonempty_files() {
+    Playground::setup(
+        "format_filesize_works_with_nonempty_files",
+        |dirs, sandbox| {
+            sandbox.with_files(vec![FileWithContentToBeTrimmed(
+                "sample.toml",
+                r#"
+                    [dependency]
+                    name = "nu"
+                "#,
+            )]);
+
+            let actual = nu!(
+                cwd: dirs.test(),
+                "ls sample.toml | format filesize size B | get size | first"
+            );
+
+            #[cfg(not(windows))]
+            assert_eq!(actual.out, "25");
+
+            #[cfg(windows)]
+            assert_eq!(actual.out, "27");
+        },
+    )
+}

crates/nu-data/src/base/shape.rs

@@ -128,15 +128,21 @@ impl InlineShape {
         }
     }
 
-    pub fn format_bytes(bytesize: &BigInt) -> (DbgDocBldr, String) {
+    pub fn format_bytes(bytesize: &BigInt, forced_format: Option<&str>) -> (DbgDocBldr, String) {
         use bigdecimal::ToPrimitive;
 
         // get the config value, if it doesn't exist make it 'auto' so it works how it originally did
-        let filesize_format_var = crate::config::config(Tag::unknown())
+        let filesize_format_var;
+        if let Some(fmt) = forced_format {
+            filesize_format_var = fmt.to_ascii_lowercase();
+        } else {
+            filesize_format_var = crate::config::config(Tag::unknown())
                 .expect("unable to get the config.toml file")
                 .get("filesize_format")
                 .map(|val| val.convert_to_string().to_ascii_lowercase())
                 .unwrap_or_else(|| "auto".to_string());
+        }
+
         // if there is a value, match it to one of the valid values for byte units
         let filesize_format = match filesize_format_var.as_str() {
             "b" => (byte_unit::ByteUnit::B, ""),
@@ -233,7 +239,7 @@ impl PrettyDebug for FormatInlineShape {
                     + right.clone().format().pretty()
             }
             InlineShape::Bytesize(bytesize) => {
-                let bytes = InlineShape::format_bytes(bytesize);
+                let bytes = InlineShape::format_bytes(bytesize, None);
                 bytes.0
             }
             InlineShape::String(string) => DbgDocBldr::primitive(string),