Move 'where' to parser keywords; Add 'filter' command (#7365)

# Description

This PR moves the `where` command to a parser keyword. While it still
uses the shape-directed parsing dictated by the signature, we're free to
change the parsing code now to a custom one once we remove the syntax
shapes.

As a side effect, the `where -b` flag was removed and its functionality
has moved to the new `filter` command.

Just FYI, other commands that take row conditions:
- `take until`
- `take while`
- `skip until`
- `skip while`
- `any`
- `all`

We can either move these to the parser as well or make them accept a
closure instead of row condition.

# User-Facing Changes

New `filter` command which replaces `where -b` functionality.

# 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 -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# 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.
This commit is contained in:
Jakub Žádník
2022-12-10 19:23:24 +02:00
committed by GitHub
parent 7e2781a2af
commit 6b4282eadf
8 changed files with 499 additions and 245 deletions

View File

@ -92,6 +92,7 @@ pub fn create_default_context() -> EngineState {
EachWhile,
Empty,
Every,
Filter,
Find,
First,
Flatten,

View File

@ -0,0 +1,282 @@
use super::utils::chain_error_with_input;
use nu_engine::{eval_block, CallExt};
use nu_protocol::ast::Call;
use nu_protocol::engine::{Closure, Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, Signature,
Span, SyntaxShape, Type, Value,
};
#[derive(Clone)]
pub struct Filter;
impl Command for Filter {
fn name(&self) -> &str {
"filter"
}
fn usage(&self) -> &str {
"Filter values based on a predicate closure."
}
fn extra_usage(&self) -> &str {
r#"This command works similar to 'where' but allows reading the predicate closure from
a variable. On the other hand, the "row condition" syntax is not supported."#
}
fn signature(&self) -> nu_protocol::Signature {
Signature::build("filter")
.input_output_types(vec![
(
Type::List(Box::new(Type::Any)),
Type::List(Box::new(Type::Any)),
),
(Type::Table(vec![]), Type::Table(vec![])),
])
.required(
"closure",
SyntaxShape::Closure(Some(vec![SyntaxShape::Any, SyntaxShape::Int])),
"Predicate closure",
)
.category(Category::Filters)
}
fn search_terms(&self) -> Vec<&str> {
vec!["where", "find", "search", "condition"]
}
fn run(
&self,
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
let capture_block: Closure = call.req(engine_state, stack, 0)?;
let metadata = input.metadata();
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let block = engine_state.get_block(capture_block.block_id).clone();
let mut stack = stack.captures_to_stack(&capture_block.captures);
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let span = call.head;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
match input {
PipelineData::Empty => Ok(PipelineData::Empty),
PipelineData::Value(Value::Range { .. }, ..)
| PipelineData::Value(Value::List { .. }, ..)
| PipelineData::ListStream { .. } => Ok(input
// To enumerate over the input (for the index argument),
// it must be converted into an iterator using into_iter().
.into_iter()
.enumerate()
.filter_map(move |(idx, x)| {
// with_env() is used here to ensure that each iteration uses
// a different set of environment variables.
// Hence, a 'cd' in the first loop won't affect the next loop.
stack.with_env(&orig_env_vars, &orig_env_hidden);
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, x.clone());
}
}
// Optional index argument
if let Some(var) = block.signature.get_positional(1) {
if let Some(var_id) = &var.var_id {
stack.add_var(
*var_id,
Value::Int {
val: idx as i64,
span,
},
);
}
}
match eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
Some(x)
} else {
None
}
}
Err(error) => Some(Value::Error {
error: chain_error_with_input(error, x.span()),
}),
}
})
.into_pipeline_data(ctrlc)),
PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::empty()),
PipelineData::ExternalStream {
stdout: Some(stream),
..
} => Ok(stream
.into_iter()
.enumerate()
.filter_map(move |(idx, x)| {
// see note above about with_env()
stack.with_env(&orig_env_vars, &orig_env_hidden);
let x = match x {
Ok(x) => x,
Err(err) => return Some(Value::Error { error: err }),
};
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, x.clone());
}
}
// Optional index argument
if let Some(var) = block.signature.get_positional(1) {
if let Some(var_id) = &var.var_id {
stack.add_var(
*var_id,
Value::Int {
val: idx as i64,
span,
},
);
}
}
match eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
Some(x)
} else {
None
}
}
Err(error) => Some(Value::Error {
error: chain_error_with_input(error, x.span()),
}),
}
})
.into_pipeline_data(ctrlc)),
// This match allows non-iterables to be accepted,
// which is currently considered undesirable (Nov 2022).
PipelineData::Value(x, ..) => {
// see note above about with_env()
stack.with_env(&orig_env_vars, &orig_env_hidden);
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, x.clone());
}
}
Ok(match eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
Some(x)
} else {
None
}
}
Err(error) => Some(Value::Error {
error: chain_error_with_input(error, x.span()),
}),
}
.into_pipeline_data(ctrlc))
}
}
.map(|x| x.set_metadata(metadata))
}
fn examples(&self) -> Vec<Example> {
vec![
Example {
description: "Filter items of a list according to a condition",
example: "[1 2] | filter {|x| $x > 1}",
result: Some(Value::List {
vals: vec![Value::test_int(2)],
span: Span::test_data(),
}),
},
Example {
description: "Filter rows of a table according to a condition",
example: "[{a: 1} {a: 2}] | filter {|x| $x.a > 1}",
result: Some(Value::List {
vals: vec![Value::Record {
cols: vec!["a".to_string()],
vals: vec![Value::test_int(2)],
span: Span::test_data(),
}],
span: Span::test_data(),
}),
},
Example {
description: "Filter rows of a table according to a stored condition",
example: "let cond = {|x| $x.a > 1}; [{a: 1} {a: 2}] | filter $cond",
result: Some(Value::List {
vals: vec![Value::Record {
cols: vec!["a".to_string()],
vals: vec![Value::test_int(2)],
span: Span::test_data(),
}],
span: Span::test_data(),
}),
},
// TODO: This should work but does not. (Note that `Let` must be present in the working_set in `example_test.rs`).
// See https://github.com/nushell/nushell/issues/7034
// Example {
// description: "List all numbers above 3, using an existing closure condition",
// example: "let a = {$in > 3}; [1, 2, 5, 6] | filter $a",
// result: Some(Value::List {
// vals: vec![
// Value::Int {
// val: 5,
// span: Span::test_data(),
// },
// Value::Int {
// val: 6,
// span: Span::test_data(),
// },
// ],
// span: Span::test_data(),
// }),
// },
]
}
}
#[cfg(test)]
mod test {
use super::*;
#[test]
fn test_examples() {
use crate::test_examples;
test_examples(Filter {})
}
}

