From 26f31da71172c12e0c4006b8257ac6d6da2c0db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Thu, 14 Jul 2022 17:09:27 +0300 Subject: [PATCH] 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 | 52 ++++++------------- crates/nu-cli/src/config_files.rs | 8 ++- crates/nu-cli/src/lib.rs | 2 +- crates/nu-cli/src/repl.rs | 33 ++++++------ crates/nu-cli/src/util.rs | 26 ++++++---- .../tests/support/completions_helpers.rs | 20 +++---- .../nu-command/src/database/test_database.rs | 9 ++-- .../src/dataframe/test_dataframe.rs | 9 ++-- crates/nu-command/src/default_context.rs | 8 +-- crates/nu-command/src/example_test.rs | 18 ++++--- crates/nu-command/tests/main.rs | 3 +- crates/nu-parser/tests/test_parser.rs | 12 +++-- crates/nu-protocol/src/engine/engine_state.rs | 52 ++++++++----------- src/config_files.rs | 24 +++++++-- src/main.rs | 18 +++---- tests/hooks/mod.rs | 27 +++------- tests/nu_repl/mod.rs | 31 +++++------ 17 files changed, 172 insertions(+), 180 deletions(-) diff --git a/crates/nu-cli/src/commands.rs b/crates/nu-cli/src/commands.rs index 9a9ff72292..3188985ff7 100644 --- a/crates/nu-cli/src/commands.rs +++ b/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, - init_cwd: &Path, engine_state: &mut EngineState, stack: &mut Stack, input: PipelineData, is_perf_true: bool, table_mode: Option, ) -> Result> { - // 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); } - 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); - } - + // 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()?; + } crate::eval_file::print_table_or_error(engine_state, stack, pipeline_data, &mut config) } Err(err) => { diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 18d85eea8a..ff6b4916f9 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/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) = - engine_state.merge_delta(StateDelta::new(engine_state), Some(stack), cwd) - { + if let Err(e) = engine_state.merge_env(stack, cwd) { let working_set = StateWorkingSet::new(engine_state); report_error(&working_set, &e); } diff --git a/crates/nu-cli/src/lib.rs b/crates/nu-cli/src/lib.rs index 23f64bb5bf..7549096267 100644 --- a/crates/nu-cli/src/lib.rs +++ b/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")] diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 536808a9cc..13c4db7968 100644 --- a/crates/nu-cli/src/repl.rs +++ b/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) { - Ok(p) => p, - Err(e) => { - report_error_new(engine_state, &e); - get_init_cwd() - } - }; - - let _ = engine_state.merge_delta(delta, Some(stack), &cwd); + engine_state.merge_delta(delta)?; let input = PipelineData::new(value_span); let var_ids: Vec = 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( diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs index 4c9f6d0852..75cdfa3dc9 100644 --- a/crates/nu-cli/src/util.rs +++ b/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) { - Ok(p) => p, - Err(e) => { - let working_set = StateWorkingSet::new(engine_state); - report_error(&working_set, &e); - get_init_cwd() - } - }; - - let _ = engine_state.merge_delta(delta, Some(stack), &cwd); + if let Err(err) = engine_state.merge_delta(delta) { + set_last_exit_code(stack, 1); + report_error_new(engine_state, &err); + return false; + } 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::*; diff --git a/crates/nu-cli/tests/support/completions_helpers.rs b/crates/nu-cli/tests/support/completions_helpers.rs index 9ab661d980..a4e7ab641d 100644 --- a/crates/nu-cli/tests/support/completions_helpers.rs +++ b/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 - let merge_result = engine_state.merge_delta(delta, Some(&mut stack), &dir); + // Merge environment into the permanent state + 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 - engine_state.merge_delta(delta, Some(stack), &dir) + // Merge environment into the permanent state + engine_state.merge_env(stack, &dir) } diff --git a/crates/nu-command/src/database/test_database.rs b/crates/nu-command/src/database/test_database.rs index 77cf540e3f..985d8d5912 100644 --- a/crates/nu-command/src/database/test_database.rs +++ b/crates/nu-command/src/database/test_database.rs @@ -75,8 +75,9 @@ pub fn test_database(cmds: Vec>) { working_set.render() }; - 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 @@ -102,7 +103,9 @@ pub fn test_database(cmds: Vec>) { (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(); diff --git a/crates/nu-command/src/dataframe/test_dataframe.rs b/crates/nu-command/src/dataframe/test_dataframe.rs index 828cb4e854..4131c612d9 100644 --- a/crates/nu-command/src/dataframe/test_dataframe.rs +++ b/crates/nu-command/src/dataframe/test_dataframe.rs @@ -37,8 +37,9 @@ pub fn test_dataframe(cmds: Vec>) { working_set.render() }; - 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 @@ -64,7 +65,9 @@ pub fn test_dataframe(cmds: Vec>) { (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(); diff --git a/crates/nu-command/src/default_context.rs b/crates/nu-command/src/default_context.rs index b1a7453e72..1dcc43e93d 100644 --- a/crates/nu-command/src/default_context.rs +++ b/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) -> 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) -> 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 } diff --git a/crates/nu-command/src/example_test.rs b/crates/nu-command/src/example_test.rs index 26126991d2..a16e88be1e 100644 --- a/crates/nu-command/src/example_test.rs +++ b/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(), - Some(&mut stack), - &cwd, - ); + + engine_state + .merge_env(&mut stack, &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(); diff --git a/crates/nu-command/tests/main.rs b/crates/nu-command/tests/main.rs index 9fd4b729d5..fcca4fa272 100644 --- a/crates/nu-command/tests/main.rs +++ b/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(cwd); + let context = create_default_context(); { let mut working_set = StateWorkingSet::new(&context); working_set.add_file("quickcheck".into(), data.as_bytes()); diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 023330dde9..c75a74c39e 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/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."); - let _ = engine_state.merge_delta(delta, None, &cwd); + engine_state + .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."); - let _ = engine_state.merge_delta(delta, None, &cwd); + engine_state.merge_delta(delta).unwrap(); let expressions = &block[0]; match &expressions[3].expr { diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index d6af043601..a9b3e3122c 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/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( - &mut self, - mut delta: StateDelta, - stack: Option<&mut Stack>, - cwd: impl AsRef, - ) -> Result<(), ShellError> { + pub fn merge_delta(&mut self, mut delta: StateDelta) -> 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,28 +194,34 @@ impl EngineState { return result; } - if let Some(stack) = stack { - 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) { - // Updating existing overlay - for (k, v) in env.drain() { - if k == "config" { - self.config = v.clone().into_config().unwrap_or_default(); - } + Ok(()) + } - env_vars.insert(k, v); + /// Merge the environment from the runtime Stack into the engine state + pub fn merge_env( + &mut self, + stack: &mut Stack, + cwd: impl AsRef, + ) -> 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) { + // Updating existing overlay + for (k, v) in env.drain() { + if k == "config" { + self.config = v.clone().into_config().unwrap_or_default(); } - } else { - // Pushing a new overlay - self.env_vars.insert(overlay_name, env); + + env_vars.insert(k, v); } + } else { + // Pushing a new overlay + self.env_vars.insert(overlay_name, env); } } } - // FIXME: permanent state changes like this hopefully in time can be removed - // and be replaced by just passing the cwd in where needed + // TODO: better error 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, None, &cwd)?; + engine_state.merge_delta(delta)?; assert_eq!(engine_state.num_files(), 2); assert_eq!(&engine_state.files[0].0, "test.nu"); diff --git a/src/config_files.rs b/src/config_files.rs index 18a2586fd7..c07ca80b8a 100644 --- a/src/config_files.rs +++ b/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) = - engine_state.merge_delta(StateDelta::new(engine_state), Some(stack), cwd) - { + if let Err(e) = engine_state.merge_env(stack, cwd) { let working_set = StateWorkingSet::new(engine_state); report_error(&working_set, &e); } diff --git a/src/main.rs b/src/main.rs index ece58ae0e8..cec1130881 100644 --- a/src/main.rs +++ b/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 = - parse_commandline_args(&nushell_commandline_args, &init_cwd, &mut engine_state); + let parsed_nu_cli_args = parse_commandline_args(&nushell_commandline_args, &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 { 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(); diff --git a/tests/hooks/mod.rs b/tests/hooks/mod.rs index 87fc953ad8..4e05b799ac 100644 --- a/tests/hooks/mod.rs +++ b/tests/hooks/mod.rs @@ -183,9 +183,9 @@ fn env_change_simple_block_list_shadow_env_var() { &env_change_hook( "FOO", r#"[ - { let-env SPAM = "foo" } - { let-env SPAM = "spam" } - ]"#, + { let-env SPAM = "foo" } + { let-env SPAM = "spam" } + ]"#, ), "let-env FOO = 1", "$env.SPAM", @@ -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", ]; @@ -244,11 +242,10 @@ fn pre_prompt_simple_block_list_shadow_env_var() { let inp = &[ &pre_prompt_hook( r#"[ - { let-env SPAM = "foo" } - { let-env SPAM = "spam" } - ]"#, + { let-env SPAM = "foo" } + { 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 = &[ - &pre_prompt_hook(r#"'def foo [] { "got foo!" }'"#), - "", - "foo", - ]; + let inp = &[&pre_prompt_hook(r#"'def foo [] { "got foo!" }'"#), "foo"]; let actual_repl = nu_repl("tests/hooks", inp); diff --git a/tests/nu_repl/mod.rs b/tests/nu_repl/mod.rs index 495fbf4f09..586c073e94 100644 --- a/tests/nu_repl/mod.rs +++ b/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) { - Ok(p) => p, - Err(e) => { - return outcome_err(&engine_state, &e); - } - }; - - if let Err(err) = engine_state.merge_delta(delta, Some(&mut stack), &cwd) { + if let Err(err) = engine_state.merge_delta(delta) { return outcome_err(&engine_state, &err); }