Add --env and --wrapped flags to def (#10566)

crates/nu-cmd-lang/src/core_commands/def.rs

@@ -19,9 +19,11 @@ impl Command for Def {
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("def")
             .input_output_types(vec![(Type::Nothing, Type::Nothing)])
-            .required("def_name", SyntaxShape::String, "definition name")
+            .required("def_name", SyntaxShape::String, "command name")
             .required("params", SyntaxShape::Signature, "parameters")
-            .required("body", SyntaxShape::Closure(None), "body of the definition")
+            .required("block", SyntaxShape::Closure(None), "body of the definition")
+            .switch("env", "keep the environment defined inside the command", None)
+            .switch("wrapped", "treat unknown flags and arguments as strings (requires ...rest-like parameter in signature)", None)
             .category(Category::Core)
     }
 
@@ -56,6 +58,16 @@ impl Command for Def {
                 example: r#"def say-sth [sth: string] { echo $sth }; say-sth hi"#,
                 result: Some(Value::test_string("hi")),
             },
+            Example {
+                description: "Set environment variable by call a custom command",
+                example: r#"def --env foo [] { $env.BAR = "BAZ" }; foo; $env.BAR"#,
+                result: Some(Value::test_string("BAZ")),
+            },
+            Example {
+                description: "Define a custom wrapper for an external command",
+                example: r#"def --wrapped my-echo [...rest] { echo $rest }; my-echo spam"#,
+                result: Some(Value::test_list(vec![Value::test_string("spam")])),
+            },
         ]
     }
 }

crates/nu-cmd-lang/src/core_commands/export_def.rs

@@ -19,9 +19,11 @@ impl Command for ExportDef {
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("export def")
             .input_output_types(vec![(Type::Nothing, Type::Nothing)])
-            .required("name", SyntaxShape::String, "definition name")
+            .required("def_name", SyntaxShape::String, "command name")
             .required("params", SyntaxShape::Signature, "parameters")
             .required("block", SyntaxShape::Block, "body of the definition")
+            .switch("env", "keep the environment defined inside the command", None)
+            .switch("wrapped", "treat unknown flags and arguments as strings (requires ...rest-like parameter in signature)", None)
             .category(Category::Core)
     }
 

crates/nu-cmd-lang/src/core_commands/module.rs

@@ -62,7 +62,7 @@ impl Command for Module {
             },
             Example {
                 description: "Define a custom command that participates in the environment in a module and call it",
-                example: r#"module foo { export def-env bar [] { $env.FOO_BAR = "BAZ" } }; use foo bar; bar; $env.FOO_BAR"#,
+                example: r#"module foo { export def --env bar [] { $env.FOO_BAR = "BAZ" } }; use foo bar; bar; $env.FOO_BAR"#,
                 result: Some(Value::test_string("BAZ")),
             },
         ]

crates/nu-cmd-lang/src/core_commands/use_.rs

