From 81e86c40e10b998c67a121c78c148ac60923fe4c Mon Sep 17 00:00:00 2001 From: Wind Date: Fri, 13 Jun 2025 07:22:23 +0800 Subject: [PATCH] Try to make hide-env respects overlays (#15904) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Closes: #15755 I think it's a good feature, to achieve this, we need to get all hidden envs(it's defined in `get_hidden_env_vars`, and then restore these envs back to stack) # User-Facing Changes ### Before ```nushell > $env.foo = 'bar' > overlay new xxx > hide-env foo > overlay hide xxx > $env.foo Error: nu::shell::column_not_found × Cannot find column 'foo' ╭─[entry #21:5:1] 4 │ overlay hide xxx 5 │ $env.foo · ────┬───┬ · │ ╰── value originates here · ╰── cannot find column 'foo' ╰──── ``` ### After ```nushell > $env.foo = 'bar' > overlay new xxx > hide-env foo > overlay hide xxx > $env.foo bar ``` ## Note But it doesn't work if it runs the example code in script: `nu -c "$env.foo = 'bar'; overlay new xxx; hide-env foo; overlay hide xxx; $env.foo"` still raises an error says `foo` doesn't found. That's because if we run the script at once, the envs in stack doesn't have a chance to merge back into `engine_state`, which is only called in `repl`. It introduces some sort of inconsistency, but I think users use overlays mostly in repl, so it's good to have such feature first. # Tests + Formatting Added 2 tests # After Submitting NaN --- .../src/core_commands/overlay/hide.rs | 6 +++- crates/nu-protocol/src/engine/stack.rs | 31 +++++++++++++++++++ tests/overlays/mod.rs | 30 ++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/crates/nu-cmd-lang/src/core_commands/overlay/hide.rs b/crates/nu-cmd-lang/src/core_commands/overlay/hide.rs index 02c0de0ec7..666fedead8 100644 --- a/crates/nu-cmd-lang/src/core_commands/overlay/hide.rs +++ b/crates/nu-cmd-lang/src/core_commands/overlay/hide.rs @@ -86,12 +86,16 @@ impl Command for OverlayHide { vec![] }; + // also restore env vars which has been hidden + let env_vars_to_restore = stack.get_hidden_env_vars(&overlay_name.item, engine_state); stack.remove_overlay(&overlay_name.item); + for (name, val) in env_vars_to_restore { + stack.add_env_var(name, val); + } for (name, val) in env_vars_to_keep { stack.add_env_var(name, val); } - Ok(PipelineData::empty()) } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index cd1add986e..be46327bef 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -414,6 +414,37 @@ impl Stack { result } + /// Get hidden envs, but without envs defined previously in `excluded_overlay_name`. + pub fn get_hidden_env_vars( + &self, + excluded_overlay_name: &str, + engine_state: &EngineState, + ) -> HashMap { + let mut result = HashMap::new(); + + for overlay_name in self.active_overlays.iter().rev() { + if overlay_name == excluded_overlay_name { + continue; + } + if let Some(env_names) = self.env_hidden.get(overlay_name) { + for n in env_names { + if result.contains_key(n) { + continue; + } + // get env value. + if let Some(Some(v)) = engine_state + .env_vars + .get(overlay_name) + .map(|env_vars| env_vars.get(n)) + { + result.insert(n.to_string(), v.clone()); + } + } + } + } + result + } + /// Same as get_env_vars, but returns only the names as a HashSet pub fn get_env_var_names(&self, engine_state: &EngineState) -> HashSet { let mut result = HashSet::new(); diff --git a/tests/overlays/mod.rs b/tests/overlays/mod.rs index 8c045db7d0..cd101bda6d 100644 --- a/tests/overlays/mod.rs +++ b/tests/overlays/mod.rs @@ -856,6 +856,36 @@ fn overlay_hide_renamed_overlay() { assert!(actual_repl.err.contains("external_command")); } +#[test] +fn overlay_hide_restore_hidden_env() { + let inp = &[ + "$env.foo = 'bar'", + "overlay new aa", + "hide-env foo", + "overlay hide aa", + "$env.foo", + ]; + + let actual_repl = nu!(nu_repl_code(inp)); + + assert_eq!(actual_repl.out, "bar"); +} + +#[test] +fn overlay_hide_dont_restore_hidden_env_which_is_introduce_currently() { + let inp = &[ + "overlay new aa", + "$env.foo = 'bar'", + "hide-env foo", // hide the env in overlay `aa` + "overlay hide aa", + "'foo' in $env", + ]; + + let actual_repl = nu!(nu_repl_code(inp)); + + assert_eq!(actual_repl.out, "false"); +} + #[test] fn overlay_hide_and_add_renamed_overlay() { let inp = &[