diff --git a/crates/nu-cli/src/completions/command_completions.rs b/crates/nu-cli/src/completions/command_completions.rs index ce4383483a..ca54913ea1 100644 --- a/crates/nu-cli/src/completions/command_completions.rs +++ b/crates/nu-cli/src/completions/command_completions.rs @@ -43,7 +43,7 @@ impl CommandCompletion { let paths = working_set.permanent_state.get_env_var_insensitive("path"); - if let Some(paths) = paths { + if let Some((_, paths)) = paths { if let Ok(paths) = paths.as_list() { for path in paths { let path = path.coerce_str().unwrap_or_default(); diff --git a/crates/nu-cli/src/eval_cmds.rs b/crates/nu-cli/src/eval_cmds.rs index 5fba00f5c9..c126d3c6fc 100644 --- a/crates/nu-cli/src/eval_cmds.rs +++ b/crates/nu-cli/src/eval_cmds.rs @@ -1,5 +1,5 @@ use log::info; -use nu_engine::{convert_env_values, eval_block}; +use nu_engine::eval_block; use nu_parser::parse; use nu_protocol::{ cli_error::report_compile_error, @@ -50,9 +50,6 @@ pub fn evaluate_commands( } } - // Translate environment variables from Strings to Values - convert_env_values(engine_state, stack)?; - // Parse the source code let (block, delta) = { if let Some(ref t_mode) = table_mode { diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs index 826b6c8eb5..aa4dbfca78 100644 --- a/crates/nu-cli/src/eval_file.rs +++ b/crates/nu-cli/src/eval_file.rs @@ -1,6 +1,6 @@ use crate::util::{eval_source, print_pipeline}; use log::{info, trace}; -use nu_engine::{convert_env_values, eval_block}; +use nu_engine::eval_block; use nu_parser::parse; use nu_path::canonicalize_with; use nu_protocol::{ @@ -22,9 +22,6 @@ pub fn evaluate_file( stack: &mut Stack, input: PipelineData, ) -> Result<(), ShellError> { - // Convert environment variables from Strings to Values and store them in the engine state. - convert_env_values(engine_state, stack)?; - let cwd = engine_state.cwd_as_string(Some(stack))?; let file_path = diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index 0d3fde7de6..345d81d6f7 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -32,7 +32,9 @@ pub fn convert_env_vars( ) -> Result<(), ShellError> { let conversions = conversions.as_record()?; for (key, conversion) in conversions.into_iter() { - if let Some(val) = stack.get_env_var_insensitive(engine_state, key) { + if let Some((case_preserve_env_name, val)) = + stack.get_env_var_insensitive(engine_state, key) + { match val.get_type() { Type::String => {} _ => continue, @@ -52,7 +54,7 @@ pub fn convert_env_vars( .run_with_value(val.clone())? .into_value(val.span())?; - stack.add_env_var(key.clone(), new_val); + stack.add_env_var(case_preserve_env_name.to_string(), new_val); } } Ok(()) @@ -64,7 +66,10 @@ pub fn convert_env_vars( /// It returns Option instead of Result since we do want to translate all the values we can and /// skip errors. This function is called in the main() so we want to keep running, we cannot just /// exit. -pub fn convert_env_values(engine_state: &mut EngineState, stack: &Stack) -> Result<(), ShellError> { +pub fn convert_env_values( + engine_state: &mut EngineState, + stack: &mut Stack, +) -> Result<(), ShellError> { let mut error = None; let mut new_scope = HashMap::new(); @@ -89,7 +94,7 @@ pub fn convert_env_values(engine_state: &mut EngineState, stack: &Stack) -> Resu } } - error = error.or_else(|| ensure_path(&mut new_scope, "PATH")); + error = error.or_else(|| ensure_path(engine_state, stack)); if let Ok(last_overlay_name) = &stack.last_overlay_name() { if let Some(env_vars) = Arc::make_mut(&mut engine_state.env_vars).get_mut(last_overlay_name) @@ -242,7 +247,7 @@ pub fn path_str( span: Span, ) -> Result { let (pathname, pathval) = match stack.get_env_var_insensitive(engine_state, "path") { - Some(v) => Ok((if cfg!(windows) { "Path" } else { "PATH" }, v)), + Some((_, v)) => Ok((if cfg!(windows) { "Path" } else { "PATH" }, v)), None => Err(ShellError::EnvVarNotFoundAtRuntime { envvar_name: if cfg!(windows) { "Path".to_string() @@ -362,11 +367,11 @@ fn get_converted_value( ) } -fn ensure_path(scope: &mut HashMap, env_path_name: &str) -> Option { +fn ensure_path(engine_state: &EngineState, stack: &mut Stack) -> Option { let mut error = None; // If PATH/Path is still a string, force-convert it to a list - if let Some(value) = scope.get(env_path_name) { + if let Some((preserve_case_name, value)) = stack.get_env_var_insensitive(engine_state, "Path") { let span = value.span(); match value { Value::String { val, .. } => { @@ -375,15 +380,17 @@ fn ensure_path(scope: &mut HashMap, env_path_name: &str) -> Optio .map(|p| Value::string(p.to_string_lossy().to_string(), span)) .collect(); - scope.insert(env_path_name.to_string(), Value::list(paths, span)); + stack.add_env_var(preserve_case_name.to_string(), Value::list(paths, span)); } Value::List { vals, .. } => { // Must be a list of strings if !vals.iter().all(|v| matches!(v, Value::String { .. })) { error = error.or_else(|| { Some(ShellError::GenericError { - error: format!("Wrong {env_path_name} environment variable value"), - msg: format!("{env_path_name} must be a list of strings"), + error: format!( + "Incorrect {preserve_case_name} environment variable value" + ), + msg: format!("{preserve_case_name} must be a list of strings"), span: Some(span), help: None, inner: vec![], @@ -398,8 +405,8 @@ fn ensure_path(scope: &mut HashMap, env_path_name: &str) -> Optio error = error.or_else(|| { Some(ShellError::GenericError { - error: format!("Wrong {env_path_name} environment variable value"), - msg: format!("{env_path_name} must be a list of strings"), + error: format!("Incorrect {preserve_case_name} environment variable value"), + msg: format!("{preserve_case_name} must be a list of strings"), span: Some(span), help: None, inner: vec![], diff --git a/crates/nu-plugin-engine/src/context.rs b/crates/nu-plugin-engine/src/context.rs index 5652cf1a5f..5870ab7d47 100644 --- a/crates/nu-plugin-engine/src/context.rs +++ b/crates/nu-plugin-engine/src/context.rs @@ -126,7 +126,10 @@ impl<'a> PluginExecutionContext for PluginExecutionCommandContext<'a> { } fn get_env_var(&self, name: &str) -> Result, ShellError> { - Ok(self.stack.get_env_var_insensitive(&self.engine_state, name)) + Ok(self + .stack + .get_env_var_insensitive(&self.engine_state, name) + .map(|(_, value)| value)) } fn get_env_vars(&self) -> Result, ShellError> { diff --git a/crates/nu-protocol/src/config/ansi_coloring.rs b/crates/nu-protocol/src/config/ansi_coloring.rs index af016923da..4cbc893faa 100644 --- a/crates/nu-protocol/src/config/ansi_coloring.rs +++ b/crates/nu-protocol/src/config/ansi_coloring.rs @@ -48,6 +48,7 @@ impl UseAnsiColoring { let env_value = |env_name| { engine_state .get_env_var_insensitive(env_name) + .map(|(_, v)| v) .and_then(|v| v.coerce_bool().ok()) .unwrap_or(false) }; @@ -60,7 +61,7 @@ impl UseAnsiColoring { return false; } - if let Some(cli_color) = engine_state.get_env_var_insensitive("clicolor") { + if let Some((_, cli_color)) = engine_state.get_env_var_insensitive("clicolor") { if let Ok(cli_color) = cli_color.coerce_bool() { return cli_color; } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 328c3d7694..8270e4111a 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -466,12 +466,16 @@ impl EngineState { None } - pub fn get_env_var_insensitive(&self, name: &str) -> Option<&Value> { + // Returns Some((name, value)) if found, None otherwise. + // When updating environment variables, make sure to use + // the same case (the returned "name") as the original + // environment variable name. + pub fn get_env_var_insensitive(&self, name: &str) -> Option<(&String, &Value)> { for overlay_id in self.scope.active_overlays.iter().rev() { let overlay_name = String::from_utf8_lossy(self.get_overlay_name(*overlay_id)); if let Some(env_vars) = self.env_vars.get(overlay_name.as_ref()) { if let Some(v) = env_vars.iter().find(|(k, _)| k.eq_ignore_case(name)) { - return Some(v.1); + return Some((v.0, v.1)); } } } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index a1d50381dc..0e8df6b654 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -481,16 +481,20 @@ impl Stack { } // Case-Insensitive version of get_env_var + // Returns Some((name, value)) if found, None otherwise. + // When updating environment variables, make sure to use + // the same case (from the returned "name") as the original + // environment variable name. pub fn get_env_var_insensitive<'a>( &'a self, engine_state: &'a EngineState, name: &str, - ) -> Option<&'a Value> { + ) -> Option<(&'a String, &'a Value)> { for scope in self.env_vars.iter().rev() { for active_overlay in self.active_overlays.iter().rev() { if let Some(env_vars) = scope.get(active_overlay) { if let Some(v) = env_vars.iter().find(|(k, _)| k.eq_ignore_case(name)) { - return Some(v.1); + return Some((v.0, v.1)); } } } @@ -506,7 +510,7 @@ impl Stack { if !is_hidden { if let Some(env_vars) = engine_state.env_vars.get(active_overlay) { if let Some(v) = env_vars.iter().find(|(k, _)| k.eq_ignore_case(name)) { - return Some(v.1); + return Some((v.0, v.1)); } } } diff --git a/src/config_files.rs b/src/config_files.rs index 3493fe0be8..ca77113ae5 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -2,14 +2,13 @@ use log::warn; #[cfg(feature = "plugin")] use nu_cli::read_plugin_file; use nu_cli::{eval_config_contents, eval_source}; -use nu_engine::convert_env_values; use nu_path::canonicalize_with; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, eval_const::{get_user_autoload_dirs, get_vendor_autoload_dirs}, report_parse_error, report_shell_error, Config, ParseError, PipelineData, Spanned, }; -use nu_utils::{get_default_config, get_default_env, get_scaffold_config, get_scaffold_env, perf}; +use nu_utils::{get_default_config, get_default_env, get_scaffold_config, get_scaffold_env}; use std::{ fs, fs::File, @@ -37,20 +36,6 @@ pub(crate) fn read_config_file( if is_env_config { eval_default_config(engine_state, stack, get_default_env(), is_env_config); - - let start_time = std::time::Instant::now(); - let config = engine_state.get_config(); - let use_color = config.use_ansi_coloring.get(engine_state); - // Translate environment variables from Strings to Values - if let Err(e) = convert_env_values(engine_state, stack) { - report_shell_error(engine_state, &e); - } - - perf!( - "translate env vars after default_env.nu", - start_time, - use_color - ); } else { eval_default_config(engine_state, stack, get_default_config(), is_env_config); }; diff --git a/src/main.rs b/src/main.rs index cb86557ac2..0e7c656fc7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,11 +21,13 @@ use command::gather_commandline_args; use log::{trace, Level}; use miette::Result; use nu_cli::gather_parent_env_vars; +use nu_engine::convert_env_values; use nu_lsp::LanguageServer; use nu_path::canonicalize_with; use nu_protocol::{ - engine::EngineState, record, report_shell_error, ByteStream, Config, IntoValue, PipelineData, - ShellError, Span, Spanned, Type, Value, + engine::{EngineState, Stack}, + record, report_shell_error, ByteStream, Config, IntoValue, PipelineData, ShellError, Span, + Spanned, Type, Value, }; use nu_std::load_standard_library; use nu_utils::perf; @@ -319,6 +321,16 @@ fn main() -> Result<()> { gather_parent_env_vars(&mut engine_state, init_cwd.as_ref()); perf!("gather env vars", start_time, use_color); + let mut stack = Stack::new(); + start_time = std::time::Instant::now(); + let config = engine_state.get_config(); + let use_color = config.use_ansi_coloring.get(&engine_state); + // Translate environment variables from Strings to Values + if let Err(e) = convert_env_values(&mut engine_state, &mut stack) { + report_shell_error(&engine_state, &e); + } + perf!("Convert path to list", start_time, use_color); + engine_state.add_env_var( "NU_VERSION".to_string(), Value::string(env!("CARGO_PKG_VERSION"), Span::unknown()), @@ -464,6 +476,7 @@ fn main() -> Result<()> { } else if let Some(commands) = parsed_nu_cli_args.commands.clone() { run_commands( &mut engine_state, + stack, parsed_nu_cli_args, use_color, &commands, @@ -473,6 +486,7 @@ fn main() -> Result<()> { } else if !script_name.is_empty() { run_file( &mut engine_state, + stack, parsed_nu_cli_args, use_color, script_name, @@ -509,7 +523,12 @@ fn main() -> Result<()> { shlvl += 1; engine_state.add_env_var("SHLVL".to_string(), Value::int(shlvl, Span::unknown())); - run_repl(&mut engine_state, parsed_nu_cli_args, entire_start_time)? + run_repl( + &mut engine_state, + stack, + parsed_nu_cli_args, + entire_start_time, + )? } Ok(()) diff --git a/src/run.rs b/src/run.rs index 640cc9bc35..fc3bca3676 100644 --- a/src/run.rs +++ b/src/run.rs @@ -14,6 +14,7 @@ use nu_utils::perf; pub(crate) fn run_commands( engine_state: &mut EngineState, + mut stack: Stack, parsed_nu_cli_args: command::NushellCliArgs, use_color: bool, commands: &Spanned, @@ -25,8 +26,6 @@ pub(crate) fn run_commands( let start_time = std::time::Instant::now(); let create_scaffold = nu_path::nu_config_dir().map_or(false, |p| !p.exists()); - let mut stack = Stack::new(); - // if the --no-config-file(-n) option is NOT passed, load the plugin file, // load the default env file or custom (depending on parsed_nu_cli_args.env_file), // and maybe a custom config file (depending on parsed_nu_cli_args.config_file) @@ -107,6 +106,7 @@ pub(crate) fn run_commands( pub(crate) fn run_file( engine_state: &mut EngineState, + mut stack: Stack, parsed_nu_cli_args: command::NushellCliArgs, use_color: bool, script_name: String, @@ -114,7 +114,6 @@ pub(crate) fn run_file( input: PipelineData, ) { trace!("run_file"); - let mut stack = Stack::new(); // if the --no-config-file(-n) option is NOT passed, load the plugin file, // load the default env file or custom (depending on parsed_nu_cli_args.env_file), @@ -177,11 +176,11 @@ pub(crate) fn run_file( pub(crate) fn run_repl( engine_state: &mut EngineState, + mut stack: Stack, parsed_nu_cli_args: command::NushellCliArgs, entire_start_time: std::time::Instant, ) -> Result<(), miette::ErrReport> { trace!("run_repl"); - let mut stack = Stack::new(); let start_time = std::time::Instant::now(); if parsed_nu_cli_args.no_config_file.is_none() { diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs index ab97eb6872..6513db1fbc 100644 --- a/tests/shell/environment/env.rs +++ b/tests/shell/environment/env.rs @@ -258,10 +258,39 @@ fn env_shlvl_in_repl() { #[test] fn env_shlvl_in_exec_repl() { - let actual = nu!(" + let actual = nu!(r#" $env.SHLVL = 29 - nu -c \"exec nu --no-std-lib -n -e 'print $env.SHLVL; exit'\" - "); + nu -c "exec nu --no-std-lib -n -e 'print $env.SHLVL; exit'" + "#); assert_eq!(actual.out, "30"); } + +#[test] +fn path_is_a_list_in_repl() { + let actual = nu!(r#" + nu -c "exec nu --no-std-lib -n -e 'print ($env.pATh | describe); exit'" + "#); + + assert_eq!(actual.out, "list"); +} + +#[test] +fn path_is_a_list() { + let actual = nu!(" + print ($env.path | describe) + "); + + assert_eq!(actual.out, "list"); +} + +#[test] +fn path_is_a_list_in_script() { + Playground::setup("has_file_pwd", |dirs, sandbox| { + sandbox.with_files(&[FileWithContent("checkpath.nu", "$env.path | describe")]); + + let actual = nu!(cwd: dirs.test(), "nu checkpath.nu"); + + assert!(actual.out.ends_with("list")); + }) +}