extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-05-19 09:20:45 +02:00
Code Issues Packages Projects Releases Wiki Activity

small refactoring around units and add tests (#15746)

Browse Source 
Closes #14469

# Description
- ~~Implement the ``--unit`` conversion in "into int" command~~
- New ``ShellError::InvalidUnit`` unit if users enter wrong units
- Made ``ShellError::CantConvertToDuration`` more generic: became
``CantConvertToUnit``
- Tried to improve the way we parse units and get the supported units.
It's not complete, though, I will continue this refactoring in another
PR. But I already did some small refactorings in the "format duration"
and "format filesize" commands
- Add tests for "format filesize" and "format duration"

# User-Facing Changes

```nu
~> 1MB | format filesize sec
Error: nu:🐚:invalid_unit

  × Invalid unit
   ╭─[entry #7:1:23]
 1 │ 1MB | format filesize sec
   ·                       ─┬─
   ·                        ╰── encountered here
   ╰────
  help: Supported units are: B, kB, MB, GB, TB, PB, EB, KiB, MiB, GiB, TiB, PiB, EiB

```
This commit is contained in:
Loïc Riegel 2025-05-17 00:41:26 +02:00 committed by GitHub
parent 70ba5d9d68
commit 58a8f30a25
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 258 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
crates/nu-command/src/conversions/into/duration.rs
View File

@ -1,7 +1,9 @@
use std::str::FromStr;
use nu_cmd_base::input_handler::{CmdArgument, operate};
use nu_engine::command_prelude::*;
use nu_parser::{DURATION_UNIT_GROUPS, parse_unit_value};
use nu_protocol::{Unit, ast::Expr};
use nu_protocol::{SUPPORTED_DURATION_UNITS, Unit, ast::Expr};
const NS_PER_US: i64 = 1_000;
const NS_PER_MS: i64 = 1_000_000;
@ -26,7 +28,7 @@ const ALLOWED_SIGNS: [&str; 2] = ["+", "-"];
#[derive(Clone, Debug)]
struct Arguments {
unit: Option<Spanned<String>>,
unit: Option<Spanned<Unit>>,
cell_paths: Option<Vec<CellPath>>,
}
@ -95,28 +97,27 @@ impl Command for IntoDuration {
let cell_paths = call.rest(engine_state, stack, 0)?;
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
let span = match input.span() {
Some(t) => t,
None => call.head,
};
let unit = match call.get_flag::<Spanned<String>>(engine_state, stack, "unit")? {
Some(spanned_unit) => {
if ["ns", "us", "µs", "ms", "sec", "min", "hr", "day", "wk"]
.contains(&spanned_unit.item.as_str())
{
Some(spanned_unit)
} else {
return Err(ShellError::CantConvertToDuration {
details: spanned_unit.item,
dst_span: span,
src_span: span,
help: Some(
"supported units are ns, us/µs, ms, sec, min, hr, day, and wk"
.to_string(),
),
Some(spanned_unit) => match Unit::from_str(&spanned_unit.item) {
Ok(u) => match u {
Unit::Filesize(_) => {
return Err(ShellError::InvalidUnit {
span: spanned_unit.span,
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
});
}
_ => Some(Spanned {
item: u,
span: spanned_unit.span,
}),
},
Err(_) => {
return Err(ShellError::InvalidUnit {
span: spanned_unit.span,
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
});
}
}
},
None => None,
};
let args = Arguments { unit, cell_paths };
@ -244,11 +245,9 @@ fn string_to_duration(s: &str, span: Span) -> Result<i64, ShellError> {
}
}
Err(ShellError::CantConvertToDuration {
details: s.to_string(),
dst_span: span,
src_span: span,
help: Some("supported units are ns, us/µs, ms, sec, min, hr, day, and wk".to_string()),
Err(ShellError::InvalidUnit {
span,
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
})
}
@ -270,9 +269,9 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value {
}
}
let unit: &str = match unit_option {
let unit = match unit_option {
Some(unit) => &unit.item,
None => "ns",
None => &Unit::Nanosecond,
};
match input {
@ -417,16 +416,16 @@ fn parse_number_from_record(col_val: &Value, head: &Span) -> Result<i64, ShellEr
Ok(value)
}
fn unit_to_ns_factor(unit: &str) -> i64 {
fn unit_to_ns_factor(unit: &Unit) -> i64 {
match unit {
"ns" => 1,
"us" | "µs" => NS_PER_US,
"ms" => NS_PER_MS,
"sec" => NS_PER_SEC,
"min" => NS_PER_MINUTE,
"hr" => NS_PER_HOUR,
"day" => NS_PER_DAY,
"wk" => NS_PER_WEEK,
Unit::Nanosecond => 1,
Unit::Microsecond => NS_PER_US,
Unit::Millisecond => NS_PER_MS,
Unit::Second => NS_PER_SEC,
Unit::Minute => NS_PER_MINUTE,
Unit::Hour => NS_PER_HOUR,
Unit::Day => NS_PER_DAY,
Unit::Week => NS_PER_WEEK,
_ => 0,
}
}
@ -462,7 +461,7 @@ mod test {
fn turns_string_to_duration(#[case] phrase: &str, #[case] expected_duration_val: i64) {
let args = Arguments {
unit: Some(Spanned {
item: "ns".to_string(),
item: Unit::Nanosecond,
span: Span::test_data(),
}),
cell_paths: None,

53
crates/nu-command/src/conversions/into/int.rs
View File

@ -240,58 +240,59 @@ impl Command for IntoInt {
}
}
fn action(input: &Value, args: &Arguments, span: Span) -> Value {
fn action(input: &Value, args: &Arguments, head: Span) -> Value {
let radix = args.radix;
let signed = args.signed;
let little_endian = args.little_endian;
let val_span = input.span();
match input {
Value::Int { val: _, .. } => {
if radix == 10 {
input.clone()
} else {
convert_int(input, span, radix)
convert_int(input, head, radix)
}
}
Value::Filesize { val, .. } => Value::int(val.get(), span),
Value::Filesize { val, .. } => Value::int(val.get(), head),
Value::Float { val, .. } => Value::int(
{
if radix == 10 {
*val as i64
} else {
match convert_int(&Value::int(*val as i64, span), span, radix).as_int() {
match convert_int(&Value::int(*val as i64, head), head, radix).as_int() {
Ok(v) => v,
_ => {
return Value::error(
ShellError::CantConvert {
to_type: "float".to_string(),
from_type: "int".to_string(),
span,
span: head,
help: None,
},
span,
head,
);
}
}
}
},
span,
head,
),
Value::String { val, .. } => {
if radix == 10 {
match int_from_string(val, span) {
Ok(val) => Value::int(val, span),
Err(error) => Value::error(error, span),
match int_from_string(val, head) {
Ok(val) => Value::int(val, head),
Err(error) => Value::error(error, head),
}
} else {
convert_int(input, span, radix)
convert_int(input, head, radix)
}
}
Value::Bool { val, .. } => {
if *val {
Value::int(1, span)
Value::int(1, head)
} else {
Value::int(0, span)
Value::int(0, head)
}
}
Value::Date { val, .. } => {
@ -310,15 +311,15 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
ShellError::IncorrectValue {
msg: "DateTime out of range for timestamp: 1677-09-21T00:12:43Z to 2262-04-11T23:47:16".to_string(),
val_span,
call_span: span,
call_span: head,
},
span,
head,
)
} else {
Value::int(val.timestamp_nanos_opt().unwrap_or_default(), span)
Value::int(val.timestamp_nanos_opt().unwrap_or_default(), head)
}
}
Value::Duration { val, .. } => Value::int(*val, span),
Value::Duration { val, .. } => Value::int(*val, head),
Value::Binary { val, .. } => {
use byteorder::{BigEndian, ByteOrder, LittleEndian};
@ -326,7 +327,7 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
let size = val.len();
if size == 0 {
return Value::int(0, span);
return Value::int(0, head);
}
if size > 8 {
@ -334,22 +335,22 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
ShellError::IncorrectValue {
msg: format!("binary input is too large to convert to int ({size} bytes)"),
val_span,
call_span: span,
call_span: head,
},
span,
head,
);
}
match (little_endian, signed) {
(true, true) => Value::int(LittleEndian::read_int(&val, size), span),
(false, true) => Value::int(BigEndian::read_int(&val, size), span),
(true, true) => Value::int(LittleEndian::read_int(&val, size), head),
(false, true) => Value::int(BigEndian::read_int(&val, size), head),
(true, false) => {
while val.len() < 8 {
val.push(0);
}
val.resize(8, 0);
Value::int(LittleEndian::read_i64(&val), span)
Value::int(LittleEndian::read_i64(&val), head)
}
(false, false) => {
while val.len() < 8 {
@ -357,7 +358,7 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
}
val.resize(8, 0);
Value::int(BigEndian::read_i64(&val), span)
Value::int(BigEndian::read_i64(&val), head)
}
}
}
@ -368,10 +369,10 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
exp_input_type: "int, float, filesize, date, string, binary, duration, or bool"
.into(),
wrong_type: other.get_type().to_string(),
dst_span: span,
dst_span: head,
src_span: other.span(),
},
span,
head,
),
}
}

48
crates/nu-command/src/strings/format/duration.rs
View File

@ -1,8 +1,9 @@
use nu_cmd_base::input_handler::{CmdArgument, operate};
use nu_engine::command_prelude::*;
use nu_protocol::SUPPORTED_DURATION_UNITS;
struct Arguments {
format_value: String,
format_value: Spanned<String>,
float_precision: usize,
cell_paths: Option<Vec<CellPath>>,
}
@ -64,10 +65,12 @@ impl Command for FormatDuration {
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let format_value = call
.req::<Value>(engine_state, stack, 0)?
.coerce_into_string()?
.to_ascii_lowercase();
let format_value = call.req::<Value>(engine_state, stack, 0)?;
let format_value_span = format_value.span();
let format_value = Spanned {
item: format_value.coerce_into_string()?.to_ascii_lowercase(),
span: format_value_span,
};
let cell_paths: Vec<CellPath> = call.rest(engine_state, stack, 1)?;
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
let float_precision = engine_state.config.float_precision as usize;
@ -91,10 +94,12 @@ impl Command for FormatDuration {
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
let format_value = call
.req_const::<Value>(working_set, 0)?
.coerce_into_string()?
.to_ascii_lowercase();
let format_value = call.req_const::<Value>(working_set, 0)?;
let format_value_span = format_value.span();
let format_value = Spanned {
item: format_value.coerce_into_string()?.to_ascii_lowercase(),
span: format_value_span,
};
let cell_paths: Vec<CellPath> = call.rest_const(working_set, 1)?;
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
let float_precision = working_set.permanent().config.float_precision as usize;
@ -142,12 +147,12 @@ fn format_value_impl(val: &Value, arg: &Arguments, span: Span) -> Value {
Value::Duration { val: inner, .. } => {
let duration = *inner;
let float_precision = arg.float_precision;
match convert_inner_to_unit(duration, &arg.format_value, span, inner_span) {
match convert_inner_to_unit(duration, &arg.format_value.item, arg.format_value.span) {
Ok(d) => {
let unit = if &arg.format_value == "us" {
let unit = if &arg.format_value.item == "us" {
"µs"
} else {
&arg.format_value
&arg.format_value.item
};
if d.fract() == 0.0 {
Value::string(format!("{} {}", d, unit), inner_span)
@ -171,12 +176,7 @@ fn format_value_impl(val: &Value, arg: &Arguments, span: Span) -> Value {
}
}
fn convert_inner_to_unit(
val: i64,
to_unit: &str,
span: Span,
value_span: Span,
) -> Result<f64, ShellError> {
fn convert_inner_to_unit(val: i64, to_unit: &str, span: Span) -> Result<f64, ShellError> {
match to_unit {
"ns" => Ok(val as f64),
"us" => Ok(val as f64 / 1000.0),
@ -192,17 +192,13 @@ fn convert_inner_to_unit(
"yr" => Ok(val as f64 / 1000.0 / 1000.0 / 1000.0 / 60.0 / 60.0 / 24.0 / 365.0),
"dec" => Ok(val as f64 / 10.0 / 1000.0 / 1000.0 / 1000.0 / 60.0 / 60.0 / 24.0 / 365.0),
_ => Err(ShellError::CantConvertToDuration {
details: to_unit.to_string(),
dst_span: span,
src_span: value_span,
help: Some(
"supported units are ns, us/µs, ms, sec, min, hr, day, wk, month, yr, and dec"
.to_string(),
),
_ => Err(ShellError::InvalidUnit {
span,
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
}),
}
}
#[cfg(test)]
mod tests {
use super::*;

11
crates/nu-command/src/strings/format/filesize.rs
View File

@ -1,6 +1,8 @@
use nu_cmd_base::input_handler::{CmdArgument, operate};
use nu_engine::command_prelude::*;
use nu_protocol::{FilesizeFormatter, FilesizeUnit, engine::StateWorkingSet};
use nu_protocol::{
FilesizeFormatter, FilesizeUnit, SUPPORTED_FILESIZE_UNITS, engine::StateWorkingSet,
};
struct Arguments {
unit: FilesizeUnit,
@ -115,11 +117,8 @@ impl Command for FormatFilesize {
}
fn parse_filesize_unit(format: Spanned<String>) -> Result<FilesizeUnit, ShellError> {
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,
format.item.parse().map_err(|_| ShellError::InvalidUnit {
supported_units: SUPPORTED_FILESIZE_UNITS.join(", "),
span: format.span,
})
}

14
crates/nu-command/tests/commands/format.rs
View File

@ -112,3 +112,17 @@ fn format_filesize_works_with_nonempty_files() {
},
)
}
#[test]
fn format_filesize_with_invalid_unit() {
let actual = nu!("1MB | format filesize sec");
assert!(actual.err.contains("invalid_unit"));
}
#[test]
fn format_duration_with_invalid_unit() {
let actual = nu!("1sec | format duration MB");
assert!(actual.err.contains("invalid_unit"));
}

14
crates/nu-command/tests/commands/into_duration.rs
View File

@ -95,6 +95,20 @@ fn into_duration_from_record_fails_with_invalid_sign() {
// Tests invalid usage
#[test]
fn into_duration_invalid_unit() {
let actual = nu!(r#"1 | into duration --unit xx"#);
assert!(actual.err.contains("nu::shell::invalid_unit"));
}
#[test]
fn into_duration_filesize_unit() {
let actual = nu!(r#"1 | into duration --unit MB"#);
assert!(actual.err.contains("nu::shell::invalid_unit"));
}
#[test]
fn into_duration_from_record_fails_with_unknown_key() {
let actual = nu!(r#"{week: 10, unknown: 1} | into duration"#);

1
crates/nu-command/tests/main.rs
View File

@ -1,3 +1,4 @@
mod commands;
mod format_conversions;
mod sort_utils;
mod string;

15
crates/nu-command/tests/string/format/duration.rs Normal file
View File

@ -0,0 +1,15 @@
use nu_test_support::nu;
#[test]
fn format_duration() {
let actual = nu!(r#"1hr | format duration sec"#);
assert_eq!("3600 sec", actual.out);
}
#[test]
fn format_duration_with_invalid_unit() {
let actual = nu!(r#"1hr | format duration MB"#);
assert!(actual.err.contains("invalid_unit"));
}

15
crates/nu-command/tests/string/format/filesize.rs Normal file
View File

@ -0,0 +1,15 @@
use nu_test_support::nu;
#[test]
fn format_duration() {
let actual = nu!(r#"1MB | format filesize kB"#);
assert_eq!("1000 kB", actual.out);
}
#[test]
fn format_duration_with_invalid_unit() {
let actual = nu!(r#"1MB | format filesize sec"#);
assert!(actual.err.contains("invalid_unit"));
}

2
crates/nu-command/tests/string/format/mod.rs Normal file
View File

@ -0,0 +1,2 @@
mod duration;
mod filesize;

1
crates/nu-command/tests/string/mod.rs Normal file
View File

@ -0,0 +1 @@
mod format;

2
crates/nu-protocol/src/ast/mod.rs
View File

@ -13,7 +13,7 @@ mod pipeline;
mod range;
mod table;
mod traverse;
mod unit;
pub mod unit;
mod value_with_unit;
pub use attribute::*;

68
crates/nu-protocol/src/ast/unit.rs
View File

@ -1,5 +1,24 @@
use crate::{Filesize, FilesizeUnit, IntoValue, ShellError, Span, Value};
use serde::{Deserialize, Serialize};
use std::fmt;
use std::str::FromStr;
use thiserror::Error;
pub const SUPPORTED_DURATION_UNITS: [&str; 9] =
["ns", "us", "µs", "ms", "sec", "min", "hr", "day", "wk"];
/// The error returned when failing to parse a [`Unit`].
///
/// This occurs when the string being parsed does not exactly match the name of one of the
/// enum cases in [`Unit`].
#[derive(Debug, Copy, Clone, PartialEq, Eq, Error)]
pub struct ParseUnitError(());
impl fmt::Display for ParseUnitError {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(fmt, "invalid file size or duration unit")
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum Unit {
@ -78,4 +97,53 @@ impl Unit {
},
}
}
/// Returns the symbol [`str`] for a [`Unit`].
///
/// The returned string is the same exact string needed for a successful call to
/// [`parse`](str::parse) for a [`Unit`].
///
/// # Examples
/// ```
/// # use nu_protocol::{Unit, FilesizeUnit};
/// assert_eq!(Unit::Nanosecond.as_str(), "ns");
/// assert_eq!(Unit::Filesize(FilesizeUnit::B).as_str(), "B");
/// assert_eq!(Unit::Second.as_str().parse(), Ok(Unit::Second));
/// assert_eq!(Unit::Filesize(FilesizeUnit::KB).as_str().parse(), Ok(Unit::Filesize(FilesizeUnit::KB)));
/// ```
pub const fn as_str(&self) -> &'static str {
match self {
Unit::Filesize(u) => u.as_str(),
Unit::Nanosecond => "ns",
Unit::Microsecond => "us",
Unit::Millisecond => "ms",
Unit::Second => "sec",
Unit::Minute => "min",
Unit::Hour => "hr",
Unit::Day => "day",
Unit::Week => "wk",
}
}
}
impl FromStr for Unit {
type Err = ParseUnitError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
if let Ok(filesize_unit) = FilesizeUnit::from_str(s) {
return Ok(Unit::Filesize(filesize_unit));
};
match s {
"ns" => Ok(Unit::Nanosecond),
"us" | "µs" => Ok(Unit::Microsecond),
"ms" => Ok(Unit::Millisecond),
"sec" => Ok(Unit::Second),
"min" => Ok(Unit::Minute),
"hr" => Ok(Unit::Hour),
"day" => Ok(Unit::Day),
"wk" => Ok(Unit::Week),
_ => Err(ParseUnitError(())),
}
}
}

40
crates/nu-protocol/src/errors/shell_error/mod.rs
View File

@ -76,7 +76,7 @@ pub enum ShellError {
exp_input_type: String,
#[label("expected: {exp_input_type}")]
dst_span: Span,
#[label("value originates from here")]
#[label("value originates here")]
src_span: Span,
},
@ -431,14 +431,20 @@ pub enum ShellError {
help: Option<String>,
},
#[error("Can't convert string `{details}` to duration.")]
#[diagnostic(code(nu::shell::cant_convert_with_value))]
CantConvertToDuration {
details: String,
#[label("can't be converted to duration")]
dst_span: Span,
#[label("this string value...")]
src_span: Span,
/// Failed to convert a value of one type into a different type by specifying a unit.
///
/// ## Resolution
///
/// Check that the provided value can be converted in the provided: only Durations can be converted to duration units, and only Filesize can be converted to filesize units.
#[error("Can't convert {from_type} to the specified unit.")]
#[diagnostic(code(nu::shell::cant_convert_value_to_unit))]
CantConvertToUnit {
to_type: String,
from_type: String,
#[label("can't convert {from_type} to {to_type}")]
span: Span,
#[label("conversion originates here")]
unit_span: Span,
#[help]
help: Option<String>,
},
@ -1238,6 +1244,22 @@ This is an internal Nushell error, please file an issue https://github.com/nushe
span: Span,
},
/// Invalid unit
///
/// ## Resolution
///
/// Correct unit
#[error("Invalid unit")]
#[diagnostic(
code(nu::shell::invalid_unit),
help("Supported units are: {supported_units}")
)]
InvalidUnit {
supported_units: String,
#[label("encountered here")]
span: Span,
},
/// Tried spreading a non-list inside a list or command call.
///
/// ## Resolution

2
crates/nu-protocol/src/lib.rs
View File

@ -27,7 +27,7 @@ mod ty;
mod value;
pub use alias::*;
pub use ast::Unit;
pub use ast::unit::*;
pub use config::*;
pub use did_you_mean::did_you_mean;
pub use engine::{ENV_VARIABLE_ID, IN_VARIABLE_ID, NU_VARIABLE_ID};

4
crates/nu-protocol/src/value/filesize.rs
View File

@ -10,6 +10,10 @@ use std::{
};
use thiserror::Error;
pub const SUPPORTED_FILESIZE_UNITS: [&str; 13] = [
"B", "kB", "MB", "GB", "TB", "PB", "EB", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB",
];
/// A signed number of bytes.
///
/// [`Filesize`] is a wrapper around [`i64`]. Whereas [`i64`] is a dimensionless value, [`Filesize`] represents a