From 93e121782ccab575382dc20fb3bf7754403b38ec Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Thu, 23 Jan 2025 06:24:51 +0000 Subject: [PATCH] Improve and fix filesize formatting/display (#14397) # Description This PR cleans up the code surrounding formatting and displaying file sizes. - The `byte_unit` crate we use for file size units displays kilobytes as `KB`, which is not the SI or ISO/IEC standard. Rather it should be `kB`, so this fixes #8872. On some systems, `KB` actually means `KiB`, so this avoids any potential confusion. - The `byte_unit` crate, when displaying file sizes, casts integers to floats which will lose precision for large file sizes. This PR adds a custom `Display` implementation for `Filesize` that can give an exact string representation of a `Filesize` for metric/SI units. - This PR also removes the dependency on the `byte_unit` crate which brought in several other dependencies. Additionally, this PR makes some changes to the config for filesize formatting (`$env.config.filesize`). - The previous filesize config had the `metric` and `format` options. If a metric (SI) unit was set in `format`, but `metric` was set to false, then the `metric` option would take precedence and convert `format` to the corresponding binary unit (or vice versa). E.g., `{ format: kB, metric: false }` => `KiB`. Instead, this PR adds the `unit` option to replace the `format` and `metric` options. `unit` can be set to a fixed file size unit like `kB` or `KiB`, or it can be set to one of the special options: `binary` or `metric`. These options tells nushell to format file sizes using an appropriately scaled metric or binary unit (examples below). ```nushell # precision = null # unit = kB 1kB # 1 kB 1KiB # 1.024 kB # unit = KiB 1kB # 0.9765625 KiB 1KiB # 1 KiB # unit = metric 1000B # 1 kB 1024B # 1.024 kB 10_000MB # 10 GB 10_240MiB # 10.73741824 GB # unit = binary 1000B # 1000 B 1024B # 1 KiB 10_000MB # 9.313225746154785 GiB 10_240MiB # 10 GiB ``` - In addition, this PR also adds the `precision` option to the filesize config. It determines how many digits to show after the decimal point. If set to null, then everything after the decimal point is shown. - The default filesize config is `{ unit: metric, precision: 1 }`. # User-Facing Changes - Commands that use the config to format file sizes will follow the changes described above (e.g., `table`, `into string`, `to text`, etc.). - The file size unit/format passed to `format filesize` is now case sensitive. An error with the valid units is shown if the case does not match. - `$env.config.filesize.format` and `$env.config.filesize.metric` are deprecated and replaced by `$env.config.filesize.unit`. - A new `$env.config.filesize.precision` option was added. # Tests + Formatting Mostly updated test expected outputs. # After Submitting This PR does not change the way NUON serializes file sizes, because that would require changing the nu parser to be able to losslessly decode the new, exact string representation introduced in this PR. Similarly, this PR also does not change the file size parsing in any way. Although the file size units provided to `format filesize` or the filesize config are now case-sensitive, the same is not yet true for file size literals in nushell code. --- Cargo.lock | 225 +----------- .../nu-command/src/conversions/into/string.rs | 4 +- crates/nu-command/src/formats/to/text.rs | 6 +- crates/nu-command/src/random/binary.rs | 3 +- crates/nu-command/src/random/chars.rs | 3 +- .../nu-command/src/strings/format/filesize.rs | 44 +-- crates/nu-command/tests/commands/format.rs | 8 +- .../tests/commands/into_filesize.rs | 69 ++-- crates/nu-command/tests/commands/math/mod.rs | 45 +-- crates/nu-command/tests/commands/ucp.rs | 2 +- crates/nu-protocol/Cargo.toml | 1 - crates/nu-protocol/src/config/filesize.rs | 90 ++++- crates/nu-protocol/src/value/filesize.rs | 326 ++++++++++++------ crates/nu-protocol/src/value/mod.rs | 2 +- crates/nu-protocol/tests/into_config.rs | 14 +- crates/nu-protocol/tests/test_config.rs | 38 +- crates/nu-test-support/src/playground/play.rs | 2 +- .../nu-utils/src/default_files/doc_config.nu | 29 +- tests/const_/mod.rs | 4 +- tests/eval/mod.rs | 2 +- tests/repl/test_config.rs | 4 +- tests/repl/test_parser.rs | 4 +- tests/shell/pipeline/commands/internal.rs | 54 +-- 23 files changed, 433 insertions(+), 546 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03304fcdc5..7fe1c822b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -23,17 +23,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aae1277d39aeec15cb388266ecc24b11c80469deae6067e17a1a7aa9e5c1f234" -[[package]] -name = "ahash" -version = "0.7.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "891477e0c6a8957309ee5c45a6368af3ae14bb510732d2684ffa19af310920f9" -dependencies = [ - "getrandom", - "once_cell", - "version_check", -] - [[package]] name = "ahash" version = "0.8.11" @@ -730,18 +719,6 @@ dependencies = [ "serde", ] -[[package]] -name = "bitvec" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bc2832c24239b0141d5674bb9174f9d68a8b5b3f2753311927c172ca46f7e9c" -dependencies = [ - "funty", - "radium", - "tap", - "wyz", -] - [[package]] name = "blake3" version = "1.5.5" @@ -773,29 +750,6 @@ dependencies = [ "objc2", ] -[[package]] -name = "borsh" -version = "1.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2506947f73ad44e344215ccd6403ac2ae18cd8e046e581a441bf8d199f257f03" -dependencies = [ - "borsh-derive", - "cfg_aliases 0.2.1", -] - -[[package]] -name = "borsh-derive" -version = "1.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c2593a3b8b938bd68373196c9832f516be11fa487ef4ae745eb282e6a56a7244" -dependencies = [ - "once_cell", - "proc-macro-crate", - "proc-macro2", - "quote", - "syn 2.0.90", -] - [[package]] name = "bracoxide" version = "0.1.4" @@ -840,39 +794,6 @@ version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" -[[package]] -name = "byte-unit" -version = "5.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1cd29c3c585209b0cbc7309bfe3ed7efd8c84c21b7af29c8bfae908f8777174" -dependencies = [ - "rust_decimal", - "serde", - "utf8-width", -] - -[[package]] -name = "bytecheck" -version = "0.6.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23cdc57ce23ac53c931e88a43d06d070a6fd142f2617be5855eb75efc9beb1c2" -dependencies = [ - "bytecheck_derive", - "ptr_meta", - "simdutf8", -] - -[[package]] -name = "bytecheck_derive" -version = "0.6.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3db406d29fbcd95542e92559bed4d8ad92636d1ca8b3b72ede10b4bcc010e659" -dependencies = [ - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "bytecount" version = "0.6.8" @@ -2043,12 +1964,6 @@ dependencies = [ "libc", ] -[[package]] -name = "funty" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" - [[package]] name = "futf" version = "0.1.5" @@ -2316,22 +2231,13 @@ dependencies = [ "byteorder", ] -[[package]] -name = "hashbrown" -version = "0.12.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" -dependencies = [ - "ahash 0.7.8", -] - [[package]] name = "hashbrown" version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" dependencies = [ - "ahash 0.8.11", + "ahash", "allocator-api2", "rayon", "serde", @@ -4086,7 +3992,6 @@ name = "nu-protocol" version = "0.101.1" dependencies = [ "brotli", - "byte-unit", "bytes", "chrono", "chrono-humanize", @@ -5007,7 +4912,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "87dbb24d29ddea5abb73d7954df8b8d3d4bb7f02a3e5c96d1519cdad9e816a3d" dependencies = [ - "ahash 0.8.11", + "ahash", "atoi", "atoi_simd", "avro-schema", @@ -5072,7 +4977,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd5df9b55e614088a3270b06f8649dce76537c268d6b1ca4d9c37008b2be5949" dependencies = [ - "ahash 0.8.11", + "ahash", "bitflags 2.6.0", "bytemuck", "chrono", @@ -5122,7 +5027,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea1b431ed816cba1120cff200f06b962748001bbb2e615ce53cfbbdf701cc136" dependencies = [ - "ahash 0.8.11", + "ahash", "bitflags 2.6.0", "hashbrown 0.15.2", "num-traits", @@ -5146,7 +5051,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b2fab2c016635cb416b49461fd6419b0208c6c13a4fd065bd65e4a87dbb66314" dependencies = [ - "ahash 0.8.11", + "ahash", "async-trait", "atoi_simd", "blake3", @@ -5196,7 +5101,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d5c8c057ef04feaf34b6ce52096bdea3a766fa4725f50442078c8a4ee86397bf" dependencies = [ - "ahash 0.8.11", + "ahash", "chrono", "chrono-tz 0.8.6", "fallible-streaming-iterator", @@ -5218,7 +5123,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a8ca74f42e7b47cad241b36b98d991cc7fbb51b8d0695a055eb937588d1f310" dependencies = [ - "ahash 0.8.11", + "ahash", "bitflags 2.6.0", "futures", "memchr", @@ -5269,7 +5174,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "035c800fbe5bbd820afeb8313713ed345853bb014e0f821a4025d40cf0d60e1a" dependencies = [ - "ahash 0.8.11", + "ahash", "argminmax", "base64 0.22.1", "bytemuck", @@ -5307,7 +5212,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "91dcf1d9f048079376949eaf2e24e240b313ff4a102fb83b57c9a5f807cdca52" dependencies = [ - "ahash 0.8.11", + "ahash", "async-stream", "base64 0.22.1", "brotli", @@ -5373,7 +5278,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23de436f33f4d1134c58f24e7059a221b957ec20730807e0ef0c80c8e4b3d06a" dependencies = [ - "ahash 0.8.11", + "ahash", "bitflags 2.6.0", "bytemuck", "bytes", @@ -5508,7 +5413,7 @@ version = "0.44.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96186b70bda00c90b5027bf2f69193c5c40571e80d3e8ec505c22cdc8e3e39aa" dependencies = [ - "ahash 0.8.11", + "ahash", "bytemuck", "bytes", "compact_str 0.8.0", @@ -5610,15 +5515,6 @@ dependencies = [ "unicode-segmentation", ] -[[package]] -name = "proc-macro-crate" -version = "3.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ecf48c7ca261d60b74ab1a7b20da18bede46776b2e55535cb958eb595c5fa7b" -dependencies = [ - "toml_edit", -] - [[package]] name = "proc-macro-error" version = "1.0.4" @@ -5685,26 +5581,6 @@ dependencies = [ "cc", ] -[[package]] -name = "ptr_meta" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0738ccf7ea06b608c10564b31debd4f5bc5e197fc8bfe088f68ae5ce81e7a4f1" -dependencies = [ - "ptr_meta_derive", -] - -[[package]] -name = "ptr_meta_derive" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "16b845dbfca988fa33db069c0e230574d15a3088f147a87b64c7589eb662c9ac" -dependencies = [ - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "pure-rust-locales" version = "0.8.1" @@ -5923,12 +5799,6 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "640c9bd8497b02465aeef5375144c26062e0dcd5939dfcbb0f5db76cb8c17c73" -[[package]] -name = "radium" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" - [[package]] name = "rand" version = "0.8.5" @@ -6148,15 +6018,6 @@ version = "1.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" -[[package]] -name = "rend" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71fe3824f5629716b1589be05dacd749f6aa084c87e00e016714a8cdfccc997c" -dependencies = [ - "bytecheck", -] - [[package]] name = "reqwest" version = "0.12.9" @@ -6231,35 +6092,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "rkyv" -version = "0.7.45" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9008cd6385b9e161d8229e1f6549dd23c3d022f132a2ea37ac3a10ac4935779b" -dependencies = [ - "bitvec", - "bytecheck", - "bytes", - "hashbrown 0.12.3", - "ptr_meta", - "rend", - "rkyv_derive", - "seahash", - "tinyvec", - "uuid", -] - -[[package]] -name = "rkyv_derive" -version = "0.7.45" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "503d1d27590a2b0a3a4ca4c94755aa2875657196ecbf401a42eff41d7de532c0" -dependencies = [ - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "rle-decode-fast" version = "1.0.3" @@ -6399,13 +6231,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b082d80e3e3cc52b2ed634388d436fe1f4de6af5786cc2de9ba9737527bdf555" dependencies = [ "arrayvec 0.7.6", - "borsh", - "bytes", "num-traits", - "rand", - "rkyv", - "serde", - "serde_json", ] [[package]] @@ -6647,12 +6473,6 @@ version = "3.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49c1eeaf4b6a87c7479688c6d52b9f1153cedd3c489300564f932b065c6eab95" -[[package]] -name = "seahash" -version = "4.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1c107b6f4780854c8b126e228ea8869f4d7b71260f962fefb57b996b8959ba6b" - [[package]] name = "security-framework" version = "2.11.1" @@ -6905,7 +6725,7 @@ version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aa2bcf6c6e164e81bc7a5d49fc6988b3d515d9e8c07457d7b74ffb9324b9cd40" dependencies = [ - "ahash 0.8.11", + "ahash", "getrandom", "halfbrown", "once_cell", @@ -7299,12 +7119,6 @@ dependencies = [ "thiserror 1.0.69", ] -[[package]] -name = "tap" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" - [[package]] name = "target-features" version = "0.1.6" @@ -7857,12 +7671,6 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8232dd3cdaed5356e0f716d285e4b40b932ac434100fe9b7e0e8e935b9e6246" -[[package]] -name = "utf8-width" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "86bd8d4e895da8537e5315b8254664e6b769c4ff3db18321b297a1e7004392e3" - [[package]] name = "utf8_iter" version = "1.0.4" @@ -8724,15 +8532,6 @@ version = "0.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e9df38ee2d2c3c5948ea468a8406ff0db0b29ae1ffde1bcf20ef305bcc95c51" -[[package]] -name = "wyz" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" -dependencies = [ - "tap", -] - [[package]] name = "x11rb" version = "0.13.1" diff --git a/crates/nu-command/src/conversions/into/string.rs b/crates/nu-command/src/conversions/into/string.rs index f98f3aa5f8..460f6d973d 100644 --- a/crates/nu-command/src/conversions/into/string.rs +++ b/crates/nu-command/src/conversions/into/string.rs @@ -128,8 +128,8 @@ impl Command for SubCommand { }, Example { description: "convert filesize to string", - example: "1KiB | into string", - result: Some(Value::test_string("1.0 KiB")), + example: "1kB | into string", + result: Some(Value::test_string("1.0 kB")), }, Example { description: "convert duration to string", diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 2069df3dd4..2fc3046e4e 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -1,8 +1,6 @@ use chrono_humanize::HumanTime; use nu_engine::command_prelude::*; -use nu_protocol::{ - format_duration, format_filesize_from_conf, ByteStream, Config, PipelineMetadata, -}; +use nu_protocol::{format_duration, ByteStream, Config, PipelineMetadata}; use std::io::Write; const LINE_ENDING: &str = if cfg!(target_os = "windows") { @@ -170,7 +168,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, .. } => format_filesize_from_conf(val, config), + Value::Filesize { val, .. } => config.filesize.display(val).to_string(), Value::Duration { val, .. } => format_duration(val), Value::Date { val, .. } => { format!("{} ({})", val.to_rfc2822(), HumanTime::from(val)) diff --git a/crates/nu-command/src/random/binary.rs b/crates/nu-command/src/random/binary.rs index 36d0179e3a..2ef67e0f18 100644 --- a/crates/nu-command/src/random/binary.rs +++ b/crates/nu-command/src/random/binary.rs @@ -1,5 +1,4 @@ use nu_engine::command_prelude::*; -use nu_protocol::format_filesize_from_conf; use rand::{thread_rng, RngCore}; #[derive(Clone)] @@ -47,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: format_filesize_from_conf(val, engine_state.get_config()), + actual: engine_state.get_config().filesize.display(val).to_string(), span: length_val.span(), }) } diff --git a/crates/nu-command/src/random/chars.rs b/crates/nu-command/src/random/chars.rs index e426fb5dbc..409d5e5c37 100644 --- a/crates/nu-command/src/random/chars.rs +++ b/crates/nu-command/src/random/chars.rs @@ -1,5 +1,4 @@ use nu_engine::command_prelude::*; -use nu_protocol::format_filesize_from_conf; use rand::{ distributions::{Alphanumeric, Distribution}, thread_rng, @@ -84,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: format_filesize_from_conf(val, engine_state.get_config()), + actual: engine_state.get_config().filesize.display(val).to_string(), span: length_val.span(), }) } diff --git a/crates/nu-command/src/strings/format/filesize.rs b/crates/nu-command/src/strings/format/filesize.rs index d6f898fee8..77c45fcac0 100644 --- a/crates/nu-command/src/strings/format/filesize.rs +++ b/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, format_filesize}; +use nu_protocol::{engine::StateWorkingSet, FilesizeUnit}; struct Arguments { - format_value: String, + format: FilesizeUnit, cell_paths: Option>, } @@ -61,16 +61,10 @@ impl Command for FormatFilesize { call: &Call, input: PipelineData, ) -> Result { - let format_value = call - .req::(engine_state, stack, 0)? - .coerce_into_string()? - .to_ascii_lowercase(); + let format = parse_filesize_unit(call.req::>(engine_state, stack, 0)?)?; let cell_paths: Vec = call.rest(engine_state, stack, 1)?; let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths); - let arg = Arguments { - format_value, - cell_paths, - }; + let arg = Arguments { format, cell_paths }; operate( format_value_impl, arg, @@ -86,16 +80,10 @@ impl Command for FormatFilesize { call: &Call, input: PipelineData, ) -> Result { - let format_value = call - .req_const::(working_set, 0)? - .coerce_into_string()? - .to_ascii_lowercase(); + let format = parse_filesize_unit(call.req_const::>(working_set, 0)?)?; let cell_paths: Vec = call.rest_const(working_set, 1)?; let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths); - let arg = Arguments { - format_value, - cell_paths, - }; + let arg = Arguments { format, cell_paths }; operate( format_value_impl, arg, @@ -119,21 +107,27 @@ impl Command for FormatFilesize { }, Example { description: "Convert the size data to MB", - example: "4Gb | format filesize MB", - result: Some(Value::test_string("4000.0 MB")), + example: "4GB | format filesize MB", + result: Some(Value::test_string("4000 MB")), }, ] } } +fn parse_filesize_unit(format: Spanned) -> Result { + format.item.parse().map_err(|_| ShellError::InvalidValue { + valid: + "'B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', or 'EiB'" + .into(), + actual: format.item, + span: format.span, + }) +} + fn format_value_impl(val: &Value, arg: &Arguments, span: Span) -> Value { let value_span = val.span(); match val { - Value::Filesize { val, .. } => Value::string( - // don't need to concern about metric, we just format units by what user input. - format_filesize(*val, &arg.format_value, None), - span, - ), + Value::Filesize { val, .. } => Value::string(val.display(arg.format).to_string(), span), Value::Error { .. } => val.clone(), _ => Value::error( ShellError::OnlySupportsThisInputType { diff --git a/crates/nu-command/tests/commands/format.rs b/crates/nu-command/tests/commands/format.rs index 6084faf1e4..7304ed0a77 100644 --- a/crates/nu-command/tests/commands/format.rs +++ b/crates/nu-command/tests/commands/format.rs @@ -76,13 +76,13 @@ fn format_filesize_works() { cwd: dirs.test(), pipeline( " ls - | format filesize KB size + | format filesize kB size | get size | first " )); - assert_eq!(actual.out, "0.0 KB"); + assert_eq!(actual.out, "0 kB"); }) } @@ -105,10 +105,10 @@ fn format_filesize_works_with_nonempty_files() { ); #[cfg(not(windows))] - assert_eq!(actual.out, "25"); + assert_eq!(actual.out, "25 B"); #[cfg(windows)] - assert_eq!(actual.out, "27"); + assert_eq!(actual.out, "27 B"); }, ) } diff --git a/crates/nu-command/tests/commands/into_filesize.rs b/crates/nu-command/tests/commands/into_filesize.rs index 6a72c76b5d..6ed6047935 100644 --- a/crates/nu-command/tests/commands/into_filesize.rs +++ b/crates/nu-command/tests/commands/into_filesize.rs @@ -1,30 +1,27 @@ use nu_test_support::{nu, pipeline}; #[test] -fn into_filesize_int() { +fn int() { let actual = nu!("1 | into filesize"); assert!(actual.out.contains("1 B")); } #[test] -fn into_filesize_float() { +fn float() { let actual = nu!("1.2 | into filesize"); assert!(actual.out.contains("1 B")); } #[test] -fn into_filesize_str() { - let actual = nu!(r#" - '2000' | into filesize - "#); - - assert!(actual.out.contains("2.0 KiB")); +fn str() { + let actual = nu!("'2000' | into filesize"); + assert!(actual.out.contains("2.0 kB")); } #[test] -fn into_filesize_str_newline() { +fn str_newline() { let actual = nu!(pipeline( r#" "2000 @@ -32,11 +29,11 @@ fn into_filesize_str_newline() { "# )); - assert!(actual.out.contains("2.0 KiB")); + assert!(actual.out.contains("2.0 kB")); } #[test] -fn into_filesize_str_many_newlines() { +fn str_many_newlines() { let actual = nu!(pipeline( r#" "2000 @@ -45,88 +42,88 @@ fn into_filesize_str_many_newlines() { "# )); - assert!(actual.out.contains("2.0 KiB")); + assert!(actual.out.contains("2.0 kB")); } #[test] -fn into_filesize_filesize() { - let actual = nu!("3kib | into filesize"); +fn filesize() { + let actual = nu!("3kB | into filesize"); - assert!(actual.out.contains("3.0 KiB")); + assert!(actual.out.contains("3.0 kB")); } #[test] -fn into_filesize_negative_filesize() { - let actual = nu!("-3kib | into filesize"); +fn negative_filesize() { + let actual = nu!("-3kB | into filesize"); - assert!(actual.out.contains("-3.0 KiB")); + assert!(actual.out.contains("-3.0 kB")); } #[test] -fn into_filesize_negative_str_filesize() { - let actual = nu!("'-3kib' | into filesize"); +fn negative_str_filesize() { + let actual = nu!("'-3kB' | into filesize"); - assert!(actual.out.contains("-3.0 KiB")); + assert!(actual.out.contains("-3.0 kB")); } #[test] -fn into_filesize_wrong_negative_str_filesize() { - let actual = nu!("'--3kib' | into filesize"); +fn wrong_negative_str_filesize() { + let actual = nu!("'--3kB' | into filesize"); assert!(actual.err.contains("can't convert string to filesize")); } #[test] -fn into_filesize_large_negative_str_filesize() { - let actual = nu!("'-10000PiB' | into filesize"); +fn large_negative_str_filesize() { + let actual = nu!("'-10000PB' | into filesize"); assert!(actual.err.contains("can't convert string to filesize")); } #[test] -fn into_filesize_negative_str() { +fn negative_str() { let actual = nu!("'-1' | into filesize"); assert!(actual.out.contains("-1 B")); } #[test] -fn into_filesize_wrong_negative_str() { +fn wrong_negative_str() { let actual = nu!("'--1' | into filesize"); assert!(actual.err.contains("can't convert string to filesize")); } #[test] -fn into_filesize_positive_str_filesize() { - let actual = nu!("'+1Kib' | into filesize"); +fn positive_str_filesize() { + let actual = nu!("'+1kB' | into filesize"); - assert!(actual.out.contains("1.0 KiB")); + assert!(actual.out.contains("1.0 kB")); } #[test] -fn into_filesize_wrong_positive_str_filesize() { - let actual = nu!("'++1Kib' | into filesize"); +fn wrong_positive_str_filesize() { + let actual = nu!("'++1kB' | into filesize"); assert!(actual.err.contains("can't convert string to filesize")); } #[test] -fn into_filesize_large_positive_str_filesize() { - let actual = nu!("'+10000PiB' | into filesize"); +fn large_positive_str_filesize() { + let actual = nu!("'+10000PB' | into filesize"); assert!(actual.err.contains("can't convert string to filesize")); } #[test] -fn into_filesize_positive_str() { +fn positive_str() { let actual = nu!("'+1' | into filesize"); assert!(actual.out.contains("1 B")); } #[test] -fn into_filesize_wrong_positive_str() { +fn wrong_positive_str() { let actual = nu!("'++1' | into filesize"); assert!(actual.err.contains("can't convert string to filesize")); diff --git a/crates/nu-command/tests/commands/math/mod.rs b/crates/nu-command/tests/commands/math/mod.rs index 5d04f450a4..5d383b9d9d 100644 --- a/crates/nu-command/tests/commands/math/mod.rs +++ b/crates/nu-command/tests/commands/math/mod.rs @@ -306,57 +306,32 @@ fn floor_div_mod_large_num() { #[test] fn unit_multiplication_math() { - let actual = nu!(pipeline( - r#" - 1mb * 2 - "# - )); - - assert_eq!(actual.out, "1.9 MiB"); + let actual = nu!("1MB * 2"); + assert_eq!(actual.out, "2.0 MB"); } #[test] fn unit_multiplication_float_math() { - let actual = nu!(pipeline( - r#" - 1mb * 1.2 - "# - )); - - assert_eq!(actual.out, "1.1 MiB"); + let actual = nu!("1MB * 1.2"); + assert_eq!(actual.out, "1.2 MB"); } #[test] fn unit_float_floor_division_math() { - let actual = nu!(pipeline( - r#" - 1mb // 3.0 - "# - )); - - assert_eq!(actual.out, "325.5 KiB"); + let actual = nu!("1MB // 3.0"); + assert_eq!(actual.out, "333.3 kB"); } #[test] fn unit_division_math() { - let actual = nu!(pipeline( - r#" - 1mb / 4 - "# - )); - - assert_eq!(actual.out, "244.1 KiB"); + let actual = nu!("1MB / 4"); + assert_eq!(actual.out, "250.0 kB"); } #[test] fn unit_float_division_math() { - let actual = nu!(pipeline( - r#" - 1mb / 3.1 - "# - )); - - assert_eq!(actual.out, "315.0 KiB"); + let actual = nu!("1MB / 3.2"); + assert_eq!(actual.out, "312.5 kB"); } #[test] diff --git a/crates/nu-command/tests/commands/ucp.rs b/crates/nu-command/tests/commands/ucp.rs index fe18d9187d..9b4cdce889 100644 --- a/crates/nu-command/tests/commands/ucp.rs +++ b/crates/nu-command/tests/commands/ucp.rs @@ -854,7 +854,7 @@ fn test_cp_arg_no_clobber() { fn test_cp_arg_no_clobber_twice() { Playground::setup("ucp_test_29", |dirs, sandbox| { sandbox.with_files(&[ - EmptyFile("source.txt"), + FileWithContent("source.txt", "fake data"), FileWithContent("source_with_body.txt", "some-body"), ]); nu!( diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index b4a126511b..384b677d1b 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -23,7 +23,6 @@ nu-derive-value = { path = "../nu-derive-value", version = "0.101.1" } brotli = { workspace = true, optional = true } bytes = { workspace = true } -byte-unit = { version = "5.1" } chrono = { workspace = true, features = [ "serde", "std", "unstable-locales" ], default-features = false } chrono-humanize = { workspace = true } dirs = { workspace = true } diff --git a/crates/nu-protocol/src/config/filesize.rs b/crates/nu-protocol/src/config/filesize.rs index 8c80f0f72c..75f27961c1 100644 --- a/crates/nu-protocol/src/config/filesize.rs +++ b/crates/nu-protocol/src/config/filesize.rs @@ -1,17 +1,70 @@ -use super::prelude::*; -use crate as nu_protocol; +use super::{config_update_string_enum, prelude::*}; +use crate::{DisplayFilesize, Filesize, FilesizeUnit}; -#[derive(Clone, Debug, IntoValue, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub enum FilesizeFormatUnit { + Metric, + Binary, + Unit(FilesizeUnit), +} + +impl From for FilesizeFormatUnit { + fn from(unit: FilesizeUnit) -> Self { + Self::Unit(unit) + } +} + +impl FromStr for FilesizeFormatUnit { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + 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", + FilesizeFormatUnit::Binary => "binary", + FilesizeFormatUnit::Unit(unit) => unit.as_str(), + } + .into_value(span) + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct FilesizeConfig { - pub metric: bool, - pub format: String, + pub unit: FilesizeFormatUnit, + pub precision: Option, +} + +impl FilesizeConfig { + pub fn display(&self, filesize: Filesize) -> DisplayFilesize { + let unit = match self.unit { + FilesizeFormatUnit::Metric => filesize.largest_metric_unit(), + FilesizeFormatUnit::Binary => filesize.largest_binary_unit(), + FilesizeFormatUnit::Unit(unit) => unit, + }; + filesize.display(unit).precision(self.precision) + } } impl Default for FilesizeConfig { fn default() -> Self { Self { - metric: false, - format: "auto".into(), + unit: FilesizeFormatUnit::Metric, + precision: Some(1), } } } @@ -31,10 +84,29 @@ impl UpdateFromValue for FilesizeConfig { for (col, val) in record.iter() { let path = &mut path.push(col); match col.as_str() { - "metric" => self.metric.update(val, path, errors), - "format" => self.format.update(val, path, errors), + "unit" => config_update_string_enum(&mut self.unit, val, path, errors), + "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), } } } } + +impl IntoValue for FilesizeConfig { + fn into_value(self, span: Span) -> Value { + record! { + "unit" => self.unit.into_value(span), + "precision" => self.precision.map(|x| x as i64).into_value(span), + } + .into_value(span) + } +} diff --git a/crates/nu-protocol/src/value/filesize.rs b/crates/nu-protocol/src/value/filesize.rs index 9e291db5bb..f1cc97430b 100644 --- a/crates/nu-protocol/src/value/filesize.rs +++ b/crates/nu-protocol/src/value/filesize.rs @@ -1,7 +1,4 @@ -use crate::{Config, FromValue, IntoValue, ShellError, Span, Type, Value}; -use byte_unit::UnitType; -use nu_utils::get_system_locale; -use num_format::ToFormattedString; +use crate::{FromValue, IntoValue, ShellError, Span, Type, Value}; use serde::{Deserialize, Serialize}; use std::{ fmt, @@ -66,12 +63,108 @@ impl Filesize { } /// Returns a [`Filesize`] representing the sign of `self`. - /// - 0 if the filesize is zero - /// - 1 if the filesize is positive - /// - -1 if the filesize is negative + /// - 0 if the file size is zero + /// - 1 if the file size is positive + /// - -1 if the file size is negative pub const fn signum(self) -> Self { Self(self.0.signum()) } + + /// Returns the largest [`FilesizeUnit`] with a metric prefix that is smaller than or equal to `self`. + /// + /// # Examples + /// ``` + /// # use nu_protocol::{Filesize, FilesizeUnit}; + /// + /// let filesize = Filesize::from(FilesizeUnit::KB); + /// assert_eq!(filesize.largest_metric_unit(), FilesizeUnit::KB); + /// + /// let filesize = Filesize::new(FilesizeUnit::KB.as_bytes() as i64 - 1); + /// assert_eq!(filesize.largest_metric_unit(), FilesizeUnit::B); + /// + /// let filesize = Filesize::from(FilesizeUnit::KiB); + /// assert_eq!(filesize.largest_metric_unit(), FilesizeUnit::KB); + /// ``` + pub const fn largest_metric_unit(&self) -> FilesizeUnit { + const KB: u64 = FilesizeUnit::KB.as_bytes(); + const MB: u64 = FilesizeUnit::MB.as_bytes(); + const GB: u64 = FilesizeUnit::GB.as_bytes(); + const TB: u64 = FilesizeUnit::TB.as_bytes(); + const PB: u64 = FilesizeUnit::PB.as_bytes(); + const EB: u64 = FilesizeUnit::EB.as_bytes(); + + match self.0.unsigned_abs() { + 0..KB => FilesizeUnit::B, + KB..MB => FilesizeUnit::KB, + MB..GB => FilesizeUnit::MB, + GB..TB => FilesizeUnit::GB, + TB..PB => FilesizeUnit::TB, + PB..EB => FilesizeUnit::PB, + EB.. => FilesizeUnit::EB, + } + } + + /// Returns the largest [`FilesizeUnit`] with a binary prefix that is smaller than or equal to `self`. + /// + /// # Examples + /// ``` + /// # use nu_protocol::{Filesize, FilesizeUnit}; + /// + /// let filesize = Filesize::from(FilesizeUnit::KiB); + /// assert_eq!(filesize.largest_binary_unit(), FilesizeUnit::KiB); + /// + /// let filesize = Filesize::new(FilesizeUnit::KiB.as_bytes() as i64 - 1); + /// assert_eq!(filesize.largest_binary_unit(), FilesizeUnit::B); + /// + /// let filesize = Filesize::from(FilesizeUnit::MB); + /// assert_eq!(filesize.largest_binary_unit(), FilesizeUnit::KiB); + /// ``` + pub const fn largest_binary_unit(&self) -> FilesizeUnit { + const KIB: u64 = FilesizeUnit::KiB.as_bytes(); + const MIB: u64 = FilesizeUnit::MiB.as_bytes(); + const GIB: u64 = FilesizeUnit::GiB.as_bytes(); + const TIB: u64 = FilesizeUnit::TiB.as_bytes(); + const PIB: u64 = FilesizeUnit::PiB.as_bytes(); + const EIB: u64 = FilesizeUnit::EiB.as_bytes(); + + match self.0.unsigned_abs() { + 0..KIB => FilesizeUnit::B, + KIB..MIB => FilesizeUnit::KiB, + MIB..GIB => FilesizeUnit::MiB, + GIB..TIB => FilesizeUnit::GiB, + TIB..PIB => FilesizeUnit::TiB, + PIB..EIB => FilesizeUnit::PiB, + EIB.. => FilesizeUnit::EiB, + } + } + + /// Returns a struct that can be used to display a [`Filesize`] scaled to the given + /// [`FilesizeUnit`]. + /// + /// You can use [`largest_binary_unit`](Filesize::largest_binary_unit) or + /// [`largest_metric_unit`](Filesize::largest_metric_unit) to automatically determine a + /// [`FilesizeUnit`] of appropriate scale for a specific [`Filesize`]. + /// + /// The default [`Display`](fmt::Display) implementation for [`Filesize`] is + /// `self.display(self.largest_metric_unit())`. + /// + /// # Examples + /// ``` + /// # use nu_protocol::{Filesize, FilesizeUnit}; + /// let filesize = Filesize::from_unit(4, FilesizeUnit::KiB).unwrap(); + /// + /// assert_eq!(filesize.display(FilesizeUnit::B).to_string(), "4096 B"); + /// 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_metric_unit()).to_string(), "4.096 kB"); + /// ``` + pub fn display(&self, unit: FilesizeUnit) -> DisplayFilesize { + DisplayFilesize { + filesize: *self, + unit, + precision: None, + } + } } impl From for Filesize { @@ -266,13 +359,13 @@ impl Sum for Option { impl fmt::Display for Filesize { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - format_filesize(*self, "auto", Some(false)).fmt(f) + write!(f, "{}", self.display(self.largest_metric_unit())) } } /// All the possible filesize units for a [`Filesize`]. /// -/// This type contains both units with metric (SI) decimal prefixes which are powers of 10 (e.g., kB = 1000 bytes) +/// This type contains both units with metric (SI) prefixes which are powers of 10 (e.g., kB = 1000 bytes) /// and units with binary prefixes which are powers of 2 (e.g., KiB = 1024 bytes). /// /// The number of bytes in a [`FilesizeUnit`] can be obtained using @@ -334,9 +427,10 @@ impl FilesizeUnit { Filesize::new(self.as_bytes() as i64) } - /// Returns the abbreviated unit for a [`FilesizeUnit`] as a [`str`]. + /// Returns the symbol [`str`] for a [`FilesizeUnit`]. /// - /// The abbreviated unit is exactly the same as the enum case name in Rust code. + /// The symbol is exactly the same as the enum case name in Rust code except for + /// [`FilesizeUnit::KB`] which is `kB`. /// /// # Examples /// ``` @@ -363,10 +457,10 @@ impl FilesizeUnit { } } - /// Returns `true` if a [`FilesizeUnit`] has a metric (SI) decimal prefix (a power of 10). + /// Returns `true` if a [`FilesizeUnit`] has a metric (SI) prefix (a power of 10). /// /// Note that this returns `true` for [`FilesizeUnit::B`] as well. - pub const fn is_decimal(&self) -> bool { + pub const fn is_metric(&self) -> bool { match self { Self::B | Self::KB | Self::MB | Self::GB | Self::TB | Self::PB | Self::EB => true, Self::KiB | Self::MiB | Self::GiB | Self::TiB | Self::PiB | Self::EiB => false, @@ -390,7 +484,7 @@ impl From for Filesize { } } -/// An error returned when failing to parse a [`FilesizeUnit`]. +/// The error returned when failing to parse a [`FilesizeUnit`]. /// /// This occurs when the string being parsed does not exactly match the name of one of the /// enum cases in [`FilesizeUnit`]. @@ -399,7 +493,7 @@ pub struct ParseFilesizeUnitError(()); impl fmt::Display for ParseFilesizeUnitError { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(fmt, "invalid filesize unit") + write!(fmt, "invalid file size unit") } } @@ -432,97 +526,86 @@ impl fmt::Display for FilesizeUnit { } } -pub fn format_filesize_from_conf(filesize: Filesize, config: &Config) -> String { - // We need to take into account config.filesize_metric so, if someone asks for KB - // and filesize_metric is false, return KiB - format_filesize( - filesize, - &config.filesize.format, - Some(config.filesize.metric), - ) -} - -// filesize_metric is explicit when printed a value according to user config; -// other places (such as `format filesize`) don't. -pub fn format_filesize( +#[derive(Debug)] +pub struct DisplayFilesize { filesize: Filesize, - format_value: &str, - filesize_metric: Option, -) -> String { - // Allow the user to specify how they want their numbers formatted - - // When format_value is "auto" or an invalid value, the returned ByteUnit doesn't matter - // and is always B. - let filesize_unit = get_filesize_format(format_value, filesize_metric); - let byte = byte_unit::Byte::from_u64(filesize.0.unsigned_abs()); - let adj_byte = if let Some(unit) = filesize_unit { - byte.get_adjusted_unit(unit) - } else { - // When filesize_metric is None, format_value should never be "auto", so this - // unwrap_or() should always work. - byte.get_appropriate_unit(if filesize_metric.unwrap_or(false) { - UnitType::Decimal - } else { - UnitType::Binary - }) - }; + unit: FilesizeUnit, + precision: Option, +} - match adj_byte.get_unit() { - byte_unit::Unit::B => { - let locale = get_system_locale(); - let locale_byte = adj_byte.get_value() as u64; - let locale_byte_string = locale_byte.to_formatted_string(&locale); - let locale_signed_byte_string = if filesize.is_negative() { - format!("-{locale_byte_string}") - } else { - locale_byte_string - }; - - if filesize_unit.is_none() { - format!("{locale_signed_byte_string} B") - } else { - locale_signed_byte_string - } - } - _ => { - if filesize.is_negative() { - format!("-{:.1}", adj_byte) - } else { - format!("{:.1}", adj_byte) - } - } +impl DisplayFilesize { + pub fn precision(mut self, precision: impl Into>) -> Self { + self.precision = precision.into(); + self } } -/// Get the filesize unit, or None if format is "auto" -fn get_filesize_format( - format_value: &str, - filesize_metric: Option, -) -> Option { - // filesize_metric always overrides the unit of filesize_format. - let metric = filesize_metric.unwrap_or(!format_value.ends_with("ib")); - - if metric { - match format_value { - "b" => Some(byte_unit::Unit::B), - "kb" | "kib" => Some(byte_unit::Unit::KB), - "mb" | "mib" => Some(byte_unit::Unit::MB), - "gb" | "gib" => Some(byte_unit::Unit::GB), - "tb" | "tib" => Some(byte_unit::Unit::TB), - "pb" | "pib" => Some(byte_unit::Unit::TB), - "eb" | "eib" => Some(byte_unit::Unit::EB), - _ => None, - } - } else { - match format_value { - "b" => Some(byte_unit::Unit::B), - "kb" | "kib" => Some(byte_unit::Unit::KiB), - "mb" | "mib" => Some(byte_unit::Unit::MiB), - "gb" | "gib" => Some(byte_unit::Unit::GiB), - "tb" | "tib" => Some(byte_unit::Unit::TiB), - "pb" | "pib" => Some(byte_unit::Unit::TiB), - "eb" | "eib" => Some(byte_unit::Unit::EiB), - _ => None, +impl fmt::Display for DisplayFilesize { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let Self { + filesize: Filesize(filesize), + unit, + precision, + } = *self; + let precision = precision.or(f.precision()); + match unit { + FilesizeUnit::B => write!(f, "{filesize} B"), + FilesizeUnit::KiB + | FilesizeUnit::MiB + | FilesizeUnit::GiB + | FilesizeUnit::TiB + | FilesizeUnit::PiB + | FilesizeUnit::EiB => { + // This won't give exact results for large filesizes and/or units. + let val = filesize as f64 / unit.as_bytes() as f64; + if let Some(precision) = precision { + write!(f, "{val:.precision$} {unit}") + } else { + write!(f, "{val} {unit}") + } + } + 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. + let fract = unsafe { std::str::from_utf8_unchecked(&buf[..i]) }; + + if let Some(p) = precision { + write!(f, "{whole}.{fract:0, - #[case] filesize_format: String, - #[case] exp: &str, - ) { + #[case(1024, FilesizeUnit::KB, "1.024 kB")] + #[case(1024, FilesizeUnit::B, "1024 B")] + #[case(1024, FilesizeUnit::KiB, "1 KiB")] + #[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()); + } + + #[rstest] + #[case(1000, "1000 B")] + #[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(), + ); + } + + #[rstest] + #[case(999, "999 B")] + #[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, - format_filesize(Filesize::new(val), &filesize_format, filesize_metric) + filesize.display(filesize.largest_metric_unit()).to_string(), ); } } diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 58cde75b50..09ec0fb04f 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/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, .. } => format_filesize_from_conf(*val, config), + Value::Filesize { val, .. } => config.filesize.display(*val).to_string(), Value::Duration { val, .. } => format_duration(*val), Value::Date { val, .. } => match &config.datetime_format.normal { Some(format) => self.format_datetime(val, format), diff --git a/crates/nu-protocol/tests/into_config.rs b/crates/nu-protocol/tests/into_config.rs index 3ff8d44672..eae14141cf 100644 --- a/crates/nu-protocol/tests/into_config.rs +++ b/crates/nu-protocol/tests/into_config.rs @@ -25,22 +25,22 @@ fn config_preserved_after_do() { #[test] fn config_affected_when_mutated() { let actual = nu!(nu_repl_code(&[ - r#"$env.config = { filesize: { metric: false, format:"auto" } }"#, - r#"$env.config = { filesize: { metric: true, format:"auto" } }"#, - "20mib | into string" + r#"$env.config = { filesize: { unit: binary } }"#, + r#"$env.config = { filesize: { unit: metric } }"#, + "20MB | into string" ])); - assert_eq!(actual.out, "21.0 MB"); + assert_eq!(actual.out, "20.0 MB"); } #[test] fn config_affected_when_deep_mutated() { let actual = nu!(cwd: "crates/nu-utils/src/default_files", nu_repl_code(&[ r#"source default_config.nu"#, - r#"$env.config.filesize.metric = true"#, - r#"20mib | into string"#])); + r#"$env.config.filesize.unit = 'binary'"#, + r#"20MiB | into string"#])); - assert_eq!(actual.out, "21.0 MB"); + assert_eq!(actual.out, "20.0 MiB"); } #[test] diff --git a/crates/nu-protocol/tests/test_config.rs b/crates/nu-protocol/tests/test_config.rs index 81a0a7dede..f6d0aef67d 100644 --- a/crates/nu-protocol/tests/test_config.rs +++ b/crates/nu-protocol/tests/test_config.rs @@ -1,53 +1,43 @@ use nu_test_support::{nu, nu_repl_code}; #[test] -fn filesize_metric_true() { +fn filesize_mb() { let code = &[ - r#"$env.config = { filesize: { metric: true, format:"mb" } }"#, - r#"20mib | into string"#, + r#"$env.config = { filesize: { unit: MB } }"#, + r#"20MB | into string"#, ]; let actual = nu!(nu_repl_code(code)); - assert_eq!(actual.out, "21.0 MB"); + assert_eq!(actual.out, "20.0 MB"); } #[test] -fn filesize_metric_false() { +fn filesize_mib() { let code = &[ - r#"$env.config = { filesize: { metric: false, format:"mib" } }"#, - r#"20mib | into string"#, + r#"$env.config = { filesize: { unit: MiB } }"#, + r#"20MiB | into string"#, ]; let actual = nu!(nu_repl_code(code)); assert_eq!(actual.out, "20.0 MiB"); } #[test] -fn filesize_metric_overrides_format() { +fn filesize_format_decimal() { let code = &[ - r#"$env.config = { filesize: { metric: false, format:"mb" } }"#, - r#"20mib | into string"#, - ]; - let actual = nu!(nu_repl_code(code)); - assert_eq!(actual.out, "20.0 MiB"); -} - -#[test] -fn filesize_format_auto_metric_true() { - let code = &[ - r#"$env.config = { filesize: { metric: true, format:"auto" } }"#, - r#"[2mb 2gb 2tb] | into string | to nuon"#, + r#"$env.config = { filesize: { unit: metric } }"#, + r#"[2MB 2GB 2TB] | into string | to nuon"#, ]; let actual = nu!(nu_repl_code(code)); assert_eq!(actual.out, r#"["2.0 MB", "2.0 GB", "2.0 TB"]"#); } #[test] -fn filesize_format_auto_metric_false() { +fn filesize_format_binary() { let code = &[ - r#"$env.config = { filesize: { metric: false, format:"auto" } }"#, - r#"[2mb 2gb 2tb] | into string | to nuon"#, + r#"$env.config = { filesize: { unit: binary } }"#, + r#"[2MiB 2GiB 2TiB] | into string | to nuon"#, ]; let actual = nu!(nu_repl_code(code)); - assert_eq!(actual.out, r#"["1.9 MiB", "1.9 GiB", "1.8 TiB"]"#); + assert_eq!(actual.out, r#"["2.0 MiB", "2.0 GiB", "2.0 TiB"]"#); } #[test] diff --git a/crates/nu-test-support/src/playground/play.rs b/crates/nu-test-support/src/playground/play.rs index 12295c2690..a876fbd3dc 100644 --- a/crates/nu-test-support/src/playground/play.rs +++ b/crates/nu-test-support/src/playground/play.rs @@ -187,7 +187,7 @@ impl<'a> Playground<'a> { let mut permission_set = false; let mut write_able = true; let (file_name, contents) = match *f { - Stub::EmptyFile(name) => (name, "fake data".to_string()), + Stub::EmptyFile(name) => (name, String::new()), Stub::FileWithContent(name, content) => (name, content.to_string()), Stub::FileWithContentToBeTrimmed(name, content) => ( name, diff --git a/crates/nu-utils/src/default_files/doc_config.nu b/crates/nu-utils/src/default_files/doc_config.nu index 08de7724f2..1a2443960f 100644 --- a/crates/nu-utils/src/default_files/doc_config.nu +++ b/crates/nu-utils/src/default_files/doc_config.nu @@ -251,9 +251,9 @@ $env.config.bracketed_paste = true # - If `FORCE_COLOR` is set, coloring is always enabled. # - If `NO_COLOR` is set, coloring is disabled. # - If `CLICOLOR` is set, its value (0 or 1) decides whether coloring is used. -# If none of these are set, it checks whether the standard output is a terminal +# If none of these are set, it checks whether the standard output is a terminal # and enables coloring if it is. -# A value of `true` or `false` overrides this behavior, explicitly enabling or +# A value of `true` or `false` overrides this behavior, explicitly enabling or # disabling ANSI coloring in Nushell's internal commands. # When disabled, built-in commands will only use the default foreground color. # Note: This setting does not affect the `ansi` command. @@ -371,17 +371,18 @@ $env.config.datetime_format.normal = "%m/%d/%y %I:%M:%S%p" # ---------------- # Filesize Display # ---------------- -# filesize.metric (bool): When displaying filesize values ... -# true: Use the ISO-standard KB, MB, GB -# false: Use the Windows-standard KiB, MiB, GiB -$env.config.filesize.metric = false - -# filesize.format (string): One of either: -# - The filesize units such as "KB", "KiB", etc. In this case, filesize values always display using -# this unit. -# - Or "auto": Filesizes are displayed using the closest unit. For example, 1_000_000_000b will display -# as 953.7 MiB (when `metric = false`) or 1.0GB (when `metric = true`) -$env.config.filesize.format = "auto" +# filesize.unit (string): One of either: +# - A filesize unit: "B", "kB", "KiB", "MB", "MiB", "GB", "GiB", "TB", "TiB", "PB", "PiB", "EB", or "EiB". +# - An automatically scaled unit: "metric" or "binary". +# "metric" will use units with metric (SI) prefixes like kB, MB, or GB. +# "binary" will use units with binary prefixes like KiB, MiB, or GiB. +# Otherwise, setting this to one of the filesize units will use that particular unit when displaying all file sizes. +$env.config.filesize.unit = 'metric' + +# filesize.precision (int or nothing): +# The number of digits to display after the decimal point for file sizes. +# When set to `null`, all digits after the decimal point will be displayed. +$env.config.filesize.precision = 1 # --------------------- # Miscellaneous Display @@ -942,4 +943,4 @@ path add "~/.local/bin" path add ($env.CARGO_HOME | path join "bin") # You can remove duplicate directories from the path using: -$env.PATH = ($env.PATH | uniq) \ No newline at end of file +$env.PATH = ($env.PATH | uniq) diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index a1744e1601..fec81eba78 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -122,8 +122,8 @@ fn const_string_interpolation_date() { #[test] fn const_string_interpolation_filesize() { - let actual = nu!(r#"const s = $"(2kb)"; $s"#); - assert_eq!(actual.out, "2.0 KiB"); + let actual = nu!(r#"const s = $"(2kB)"; $s"#); + assert_eq!(actual.out, "2.0 kB"); } #[test] diff --git a/tests/eval/mod.rs b/tests/eval/mod.rs index 65c26a5cc6..db954df231 100644 --- a/tests/eval/mod.rs +++ b/tests/eval/mod.rs @@ -88,7 +88,7 @@ fn literal_float() { #[test] fn literal_filesize() { - test_eval("30MiB", Eq("30.0 MiB")) + test_eval("30MB", Eq("30.0 MB")) } #[test] diff --git a/tests/repl/test_config.rs b/tests/repl/test_config.rs index 839b795b48..f798d05606 100644 --- a/tests/repl/test_config.rs +++ b/tests/repl/test_config.rs @@ -95,8 +95,8 @@ fn mutate_nu_config_nested_history() -> TestResult { #[test] fn mutate_nu_config_nested_filesize() -> TestResult { run_test_std( - r#"$env.config.filesize.format = 'kb'; $env.config.filesize.format"#, - "kb", + r#"$env.config.filesize.unit = 'kB'; $env.config.filesize.unit"#, + "kB", ) } diff --git a/tests/repl/test_parser.rs b/tests/repl/test_parser.rs index 47529c69f0..e40c27dbab 100644 --- a/tests/repl/test_parser.rs +++ b/tests/repl/test_parser.rs @@ -791,12 +791,12 @@ fn duration_with_faulty_number() -> TestResult { #[test] fn filesize_with_underscores_1() -> TestResult { - run_test("420_mb", "400.5 MiB") + run_test("420_MB", "420.0 MB") } #[test] fn filesize_with_underscores_2() -> TestResult { - run_test("1_000_000B", "976.6 KiB") + run_test("1_000_000B", "1.0 MB") } #[test] diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index 59c9f31fce..833dcaab8d 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/tests/shell/pipeline/commands/internal.rs @@ -764,66 +764,32 @@ fn range_with_mixed_types() { #[test] fn filesize_math() { - let actual = nu!(" - 100 * 10kib - "); - - assert_eq!(actual.out, "1000.0 KiB"); - // why 1000.0 KB instead of 1.0 MB? - // looks like `byte.get_appropriate_unit(false)` behaves this way + let actual = nu!("100 * 10kB"); + assert_eq!(actual.out, "1.0 MB"); } #[test] fn filesize_math2() { - let actual = nu!(" - 100 / 10kb - "); - + let actual = nu!("100 / 10kb"); assert!(actual.err.contains("doesn't support")); } #[test] fn filesize_math3() { - let actual = nu!(" - 100kib / 10 - "); - - assert_eq!(actual.out, "10.0 KiB"); + let actual = nu!("100kB / 10"); + assert_eq!(actual.out, "10.0 kB"); } + #[test] fn filesize_math4() { - let actual = nu!(" - 100kib * 5 - "); - - assert_eq!(actual.out, "500.0 KiB"); + let actual = nu!("100kB * 5"); + assert_eq!(actual.out, "500.0 kB"); } #[test] fn filesize_math5() { - let actual = nu!(" - 1000 * 1kib - "); - - assert_eq!(actual.out, "1000.0 KiB"); -} - -#[test] -fn filesize_math6() { - let actual = nu!(" - 1000 * 1mib - "); - - assert_eq!(actual.out, "1000.0 MiB"); -} - -#[test] -fn filesize_math7() { - let actual = nu!(" - 1000 * 1gib - "); - - assert_eq!(actual.out, "1000.0 GiB"); + let actual = nu!("100 * 1kB"); + assert_eq!(actual.out, "100.0 kB"); } #[test]