From 97eb8492a3a6cae82bc65198b898bbf51409dfb4 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Sat, 9 Apr 2022 09:41:05 +1200 Subject: [PATCH] Improve `$in` handling (#5137) * Simplify in logic * Add tests * more tests, and fixes --- crates/nu-command/src/filters/collect.rs | 8 +++--- crates/nu-command/src/formats/from/nuon.rs | 3 ++- crates/nu-command/tests/main.rs | 2 +- crates/nu-parser/src/parser.rs | 29 +++++++++++++++++----- src/tests/test_engine.rs | 26 +++++++++++++++++++ 5 files changed, 57 insertions(+), 11 deletions(-) diff --git a/crates/nu-command/src/filters/collect.rs b/crates/nu-command/src/filters/collect.rs index e426af9f0e..76b4428fcb 100644 --- a/crates/nu-command/src/filters/collect.rs +++ b/crates/nu-command/src/filters/collect.rs @@ -1,7 +1,9 @@ use nu_engine::{eval_block, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{CaptureBlock, Command, EngineState, Stack}; -use nu_protocol::{Category, Example, PipelineData, Signature, SyntaxShape, Value}; +use nu_protocol::{ + Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Value, +}; #[derive(Clone)] pub struct Collect; @@ -42,7 +44,7 @@ impl Command for Collect { if let Some(var) = block.signature.get_positional(0) { if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, input); + stack.add_var(*var_id, input.clone()); } } @@ -50,7 +52,7 @@ impl Command for Collect { engine_state, &mut stack, &block, - PipelineData::new(call.head), + input.into_pipeline_data(), call.redirect_stdout, call.redirect_stderr, ) diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 7337e1bad8..6accc7764c 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -89,7 +89,8 @@ impl Command for FromNuon { let (lite_block, err) = nu_parser::lite_parse(&lexed); error = error.or(err); - let (mut block, err) = nu_parser::parse_block(&mut working_set, &lite_block, true, &[]); + let (mut block, err) = + nu_parser::parse_block(&mut working_set, &lite_block, true, &[], false); error = error.or(err); if let Some(pipeline) = block.pipelines.get(1) { diff --git a/crates/nu-command/tests/main.rs b/crates/nu-command/tests/main.rs index 2028edcda2..9fd4b729d5 100644 --- a/crates/nu-command/tests/main.rs +++ b/crates/nu-command/tests/main.rs @@ -19,7 +19,7 @@ fn quickcheck_parse(data: String) -> bool { let mut working_set = StateWorkingSet::new(&context); working_set.add_file("quickcheck".into(), data.as_bytes()); - let _ = nu_parser::parse_block(&mut working_set, &lite_block, false, &[]); + let _ = nu_parser::parse_block(&mut working_set, &lite_block, false, &[], false); } } true diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 5f634e94d4..3a4193f416 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1776,7 +1776,8 @@ pub fn parse_full_cell_path( let (output, err) = lite_parse(&output); error = error.or(err); - let (output, err) = parse_block(working_set, &output, true, expand_aliases_denylist); + let (output, err) = + parse_block(working_set, &output, true, expand_aliases_denylist, true); error = error.or(err); let block_id = working_set.add_block(output); @@ -3660,7 +3661,8 @@ pub fn parse_block_expression( } } - let (mut output, err) = parse_block(working_set, &output, false, expand_aliases_denylist); + let (mut output, err) = + parse_block(working_set, &output, false, expand_aliases_denylist, false); error = error.or(err); if let Some(signature) = signature { @@ -4570,6 +4572,7 @@ pub fn parse_block( lite_block: &LiteBlock, scoped: bool, expand_aliases_denylist: &[usize], + is_subexpression: bool, ) -> (Block, Option) { trace!("parsing block: {:?}", lite_block); @@ -4614,9 +4617,17 @@ pub fn parse_block( }) .collect::>(); - for expr in output.iter_mut().skip(1) { - if expr.has_in_variable(working_set) { - *expr = wrap_expr_with_collect(working_set, expr); + if is_subexpression { + for expr in output.iter_mut().skip(1) { + if expr.has_in_variable(working_set) { + *expr = wrap_expr_with_collect(working_set, expr); + } + } + } else { + for expr in output.iter_mut() { + if expr.has_in_variable(working_set) { + *expr = wrap_expr_with_collect(working_set, expr); + } } } @@ -4656,7 +4667,12 @@ pub fn parse_block( } } continue; + } else if expr.has_in_variable(working_set) && !is_subexpression + { + *expr = wrap_expr_with_collect(working_set, expr); } + } else if expr.has_in_variable(working_set) && !is_subexpression { + *expr = wrap_expr_with_collect(working_set, expr); } } } @@ -5026,7 +5042,8 @@ pub fn parse( let (output, err) = lite_parse(&output); error = error.or(err); - let (mut output, err) = parse_block(working_set, &output, scoped, expand_aliases_denylist); + let (mut output, err) = + parse_block(working_set, &output, scoped, expand_aliases_denylist, false); error = error.or(err); let mut seen = vec![]; diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs index 8834d8e33b..efaf9d7978 100644 --- a/src/tests/test_engine.rs +++ b/src/tests/test_engine.rs @@ -340,3 +340,29 @@ fn default_value11() -> TestResult { fn default_value12() -> TestResult { fail_test(r#"def foo [--x:int = "a"] { $x }"#, "default value not int") } + +#[test] +fn loose_each() -> TestResult { + run_test(r#"[[1, 2, 3], [4, 5, 6]] | each { $in.1 } | math sum"#, "7") +} + +#[test] +fn in_means_input() -> TestResult { + run_test(r#"def shl [] { $in * 2 }; 2 | shl"#, "4") +} + +#[test] +fn in_iteration() -> TestResult { + run_test( + r#"[3, 4, 5] | each { echo $"hi ($in)" } | str collect"#, + "hi 3hi 4hi 5", + ) +} + +#[test] +fn reuseable_in() -> TestResult { + run_test( + r#"[1, 2, 3, 4] | take (($in | length) - 1) | math sum"#, + "6", + ) +}