mirror of
https://github.com/nushell/nushell.git
synced 2025-06-30 22:50:14 +02:00
Use Value::coerce_bool
in into bool
(#14731)
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> I realized that the `into bool` command somehow implements a conversion into a boolean value which was very similar to my implementation of ~`Value::as_env_bool`~ `Value::coerce_bool`. To streamline that behavior a bit, I replaced most of the implementation of `into bool` with my ~`Value::as_env_bool`~ `Value::coerce_bool` method. Also I added a new flag called `--relaxed` which lets the command behave more closely to the ~`Value::as_env_bool`~ `Value::coerce_bool` method as it allows null values and is more loose to strings. ~Which now begs the question, should I rename `Value::as_env_bool` just to `Value::coerce_bool` which would fit the `Value::coerce_str` method name?~ (Renamed that.) # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> The `into bool` command behaves the same but with `--relaxed` you can also throw a `null` or some more strings at it which makes it more ergonomic for env conversions. # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> I added some more tests to see that the strict handling works and added some more examples to the command to showcase the `--relaxed` flag which also gets tested. - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> @Bahex mentioned in #14704 that it broke the zoxide script, this PR should help to fix the issue.
This commit is contained in:
@ -1,4 +1,4 @@
|
||||
use nu_cmd_base::input_handler::{operate, CellPathOnlyArgs};
|
||||
use nu_cmd_base::input_handler::{operate, CmdArgument};
|
||||
use nu_engine::command_prelude::*;
|
||||
|
||||
#[derive(Clone)]
|
||||
@ -16,10 +16,16 @@ impl Command for SubCommand {
|
||||
(Type::Number, Type::Bool),
|
||||
(Type::String, Type::Bool),
|
||||
(Type::Bool, Type::Bool),
|
||||
(Type::Nothing, Type::Bool),
|
||||
(Type::List(Box::new(Type::Any)), Type::table()),
|
||||
(Type::table(), Type::table()),
|
||||
(Type::record(), Type::record()),
|
||||
])
|
||||
.switch(
|
||||
"relaxed",
|
||||
"Relaxes conversion to also allow null and any strings.",
|
||||
None,
|
||||
)
|
||||
.allow_variants_without_examples(true)
|
||||
.rest(
|
||||
"rest",
|
||||
@ -44,7 +50,10 @@ impl Command for SubCommand {
|
||||
call: &Call,
|
||||
input: PipelineData,
|
||||
) -> Result<PipelineData, ShellError> {
|
||||
into_bool(engine_state, stack, call, input)
|
||||
let relaxed = call
|
||||
.has_flag(engine_state, stack, "relaxed")
|
||||
.unwrap_or(false);
|
||||
into_bool(engine_state, stack, call, input, relaxed)
|
||||
}
|
||||
|
||||
fn examples(&self) -> Vec<Example> {
|
||||
@ -95,22 +104,47 @@ impl Command for SubCommand {
|
||||
example: "'true' | into bool",
|
||||
result: Some(Value::test_bool(true)),
|
||||
},
|
||||
Example {
|
||||
description: "interpret a null as false",
|
||||
example: "null | into bool --relaxed",
|
||||
result: Some(Value::test_bool(false)),
|
||||
},
|
||||
Example {
|
||||
description: "interpret any non-false, non-zero string as true",
|
||||
example: "'something' | into bool --relaxed",
|
||||
result: Some(Value::test_bool(true)),
|
||||
},
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
struct IntoBoolCmdArgument {
|
||||
cell_paths: Option<Vec<CellPath>>,
|
||||
relaxed: bool,
|
||||
}
|
||||
|
||||
impl CmdArgument for IntoBoolCmdArgument {
|
||||
fn take_cell_paths(&mut self) -> Option<Vec<CellPath>> {
|
||||
self.cell_paths.take()
|
||||
}
|
||||
}
|
||||
|
||||
fn into_bool(
|
||||
engine_state: &EngineState,
|
||||
stack: &mut Stack,
|
||||
call: &Call,
|
||||
input: PipelineData,
|
||||
relaxed: bool,
|
||||
) -> Result<PipelineData, ShellError> {
|
||||
let cell_paths: Vec<CellPath> = call.rest(engine_state, stack, 0)?;
|
||||
let args = CellPathOnlyArgs::from(cell_paths);
|
||||
let cell_paths = Some(call.rest(engine_state, stack, 0)?).filter(|v| !v.is_empty());
|
||||
let args = IntoBoolCmdArgument {
|
||||
cell_paths,
|
||||
relaxed,
|
||||
};
|
||||
operate(action, args, input, call.head, engine_state.signals())
|
||||
}
|
||||
|
||||
fn string_to_boolean(s: &str, span: Span) -> Result<bool, ShellError> {
|
||||
fn strict_string_to_boolean(s: &str, span: Span) -> Result<bool, ShellError> {
|
||||
match s.trim().to_ascii_lowercase().as_str() {
|
||||
"true" => Ok(true),
|
||||
"false" => Ok(false),
|
||||
@ -132,26 +166,31 @@ fn string_to_boolean(s: &str, span: Span) -> Result<bool, ShellError> {
|
||||
}
|
||||
}
|
||||
|
||||
fn action(input: &Value, _args: &CellPathOnlyArgs, span: Span) -> Value {
|
||||
match input {
|
||||
Value::Bool { .. } => input.clone(),
|
||||
Value::Int { val, .. } => Value::bool(*val != 0, span),
|
||||
Value::Float { val, .. } => Value::bool(val.abs() >= f64::EPSILON, span),
|
||||
Value::String { val, .. } => match string_to_boolean(val, span) {
|
||||
fn action(input: &Value, args: &IntoBoolCmdArgument, span: Span) -> Value {
|
||||
let err = || {
|
||||
Value::error(
|
||||
ShellError::OnlySupportsThisInputType {
|
||||
exp_input_type: "bool, int, float or string".into(),
|
||||
wrong_type: input.get_type().to_string(),
|
||||
dst_span: span,
|
||||
src_span: input.span(),
|
||||
},
|
||||
span,
|
||||
)
|
||||
};
|
||||
|
||||
match (input, args.relaxed) {
|
||||
(Value::Error { .. } | Value::Bool { .. }, _) => input.clone(),
|
||||
// In strict mode is this an error, while in relaxed this is just `false`
|
||||
(Value::Nothing { .. }, false) => err(),
|
||||
(Value::String { val, .. }, false) => match strict_string_to_boolean(val, span) {
|
||||
Ok(val) => Value::bool(val, span),
|
||||
Err(error) => Value::error(error, span),
|
||||
},
|
||||
// Propagate errors by explicitly matching them before the final case.
|
||||
Value::Error { .. } => input.clone(),
|
||||
other => Value::error(
|
||||
ShellError::OnlySupportsThisInputType {
|
||||
exp_input_type: "bool, int, float or string".into(),
|
||||
wrong_type: other.get_type().to_string(),
|
||||
dst_span: span,
|
||||
src_span: other.span(),
|
||||
},
|
||||
span,
|
||||
),
|
||||
_ => match input.coerce_bool() {
|
||||
Ok(val) => Value::bool(val, span),
|
||||
Err(_) => err(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@ -165,4 +204,32 @@ mod test {
|
||||
|
||||
test_examples(SubCommand {})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_strict_handling() {
|
||||
let span = Span::test_data();
|
||||
let args = IntoBoolCmdArgument {
|
||||
cell_paths: vec![].into(),
|
||||
relaxed: false,
|
||||
};
|
||||
|
||||
assert!(action(&Value::test_nothing(), &args, span).is_error());
|
||||
assert!(action(&Value::test_string("abc"), &args, span).is_error());
|
||||
assert!(action(&Value::test_string("true"), &args, span).is_true());
|
||||
assert!(action(&Value::test_string("FALSE"), &args, span).is_false());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_relaxed_handling() {
|
||||
let span = Span::test_data();
|
||||
let args = IntoBoolCmdArgument {
|
||||
cell_paths: vec![].into(),
|
||||
relaxed: true,
|
||||
};
|
||||
|
||||
assert!(action(&Value::test_nothing(), &args, span).is_false());
|
||||
assert!(action(&Value::test_string("abc"), &args, span).is_true());
|
||||
assert!(action(&Value::test_string("true"), &args, span).is_true());
|
||||
assert!(action(&Value::test_string("FALSE"), &args, span).is_false());
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user