From 6b5906613c221a4309c9981560334fd22f4c9cc6 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Mon, 9 Sep 2024 20:03:06 -0700 Subject: [PATCH] Fix remaining mismatch for env handling in IR (#13796) # Description This fixes a couple of remaining differences between the IR evaluator's handling of env vars and the AST evaluator's handling of env vars. Blocker for #13718 (this is why those tests failed) # User-Facing Changes 1. Handles checking overlays for hidden env vars properly, when getting an env var from IR instruction 2. Updates config properly when doing `redirect_env()` (these probably shouldn't be separate functions anyway, though, they're basically the same. I did this because I intended to remove one, but now it's just like that) # Tests + Formatting The `nu_repl` testbin now handles `NU_USE_IR` properly, so these tests now work as expected. # After Submitting - [ ] check in on #13718 again --- crates/nu-engine/src/eval_ir.rs | 51 ++++++++++++++++++++++----------- src/test_bins.rs | 5 ++++ 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 62e9b0caad..b6f5287196 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -1274,27 +1274,41 @@ fn get_var(ctx: &EvalContext<'_>, var_id: VarId, span: Span) -> Result(ctx: &'a mut EvalContext<'_>, key: &str) -> Option<&'a Value> { // Read scopes in order - ctx.stack + for overlays in ctx + .stack .env_vars .iter() .rev() .chain(std::iter::once(&ctx.engine_state.env_vars)) - .flat_map(|overlays| { - // Read overlays in order - ctx.stack - .active_overlays - .iter() - .rev() - .filter_map(|name| overlays.get(name)) - }) - .find_map(|map| { - // Use the hashmap first to try to be faster? - map.get(key).or_else(|| { - // Check to see if it exists at all in the map - map.iter() - .find_map(|(k, v)| k.eq_ignore_case(key).then_some(v)) - }) - }) + { + // Read overlays in order + for overlay_name in ctx.stack.active_overlays.iter().rev() { + let Some(map) = overlays.get(overlay_name) else { + // Skip if overlay doesn't exist in this scope + continue; + }; + let hidden = ctx.stack.env_hidden.get(overlay_name); + let is_hidden = |key: &str| hidden.is_some_and(|hidden| hidden.contains(key)); + + if let Some(val) = map + // Check for exact match + .get(key) + // Skip when encountering an overlay where the key is hidden + .filter(|_| !is_hidden(key)) + .or_else(|| { + // Check to see if it exists at all in the map, with a different case + map.iter().find_map(|(k, v)| { + // Again, skip something that's hidden + (k.eq_ignore_case(key) && !is_hidden(k)).then_some(v) + }) + }) + { + return Some(val); + } + } + } + // Not found + None } /// Get the existing name of an environment variable, case-insensitively. This is used to implement @@ -1472,4 +1486,7 @@ fn redirect_env(engine_state: &EngineState, caller_stack: &mut Stack, callee_sta for (var, value) in callee_stack.get_stack_env_vars() { caller_stack.add_env_var(var, value); } + + // set config to callee config, to capture any updates to that + caller_stack.config.clone_from(&callee_stack.config); } diff --git a/src/test_bins.rs b/src/test_bins.rs index b20c01f997..c2ec287138 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -244,6 +244,11 @@ pub fn nu_repl() { engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy())); engine_state.add_env_var("PATH".into(), Value::test_string("")); + // Enable IR in tests if set + if std::env::var_os("NU_USE_IR").is_some() { + Arc::make_mut(&mut top_stack).use_ir = true; + } + let mut last_output = String::new(); load_standard_library(&mut engine_state).expect("Could not load the standard library.");