Skip to content

allow into duration to take an integer amount of ns#10286

Merged
fdncred merged 3 commits intonushell:mainfrom
amtoine:int-into-duration
Sep 9, 2023
Merged

allow into duration to take an integer amount of ns#10286
fdncred merged 3 commits intonushell:mainfrom
amtoine:int-into-duration

Conversation

@amtoine
Copy link
Copy Markdown
Member

@amtoine amtoine commented Sep 9, 2023

related to

Description

because 1_234 | into datetime takes an integer number of ns and 1_234 | into filesize takes an integer amount of bytes, i think 1_234 | into duration should also be valid and see 1_234 as an integer amount of ns 😋

User-Facing Changes

before

either

1234 | into string | $in ++ "ns" | into duration
1234 | $"($in)ns" | into duration

or

1234 * 1ns

and

> 1_234 | into duration
Error: nu::parser::input_type_mismatch

  × Command does not support int input.
   ╭─[entry #2:1:1]
 1  1_234 | into duration
   ·         ──────┬──────
   ·               ╰── command doesn't support int input
   ╰────

after

> 1_234 | into duration
1µs 234ns

Tests + Formatting

new example test

Example {
    description: "Convert a number of ns to duration",
    example: "1_234_567 | into duration",
    result: Some(Value::duration(1_234_567, span)),
}

After Submitting

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Sep 9, 2023

i wanted to add a --unit: string option to into duration, tried to get the option with

    let unit: String = call
        .get_flag::<String>(engine_state, stack, "unit")
        .unwrap()
        .unwrap_or("ns".into())
        .clone();

but keep getting the following error

error[E0507]: cannot move out of `unit`, a captured variable in an `FnMut` closure
   --> crates/nu-command/src/conversions/into/duration.rs:152:28
    |
143 |     let unit: String = call
    |         ---- captured outer variable
...
150 |         move |v| {
    |         -------- captured by this `FnMut` closure
151 |             if column_paths.is_empty() {
152 |                 action(&v, unit, span)
    |                            ^^^^ move occurs because `unit` has type `std::string::String`, which does not implement the `Copy` trait

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 9, 2023

I wanted to add a --unit: int

I don't understand why --unit would be an int

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Sep 9, 2023

woopsie @fdncred
i wanted to say a string, e.g. ms or sec 😌

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 9, 2023

Maybe this will help @amtoine? I figured some of it out I think.

Add to the signature

            .named(
                "unit",
                SyntaxShape::String,
                "Unit to convert number into",
                Some('u'),
            )

Parse the unit

    let unit = match call.get_flag::<String>(engine_state, stack, "unit")? {
        Some(sep) => {
            if ["ns", "us", "µs", "ms", "sec", "min", "hr", "day", "wk"]
                .iter()
                .any(|d| d == &sep)
            {
                sep
            } else {
                return Err(ShellError::CantConvertToDuration {
                    details: sep,
                    dst_span: span,
                    src_span: span,
                    help: Some(
                        "supported units are ns, us/µs, ms, sec, min, hr, day, and wk".to_string(),
                    ),
                });
            }
        }
        None => "ns".to_string(),
    };

Use it in the map

    input.map(
        move |v| {
            if column_paths.is_empty() {
                action(&v, &unit.clone(), span)
            } else {
                let unitclone = &unit.clone();
                let mut ret = v;
                for path in &column_paths {
                    let r = ret.update_cell_path(
                        &path.members,
                        Box::new(move |old| action(old, &unitclone, span)),
                    );
                    if let Err(error) = r {
                        return Value::error(error, span);
                    }
                }

                ret
            }
        },
        engine_state.ctrlc.clone(),
    )

Change the action signature

fn action(input: &Value, unit: &str, span: Span) -> Value {

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Sep 9, 2023

mm so you use a cloned binding in the move block, interesting 😏

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented Sep 9, 2023

pretty cool now, right? 😏

> random dice --dice 4 --sides 1_000_000_000_000 | each { into duration }
╭───┬──────────────────────────────╮
 0  9min 43sec 287ms 758µs 891ns 
 1  3min 21sec 594ms 789µs 374ns 
 2  5min 11sec 313ms 674µs 663ns 
 3  4min 36sec 708ms 968µs 586ns 
╰───┴──────────────────────────────╯

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 9, 2023

mm so you use a cloned binding in the move block, interesting 😏

And made action take a &str instead of a String. I tried to make unit a &str too but it needed a lifetime, which is just a pain.

I tested it, looks good to me.

@fdncred fdncred merged commit 17abbdf into nushell:main Sep 9, 2023
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Sep 9, 2023

thanks

@amtoine amtoine deleted the int-into-duration branch September 10, 2023 08:46
@sholderbach sholderbach added the notes:mention Include the release notes summary in the "Hall of Fame" section label Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:mention Include the release notes summary in the "Hall of Fame" section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants