Skip to content

change duration mod duration to duration instead of float#13300

Merged
fdncred merged 1 commit intonushell:mainfrom
fdncred:fix_duration_mod_div
Jul 5, 2024
Merged

change duration mod duration to duration instead of float#13300
fdncred merged 1 commit intonushell:mainfrom
fdncred:fix_duration_mod_div

Conversation

@fdncred
Copy link
Copy Markdown
Contributor

@fdncred fdncred commented Jul 5, 2024

Description

closes #13298 so that duration mod duration / duration = duration

Before

(92sec mod 1min) / 1sec
Error: nu::parser::unsupported_operation

  × division is not supported between float and duration.
   ╭─[entry #5:1:1]
 1 │ (92sec mod 1min) / 1sec
   · ────────┬─────── ┬ ──┬─
   ·         │        │   ╰── duration
   ·         │        ╰── doesn't support these values
   ·         ╰── float
   ╰────

After

❯ (92sec mod 1min) / 1sec
32

User-Facing Changes

Tests + Formatting

After Submitting

@fdncred fdncred added the notes:fixes Include the release notes summary in the "Bug fixes" section label Jul 5, 2024
@fdncred fdncred merged commit 8833d3f into nushell:main Jul 5, 2024
@fdncred fdncred deleted the fix_duration_mod_div branch July 5, 2024 12:16
@lavafroth
Copy link
Copy Markdown
Contributor

lavafroth commented Jul 5, 2024 •
edited
Loading

I think we should add a separate case for the modulo operator on types like duration or filesize. Like 512MiB mod 3MiB should be 2MiB, a filesize but 512MiB / 3MiB should be 170.6667, a float.

@fdncred
Copy link
Copy Markdown
Contributor Author

fdncred commented Jul 5, 2024

have at it @lavafroth

@hustcer hustcer added this to the v0.96.0 milestone Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

notes:fixes Include the release notes summary in the "Bug fixes" section

Projects

None yet

Milestone

v0.96.0

Development

Successfully merging this pull request may close these issues.

<duration> mod <duration> gives duration, but said to be float

3 participants

@fdncred @lavafroth @hustcer