Repository: google/argh (rust)

Issue:
positional arguments made on field with reserved named (ref_) keep the trailing _ in help text
This kind of declaration

```rust
	#[argh(positional)]
	/// reference (tag or branch)
	pub ref_: Option<String>,
```
will generate this as help text

```
Positional Arguments:
  ref_              reference (tag or branch)
```

When using option or switch, fields are correctly renamed

Pull request description:
Fix #149 and a few lints from newer clippy

Patch:
diff --git a/argh/src/lib.rs b/argh/src/lib.rs
index c507be0..9e955da 100644
--- a/argh/src/lib.rs
+++ b/argh/src/lib.rs
@@ -290,6 +290,29 @@
 //!     }
 //! }
 //! ```
+//!
+//! Programs that are run from an environment such as cargo may find it
+//! useful to have positional arguments present in the structure but
+//! omitted from the usage output. This can be accomplished by adding
+//! the `hidden_help` attribute to that argument:
+//!
+//! ```rust
+//! # use argh::FromArgs;
+//!
+//! #[derive(FromArgs)]
+//! /// Cargo arguments
+//! struct CargoArgs {
+//!     // Cargo puts the command name invoked into the first argument,
+//!     // so we don't want this argument to show up in the usage text.
+//!     #[argh(positional, hidden_help)]
+//!     command: String,
+//!     /// an option used for internal debugging
+//!     #[argh(option, hidden_help)]
+//!     internal_debugging: String,
+//!     #[argh(positional)]
+//!     real_first_arg: String,
+//! }
+//! ```
 
 #![deny(missing_docs)]
 