View File

@ -10,6 +10,7 @@ mod each;
mod each_while;
mod empty;
mod every;
mod filter;
mod find;
mod first;
mod flatten;
@ -63,6 +64,7 @@ pub use each::Each;
pub use each_while::EachWhile;
pub use empty::Empty;
pub use every::Every;
pub use filter::Filter;
pub use find::Find;
pub use first::First;
pub use flatten::Flatten;

View File

@ -1,10 +1,9 @@
use super::utils::chain_error_with_input;
use nu_engine::{eval_block, CallExt};
use nu_protocol::ast::Call;
use nu_protocol::engine::{Closure, Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, ShellError,
Signature, Span, SyntaxShape, Type, Value,
Signature, Span, Spanned, SyntaxShape, Type, Value,
};
#[derive(Clone)]
@ -16,7 +15,13 @@ impl Command for Where {
}
fn usage(&self) -> &str {
"Filter values based on a condition."
"Filter values based on a row condition."
}
fn extra_usage(&self) -> &str {
r#"This command works similar to 'filter' but allows extra shorthands for working with
tables, known as "row conditions". On the other hand, reading the condition from a variable is
not supported."#
}
fn signature(&self) -> nu_protocol::Signature {
@ -28,11 +33,16 @@ impl Command for Where {
),
(Type::Table(vec![]), Type::Table(vec![])),
])
.optional("cond", SyntaxShape::RowCondition, "condition")
.required(
"row_condition",
SyntaxShape::RowCondition,
"Filter condition",
)
// TODO: Remove this flag after 0.73.0 release
.named(
"closure",
SyntaxShape::Closure(Some(vec![SyntaxShape::Any, SyntaxShape::Int])),
"use with a closure instead",
"use with a closure instead (deprecated: use 'filter' command instead)",
Some('b'),
)
.category(Category::Filters)
@ -49,235 +59,77 @@ impl Command for Where {
call: &Call,
input: PipelineData,
) -> Result<nu_protocol::PipelineData, nu_protocol::ShellError> {
if let Ok(Some(capture_block)) = call.get_flag::<Closure>(engine_state, stack, "closure") {
let metadata = input.metadata();
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let block = engine_state.get_block(capture_block.block_id).clone();
let mut stack = stack.captures_to_stack(&capture_block.captures);
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let span = call.head;
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
match input {
PipelineData::Empty => Ok(PipelineData::Empty),
PipelineData::Value(Value::Range { .. }, ..)
| PipelineData::Value(Value::List { .. }, ..)
| PipelineData::ListStream { .. } => Ok(input
// To enumerate over the input (for the index argument),
// it must be converted into an iterator using into_iter().
.into_iter()
.enumerate()
.filter_map(move |(idx, x)| {
// with_env() is used here to ensure that each iteration uses
// a different set of environment variables.
// Hence, a 'cd' in the first loop won't affect the next loop.
stack.with_env(&orig_env_vars, &orig_env_hidden);
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, x.clone());
}
}
// Optional index argument
if let Some(var) = block.signature.get_positional(1) {
if let Some(var_id) = &var.var_id {
stack.add_var(
*var_id,
Value::Int {
val: idx as i64,
span,
},
);
}
}
match eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
Some(x)
} else {
None
}
}
Err(error) => Some(Value::Error {
error: chain_error_with_input(error, x.span()),
}),
}
})
.into_pipeline_data(ctrlc)),
PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::empty()),
PipelineData::ExternalStream {
stdout: Some(stream),
..
} => Ok(stream
.into_iter()
.enumerate()
.filter_map(move |(idx, x)| {
// see note above about with_env()
stack.with_env(&orig_env_vars, &orig_env_hidden);
let x = match x {
Ok(x) => x,
Err(err) => return Some(Value::Error { error: err }),
};
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, x.clone());
}
}
// Optional index argument
if let Some(var) = block.signature.get_positional(1) {
if let Some(var_id) = &var.var_id {
stack.add_var(
*var_id,
Value::Int {
val: idx as i64,
span,
},
);
}
}
match eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
Some(x)
} else {
None
}
}
Err(error) => Some(Value::Error {
error: chain_error_with_input(error, x.span()),
}),
}
})
.into_pipeline_data(ctrlc)),
// This match allows non-iterables to be accepted,
// which is currently considered undesirable (Nov 2022).
PipelineData::Value(x, ..) => {
// see note above about with_env()
stack.with_env(&orig_env_vars, &orig_env_hidden);
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, x.clone());
}
}
Ok(match eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
x.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
) {
Ok(v) => {
if v.into_value(span).is_true() {
Some(x)
} else {
None
}
}
Err(error) => Some(Value::Error {
error: chain_error_with_input(error, x.span()),
}),
}
.into_pipeline_data(ctrlc))
}
}
.map(|x| x.set_metadata(metadata))
} else {
let capture_block: Option<Closure> = call.opt(engine_state, stack, 0)?;
if let Some(block) = capture_block {
let span = call.head;
let metadata = input.metadata();
let mut stack = stack.captures_to_stack(&block.captures);
let block = engine_state.get_block(block.block_id).clone();
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
Ok(input
.into_iter()
.enumerate()
.filter_map(move |(idx, value)| {
stack.with_env(&orig_env_vars, &orig_env_hidden);
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, value.clone());
}
}
// Optional index argument
if let Some(var) = block.signature.get_positional(1) {
if let Some(var_id) = &var.var_id {
stack.add_var(
*var_id,
Value::Int {
val: idx as i64,
span,
},
);
}
}
let result = eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
value.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
);
match result {
Ok(result) => {
let result = result.into_value(span);
if result.is_true() {
Some(value)
} else {
None
}
}
Err(err) => Some(Value::Error { error: err }),
}
})
.into_pipeline_data(ctrlc))
.map(|x| x.set_metadata(metadata))
} else {
Err(ShellError::MissingParameter(
"condition".to_string(),
call.head,
))
}
if let Some(closure) = call.get_flag::<Spanned<Closure>>(engine_state, stack, "closure")? {
return Err(ShellError::DeprecatedParameter(
"-b, --closure".to_string(),
"filter command".to_string(),
closure.span,
));
}
let closure: Closure = call.req(engine_state, stack, 0)?;
let span = call.head;
let metadata = input.metadata();
let mut stack = stack.captures_to_stack(&closure.captures);
let block = engine_state.get_block(closure.block_id).clone();
let orig_env_vars = stack.env_vars.clone();
let orig_env_hidden = stack.env_hidden.clone();
let ctrlc = engine_state.ctrlc.clone();
let engine_state = engine_state.clone();
let redirect_stdout = call.redirect_stdout;
let redirect_stderr = call.redirect_stderr;
Ok(input
.into_iter()
.enumerate()
.filter_map(move |(idx, value)| {
stack.with_env(&orig_env_vars, &orig_env_hidden);
if let Some(var) = block.signature.get_positional(0) {
if let Some(var_id) = &var.var_id {
stack.add_var(*var_id, value.clone());
}
}
// Optional index argument
if let Some(var) = block.signature.get_positional(1) {
if let Some(var_id) = &var.var_id {
stack.add_var(
*var_id,
Value::Int {
val: idx as i64,
span,
},
);
}
}
let result = eval_block(
&engine_state,
&mut stack,
&block,
// clone() is used here because x is given to Ok() below.
value.clone().into_pipeline_data(),
redirect_stdout,
redirect_stderr,
);
match result {
Ok(result) => {
let result = result.into_value(span);
if result.is_true() {
Some(value)
} else {
None
}
}
Err(err) => Some(Value::Error { error: err }),
}
})
.into_pipeline_data(ctrlc))
.map(|x| x.set_metadata(metadata))
}
fn examples(&self) -> Vec<Example> {
@ -302,14 +154,6 @@ impl Command for Where {
span: Span::test_data(),
}),
},
Example {
description: "Filter items of a list according to a stored condition",
example: "let cond = {|x| $x > 1}; [1 2] | where -b $cond",
result: Some(Value::List {
vals: vec![Value::test_int(2)],
span: Span::test_data(),
}),
},
Example {
description: "List all files in the current directory with sizes greater than 2kb",
example: "ls | where size > 2kb",