From ecc820a8c14e5d5d1619e0e0d71c74baad87217a Mon Sep 17 00:00:00 2001 From: TrMen <31260183+TrMen@users.noreply.github.com> Date: Wed, 26 Apr 2023 23:27:27 +0200 Subject: [PATCH] Fix unexpected flattening of data by par-each (Issue #8497) (#9007) # Description Previously, `par-each` acted like a `flatmap`: first mapping the data, then applying a `flatten`. This is unlike `each`, which just maps the data. Now `par-each` works like `each` in this regard, leaving nested data unflattened. Fixes #8497 # User-Facing Changes Previously: `[1 2 3] | par-each {|e| [$e, $e] }` --> `[1,1,2,2,3,3]` Now: `[1 2 3] | par-each {|e| [$e, $e] }` --> `[[1,1],[2,2],[3,3]]` # Tests This adds one test that verifies the lack of flattening for `par-each`. --- crates/nu-command/src/filters/par_each.rs | 29 ++++++++------------ crates/nu-command/tests/commands/mod.rs | 1 + crates/nu-command/tests/commands/par_each.rs | 12 ++++++++ 3 files changed, 25 insertions(+), 17 deletions(-) create mode 100644 crates/nu-command/tests/commands/par_each.rs diff --git a/crates/nu-command/src/filters/par_each.rs b/crates/nu-command/src/filters/par_each.rs index 18a338ed99..324f8f0fd4 100644 --- a/crates/nu-command/src/filters/par_each.rs +++ b/crates/nu-command/src/filters/par_each.rs @@ -114,8 +114,10 @@ impl Command for ParEach { let max_threads = threads.unwrap_or(0); let metadata = input.metadata(); let ctrlc = engine_state.ctrlc.clone(); + let outer_ctrlc = engine_state.ctrlc.clone(); let block_id = capture_block.block_id; let mut stack = stack.captures_to_stack(&capture_block.captures); + let span = call.head; let redirect_stdout = call.redirect_stdout; let redirect_stderr = call.redirect_stderr; @@ -146,16 +148,15 @@ impl Command for ParEach { redirect_stdout, redirect_stderr, ) { - Ok(v) => v, + Ok(v) => v.into_value(span), + Err(error) => Value::Error { error: Box::new(chain_error_with_input(error, val_span)), - } - .into_pipeline_data(), + }, } }) .collect::>() .into_iter() - .flatten() .into_pipeline_data(ctrlc) })), PipelineData::Value(Value::List { vals: val, .. }, ..) => Ok(create_pool(max_threads)? @@ -181,16 +182,14 @@ impl Command for ParEach { redirect_stdout, redirect_stderr, ) { - Ok(v) => v, + Ok(v) => v.into_value(span), Err(error) => Value::Error { error: Box::new(chain_error_with_input(error, val_span)), - } - .into_pipeline_data(), + }, } }) .collect::>() .into_iter() - .flatten() .into_pipeline_data(ctrlc) })), PipelineData::ListStream(stream, ..) => Ok(create_pool(max_threads)?.install(|| { @@ -216,16 +215,14 @@ impl Command for ParEach { redirect_stdout, redirect_stderr, ) { - Ok(v) => v, + Ok(v) => v.into_value(span), Err(error) => Value::Error { error: Box::new(chain_error_with_input(error, val_span)), - } - .into_pipeline_data(), + }, } }) .collect::>() .into_iter() - .flatten() .into_pipeline_data(ctrlc) })), PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::empty()), @@ -242,7 +239,6 @@ impl Command for ParEach { return Value::Error { error: Box::new(err), } - .into_pipeline_data() } }; @@ -264,16 +260,14 @@ impl Command for ParEach { redirect_stdout, redirect_stderr, ) { - Ok(v) => v, + Ok(v) => v.into_value(span), Err(error) => Value::Error { error: Box::new(error), - } - .into_pipeline_data(), + }, } }) .collect::>() .into_iter() - .flatten() .into_pipeline_data(ctrlc) })), // This match allows non-iterables to be accepted, @@ -297,6 +291,7 @@ impl Command for ParEach { ) } } + .and_then(|x| x.filter(|v| !v.is_nothing(), outer_ctrlc)) .map(|res| res.set_metadata(metadata)) } } diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 564457c307..9bff39c3b3 100644 --- a/crates/nu-command/tests/commands/mod.rs +++ b/crates/nu-command/tests/commands/mod.rs @@ -59,6 +59,7 @@ mod network; mod nu_check; mod open; mod p; +mod par_each; mod parse; mod path; mod platform; diff --git a/crates/nu-command/tests/commands/par_each.rs b/crates/nu-command/tests/commands/par_each.rs new file mode 100644 index 0000000000..53c9e151c8 --- /dev/null +++ b/crates/nu-command/tests/commands/par_each.rs @@ -0,0 +1,12 @@ +use nu_test_support::{nu, pipeline}; + +#[test] +fn par_each_does_not_flatten_nested_structures() { + // This is a regression test for issue #8497 + let actual = nu!( + cwd: ".", pipeline( + r#"[1 2 3] | par-each { |it| [$it, $it] } | sort | to json --raw"# + )); + + assert_eq!(actual.out, "[[1,1],[2,2],[3,3]]"); +}