From 46eb34b35d61ad572026f1d708b9250f0cdb40fe Mon Sep 17 00:00:00 2001 From: Clements <37786996+mjclements@users.noreply.github.com> Date: Sun, 29 May 2022 09:14:15 -0400 Subject: [PATCH] Differentiate internal signature from external signature w.r.t. help (#5667) * Differentiate internal signature from external signature w.r.t. help * Add in the --help flag to default externs in default config * Remove unusued build_extern Co-authored-by: mjclements --- crates/nu-parser/src/parse_keywords.rs | 2 +- crates/nu-protocol/src/signature.rs | 36 +++++++++++++--------- crates/nu-protocol/tests/test_signature.rs | 9 ++---- docs/sample_config/default_config.nu | 3 ++ src/tests/test_custom_commands.rs | 15 ++++++++- 5 files changed, 42 insertions(+), 23 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 71b0c009f9..34d28a239a 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -54,7 +54,6 @@ pub fn parse_def_predecl( let (sig, ..) = parse_signature(working_set, spans[2], expand_aliases_denylist); let signature = sig.as_signature(); working_set.exit_scope(); - if let (Some(name), Some(mut signature)) = (name, signature) { signature.name = name; let decl = signature.predeclare(); @@ -360,6 +359,7 @@ pub fn parse_def( let declaration = working_set.get_decl_mut(decl_id); signature.name = name.clone(); + *signature = signature.add_help(); signature.usage = usage; *declaration = signature.clone().into_block_command(block_id); diff --git a/crates/nu-protocol/src/signature.rs b/crates/nu-protocol/src/signature.rs index 49e14b0357..ba2061a842 100644 --- a/crates/nu-protocol/src/signature.rs +++ b/crates/nu-protocol/src/signature.rs @@ -124,6 +124,23 @@ impl Eq for Signature {} impl Signature { pub fn new(name: impl Into) -> Signature { + Signature { + name: name.into(), + usage: String::new(), + extra_usage: String::new(), + search_terms: vec![], + required_positional: vec![], + optional_positional: vec![], + rest_positional: None, + named: vec![], + is_filter: false, + creates_scope: false, + category: Category::Default, + } + } + + // Add a default help option to a signature + pub fn add_help(mut self) -> Signature { // default help flag let flag = Flag { long: "help".into(), @@ -134,24 +151,13 @@ impl Signature { var_id: None, default_value: None, }; - - Signature { - name: name.into(), - usage: String::new(), - extra_usage: String::new(), - search_terms: vec![], - required_positional: vec![], - optional_positional: vec![], - rest_positional: None, - named: vec![flag], - is_filter: false, - creates_scope: false, - category: Category::Default, - } + self.named.push(flag); + self } + // Build an internal signature with default help option pub fn build(name: impl Into) -> Signature { - Signature::new(name.into()) + Signature::new(name.into()).add_help() } /// Add a description to the signature diff --git a/crates/nu-protocol/tests/test_signature.rs b/crates/nu-protocol/tests/test_signature.rs index cf90d5bf6c..000abb0d00 100644 --- a/crates/nu-protocol/tests/test_signature.rs +++ b/crates/nu-protocol/tests/test_signature.rs @@ -31,13 +31,10 @@ fn test_signature_chained() { assert_eq!(signature.required_positional.len(), 1); assert_eq!(signature.optional_positional.len(), 1); - assert_eq!(signature.named.len(), 4); // The 3 above + help + assert_eq!(signature.named.len(), 3); assert!(signature.rest_positional.is_some()); - assert_eq!(signature.get_shorts(), vec!['h', 'r', 'n']); - assert_eq!( - signature.get_names(), - vec!["help", "req-named", "named", "switch"] - ); + assert_eq!(signature.get_shorts(), vec!['r', 'n']); + assert_eq!(signature.get_names(), vec!["req-named", "named", "switch"]); assert_eq!(signature.num_positionals(), 2); assert_eq!( diff --git a/docs/sample_config/default_config.nu b/docs/sample_config/default_config.nu index 502afc8f52..1036a8ff50 100644 --- a/docs/sample_config/default_config.nu +++ b/docs/sample_config/default_config.nu @@ -60,6 +60,7 @@ module completions { --no-show-forced-updates # Don't check if a branch is force-updated -4 # Use IPv4 addresses, ignore IPv6 addresses -6 # Use IPv6 addresses, ignore IPv4 addresses + --help # Display this help message ] # Check out git branches and files @@ -86,6 +87,7 @@ module completions { -b: string # create and checkout a new branch -B: string # create/reset and checkout a branch -l # create reflog for new branch + --help # Display this help message ] # Push changes @@ -117,6 +119,7 @@ module completions { --tags # push tags (can't be used with --all or --mirror) --thin # use thin pack --verbose(-v) # be more verbose + --help # Display this help message ] } diff --git a/src/tests/test_custom_commands.rs b/src/tests/test_custom_commands.rs index 45a2acaee9..67d3cd274e 100644 --- a/src/tests/test_custom_commands.rs +++ b/src/tests/test_custom_commands.rs @@ -1,4 +1,4 @@ -use crate::tests::{fail_test, run_test, TestResult}; +use crate::tests::{fail_test, run_test, run_test_contains, TestResult}; #[test] fn no_scope_leak1() -> TestResult { @@ -117,3 +117,16 @@ fn allow_missing_optional_params() -> TestResult { "5", ) } + +#[test] +fn help_present_in_def() -> TestResult { + run_test_contains("def foo [] {}; help foo;", "Display this help message") +} + +#[test] +fn help_not_present_in_extern() -> TestResult { + run_test( + "module test {export extern \"git fetch\" []}; use test; help git fetch", + "Usage:\n > git fetch", + ) +}