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 <clements.michael.james@gmail.com>

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);

crates/nu-protocol/src/signature.rs

@@ -124,6 +124,23 @@ impl Eq for Signature {}
 
 impl Signature {
     pub fn new(name: impl Into<String>) -> 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<String>) -> Signature {
-        Signature::new(name.into())
+        Signature::new(name.into()).add_help()
     }
 
     /// Add a description to the signature

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!(

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
   ]
 }
 

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",
+    )
+}