Skip to content

Commit 58a8f30

Browse files
LoicRiegelLoicRiegel
authored
small refactoring around units and add tests (nushell#15746)
Closes nushell#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::shell::invalid_unit × Invalid unit ╭─[entry nushell#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 ```
1 parent 70ba5d9 commit 58a8f30

File tree

16 files changed

+258
-107
lines changed

16 files changed

+258
-107
lines changed

‎crates/nu-command/src/conversions/into/duration.rs‎

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
use std::str::FromStr;
2+
13
use nu_cmd_base::input_handler::{CmdArgument, operate};
24
use nu_engine::command_prelude::*;
35
use nu_parser::{DURATION_UNIT_GROUPS, parse_unit_value};
4-
use nu_protocol::{Unit, ast::Expr};
6+
use nu_protocol::{SUPPORTED_DURATION_UNITS, Unit, ast::Expr};
57

68
const NS_PER_US: i64 = 1_000;
79
const NS_PER_MS: i64 = 1_000_000;
@@ -26,7 +28,7 @@ const ALLOWED_SIGNS: [&str; 2] = ["+", "-"];
2628

2729
#[derive(Clone, Debug)]
2830
struct Arguments {
29-
unit: Option<Spanned<String>>,
31+
unit: Option<Spanned<Unit>>,
3032
cell_paths: Option<Vec<CellPath>>,
3133
}
3234

@@ -95,28 +97,27 @@ impl Command for IntoDuration {
9597
let cell_paths = call.rest(engine_state, stack, 0)?;
9698
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
9799

98-
let span = match input.span() {
99-
Some(t) => t,
100-
None => call.head,
101-
};
102100
let unit = match call.get_flag::<Spanned<String>>(engine_state, stack, "unit")? {
103-
Some(spanned_unit) => {
104-
if ["ns", "us", "µs", "ms", "sec", "min", "hr", "day", "wk"]
105-
.contains(&spanned_unit.item.as_str())
106-
{
107-
Some(spanned_unit)
108-
} else {
109-
return Err(ShellError::CantConvertToDuration {
110-
details: spanned_unit.item,
111-
dst_span: span,
112-
src_span: span,
113-
help: Some(
114-
"supported units are ns, us/µs, ms, sec, min, hr, day, and wk"
115-
.to_string(),
116-
),
101+
Some(spanned_unit) => match Unit::from_str(&spanned_unit.item) {
102+
Ok(u) => match u {
103+
Unit::Filesize(_) => {
104+
return Err(ShellError::InvalidUnit {
105+
span: spanned_unit.span,
106+
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
107+
});
108+
}
109+
_ => Some(Spanned {
110+
item: u,
111+
span: spanned_unit.span,
112+
}),
113+
},
114+
Err(_) => {
115+
return Err(ShellError::InvalidUnit {
116+
span: spanned_unit.span,
117+
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
117118
});
118119
}
119-
}
120+
},
120121
None => None,
121122
};
122123
let args = Arguments { unit, cell_paths };
@@ -244,11 +245,9 @@ fn string_to_duration(s: &str, span: Span) -> Result<i64, ShellError> {
244245
}
245246
}
246247

247-
Err(ShellError::CantConvertToDuration {
248-
details: s.to_string(),
249-
dst_span: span,
250-
src_span: span,
251-
help: Some("supported units are ns, us/µs, ms, sec, min, hr, day, and wk".to_string()),
248+
Err(ShellError::InvalidUnit {
249+
span,
250+
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
252251
})
253252
}
254253

@@ -270,9 +269,9 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value {
270269
}
271270
}
272271

273-
let unit: &str = match unit_option {
272+
let unit = match unit_option {
274273
Some(unit) => &unit.item,
275-
None => "ns",
274+
None => &Unit::Nanosecond,
276275
};
277276

278277
match input {
@@ -417,16 +416,16 @@ fn parse_number_from_record(col_val: &Value, head: &Span) -> Result<i64, ShellEr
417416
Ok(value)
418417
}
419418

420-
fn unit_to_ns_factor(unit: &str) -> i64 {
419+
fn unit_to_ns_factor(unit: &Unit) -> i64 {
421420
match unit {
422-
"ns" => 1,
423-
"us" | "µs" => NS_PER_US,
424-
"ms" => NS_PER_MS,
425-
"sec" => NS_PER_SEC,
426-
"min" => NS_PER_MINUTE,
427-
"hr" => NS_PER_HOUR,
428-
"day" => NS_PER_DAY,
429-
"wk" => NS_PER_WEEK,
421+
Unit::Nanosecond => 1,
422+
Unit::Microsecond => NS_PER_US,
423+
Unit::Millisecond => NS_PER_MS,
424+
Unit::Second => NS_PER_SEC,
425+
Unit::Minute => NS_PER_MINUTE,
426+
Unit::Hour => NS_PER_HOUR,
427+
Unit::Day => NS_PER_DAY,
428+
Unit::Week => NS_PER_WEEK,
430429
_ => 0,
431430
}
432431
}
@@ -462,7 +461,7 @@ mod test {
462461
fn turns_string_to_duration(#[case] phrase: &str, #[case] expected_duration_val: i64) {
463462
let args = Arguments {
464463
unit: Some(Spanned {
465-
item: "ns".to_string(),
464+
item: Unit::Nanosecond,
466465
span: Span::test_data(),
467466
}),
468467
cell_paths: None,

‎crates/nu-command/src/conversions/into/int.rs‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -240,58 +240,59 @@ impl Command for IntoInt {
240240
}
241241
}
242242

243-
fn action(input: &Value, args: &Arguments, span: Span) -> Value {
243+
fn action(input: &Value, args: &Arguments, head: Span) -> Value {
244244
let radix = args.radix;
245245
let signed = args.signed;
246246
let little_endian = args.little_endian;
247247
let val_span = input.span();
248+
248249
match input {
249250
Value::Int { val: _, .. } => {
250251
if radix == 10 {
251252
input.clone()
252253
} else {
253-
convert_int(input, span, radix)
254+
convert_int(input, head, radix)
254255
}
255256
}
256-
Value::Filesize { val, .. } => Value::int(val.get(), span),
257+
Value::Filesize { val, .. } => Value::int(val.get(), head),
257258
Value::Float { val, .. } => Value::int(
258259
{
259260
if radix == 10 {
260261
*val as i64
261262
} else {
262-
match convert_int(&Value::int(*val as i64, span), span, radix).as_int() {
263+
match convert_int(&Value::int(*val as i64, head), head, radix).as_int() {
263264
Ok(v) => v,
264265
_ => {
265266
return Value::error(
266267
ShellError::CantConvert {
267268
to_type: "float".to_string(),
268269
from_type: "int".to_string(),
269-
span,
270+
span: head,
270271
help: None,
271272
},
272-
span,
273+
head,
273274
);
274275
}
275276
}
276277
}
277278
},
278-
span,
279+
head,
279280
),
280281
Value::String { val, .. } => {
281282
if radix == 10 {
282-
match int_from_string(val, span) {
283-
Ok(val) => Value::int(val, span),
284-
Err(error) => Value::error(error, span),
283+
match int_from_string(val, head) {
284+
Ok(val) => Value::int(val, head),
285+
Err(error) => Value::error(error, head),
285286
}
286287
} else {
287-
convert_int(input, span, radix)
288+
convert_int(input, head, radix)
288289
}
289290
}
290291
Value::Bool { val, .. } => {
291292
if *val {
292-
Value::int(1, span)
293+
Value::int(1, head)
293294
} else {
294-
Value::int(0, span)
295+
Value::int(0, head)
295296
}
296297
}
297298
Value::Date { val, .. } => {
@@ -310,54 +311,54 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
310311
ShellError::IncorrectValue {
311312
msg: "DateTime out of range for timestamp: 1677-09-21T00:12:43Z to 2262-04-11T23:47:16".to_string(),
312313
val_span,
313-
call_span: span,
314+
call_span: head,
314315
},
315-
span,
316+
head,
316317
)
317318
} else {
318-
Value::int(val.timestamp_nanos_opt().unwrap_or_default(), span)
319+
Value::int(val.timestamp_nanos_opt().unwrap_or_default(), head)
319320
}
320321
}
321-
Value::Duration { val, .. } => Value::int(*val, span),
322+
Value::Duration { val, .. } => Value::int(*val, head),
322323
Value::Binary { val, .. } => {
323324
use byteorder::{BigEndian, ByteOrder, LittleEndian};
324325

325326
let mut val = val.to_vec();
326327
let size = val.len();
327328

328329
if size == 0 {
329-
return Value::int(0, span);
330+
return Value::int(0, head);
330331
}
331332

332333
if size > 8 {
333334
return Value::error(
334335
ShellError::IncorrectValue {
335336
msg: format!("binary input is too large to convert to int ({size} bytes)"),
336337
val_span,
337-
call_span: span,
338+
call_span: head,
338339
},
339-
span,
340+
head,
340341
);
341342
}
342343

343344
match (little_endian, signed) {
344-
(true, true) => Value::int(LittleEndian::read_int(&val, size), span),
345-
(false, true) => Value::int(BigEndian::read_int(&val, size), span),
345+
(true, true) => Value::int(LittleEndian::read_int(&val, size), head),
346+
(false, true) => Value::int(BigEndian::read_int(&val, size), head),
346347
(true, false) => {
347348
while val.len() < 8 {
348349
val.push(0);
349350
}
350351
val.resize(8, 0);
351352

352-
Value::int(LittleEndian::read_i64(&val), span)
353+
Value::int(LittleEndian::read_i64(&val), head)
353354
}
354355
(false, false) => {
355356
while val.len() < 8 {
356357
val.insert(0, 0);
357358
}
358359
val.resize(8, 0);
359360

360-
Value::int(BigEndian::read_i64(&val), span)
361+
Value::int(BigEndian::read_i64(&val), head)
361362
}
362363
}
363364
}
@@ -368,10 +369,10 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
368369
exp_input_type: "int, float, filesize, date, string, binary, duration, or bool"
369370
.into(),
370371
wrong_type: other.get_type().to_string(),
371-
dst_span: span,
372+
dst_span: head,
372373
src_span: other.span(),
373374
},
374-
span,
375+
head,
375376
),
376377
}
377378
}

‎crates/nu-command/src/strings/format/duration.rs‎

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use nu_cmd_base::input_handler::{CmdArgument, operate};
22
use nu_engine::command_prelude::*;
3+
use nu_protocol::SUPPORTED_DURATION_UNITS;
34

45
struct Arguments {
5-
format_value: String,
6+
format_value: Spanned<String>,
67
float_precision: usize,
78
cell_paths: Option<Vec<CellPath>>,
89
}
@@ -64,10 +65,12 @@ impl Command for FormatDuration {
6465
call: &Call,
6566
input: PipelineData,
6667
) -> Result<PipelineData, ShellError> {
67-
let format_value = call
68-
.req::<Value>(engine_state, stack, 0)?
69-
.coerce_into_string()?
70-
.to_ascii_lowercase();
68+
let format_value = call.req::<Value>(engine_state, stack, 0)?;
69+
let format_value_span = format_value.span();
70+
let format_value = Spanned {
71+
item: format_value.coerce_into_string()?.to_ascii_lowercase(),
72+
span: format_value_span,
73+
};
7174
let cell_paths: Vec<CellPath> = call.rest(engine_state, stack, 1)?;
7275
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
7376
let float_precision = engine_state.config.float_precision as usize;
@@ -91,10 +94,12 @@ impl Command for FormatDuration {
9194
call: &Call,
9295
input: PipelineData,
9396
) -> Result<PipelineData, ShellError> {
94-
let format_value = call
95-
.req_const::<Value>(working_set, 0)?
96-
.coerce_into_string()?
97-
.to_ascii_lowercase();
97+
let format_value = call.req_const::<Value>(working_set, 0)?;
98+
let format_value_span = format_value.span();
99+
let format_value = Spanned {
100+
item: format_value.coerce_into_string()?.to_ascii_lowercase(),
101+
span: format_value_span,
102+
};
98103
let cell_paths: Vec<CellPath> = call.rest_const(working_set, 1)?;
99104
let cell_paths = (!cell_paths.is_empty()).then_some(cell_paths);
100105
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 {
142147
Value::Duration { val: inner, .. } => {
143148
let duration = *inner;
144149
let float_precision = arg.float_precision;
145-
match convert_inner_to_unit(duration, &arg.format_value, span, inner_span) {
150+
match convert_inner_to_unit(duration, &arg.format_value.item, arg.format_value.span) {
146151
Ok(d) => {
147-
let unit = if &arg.format_value == "us" {
152+
let unit = if &arg.format_value.item == "us" {
148153
"µs"
149154
} else {
150-
&arg.format_value
155+
&arg.format_value.item
151156
};
152157
if d.fract() == 0.0 {
153158
Value::string(format!("{} {}", d, unit), inner_span)
@@ -171,12 +176,7 @@ fn format_value_impl(val: &Value, arg: &Arguments, span: Span) -> Value {
171176
}
172177
}
173178

174-
fn convert_inner_to_unit(
175-
val: i64,
176-
to_unit: &str,
177-
span: Span,
178-
value_span: Span,
179-
) -> Result<f64, ShellError> {
179+
fn convert_inner_to_unit(val: i64, to_unit: &str, span: Span) -> Result<f64, ShellError> {
180180
match to_unit {
181181
"ns" => Ok(val as f64),
182182
"us" => Ok(val as f64 / 1000.0),
@@ -192,17 +192,13 @@ fn convert_inner_to_unit(
192192
"yr" => Ok(val as f64 / 1000.0 / 1000.0 / 1000.0 / 60.0 / 60.0 / 24.0 / 365.0),
193193
"dec" => Ok(val as f64 / 10.0 / 1000.0 / 1000.0 / 1000.0 / 60.0 / 60.0 / 24.0 / 365.0),
194194

195-
_ => Err(ShellError::CantConvertToDuration {
196-
details: to_unit.to_string(),
197-
dst_span: span,
198-
src_span: value_span,
199-
help: Some(
200-
"supported units are ns, us/µs, ms, sec, min, hr, day, wk, month, yr, and dec"
201-
.to_string(),
202-
),
195+
_ => Err(ShellError::InvalidUnit {
196+
span,
197+
supported_units: SUPPORTED_DURATION_UNITS.join(", "),
203198
}),
204199
}
205200
}
201+
206202
#[cfg(test)]
207203
mod tests {
208204
use super::*;

0 commit comments

Comments
 (0)