@@ -155,7 +155,7 @@ This command is a parser keyword. For details, check:
             },
             Example {
                 description: "Define a custom command that participates in the environment in a module and call it",
-                example: r#"module foo { export def-env bar [] { $env.FOO_BAR = "BAZ" } }; use foo bar; bar; $env.FOO_BAR"#,
+                example: r#"module foo { export def --env bar [] { $env.FOO_BAR = "BAZ" } }; use foo bar; bar; $env.FOO_BAR"#,
                 result: Some(Value::test_string("BAZ")),
             },
             Example {

crates/nu-command/src/debug/view_source.rs

@@ -163,7 +163,7 @@ impl Command for ViewSource {
             },
             Example {
                 description: "View the source of a custom command, which participates in the caller environment",
-                example: r#"def-env foo [] { $env.BAR = 'BAZ' }; view source foo"#,
+                example: r#"def --env foo [] { $env.BAR = 'BAZ' }; view source foo"#,
                 result: Some(Value::test_string("def foo [] { $env.BAR = 'BAZ' }")),
             },
             Example {

crates/nu-command/tests/commands/def.rs

@@ -46,7 +46,7 @@ fn def_errors_with_no_space_between_params_and_name_1() {
 
 #[test]
 fn def_errors_with_no_space_between_params_and_name_2() {
-    let actual = nu!("def-env test-command() {}");
+    let actual = nu!("def --env test-command() {}");
 
     assert!(actual.err.contains("expected space"));
 }
@@ -156,15 +156,6 @@ fn def_with_paren_params() {
     assert_eq!(actual.out, "3");
 }
 
-#[test]
-fn extern_with_block() {
-    let actual = nu!(
-        "extern-wrapped foo [...rest] { print ($rest | str join ',' ) }; foo --bar baz -- -q -u -x"
-    );
-
-    assert_eq!(actual.out, "--bar,baz,--,-q,-u,-x");
-}
-
 #[test]
 fn def_default_value_shouldnt_restrict_explicit_type() {
     let actual = nu!("def foo [x: any = null] { $x }; foo 1");
@@ -193,3 +184,118 @@ fn def_boolean_flags() {
     let actual = nu!("def foo [--x: bool] { $x == null }; foo");
     assert_eq!(actual.out, "true");
 }
+
+#[test]
+fn def_wrapped_with_block() {
+    let actual = nu!(
+        "def --wrapped foo [...rest] { print ($rest | str join ',' ) }; foo --bar baz -- -q -u -x"
+    );
+
+    assert_eq!(actual.out, "--bar,baz,--,-q,-u,-x");
+}
+
+#[test]
+fn def_wrapped_from_module() {
+    let actual = nu!(r#"module spam {
+            export def --wrapped my-echo [...rest] { ^echo $rest }
+        }
+
+        use spam
+        spam my-echo foo -b -as -9 --abc -- -Dxmy=AKOO - bar
+        "#);
+
+    assert!(actual
+        .out
+        .contains("foo -b -as -9 --abc -- -Dxmy=AKOO - bar"));
+}
+
+#[test]
+fn def_cursed_env_flag_positions() {
+    let actual = nu!("def spam --env [] { $env.SPAM = 'spam' }; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+
+    let actual =
+        nu!("def spam --env []: nothing -> nothing { $env.SPAM = 'spam' }; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+}
+
+#[test]
+#[ignore = "TODO: Investigate why it's not working, it might be the signature parsing"]
+fn def_cursed_env_flag_positions_2() {
+    let actual = nu!("def spam [] --env { $env.SPAM = 'spam' }; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+
+    let actual = nu!("def spam [] { $env.SPAM = 'spam' } --env; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+
+    let actual =
+        nu!("def spam []: nothing -> nothing { $env.SPAM = 'spam' } --env; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+}
+
+#[test]
+fn export_def_cursed_env_flag_positions() {
+    let actual = nu!("export def spam --env [] { $env.SPAM = 'spam' }; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+
+    let actual =
+        nu!("export def spam --env []: nothing -> nothing { $env.SPAM = 'spam' }; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+}
+
+#[test]
+#[ignore = "TODO: Investigate why it's not working, it might be the signature parsing"]
+fn export_def_cursed_env_flag_positions_2() {
+    let actual = nu!("export def spam [] --env { $env.SPAM = 'spam' }; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+
+    let actual = nu!("export def spam [] { $env.SPAM = 'spam' } --env; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+
+    let actual =
+        nu!("export def spam []: nothing -> nothing { $env.SPAM = 'spam' } --env; spam; $env.SPAM");
+    assert_eq!(actual.out, "spam");
+}
+
+#[test]
+fn def_cursed_wrapped_flag_positions() {
+    let actual = nu!("def spam --wrapped [...rest] { $rest.0 }; spam --foo");
+    assert_eq!(actual.out, "--foo");
+
+    let actual = nu!("def spam --wrapped [...rest]: nothing -> nothing { $rest.0 }; spam --foo");
+    assert_eq!(actual.out, "--foo");
+}
+
+#[test]
+#[ignore = "TODO: Investigate why it's not working, it might be the signature parsing"]
+fn def_cursed_wrapped_flag_positions_2() {
+    let actual = nu!("def spam [...rest] --wrapped { $rest.0 }; spam --foo");
+    assert_eq!(actual.out, "--foo");
+
+    let actual = nu!("def spam [...rest] { $rest.0 } --wrapped; spam --foo");
+    assert_eq!(actual.out, "--foo");
+
+    let actual = nu!("def spam [...rest]: nothing -> nothing { $rest.0 } --wrapped; spam --foo");
+    assert_eq!(actual.out, "--foo");
+}
+
+#[test]
+fn def_wrapped_missing_rest_error() {
+    let actual = nu!("def --wrapped spam [] {}");
+    assert!(actual.err.contains("missing_positional"))
+}
+
+#[test]
+fn def_wrapped_wrong_rest_type_error() {
+    let actual = nu!("def --wrapped spam [...eggs: list<string>] { $eggs }");
+    assert!(actual.err.contains("type_mismatch_help"));
+    assert!(actual.err.contains("of ...eggs to 'string'"));
+}
+
+#[test]
+fn def_env_wrapped() {
+    let actual = nu!(
+        "def --env --wrapped spam [...eggs: string] { $env.SPAM = $eggs.0 }; spam bacon; $env.SPAM"
+    );
+    assert_eq!(actual.out, "bacon");
+}

crates/nu-command/tests/commands/use_.rs

@@ -259,7 +259,7 @@ fn use_main_4() {
 #[test]
 fn use_main_def_env() {
     let inp = &[
-        r#"module spam { export def-env main [] { $env.SPAM = "spam" } }"#,
+        r#"module spam { export def --env main [] { $env.SPAM = "spam" } }"#,
         r#"use spam"#,
         r#"spam"#,
         r#"$env.SPAM"#,

crates/nu-parser/src/parse_keywords.rs

@@ -13,7 +13,7 @@ use nu_protocol::{
     },
     engine::{StateWorkingSet, DEFAULT_OVERLAY_NAME},
     eval_const::{eval_constant, value_as_string},
-    span, Alias, BlockId, Exportable, Module, ModuleId, ParseError, PositionalArg,
+    span, Alias, BlockId, DeclId, Exportable, Module, ModuleId, ParseError, PositionalArg,
     ResolvedImportPattern, Span, Spanned, SyntaxShape, Type, VarId,
 };
 use std::collections::{HashMap, HashSet};
@@ -134,97 +134,118 @@ pub fn parse_keyword(
 }
 
 pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) {
-    let name = working_set.get_span_contents(spans[0]);
+    let mut pos = 0;
 
-    // handle "export def" same as "def"
-    let (decl_name, spans) = if name == b"export" && spans.len() >= 2 {
-        (working_set.get_span_contents(spans[1]), &spans[1..])
+    let def_type_name = if spans.len() >= 3 {
+        // definition can't have only two spans, minimum is 3, e.g., 'extern spam []'
+        let first_word = working_set.get_span_contents(spans[0]);
+
+        if first_word == b"export" {
+            pos += 2;
+        } else {
+            pos += 1;
+        }
+
+        working_set.get_span_contents(spans[pos - 1]).to_vec()
     } else {
-        (name, spans)
+        return;
     };
 
-    if (decl_name == b"def" || decl_name == b"def-env") && spans.len() >= 4 {
-        let starting_error_count = working_set.parse_errors.len();
-        let name = if let Some(err) = detect_params_in_name(
-            working_set,
-            spans[1],
-            String::from_utf8_lossy(decl_name).as_ref(),
-        ) {
-            working_set.error(err);
-            return;
-        } else {
-            working_set.get_span_contents(spans[1])
-        };
-        let name = trim_quotes(name);
-        let name = String::from_utf8_lossy(name).to_string();
+    if def_type_name != b"def"
+        && def_type_name != b"def-env"
+        && def_type_name != b"extern"
+        && def_type_name != b"extern-wrapped"
+    {
+        return;
+    }
 
-        working_set.enter_scope();
-        // FIXME: because parse_signature will update the scope with the variables it sees
-        // we end up parsing the signature twice per def. The first time is during the predecl
-        // so that we can see the types that are part of the signature, which we need for parsing.
-        // The second time is when we actually parse the body itworking_set.
-        // We can't reuse the first time because the variables that are created during parse_signature
-        // are lost when we exit the scope below.
-        let sig = parse_signature(working_set, spans[2]);
-        working_set.parse_errors.truncate(starting_error_count);
+    // Now, pos should point at the next span after the def-like call.
+    // Skip all potential flags, like --env, --wrapped or --help:
+    while pos < spans.len()
+        && working_set
+            .get_span_contents(spans[pos])
+            .starts_with(&[b'-'])
+    {
+        pos += 1;
+    }
 
-        let signature = sig.as_signature();
-        working_set.exit_scope();
-        if name.contains('#')
-            || name.contains('^')
-            || name.parse::<bytesize::ByteSize>().is_ok()
-            || name.parse::<f64>().is_ok()
+    if pos >= spans.len() {
+        // This can happen if the call ends with a flag, e.g., 'def --help'
+        return;
+    }
+
+    // Now, pos should point at the command name.
+    let name_pos = pos;
+
+    let Some(name) = parse_string(working_set, spans[name_pos]).as_string() else {
+        return;
+    };
+
+    if name.contains('#')
+        || name.contains('^')
+        || name.parse::<bytesize::ByteSize>().is_ok()
+        || name.parse::<f64>().is_ok()
+    {
+        working_set.error(ParseError::CommandDefNotValid(spans[name_pos]));
+        return;
+    }
+
+    // Find signature
+    let mut signature_pos = None;
+
+    while pos < spans.len() {
+        if working_set
+            .get_span_contents(spans[pos])
+            .starts_with(&[b'['])
+            || working_set
+                .get_span_contents(spans[pos])
+                .starts_with(&[b'('])
         {
-            working_set.error(ParseError::CommandDefNotValid(spans[1]));
-            return;
+            signature_pos = Some(pos);
         }
 
-        if let Some(mut signature) = signature {
-            signature.name = name;
-            let decl = signature.predeclare();
+        pos += 1;
+    }
 
-            if working_set.add_predecl(decl).is_some() {
-                working_set.error(ParseError::DuplicateCommandDef(spans[1]));
-            }
+    let Some(signature_pos) = signature_pos else {
+        return;
+    };
+
+    let mut allow_unknown_args = false;
+
+    for span in spans {
+        if working_set.get_span_contents(*span) == b"--wrapped" && def_type_name == b"def" {
+            allow_unknown_args = true;
         }
-    } else if (decl_name == b"extern" || decl_name == b"extern-wrapped") && spans.len() >= 3 {
-        let name_expr = parse_string(working_set, spans[1]);
-        let name = name_expr.as_string();
+    }
 
-        working_set.enter_scope();
-        // FIXME: because parse_signature will update the scope with the variables it sees
-        // we end up parsing the signature twice per def. The first time is during the predecl
-        // so that we can see the types that are part of the signature, which we need for parsing.
-        // The second time is when we actually parse the body itworking_set.
-        // We can't reuse the first time because the variables that are created during parse_signature
-        // are lost when we exit the scope below.
-        let sig = parse_signature(working_set, spans[2]);
-        let signature = sig.as_signature();
-        working_set.exit_scope();
+    let starting_error_count = working_set.parse_errors.len();
 
-        if let (Some(name), Some(mut signature)) = (name, signature) {
-            if name.contains('#')
-                || name.parse::<bytesize::ByteSize>().is_ok()
-                || name.parse::<f64>().is_ok()
-            {
-                working_set.error(ParseError::CommandDefNotValid(spans[1]));
-                return;
-            }
+    working_set.enter_scope();
+    // FIXME: because parse_signature will update the scope with the variables it sees
+    // we end up parsing the signature twice per def. The first time is during the predecl
+    // so that we can see the types that are part of the signature, which we need for parsing.
+    // The second time is when we actually parse the body itworking_set.
+    // We can't reuse the first time because the variables that are created during parse_signature
+    // are lost when we exit the scope below.
+    let sig = parse_signature(working_set, spans[signature_pos]);
+    working_set.parse_errors.truncate(starting_error_count);
+    working_set.exit_scope();
 
-            signature.name = name.clone();
+    let Some(mut signature) = sig.as_signature() else {
+        return;
+    };
 
-            let decl = KnownExternal {
-                name,
-                usage: "run external command".into(),
-                extra_usage: "".into(),
-                signature,
-            };
+    signature.name = name;
 
-            if working_set.add_predecl(Box::new(decl)).is_some() {
-                working_set.error(ParseError::DuplicateCommandDef(spans[1]));
-                return;
-            }
-        }
+    if allow_unknown_args {
+        signature.allows_unknown_args = true;
+    }
+
+    let decl = signature.predeclare();
+
+    if working_set.add_predecl(decl).is_some() {
+        working_set.error(ParseError::DuplicateCommandDef(spans[name_pos]));
     }
 }
 
@@ -334,11 +355,12 @@ pub fn parse_for(working_set: &mut StateWorkingSet, spans: &[Span]) -> Expressio
     }
 }
 
+// Returns also the parsed command name and ID
 pub fn parse_def(
     working_set: &mut StateWorkingSet,
     lite_command: &LiteCommand,
     module_name: Option<&[u8]>,
-) -> Pipeline {
+) -> (Pipeline, Option<(Vec<u8>, DeclId)>) {
     let spans = &lite_command.parts[..];
 
     let (usage, extra_usage) = working_set.build_usage(&lite_command.comments);
@@ -360,7 +382,7 @@ pub fn parse_def(
             "internal error: Wrong call name for def function".into(),
             span(spans),
         ));
-        return garbage_pipeline(spans);
+        return (garbage_pipeline(spans), None);
     }
 
     // Parsing the spans and checking that they match the register signature
@@ -372,20 +394,31 @@ pub fn parse_def(
                 "internal error: def declaration not found".into(),
                 span(spans),
             ));
-            return garbage_pipeline(spans);
+            return (garbage_pipeline(spans), None);
         }
         Some(decl_id) => {
             working_set.enter_scope();
             let (command_spans, rest_spans) = spans.split_at(split_id);
 
-            if let Some(name_span) = rest_spans.get(0) {
+            // Find the first span that is not a flag
+            let mut decl_name_span = None;
+
+            for span in rest_spans {
+                if !working_set.get_span_contents(*span).starts_with(&[b'-']) {
+                    decl_name_span = Some(*span);
+                    break;
+                }
+            }
+
+            if let Some(name_span) = decl_name_span {
+                // Check whether name contains [] or () -- possible missing space error
                 if let Some(err) = detect_params_in_name(
                     working_set,
-                    *name_span,
+                    name_span,
                     String::from_utf8_lossy(&def_call).as_ref(),
                 ) {
                     working_set.error(err);
-                    return garbage_pipeline(spans);
+                    return (garbage_pipeline(spans), None);
                 }
             }
 
@@ -429,18 +462,24 @@ pub fn parse_def(
             check_call(working_set, call_span, &sig, &call);
             working_set.parse_errors.append(&mut new_errors);
             if starting_error_count != working_set.parse_errors.len() || call.has_flag("help") {
-                return Pipeline::from_vec(vec![Expression {
-                    expr: Expr::Call(call),
-                    span: call_span,
-                    ty: output,
-                    custom_completion: None,
-                }]);
+                return (
+                    Pipeline::from_vec(vec![Expression {
+                        expr: Expr::Call(call),
+                        span: call_span,
+                        ty: output,
+                        custom_completion: None,
+                    }]),
+                    None,
+                );
             }
 
             (call, call_span)
         }
     };
 
+    let has_env = call.has_flag("env");
+    let has_wrapped = call.has_flag("wrapped");
+
     // All positional arguments must be in the call positional vector by this point
     let name_expr = call.positional_nth(0).expect("def call already checked");
     let sig = call.positional_nth(1).expect("def call already checked");
@@ -457,12 +496,15 @@ pub fn parse_def(
                     "main".to_string(),
                     name_expr_span,
                 ));
-                return Pipeline::from_vec(vec![Expression {
-                    expr: Expr::Call(call),
-                    span: call_span,
-                    ty: Type::Any,
-                    custom_completion: None,
-                }]);
+                return (
+                    Pipeline::from_vec(vec![Expression {
+                        expr: Expr::Call(call),
+                        span: call_span,
+                        ty: Type::Any,
+                        custom_completion: None,
+                    }]),
+                    None,
+                );
             }
         }
 
@@ -472,10 +514,51 @@ pub fn parse_def(
             "Could not get string from string expression".into(),
             name_expr.span,
         ));
-        return garbage_pipeline(spans);
+        return (garbage_pipeline(spans), None);
     };
 
+    let mut result = None;
+
     if let (Some(mut signature), Some(block_id)) = (sig.as_signature(), block.as_block()) {
+        if has_wrapped {
+            if let Some(rest) = &signature.rest_positional {
+                if let Some(var_id) = rest.var_id {
+                    let rest_var = &working_set.get_variable(var_id);
+
+                    if rest_var.ty != Type::Any && rest_var.ty != Type::List(Box::new(Type::String))
+                    {
+                        working_set.error(ParseError::TypeMismatchHelp(
+                            Type::List(Box::new(Type::String)),
+                            rest_var.ty.clone(),
+                            rest_var.declaration_span,
+                            format!("...rest-like positional argument used in 'def --wrapped' supports only strings. Change the type annotation of ...{} to 'string'.", &rest.name)));
+
+                        return (
+                            Pipeline::from_vec(vec![Expression {
+                                expr: Expr::Call(call),
+                                span: call_span,
+                                ty: Type::Any,
+                                custom_completion: None,
+                            }]),
+                            result,
+                        );
+                    }
+                }
+            } else {
+                working_set.error(ParseError::MissingPositional("...rest-like positional argument".to_string(), name_expr.span, "def --wrapped must have a ...rest-like positional argument. Add '...rest: string' to the command's signature.".to_string()));
+
+                return (
+                    Pipeline::from_vec(vec![Expression {
+                        expr: Expr::Call(call),
+                        span: call_span,
+                        ty: Type::Any,
+                        custom_completion: None,
+                    }]),
+                    result,
+                );
+            }
+        }
+
         if let Some(decl_id) = working_set.find_predecl(name.as_bytes()) {
             let declaration = working_set.get_decl_mut(decl_id);
 
@@ -483,6 +566,7 @@ pub fn parse_def(
             *signature = signature.add_help();
             signature.usage = usage;
             signature.extra_usage = extra_usage;
+            signature.allows_unknown_args = has_wrapped;
 
             *declaration = signature.clone().into_block_command(block_id);
 
@@ -490,7 +574,7 @@ pub fn parse_def(
             let calls_itself = block_calls_itself(block, decl_id);
             block.recursive = Some(calls_itself);
             block.signature = signature;
-            block.redirect_env = def_call == b"def-env";
+            block.redirect_env = def_call == b"def-env" || has_env;
 
             if block.signature.input_output_types.is_empty() {
                 block
@@ -506,6 +590,8 @@ pub fn parse_def(
             working_set
                 .parse_errors
                 .extend_from_slice(&typecheck_errors);
+
+            result = Some((name.as_bytes().to_vec(), decl_id));
         } else {
             working_set.error(ParseError::InternalError(
                 "Predeclaration failed to add declaration".into(),
@@ -517,12 +603,15 @@ pub fn parse_def(
     // It's OK if it returns None: The decl was already merged in previous parse pass.
     working_set.merge_predecl(name.as_bytes());
 
-    Pipeline::from_vec(vec![Expression {
-        expr: Expr::Call(call),
-        span: call_span,
-        ty: Type::Any,
-        custom_completion: None,
-    }])
+    (
+        Pipeline::from_vec(vec![Expression {
+            expr: Expr::Call(call),
+            span: call_span,
+            ty: Type::Any,
+            custom_completion: None,
+        }]),
+        result,
+    )
 }
 
 pub fn parse_extern(
@@ -1002,7 +1091,7 @@ pub fn parse_export_in_block(
 
     match full_name.as_slice() {
         b"export alias" => parse_alias(working_set, lite_command, None),
-        b"export def" | b"export def-env" => parse_def(working_set, lite_command, None),
+        b"export def" | b"export def-env" => parse_def(working_set, lite_command, None).0,
         b"export const" => parse_const(working_set, &lite_command.parts[1..]),
         b"export use" => {
             let (pipeline, _) = parse_use(working_set, &lite_command.parts);
@@ -1075,7 +1164,17 @@ pub fn parse_export_in_module(
                     comments: lite_command.comments.clone(),
                     parts: spans[1..].to_vec(),
                 };
-                let pipeline = parse_def(working_set, &lite_command, Some(module_name));
+                let (pipeline, cmd_result) =
+                    parse_def(working_set, &lite_command, Some(module_name));
+
+                let mut result = vec![];
+
+                if let Some((decl_name, decl_id)) = cmd_result {
+                    result.push(Exportable::Decl {
+                        name: decl_name.to_vec(),
+                        id: decl_id,
+                    });
+                }
 
                 let export_def_decl_id = if let Some(id) = working_set.find_decl(b"export def") {
                     id
@@ -1107,25 +1206,6 @@ pub fn parse_export_in_module(
                     ));
                 };
 
-                let mut result = vec![];
-
-                if let Some(decl_name_span) = spans.get(2) {
-                    let decl_name = working_set.get_span_contents(*decl_name_span);
-                    let decl_name = trim_quotes(decl_name);
-
-                    if let Some(decl_id) = working_set.find_decl(decl_name) {
-                        result.push(Exportable::Decl {
-                            name: decl_name.to_vec(),
-                            id: decl_id,
-                        });
-                    } else {
-                        working_set.error(ParseError::InternalError(
-                            "failed to find added declaration".into(),
-                            span(&spans[1..]),
-                        ));
-                    }
-                }
-
                 result
             }
             b"def-env" => {
@@ -1133,7 +1213,7 @@ pub fn parse_export_in_module(
                     comments: lite_command.comments.clone(),
                     parts: spans[1..].to_vec(),
                 };
-                let pipeline = parse_def(working_set, &lite_command, Some(module_name));
+                let (pipeline, _) = parse_def(working_set, &lite_command, Some(module_name));
 
                 let export_def_decl_id = if let Some(id) = working_set.find_decl(b"export def-env")
                 {
@@ -1648,11 +1728,14 @@ pub fn parse_module_block(
 
                     match name {
                         b"def" | b"def-env" => {
-                            block.pipelines.push(parse_def(
-                                working_set,
-                                command,
-                                None, // using commands named as the module locally is OK
-                            ))
+                            block.pipelines.push(
+                                parse_def(
+                                    working_set,
+                                    command,
+                                    None, // using commands named as the module locally is OK
+                                )
+                                .0,
+                            )
                         }
                         b"const" => block
                             .pipelines

crates/nu-parser/src/parser.rs

@@ -5337,7 +5337,7 @@ pub fn parse_builtin_commands(
     let name = working_set.get_span_contents(lite_command.parts[0]);
 
     match name {
-        b"def" | b"def-env" => parse_def(working_set, lite_command, None),
+        b"def" | b"def-env" => parse_def(working_set, lite_command, None).0,
         b"extern" | b"extern-wrapped" => parse_extern(working_set, lite_command, None),
         b"let" => parse_let(working_set, &lite_command.parts),
         b"const" => parse_const(working_set, &lite_command.parts),

crates/nu-protocol/src/parse_error.rs

@@ -358,6 +358,10 @@ pub enum ParseError {
     #[diagnostic(code(nu::parser::type_mismatch))]
     TypeMismatch(Type, Type, #[label("expected {0}, found {1}")] Span), // expected, found, span
 
+    #[error("Type mismatch.")]
+    #[diagnostic(code(nu::parser::type_mismatch_help), help("{3}"))]
+    TypeMismatchHelp(Type, Type, #[label("expected {0}, found {1}")] Span, String), // expected, found, span, help
+
     #[error("Missing required flag.")]
     #[diagnostic(code(nu::parser::missing_required_flag))]
     MissingRequiredFlag(String, #[label("missing required flag {0}")] Span),
@@ -532,6 +536,7 @@ impl ParseError {
             ParseError::KeywordMissingArgument(_, _, s) => *s,
             ParseError::MissingType(s) => *s,
             ParseError::TypeMismatch(_, _, s) => *s,
+            ParseError::TypeMismatchHelp(_, _, s, _) => *s,
             ParseError::InputMismatch(_, s) => *s,
             ParseError::OutputMismatch(_, s) => *s,
             ParseError::MissingRequiredFlag(_, s) => *s,

crates/nu-std/CONTRIBUTING.md

@@ -117,7 +117,7 @@ is described below:
       a main command as well.  
       Use `export def` to make these names public to your users.
     * If your command is updating environment variables, you must use
-      `export def-env` (instead of `export def`) to define the subcommand,
+      `export def --env` (instead of `export def`) to define the subcommand,
       `export-env {}` to initialize the environment variables and `let-env` to
       update them. For an example of a custom command which modifies
       environment variables, see: `./crates/nu-std/lib/dirs.nu`.  

crates/nu-std/std/dirs.nu

@@ -23,7 +23,7 @@ export-env {
 
 # Add one or more directories to the list.
 # PWD becomes first of the newly added directories.
-export def-env add [
+export def --env add [
     ...paths: string    # directory or directories to add to working list
     ] {
         mut abspaths = []
@@ -45,7 +45,7 @@ export def-env add [
 export alias enter = add
 
 # Advance to the next directory in the list or wrap to beginning.
-export def-env next [
+export def --env next [
     N:int = 1   # number of positions to move.
 ] {
     _fetch $N
@@ -54,7 +54,7 @@ export def-env next [
 export alias n = next
 
 # Back up to the previous directory or wrap to the end.
-export def-env prev [
+export def --env prev [
     N:int = 1   # number of positions to move.
 ] {
     _fetch (-1 * $N)
@@ -64,7 +64,7 @@ export alias p = prev
 
 # Drop the current directory from the list, if it's not the only one.
 # PWD becomes the next working directory
-export def-env drop [] {
+export def --env drop [] {
     if ($env.DIRS_LIST | length) > 1 {
         $env.DIRS_LIST = ($env.DIRS_LIST | reject $env.DIRS_POSITION)
         if ($env.DIRS_POSITION >= ($env.DIRS_LIST | length)) {$env.DIRS_POSITION = 0}
@@ -78,7 +78,7 @@ export def-env drop [] {
 export alias dexit = drop
 
 # Display current working directories.
-export def-env show [] {
+export def --env show [] {
     mut out = []
     for $p in ($env.DIRS_LIST | enumerate) {
         let is_act_slot = $p.index == $env.DIRS_POSITION
@@ -95,7 +95,7 @@ export def-env show [] {
 
 export alias shells = show
 
-export def-env goto [shell?: int] {
+export def --env goto [shell?: int] {
     if $shell == null {
         return (show)
     }
@@ -119,7 +119,7 @@ export def-env goto [shell?: int] {
 export alias g = goto
 
 # fetch item helper
-def-env _fetch [
+def --env _fetch [
     offset: int,        # signed change to position
     --forget_current    # true to skip saving PWD
     --always_cd         # true to always cd

crates/nu-std/std/mod.nu

@@ -34,7 +34,7 @@ use log.nu
 # ```nushell
 # >_ std path add {linux: "foo", windows: "bar", darwin: "baz"}
 # ```
-export def-env "path add" [
+export def --env "path add" [
     --ret (-r)  # return $env.PATH, useful in pipelines to avoid scoping.
     --append (-a)  # append to $env.PATH instead of prepending to.
     ...paths  # the paths to add to $env.PATH.

crates/nu-std/tests/test_dirs.nu

@@ -3,8 +3,8 @@ use std assert
 use std log
 
 # A couple of nuances to understand when testing module that exports environment:
-# Each 'use' for that module in the test script will execute the def-env block.
-# PWD at the time of the `use` will be what the export def-env block will see.
+# Each 'use' for that module in the test script will execute the def --env block.
+# PWD at the time of the `use` will be what the export def --env block will see.
 
 #[before-each]
 def before-each [] {
@@ -43,11 +43,11 @@ def dirs_command [] {
     # must capture value of $in before executing `use`s
     let $c = $in
 
-    # must set PWD *before* doing `use` that will run the def-env block in dirs module.
+    # must set PWD *before* doing `use` that will run the def --env block in dirs module.
     cd $c.base_path
 
     # must execute these uses for the UOT commands *after* the test and *not* just put them at top of test module.
-    # the def-env gets messed up
+    # the def --env gets messed up
     use std dirs
 
     # Stack: [BASE]
@@ -92,7 +92,7 @@ def dirs_command [] {
 def dirs_next [] {
     # must capture value of $in before executing `use`s
     let $c = $in
-    # must set PWD *before* doing `use` that will run the def-env block in dirs module.
+    # must set PWD *before* doing `use` that will run the def --env block in dirs module.
     cd $c.base_path
     assert equal $env.PWD $c.base_path "test setup"
 
@@ -114,7 +114,7 @@ def dirs_next [] {
 def dirs_cd [] {
     # must capture value of $in before executing `use`s
     let $c = $in
-    # must set PWD *before* doing `use` that will run the def-env block in dirs module.
+    # must set PWD *before* doing `use` that will run the def --env block in dirs module.
     cd $c.base_path
 
     use std dirs

src/tests/test_engine.rs

@@ -189,7 +189,7 @@ fn let_sees_in_variable2() -> TestResult {
 #[test]
 fn def_env() -> TestResult {
     run_test(
-        r#"def-env bob [] { $env.BAR = "BAZ" }; bob; $env.BAR"#,
+        r#"def --env bob [] { $env.BAR = "BAZ" }; bob; $env.BAR"#,
         "BAZ",
     )
 }
@@ -202,7 +202,7 @@ fn not_def_env() -> TestResult {
 #[test]
 fn def_env_hiding_something() -> TestResult {
     fail_test(
-        r#"$env.FOO = "foo"; def-env bob [] { hide-env FOO }; bob; $env.FOO"#,
+        r#"$env.FOO = "foo"; def --env bob [] { hide-env FOO }; bob; $env.FOO"#,
         "",
     )
 }
@@ -210,7 +210,7 @@ fn def_env_hiding_something() -> TestResult {
 #[test]
 fn def_env_then_hide() -> TestResult {
     fail_test(
-        r#"def-env bob [] { $env.BOB = "bob" }; def-env un-bob [] { hide-env BOB }; bob; un-bob; $env.BOB"#,
+        r#"def --env bob [] { $env.BOB = "bob" }; def --env un-bob [] { hide-env BOB }; bob; un-bob; $env.BOB"#,
         "",
     )
 }
@@ -218,7 +218,7 @@ fn def_env_then_hide() -> TestResult {
 #[test]
 fn export_def_env() -> TestResult {
     run_test(
-        r#"module foo { export def-env bob [] { $env.BAR = "BAZ" } }; use foo bob; bob; $env.BAR"#,
+        r#"module foo { export def --env bob [] { $env.BAR = "BAZ" } }; use foo bob; bob; $env.BAR"#,
         "BAZ",
     )
 }

src/tests/test_known_external.rs

@@ -53,20 +53,6 @@ fn known_external_from_module() -> TestResult {
     )
 }
 
-#[test]
-fn known_external_wrapped_from_module() -> TestResult {
-    run_test_contains(
-        r#"module spam {
-            export extern-wrapped my-echo [...rest] { ^echo $rest }
-        }
-
-        use spam
-        spam my-echo foo -b -as -9 --abc -- -Dxmy=AKOO - bar
-        "#,
-        "foo -b -as -9 --abc -- -Dxmy=AKOO - bar",
-    )
-}
-
 #[test]
 fn known_external_short_flag_batch_arg_allowed() -> TestResult {
     run_test_contains("extern echo [-a, -b: int]; echo -ab 10", "-b 10")

tests/overlays/mod.rs

@@ -1215,7 +1215,7 @@ fn overlay_use_main_prefix() {
 #[test]
 fn overlay_use_main_def_env() {
     let inp = &[
-        r#"module spam { export def-env main [] { $env.SPAM = "spam" } }"#,
+        r#"module spam { export def --env main [] { $env.SPAM = "spam" } }"#,
         "overlay use spam",
         "spam",
         "$env.SPAM",

tests/shell/mod.rs

@@ -207,7 +207,7 @@ fn run_script_that_looks_like_module() {
             "export use spam eggs",
             "export def foo [] { eggs }",
             "export alias bar = foo",
-            "export def-env baz [] { bar }",
+            "export def --env baz [] { bar }",
             "baz",
         ];
 

tests/shell/pipeline/commands/external.rs

@@ -129,15 +129,6 @@ fn command_not_found_error_shows_not_found_1() {
     assert!(actual.err.contains("'foo' was not found"));
 }
 
-#[test]
-fn command_not_found_error_shows_not_found_2() {
-    let actual = nu!(r#"
-            export extern-wrapped my-foo [...rest] { foo };
-            my-foo
-        "#);
-    assert!(actual.err.contains("did you mean"));
-}
-
 #[test]
 fn command_substitution_wont_output_extra_newline() {
     let actual = nu!(r#"

tests/shell/pipeline/commands/internal.rs

@@ -1118,3 +1118,12 @@ fn pipe_input_to_print() {
     assert_eq!(actual.out, "foo");
     assert!(actual.err.is_empty());
 }
+
+#[test]
+fn command_not_found_error_shows_not_found_2() {
+    let actual = nu!(r#"
+            export def --wrapped my-foo [...rest] { foo };
+            my-foo
+        "#);
+    assert!(actual.err.contains("did you mean"));
+}