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.

Sign up for GitHub

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

Jump to bottom

options are broken? #10450

Closed
amtoine opened this issue Sep 20, 2023 · 3 comments · Fixed by #10456
Closed

options are broken? #10450

amtoine opened this issue Sep 20, 2023 · 3 comments · Fixed by #10456
Labels
parser Issues related to parsing
Milestone
v0.86.0

Comments

@amtoine
Copy link
Member

amtoine commented Sep 20, 2023 •
edited

related to

Describe the bug

@WindSoilder
i think #10424 did break Nushell options 😢

How to reproduce

  1. define
def foo [--option: bool = false] { $option }

before

> foo
false
> foo --option
Error: nu::parser::missing_flag_param

  × Missing flag argument.
   ╭─[entry #8:1:1]
 1 │ foo --option
   ·     ────┬───
   ·         ╰── flag missing bool argument
   ╰────
> foo --option true
true

after

> foo
false
> foo --option
false
> foo --option false
Error: nu::parser::extra_positional

  × Extra positional argument.
   ╭─[entry #4:1:1]
 1 │ foo --option false
   ·              ──┬──
   ·                ╰── extra positional argument
   ╰────
  help: Usage: foo {flags}

Expected behavior

i think i expected

foo --option

to give true and

foo --option false

to not give an error

Screenshots

No response

Configuration

key value
version 0.85.1
branch
commit_hash 989a147
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.70.0 (90c541806 2023-05-31)
rust_channel 1.70.0-x86_64-unknown-linux-gnu
cargo_version cargo 1.70.0 (ec8a8a0ca 2023-04-25)
build_time 2023-09-20 22:42:11 +02:00
build_rust_channel release
allocator mimalloc
features default, sqlite, trash, which, zip
installed_plugins gstat, nu_plugin_explore

Additional context

No response
@amtoine amtoine added the needs-triage An issue that hasn't had any proper look label Sep 20, 2023
@amtoine
Copy link
Member Author

amtoine commented Sep 20, 2023

looks to me it's only with boolean options, because

def foo [-x: string = "foo"] { $x }

works fine

@WindSoilder
Copy link
Collaborator

Oh....My bad, thanks for the issue. I'll take a look into this

@WindSoilder WindSoilder added parser Issues related to parsing and removed needs-triage An issue that hasn't had any proper look labels Sep 20, 2023
@WindSoilder WindSoilder mentioned this issue Sep 21, 2023
Merged
@amtoine
Copy link
Member Author

amtoine commented Sep 21, 2023

we'll have a look shortly 😌

amtoine added a commit that referenced this issue Sep 23, 2023
@WindSoilder @amtoine
differentiating between --x and --x: bool (#10456)
d2c87ad 
# Description
Fixes: #10450 

This pr differentiating between `--x: bool` and `--x`

Here are examples which demostrate difference between them:
```nushell
def a [--x: bool] { $x };
a --x    # not allowed, you need to parse a value to the flag.
a        # it's allowed, and the value of `$x` is false, which behaves the same to `def a [--x] { $x }; a`
```

For boolean flag with default value, it works a little bit different to
#10450 mentioned:
```nushell
def foo [--option: bool = false] { $option }
foo                  # output false
foo --option         # not allowed, you need to parse a value to the flag.
foo --option true    # output true
```

# User-Facing Changes
After the pr, the following code is not allowed:
```nushell
def a [--x: bool] { $x }; a --x
```

Instead, you have to pass a value to flag `--x` like `a --x false`. But
bare flag works in the same way as before.

## Update: one more breaking change to help on #7260 
```
def foo [--option: bool] { $option == null }
foo
```
After the pr, if we don't use a boolean flag, the value will be `null`
instead of `true`. Because here `--option: bool` is treated as a flag
rather than a switch

---------

Co-authored-by: amtoine <stevan.antoine@gmail.com>
@hustcer hustcer added this to the v0.86.0 milestone Sep 24, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this issue Dec 14, 2023
@WindSoilder @amtoine @hardfau1t
differentiating between --x and --x: bool (nushell#10456)
4a3e906 
# Description
Fixes: nushell#10450 

This pr differentiating between `--x: bool` and `--x`

Here are examples which demostrate difference between them:
```nushell
def a [--x: bool] { $x };
a --x    # not allowed, you need to parse a value to the flag.
a        # it's allowed, and the value of `$x` is false, which behaves the same to `def a [--x] { $x }; a`
```

For boolean flag with default value, it works a little bit different to
nushell#10450 mentioned:
```nushell
def foo [--option: bool = false] { $option }
foo                  # output false
foo --option         # not allowed, you need to parse a value to the flag.
foo --option true    # output true
```

# User-Facing Changes
After the pr, the following code is not allowed:
```nushell
def a [--x: bool] { $x }; a --x
```

Instead, you have to pass a value to flag `--x` like `a --x false`. But
bare flag works in the same way as before.

## Update: one more breaking change to help on nushell#7260 
```
def foo [--option: bool] { $option == null }
foo
```
After the pr, if we don't use a boolean flag, the value will be `null`
instead of `true`. Because here `--option: bool` is treated as a flag
rather than a switch

---------

Co-authored-by: amtoine <stevan.antoine@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
parser Issues related to parsing
Projects
None yet
Milestone
v0.86.0
Development

Successfully merging a pull request may close this issue.

differentiating between --x and --x: bool WindSoilder/nushell
3 participants
@hustcer @WindSoilder @amtoine