Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow duration defaults #9249

Merged
merged 1 commit into from May 20, 2023
Merged

Allow duration defaults #9249

merged 1 commit into from May 20, 2023

Conversation

MariaSolOs
Copy link
Contributor

@MariaSolOs MariaSolOs commented May 20, 2023

Description

Fixes #9217 (some cases, still can't accept all default expressions)

User-Facing Changes

Tests + Formatting

  • A unit test was added to verify that defaults like 1sec are accepted.

@MariaSolOs
Copy link
Contributor Author

Ehhh should I be concerned about the failed coverage check? 😅

@fdncred
Copy link
Collaborator

fdncred commented May 20, 2023

oh, wow. that's a nice fix! thanks so much.

no, don't worry about the coverage failing. i restarted it to see if it would finish.

this seems to work great with any durations and filesizes. I'm totally down for incremental fixes because it's one step forward.

i did notice, unrelated to this pr, that it's kind of weird how our file sizes reduce. 1mb says 1000.0kb, 1kb says 1,000 b, etc. i mean, i guess it's right, but it's kind of weird.

❯ def f2 [ g = 1mb ] {echo $g}
❯ f2
1000.0 KB
❯ f2 1kb
1,000 B
❯ f2 1b
1 B
❯ f2 1_000_000b
1000.0 KB
❯ 1_000_000b
1000.0 KB
❯ 1mb
1000.0 KB

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks maria 😊

@fdncred fdncred merged commit 4954a76 into nushell:main May 20, 2023
15 of 16 checks passed
@MariaSolOs
Copy link
Contributor Author

Always happy to help!

@MariaSolOs MariaSolOs deleted the def-values branch May 20, 2023 15:08
@1Kinoti
Copy link
Contributor

1Kinoti commented May 20, 2023

i did notice, unrelated to this pr, that it's kind of weird how our file sizes reduce. 1mb says 1000.0kb, 1kb says 1,000 b, etc. i mean, i guess it's right, but it's kind of weird.

@fdncred this conversion is the one responsible for this reduction. the conversion values come from the third part of this tuple.

@fdncred
Copy link
Collaborator

fdncred commented May 20, 2023

@1Kinoti has it always reduced this way, i can't remember?

@1Kinoti
Copy link
Contributor

1Kinoti commented May 20, 2023

i just copied it over in #9190, so yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function input can't assign default closure/duration/filesize/range anymore
4 participants