From 03667bdf8cc73332b1dbe3c275c32fec722db4d5 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Sat, 6 Apr 2024 15:03:22 +0000 Subject: [PATCH] Fix merging child stack into parent (#12426) # Description Fixes #12423 where changes to mutable variables are not properly persisted after a REPL entry. --- .../nu-plugin-test-support/src/plugin_test.rs | 6 ----- crates/nu-protocol/src/engine/stack.rs | 13 +++++++---- src/test_bins.rs | 23 ++++++++++++------- tests/shell/mod.rs | 1 + tests/shell/repl.rs | 9 ++++++++ 5 files changed, 33 insertions(+), 19 deletions(-) create mode 100644 tests/shell/repl.rs diff --git a/crates/nu-plugin-test-support/src/plugin_test.rs b/crates/nu-plugin-test-support/src/plugin_test.rs index d2df2d861a..42cec9990e 100644 --- a/crates/nu-plugin-test-support/src/plugin_test.rs +++ b/crates/nu-plugin-test-support/src/plugin_test.rs @@ -337,12 +337,6 @@ impl PluginTest { // All equal, and same length Ok(true) } - (Value::Range { val: a_rng, .. }, Value::Range { val: b_rng, .. }) => { - Ok(a_rng.inclusion == b_rng.inclusion - && self.value_eq(&a_rng.from, &b_rng.from)? - && self.value_eq(&a_rng.to, &b_rng.to)? - && self.value_eq(&a_rng.incr, &b_rng.incr)?) - } // Must collect lazy records to compare. (Value::LazyRecord { val: a_val, .. }, _) => self.value_eq(&a_val.collect()?, b), (_, Value::LazyRecord { val: b_val, .. }) => self.value_eq(a, &b_val.collect()?), diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index b33a392a50..990b512551 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -112,7 +112,7 @@ impl Stack { /// /// Here it is assumed that child was created with a call to Stack::with_parent /// with parent - pub fn with_changes_from_child(parent: Arc, mut child: Stack) -> Stack { + pub fn with_changes_from_child(parent: Arc, child: Stack) -> Stack { // we're going to drop the link to the parent stack on our new stack // so that we can unwrap the Arc as a unique reference // @@ -121,15 +121,18 @@ impl Stack { drop(child.parent_stack); let mut unique_stack = Stack::unwrap_unique(parent); - unique_stack.vars.append(&mut child.vars); + unique_stack + .vars + .retain(|(var, _)| !child.parent_deletions.contains(var)); + for (var, value) in child.vars { + unique_stack.add_var(var, value); + } unique_stack.env_vars = child.env_vars; unique_stack.env_hidden = child.env_hidden; unique_stack.active_overlays = child.active_overlays; unique_stack - .vars - .retain(|(var, _)| !child.parent_deletions.contains(var)); - unique_stack } + pub fn with_env( &mut self, env_vars: &[EnvVars], diff --git a/src/test_bins.rs b/src/test_bins.rs index 26c9dd6a0a..c491bcbdbc 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -8,7 +8,10 @@ use nu_protocol::{ PipelineData, Value, }; use nu_std::load_standard_library; -use std::io::{self, BufRead, Read, Write}; +use std::{ + io::{self, BufRead, Read, Write}, + sync::Arc, +}; /// Echo's value of env keys from args /// Example: nu --testbin env_echo FOO BAR @@ -236,7 +239,7 @@ pub fn nu_repl() { let source_lines = args(); let mut engine_state = get_engine_state(); - let mut stack = Stack::new(); + let mut top_stack = Arc::new(Stack::new()); engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy())); @@ -245,6 +248,7 @@ pub fn nu_repl() { load_standard_library(&mut engine_state).expect("Could not load the standard library."); for (i, line) in source_lines.iter().enumerate() { + let mut stack = Stack::with_parent(top_stack.clone()); let cwd = nu_engine::env::current_dir(&engine_state, &stack) .unwrap_or_else(|err| outcome_err(&engine_state, &err)); @@ -324,13 +328,15 @@ pub fn nu_repl() { let input = PipelineData::empty(); let config = engine_state.get_config(); - let stack = &mut stack.start_capture(); - match eval_block::(&engine_state, stack, &block, input) { - Ok(pipeline_data) => match pipeline_data.collect_string("", config) { - Ok(s) => last_output = s, + { + let stack = &mut stack.start_capture(); + match eval_block::(&engine_state, stack, &block, input) { + Ok(pipeline_data) => match pipeline_data.collect_string("", config) { + Ok(s) => last_output = s, + Err(err) => outcome_err(&engine_state, &err), + }, Err(err) => outcome_err(&engine_state, &err), - }, - Err(err) => outcome_err(&engine_state, &err), + } } if let Some(cwd) = stack.get_env_var(&engine_state, "PWD") { @@ -340,6 +346,7 @@ pub fn nu_repl() { let _ = std::env::set_current_dir(path.as_ref()); engine_state.add_env_var("PWD".into(), cwd); } + top_stack = Arc::new(Stack::with_changes_from_child(top_stack, stack)); } outcome_ok(last_output) diff --git a/tests/shell/mod.rs b/tests/shell/mod.rs index bd49cc0c64..69fb92bd51 100644 --- a/tests/shell/mod.rs +++ b/tests/shell/mod.rs @@ -7,6 +7,7 @@ use pretty_assertions::assert_eq; mod environment; mod pipeline; +mod repl; //FIXME: jt: we need to focus some fixes on wix as the plugins will differ #[ignore] diff --git a/tests/shell/repl.rs b/tests/shell/repl.rs new file mode 100644 index 0000000000..0a244c5fcc --- /dev/null +++ b/tests/shell/repl.rs @@ -0,0 +1,9 @@ +use nu_test_support::{nu, nu_repl_code}; +use pretty_assertions::assert_eq; + +#[test] +fn mut_variable() { + let lines = &["mut x = 0", "$x = 1", "$x"]; + let actual = nu!(nu_repl_code(lines)); + assert_eq!(actual.out, "1"); +}