diff --git a/crates/nu-command/src/env/with_env.rs b/crates/nu-command/src/env/with_env.rs index 91ccc78e4b..7eaf64cb54 100644 --- a/crates/nu-command/src/env/with_env.rs +++ b/crates/nu-command/src/env/with_env.rs @@ -41,31 +41,14 @@ impl Command for WithEnv { } fn examples(&self) -> Vec { - vec![ - Example { - description: "Set the MYENV environment variable", - example: r#"with-env [MYENV "my env value"] { $env.MYENV }"#, - result: Some(Value::test_string("my env value")), - }, - Example { - description: "Set by primitive value list", - example: r#"with-env [X Y W Z] { $env.X }"#, - result: Some(Value::test_string("Y")), - }, - Example { - description: "Set by single row table", - example: r#"with-env [[X W]; [Y Z]] { $env.W }"#, - result: Some(Value::test_string("Z")), - }, - Example { - description: "Set by key-value record", - example: r#"with-env {X: "Y", W: "Z"} { [$env.X $env.W] }"#, - result: Some(Value::list( - vec![Value::test_string("Y"), Value::test_string("Z")], - Span::test_data(), - )), - }, - ] + vec![Example { + description: "Set by key-value record", + example: r#"with-env {X: "Y", W: "Z"} { [$env.X $env.W] }"#, + result: Some(Value::list( + vec![Value::test_string("Y"), Value::test_string("Z")], + Span::test_data(), + )), + }] } } @@ -85,6 +68,16 @@ fn with_env( match &variable { Value::List { vals: table, .. } => { + nu_protocol::report_error_new( + engine_state, + &ShellError::GenericError { + error: "Deprecated argument type".into(), + msg: "providing the variables to `with-env` as a list or single row table has been deprecated".into(), + span: Some(variable.span()), + help: Some("use the record form instead".into()), + inner: vec![], + }, + ); if table.len() == 1 { // single row([[X W]; [Y Z]]) match &table[0] { @@ -95,7 +88,7 @@ fn with_env( } x => { return Err(ShellError::CantConvert { - to_type: "string list or single row".into(), + to_type: "record".into(), from_type: x.get_type().to_string(), span: call .positional_nth(1) @@ -111,7 +104,13 @@ fn with_env( if row.len() == 2 { env.insert(row[0].coerce_string()?, row[1].clone()); } - // TODO: else error? + if row.len() == 1 { + return Err(ShellError::IncorrectValue { + msg: format!("Missing value for $env.{}", row[0].coerce_string()?), + val_span: row[0].span(), + call_span: call.head, + }); + } } } } @@ -123,7 +122,7 @@ fn with_env( } x => { return Err(ShellError::CantConvert { - to_type: "string list or single row".into(), + to_type: "record".into(), from_type: x.get_type().to_string(), span: call .positional_nth(1) @@ -134,6 +133,16 @@ fn with_env( } }; + // TODO: factor list of prohibited env vars into common place + for prohibited in ["PWD", "FILE_PWD", "CURRENT_FILE"] { + if env.contains_key(prohibited) { + return Err(ShellError::AutomaticEnvVarSetManually { + envvar_name: prohibited.into(), + span: call.head, + }); + } + } + for (k, v) in env { stack.add_env_var(k, v); } diff --git a/crates/nu-command/tests/commands/let_.rs b/crates/nu-command/tests/commands/let_.rs index 94fb19315d..a9a6c4b3b1 100644 --- a/crates/nu-command/tests/commands/let_.rs +++ b/crates/nu-command/tests/commands/let_.rs @@ -63,7 +63,7 @@ fn let_pipeline_redirects_externals() { #[test] fn let_err_pipeline_redirects_externals() { let actual = nu!( - r#"let x = with-env [FOO "foo"] {nu --testbin echo_env_stderr FOO e>| str length}; $x"# + r#"let x = with-env { FOO: "foo" } {nu --testbin echo_env_stderr FOO e>| str length}; $x"# ); assert_eq!(actual.out, "3"); } @@ -71,7 +71,7 @@ fn let_err_pipeline_redirects_externals() { #[test] fn let_outerr_pipeline_redirects_externals() { let actual = nu!( - r#"let x = with-env [FOO "foo"] {nu --testbin echo_env_stderr FOO o+e>| str length}; $x"# + r#"let x = with-env { FOO: "foo" } {nu --testbin echo_env_stderr FOO o+e>| str length}; $x"# ); assert_eq!(actual.out, "3"); } diff --git a/crates/nu-command/tests/commands/reduce.rs b/crates/nu-command/tests/commands/reduce.rs index 2b6c48c2e5..cf58d093bb 100644 --- a/crates/nu-command/tests/commands/reduce.rs +++ b/crates/nu-command/tests/commands/reduce.rs @@ -90,7 +90,7 @@ fn folding_with_tables() { " echo [10 20 30 40] | reduce --fold [] { |it, acc| - with-env [value $it] { + with-env { value: $it } { echo $acc | append (10 * ($env.value | into int)) } } diff --git a/crates/nu-command/tests/commands/with_env.rs b/crates/nu-command/tests/commands/with_env.rs index d037b77115..08d8e85d0c 100644 --- a/crates/nu-command/tests/commands/with_env.rs +++ b/crates/nu-command/tests/commands/with_env.rs @@ -2,7 +2,7 @@ use nu_test_support::nu; #[test] fn with_env_extends_environment() { - let actual = nu!("with-env [FOO BARRRR] {echo $env} | get FOO"); + let actual = nu!("with-env { FOO: BARRRR } {echo $env} | get FOO"); assert_eq!(actual.out, "BARRRR"); } @@ -32,7 +32,7 @@ fn with_env_shorthand_trims_quotes() { fn with_env_and_shorthand_same_result() { let actual_shorthand = nu!("FOO='BARRRR' echo $env | get FOO"); - let actual_normal = nu!("with-env [FOO BARRRR] {echo $env} | get FOO"); + let actual_normal = nu!("with-env { FOO: BARRRR } {echo $env} | get FOO"); assert_eq!(actual_shorthand.out, actual_normal.out); } @@ -50,7 +50,7 @@ fn with_env_hides_variables_in_parent_scope() { let actual = nu!(r#" $env.FOO = "1" print $env.FOO - with-env [FOO null] { + with-env { FOO: null } { echo $env.FOO } print $env.FOO diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index a665c24805..6cf3a84b9b 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5164,9 +5164,8 @@ pub fn parse_expression(working_set: &mut StateWorkingSet, spans: &[Span]) -> Ex } }; - let with_env = working_set.find_decl(b"with-env"); - if !shorthand.is_empty() { + let with_env = working_set.find_decl(b"with-env"); if let Some(decl_id) = with_env { let mut block = Block::default(); let ty = output.ty.clone(); @@ -5176,13 +5175,12 @@ pub fn parse_expression(working_set: &mut StateWorkingSet, spans: &[Span]) -> Ex let mut env_vars = vec![]; for sh in shorthand { - env_vars.push(sh.0); - env_vars.push(sh.1); + env_vars.push(RecordItem::Pair(sh.0, sh.1)); } let arguments = vec![ Argument::Positional(Expression { - expr: Expr::List(env_vars), + expr: Expr::Record(env_vars), span: span(&spans[..pos]), ty: Type::Any, custom_completion: None, diff --git a/crates/nu-std/std/mod.nu b/crates/nu-std/std/mod.nu index be34bef661..9a58134b42 100644 --- a/crates/nu-std/std/mod.nu +++ b/crates/nu-std/std/mod.nu @@ -22,7 +22,7 @@ use dt.nu [datetime-diff, pretty-print-duration] # # Example # - adding some dummy paths to an empty PATH # ```nushell -# >_ with-env [PATH []] { +# >_ with-env { PATH: [] } { # std path add "foo" # std path add "bar" "baz" # std path add "fooo" --append diff --git a/crates/nu-std/tests/test_std.nu b/crates/nu-std/tests/test_std.nu index 45f11e5729..45633719a0 100644 --- a/crates/nu-std/tests/test_std.nu +++ b/crates/nu-std/tests/test_std.nu @@ -6,7 +6,7 @@ def path_add [] { let path_name = if "PATH" in $env { "PATH" } else { "Path" } - with-env [$path_name []] { + with-env {$path_name: []} { def get_path [] { $env | get $path_name } assert equal (get_path) [] diff --git a/src/tests/test_env.rs b/src/tests/test_env.rs index 5604ee7e99..2241839f85 100644 --- a/src/tests/test_env.rs +++ b/src/tests/test_env.rs @@ -1,4 +1,4 @@ -use crate::tests::{run_test, TestResult}; +use crate::tests::{fail_test, run_test, TestResult}; #[test] fn shorthand_env_1() -> TestResult { @@ -7,7 +7,7 @@ fn shorthand_env_1() -> TestResult { #[test] fn shorthand_env_2() -> TestResult { - run_test(r#"FOO=BAZ FOO=MOO $env.FOO"#, "MOO") + fail_test(r#"FOO=BAZ FOO=MOO $env.FOO"#, "defined_twice") } #[test] diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs index f9b9bfce6d..450309c0a6 100644 --- a/tests/shell/environment/env.rs +++ b/tests/shell/environment/env.rs @@ -121,7 +121,7 @@ fn load_env_pwd_env_var_fails() { #[test] fn passes_with_env_env_var_to_external_process() { let actual = nu!(" - with-env [FOO foo] {nu --testbin echo_env FOO} + with-env { FOO: foo } {nu --testbin echo_env FOO} "); assert_eq!(actual.out, "foo"); } diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 6cd11fd5e9..4b9427c32c 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -133,26 +133,27 @@ fn command_not_found_error_shows_not_found_1() { #[test] fn command_substitution_wont_output_extra_newline() { let actual = nu!(r#" - with-env [FOO "bar"] { echo $"prefix (nu --testbin echo_env FOO) suffix" } + with-env { FOO: "bar" } { echo $"prefix (nu --testbin echo_env FOO) suffix" } "#); assert_eq!(actual.out, "prefix bar suffix"); let actual = nu!(r#" - with-env [FOO "bar"] { (nu --testbin echo_env FOO) } + with-env { FOO: "bar" } { (nu --testbin echo_env FOO) } "#); assert_eq!(actual.out, "bar"); } #[test] fn basic_err_pipe_works() { - let actual = nu!(r#"with-env [FOO "bar"] { nu --testbin echo_env_stderr FOO e>| str length }"#); + let actual = + nu!(r#"with-env { FOO: "bar" } { nu --testbin echo_env_stderr FOO e>| str length }"#); assert_eq!(actual.out, "3"); } #[test] fn basic_outerr_pipe_works() { let actual = nu!( - r#"with-env [FOO "bar"] { nu --testbin echo_env_mixed out-err FOO FOO o+e>| str length }"# + r#"with-env { FOO: "bar" } { nu --testbin echo_env_mixed out-err FOO FOO o+e>| str length }"# ); assert_eq!(actual.out, "7"); } @@ -160,7 +161,7 @@ fn basic_outerr_pipe_works() { #[test] fn err_pipe_with_failed_external_works() { let actual = - nu!(r#"with-env [FOO "bar"] { nu --testbin echo_env_stderr_fail FOO e>| str length }"#); + nu!(r#"with-env { FOO: "bar" } { nu --testbin echo_env_stderr_fail FOO e>| str length }"#); assert_eq!(actual.out, "3"); }