diff --git a/argh_derive/src/help.rs b/argh_derive/src/help.rs
index c9daad5..bd20622 100644
--- a/argh_derive/src/help.rs
+++ b/argh_derive/src/help.rs
@@ -27,11 +27,11 @@ pub(crate) fn help(
     fields: &[StructField<'_>],
     subcommand: Option<&StructField<'_>>,
 ) -> TokenStream {
-    #![allow(clippy::format_push_string)]
     let mut format_lit = "Usage: {command_name}".to_string();
 
-    let positional =
-        fields.iter().filter(|f| f.kind == FieldKind::Positional && f.attrs.greedy.is_none());
+    let positional = fields.iter().filter(|f| {
+        f.kind == FieldKind::Positional && f.attrs.greedy.is_none() && !f.attrs.hidden_help
+    });
     let mut has_positional = false;
     for arg in positional.clone() {
         has_positional = true;
@@ -39,14 +39,15 @@ pub(crate) fn help(
         positional_usage(&mut format_lit, arg);
     }
 
-    let options = fields.iter().filter(|f| f.long_name.is_some());
+    let options = fields.iter().filter(|f| f.long_name.is_some() && !f.attrs.hidden_help);
     for option in options.clone() {
         format_lit.push(' ');
         option_usage(&mut format_lit, option);
     }
 
-    let remain =
-        fields.iter().filter(|f| f.kind == FieldKind::Positional && f.attrs.greedy.is_some());
+    let remain = fields.iter().filter(|f| {
+        f.kind == FieldKind::Positional && f.attrs.greedy.is_some() && !f.attrs.hidden_help
+    });
     for arg in remain {
         format_lit.push(' ');
         positional_usage(&mut format_lit, arg);
@@ -154,7 +155,7 @@ fn positional_usage(out: &mut String, field: &StructField<'_>) {
     if field.attrs.greedy.is_none() {
         out.push('<');
     }
-    let name = field.arg_name();
+    let name = field.positional_arg_name();
     out.push_str(&name);
     if field.optionality == Optionality::Repeating {
         out.push_str("...");
@@ -229,7 +230,7 @@ Add a doc comment or an `#[argh(description = \"...\")]` attribute.",
 /// Describes a positional argument like this:
 ///  hello       positional argument description
 fn positional_description(out: &mut String, field: &StructField<'_>) {
-    let field_name = field.arg_name();
+    let field_name = field.positional_arg_name();
 
     let mut description = String::from("");
     if let Some(desc) = &field.attrs.description {
diff --git a/argh_derive/src/lib.rs b/argh_derive/src/lib.rs
index 49a6c17..183cb6c 100644
--- a/argh_derive/src/lib.rs
+++ b/argh_derive/src/lib.rs
@@ -196,8 +196,12 @@ impl<'a> StructField<'a> {
         Some(StructField { field, attrs, kind, optionality, ty_without_wrapper, name, long_name })
     }
 
-    pub(crate) fn arg_name(&self) -> String {
-        self.attrs.arg_name.as_ref().map(LitStr::value).unwrap_or_else(|| self.name.to_string())
+    pub(crate) fn positional_arg_name(&self) -> String {
+        self.attrs
+            .arg_name
+            .as_ref()
+            .map(LitStr::value)
+            .unwrap_or_else(|| self.name.to_string().trim_matches('_').to_owned())
     }
 }
 
@@ -530,7 +534,7 @@ fn ensure_unique_names(errors: &Errors, fields: &[StructField<'_>]) {
                     first_use_field,
                     &format!("The short name of \"-{}\" was already used here.", short_name),
                 );
-                errors.err_span_tokens(&field.field, "Later usage here.");
+                errors.err_span_tokens(field.field, "Later usage here.");
             }
 
             seen_short_names.insert(short_name, &field.field);
@@ -542,7 +546,7 @@ fn ensure_unique_names(errors: &Errors, fields: &[StructField<'_>]) {
                     *first_use_field,
                     &format!("The long name of \"{}\" was already used here.", long_name),
                 );
-                errors.err_span_tokens(&field.field, "Later usage here.");
+                errors.err_span_tokens(field.field, "Later usage here.");
             }
 
             seen_long_names.insert(long_name, field.field);
@@ -710,7 +714,7 @@ fn declare_local_storage_for_redacted_fields<'a>(
                     }
                 };
 
-                let arg_name = field.arg_name();
+                let arg_name = field.positional_arg_name();
                 quote! {
                     let mut #field_name: argh::ParseValueSlotTy::<#field_slot_type, String> =
                         argh::ParseValueSlotTy {
@@ -802,7 +806,7 @@ fn append_missing_requirements<'a>(
         match field.kind {
             FieldKind::Switch => unreachable!("switches are always optional"),
             FieldKind::Positional => {
-                let name = field.arg_name();
+                let name = field.positional_arg_name();
                 quote! {
                     if #field_name.slot.is_none() {
                         #mri.missing_positional_arg(#name)
diff --git a/argh_derive/src/parse_attrs.rs b/argh_derive/src/parse_attrs.rs
index f5fa841..0d1cca3 100644
--- a/argh_derive/src/parse_attrs.rs
+++ b/argh_derive/src/parse_attrs.rs
@@ -19,6 +19,7 @@ pub struct FieldAttrs {
     pub short: Option<syn::LitChar>,
     pub arg_name: Option<syn::LitStr>,
     pub greedy: Option<syn::Path>,
+    pub hidden_help: bool,
 }
 
 /// The purpose of a particular field on a `#![derive(FromArgs)]` struct.
@@ -123,13 +124,15 @@ impl FieldAttrs {
                     );
                 } else if name.is_ident("greedy") {
                     this.greedy = Some(name.clone());
+                } else if name.is_ident("hidden_help") {
+                    this.hidden_help = true;
                 } else {
                     errors.err(
                         &meta,
                         concat!(
                             "Invalid field-level `argh` attribute\n",
                             "Expected one of: `arg_name`, `default`, `description`, `from_str_fn`, `greedy`, ",
-                            "`long`, `option`, `short`, `subcommand`, `switch`",
+                            "`long`, `option`, `short`, `subcommand`, `switch`, `hidden_help`",
                         ),
                     );
                 }


Test patch:
diff --git a/argh/tests/lib.rs b/argh/tests/lib.rs
index 584897c..f8068ff 100644
--- a/argh/tests/lib.rs
+++ b/argh/tests/lib.rs
@@ -703,6 +703,26 @@ Required options not provided:
             },
         );
     }
+
+    #[derive(FromArgs, Debug, PartialEq)]
+    /// Woot
+    struct Underscores {
+        #[argh(positional)]
+        /// fooey
+        a_: String,
+    }
+
+    #[test]
+    fn positional_name_with_underscores() {
+        assert_output(&["first"], Underscores { a_: "first".into() });
+
+        assert_error::<Underscores>(
+            &[],
+            r###"Required positional arguments not provided:
+    a
+"###,
+        );
+    }
 }
 
 /// Tests derived from
@@ -1168,6 +1188,36 @@ Destroy the contents of <file>.
 Positional Arguments:
   name
 
+Options:
+  --help            display usage information
+"###,
+        );
+    }
+
+    #[test]
+    fn hidden_help_attribute() {
+        #[derive(FromArgs)]
+        /// Short description
+        struct Cmd {
+            /// this one should be hidden
+            #[argh(positional, hidden_help)]
+            _one: String,
+            #[argh(positional)]
+            /// this one is real
+            _two: String,
+            /// this one should be hidden
+            #[argh(option, hidden_help)]
+            _three: String,
+        }
+
+        assert_help_string::<Cmd>(
+            r###"Usage: test_arg_0 <_two>
+
+Short description
+
+Positional Arguments:
+  _two              this one is real
+
 Options:
   --help            display usage information
 "###,