From f69a8120555289f696bb9d1a92eeab4ed932b270 Mon Sep 17 00:00:00 2001 From: Bahex Date: Thu, 26 Jun 2025 00:22:43 +0300 Subject: [PATCH] fix(hooks): updating `$env.config` now correctly updates config state. (#16021) - fixes #14946 - related #15227 - > [When I run nushell with the hook, the hook itself works as expected, correctly detects system theme and changes $env.config.color_config. However, it seems that the change to $env.config.color_config is not propagated outside the hook](https://github.com/nushell/nushell/issues/15227#issuecomment-2695287318) - > [But it suffers from the same problem - modifications made to the $env.config variable are not visible outside of the hook (which I'm not sure if is correct behavior or bug).](https://github.com/nushell/nushell/issues/15227#issuecomment-2695741542) - > [I also managed to get it working with def --env, but there was one more issue, I had to change $env.config.hooks.pre_prompt = [{ switch_theme }] into $env.config.hooks.pre_execution = ([ switch_theme ])](https://github.com/nushell/nushell/issues/15227#issuecomment-2704537565) (having to use a string hook rather than a closure) - related #11082 > Might be possible solve or at least mitigate using a similar method # Description Recently realized that changes made to `$env.config` in closure hooks don't take effect whereas string hooks don't have that problem. After some investigation: - Hooks' environment was not preserved prior to #5982 > [2309601](https://github.com/kubouch/nushell/blob/2309601dd42eee5009420464d94df275d688dbef/crates/nu-cli/src/repl.rs#L823-L840) - `redirect_env` which properly updates the config state was implemented afterwards in #6355 > [ea8b0e8](https://github.com/kubouch/nushell/blob/ea8b0e8a1deb53b6cf0cf35ccc906e942ed4612b/crates/nu-engine/src/eval.rs#L174-L190) Simply using `nu_engine::eval::redirect_env` for the environment update was enough to fix the issue. # User-Facing Changes Hooks can update `$env.config` and the configuration change will work as expected. # Tests + Formatting - :green_circle: toolkit fmt - :green_circle: toolkit clippy - :green_circle: toolkit test - :green_circle: toolkit test stdlib # After Submitting N/A Co-authored-by: Bahex <17417311+Bahex@users.noreply.github.com> --- crates/nu-cmd-base/src/hook.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/crates/nu-cmd-base/src/hook.rs b/crates/nu-cmd-base/src/hook.rs index c6fd81409f..feb912efa4 100644 --- a/crates/nu-cmd-base/src/hook.rs +++ b/crates/nu-cmd-base/src/hook.rs @@ -1,5 +1,5 @@ use miette::Result; -use nu_engine::{eval_block, eval_block_with_early_return}; +use nu_engine::{eval_block, eval_block_with_early_return, redirect_env}; use nu_parser::parse; use nu_protocol::{ PipelineData, PositionalArg, ShellError, Span, Type, Value, VarId, @@ -325,19 +325,7 @@ fn run_hook( } // If all went fine, preserve the environment of the called block - let caller_env_vars = stack.get_env_var_names(engine_state); + redirect_env(engine_state, stack, &callee_stack); - // remove env vars that are present in the caller but not in the callee - // (the callee hid them) - for var in caller_env_vars.iter() { - if !callee_stack.has_env_var(engine_state, var) { - stack.remove_env_var(engine_state, var); - } - } - - // add new env vars from callee to caller - for (var, value) in callee_stack.get_stack_env_vars() { - stack.add_env_var(var, value); - } Ok(pipeline_data) }