diff --git a/crates/nu-command/src/env/env_command.rs b/crates/nu-command/src/env/env_command.rs index 4dab284b24..20023a96d2 100644 --- a/crates/nu-command/src/env/env_command.rs +++ b/crates/nu-command/src/env/env_command.rs @@ -1,7 +1,9 @@ use nu_engine::env_to_string; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; -use nu_protocol::{Category, Example, IntoPipelineData, PipelineData, Signature, Value}; +use nu_protocol::{ + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Value, +}; #[derive(Clone)] pub struct Env; @@ -27,7 +29,6 @@ impl Command for Env { _input: PipelineData, ) -> Result { let span = call.head; - let config = stack.get_config().unwrap_or_default(); let mut env_vars: Vec<(String, Value)> = stack.get_env_vars(engine_state).into_iter().collect(); @@ -39,7 +40,12 @@ impl Command for Env { let mut cols = vec![]; let mut vals = vec![]; - let raw = env_to_string(&name, val.clone(), engine_state, stack, &config)?; + let raw_val = match env_to_string(&name, &val, engine_state, stack) { + Ok(raw) => Value::string(raw, span), + Err(ShellError::EnvVarNotAString(..)) => Value::nothing(span), + Err(e) => return Err(e), + }; + let val_type = val.get_type(); cols.push("name".into()); @@ -52,7 +58,7 @@ impl Command for Env { vals.push(val); cols.push("raw".into()); - vals.push(Value::string(raw, span)); + vals.push(raw_val); values.push(Value::Record { cols, vals, span }); } diff --git a/crates/nu-command/src/system/exec.rs b/crates/nu-command/src/system/exec.rs index 26d6c16419..792aba1593 100644 --- a/crates/nu-command/src/system/exec.rs +++ b/crates/nu-command/src/system/exec.rs @@ -76,8 +76,7 @@ fn exec( let args: Vec> = call.rest(engine_state, stack, 1)?; let cwd = current_dir(engine_state, stack)?; - let config = stack.get_config()?; - let env_vars = env_to_strings(engine_state, stack, &config)?; + let env_vars = env_to_strings(engine_state, stack)?; let current_dir = current_dir(engine_state, stack)?; let external_command = ExternalCommand { diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 9244c56fcb..11d63a7b87 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -55,8 +55,7 @@ impl Command for External { let redirect_stderr = call.has_flag("redirect-stderr"); // Translate environment variables from Values to Strings - let config = stack.get_config().unwrap_or_default(); - let env_vars_str = env_to_strings(engine_state, stack, &config)?; + let env_vars_str = env_to_strings(engine_state, stack)?; fn value_as_spanned(value: Value) -> Result, ShellError> { let span = value.span()?; diff --git a/crates/nu-command/src/viewers/griddle.rs b/crates/nu-command/src/viewers/griddle.rs index b851dafe4e..c48a951da6 100644 --- a/crates/nu-command/src/viewers/griddle.rs +++ b/crates/nu-command/src/viewers/griddle.rs @@ -66,7 +66,7 @@ prints out the list properly."# let separator_param: Option = call.get_flag(engine_state, stack, "separator")?; let config = stack.get_config().unwrap_or_default(); let env_str = match stack.get_env_var(engine_state, "LS_COLORS") { - Some(v) => Some(env_to_string("LS_COLORS", v, engine_state, stack, &config)?), + Some(v) => Some(env_to_string("LS_COLORS", &v, engine_state, stack)?), None => None, }; let use_grid_icons = config.use_grid_icons; diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 9c789e74ed..f4109b76c9 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -199,10 +199,9 @@ fn handle_row_stream( let ls_colors = match stack.get_env_var(engine_state, "LS_COLORS") { Some(v) => LsColors::from_string(&env_to_string( "LS_COLORS", - v, + &v, engine_state, stack, - &config, )?), None => LsColors::from_string("st=0:di=0;38;5;81:so=0;38;5;16;48;5;203:ln=0;38;5;203:cd=0;38;5;203;48;5;236:ex=1;38;5;203:or=0;38;5;16;48;5;203:fi=0:bd=0;38;5;81;48;5;236:ow=0:mi=0;38;5;16;48;5;203:*~=0;38;5;243:no=0:tw=0:pi=0;38;5;16;48;5;81:*.z=4;38;5;203:*.t=0;38;5;48:*.o=0;38;5;243:*.d=0;38;5;48:*.a=1;38;5;203:*.c=0;38;5;48:*.m=0;38;5;48:*.p=0;38;5;48:*.r=0;38;5;48:*.h=0;38;5;48:*.ml=0;38;5;48:*.ll=0;38;5;48:*.gv=0;38;5;48:*.cp=0;38;5;48:*.xz=4;38;5;203:*.hs=0;38;5;48:*css=0;38;5;48:*.ui=0;38;5;149:*.pl=0;38;5;48:*.ts=0;38;5;48:*.gz=4;38;5;203:*.so=1;38;5;203:*.cr=0;38;5;48:*.fs=0;38;5;48:*.bz=4;38;5;203:*.ko=1;38;5;203:*.as=0;38;5;48:*.sh=0;38;5;48:*.pp=0;38;5;48:*.el=0;38;5;48:*.py=0;38;5;48:*.lo=0;38;5;243:*.bc=0;38;5;243:*.cc=0;38;5;48:*.pm=0;38;5;48:*.rs=0;38;5;48:*.di=0;38;5;48:*.jl=0;38;5;48:*.rb=0;38;5;48:*.md=0;38;5;185:*.js=0;38;5;48:*.go=0;38;5;48:*.vb=0;38;5;48:*.hi=0;38;5;243:*.kt=0;38;5;48:*.hh=0;38;5;48:*.cs=0;38;5;48:*.mn=0;38;5;48:*.nb=0;38;5;48:*.7z=4;38;5;203:*.ex=0;38;5;48:*.rm=0;38;5;208:*.ps=0;38;5;186:*.td=0;38;5;48:*.la=0;38;5;243:*.aux=0;38;5;243:*.xmp=0;38;5;149:*.mp4=0;38;5;208:*.rpm=4;38;5;203:*.m4a=0;38;5;208:*.zip=4;38;5;203:*.dll=1;38;5;203:*.bcf=0;38;5;243:*.awk=0;38;5;48:*.aif=0;38;5;208:*.zst=4;38;5;203:*.bak=0;38;5;243:*.tgz=4;38;5;203:*.com=1;38;5;203:*.clj=0;38;5;48:*.sxw=0;38;5;186:*.vob=0;38;5;208:*.fsx=0;38;5;48:*.doc=0;38;5;186:*.mkv=0;38;5;208:*.tbz=4;38;5;203:*.ogg=0;38;5;208:*.wma=0;38;5;208:*.mid=0;38;5;208:*.kex=0;38;5;186:*.out=0;38;5;243:*.ltx=0;38;5;48:*.sql=0;38;5;48:*.ppt=0;38;5;186:*.tex=0;38;5;48:*.odp=0;38;5;186:*.log=0;38;5;243:*.arj=4;38;5;203:*.ipp=0;38;5;48:*.sbt=0;38;5;48:*.jpg=0;38;5;208:*.yml=0;38;5;149:*.txt=0;38;5;185:*.csv=0;38;5;185:*.dox=0;38;5;149:*.pro=0;38;5;149:*.bst=0;38;5;149:*TODO=1:*.mir=0;38;5;48:*.bat=1;38;5;203:*.m4v=0;38;5;208:*.pod=0;38;5;48:*.cfg=0;38;5;149:*.pas=0;38;5;48:*.tml=0;38;5;149:*.bib=0;38;5;149:*.ini=0;38;5;149:*.apk=4;38;5;203:*.h++=0;38;5;48:*.pyc=0;38;5;243:*.img=4;38;5;203:*.rst=0;38;5;185:*.swf=0;38;5;208:*.htm=0;38;5;185:*.ttf=0;38;5;208:*.elm=0;38;5;48:*hgrc=0;38;5;149:*.bmp=0;38;5;208:*.fsi=0;38;5;48:*.pgm=0;38;5;208:*.dpr=0;38;5;48:*.xls=0;38;5;186:*.tcl=0;38;5;48:*.mli=0;38;5;48:*.ppm=0;38;5;208:*.bbl=0;38;5;243:*.lua=0;38;5;48:*.asa=0;38;5;48:*.pbm=0;38;5;208:*.avi=0;38;5;208:*.def=0;38;5;48:*.mov=0;38;5;208:*.hxx=0;38;5;48:*.tif=0;38;5;208:*.fon=0;38;5;208:*.zsh=0;38;5;48:*.png=0;38;5;208:*.inc=0;38;5;48:*.jar=4;38;5;203:*.swp=0;38;5;243:*.pid=0;38;5;243:*.gif=0;38;5;208:*.ind=0;38;5;243:*.erl=0;38;5;48:*.ilg=0;38;5;243:*.eps=0;38;5;208:*.tsx=0;38;5;48:*.git=0;38;5;243:*.inl=0;38;5;48:*.rtf=0;38;5;186:*.hpp=0;38;5;48:*.kts=0;38;5;48:*.deb=4;38;5;203:*.svg=0;38;5;208:*.pps=0;38;5;186:*.ps1=0;38;5;48:*.c++=0;38;5;48:*.cpp=0;38;5;48:*.bsh=0;38;5;48:*.php=0;38;5;48:*.exs=0;38;5;48:*.toc=0;38;5;243:*.mp3=0;38;5;208:*.epp=0;38;5;48:*.rar=4;38;5;203:*.wav=0;38;5;208:*.xlr=0;38;5;186:*.tmp=0;38;5;243:*.cxx=0;38;5;48:*.iso=4;38;5;203:*.dmg=4;38;5;203:*.gvy=0;38;5;48:*.bin=4;38;5;203:*.wmv=0;38;5;208:*.blg=0;38;5;243:*.ods=0;38;5;186:*.psd=0;38;5;208:*.mpg=0;38;5;208:*.dot=0;38;5;48:*.cgi=0;38;5;48:*.xml=0;38;5;185:*.htc=0;38;5;48:*.ics=0;38;5;186:*.bz2=4;38;5;203:*.tar=4;38;5;203:*.csx=0;38;5;48:*.ico=0;38;5;208:*.sxi=0;38;5;186:*.nix=0;38;5;149:*.pkg=4;38;5;203:*.bag=4;38;5;203:*.fnt=0;38;5;208:*.idx=0;38;5;243:*.xcf=0;38;5;208:*.exe=1;38;5;203:*.flv=0;38;5;208:*.fls=0;38;5;243:*.otf=0;38;5;208:*.vcd=4;38;5;203:*.vim=0;38;5;48:*.sty=0;38;5;243:*.pdf=0;38;5;186:*.odt=0;38;5;186:*.purs=0;38;5;48:*.h264=0;38;5;208:*.jpeg=0;38;5;208:*.dart=0;38;5;48:*.pptx=0;38;5;186:*.lock=0;38;5;243:*.bash=0;38;5;48:*.rlib=0;38;5;243:*.hgrc=0;38;5;149:*.psm1=0;38;5;48:*.toml=0;38;5;149:*.tbz2=4;38;5;203:*.yaml=0;38;5;149:*.make=0;38;5;149:*.orig=0;38;5;243:*.html=0;38;5;185:*.fish=0;38;5;48:*.diff=0;38;5;48:*.xlsx=0;38;5;186:*.docx=0;38;5;186:*.json=0;38;5;149:*.psd1=0;38;5;48:*.tiff=0;38;5;208:*.flac=0;38;5;208:*.java=0;38;5;48:*.less=0;38;5;48:*.mpeg=0;38;5;208:*.conf=0;38;5;149:*.lisp=0;38;5;48:*.epub=0;38;5;186:*.cabal=0;38;5;48:*.patch=0;38;5;48:*.shtml=0;38;5;185:*.class=0;38;5;243:*.xhtml=0;38;5;185:*.mdown=0;38;5;185:*.dyn_o=0;38;5;243:*.cache=0;38;5;243:*.swift=0;38;5;48:*README=0;38;5;16;48;5;186:*passwd=0;38;5;149:*.ipynb=0;38;5;48:*shadow=0;38;5;149:*.toast=4;38;5;203:*.cmake=0;38;5;149:*.scala=0;38;5;48:*.dyn_hi=0;38;5;243:*.matlab=0;38;5;48:*.config=0;38;5;149:*.gradle=0;38;5;48:*.groovy=0;38;5;48:*.ignore=0;38;5;149:*LICENSE=0;38;5;249:*TODO.md=1:*COPYING=0;38;5;249:*.flake8=0;38;5;149:*INSTALL=0;38;5;16;48;5;186:*setup.py=0;38;5;149:*.gemspec=0;38;5;149:*.desktop=0;38;5;149:*Makefile=0;38;5;149:*Doxyfile=0;38;5;149:*TODO.txt=1:*README.md=0;38;5;16;48;5;186:*.kdevelop=0;38;5;149:*.rgignore=0;38;5;149:*configure=0;38;5;149:*.DS_Store=0;38;5;243:*.fdignore=0;38;5;149:*COPYRIGHT=0;38;5;249:*.markdown=0;38;5;185:*.cmake.in=0;38;5;149:*.gitconfig=0;38;5;149:*INSTALL.md=0;38;5;16;48;5;186:*CODEOWNERS=0;38;5;149:*.gitignore=0;38;5;149:*Dockerfile=0;38;5;149:*SConstruct=0;38;5;149:*.scons_opt=0;38;5;243:*README.txt=0;38;5;16;48;5;186:*SConscript=0;38;5;149:*.localized=0;38;5;243:*.travis.yml=0;38;5;186:*Makefile.in=0;38;5;243:*.gitmodules=0;38;5;149:*LICENSE-MIT=0;38;5;249:*Makefile.am=0;38;5;149:*INSTALL.txt=0;38;5;16;48;5;186:*MANIFEST.in=0;38;5;149:*.synctex.gz=0;38;5;243:*.fdb_latexmk=0;38;5;243:*CONTRIBUTORS=0;38;5;16;48;5;186:*configure.ac=0;38;5;149:*.applescript=0;38;5;48:*appveyor.yml=0;38;5;186:*.clang-format=0;38;5;149:*.gitattributes=0;38;5;149:*LICENSE-APACHE=0;38;5;249:*CMakeCache.txt=0;38;5;243:*CMakeLists.txt=0;38;5;149:*CONTRIBUTORS.md=0;38;5;16;48;5;186:*requirements.txt=0;38;5;149:*CONTRIBUTORS.txt=0;38;5;16;48;5;186:*.sconsign.dblite=0;38;5;243:*package-lock.json=0;38;5;243:*.CFUserTextEncoding=0;38;5;243"), }; diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index d2785746f2..1e36a93f01 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -3,15 +3,10 @@ use std::path::{Path, PathBuf}; use nu_protocol::ast::PathMember; use nu_protocol::engine::{EngineState, Stack}; -use nu_protocol::{Config, PipelineData, ShellError, Span, Value}; +use nu_protocol::{PipelineData, ShellError, Span, Value}; use crate::eval_block; -#[cfg(windows)] -const ENV_SEP: &str = ";"; -#[cfg(not(windows))] -const ENV_SEP: &str = ":"; - #[cfg(windows)] const ENV_PATH_NAME: &str = "Path"; #[cfg(windows)] @@ -52,6 +47,23 @@ pub fn convert_env_values(engine_state: &mut EngineState, stack: &Stack) -> Opti } } + #[cfg(not(windows))] + { + error = error.or_else(|| ensure_path(&mut new_scope, ENV_PATH_NAME)); + } + + #[cfg(windows)] + { + let first_result = ensure_path(&mut new_scope, ENV_PATH_NAME); + if first_result.is_some() { + let second_result = ensure_path(&mut new_scope, ENV_PATH_NAME_SECONDARY); + + if second_result.is_some() { + error = error.or(first_result); + } + } + } + for (k, v) in new_scope { engine_state.env_vars.insert(k, v); } @@ -60,18 +72,51 @@ pub fn convert_env_values(engine_state: &mut EngineState, stack: &Stack) -> Opti } /// Translate one environment variable from Value to String +/// +/// Returns Ok(None) if the env var is not pub fn env_to_string( env_name: &str, - value: Value, + value: &Value, engine_state: &EngineState, stack: &Stack, - config: &Config, ) -> Result { - match get_converted_value(engine_state, stack, env_name, &value, "to_string") { + match get_converted_value(engine_state, stack, env_name, value, "to_string") { ConversionResult::Ok(v) => Ok(v.as_string()?), ConversionResult::ConversionError(e) => Err(e), ConversionResult::GeneralError(e) => Err(e), - ConversionResult::CellPathError => Ok(value.into_string(ENV_SEP, config)), + ConversionResult::CellPathError => match value.as_string() { + Ok(s) => Ok(s), + Err(_) => { + if env_name == ENV_PATH_NAME { + // Try to convert PATH/Path list to a string + match value { + Value::List { vals, .. } => { + let paths = vals + .iter() + .map(|v| v.as_string()) + .collect::, _>>()?; + + match std::env::join_paths(paths) { + Ok(p) => Ok(p.to_string_lossy().to_string()), + Err(_) => Err(ShellError::EnvVarNotAString( + env_name.to_string(), + value.span()?, + )), + } + } + _ => Err(ShellError::EnvVarNotAString( + env_name.to_string(), + value.span()?, + )), + } + } else { + Err(ShellError::EnvVarNotAString( + env_name.to_string(), + value.span()?, + )) + } + } + }, } } @@ -79,13 +124,17 @@ pub fn env_to_string( pub fn env_to_strings( engine_state: &EngineState, stack: &Stack, - config: &Config, ) -> Result, ShellError> { let env_vars = stack.get_env_vars(engine_state); let mut env_vars_str = HashMap::new(); for (env_name, val) in env_vars { - let val_str = env_to_string(&env_name, val, engine_state, stack, config)?; - env_vars_str.insert(env_name, val_str); + match env_to_string(&env_name, &val, engine_state, stack) { + Ok(val_str) => { + env_vars_str.insert(env_name, val_str); + } + Err(ShellError::EnvVarNotAString(..)) => {} // ignore non-string values + Err(e) => return Err(e), + } } Ok(env_vars_str) @@ -93,16 +142,16 @@ pub fn env_to_strings( /// Shorthand for env_to_string() for PWD with custom error pub fn current_dir_str(engine_state: &EngineState, stack: &Stack) -> Result { - let config = stack.get_config()?; if let Some(pwd) = stack.get_env_var(engine_state, "PWD") { - match env_to_string("PWD", pwd, engine_state, stack, &config) { + match env_to_string("PWD", &pwd, engine_state, stack) { Ok(cwd) => { if Path::new(&cwd).is_absolute() { Ok(cwd) } else { - Err(ShellError::LabeledError( + Err(ShellError::SpannedLabeledError( "Invalid current directory".to_string(), - format!("The 'PWD' environment variable must be set to an absolute path. Found: '{}'", cwd) + format!("The 'PWD' environment variable must be set to an absolute path. Found: '{}'", cwd), + pwd.span()? )) } } @@ -149,21 +198,7 @@ pub fn path_str( } }?; - if let Value::String { val, .. } = pathval { - Ok(val) - } else { - match get_converted_value(engine_state, stack, pathname, &pathval, "to_string") { - ConversionResult::Ok(v) => Ok(v.as_string()?), - ConversionResult::ConversionError(e) => Err(e), - ConversionResult::GeneralError(e) => Err(e), - ConversionResult::CellPathError => Err(ShellError::SpannedLabeledErrorHelp( - format!("Missing environment conversion of {} to string", pathname), - "Could not convert to string".to_string(), - pathval.span()?, - "The 'to_string' field of ENV_CONVERSIONS environment variable must be set up correctly".to_string() - )) - } - } + env_to_string(pathname, &pathval, engine_state, stack) } fn get_converted_value( @@ -226,7 +261,6 @@ fn get_converted_value( Err(e) => ConversionResult::ConversionError(e), } } else { - // This one is OK to fail: We want to know if custom conversion is working ConversionResult::ConversionError(ShellError::MissingParameter( "block input".into(), from_span, @@ -239,3 +273,56 @@ fn get_converted_value( ConversionResult::CellPathError } } + +fn ensure_path(scope: &mut HashMap, env_path_name: &str) -> Option { + let mut error = None; + + // If PATH/Path is still a string, force-convert it to a list + match scope.get(env_path_name) { + Some(Value::String { val, span }) => { + // Force-split path into a list + let span = *span; + let paths = std::env::split_paths(val) + .map(|p| Value::String { + val: p.to_string_lossy().to_string(), + span, + }) + .collect(); + + scope.insert(env_path_name.to_string(), Value::List { vals: paths, span }); + } + Some(Value::List { vals, span }) => { + // Must be a list of strings + if !vals.iter().all(|v| matches!(v, Value::String { .. })) { + error = error.or_else(|| { + Some(ShellError::SpannedLabeledError( + format!("Wrong {} environment variable value", env_path_name), + format!("{} must be a list of strings", env_path_name), + *span, + )) + }); + } + } + Some(val) => { + // All other values are errors + let span = match val.span() { + Ok(sp) => sp, + Err(e) => { + error = error.or(Some(e)); + Span::test_data() // FIXME: any better span to use here? + } + }; + + error = error.or_else(|| { + Some(ShellError::SpannedLabeledError( + format!("Wrong {} environment variable value", env_path_name), + format!("{} must be a list of strings", env_path_name), + span, + )) + }); + } + None => { /* not preset, do nothing */ } + } + + error +} diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 25d8ee1d53..d900a4cd34 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -112,6 +112,17 @@ pub enum ShellError { #[diagnostic(code(nu::shell::cant_convert), url(docsrs))] CantConvert(String, String, #[label("can't convert {1} to {0}")] Span), + #[error("{0} is not representable as a string.")] + #[diagnostic( + code(nu::shell::env_var_not_a_string), + url(docsrs), + help( + r#"The '{0}' environment variable must be a string or be convertible to a string. +Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVERSIONS."# + ) + )] + EnvVarNotAString(String, #[label("value not representable as a string")] Span), + #[error("Division by zero.")] #[diagnostic(code(nu::shell::division_by_zero), url(docsrs))] DivisionByZero(#[label("division by zero")] Span), diff --git a/docs/sample_config/default_config.nu b/docs/sample_config/default_config.nu index b75c02996a..1e3a10a7e5 100644 --- a/docs/sample_config/default_config.nu +++ b/docs/sample_config/default_config.nu @@ -28,6 +28,7 @@ let-env PROMPT_MULTILINE_INDICATOR = "::: " # Specifies how environment variables are: # - converted from a string to a value on Nushell startup (from_string) # - converted from a value back to a string when running extrnal commands (to_string) +# Note: The conversions happen *after* config.nu is loaded let-env ENV_CONVERSIONS = { "PATH": { from_string: { |s| $s | split row (char esep) } diff --git a/src/tests/test_env.rs b/src/tests/test_env.rs index 5604ee7e99..23f2aaf42d 100644 --- a/src/tests/test_env.rs +++ b/src/tests/test_env.rs @@ -14,3 +14,11 @@ fn shorthand_env_2() -> TestResult { fn shorthand_env_3() -> TestResult { run_test(r#"FOO=BAZ BAR=MOO $env.FOO"#, "BAZ") } + +#[test] +fn convert_non_string_env_var_to_nothing() -> TestResult { + run_test( + r#"let-env FOO = true; env | where name == FOO | get raw.0 | describe"#, + "nothing", + ) +}