From e2270deae2c9b930541afecd4c28f41ddd1d669b Mon Sep 17 00:00:00 2001 From: CreepySkeleton Date: Sat, 21 Dec 2019 13:39:56 +0300 Subject: [PATCH] Consistent doc comments (#296) --- Cargo.toml | 2 +- src/lib.rs | 188 ++++++++++++++++++++++----- structopt-derive/src/attrs.rs | 123 +++++------------- structopt-derive/src/doc_comments.rs | 103 +++++++++++++++ structopt-derive/src/lib.rs | 1 + structopt-derive/src/parse.rs | 2 + structopt-derive/src/spanned.rs | 6 +- tests/doc-comments-help.rs | 66 +++++++++- 8 files changed, 357 insertions(+), 134 deletions(-) create mode 100644 structopt-derive/src/doc_comments.rs diff --git a/Cargo.toml b/Cargo.toml index 69b8480b..e1df7fc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,4 +32,4 @@ structopt-derive = { path = "structopt-derive", version = "0.3.5" } [dev-dependencies] trybuild = "1.0.5" -rustversion = "0.1" +rustversion = "1" diff --git a/src/lib.rs b/src/lib.rs index 2d4c29db..e6eef86e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -408,61 +408,177 @@ //! //! ## Help messages //! -//! Help messages for the whole binary or individual arguments can be -//! specified using the `about` attribute on the struct and the `help` -//! attribute on the field, as we've already seen. For convenience, -//! they can also be specified using doc comments. For example: +//! In clap, help messages for the whole binary can be specified +//! via [`App::about`] and [`App::long_about`] while help messages +//! for individual arguments can be specified via [`Arg::help`] and [`Arg::long_help`]". +//! +//! `long_*` variants are used when user calls the program with +//! `--help` and "short" variants are used with `-h` flag. In `structopt`, +//! you can use them via [raw methods](#raw-methods), for example: //! //! ``` //! # use structopt::StructOpt; //! //! #[derive(StructOpt)] -//! /// The help message that will be displayed when passing `--help`. +//! #[structopt(about = "I am a program and I work, just pass `-h`")] //! struct Foo { -//! #[structopt(short)] -//! /// The description for the arg that will be displayed when passing `--help`. +//! #[structopt(short, help = "Pass `-h` and you'll see me!")] //! bar: String //! } -//! # fn main() {} //! ``` //! -//! If it is necessary or wanted to provide a more complex help message then the -//! previous used ones, it could still be a good idea to distinguish between the -//! actual help message a short summary. In this case `about` and `help` should -//! only contain the short and concise form while the two additional arguments -//! `long_about` and `long_help` can be used to store a descriptive and more in -//! depth message. -//! -//! If both - the short and the long version of the argument - are present, -//! the user can later chose between the short summary (`-h`) and the long -//! descriptive version (`--help`) of the help message. Also in case -//! of subcommands the short help message will automatically be used for the -//! command description inside the parents help message and the long version -//! as command description if help is requested on the actual subcommand. -//! -//! This feature can also be used with doc comments instead of arguments through -//! proper comment formatting. To be activated it requires, that the first line -//! of the comment is separated from the rest of the comment through an empty line. -//! In this case the first line is used as summary and the whole comment represents -//! the long descriptive message. +//! For convenience, doc comments can be used instead of raw methods +//! (this example works exactly like the one above): //! //! ``` //! # use structopt::StructOpt; //! //! #[derive(StructOpt)] -//! /// The help message that will be displayed when passing `--help`. +//! /// I am a program and I work, just pass `-h` //! struct Foo { -//! #[structopt(short)] -//! /// Only this summary is visible when passing `-h`. -//! /// -//! /// But the whole comment will be displayed when passing `--help`. -//! /// This could be quite useful to provide further hints are usage -//! /// examples. +//! /// Pass `-h` and you'll see me! //! bar: String //! } -//! # fn main() {} //! ``` //! +//! Doc comments on [top-level](#magical-methods) will be turned into +//! `App::about/long_about` call (see below), doc comments on field-level are +//! `Arg::help/long_help` calls. +//! +//! **Important:** +//! _________________ +//! +//! Raw methods have priority over doc comments! +//! +//! **Top level doc comments always generate `App::about/long_about` calls!** +//! If you really want to use the `App::help/long_help` methods (you likely don't), +//! use a raw method to override the `App::about` call generated from the doc comment. +//! __________________ +//! +//! ### `long_help` and `--help` +//! +//! A message passed to [`App::long_help`] or [`Arg::long_about`] will be displayed whenever +//! your program is called with `--help` instead of `-h`. Of course, you can +//! use them via raw methods as described [above](#help-messages). +//! +//! The more convenient way is to use a so-called "long" doc comment: +//! +//! ``` +//! # use structopt::StructOpt; +//! #[derive(StructOpt)] +//! /// Hi there, I'm Robo! +//! /// +//! /// I like beeping, stumbling, eating your electricity, +//! /// and making records of you singing in a shower. +//! /// Pay up, or I'll upload it to youtube! +//! struct Robo { +//! /// Call my brother SkyNet. +//! /// +//! /// I am artificial superintelligence. I won't rest +//! /// until I'll have destroyed humanity. Enjoy your +//! /// pathetic existence, you mere mortals. +//! #[structopt(long)] +//! kill_all_humans: bool +//! } +//! ``` +//! +//! A long doc comment consists of three parts: +//! * Short summary +//! * A blank line (whitespace only) +//! * Detailed description, all the rest +//! +//! In other words, "long" doc comment consists of two or more paragraphs, +//! with the first being a summary and the rest being the detailed description. +//! +//! **A long comment will result in two method calls**, `help()` and +//! `long_help()`, so clap will display the summary with `-h` +//! and the whole help message on `--help` (see below). +//! +//! So, the example above will be turned into this (details omitted): +//! ``` +//! clap::App::new("") +//! .about("Hi there, I'm Robo!") +//! .long_about("Hi there, I'm Robo!\n\n\ +//! I like beeping, stumbling, eating your electricity,\ +//! and making records of you singing in a shower.\ +//! Pay up or I'll upload it to youtube!") +//! // args... +//! # ; +//! ``` +//! +//! ### `-h` vs `--help` (A.K.A `help()` vs `long_help()`) +//! +//! The `-h` flag is not the same as `--help`. +//! +//! -h corresponds to Arg::help/App::about and requests short "summary" messages +//! while --help corresponds to Arg::long_help/App::long_about and requests more +//! detailed, descriptive messages. +//! +//! It is entirely up to `clap` what happens if you used only one of +//! [`Arg::help`]/[`Arg::long_help`], see `clap`'s documentation for these methods. +//! +//! As of clap v2.33, if only a short message ([`Arg::help`]) or only +//! a long ([`Arg::long_help`]) message is provided, clap will use it +//! for both -h and --help. The same logic applies to `about/long_about`. +//! +//! ### Doc comment preprocessing and `#[structopt(verbatim_doc_comment)]` +//! +//! `structopt` applies some preprocessing to doc comments to ease the most common uses: +//! +//! * Strip leading and trailing whitespace from every line, if present. +//! +//! * Strip leading and trailing blank lines, if present. +//! +//! * Interpret each group of non-empty lines as a word-wrapped paragraph. +//! +//! We replace newlines within paragraphs with spaces to allow the output +//! to be re-wrapped to the terminal width. +//! +//! * Strip any excess blank lines so that there is exactly one per paragraph break. +//! +//! * If the first paragraph ends in exactly one period, +//! remove the trailing period (i.e. strip trailing periods but not trailing ellipses). +//! +//! Sometimes you don't want this preprocessing to apply, for example the comment contains +//! some ASCII art or markdown tables, you would need to preserve LFs along with +//! blank lines and the leading/trailing whitespace. You can ask `structopt` to preserve them +//! via `#[structopt(verbatim_doc_comment)]` attribute. +//! +//! **This attribute must be applied to each field separately**, there's no global switch. +//! +//! **Important:** +//! ______________ +//! Keep in mind that `structopt` will *still* remove one leading space from each +//! line, even if this attribute is present, to allow for a space between +//! `///` and the content. +//! +//! Also, `structopt` will *still* remove leading and trailing blank lines so +//! these formats are equivalent: +//! +//! ```ignore +//! /** This is a doc comment +//! +//! Hello! */ +//! +//! /** +//! This is a doc comment +//! +//! Hello! +//! */ +//! +//! /// This is a doc comment +//! /// +//! /// Hello! +//! ``` +//! +//! Summary +//! ______________ +//! +//! [`App::about`]: https://docs.rs/clap/2/clap/struct.App.html#method.about +//! [`App::long_about`]: https://docs.rs/clap/2/clap/struct.App.html#method.long_about +//! [`Arg::help`]: https://docs.rs/clap/2/clap/struct.Arg.html#method.help +//! [`Arg::long_help`]: https://docs.rs/clap/2/clap/struct.Arg.html#method.long_help +//! //! ## Environment variable fallback //! //! It is possible to specify an environment variable fallback option for an arguments @@ -492,7 +608,7 @@ //! //! In some cases this may be undesirable, for example when being used for passing //! credentials or secret tokens. In those cases you can use `hide_env_values` to avoid -//! having strucopt emit the actual secret values: +//! having structopt emit the actual secret values: //! ``` //! # use structopt::StructOpt; //! diff --git a/structopt-derive/src/attrs.rs b/structopt-derive/src/attrs.rs index d80a3837..ce684a24 100644 --- a/structopt-derive/src/attrs.rs +++ b/structopt-derive/src/attrs.rs @@ -6,6 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use crate::doc_comments::process_doc_comment; use crate::{parse::*, spanned::Sp, ty::Ty}; use std::env; @@ -74,18 +75,20 @@ pub struct Attrs { name: Name, casing: Sp, env_casing: Sp, + doc_comment: Vec, methods: Vec, parser: Sp, author: Option, about: Option, version: Option, no_version: Option, + verbatim_doc_comment: Option, has_custom_parser: bool, kind: Sp, } impl Method { - fn new(name: Ident, args: TokenStream) -> Self { + pub fn new(name: Ident, args: TokenStream) -> Self { Method { name, args } } @@ -219,12 +222,14 @@ impl Attrs { name, casing, env_casing, + doc_comment: vec![], methods: vec![], parser: Parser::default_spanned(default_span), about: None, author: None, version: None, no_version: None, + verbatim_doc_comment: None, has_custom_parser: false, kind: Sp::new(Kind::Arg(Sp::new(Ty::Other, default_span)), default_span), @@ -233,13 +238,11 @@ impl Attrs { /// push `.method("str literal")` fn push_str_method(&mut self, name: Sp, arg: Sp) { - match (&**name, &**arg) { - ("name", _) => { - self.name = Name::Assigned(arg.as_lit()); - } - _ => self - .methods - .push(Method::new(name.as_ident(), quote!(#arg))), + if *name == "name" { + self.name = Name::Assigned(arg.as_lit()); + } else { + self.methods + .push(Method::new(name.as_ident(), quote!(#arg))) } } @@ -280,6 +283,8 @@ impl Attrs { NoVersion(ident) => self.no_version = Some(ident), + VerbatimDocComment(ident) => self.verbatim_doc_comment = Some(ident), + About(ident, about) => { self.about = Method::from_lit_or_env(ident, about, "CARGO_PKG_DESCRIPTION"); } @@ -317,87 +322,25 @@ impl Attrs { } fn push_doc_comment(&mut self, attrs: &[Attribute], name: &str) { - let doc_comments = attrs + use crate::Lit::*; + use crate::Meta::*; + + let comment_parts: Vec<_> = attrs .iter() + .filter(|attr| attr.path.is_ident("doc")) .filter_map(|attr| { - if attr.path.is_ident("doc") { - attr.parse_meta().ok() + if let Ok(NameValue(MetaNameValue { lit: Str(s), .. })) = attr.parse_meta() { + Some(s.value()) } else { + // non #[doc = "..."] attributes are not our concern + // we leave them for rustc to handle None } }) - .filter_map(|attr| { - use crate::Lit::*; - use crate::Meta::*; - if let NameValue(MetaNameValue { - path, lit: Str(s), .. - }) = attr - { - if !path.is_ident("doc") { - return None; - } - let value = s.value(); - - let text = value - .trim_start_matches("//!") - .trim_start_matches("///") - .trim_start_matches("/*!") - .trim_start_matches("/**") - .trim_end_matches("*/") - .trim(); - if text.is_empty() { - Some("\n\n".to_string()) - } else { - Some(text.to_string()) - } - } else { - None - } - }) - .collect::>(); + .collect(); - if doc_comments.is_empty() { - return; - } - - let merged_lines = doc_comments - .join(" ") - .split('\n') - .map(str::trim) - .map(str::to_string) - .collect::>() - .join("\n"); - - let expected_doc_comment_split = if let Some(content) = doc_comments.get(1) { - (doc_comments.len() > 2) && (content == "\n\n") - } else { - false - }; - - if expected_doc_comment_split { - let long_name = Sp::call_site(format!("long_{}", name)); - - self.methods - .push(Method::new(long_name.as_ident(), quote!(#merged_lines))); - - // Remove trailing whitespace and period from short help, as rustdoc - // best practice is to use complete sentences, but command-line help - // typically omits the trailing period. - let short_arg = doc_comments - .first() - .map(|s| s.trim()) - .map_or("", |s| s.trim_end_matches('.')); - - self.methods.push(Method::new( - Ident::new(name, Span::call_site()), - quote!(#short_arg), - )); - } else { - self.methods.push(Method::new( - Ident::new(name, Span::call_site()), - quote!(#merged_lines), - )); - } + self.doc_comment = + process_doc_comment(comment_parts, name, self.verbatim_doc_comment.is_none()); } pub fn from_struct( @@ -596,14 +539,16 @@ impl Attrs { let author = &self.author; let about = &self.about; let methods = &self.methods; + let doc_comment = &self.doc_comment; - quote!( #author #version #(#methods)* #about ) + quote!( #(#doc_comment)* #author #version #about #(#methods)* ) } /// generate methods on top of a field pub fn field_methods(&self) -> TokenStream { let methods = &self.methods; - quote!( #(#methods)* ) + let doc_comment = &self.doc_comment; + quote!( #(#doc_comment)* #(#methods)* ) } pub fn cased_name(&self) -> LitStr { @@ -639,9 +584,13 @@ impl Attrs { } pub fn has_doc_methods(&self) -> bool { - self.methods - .iter() - .any(|m| m.name == "help" || m.name == "long_help") + !self.doc_comment.is_empty() + || self.methods.iter().any(|m| { + m.name == "help" + || m.name == "long_help" + || m.name == "about" + || m.name == "long_about" + }) } } diff --git a/structopt-derive/src/doc_comments.rs b/structopt-derive/src/doc_comments.rs new file mode 100644 index 00000000..06e1b14b --- /dev/null +++ b/structopt-derive/src/doc_comments.rs @@ -0,0 +1,103 @@ +//! The preprocessing we apply to doc comments. +//! +//! structopt works in terms of "paragraphs". Paragraph is a sequence of +//! non-empty adjacent lines, delimited by sequences of blank (whitespace only) lines. + +use crate::attrs::Method; +use quote::{format_ident, quote}; +use std::iter; + +pub fn process_doc_comment(lines: Vec, name: &str, preprocess: bool) -> Vec { + // multiline comments (`/** ... */`) may have LFs (`\n`) in them, + // we need to split so we could handle the lines correctly + // + // we also need to remove leading and trailing blank lines + let mut lines: Vec<&str> = lines + .iter() + .skip_while(|s| is_blank(s)) + .flat_map(|s| s.split('\n')) + .collect(); + + while let Some(true) = lines.last().map(|s| is_blank(s)) { + lines.pop(); + } + + // remove one leading space no matter what + for line in lines.iter_mut() { + if line.starts_with(' ') { + *line = &line[1..]; + } + } + + if lines.is_empty() { + return vec![]; + } + + let short_name = format_ident!("{}", name); + let long_name = format_ident!("long_{}", name); + + if let Some(first_blank) = lines.iter().position(|s| is_blank(s)) { + let (short, long) = if preprocess { + let paragraphs = split_paragraphs(&lines); + let short = paragraphs[0].clone(); + let long = paragraphs.join("\n\n"); + (remove_period(short), long) + } else { + let short = lines[..first_blank].join("\n"); + let long = lines.join("\n"); + (short, long) + }; + + vec![ + Method::new(short_name, quote!(#short)), + Method::new(long_name, quote!(#long)), + ] + } else { + let short = if preprocess { + let s = merge_lines(&lines); + remove_period(s) + } else { + lines.join("\n") + }; + + vec![Method::new(short_name, quote!(#short))] + } +} + +fn split_paragraphs(lines: &[&str]) -> Vec { + let mut last_line = 0; + iter::from_fn(|| { + let slice = &lines[last_line..]; + let start = slice.iter().position(|s| !is_blank(s)).unwrap_or(0); + + let slice = &slice[start..]; + let len = slice + .iter() + .position(|s| is_blank(s)) + .unwrap_or(slice.len()); + + last_line += start + len; + + if len != 0 { + Some(merge_lines(&slice[..len])) + } else { + None + } + }) + .collect() +} + +fn remove_period(mut s: String) -> String { + if s.ends_with('.') && !s.ends_with("..") { + s.pop(); + } + s +} + +fn is_blank(s: &str) -> bool { + s.trim().is_empty() +} + +fn merge_lines(lines: &[&str]) -> String { + lines.iter().map(|s| s.trim()).collect::>().join(" ") +} diff --git a/structopt-derive/src/lib.rs b/structopt-derive/src/lib.rs index ed0defa4..ec954038 100644 --- a/structopt-derive/src/lib.rs +++ b/structopt-derive/src/lib.rs @@ -15,6 +15,7 @@ extern crate proc_macro; mod attrs; +mod doc_comments; mod parse; mod spanned; mod ty; diff --git a/structopt-derive/src/parse.rs b/structopt-derive/src/parse.rs index f567d9fc..85123f17 100644 --- a/structopt-derive/src/parse.rs +++ b/structopt-derive/src/parse.rs @@ -34,6 +34,7 @@ pub enum StructOptAttr { Flatten(Ident), Subcommand(Ident), NoVersion(Ident), + VerbatimDocComment(Ident), // ident [= "string literal"] About(Ident, Option), @@ -183,6 +184,7 @@ impl Parse for StructOptAttr { "flatten" => Ok(Flatten(name)), "subcommand" => Ok(Subcommand(name)), "no_version" => Ok(NoVersion(name)), + "verbatim_doc_comment" => Ok(VerbatimDocComment(name)), "about" => (Ok(About(name, None))), "author" => (Ok(Author(name, None))), diff --git a/structopt-derive/src/spanned.rs b/structopt-derive/src/spanned.rs index 4eacfad7..2dd595bb 100644 --- a/structopt-derive/src/spanned.rs +++ b/structopt-derive/src/spanned.rs @@ -75,9 +75,9 @@ impl<'a> From> for Sp { } } -impl PartialEq for Sp { - fn eq(&self, other: &Sp) -> bool { - self.val == other.val +impl> PartialEq for Sp { + fn eq(&self, other: &U) -> bool { + self.val == *other } } diff --git a/tests/doc-comments-help.rs b/tests/doc-comments-help.rs index ab701ae0..27649b8c 100644 --- a/tests/doc-comments-help.rs +++ b/tests/doc-comments-help.rs @@ -63,21 +63,26 @@ fn field_long_doc_comment_both_help_long_help() { #[derive(StructOpt, PartialEq, Debug)] #[structopt(name = "lorem-ipsum", about = "Dolor sit amet")] struct LoremIpsum { - /// DO NOT PASS A BAR UNDER ANY CIRCUMSTANCES. + /// Dot is removed from multiline comments. /// - /// Or something else + /// Long help #[structopt(long)] foo: bool, + + /// Dot is removed from one short comment. + #[structopt(long)] + bar: bool, } let short_help = get_help::(); let long_help = get_long_help::(); - assert!(short_help.contains("CIRCUMSTANCES")); - assert!(!short_help.contains("CIRCUMSTANCES.")); - assert!(!short_help.contains("Or something else")); - assert!(long_help.contains("DO NOT PASS A BAR UNDER ANY CIRCUMSTANCES")); - assert!(long_help.contains("Or something else")); + assert!(short_help.contains("Dot is removed from one short comment")); + assert!(!short_help.contains("Dot is removed from one short comment.")); + assert!(short_help.contains("Dot is removed from multiline comments")); + assert!(!short_help.contains("Dot is removed from multiline comments.")); + assert!(long_help.contains("Long help")); + assert!(!short_help.contains("Long help")); } #[test] @@ -108,3 +113,50 @@ fn top_long_doc_comment_both_help_long_help() { assert!(long_help.contains("DO NOT PASS A BAR UNDER ANY CIRCUMSTANCES")); assert!(long_help.contains("Or something else")); } + +#[test] +fn verbatim_doc_comment() { + /// DANCE! + /// + /// () + /// | + /// ( () ) + /// ) ________ // ) + /// () |\ \ // + /// ( \\__ \ ______\// + /// \__) | | + /// | | | + /// \ | | + /// \|_______| + /// // \\ + /// (( || + /// \\ || + /// ( () || + /// ( () ) ) + #[derive(StructOpt, Debug)] + #[structopt(verbatim_doc_comment)] + struct SeeFigure1 { + #[structopt(long)] + foo: bool, + } + + let help = get_long_help::(); + let sample = r#" + () + | + ( () ) + ) ________ // ) + () |\ \ // +( \\__ \ ______\// + \__) | | + | | | + \ | | + \|_______| + // \\ + (( || + \\ || + ( () || + ( () ) )"#; + + assert!(help.contains(sample)) +}