-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Deprecate --flag: bool
in custom command
#11365
Conversation
i don't think this is a breaking change as both behaviour are still allowed 😉 the following PR that removes |
There was a problem hiding this 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>
There was a problem hiding this 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 🙏
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
related to - nushell/nushell#11365 ## description this PR will suppress the deprecation warning from nushell/nushell#11365 which is deprecating `: bool` annotations for switches
related to - nushell/nushell#11365
related to - nushell/nushell#11365 ## description this PR will suppress the deprecation warning from nushell/nushell#11365 which is deprecating `: bool` annotations for switches
# 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
# 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>
# 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
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.
cc @kubouch
Tests + Formatting
Done
After Submitting
--dry-run: bool
is not allowed,: bool
from custom commands between 0.89 and 0.90--flag: bool
support #11541