Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to disable requirement for uncapitalized descriptions #155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions argh/examples/simple_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ use {argh::FromArgs, std::fmt::Debug};

#[derive(FromArgs, PartialEq, Debug)]
/// Top-level command.
#[argh(lax_descriptions)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a weird place to add this to the example.

struct TopLevel {
/// Foo
#[argh(switch)]
common: bool,

#[argh(subcommand)]
nested: MySubCommandEnum,
}
Expand Down
18 changes: 18 additions & 0 deletions argh/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,24 @@
//! real_first_arg: String,
//! }
//! ```
//!
//! Programs that are not designed to conform to the Fuchsia commandline tools
//! specification may find the requirement for descriptions to begin with a
//! lowercase letter too restrictive. This can be disabled by adding the
//! `lax_descriptions` attribute to the type:
//!
//! ```rust,no_run
//! use argh::FromArgs;
//!
//! #[derive(FromArgs)]
//! #[argh(lax_descriptions)]
//! /// Reach new heights.
//! struct GoUp {
//! /// Now you can capitalize descriptions.
//! #[argh(switch, short = 'j')]
//! jump: bool,
//! }
//! ```

#![deny(missing_docs)]

Expand Down
15 changes: 15 additions & 0 deletions argh/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,21 @@ struct DescriptionStartsWithInitialism {
x: u8,
}

/// Test that descriptions can start with any case when
/// the type attribute `lax_descriptions` is specified.
#[derive(FromArgs)]
#[argh(lax_descriptions)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I like the name "lax_descriptions". Seems a bit generic.

#[allow(unused)]
struct LaxDescription {
/// Don't be so strict.
#[argh(option)]
x: u8,

/// don't be so strict.
#[argh(option)]
y: u8,
}

#[test]
fn default_number() {
#[derive(FromArgs)]
Expand Down
2 changes: 1 addition & 1 deletion argh_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ fn impl_from_args_struct(
.named
.iter()
.filter_map(|field| {
let attrs = FieldAttrs::parse(errors, field);
let attrs = FieldAttrs::parse(errors, field, type_attrs);
StructField::new(errors, field, attrs)
})
.collect();
Expand Down
23 changes: 18 additions & 5 deletions argh_derive/src/parse_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub struct Description {
}

impl FieldAttrs {
pub fn parse(errors: &Errors, field: &syn::Field) -> Self {
pub fn parse(errors: &Errors, field: &syn::Field, type_attrs: &TypeAttrs) -> Self {
let mut this = Self::default();

for attr in &field.attrs {
Expand Down Expand Up @@ -160,8 +160,10 @@ impl FieldAttrs {
_ => {}
}

if let Some(d) = &this.description {
check_option_description(errors, d.content.value().trim(), d.content.span());
if !type_attrs.lax_descriptions {
if let Some(d) = &this.description {
check_option_description(errors, d.content.value().trim(), d.content.span());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change the behavior of check_option_description rather than skipping it entirely when the option is set.

}
}

this
Expand Down Expand Up @@ -298,6 +300,7 @@ pub struct TypeAttrs {
pub examples: Vec<syn::LitStr>,
pub notes: Vec<syn::LitStr>,
pub error_codes: Vec<(syn::LitInt, syn::LitStr)>,
pub lax_descriptions: bool,
}

impl TypeAttrs {
Expand Down Expand Up @@ -345,13 +348,15 @@ impl TypeAttrs {
if let Some(ident) = errors.expect_meta_word(meta).and_then(|p| p.get_ident()) {
this.parse_attr_subcommand(errors, ident);
}
} else if name.is_ident("lax_descriptions") {
this.lax_descriptions = true;
} else {
errors.err(
&meta,
concat!(
"Invalid type-level `argh` attribute\n",
"Expected one of: `description`, `error_code`, `example`, `name`, ",
"`note`, `subcommand`",
"`note`, `subcommand`, `lax_descriptions`",
),
);
}
Expand Down Expand Up @@ -566,7 +571,15 @@ fn parse_attr_description(errors: &Errors, m: &syn::MetaNameValue, slot: &mut Op
/// Checks that a `#![derive(FromArgs)]` enum has an `#[argh(subcommand)]`
/// attribute and that it does not have any other type-level `#[argh(...)]` attributes.
pub fn check_enum_type_attrs(errors: &Errors, type_attrs: &TypeAttrs, type_span: &Span) {
let TypeAttrs { is_subcommand, name, description, examples, notes, error_codes } = type_attrs;
let TypeAttrs {
is_subcommand,
name,
description,
examples,
notes,
error_codes,
lax_descriptions: _,
} = type_attrs;

// Ensure that `#[argh(subcommand)]` is present.
if is_subcommand.is_none() {
Expand Down