From 2c238aea6a445bba1ac35bfce7783c696a689863 Mon Sep 17 00:00:00 2001 From: Leon Date: Thu, 10 Nov 2022 08:05:15 +1000 Subject: [PATCH] Fixed $in in `where` blocks (#6976) --- crates/nu-command/src/core_commands/for_.rs | 6 +++++ crates/nu-command/src/filters/all.rs | 3 +++ crates/nu-command/src/filters/any.rs | 3 +++ crates/nu-command/src/filters/each.rs | 6 +++++ crates/nu-command/src/filters/each_while.rs | 6 +++++ crates/nu-command/src/filters/insert.rs | 3 +++ crates/nu-command/src/filters/reduce.rs | 3 +++ crates/nu-command/src/filters/update.rs | 3 +++ crates/nu-command/src/filters/upsert.rs | 3 +++ crates/nu-command/src/filters/where_.rs | 18 ++++++++++++- crates/nu-command/tests/commands/all.rs | 10 +++++++ crates/nu-command/tests/commands/any.rs | 10 +++++++ crates/nu-command/tests/commands/where_.rs | 30 +++++++++++++++++++++ 13 files changed, 103 insertions(+), 1 deletion(-) diff --git a/crates/nu-command/src/core_commands/for_.rs b/crates/nu-command/src/core_commands/for_.rs index bf807dd5a7..b5c44ae757 100644 --- a/crates/nu-command/src/core_commands/for_.rs +++ b/crates/nu-command/src/core_commands/for_.rs @@ -93,6 +93,9 @@ impl Command for For { Ok(ListStream::from_stream(vals.into_iter(), ctrlc.clone()) .enumerate() .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); stack.add_var( @@ -134,6 +137,9 @@ impl Command for For { .into_range_iter(ctrlc.clone())? .enumerate() .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); stack.add_var( diff --git a/crates/nu-command/src/filters/all.rs b/crates/nu-command/src/filters/all.rs index f8f2ae6187..ce6ab9c83b 100644 --- a/crates/nu-command/src/filters/all.rs +++ b/crates/nu-command/src/filters/all.rs @@ -82,6 +82,9 @@ impl Command for All { let engine_state = engine_state.clone(); for value in input.into_interruptible_iter(ctrlc) { + // 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_id) = var_id { diff --git a/crates/nu-command/src/filters/any.rs b/crates/nu-command/src/filters/any.rs index 335c6597a0..62654f3437 100644 --- a/crates/nu-command/src/filters/any.rs +++ b/crates/nu-command/src/filters/any.rs @@ -81,6 +81,9 @@ impl Command for Any { let engine_state = engine_state.clone(); for value in input.into_interruptible_iter(ctrlc) { + // 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_id) = var_id { diff --git a/crates/nu-command/src/filters/each.rs b/crates/nu-command/src/filters/each.rs index f591b4ffaf..1fb9b1ea52 100644 --- a/crates/nu-command/src/filters/each.rs +++ b/crates/nu-command/src/filters/each.rs @@ -151,6 +151,9 @@ with 'transpose' first."# .into_iter() .enumerate() .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) { @@ -201,6 +204,9 @@ with 'transpose' first."# .into_iter() .enumerate() .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); let x = match x { diff --git a/crates/nu-command/src/filters/each_while.rs b/crates/nu-command/src/filters/each_while.rs index 70b7e29cd4..42c4b8cda5 100644 --- a/crates/nu-command/src/filters/each_while.rs +++ b/crates/nu-command/src/filters/each_while.rs @@ -118,6 +118,9 @@ impl Command for EachWhile { .into_iter() .enumerate() .map_while(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) { @@ -172,6 +175,9 @@ impl Command for EachWhile { .into_iter() .enumerate() .map_while(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); let x = match x { diff --git a/crates/nu-command/src/filters/insert.rs b/crates/nu-command/src/filters/insert.rs index 8682d51862..6c07b15d88 100644 --- a/crates/nu-command/src/filters/insert.rs +++ b/crates/nu-command/src/filters/insert.rs @@ -100,6 +100,9 @@ fn insert( input.map( move |mut input| { + // 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) { diff --git a/crates/nu-command/src/filters/reduce.rs b/crates/nu-command/src/filters/reduce.rs index 8eef1e9936..3958bc1122 100644 --- a/crates/nu-command/src/filters/reduce.rs +++ b/crates/nu-command/src/filters/reduce.rs @@ -165,6 +165,9 @@ impl Command for Reduce { .peekable(); while let Some((idx, x)) = input_iter.next() { + // 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) { diff --git a/crates/nu-command/src/filters/update.rs b/crates/nu-command/src/filters/update.rs index 027eb65bd3..be049ac09f 100644 --- a/crates/nu-command/src/filters/update.rs +++ b/crates/nu-command/src/filters/update.rs @@ -99,6 +99,9 @@ fn update( input.map( move |mut input| { + // 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) { diff --git a/crates/nu-command/src/filters/upsert.rs b/crates/nu-command/src/filters/upsert.rs index ac85701569..e565a1ab57 100644 --- a/crates/nu-command/src/filters/upsert.rs +++ b/crates/nu-command/src/filters/upsert.rs @@ -119,6 +119,9 @@ fn upsert( input.map( move |mut input| { + // 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) { diff --git a/crates/nu-command/src/filters/where_.rs b/crates/nu-command/src/filters/where_.rs index 73f5114e5a..bed3f723ac 100644 --- a/crates/nu-command/src/filters/where_.rs +++ b/crates/nu-command/src/filters/where_.rs @@ -68,6 +68,9 @@ impl Command for Where { | PipelineData::ListStream { .. } => Ok(input .into_iter() .filter_map(move |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) { @@ -80,6 +83,7 @@ impl Command for Where { &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, @@ -106,6 +110,7 @@ impl Command for Where { } => Ok(stream .into_iter() .filter_map(move |x| { + // see note above about with_env() stack.with_env(&orig_env_vars, &orig_env_hidden); let x = match x { @@ -123,6 +128,7 @@ impl Command for Where { &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, @@ -141,6 +147,9 @@ impl Command for Where { }) .into_pipeline_data(ctrlc)), 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()); @@ -150,6 +159,7 @@ impl Command for Where { &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, @@ -178,6 +188,9 @@ impl Command for Where { 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(); @@ -186,6 +199,8 @@ impl Command for Where { Ok(input .into_iter() .filter_map(move |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()); @@ -195,7 +210,8 @@ impl Command for Where { &engine_state, &mut stack, &block, - PipelineData::new(span), + // clone() is used here because x is given to Ok() below. + value.clone().into_pipeline_data(), redirect_stdout, redirect_stderr, ); diff --git a/crates/nu-command/tests/commands/all.rs b/crates/nu-command/tests/commands/all.rs index a4bbd9c357..3dfb3ad093 100644 --- a/crates/nu-command/tests/commands/all.rs +++ b/crates/nu-command/tests/commands/all.rs @@ -107,3 +107,13 @@ fn early_exits_with_0_param_blocks() { assert_eq!(actual.out, "1false"); } + +#[test] +fn unique_env_each_iteration() { + let actual = nu!( + cwd: "tests/fixtures/formats", + "[1 2] | all { print ($env.PWD | str ends-with 'formats') | cd '/' | true } | to nuon" + ); + + assert_eq!(actual.out, "truetruetrue"); +} diff --git a/crates/nu-command/tests/commands/any.rs b/crates/nu-command/tests/commands/any.rs index 5156816cf9..dee6e59e65 100644 --- a/crates/nu-command/tests/commands/any.rs +++ b/crates/nu-command/tests/commands/any.rs @@ -83,3 +83,13 @@ fn early_exits_with_0_param_blocks() { assert_eq!(actual.out, "1true"); } + +#[test] +fn unique_env_each_iteration() { + let actual = nu!( + cwd: "tests/fixtures/formats", + "[1 2] | any { print ($env.PWD | str ends-with 'formats') | cd '/' | false } | to nuon" + ); + + assert_eq!(actual.out, "truetruefalse"); +} diff --git a/crates/nu-command/tests/commands/where_.rs b/crates/nu-command/tests/commands/where_.rs index 227566e24f..5cc37747d0 100644 --- a/crates/nu-command/tests/commands/where_.rs +++ b/crates/nu-command/tests/commands/where_.rs @@ -22,6 +22,36 @@ fn filters_with_nothing_comparison() { assert_eq!(actual.out, "7"); } +#[test] +fn filters_with_0_arity_block() { + let actual = nu!( + cwd: ".", + "[1 2 3 4] | where { $in < 3 } | to nuon" + ); + + assert_eq!(actual.out, "[1, 2]"); +} + +#[test] +fn filters_with_1_arity_block() { + let actual = nu!( + cwd: ".", + "[1 2 3 6 7 8] | where {|e| $e < 5 } | to nuon" + ); + + assert_eq!(actual.out, "[1, 2, 3]"); +} + +#[test] +fn unique_env_each_iteration() { + let actual = nu!( + cwd: "tests/fixtures/formats", + "[1 2] | where { print ($env.PWD | str ends-with 'formats') | cd '/' | true } | to nuon" + ); + + assert_eq!(actual.out, "truetrue[1, 2]"); +} + #[test] fn where_in_table() { let actual = nu!(