From 4f43d75130eb5e62bcb725b62c0dd7f14afacd3c Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Sun, 6 Mar 2022 20:01:29 -0500 Subject: [PATCH] Simplify group/window into their own commands (#4760) --- crates/nu-command/src/default_context.rs | 5 +- .../src/filters/{each_group.rs => group.rs} | 109 ++++-------- crates/nu-command/src/filters/mod.rs | 10 +- .../nu-command/src/filters/par_each_group.rs | 141 --------------- .../src/filters/{each_window.rs => window.rs} | 161 +++++++++--------- crates/nu-command/tests/commands/each.rs | 6 +- src/tests/test_engine.rs | 2 +- 7 files changed, 126 insertions(+), 308 deletions(-) rename crates/nu-command/src/filters/{each_group.rs => group.rs} (50%) delete mode 100644 crates/nu-command/src/filters/par_each_group.rs rename crates/nu-command/src/filters/{each_window.rs => window.rs} (61%) diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index 9c6e9489af..33ab8ee925 100644 --- a/crates/nu-command/src/default_context.rs +++ b/crates/nu-command/src/default_context.rs @@ -67,14 +67,13 @@ pub fn create_default_context(cwd: impl AsRef) -> EngineState { DropColumn, DropNth, Each, - EachGroup, - EachWindow, Empty, Every, Find, First, Flatten, Get, + Group, GroupBy, Headers, SplitBy, @@ -87,7 +86,6 @@ pub fn create_default_context(cwd: impl AsRef) -> EngineState { Length, Lines, ParEach, - ParEachGroup, Prepend, Range, Reduce, @@ -111,6 +109,7 @@ pub fn create_default_context(cwd: impl AsRef) -> EngineState { Update, UpdateCells, Where, + Window, Wrap, Zip, }; diff --git a/crates/nu-command/src/filters/each_group.rs b/crates/nu-command/src/filters/group.rs similarity index 50% rename from crates/nu-command/src/filters/each_group.rs rename to crates/nu-command/src/filters/group.rs index 20ee65990e..1125ae47e5 100644 --- a/crates/nu-command/src/filters/each_group.rs +++ b/crates/nu-command/src/filters/group.rs @@ -1,49 +1,62 @@ -use nu_engine::{eval_block, CallExt}; +use nu_engine::CallExt; use nu_protocol::ast::Call; -use nu_protocol::engine::{CaptureBlock, Command, EngineState, Stack}; +use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ Category, Example, IntoInterruptiblePipelineData, PipelineData, Signature, Span, Spanned, SyntaxShape, Value, }; #[derive(Clone)] -pub struct EachGroup; +pub struct Group; -impl Command for EachGroup { +impl Command for Group { fn name(&self) -> &str { - "each group" + "group" } fn signature(&self) -> Signature { - Signature::build("each group") + Signature::build("group") .required("group_size", SyntaxShape::Int, "the size of each group") - .required( - "block", - SyntaxShape::Block(Some(vec![SyntaxShape::Any])), - "the block to run on each group", - ) .category(Category::Filters) } fn usage(&self) -> &str { - "Runs a block on groups of `group_size` rows of a table at a time." + "Groups input into groups of `group_size`." } fn examples(&self) -> Vec { let stream_test_1 = vec![ - Value::Int { - val: 3, + Value::List { + vals: vec![ + Value::Int { + val: 1, + span: Span::test_data(), + }, + Value::Int { + val: 2, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, - Value::Int { - val: 7, + Value::List { + vals: vec![ + Value::Int { + val: 3, + span: Span::test_data(), + }, + Value::Int { + val: 4, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, ]; vec![Example { - example: "echo [1 2 3 4] | each group 2 { |it| $it.0 + $it.1 }", - description: "Echo the sum of each pair", + example: "echo [1 2 3 4] | group 2", + description: "Group the a list by pairs", result: Some(Value::List { vals: stream_test_1, span: Span::test_data(), @@ -59,18 +72,12 @@ impl Command for EachGroup { input: PipelineData, ) -> Result { let group_size: Spanned = call.req(engine_state, stack, 0)?; - let capture_block: CaptureBlock = call.req(engine_state, stack, 1)?; let ctrlc = engine_state.ctrlc.clone(); //FIXME: add in support for external redirection when engine-q supports it generally let each_group_iterator = EachGroupIterator { - block: capture_block, - engine_state: engine_state.clone(), - stack: stack.clone(), group_size: group_size.item, - redirect_stdout: call.redirect_stdout, - redirect_stderr: call.redirect_stderr, input: Box::new(input.into_iter()), span: call.head, }; @@ -80,12 +87,7 @@ impl Command for EachGroup { } struct EachGroupIterator { - block: CaptureBlock, - engine_state: EngineState, - stack: Stack, group_size: usize, - redirect_stdout: bool, - redirect_stderr: bool, input: Box + Send>, span: Span, } @@ -117,49 +119,10 @@ impl Iterator for EachGroupIterator { return None; } - Some(run_block_on_vec( - group, - self.block.clone(), - self.engine_state.clone(), - self.stack.clone(), - self.redirect_stdout, - self.redirect_stderr, - self.span, - )) - } -} - -pub(crate) fn run_block_on_vec( - input: Vec, - capture_block: CaptureBlock, - engine_state: EngineState, - stack: Stack, - redirect_stdout: bool, - redirect_stderr: bool, - span: Span, -) -> Value { - let value = Value::List { vals: input, span }; - - let mut stack = stack.captures_to_stack(&capture_block.captures); - - let block = engine_state.get_block(capture_block.block_id); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, value); - } - } - - match eval_block( - &engine_state, - &mut stack, - block, - PipelineData::new(span), - redirect_stdout, - redirect_stderr, - ) { - Ok(pipeline) => pipeline.into_value(span), - Err(error) => Value::Error { error }, + Some(Value::List { + vals: group, + span: self.span, + }) } } @@ -171,6 +134,6 @@ mod test { fn test_examples() { use crate::test_examples; - test_examples(EachGroup {}) + test_examples(Group {}) } } diff --git a/crates/nu-command/src/filters/mod.rs b/crates/nu-command/src/filters/mod.rs index cc29fa24d8..ea7ce97591 100644 --- a/crates/nu-command/src/filters/mod.rs +++ b/crates/nu-command/src/filters/mod.rs @@ -7,14 +7,13 @@ mod compact; mod default; mod drop; mod each; -mod each_group; -mod each_window; mod empty; mod every; mod find; mod first; mod flatten; mod get; +mod group; mod group_by; mod headers; mod keep; @@ -24,7 +23,6 @@ mod lines; mod merge; mod move_; mod par_each; -mod par_each_group; mod prepend; mod range; mod reduce; @@ -43,6 +41,7 @@ mod uniq; mod update; mod update_cells; mod where_; +mod window; mod wrap; mod zip_; @@ -55,14 +54,13 @@ pub use compact::Compact; pub use default::Default; pub use drop::*; pub use each::Each; -pub use each_group::EachGroup; -pub use each_window::EachWindow; pub use empty::Empty; pub use every::Every; pub use find::Find; pub use first::First; pub use flatten::Flatten; pub use get::Get; +pub use group::Group; pub use group_by::GroupBy; pub use headers::Headers; pub use keep::*; @@ -72,7 +70,6 @@ pub use lines::Lines; pub use merge::Merge; pub use move_::Move; pub use par_each::ParEach; -pub use par_each_group::ParEachGroup; pub use prepend::Prepend; pub use range::Range; pub use reduce::Reduce; @@ -91,5 +88,6 @@ pub use uniq::*; pub use update::Update; pub use update_cells::UpdateCells; pub use where_::Where; +pub use window::Window; pub use wrap::Wrap; pub use zip_::Zip; diff --git a/crates/nu-command/src/filters/par_each_group.rs b/crates/nu-command/src/filters/par_each_group.rs deleted file mode 100644 index f0ce9fa932..0000000000 --- a/crates/nu-command/src/filters/par_each_group.rs +++ /dev/null @@ -1,141 +0,0 @@ -use nu_engine::{eval_block, CallExt}; -use nu_protocol::ast::Call; -use nu_protocol::engine::{CaptureBlock, Command, EngineState, Stack}; -use nu_protocol::{ - Category, Example, IntoInterruptiblePipelineData, PipelineData, Signature, Spanned, - SyntaxShape, Value, -}; -use rayon::prelude::*; - -#[derive(Clone)] -pub struct ParEachGroup; - -impl Command for ParEachGroup { - fn name(&self) -> &str { - "par-each group" - } - - fn signature(&self) -> Signature { - Signature::build("par-each group") - .required("group_size", SyntaxShape::Int, "the size of each group") - .required( - "block", - SyntaxShape::Block(Some(vec![SyntaxShape::Any])), - "the block to run on each group", - ) - .category(Category::Filters) - } - - fn usage(&self) -> &str { - "Runs a block on groups of `group_size` rows of a table at a time." - } - - fn examples(&self) -> Vec { - vec![Example { - example: "echo [1 2 3 4] | par-each group 2 {|it| $it.0 + $it.1 }", - description: "Multiplies elements in list", - result: None, - }] - } - - fn run( - &self, - engine_state: &EngineState, - stack: &mut Stack, - call: &Call, - input: PipelineData, - ) -> Result { - let group_size: Spanned = call.req(engine_state, stack, 0)?; - let capture_block: CaptureBlock = call.req(engine_state, stack, 1)?; - let ctrlc = engine_state.ctrlc.clone(); - let span = call.head; - let redirect_stdout = call.redirect_stdout; - let redirect_stderr = call.redirect_stderr; - - let stack = stack.captures_to_stack(&capture_block.captures); - - //FIXME: add in support for external redirection when engine-q supports it generally - - let each_group_iterator = EachGroupIterator { - group_size: group_size.item, - input: Box::new(input.into_iter()), - }; - - Ok(each_group_iterator - .par_bridge() - .map(move |x| { - let block = engine_state.get_block(capture_block.block_id); - - let mut stack = stack.clone(); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, Value::List { vals: x, span }); - } - } - - match eval_block( - engine_state, - &mut stack, - block, - PipelineData::new(span), - redirect_stdout, - redirect_stderr, - ) { - Ok(v) => v.into_value(span), - Err(error) => Value::Error { error }, - } - }) - .collect::>() - .into_iter() - .into_pipeline_data(ctrlc)) - } -} - -struct EachGroupIterator { - group_size: usize, - input: Box + Send>, -} - -impl Iterator for EachGroupIterator { - type Item = Vec; - - fn next(&mut self) -> Option { - let mut group = vec![]; - let mut current_count = 0; - - loop { - let item = self.input.next(); - - match item { - Some(v) => { - group.push(v); - - current_count += 1; - if current_count >= self.group_size { - break; - } - } - None => break, - } - } - - if group.is_empty() { - return None; - } - - Some(group) - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn test_examples() { - use crate::test_examples; - - test_examples(ParEachGroup {}) - } -} diff --git a/crates/nu-command/src/filters/each_window.rs b/crates/nu-command/src/filters/window.rs similarity index 61% rename from crates/nu-command/src/filters/each_window.rs rename to crates/nu-command/src/filters/window.rs index fd6a5db68c..b1a049ddb4 100644 --- a/crates/nu-command/src/filters/each_window.rs +++ b/crates/nu-command/src/filters/window.rs @@ -1,21 +1,21 @@ -use nu_engine::{eval_block, CallExt}; +use nu_engine::CallExt; use nu_protocol::ast::Call; -use nu_protocol::engine::{CaptureBlock, Command, EngineState, Stack}; +use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ Category, Example, IntoInterruptiblePipelineData, PipelineData, Signature, Span, Spanned, SyntaxShape, Value, }; #[derive(Clone)] -pub struct EachWindow; +pub struct Window; -impl Command for EachWindow { +impl Command for Window { fn name(&self) -> &str { - "each window" + "window" } fn signature(&self) -> Signature { - Signature::build("each window") + Signature::build("window") .required("window_size", SyntaxShape::Int, "the size of each window") .named( "stride", @@ -23,52 +23,101 @@ impl Command for EachWindow { "the number of rows to slide over between windows", Some('s'), ) - .required( - "block", - SyntaxShape::Block(Some(vec![SyntaxShape::Any])), - "the block to run on each window", - ) .category(Category::Filters) } fn usage(&self) -> &str { - "Runs a block on window groups of `window_size` that slide by n rows." + "Creates a sliding window of `window_size` that slide by n rows/elements across input." } fn examples(&self) -> Vec { let stream_test_1 = vec![ - Value::Int { - val: 3, + Value::List { + vals: vec![ + Value::Int { + val: 1, + span: Span::test_data(), + }, + Value::Int { + val: 2, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, - Value::Int { - val: 5, + Value::List { + vals: vec![ + Value::Int { + val: 2, + span: Span::test_data(), + }, + Value::Int { + val: 3, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, - Value::Int { - val: 7, + Value::List { + vals: vec![ + Value::Int { + val: 3, + span: Span::test_data(), + }, + Value::Int { + val: 4, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, ]; let stream_test_2 = vec![ - Value::Int { - val: 3, + Value::List { + vals: vec![ + Value::Int { + val: 1, + span: Span::test_data(), + }, + Value::Int { + val: 2, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, - Value::Int { - val: 9, + Value::List { + vals: vec![ + Value::Int { + val: 4, + span: Span::test_data(), + }, + Value::Int { + val: 5, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, - Value::Int { - val: 15, + Value::List { + vals: vec![ + Value::Int { + val: 7, + span: Span::test_data(), + }, + Value::Int { + val: 8, + span: Span::test_data(), + }, + ], span: Span::test_data(), }, ]; vec![ Example { - example: "echo [1 2 3 4] | each window 2 { |it| $it.0 + $it.1 }", + example: "echo [1 2 3 4] | window 2", description: "A sliding window of two elements", result: Some(Value::List { vals: stream_test_1, @@ -76,7 +125,7 @@ impl Command for EachWindow { }), }, Example { - example: "[1, 2, 3, 4, 5, 6, 7, 8] | each window 2 --stride 3 { |x| $x.0 + $x.1 }", + example: "[1, 2, 3, 4, 5, 6, 7, 8] | window 2 --stride 3", description: "A sliding window of two elements, with a stride of 3", result: Some(Value::List { vals: stream_test_2, @@ -94,7 +143,6 @@ impl Command for EachWindow { input: PipelineData, ) -> Result { let group_size: Spanned = call.req(engine_state, stack, 0)?; - let capture_block: CaptureBlock = call.req(engine_state, stack, 1)?; let ctrlc = engine_state.ctrlc.clone(); let stride: Option = call.get_flag(engine_state, stack, "stride")?; @@ -103,13 +151,8 @@ impl Command for EachWindow { //FIXME: add in support for external redirection when engine-q supports it generally let each_group_iterator = EachWindowIterator { - block: capture_block, - engine_state: engine_state.clone(), - stack: stack.clone(), group_size: group_size.item, input: Box::new(input.into_iter()), - redirect_stdout: call.redirect_stdout, - redirect_stderr: call.redirect_stderr, span: call.head, previous: vec![], stride, @@ -120,13 +163,8 @@ impl Command for EachWindow { } struct EachWindowIterator { - block: CaptureBlock, - engine_state: EngineState, - stack: Stack, group_size: usize, input: Box + Send>, - redirect_stdout: bool, - redirect_stderr: bool, span: Span, previous: Vec, stride: usize, @@ -185,49 +223,10 @@ impl Iterator for EachWindowIterator { self.previous = group.clone(); - Some(run_block_on_vec( - group, - self.block.clone(), - self.engine_state.clone(), - self.stack.clone(), - self.redirect_stdout, - self.redirect_stderr, - self.span, - )) - } -} - -pub(crate) fn run_block_on_vec( - input: Vec, - capture_block: CaptureBlock, - engine_state: EngineState, - stack: Stack, - redirect_stdout: bool, - redirect_stderr: bool, - span: Span, -) -> Value { - let value = Value::List { vals: input, span }; - - let mut stack = stack.captures_to_stack(&capture_block.captures); - - let block = engine_state.get_block(capture_block.block_id); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, value); - } - } - - match eval_block( - &engine_state, - &mut stack, - block, - PipelineData::new(span), - redirect_stdout, - redirect_stderr, - ) { - Ok(pipeline) => pipeline.into_value(span), - Err(error) => Value::Error { error }, + Some(Value::List { + vals: group, + span: self.span, + }) } } @@ -239,6 +238,6 @@ mod test { fn test_examples() { use crate::test_examples; - test_examples(EachWindow {}) + test_examples(Window {}) } } diff --git a/crates/nu-command/tests/commands/each.rs b/crates/nu-command/tests/commands/each.rs index 0be56792ae..5ac0c14d91 100644 --- a/crates/nu-command/tests/commands/each.rs +++ b/crates/nu-command/tests/commands/each.rs @@ -17,7 +17,7 @@ fn each_group_works() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( r#" - echo [1 2 3 4 5 6] | each group 3 { |it| $it } | to json --raw + echo [1 2 3 4 5 6] | group 3 | to json --raw "# )); @@ -29,7 +29,7 @@ fn each_window() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( r#" - echo [1 2 3 4] | each window 3 { |it| $it } | to json --raw + echo [1 2 3 4] | window 3 | to json --raw "# )); @@ -41,7 +41,7 @@ fn each_window_stride() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( r#" - echo [1 2 3 4 5 6] | each window 3 -s 2 { |it| echo $it } | to json --raw + echo [1 2 3 4 5 6] | window 3 -s 2 | to json --raw "# )); diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs index 634b2d53df..ac0bc828a2 100644 --- a/src/tests/test_engine.rs +++ b/src/tests/test_engine.rs @@ -72,7 +72,7 @@ fn in_variable_6() -> TestResult { #[test] fn help_works_with_missing_requirements() -> TestResult { - run_test(r#"each --help | lines | length"#, "33") + run_test(r#"each --help | lines | length"#, "29") } #[test]