Skip to content

Deprecate --flag: bool in custom command#11365

Merged
amtoine merged 3 commits intonushell:mainfrom
WindSoilder:parse_warnings
Dec 21, 2023
Merged

Deprecate --flag: bool in custom command#11365
amtoine merged 3 commits intonushell:mainfrom
WindSoilder:parse_warnings

Conversation

@WindSoilder
Copy link
Copy Markdown
Contributor

@WindSoilder WindSoilder commented Dec 18, 2023

Description

While #11057 is merged, it's hard to tell the difference between --flag: bool and --flag, and it makes user hard to read custom commands' signature, and hard to use them correctly.

After discussion, I think we can deprecate --flag: bool usage, and encourage using --flag instead.

User-Facing Changes

The following code will raise warning message, but don't stop from running.

 def florb [--dry-run: bool, --another-flag] { "aaa" };  florb
Error:   × Deprecated: --flag: bool
   ╭─[entry #7:1:1]
 1  def florb [--dry-run: bool, --another-flag] { "aaa" };  florb
   ·                       ──┬─
   ·                         ╰── `--flag: bool` is deprecated. Please use `--flag` instead, more info: https://www.nushell.sh/book/custom_commands.html
   ╰────

aaa

cc @kubouch

Tests + Formatting

Done

After Submitting

@fdncred fdncred added the notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes label Dec 18, 2023
@amtoine
Copy link
Copy Markdown
Member

amtoine commented Dec 20, 2023

i don't think this is a breaking change as both behaviour are still allowed 😉

the following PR that removes --flag: bool entirely will be a breaking change though 👍

@amtoine amtoine added notes:mention Include the release notes summary in the "Hall of Fame" section category:deprecation Related to the deprecation of commands/features/options and removed notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes labels Dec 20, 2023
Copy link
Copy Markdown
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.

the changes look great to me 👌

most of them will disappear anyway once the annotation gets removed.

Co-authored-by: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Copy link
Copy Markdown
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.

looks good, thanks @WindSoilder 🙏

@amtoine amtoine merged commit 5d98a72 into nushell:main Dec 21, 2023
amtoine added a commit to nushell/nupm that referenced this pull request Dec 23, 2023
related to
- nushell/nushell#11365
- amtoine/nu-git-manager#154

## description
this PR will suppress the deprecation warning from
nushell/nushell#11365 which is deprecating `:
bool` annotations for switches.

this didn't break anything but did show a bunch of spurious deprecation
warnings in CI for instance:
https://github.com/amtoine/nu-git-manager/actions/runs/7307569422/job/19913395508#step:6:17
amtoine added a commit to amtoine/nu-git-manager that referenced this pull request Dec 23, 2023
related to
- nushell/nushell#11365

## description
this PR will suppress the deprecation warning from
nushell/nushell#11365 which is deprecating `:
bool` annotations for switches
amtoine added a commit to amtoine/scripts that referenced this pull request Dec 23, 2023
@WindSoilder WindSoilder deleted the parse_warnings branch January 2, 2024 13:35
amtoine added a commit to amtoine/nu-git-manager that referenced this pull request Jan 21, 2024
related to
- nushell/nushell#11365

## description
this PR will suppress the deprecation warning from
nushell/nushell#11365 which is deprecating `:
bool` annotations for switches
WindSoilder added a commit that referenced this pull request Jan 25, 2024
# Description
This is a follow up to: #11365

After this pr, `--flag: bool` is no longer allowed.

I think `ParseWarning::Deprecated` is useful when we want to deprecated
something at syntax level, so I just leave it there for now.

# User-Facing Changes
## Before
```
❯ def foo [--b: bool] {}
Error:   × Deprecated: --flag: bool
   ╭─[entry #15:1:1]
 1 │ def foo [--b: bool] {}
   ·               ──┬─
   ·                 ╰── `--flag: bool` is deprecated and will be removed in 0.90. Please use `--flag` instead, more info: https://www.nushell.sh/book/custom_commands.html
   ╰────
```

## After
```
❯ def foo [--b: bool] {}
Error:   × Type annotations are not allowed for boolean switches.
   ╭─[entry #2:1:1]
 1 │ def foo [--b: bool] {}
   ·               ──┬─
   ·                 ╰── Remove the `: bool` type annotation.
   ╰────
```
# Tests + Formatting
Done
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description
While nushell#11057 is merged, it's hard to tell the difference between
`--flag: bool` and `--flag`, and it makes user hard to read custom
commands' signature, and hard to use them correctly.

After discussion, I think we can deprecate `--flag: bool` usage, and
encourage using `--flag` instead.

# User-Facing Changes
The following code will raise warning message, but don't stop from
running.
```nushell
❯ def florb [--dry-run: bool, --another-flag] { "aaa" };  florb
Error:   × Deprecated: --flag: bool
   ╭─[entry nushell#7:1:1]
 1 │ def florb [--dry-run: bool, --another-flag] { "aaa" };  florb
   ·                       ──┬─
   ·                         ╰── `--flag: bool` is deprecated. Please use `--flag` instead, more info: https://www.nushell.sh/book/custom_commands.html
   ╰────

aaa
```

cc @kubouch 

# Tests + Formatting
Done

# After Submitting
- [ ] Add more information under
https://www.nushell.sh/book/custom_commands.html to indicate `--dry-run:
bool` is not allowed,
- [ ] remove `: bool` from custom commands between 0.89 and 0.90

---------

Co-authored-by: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description
This is a follow up to: nushell#11365

After this pr, `--flag: bool` is no longer allowed.

I think `ParseWarning::Deprecated` is useful when we want to deprecated
something at syntax level, so I just leave it there for now.

# User-Facing Changes
## Before
```
❯ def foo [--b: bool] {}
Error:   × Deprecated: --flag: bool
   ╭─[entry nushell#15:1:1]
 1 │ def foo [--b: bool] {}
   ·               ──┬─
   ·                 ╰── `--flag: bool` is deprecated and will be removed in 0.90. Please use `--flag` instead, more info: https://www.nushell.sh/book/custom_commands.html
   ╰────
```

## After
```
❯ def foo [--b: bool] {}
Error:   × Type annotations are not allowed for boolean switches.
   ╭─[entry nushell#2:1:1]
 1 │ def foo [--b: bool] {}
   ·               ──┬─
   ·                 ╰── Remove the `: bool` type annotation.
   ╰────
```
# Tests + Formatting
Done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:deprecation Related to the deprecation of commands/features/options notes:mention Include the release notes summary in the "Hall of Fame" section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants