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

Plugin explicit flags #11581

Merged
merged 3 commits into from
Jan 22, 2024
Merged

Plugin explicit flags #11581

merged 3 commits into from
Jan 22, 2024

Conversation

NotLebedev
Copy link
Contributor

Description

#11492 fixed flags for builtin commands but I missed that plugins don't use the same has_flag that builtins do. This PR addresses this.

Unfortunately this means that return value of has_flag needs to change from bool to Result<bool, ShellError> to produce an error when explicit value is not a boolean (just like in case of has_flag for builtin commands. It is not possible to check this in EvaluatedCall::try_from_call because

User-Facing Changes

Passing explicit values to flags of plugin commands (like --flag=true --flag=false) should work now.
BREAKING: changed return value of EvaluatedCall::has_flag method from bool to Result<bool, ShellError>

Tests + Formatting

Added tests and updated documentation and examples

@fdncred fdncred added the pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes label Jan 19, 2024
@fdncred
Copy link
Collaborator

fdncred commented Jan 19, 2024

I'm fixing that clippy ci error in another pr now.

ugh, i hate breaking changes in plugins. it usually means every single plugin has to be recompiled.

@fdncred
Copy link
Collaborator

fdncred commented Jan 19, 2024

you'll need to rebase to integrate those changes

@NotLebedev
Copy link
Contributor Author

you'll need to rebase to integrate those changes

Done

@fdncred
Copy link
Collaborator

fdncred commented Jan 22, 2024

Unfortunately, this change is probably necessary for consistency. Thanks @NotLebedev!

@fdncred fdncred merged commit 092d496 into nushell:main Jan 22, 2024
19 checks passed
@hustcer hustcer added this to the v0.90.0 milestone Feb 3, 2024
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description
nushell#11492 fixed flags for builtin commands but I missed that plugins don't
use the same `has_flag` that builtins do. This PR addresses this.

Unfortunately this means that return value of `has_flag` needs to change
from `bool` to `Result<bool, ShellError>` to produce an error when
explicit value is not a boolean (just like in case of `has_flag` for
builtin commands. It is not possible to check this in
`EvaluatedCall::try_from_call` because

# User-Facing Changes
Passing explicit values to flags of plugin commands (like `--flag=true`
`--flag=false`) should work now.
BREAKING: changed return value of `EvaluatedCall::has_flag` method from
`bool` to `Result<bool, ShellError>`

# Tests + Formatting
Added tests and updated documentation and examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants