diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 0a2910bfa..5199a1b05 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -100,12 +100,7 @@ pub fn eval_config_contents( // 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) { - let working_set = StateWorkingSet::new(engine_state); - report_error(&working_set, &e); - } - - if let Err(e) = engine_state.set_current_working_dir(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/print.rs b/crates/nu-cli/src/print.rs index 385770b75..1a6d7d146 100644 --- a/crates/nu-cli/src/print.rs +++ b/crates/nu-cli/src/print.rs @@ -53,12 +53,6 @@ Since this command has no output, there is no point in piping it with other comm let no_newline = call.has_flag("no-newline"); let to_stderr = call.has_flag("stderr"); - // We merge stack to make sure we render the changes if any were made in the `block` - // - // CONSIDERED TO BE A CODE SMELL AND IT BETTER BE RESOLVED UPWARDS THE CALLING STACK - let engine = engine_state.clone_with_env(stack)?; - let engine_state = &engine; - // This will allow for easy printing of pipelines as well if !args.is_empty() { for arg in args { diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 1fd60a16b..4c2933ff6 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -172,8 +172,7 @@ pub fn evaluate_repl( PipelineData::empty(), false, ); - engine_state.merge_env(stack)?; - engine_state.set_current_working_dir(get_guaranteed_cwd(engine_state, stack))?; + engine_state.merge_env(stack, get_guaranteed_cwd(engine_state, stack))?; } engine_state.set_startup_time(entire_start_time.elapsed().as_nanos() as i64); @@ -192,18 +191,14 @@ pub fn evaluate_repl( loop { let loop_start_time = std::time::Instant::now(); + let cwd = get_guaranteed_cwd(engine_state, stack); + start_time = std::time::Instant::now(); // Before doing anything, merge the environment from the previous REPL iteration into the // permanent state. - if let Err(err) = engine_state.merge_env(stack) { + if let Err(err) = engine_state.merge_env(stack, cwd) { report_error_new(engine_state, &err); } - - let cwd = get_guaranteed_cwd(engine_state, stack); - if let Err(err) = engine_state.set_current_working_dir(cwd) { - report_error_new(engine_state, &err); - } - perf( "merge env", start_time, diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs index febd39a18..9cdbdf157 100644 --- a/crates/nu-cli/src/util.rs +++ b/crates/nu-cli/src/util.rs @@ -246,10 +246,6 @@ pub fn eval_source( match b { Ok(pipeline_data) => { - // we merge stack here because the block could change the envirenemnt, - // and we need to render it while do print. - let _ = engine_state.merge_env(stack); - let config = engine_state.get_config(); let result; if let PipelineData::ExternalStream { diff --git a/crates/nu-cli/tests/completions.rs b/crates/nu-cli/tests/completions.rs index f2ea03090..d6608975f 100644 --- a/crates/nu-cli/tests/completions.rs +++ b/crates/nu-cli/tests/completions.rs @@ -768,8 +768,7 @@ fn run_external_completion(block: &str, input: &str) -> Vec { assert!(engine_state.merge_delta(delta).is_ok()); // Merge environment into the permanent state - assert!(engine_state.merge_env(&mut stack).is_ok()); - assert!(engine_state.set_current_working_dir(&dir).is_ok()); + assert!(engine_state.merge_env(&mut stack, &dir).is_ok()); let latest_block_id = engine_state.num_blocks() - 1; diff --git a/crates/nu-cli/tests/support/completions_helpers.rs b/crates/nu-cli/tests/support/completions_helpers.rs index 5a0f3f974..43f79a4d9 100644 --- a/crates/nu-cli/tests/support/completions_helpers.rs +++ b/crates/nu-cli/tests/support/completions_helpers.rs @@ -61,8 +61,8 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) { ); // Merge environment into the permanent state - assert!(engine_state.merge_env(&mut stack).is_ok()); - assert!(engine_state.set_current_working_dir(&dir).is_ok()); + let merge_result = engine_state.merge_env(&mut stack, &dir); + assert!(merge_result.is_ok()); (dir, dir_str, engine_state, stack) } @@ -100,8 +100,8 @@ pub fn new_quote_engine() -> (PathBuf, String, EngineState, Stack) { ); // Merge environment into the permanent state - assert!(engine_state.merge_env(&mut stack).is_ok()); - assert!(engine_state.set_current_working_dir(&dir).is_ok()); + let merge_result = engine_state.merge_env(&mut stack, &dir); + assert!(merge_result.is_ok()); (dir, dir_str, engine_state, stack) } @@ -171,8 +171,5 @@ pub fn merge_input( .is_ok()); // Merge environment into the permanent state - engine_state.merge_env(stack)?; - engine_state.set_current_working_dir(&dir)?; - - Ok(()) + engine_state.merge_env(stack, &dir) } diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index a34a958cc..c53171971 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -167,13 +167,9 @@ pub fn check_example_evaluates_to_expected_output( stack.add_env_var("PWD".to_string(), Value::test_string(cwd.to_string_lossy())); engine_state - .merge_env(&mut stack) + .merge_env(&mut stack, cwd) .expect("Error merging environment"); - engine_state - .set_current_working_dir(cwd) - .expect("Error setting CWD"); - let empty_input = PipelineData::empty(); let result = eval(example.example, empty_input, cwd, engine_state); diff --git a/crates/nu-color-config/src/style_computer.rs b/crates/nu-color-config/src/style_computer.rs index 622f68899..e1f51c455 100644 --- a/crates/nu-color-config/src/style_computer.rs +++ b/crates/nu-color-config/src/style_computer.rs @@ -1,7 +1,7 @@ use crate::text_style::Alignment; use crate::{color_record_to_nustyle, lookup_ansi_color_style, TextStyle}; use nu_ansi_term::{Color, Style}; -use nu_engine::eval_block; +use nu_engine::{env::get_config, eval_block}; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, CliError, IntoPipelineData, Value, @@ -138,7 +138,7 @@ impl<'a> StyleComputer<'a> { // The main constructor. pub fn from_config(engine_state: &'a EngineState, stack: &'a Stack) -> StyleComputer<'a> { - let config = engine_state.get_config(); + let config = get_config(engine_state, stack); // Create the hashmap #[rustfmt::skip] diff --git a/crates/nu-command/src/hook.rs b/crates/nu-command/src/hook.rs index 7ab366f69..40b9a77a0 100644 --- a/crates/nu-command/src/hook.rs +++ b/crates/nu-command/src/hook.rs @@ -281,10 +281,8 @@ pub fn eval_hook( } } - engine_state.merge_env(stack)?; - let cwd = get_guaranteed_cwd(engine_state, stack); - engine_state.set_current_working_dir(cwd)?; + engine_state.merge_env(stack, cwd)?; Ok(output) } diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 5c4737b21..189f2fb75 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -1,7 +1,7 @@ use lscolors::{LsColors, Style}; use nu_color_config::color_from_hex; use nu_color_config::{StyleComputer, TextStyle}; -use nu_engine::{env_to_string, CallExt}; +use nu_engine::{env::get_config, env_to_string, CallExt}; use nu_protocol::{ ast::Call, engine::{Command, EngineState, Stack}, @@ -102,12 +102,6 @@ impl Command for Table { call: &Call, input: PipelineData, ) -> Result { - // We merge stack to make sure we render the changes if any were made in the `block` - // - // CONSIDERED TO BE A CODE SMELL AND IT BETTER BE RESOLVED UPWARDS THE CALLING STACK - let engine = engine_state.clone_with_env(stack)?; - let engine_state = &engine; - let start_num: Option = call.get_flag(engine_state, stack, "start-number")?; let row_offset = start_num.unwrap_or_default() as usize; let list: bool = call.has_flag("list"); @@ -237,7 +231,7 @@ fn handle_table_command( term_width: Option, ) -> Result { let ctrlc = engine_state.ctrlc.clone(); - let config = engine_state.get_config(); + let config = get_config(engine_state, stack); match input { PipelineData::ExternalStream { .. } => Ok(input), @@ -293,7 +287,7 @@ fn handle_table_command( table_view, term_width, ctrlc, - config, + &config, ) } PipelineData::Value(Value::LazyRecord { val, .. }, ..) => { @@ -454,7 +448,7 @@ fn handle_row_stream( Some(PipelineMetadata { data_source: DataSource::Ls, }) => { - let config = engine_state.config.clone(); + let config = get_config(engine_state, stack); let ctrlc = ctrlc.clone(); let ls_colors_env_str = match stack.get_env_var(engine_state, "LS_COLORS") { Some(v) => Some(env_to_string("LS_COLORS", &v, engine_state, stack)?), @@ -652,13 +646,13 @@ impl PagingTableCreator { return Ok(None); } - let config = self.engine_state.get_config(); + let config = get_config(&self.engine_state, &self.stack); let style_computer = StyleComputer::from_config(&self.engine_state, &self.stack); let term_width = get_width_param(self.width_param); let ctrlc = self.ctrlc.clone(); let span = self.head; - let opts = BuildConfig::new(ctrlc, config, &style_computer, span, term_width); + let opts = BuildConfig::new(ctrlc, &config, &style_computer, span, term_width); let view = TableView::Expanded { limit, flatten, @@ -673,24 +667,24 @@ impl PagingTableCreator { return Ok(None); } - let config = self.engine_state.get_config(); + let config = get_config(&self.engine_state, &self.stack); let style_computer = StyleComputer::from_config(&self.engine_state, &self.stack); let term_width = get_width_param(self.width_param); let ctrlc = self.ctrlc.clone(); let span = self.head; - let opts = BuildConfig::new(ctrlc, config, &style_computer, span, term_width); + let opts = BuildConfig::new(ctrlc, &config, &style_computer, span, term_width); build_table_batch(batch, TableView::Collapsed, 0, opts) } fn build_general(&mut self, batch: Vec) -> StringResult { let term_width = get_width_param(self.width_param); - let config = &self.engine_state.get_config(); + let config = get_config(&self.engine_state, &self.stack); let style_computer = StyleComputer::from_config(&self.engine_state, &self.stack); let ctrlc = self.ctrlc.clone(); let span = self.head; let row_offset = self.row_offset; - let opts = BuildConfig::new(ctrlc, config, &style_computer, span, term_width); + let opts = BuildConfig::new(ctrlc, &config, &style_computer, span, term_width); build_table_batch(batch, TableView::General, row_offset, opts) } @@ -762,7 +756,7 @@ impl Iterator for PagingTableCreator { match table { Ok(Some(table)) => { - let table = maybe_strip_color(table, self.engine_state.get_config()); + let table = maybe_strip_color(table, &get_config(&self.engine_state, &self.stack)); let mut bytes = table.as_bytes().to_vec(); bytes.push(b'\n'); // nu-table tables don't come with a newline on the end @@ -898,7 +892,7 @@ fn create_empty_placeholder( engine_state: &EngineState, stack: &Stack, ) -> String { - let config = engine_state.get_config(); + let config = get_config(engine_state, stack); if !config.table_show_empty { return String::new(); } @@ -910,7 +904,7 @@ fn create_empty_placeholder( let out = TableOutput::new(table, false, false); let style_computer = &StyleComputer::from_config(engine_state, stack); - let config = create_table_config(config, style_computer, &out); + let config = create_table_config(&config, style_computer, &out); out.table .draw(config, termwidth) diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index c887f5c6e..3d69cca73 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use nu_protocol::ast::{Call, Expr, PathMember}; use nu_protocol::engine::{EngineState, Stack}; -use nu_protocol::{PipelineData, ShellError, Span, Value, VarId}; +use nu_protocol::{Config, PipelineData, ShellError, Span, Value, VarId}; use nu_path::canonicalize_with; @@ -303,6 +303,18 @@ pub fn find_in_dirs_env( Ok(check_dir(lib_dirs).or_else(|| check_dir(lib_dirs_fallback))) } +/// Get config +/// +/// This combines config stored in permanent state and any runtime updates to the environment. This +/// is the canonical way to fetch config at runtime when you have Stack available. +pub fn get_config(engine_state: &EngineState, stack: &Stack) -> Config { + if let Some(mut config_record) = stack.get_env_var(engine_state, "config") { + config_record.into_config(engine_state.get_config()).0 + } else { + engine_state.get_config().clone() + } +} + fn get_converted_value( engine_state: &EngineState, stack: &Stack, diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 002f1b8fa..aa3a3fbd2 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -279,7 +279,11 @@ impl EngineState { } /// Merge the environment from the runtime Stack into the engine state - pub fn merge_env(&mut self, stack: &mut Stack) -> Result<(), ShellError> { + 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) { @@ -306,27 +310,12 @@ impl EngineState { } } - Ok(()) - } - - /// Set a CWD. - pub fn set_current_working_dir(&mut self, cwd: impl AsRef) -> Result<(), ShellError> { // TODO: better error std::env::set_current_dir(cwd)?; + Ok(()) } - /// Merge the environment from the runtime Stack into the engine state - /// - /// A merge which does not consume the stack env. - pub fn clone_with_env(&self, stack: &Stack) -> Result { - let mut engine = self.clone(); - let mut stack = stack.clone(); - engine.merge_env(&mut stack)?; - - Ok(engine) - } - /// Mark a starting point if it is a script (e.g., nu spam.nu) pub fn start_in_file(&mut self, file_path: Option<&str>) { self.currently_parsed_cwd = if let Some(path) = file_path { @@ -1665,6 +1654,10 @@ impl<'a> StateWorkingSet<'a> { self.permanent_state.get_env_var(name) } + /// Returns a reference to the config stored at permanent state + /// + /// At runtime, you most likely want to call nu_engine::env::get_config because this method + /// does not capture environment updates during runtime. pub fn get_config(&self) -> &Config { &self.permanent_state.config } diff --git a/crates/nu-std/src/lib.rs b/crates/nu-std/src/lib.rs index 2bf1b2691..bb3db92d3 100644 --- a/crates/nu-std/src/lib.rs +++ b/crates/nu-std/src/lib.rs @@ -98,8 +98,7 @@ use std dirs [ )?; let cwd = current_dir(engine_state, &stack)?; - engine_state.merge_env(&mut stack)?; - engine_state.set_current_working_dir(cwd)?; + engine_state.merge_env(&mut stack, cwd)?; Ok(()) } diff --git a/src/config_files.rs b/src/config_files.rs index 635a79189..3793e4e57 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -136,7 +136,18 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut info!("read_config_file {}:{}:{}", file!(), line!(), column!()); // Merge the environment in case env vars changed in the config - merge_env(engine_state, stack) + 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); + } + } } fn eval_default_config( @@ -161,18 +172,9 @@ fn eval_default_config( ); // Merge the environment in case env vars changed in the config - merge_env(engine_state, stack) -} - -fn merge_env(engine_state: &mut EngineState, stack: &mut Stack) { match nu_engine::env::current_dir(engine_state, stack) { Ok(cwd) => { - if let Err(e) = engine_state.merge_env(stack) { - let working_set = StateWorkingSet::new(engine_state); - report_error(&working_set, &e); - } - - if let Err(e) = engine_state.set_current_working_dir(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/test_bins.rs b/src/test_bins.rs index 6fedeb4c9..3f14519ca 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -188,11 +188,7 @@ pub fn nu_repl() { // 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) { - outcome_err(&engine_state, &err); - } - - if let Err(err) = engine_state.set_current_working_dir(&cwd) { + if let Err(err) = engine_state.merge_env(&mut stack, &cwd) { outcome_err(&engine_state, &err); }