Split merging of parser delta and stack environment (#6005)

* Remove comment

* Split delta and environment merging

* Move table mode to a more logical place

* Cleanup

* Merge environment after reading default_env.nu

* Fmt

crates/nu-cli/src/commands.rs

@@ -5,21 +5,27 @@ use nu_engine::{convert_env_values, eval_block};
 use nu_parser::parse;
 use nu_protocol::engine::Stack;
 use nu_protocol::{
-    engine::{EngineState, StateDelta, StateWorkingSet},
+    engine::{EngineState, StateWorkingSet},
     PipelineData, Spanned, Value,
 };
-use std::path::Path;
 
+/// Run a command (or commands) given to us by the user
 pub fn evaluate_commands(
     commands: &Spanned<String>,
-    init_cwd: &Path,
     engine_state: &mut EngineState,
     stack: &mut Stack,
     input: PipelineData,
     is_perf_true: bool,
     table_mode: Option<Value>,
 ) -> Result<Option<i64>> {
-    // Run a command (or commands) given to us by the user
+    // Translate environment variables from Strings to Values
+    if let Some(e) = convert_env_values(engine_state, stack) {
+        let working_set = StateWorkingSet::new(engine_state);
+        report_error(&working_set, &e);
+        std::process::exit(1);
+    }
+
+    // Parse the source code
     let (block, delta) = {
         if let Some(ref t_mode) = table_mode {
             let mut config = engine_state.get_config().clone();
@@ -39,43 +45,19 @@ pub fn evaluate_commands(
         (output, working_set.render())
     };
 
-    if let Err(err) = engine_state.merge_delta(delta, None, init_cwd) {
+    // Update permanent state
+    if let Err(err) = engine_state.merge_delta(delta) {
         let working_set = StateWorkingSet::new(engine_state);
         report_error(&working_set, &err);
     }
 
+    // Run the block
+    let exit_code = match eval_block(engine_state, stack, &block, input, false, false) {
+        Ok(pipeline_data) => {
             let mut config = engine_state.get_config().clone();
             if let Some(t_mode) = table_mode {
                 config.table_mode = t_mode.as_string()?;
             }
-
-    // Merge the delta in case env vars changed in the config
-    match nu_engine::env::current_dir(engine_state, stack) {
-        Ok(cwd) => {
-            if let Err(e) =
-                engine_state.merge_delta(StateDelta::new(engine_state), Some(stack), cwd)
-            {
-                let working_set = StateWorkingSet::new(engine_state);
-                report_error(&working_set, &e);
-                std::process::exit(1);
-            }
-        }
-        Err(e) => {
-            let working_set = StateWorkingSet::new(engine_state);
-            report_error(&working_set, &e);
-            std::process::exit(1);
-        }
-    }
-
-    // Translate environment variables from Strings to Values
-    if let Some(e) = convert_env_values(engine_state, stack) {
-        let working_set = StateWorkingSet::new(engine_state);
-        report_error(&working_set, &e);
-        std::process::exit(1);
-    }
-
-    let exit_code = match eval_block(engine_state, stack, &block, input, false, false) {
-        Ok(pipeline_data) => {
             crate::eval_file::print_table_or_error(engine_state, stack, pipeline_data, &mut config)
         }
         Err(err) => {

crates/nu-cli/src/config_files.rs

@@ -1,7 +1,7 @@
 use crate::util::{eval_source, report_error};
 #[cfg(feature = "plugin")]
 use log::info;
-use nu_protocol::engine::{EngineState, Stack, StateDelta, StateWorkingSet};
+use nu_protocol::engine::{EngineState, Stack, StateWorkingSet};
 use nu_protocol::{HistoryFileFormat, PipelineData, Span};
 use std::path::PathBuf;
 
@@ -69,12 +69,10 @@ pub fn eval_config_contents(
                 PipelineData::new(Span::new(0, 0)),
             );
 
-            // Merge the delta in case env vars changed in the config
+            // Merge the environment in case env vars changed in the config
             match nu_engine::env::current_dir(engine_state, stack) {
                 Ok(cwd) => {
-                    if let Err(e) =
+                    if let Err(e) = engine_state.merge_env(stack, cwd) {
-                        engine_state.merge_delta(StateDelta::new(engine_state), Some(stack), cwd)
-                    {
                         let working_set = StateWorkingSet::new(engine_state);
                         report_error(&working_set, &e);
                     }

crates/nu-cli/src/lib.rs

@@ -24,7 +24,7 @@ pub use prompt::NushellPrompt;
 pub use repl::evaluate_repl;
 pub use repl::{eval_env_change_hook, eval_hook};
 pub use syntax_highlight::NuHighlighter;
-pub use util::{eval_source, gather_parent_env_vars, get_init_cwd, report_error};
+pub use util::{eval_source, gather_parent_env_vars, get_init_cwd, report_error, report_error_new};
 pub use validation::NuValidator;
 
 #[cfg(feature = "plugin")]

crates/nu-cli/src/repl.rs

@@ -2,7 +2,7 @@ use crate::{
     completions::NuCompleter,
     prompt_update,
     reedline_config::{add_menus, create_keybindings, KeybindingsMode},
-    util::{eval_source, get_init_cwd, report_error, report_error_new},
+    util::{eval_source, get_guaranteed_cwd, report_error, report_error_new},
     NuHighlighter, NuValidator, NushellPrompt,
 };
 use log::{info, trace};
@@ -122,6 +122,14 @@ pub fn evaluate_repl(
             );
         }
 
+        let cwd = get_guaranteed_cwd(engine_state, stack);
+
+        // Before doing anything, merge the environment from the previous REPL iteration into the
+        // permanent state.
+        if let Err(err) = engine_state.merge_env(stack, cwd) {
+            report_error_new(engine_state, &err);
+        }
+
         //Reset the ctrl-c handler
         if let Some(ctrlc) = &mut engine_state.ctrlc {
             ctrlc.store(false, Ordering::SeqCst);
@@ -408,14 +416,6 @@ pub fn evaluate_repl(
                     },
                 );
 
-                // FIXME: permanent state changes like this hopefully in time can be removed
-                // and be replaced by just passing the cwd in where needed
-                if let Some(cwd) = stack.get_env_var(engine_state, "PWD") {
-                    let path = cwd.as_string()?;
-                    let _ = std::env::set_current_dir(path);
-                    engine_state.add_env_var("PWD".into(), cwd);
-                }
-
                 if history_supports_meta && !s.is_empty() {
                     line_editor
                         .update_last_command_context(&|mut c| {
@@ -615,15 +615,7 @@ pub fn eval_hook(
                             (output, working_set.render(), vars)
                         };
 
-                        let cwd = match nu_engine::env::current_dir(engine_state, stack) {
+                        engine_state.merge_delta(delta)?;
-                            Ok(p) => p,
-                            Err(e) => {
-                                report_error_new(engine_state, &e);
-                                get_init_cwd()
-                            }
-                        };
-
-                        let _ = engine_state.merge_delta(delta, Some(stack), &cwd);
                         let input = PipelineData::new(value_span);
 
                         let var_ids: Vec<VarId> = vars
@@ -644,6 +636,9 @@ pub fn eval_hook(
                         for var_id in var_ids.iter() {
                             stack.vars.remove(var_id);
                         }
+
+                        let cwd = get_guaranteed_cwd(engine_state, stack);
+                        engine_state.merge_env(stack, cwd)?;
                     }
                     Value::Block {
                         val: block_id,
@@ -651,6 +646,8 @@ pub fn eval_hook(
                         ..
                     } => {
                         run_hook_block(engine_state, stack, block_id, arguments, block_span)?;
+                        let cwd = get_guaranteed_cwd(engine_state, stack);
+                        engine_state.merge_env(stack, cwd)?;
                     }
                     other => {
                         return Err(ShellError::UnsupportedConfigValue(

crates/nu-cli/src/util.rs

@@ -224,16 +224,11 @@ pub fn eval_source(
         (output, working_set.render())
     };
 
-    let cwd = match nu_engine::env::current_dir(engine_state, stack) {
+    if let Err(err) = engine_state.merge_delta(delta) {
-        Ok(p) => p,
+        set_last_exit_code(stack, 1);
-        Err(e) => {
+        report_error_new(engine_state, &err);
-            let working_set = StateWorkingSet::new(engine_state);
+        return false;
-            report_error(&working_set, &e);
-            get_init_cwd()
     }
-    };
-
-    let _ = engine_state.merge_delta(delta, Some(stack), &cwd);
 
     match eval_block(engine_state, stack, &block, input, false, false) {
         Ok(mut pipeline_data) => {
@@ -319,6 +314,17 @@ pub fn get_init_cwd() -> PathBuf {
     }
 }
 
+pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> PathBuf {
+    match nu_engine::env::current_dir(engine_state, stack) {
+        Ok(p) => p,
+        Err(e) => {
+            let working_set = StateWorkingSet::new(engine_state);
+            report_error(&working_set, &e);
+            get_init_cwd()
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

crates/nu-cli/tests/support/completions_helpers.rs

@@ -4,7 +4,7 @@ use nu_command::create_default_context;
 use nu_engine::eval_block;
 use nu_parser::parse;
 use nu_protocol::{
-    engine::{EngineState, Stack, StateDelta, StateWorkingSet},
+    engine::{EngineState, Stack, StateWorkingSet},
     PipelineData, ShellError, Span, Value,
 };
 use nu_test_support::fs;
@@ -23,14 +23,11 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) {
     dir_str.push(SEP);
 
     // Create a new engine with default context
-    let mut engine_state = create_default_context(&dir);
+    let mut engine_state = create_default_context();
 
     // New stack
     let mut stack = Stack::new();
 
-    // New delta state
-    let delta = StateDelta::new(&engine_state);
-
     // Add pwd as env var
     stack.add_env_var(
         "PWD".to_string(),
@@ -53,8 +50,8 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) {
         },
     );
 
-    // Merge delta
+    // Merge environment into the permanent state
-    let merge_result = engine_state.merge_delta(delta, Some(&mut stack), &dir);
+    let merge_result = engine_state.merge_env(&mut stack, &dir);
     assert!(merge_result.is_ok());
 
     (dir, dir_str, engine_state, stack)
@@ -97,6 +94,11 @@ pub fn merge_input(
 
         (block, working_set.render())
     };
+
+    if let Err(err) = engine_state.merge_delta(delta) {
+        return Err(err);
+    }
+
     assert!(eval_block(
         engine_state,
         stack,
@@ -112,6 +114,6 @@ pub fn merge_input(
     )
     .is_ok());
 
-    // Merge delta
+    // Merge environment into the permanent state
-    engine_state.merge_delta(delta, Some(stack), &dir)
+    engine_state.merge_env(stack, &dir)
 }

crates/nu-command/src/database/test_database.rs

@@ -75,8 +75,9 @@ pub fn test_database(cmds: Vec<Box<dyn Command + 'static>>) {
         working_set.render()
     };
 
-    let cwd = std::env::current_dir().expect("Could not get current working directory.");
+    engine_state
-    let _ = engine_state.merge_delta(delta, None, &cwd);
+        .merge_delta(delta)
+        .expect("Error merging delta");
 
     for example in examples {
         // Skip tests that don't have results to compare to
@@ -102,7 +103,9 @@ pub fn test_database(cmds: Vec<Box<dyn Command + 'static>>) {
             (output, working_set.render())
         };
 
-        let _ = engine_state.merge_delta(delta, None, &cwd);
+        engine_state
+            .merge_delta(delta)
+            .expect("Error merging delta");
 
         let mut stack = Stack::new();
 

crates/nu-command/src/dataframe/test_dataframe.rs

@@ -37,8 +37,9 @@ pub fn test_dataframe(cmds: Vec<Box<dyn Command + 'static>>) {
         working_set.render()
     };
 
-    let cwd = std::env::current_dir().expect("Could not get current working directory.");
+    engine_state
-    let _ = engine_state.merge_delta(delta, None, &cwd);
+        .merge_delta(delta)
+        .expect("Error merging delta");
 
     for example in examples {
         // Skip tests that don't have results to compare to
@@ -64,7 +65,9 @@ pub fn test_dataframe(cmds: Vec<Box<dyn Command + 'static>>) {
             (output, working_set.render())
         };
 
-        let _ = engine_state.merge_delta(delta, None, &cwd);
+        engine_state
+            .merge_delta(delta)
+            .expect("Error merging delta");
 
         let mut stack = Stack::new();
 

crates/nu-command/src/default_context.rs

@@ -1,10 +1,8 @@
 use nu_protocol::engine::{EngineState, StateWorkingSet};
 
-use std::path::Path;
-
 use crate::*;
 
-pub fn create_default_context(cwd: impl AsRef<Path>) -> EngineState {
+pub fn create_default_context() -> EngineState {
     let mut engine_state = EngineState::new();
 
     let delta = {
@@ -433,7 +431,9 @@ pub fn create_default_context(cwd: impl AsRef<Path>) -> EngineState {
         working_set.render()
     };
 
-    let _ = engine_state.merge_delta(delta, None, &cwd);
+    if let Err(err) = engine_state.merge_delta(delta) {
+        eprintln!("Error creating default context: {:?}", err);
+    }
 
     engine_state
 }

crates/nu-command/src/example_test.rs

@@ -57,7 +57,10 @@ pub fn test_examples(cmd: impl Command + 'static) {
     };
 
     let cwd = std::env::current_dir().expect("Could not get current working directory.");
-    let _ = engine_state.merge_delta(delta, None, &cwd);
+
+    engine_state
+        .merge_delta(delta)
+        .expect("Error merging delta");
 
     for example in examples {
         // Skip tests that don't have results to compare to
@@ -76,11 +79,10 @@ pub fn test_examples(cmd: impl Command + 'static) {
                 span: Span::test_data(),
             },
         );
-        let _ = engine_state.merge_delta(
+
-            StateWorkingSet::new(&*engine_state).render(),
+        engine_state
-            Some(&mut stack),
+            .merge_env(&mut stack, &cwd)
-            &cwd,
+            .expect("Error merging environment");
-        );
 
         let (block, delta) = {
             let mut working_set = StateWorkingSet::new(&*engine_state);
@@ -99,7 +101,9 @@ pub fn test_examples(cmd: impl Command + 'static) {
             (output, working_set.render())
         };
 
-        let _ = engine_state.merge_delta(delta, None, &cwd);
+        engine_state
+            .merge_delta(delta)
+            .expect("Error merging delta");
 
         let mut stack = Stack::new();
 

crates/nu-command/tests/main.rs

@@ -13,8 +13,7 @@ fn quickcheck_parse(data: String) -> bool {
     let (lite_block, err2) = nu_parser::lite_parse(&tokens);
 
     if err.is_none() && err2.is_none() {
-        let cwd = std::env::current_dir().expect("Could not get current working directory.");
+        let context = create_default_context();
-        let context = create_default_context(cwd);
         {
             let mut working_set = StateWorkingSet::new(&context);
             working_set.add_file("quickcheck".into(), data.as_bytes());

crates/nu-parser/tests/test_parser.rs

@@ -977,8 +977,9 @@ mod input_types {
             working_set.render()
         };
 
-        let cwd = std::env::current_dir().expect("Could not get current working directory.");
+        engine_state
-        let _ = engine_state.merge_delta(delta, None, &cwd);
+            .merge_delta(delta)
+            .expect("Error merging delta");
     }
 
     #[test]
@@ -999,7 +1000,9 @@ mod input_types {
 
         match &expressions[0].expr {
             Expr::Call(call) => {
-                let expected_id = working_set.find_decl(b"ls", &Type::Any).unwrap();
+                let expected_id = working_set
+                    .find_decl(b"ls", &Type::Any)
+                    .expect("Error merging delta");
                 assert_eq!(call.decl_id, expected_id)
             }
             _ => panic!("Expected expression Call not found"),
@@ -1154,8 +1157,7 @@ mod input_types {
             (block, working_set.render())
         };
 
-        let cwd = std::env::current_dir().expect("Could not get current working directory.");
+        engine_state.merge_delta(delta).unwrap();
-        let _ = engine_state.merge_delta(delta, None, &cwd);
 
         let expressions = &block[0];
         match &expressions[3].expr {

crates/nu-protocol/src/engine/engine_state.rs

@@ -120,12 +120,7 @@ impl EngineState {
     ///
     /// When we want to preserve what the parser has created, we can take its output (the `StateDelta`) and
     /// use this function to merge it into the global state.
-    pub fn merge_delta(
+    pub fn merge_delta(&mut self, mut delta: StateDelta) -> Result<(), ShellError> {
-        &mut self,
-        mut delta: StateDelta,
-        stack: Option<&mut Stack>,
-        cwd: impl AsRef<Path>,
-    ) -> Result<(), ShellError> {
         // Take the mutable reference and extend the permanent state from the working set
         self.files.extend(delta.files);
         self.file_contents.extend(delta.file_contents);
@@ -199,7 +194,15 @@ impl EngineState {
             return result;
         }
 
-        if let Some(stack) = stack {
+        Ok(())
+    }
+
+    /// Merge the environment from the runtime Stack into the engine state
+    pub fn merge_env(
+        &mut self,
+        stack: &mut Stack,
+        cwd: impl AsRef<Path>,
+    ) -> Result<(), ShellError> {
         for mut scope in stack.env_vars.drain(..) {
             for (overlay_name, mut env) in scope.drain() {
                 if let Some(env_vars) = self.env_vars.get_mut(&overlay_name) {
@@ -217,10 +220,8 @@ impl EngineState {
                 }
             }
         }
-        }
 
-        // FIXME: permanent state changes like this hopefully in time can be removed
+        // TODO: better error
-        // and be replaced by just passing the cwd in where needed
         std::env::set_current_dir(cwd)?;
 
         Ok(())
@@ -1914,12 +1915,6 @@ impl Default for ScopeFrame {
     }
 }
 
-// impl Default for OverlayFrame {
-//     fn default() -> Self {
-//         Self::new()
-//     }
-// }
-
 impl Default for EngineState {
     fn default() -> Self {
         Self::new()
@@ -2042,8 +2037,7 @@ mod engine_state_tests {
             working_set.render()
         };
 
-        let cwd = std::env::current_dir().expect("Could not get current working directory.");
+        engine_state.merge_delta(delta)?;
-        engine_state.merge_delta(delta, None, &cwd)?;
 
         assert_eq!(engine_state.num_files(), 2);
         assert_eq!(&engine_state.files[0].0, "test.nu");

src/config_files.rs

@@ -2,7 +2,7 @@ use log::info;
 use nu_cli::{eval_config_contents, eval_source, report_error};
 use nu_parser::ParseError;
 use nu_path::canonicalize_with;
-use nu_protocol::engine::{EngineState, Stack, StateDelta, StateWorkingSet};
+use nu_protocol::engine::{EngineState, Stack, StateWorkingSet};
 use nu_protocol::{PipelineData, Span, Spanned};
 use std::fs::File;
 use std::io::Write;
@@ -90,6 +90,21 @@ pub(crate) fn read_config_file(
                         },
                         PipelineData::new(Span::new(0, 0)),
                     );
+
+                    // Merge the environment in case env vars changed in the config
+                    match nu_engine::env::current_dir(engine_state, stack) {
+                        Ok(cwd) => {
+                            if let Err(e) = engine_state.merge_env(stack, cwd) {
+                                let working_set = StateWorkingSet::new(engine_state);
+                                report_error(&working_set, &e);
+                            }
+                        }
+                        Err(e) => {
+                            let working_set = StateWorkingSet::new(engine_state);
+                            report_error(&working_set, &e);
+                        }
+                    }
+
                     return;
                 }
             }
@@ -102,6 +117,7 @@ pub(crate) fn read_config_file(
         info!("read_config_file {}:{}:{}", file!(), line!(), column!());
     }
 }
+
 pub(crate) fn read_loginshell_file(
     engine_state: &mut EngineState,
     stack: &mut Stack,
@@ -139,12 +155,10 @@ pub(crate) fn read_default_env_file(
     if is_perf_true {
         info!("read_config_file {}:{}:{}", file!(), line!(), column!());
     }
-    // Merge the delta in case env vars changed in the config
+    // Merge the environment in case env vars changed in the config
     match nu_engine::env::current_dir(engine_state, stack) {
         Ok(cwd) => {
-            if let Err(e) =
+            if let Err(e) = engine_state.merge_env(stack, cwd) {
-                engine_state.merge_delta(StateDelta::new(engine_state), Some(stack), cwd)
-            {
                 let working_set = StateWorkingSet::new(engine_state);
                 report_error(&working_set, &e);
             }

src/main.rs

@@ -13,7 +13,7 @@ use miette::Result;
 use nu_cli::read_plugin_file;
 use nu_cli::{
     evaluate_commands, evaluate_file, evaluate_repl, gather_parent_env_vars, get_init_cwd,
-    report_error,
+    report_error, report_error_new,
 };
 use nu_command::{create_default_context, BufferedReader};
 use nu_engine::{get_full_help, CallExt};
@@ -28,7 +28,6 @@ use nu_utils::stdout_write_all_and_flush;
 use std::cell::RefCell;
 use std::{
     io::BufReader,
-    path::Path,
     sync::{
         atomic::{AtomicBool, Ordering},
         Arc,
@@ -47,7 +46,7 @@ fn main() -> Result<()> {
 
     // Get initial current working directory.
     let init_cwd = get_init_cwd();
-    let mut engine_state = create_default_context(&init_cwd);
+    let mut engine_state = create_default_context();
 
     // Custom additions
     let delta = {
@@ -57,7 +56,10 @@ fn main() -> Result<()> {
 
         working_set.render()
     };
-    let _ = engine_state.merge_delta(delta, None, &init_cwd);
+
+    if let Err(err) = engine_state.merge_delta(delta) {
+        report_error_new(&engine_state, &err);
+    }
 
     // TODO: make this conditional in the future
     // Ctrl-c protection section
@@ -120,8 +122,7 @@ fn main() -> Result<()> {
 
     let nushell_commandline_args = args_to_nushell.join(" ");
 
-    let parsed_nu_cli_args =
+    let parsed_nu_cli_args = parse_commandline_args(&nushell_commandline_args, &mut engine_state);
-        parse_commandline_args(&nushell_commandline_args, &init_cwd, &mut engine_state);
 
     match parsed_nu_cli_args {
         Ok(binary_args) => {
@@ -195,6 +196,7 @@ fn main() -> Result<()> {
 
             // First, set up env vars as strings only
             gather_parent_env_vars(&mut engine_state, &init_cwd);
+
             let mut stack = nu_protocol::engine::Stack::new();
 
             if let Some(commands) = &binary_args.commands {
@@ -235,7 +237,6 @@ fn main() -> Result<()> {
 
                 let ret_val = evaluate_commands(
                     commands,
-                    &init_cwd,
                     &mut engine_state,
                     &mut stack,
                     input,
@@ -357,7 +358,6 @@ fn setup_config(
 
 fn parse_commandline_args(
     commandline_args: &str,
-    init_cwd: &Path,
     engine_state: &mut EngineState,
 ) -> Result<NushellCliArgs, ShellError> {
     let (block, delta) = {
@@ -381,7 +381,7 @@ fn parse_commandline_args(
         (output, working_set.render())
     };
 
-    let _ = engine_state.merge_delta(delta, None, init_cwd);
+    engine_state.merge_delta(delta)?;
 
     let mut stack = Stack::new();
 

tests/hooks/mod.rs

@@ -215,7 +215,6 @@ fn env_change_block_preserve_env_var() {
 fn pre_prompt_define_command() {
     let inp = &[
         &pre_prompt_hook_code(r#"'def foo [] { "got foo!" }'"#),
-        "",
         "foo",
     ];
 
@@ -229,7 +228,6 @@ fn pre_prompt_define_command() {
 fn pre_prompt_simple_block_preserve_env_var() {
     let inp = &[
         &pre_prompt_hook(r#"{ let-env SPAM = "spam" }"#),
-        "",
         "$env.SPAM",
     ];
 
@@ -248,7 +246,6 @@ fn pre_prompt_simple_block_list_shadow_env_var() {
                 { let-env SPAM = "spam" }
             ]"#,
         ),
-        "",
         "$env.SPAM",
     ];
 
@@ -262,7 +259,6 @@ fn pre_prompt_simple_block_list_shadow_env_var() {
 fn pre_prompt_block_preserve_env_var() {
     let inp = &[
         &pre_prompt_hook_code(r#"{ let-env SPAM = "spam" }"#),
-        "",
         "$env.SPAM",
     ];
 
@@ -276,7 +272,6 @@ fn pre_prompt_block_preserve_env_var() {
 fn pre_execution_define_command() {
     let inp = &[
         &pre_execution_hook_code(r#"'def foo [] { "got foo!" }'"#),
-        "",
         "foo",
     ];
 
@@ -290,7 +285,6 @@ fn pre_execution_define_command() {
 fn pre_execution_simple_block_preserve_env_var() {
     let inp = &[
         &pre_execution_hook(r#"{ let-env SPAM = "spam" }"#),
-        "",
         "$env.SPAM",
     ];
 
@@ -309,7 +303,6 @@ fn pre_execution_simple_block_list_shadow_env_var() {
             { let-env SPAM = "spam" }
         ]"#,
         ),
-        "",
         "$env.SPAM",
     ];
 
@@ -323,7 +316,6 @@ fn pre_execution_simple_block_list_shadow_env_var() {
 fn pre_execution_block_preserve_env_var() {
     let inp = &[
         &pre_execution_hook_code(r#"{ let-env SPAM = "spam" }"#),
-        "",
         "$env.SPAM",
     ];
 
@@ -450,7 +442,6 @@ fn err_hook_wrong_env_type_2() {
             }
         }"#,
         "",
-        "",
     ];
 
     let actual_repl = nu_repl("tests/hooks", inp);
@@ -551,11 +542,7 @@ fn err_hook_parse_error() {
 
 #[test]
 fn err_hook_dont_allow_string() {
-    let inp = &[
+    let inp = &[&pre_prompt_hook(r#"'def foo [] { "got foo!" }'"#), "foo"];
-        &pre_prompt_hook(r#"'def foo [] { "got foo!" }'"#),
-        "",
-        "foo",
-    ];
 
     let actual_repl = nu_repl("tests/hooks", inp);
 

tests/nu_repl/mod.rs

@@ -2,7 +2,7 @@ use nu_cli::{eval_env_change_hook, eval_hook};
 use nu_command::create_default_context;
 use nu_engine::eval_block;
 use nu_parser::parse;
-use nu_protocol::engine::{EngineState, Stack, StateDelta, StateWorkingSet};
+use nu_protocol::engine::{EngineState, Stack, StateWorkingSet};
 use nu_protocol::{CliError, PipelineData, Span, Value};
 use nu_test_support::fs::in_directory;
 use nu_test_support::Outcome;
@@ -31,7 +31,7 @@ fn outcome_ok(msg: String) -> Outcome {
 pub fn nu_repl(cwd: &str, source_lines: &[&str]) -> Outcome {
     let cwd = in_directory(cwd);
 
-    let mut engine_state = create_default_context(&cwd);
+    let mut engine_state = create_default_context();
     let mut stack = Stack::new();
 
     stack.add_env_var(
@@ -42,14 +42,22 @@ pub fn nu_repl(cwd: &str, source_lines: &[&str]) -> Outcome {
         },
     );
 
-    let delta = StateDelta::new(&engine_state);
-    if let Err(err) = engine_state.merge_delta(delta, Some(&mut stack), cwd) {
-        return outcome_err(&engine_state, &err);
-    }
-
     let mut last_output = String::new();
 
     for (i, line) in source_lines.iter().enumerate() {
+        let cwd = match nu_engine::env::current_dir(&engine_state, &stack) {
+            Ok(d) => d,
+            Err(err) => {
+                return outcome_err(&engine_state, &err);
+            }
+        };
+
+        // Before doing anything, merge the environment from the previous REPL iteration into the
+        // permanent state.
+        if let Err(err) = engine_state.merge_env(&mut stack, &cwd) {
+            return outcome_err(&engine_state, &err);
+        }
+
         // Check for pre_prompt hook
         let config = engine_state.get_config();
         if let Some(hook) = config.hooks.pre_prompt.clone() {
@@ -93,14 +101,7 @@ pub fn nu_repl(cwd: &str, source_lines: &[&str]) -> Outcome {
             (block, working_set.render())
         };
 
-        let cwd = match nu_engine::env::current_dir(&engine_state, &stack) {
+        if let Err(err) = engine_state.merge_delta(delta) {
-            Ok(p) => p,
-            Err(e) => {
-                return outcome_err(&engine_state, &e);
-            }
-        };
-
-        if let Err(err) = engine_state.merge_delta(delta, Some(&mut stack), &cwd) {
             return outcome_err(&engine_state, &err);
         }