Fix config creation during printing (#9353)

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);
                     }

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 {

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,

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 {

crates/nu-cli/tests/completions.rs

@@ -768,8 +768,7 @@ fn run_external_completion(block: &str, input: &str) -> Vec<Suggestion> {
     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;
 

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)
 }

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);
 

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]

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)
 }

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<PipelineData, ShellError> {
-        // 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<i64> = 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<i64>,
 ) -> Result<PipelineData, ShellError> {
     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<Value>) -> 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)

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,

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<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) {
@@ -306,25 +310,10 @@ impl EngineState {
             }
         }
 
-        Ok(())
-    }
-
-    /// Set a CWD.
-    pub fn set_current_working_dir(&mut self, cwd: impl AsRef<Path>) -> 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<Self, ShellError> {
-        let mut engine = self.clone();
-        let mut stack = stack.clone();
-        engine.merge_env(&mut stack)?;
-
-        Ok(engine)
+        Ok(())
     }
 
     /// Mark a starting point if it is a script (e.g., nu spam.nu)
@@ -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
     }

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(())
 }