From 0646f1118cfe7c8c3cee2a76aa3b02511e979bb2 Mon Sep 17 00:00:00 2001 From: Kangaxx-0 <85712372+Kangaxx-0@users.noreply.github.com> Date: Thu, 21 Jul 2022 16:56:57 -0700 Subject: [PATCH] Refactor external command (#6083) Co-authored-by: Frank --- crates/nu-command/src/core_commands/help.rs | 4 +- crates/nu-command/src/system/which_.rs | 2 +- crates/nu-engine/src/eval.rs | 4 +- crates/nu-parser/src/known_external.rs | 4 ++ crates/nu-parser/src/parser.rs | 42 ++++++++++++--------- crates/nu-protocol/src/engine/command.rs | 10 +++++ src/tests/test_known_external.rs | 18 ++++++++- 7 files changed, 60 insertions(+), 24 deletions(-) diff --git a/crates/nu-command/src/core_commands/help.rs b/crates/nu-command/src/core_commands/help.rs index 0c1816078..eddb83bdd 100644 --- a/crates/nu-command/src/core_commands/help.rs +++ b/crates/nu-command/src/core_commands/help.rs @@ -142,7 +142,7 @@ fn help( cols.push("is_custom".into()); vals.push(Value::Bool { - val: decl.get_block_id().is_some(), + val: decl.is_custom_command(), span: head, }); @@ -243,7 +243,7 @@ fn help( cols.push("is_custom".into()); vals.push(Value::Bool { - val: decl.get_block_id().is_some(), + val: decl.is_custom_command(), span: head, }); diff --git a/crates/nu-command/src/system/which_.rs b/crates/nu-command/src/system/which_.rs index fe09f8bf5..baddb0f8b 100644 --- a/crates/nu-command/src/system/which_.rs +++ b/crates/nu-command/src/system/which_.rs @@ -95,7 +95,7 @@ fn get_entry_in_aliases(engine_state: &EngineState, name: &str, span: Span) -> O fn get_entry_in_commands(engine_state: &EngineState, name: &str, span: Span) -> Option { if let Some(decl_id) = engine_state.find_decl(name.as_bytes(), &[]) { - let (msg, is_builtin) = if engine_state.get_decl(decl_id).get_block_id().is_some() { + let (msg, is_builtin) = if engine_state.get_decl(decl_id).is_custom_command() { ("Nushell custom command", false) } else { ("Nushell built-in command", true) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 266536136..f2c775ada 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -1121,7 +1121,7 @@ pub fn create_scope( cols.push("is_builtin".to_string()); // we can only be a is_builtin or is_custom, not both vals.push(Value::Bool { - val: decl.get_block_id().is_none(), + val: !decl.is_custom_command(), span, }); @@ -1139,7 +1139,7 @@ pub fn create_scope( cols.push("is_custom".to_string()); vals.push(Value::Bool { - val: decl.get_block_id().is_some(), + val: decl.is_custom_command(), span, }); diff --git a/crates/nu-parser/src/known_external.rs b/crates/nu-parser/src/known_external.rs index 70ac93f82..ea40daf4f 100644 --- a/crates/nu-parser/src/known_external.rs +++ b/crates/nu-parser/src/known_external.rs @@ -30,6 +30,10 @@ impl Command for KnownExternal { true } + fn is_builtin(&self) -> bool { + false + } + fn run( &self, engine_state: &EngineState, diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index d48d4083d..82bc4914e 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1079,26 +1079,32 @@ pub fn parse_call( } } - trace!("parsing: internal call"); + let decl = working_set.get_decl(decl_id); + if decl.is_builtin() { + trace!("parsing: internal call"); - // parse internal command - let parsed_call = parse_internal_call( - working_set, - span(&spans[cmd_start..pos]), - &spans[pos..], - decl_id, - expand_aliases_denylist, - ); + // parse internal command + let parsed_call = parse_internal_call( + working_set, + span(&spans[cmd_start..pos]), + &spans[pos..], + decl_id, + expand_aliases_denylist, + ); - ( - Expression { - expr: Expr::Call(parsed_call.call), - span: span(spans), - ty: parsed_call.output, - custom_completion: None, - }, - parsed_call.error, - ) + ( + Expression { + expr: Expr::Call(parsed_call.call), + span: span(spans), + ty: parsed_call.output, + custom_completion: None, + }, + parsed_call.error, + ) + } else { + trace!("parsing: `extern` custom command as external call"); + parse_external_call(working_set, spans, expand_aliases_denylist) + } } else { // We might be parsing left-unbounded range ("..10") let bytes = working_set.get_span_contents(spans[0]); diff --git a/crates/nu-protocol/src/engine/command.rs b/crates/nu-protocol/src/engine/command.rs index 56f4fad5f..7d9d0242e 100644 --- a/crates/nu-protocol/src/engine/command.rs +++ b/crates/nu-protocol/src/engine/command.rs @@ -37,6 +37,16 @@ pub trait Command: Send + Sync + CommandClone { false } + // This is an enhanced method to determine if a command is custom command or not + // since extern "foo" [] and def "foo" [] behaves differently + fn is_custom_command(&self) -> bool { + if self.get_block_id().is_some() { + true + } else { + self.is_known_external() + } + } + // Is a sub command fn is_sub(&self) -> bool { self.name().contains(' ') diff --git a/src/tests/test_known_external.rs b/src/tests/test_known_external.rs index 30e65ab8f..5556d0ba3 100644 --- a/src/tests/test_known_external.rs +++ b/src/tests/test_known_external.rs @@ -12,7 +12,23 @@ fn known_external_runs() -> TestResult { fn known_external_unknown_flag() -> TestResult { fail_test( r#"extern "cargo version" []; cargo version --no-such-flag"#, - "command doesn't have flag", + "Found argument '--no-such-flag' which wasn't expected, or isn't valid in this context", + ) +} + +#[test] +fn known_external_not_defined_flag() -> TestResult { + run_test_contains( + r#"extern "cargo version" []; cargo version --help"#, + "Show version information", + ) +} + +#[test] +fn known_external_is_custom() -> TestResult { + run_test_contains( + r#"extern "cargo version" []; help commands | where is_custom == true | get name"#, + "cargo version", ) }