From bc437da5c745f4a48f6087c269f8428cb8f24ae4 Mon Sep 17 00:00:00 2001 From: Bob Hyman Date: Sun, 24 Sep 2023 01:53:56 -0700 Subject: [PATCH] std dt datetime-diff: fix uninitialized field ref when borrowing (#10466) fixes #10455 @KAAtheWiseGit, I'm sorry, I didn't mean to block your first PR #10461, didn't see you had submitted it till I got around to submitting this. If you want to incoporate useful ideas from this PR into yours, I do not mind deferring to you. # Description Changes made in `datetime-diff`: * Initialize millisecond and microsecond fields in `$current`, to fix the error when borrow needs to refer to them. * Fix `borrow_nanoseconds` to borrow from seconds, not from (unused) microseconds. * Added error check to insist that first argument is >= second argument. `datetime-diff` doesn't represent negative durations correctly (it tries to borrow out of the year, resulting in negative year and positive all other fields). We don't currently have a use case requiring negative durations. * Add comments so help is a bit clearer (I was surprised that the first argument, named `$from` was actually supposed to be the *later* datetime. The order of arguments is reasonable (reminiscent of ), so I just changed the param name to match its purpose. Changes made in `pretty-print-duration`: * changed type of argument from `duration` to `record`. (it's not clear why Nu was not complaining about this!) * changed test for skipping a clause from `> 0` to `!= 0`. Even though `datetime-diff` won't present a negative field in the record, user might call `pretty-print-duration` with one, might as well handle it. (but I think `hour:-2` will be rendered as `-2hr`, not `-2hrs`...). * added help and an example. # User-Facing Changes none requiring code changes. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` - - # After Submitting --- crates/nu-std/std/dt.nu | 80 +++++++++++++++++----------------- crates/nu-std/tests/test_dt.nu | 44 +++++++++++++++++++ 2 files changed, 84 insertions(+), 40 deletions(-) create mode 100644 crates/nu-std/tests/test_dt.nu diff --git a/crates/nu-std/std/dt.nu b/crates/nu-std/std/dt.nu index 4ee76c0cb2..9a81f55d3d 100644 --- a/crates/nu-std/std/dt.nu +++ b/crates/nu-std/std/dt.nu @@ -79,7 +79,7 @@ def borrow-minute [from: record, current: record] { def borrow-second [from: record, current: record] { mut current = $current - $current.millisecond = $current.millisecond + 1_000 + $current.nanosecond = $current.nanosecond + 1_000_000_000 $current.second = $current.second - 1 if $current.second < 0 { $current = (borrow-minute $from $current) @@ -88,37 +88,33 @@ def borrow-second [from: record, current: record] { $current } -def borrow-millisecond [from: record, current: record] { - mut current = $current - $current.microsecond = $current.microsecond + 1_000_000 - $current.millisecond = $current.millisecond - 1 - if $current.millisecond < 0 { - $current = (borrow-second $from $current) +# Subtract later from earlier datetime and return the unit differences as a record +# Example: +# > dt datetime-diff 2023-05-07T04:08:45+12:00 2019-05-10T09:59:12-07:00 +# ╭─────────────┬────╮ +# │ year │ 3 │ +# │ month │ 11 │ +# │ day │ 26 │ +# │ hour │ 23 │ +# │ minute │ 9 │ +# │ second │ 33 │ +# │ millisecond │ 0 │ +# │ microsecond │ 0 │ +# │ nanosecond │ 0 │ +# ╰─────────────┴────╯ +export def datetime-diff [ + later: datetime, # a later datetime + earlier: datetime # earlier (starting) datetime + ] { + if $earlier > $later { + let start = (metadata $later).span.start + let end = (metadata $earlier).span.end + error make {msg: "Incompatible arguments", label: {start:$start, end:$end, text:$"First datetime must be >= second, but was actually ($later - $earlier) less than it."}} } + let from_expanded = ($later | date to-timezone utc | date to-record) + let to_expanded = ($earlier | date to-timezone utc | date to-record) - $current -} - -def borrow-microsecond [from: record, current: record] { - mut current = $current - $current.nanosecond = $current.nanosecond + 1_000_000_000 - $current.microsecond = $current.microsecond - 1 - if $current.microsecond < 0 { - $current = (borrow-millisecond $from $current) - } - - $current -} - -# Subtract two datetimes and return a record with the difference -# Examples -# print (datetime-diff 2023-05-07T04:08:45+12:00 2019-05-10T09:59:12+12:00) -# print (datetime-diff (date now) 2019-05-10T09:59:12-07:00) -export def datetime-diff [from: datetime, to: datetime] { - let from_expanded = ($from | date to-timezone utc | date to-record | merge { millisecond: 0, microsecond: 0}) - let to_expanded = ($to | date to-timezone utc | date to-record | merge { millisecond: 0, microsecond: 0}) - - mut result = { year: ($from_expanded.year - $to_expanded.year), month: ($from_expanded.month - $to_expanded.month)} + mut result = { year: ($from_expanded.year - $to_expanded.year), month: ($from_expanded.month - $to_expanded.month), day:0, hour:0, minute:0, second:0, millisecond:0, microsecond:0, nanosecond:0} if $result.month < 0 { $result = (borrow-year $from_expanded $result) @@ -156,65 +152,69 @@ export def datetime-diff [from: datetime, to: datetime] { $result } -export def pretty-print-duration [dur: duration] { +# Convert record from datetime-diff into humanized string +# Example: +# > dt pretty-print-duration (dt datetime-diff 2023-05-07T04:08:45+12:00 2019-05-10T09:59:12+12:00) +# 3yrs 11months 27days 18hrs 9mins 33secs +export def pretty-print-duration [dur: record] { mut result = "" - if $dur.year > 0 { + if $dur.year != 0 { if $dur.year > 1 { $result = $"($dur.year)yrs " } else { $result = $"($dur.year)yr " } } - if $dur.month > 0 { + if $dur.month != 0 { if $dur.month > 1 { $result = $"($result)($dur.month)months " } else { $result = $"($result)($dur.month)month " } } - if $dur.day > 0 { + if $dur.day != 0 { if $dur.day > 1 { $result = $"($result)($dur.day)days " } else { $result = $"($result)($dur.day)day " } } - if $dur.hour > 0 { + if $dur.hour != 0 { if $dur.hour > 1 { $result = $"($result)($dur.hour)hrs " } else { $result = $"($result)($dur.hour)hr " } } - if $dur.minute > 0 { + if $dur.minute != 0 { if $dur.minute > 1 { $result = $"($result)($dur.minute)mins " } else { $result = $"($result)($dur.minute)min " } } - if $dur.second > 0 { + if $dur.second != 0 { if $dur.second > 1 { $result = $"($result)($dur.second)secs " } else { $result = $"($result)($dur.second)sec " } } - if $dur.millisecond > 0 { + if $dur.millisecond != 0 { if $dur.millisecond > 1 { $result = $"($result)($dur.millisecond)ms " } else { $result = $"($result)($dur.millisecond)ms " } } - if $dur.microsecond > 0 { + if $dur.microsecond != 0 { if $dur.microsecond > 1 { $result = $"($result)($dur.microsecond)µs " } else { $result = $"($result)($dur.microsecond)µs " } } - if $dur.nanosecond > 0 { + if $dur.nanosecond != 0 { if $dur.nanosecond > 1 { $result = $"($result)($dur.nanosecond)ns " } else { diff --git a/crates/nu-std/tests/test_dt.nu b/crates/nu-std/tests/test_dt.nu new file mode 100644 index 0000000000..72f608248c --- /dev/null +++ b/crates/nu-std/tests/test_dt.nu @@ -0,0 +1,44 @@ +use std assert +use std dt * + +#[test] +def equal_times [] { + let t1 = (date now) + assert equal (datetime-diff $t1 $t1) ({year:0, month:0, day:0, hour:0, minute:0, second:0, millisecond:0, microsecond:0 nanosecond:0}) +} + +#[test] +def one_ns_later [] { + let t1 = (date now) + assert equal (datetime-diff ($t1 + 1ns) $t1) ({year:0, month:0, day:0, hour:0, minute:0, second:0, millisecond:0, microsecond:0 nanosecond:1}) +} + +#[test] +def one_yr_later [] { + let t1 = ('2022-10-1T0:1:2z' | into datetime) # a date for which one year later is 365 days, since duration doesn't support year or month + assert equal (datetime-diff ($t1 + 365day) $t1) ({year:1, month:0, day:0, hour:0, minute:0, second:0, millisecond:0, microsecond:0 nanosecond:0}) +} + +#[test] +def carry_ripples [] { + let t1 = ('2023-10-9T0:0:0z' | into datetime) + let t2 = ('2022-10-9T0:0:0.000000001z' | into datetime) + assert equal (datetime-diff $t1 $t2) ({year:0, month:11, day:30, hour:23, minute:59, second:59, millisecond:999, microsecond:999 nanosecond:999}) +} + +#[test] +def earlier_arg_must_be_less_or_equal_later [] { + let t1 = ('2022-10-9T0:0:0.000000001z' | into datetime) + let t2 = ('2023-10-9T0:0:0z' | into datetime) + assert error {|| (datetime-diff $t1 $t2)} +} + +#[test] +def pp_skips_zeros [] { + assert equal (pretty-print-duration {year:1, month:0, day:0, hour:0, minute:0, second:0, millisecond:0, microsecond:0 nanosecond:0}) "1yr " +} + +#[test] +def pp_doesnt_skip_neg [] { # datetime-diff can't return negative units, but prettyprint shouldn't skip them (if passed handcrafted record) + assert equal (pretty-print-duration {year:-1, month:0, day:0, hour:0, minute:0, second:0, millisecond:0, microsecond:0 nanosecond:0}) "-1yr " +} \ No newline at end of file