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

don't overrite arg's type if it's annotated explicitly #10424

Merged
merged 3 commits into from Sep 20, 2023

Conversation

WindSoilder
Copy link
Collaborator

Description

Fixes: #10410

So the following script is possible:

def a [b: any = null] { let b = ($b | default "default_b"); }
a "given_b"

About the change

When parsing signature, and nushell meets something like a: any, it force the parser to treat a as any type. This is what arg_explicit_type means, it's only set when we goes into ParseMode::TypeMode, and it will be reset to false if the token goes to next argument.

so, when we have something like this: def a [b: any = null] { $b }, the type of $b won't be overwritten.

But if we have something like this: def a [b = null] { $b }, the type of $b is not annotated, so we make it to be nothing(which is the type of null)

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.

i really like these changes, small bug right but super confusing, thanks @WindSoilder 🙏

i propose to wait for after the release 😉

@sophiajt sophiajt merged commit bf40f03 into nushell:main Sep 20, 2023
19 checks passed
@amtoine amtoine mentioned this pull request Sep 20, 2023
@WindSoilder WindSoilder deleted the arg_type branch October 26, 2023 08:33
@hustcer hustcer modified the milestone: v0.86.0 Nov 15, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
# Description
Fixes: nushell#10410 

So the following script is possible:
```nushell
def a [b: any = null] { let b = ($b | default "default_b"); }
a "given_b"
```

## About the change
When parsing signature, and nushell meets something like `a: any`, it
force the parser to treat `a` as `any` type. This is what
`arg_explicit_type` means, it's only set when we goes into
`ParseMode::TypeMode`, and it will be reset to `false` if the token goes
to next argument.

so, when we have something like this: `def a [b: any = null] { $b }`,
the type of `$b` won't be overwritten.

But if we have something like this: `def a [b = null] { $b }`, the type
of `$b` is not annotated, so we make it to be `nothing`(which is the
type of null)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cusom command: default value shouldn't restrict argument's type
4 participants