Merge pull request #3466 from epage/usage

refactor: Centralize build logic

src/build/app.rs

@@ -1,15 +1,13 @@
 #![allow(deprecated)]
 
 // Std
-use std::{
+use std::collections::HashMap;
-    collections::HashMap,
+use std::env;
-    env,
+use std::ffi::OsString;
-    ffi::OsString,
+use std::fmt;
-    fmt,
+use std::io;
-    io::{self, Write},
+use std::ops::Index;
-    ops::Index,
+use std::path::Path;
-    path::Path,
-};
 
 // Third Party
 use os_str_bytes::RawOsStr;
@@ -25,6 +23,7 @@ use crate::error::Result as ClapResult;
 use crate::mkeymap::MKeyMap;
 use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage};
 use crate::parse::{ArgMatcher, ArgMatches, Input, Parser};
+use crate::util::ChildGraph;
 use crate::util::{color::ColorChoice, Id, Key};
 use crate::{Error, INTERNAL_ERROR_MSG};
 
@@ -81,7 +80,7 @@ pub struct App<'help> {
     pub(crate) short_flag_aliases: Vec<(char, bool)>, // (name, visible)
     pub(crate) long_flag_aliases: Vec<(&'help str, bool)>, // (name, visible)
     pub(crate) usage_str: Option<&'help str>,
-    pub(crate) usage: Option<String>,
+    pub(crate) usage_name: Option<String>,
     pub(crate) help_str: Option<&'help str>,
     pub(crate) disp_ord: Option<usize>,
     pub(crate) term_w: Option<usize>,
@@ -688,9 +687,8 @@ impl<'help> App<'help> {
         let color = self.get_color();
 
         let mut c = Colorizer::new(false, color);
-        let parser = Parser::new(self);
+        let usage = Usage::new(self);
-        let usage = Usage::new(parser.app, &parser.required);
+        Help::new(HelpWriter::Buffer(&mut c), self, &usage, false).write_help()?;
-        Help::new(HelpWriter::Buffer(&mut c), parser.app, &usage, false).write_help()?;
         c.print()
     }
 
@@ -714,9 +712,8 @@ impl<'help> App<'help> {
         let color = self.get_color();
 
         let mut c = Colorizer::new(false, color);
-        let parser = Parser::new(self);
+        let usage = Usage::new(self);
-        let usage = Usage::new(parser.app, &parser.required);
+        Help::new(HelpWriter::Buffer(&mut c), self, &usage, true).write_help()?;
-        Help::new(HelpWriter::Buffer(&mut c), parser.app, &usage, true).write_help()?;
         c.print()
     }
 
@@ -736,12 +733,11 @@ impl<'help> App<'help> {
     /// [`io::Write`]: std::io::Write
     /// [`-h` (short)]: Arg::help()
     /// [`--help` (long)]: Arg::long_help()
-    pub fn write_help<W: Write>(&mut self, w: &mut W) -> io::Result<()> {
+    pub fn write_help<W: io::Write>(&mut self, w: &mut W) -> io::Result<()> {
         self._build();
 
-        let parser = Parser::new(self);
+        let usage = Usage::new(self);
-        let usage = Usage::new(parser.app, &parser.required);
+        Help::new(HelpWriter::Normal(w), self, &usage, false).write_help()?;
-        Help::new(HelpWriter::Normal(w), parser.app, &usage, false).write_help()?;
         w.flush()
     }
 
@@ -761,12 +757,11 @@ impl<'help> App<'help> {
     /// [`io::Write`]: std::io::Write
     /// [`-h` (short)]: Arg::help()
     /// [`--help` (long)]: Arg::long_help()
-    pub fn write_long_help<W: Write>(&mut self, w: &mut W) -> io::Result<()> {
+    pub fn write_long_help<W: io::Write>(&mut self, w: &mut W) -> io::Result<()> {
         self._build();
 
-        let parser = Parser::new(self);
+        let usage = Usage::new(self);
-        let usage = Usage::new(parser.app, &parser.required);
+        Help::new(HelpWriter::Normal(w), self, &usage, true).write_help()?;
-        Help::new(HelpWriter::Normal(w), parser.app, &usage, true).write_help()?;
         w.flush()
     }
 
@@ -833,8 +828,7 @@ impl<'help> App<'help> {
         // before parsing incase we run into a subcommand
         self._build();
 
-        let parser = Parser::new(self);
+        Usage::new(self).create_usage_with_title(&[])
-        Usage::new(parser.app, &parser.required).create_usage_with_title(&[])
     }
 }
 
@@ -3827,14 +3821,14 @@ impl<'help> App<'help> {
     /// Deprecated, replaced with [`App::render_version`]
     #[deprecated(since = "3.0.0", note = "Replaced with `App::render_version`")]
     #[doc(hidden)]
-    pub fn write_version<W: Write>(&self, w: &mut W) -> ClapResult<()> {
+    pub fn write_version<W: io::Write>(&self, w: &mut W) -> ClapResult<()> {
         write!(w, "{}", self.render_version()).map_err(From::from)
     }
 
     /// Deprecated, replaced with [`App::render_long_version`]
     #[deprecated(since = "3.0.0", note = "Replaced with `App::render_long_version`")]
     #[doc(hidden)]
-    pub fn write_long_version<W: Write>(&self, w: &mut W) -> ClapResult<()> {
+    pub fn write_long_version<W: io::Write>(&self, w: &mut W) -> ClapResult<()> {
         write!(w, "{}", self.render_long_version()).map_err(From::from)
     }
 
@@ -3924,6 +3918,59 @@ impl<'help> App<'help> {
         self._build_bin_names();
     }
 
+    pub(crate) fn _build_subcommand(&mut self, name: &str) -> Option<&mut Self> {
+        use std::fmt::Write;
+
+        let mut mid_string = String::from(" ");
+        if !self.is_subcommand_negates_reqs_set() {
+            let reqs = Usage::new(self).get_required_usage_from(&[], None, true); // maybe Some(m)
+
+            for s in &reqs {
+                mid_string.push_str(s);
+                mid_string.push(' ');
+            }
+        }
+
+        let sc = self.subcommands.iter_mut().find(|s| s.name == name)?;
+
+        // Display subcommand name, short and long in usage
+        let mut sc_names = sc.name.clone();
+        let mut flag_subcmd = false;
+        if let Some(l) = sc.long_flag {
+            write!(sc_names, ", --{}", l).unwrap();
+            flag_subcmd = true;
+        }
+        if let Some(s) = sc.short_flag {
+            write!(sc_names, ", -{}", s).unwrap();
+            flag_subcmd = true;
+        }
+
+        if flag_subcmd {
+            sc_names = format!("{{{}}}", sc_names);
+        }
+
+        sc.usage_name = Some(
+            self.bin_name
+                .as_ref()
+                .map(|bin_name| format!("{}{}{}", bin_name, mid_string, sc_names))
+                .unwrap_or(sc_names),
+        );
+
+        // bin_name should be parent's bin_name + [<reqs>] + the sc's name separated by
+        // a space
+        sc.bin_name = Some(format!(
+            "{}{}{}",
+            self.bin_name.as_ref().unwrap_or(&String::new()),
+            if self.bin_name.is_some() { " " } else { "" },
+            &*sc.name
+        ));
+
+        // Ensure all args are built and ready to parse
+        sc._build();
+
+        Some(sc)
+    }
+
     // used in clap_complete (https://github.com/clap-rs/clap_complete)
     #[doc(hidden)]
     pub fn _build(&mut self) {
@@ -4446,6 +4493,23 @@ impl<'help> App<'help> {
         })
     }
 
+    pub(crate) fn required_graph(&self) -> ChildGraph<Id> {
+        let mut reqs = ChildGraph::with_capacity(5);
+        for a in self.args.args().filter(|a| a.is_required_set()) {
+            reqs.insert(a.id.clone());
+        }
+        for group in &self.groups {
+            if group.required {
+                let idx = reqs.insert(group.id.clone());
+                for a in &group.requires {
+                    reqs.insert_child(idx, a.clone());
+                }
+            }
+        }
+
+        reqs
+    }
+
     pub(crate) fn unroll_args_in_group(&self, group: &Id) -> Vec<Id> {
         debug!("App::unroll_args_in_group: group={:?}", group);
         let mut g_vec = vec![group];
@@ -4546,7 +4610,7 @@ impl<'help> Default for App<'help> {
             short_flag_aliases: Default::default(),
             long_flag_aliases: Default::default(),
             usage_str: Default::default(),
-            usage: Default::default(),
+            usage_name: Default::default(),
             help_str: Default::default(),
             disp_ord: Default::default(),
             term_w: Default::default(),

src/output/usage.rs

@@ -1,22 +1,29 @@
 use indexmap::IndexSet;
 
 // Internal
-use crate::{
+use crate::build::AppSettings as AS;
-    build::AppSettings as AS,
+use crate::build::{App, Arg, ArgPredicate};
-    build::{App, Arg, ArgPredicate},
+use crate::parse::ArgMatcher;
-    parse::ArgMatcher,
+use crate::util::ChildGraph;
-    util::{ChildGraph, Id},
+use crate::util::Id;
-    INTERNAL_ERROR_MSG,
+use crate::INTERNAL_ERROR_MSG;
-};
 
 pub(crate) struct Usage<'help, 'app> {
     app: &'app App<'help>,
-    required: &'app ChildGraph<Id>,
+    required: Option<&'app ChildGraph<Id>>,
 }
 
 impl<'help, 'app> Usage<'help, 'app> {
-    pub(crate) fn new(app: &'app App<'help>, required: &'app ChildGraph<Id>) -> Self {
+    pub(crate) fn new(app: &'app App<'help>) -> Self {
-        Usage { app, required }
+        Usage {
+            app,
+            required: None,
+        }
+    }
+
+    pub(crate) fn required(mut self, required: &'app ChildGraph<Id>) -> Self {
+        self.required = Some(required);
+        self
     }
 
     // Creates a usage string for display. This happens just after all arguments were parsed, but before
@@ -42,12 +49,12 @@ impl<'help, 'app> Usage<'help, 'app> {
     }
 
     // Creates a usage string for display in help messages (i.e. not for errors)
-    pub(crate) fn create_help_usage(&self, incl_reqs: bool) -> String {
+    fn create_help_usage(&self, incl_reqs: bool) -> String {
         debug!("Usage::create_help_usage; incl_reqs={:?}", incl_reqs);
         let mut usage = String::with_capacity(75);
         let name = self
             .app
-            .usage
+            .usage_name
             .as_ref()
             .or_else(|| self.app.bin_name.as_ref())
             .unwrap_or(&self.app.name);
@@ -167,7 +174,7 @@ impl<'help, 'app> Usage<'help, 'app> {
         usage.push_str(
             &self
                 .app
-                .usage
+                .usage_name
                 .as_ref()
                 .or_else(|| self.app.bin_name.as_ref())
                 .unwrap_or(&self.app.name)[..],
@@ -346,7 +353,15 @@ impl<'help, 'app> Usage<'help, 'app> {
 
         let mut unrolled_reqs = IndexSet::new();
 
-        for a in self.required.iter() {
+        let required_owned;
+        let required = if let Some(required) = self.required {
+            required
+        } else {
+            required_owned = self.app.required_graph();
+            &required_owned
+        };
+
+        for a in required.iter() {
             let is_relevant = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option<Id> {
                 let required = match val {
                     ArgPredicate::Equals(_) => {
@@ -380,7 +395,7 @@ impl<'help, 'app> Usage<'help, 'app> {
             .app
             .groups
             .iter()
-            .filter(|gn| self.required.contains(&gn.id))
+            .filter(|gn| required.contains(&gn.id))
             .flat_map(|g| self.app.unroll_args_in_group(&g.id))
             .collect::<Vec<_>>();
 

src/parse/parser.rs

@@ -2,7 +2,6 @@
 use std::{
     cell::{Cell, RefCell},
     ffi::{OsStr, OsString},
-    fmt::Write as _,
 };
 
 // Third Party
@@ -18,12 +17,11 @@ use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage};
 use crate::parse::features::suggestions;
 use crate::parse::{ArgMatcher, SubCommand};
 use crate::parse::{Validator, ValueSource};
-use crate::util::{color::ColorChoice, ChildGraph, Id};
+use crate::util::{color::ColorChoice, Id};
 use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
 
 pub(crate) struct Parser<'help, 'app> {
     pub(crate) app: &'app mut App<'help>,
-    pub(crate) required: ChildGraph<Id>,
     pub(crate) overridden: RefCell<Vec<Id>>,
     seen: Vec<Id>,
     cur_idx: Cell<usize>,
@@ -37,22 +35,8 @@ pub(crate) struct Parser<'help, 'app> {
 // Initializing Methods
 impl<'help, 'app> Parser<'help, 'app> {
     pub(crate) fn new(app: &'app mut App<'help>) -> Self {
-        let mut reqs = ChildGraph::with_capacity(5);
-        for a in app.args.args().filter(|a| a.is_required_set()) {
-            reqs.insert(a.id.clone());
-        }
-        for group in &app.groups {
-            if group.required {
-                let idx = reqs.insert(group.id.clone());
-                for a in &group.requires {
-                    reqs.insert_child(idx, a.clone());
-                }
-            }
-        }
-
         Parser {
             app,
-            required: reqs,
             overridden: Default::default(),
             seen: Vec::new(),
             cur_idx: Cell::new(0),
@@ -250,7 +234,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                             return Err(ClapError::no_equals(
                                 self.app,
                                 arg,
-                                Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                                Usage::new(self.app).create_usage_with_title(&[]),
                             ));
                         }
                         ParseResult::NoMatchingArg { arg } => {
@@ -265,7 +249,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                                 self.app,
                                 rest,
                                 arg,
-                                Usage::new(self.app, &self.required).create_usage_no_title(&used),
+                                Usage::new(self.app).create_usage_no_title(&used),
                             ))
                         }
                         ParseResult::HelpFlag => {
@@ -339,7 +323,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                             return Err(ClapError::no_equals(
                                 self.app,
                                 arg,
-                                Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                                Usage::new(self.app).create_usage_with_title(&[]),
                             ))
                         }
                         ParseResult::NoMatchingArg { arg } => {
@@ -347,7 +331,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                                 self.app,
                                 arg,
                                 None,
-                                Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                                Usage::new(self.app).create_usage_with_title(&[]),
                             ));
                         }
                         ParseResult::HelpFlag => {
@@ -392,7 +376,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                         self.app,
                         arg_os.to_str_lossy().into_owned(),
                         None,
-                        Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                        Usage::new(self.app).create_usage_with_title(&[]),
                     ));
                 }
 
@@ -433,7 +417,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                     None => {
                         return Err(ClapError::invalid_utf8(
                             self.app,
-                            Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                            Usage::new(self.app).create_usage_with_title(&[]),
                         ));
                     }
                 };
@@ -448,7 +432,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                     if !allow_invalid_utf8 && v.to_str().is_none() {
                         return Err(ClapError::invalid_utf8(
                             self.app,
-                            Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                            Usage::new(self.app).create_usage_with_title(&[]),
                         ));
                     }
                     sc_m.add_val_to(
@@ -501,7 +485,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                 return ClapError::unnecessary_double_dash(
                     self.app,
                     arg_os.to_str_lossy().into_owned(),
-                    Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                    Usage::new(self.app).create_usage_with_title(&[]),
                 );
             }
         }
@@ -522,7 +506,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                     .as_ref()
                     .unwrap_or(&self.app.name)
                     .to_string(),
-                Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+                Usage::new(self.app).create_usage_with_title(&[]),
             );
         }
         // If the argument must be a subcommand.
@@ -542,7 +526,7 @@ impl<'help, 'app> Parser<'help, 'app> {
             self.app,
             arg_os.to_str_lossy().into_owned(),
             None,
-            Usage::new(self.app, &self.required).create_usage_with_title(&[]),
+            Usage::new(self.app).create_usage_with_title(&[]),
         )
     }
 
@@ -687,57 +671,9 @@ impl<'help, 'app> Parser<'help, 'app> {
     ) -> ClapResult<()> {
         debug!("Parser::parse_subcommand");
 
-        let mut mid_string = String::from(" ");
-
-        if !self.app.is_subcommand_negates_reqs_set() {
-            let reqs =
-                Usage::new(self.app, &self.required).get_required_usage_from(&[], None, true); // maybe Some(m)
-
-            for s in &reqs {
-                mid_string.push_str(s);
-                mid_string.push(' ');
-            }
-        }
-
         let partial_parsing_enabled = self.app.is_ignore_errors_set();
 
-        if let Some(sc) = self.app.subcommands.iter_mut().find(|s| s.name == sc_name) {
+        if let Some(sc) = self.app._build_subcommand(sc_name) {
-            // Display subcommand name, short and long in usage
-            let mut sc_names = sc.name.clone();
-            let mut flag_subcmd = false;
-            if let Some(l) = sc.long_flag {
-                write!(sc_names, ", --{}", l).unwrap();
-                flag_subcmd = true;
-            }
-            if let Some(s) = sc.short_flag {
-                write!(sc_names, ", -{}", s).unwrap();
-                flag_subcmd = true;
-            }
-
-            if flag_subcmd {
-                sc_names = format!("{{{}}}", sc_names);
-            }
-
-            sc.usage = Some(
-                self.app
-                    .bin_name
-                    .as_ref()
-                    .map(|bin_name| format!("{}{}{}", bin_name, mid_string, sc_names))
-                    .unwrap_or(sc_names),
-            );
-
-            // bin_name should be parent's bin_name + [<reqs>] + the sc's name separated by
-            // a space
-            sc.bin_name = Some(format!(
-                "{}{}{}",
-                self.app.bin_name.as_ref().unwrap_or(&String::new()),
-                if self.app.bin_name.is_some() { " " } else { "" },
-                &*sc.name
-            ));
-
-            // Ensure all args are built and ready to parse
-            sc._build();
-
             let mut sc_matcher = ArgMatcher::new(sc);
 
             debug!("Parser::parse_subcommand: About to parse sc={}", sc.name);
@@ -919,13 +855,14 @@ impl<'help, 'app> Parser<'help, 'app> {
                 );
                 self.parse_opt(val, opt, matcher, trailing_values)
             } else if let Some(rest) = val {
+                let required = self.app.required_graph();
                 debug!("Parser::parse_long_arg: Got invalid literal `{:?}`", rest);
                 let used: Vec<Id> = matcher
                     .arg_names()
                     .filter(|&n| {
-                        self.app.find(n).map_or(true, |a| {
+                        self.app
-                            !(a.is_hide_set() || self.required.contains(&a.id))
+                            .find(n)
-                        })
+                            .map_or(true, |a| !(a.is_hide_set() || required.contains(&a.id)))
                     })
                     .cloned()
                     .collect();
@@ -1520,12 +1457,13 @@ impl<'help, 'app> Parser<'help, 'app> {
             }
         }
 
+        let required = self.app.required_graph();
         let used: Vec<Id> = matcher
             .arg_names()
             .filter(|n| {
-                self.app.find(n).map_or(true, |a| {
+                self.app
-                    !(self.required.contains(&a.id) || a.is_hide_set())
+                    .find(n)
-                })
+                    .map_or(true, |a| !(required.contains(&a.id) || a.is_hide_set()))
             })
             .cloned()
             .collect();
@@ -1534,12 +1472,14 @@ impl<'help, 'app> Parser<'help, 'app> {
             self.app,
             format!("--{}", arg),
             did_you_mean,
-            Usage::new(self.app, &self.required).create_usage_with_title(&*used),
+            Usage::new(self.app)
+                .required(&required)
+                .create_usage_with_title(&*used),
         )
     }
 
     pub(crate) fn write_help_err(&self) -> ClapResult<Colorizer> {
-        let usage = Usage::new(self.app, &self.required);
+        let usage = Usage::new(self.app);
         let mut c = Colorizer::new(true, self.color_help());
         Help::new(HelpWriter::Buffer(&mut c), self.app, &usage, false).write_help()?;
         Ok(c)
@@ -1552,7 +1492,7 @@ impl<'help, 'app> Parser<'help, 'app> {
         );
 
         use_long = use_long && self.use_long_help();
-        let usage = Usage::new(self.app, &self.required);
+        let usage = Usage::new(self.app);
         let mut c = Colorizer::new(false, self.color_help());
 
         match Help::new(HelpWriter::Buffer(&mut c), self.app, &usage, use_long).write_help() {

src/parse/validator.rs

@@ -3,16 +3,19 @@ use crate::build::{App, AppSettings, Arg, ArgPredicate, PossibleValue};
 use crate::error::{Error, Result as ClapResult};
 use crate::output::Usage;
 use crate::parse::{ArgMatcher, MatchedArg, ParseState, Parser};
+use crate::util::ChildGraph;
 use crate::util::Id;
 use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
 
 pub(crate) struct Validator<'help, 'app, 'parser> {
     p: &'parser mut Parser<'help, 'app>,
+    required: ChildGraph<Id>,
 }
 
 impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
     pub(crate) fn new(p: &'parser mut Parser<'help, 'app>) -> Self {
-        Validator { p }
+        let required = p.app.required_graph();
+        Validator { p, required }
     }
 
     pub(crate) fn validate(
@@ -46,7 +49,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                         .filter_map(PossibleValue::get_visible_name)
                         .collect::<Vec<_>>(),
                     o,
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
         }
@@ -67,7 +72,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
             return Err(Error::missing_subcommand(
                 self.p.app,
                 bn.to_string(),
-                Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                Usage::new(self.p.app)
+                    .required(&self.required)
+                    .create_usage_with_title(&[]),
             ));
         } else if !has_subcmd && self.p.app.is_set(AppSettings::SubcommandRequiredElseHelp) {
             debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true");
@@ -99,7 +106,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                 );
                 return Err(Error::invalid_utf8(
                     self.p.app,
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
             if !arg.possible_vals.is_empty() {
@@ -118,7 +127,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                         .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
                         .filter(|&n| {
                             self.p.app.find(n).map_or(true, |a| {
-                                !(a.is_hide_set() || self.p.required.contains(&a.id))
+                                !(a.is_hide_set() || self.required.contains(&a.id))
                             })
                         })
                         .cloned()
@@ -131,7 +140,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                             .filter_map(PossibleValue::get_visible_name)
                             .collect::<Vec<_>>(),
                         arg,
-                        Usage::new(self.p.app, &self.p.required).create_usage_with_title(&used),
+                        Usage::new(self.p.app)
+                            .required(&self.required)
+                            .create_usage_with_title(&used),
                     ));
                 }
             }
@@ -144,7 +155,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                         .filter_map(PossibleValue::get_visible_name)
                         .collect::<Vec<_>>(),
                     arg,
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
 
@@ -221,7 +234,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                     self.p.app,
                     arg,
                     Vec::new(),
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ))
             })
     }
@@ -277,7 +292,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
             .chain(used_filtered.iter())
             .cloned()
             .collect();
-        Usage::new(self.p.app, &self.p.required).create_usage_with_title(&required)
+        Usage::new(self.p.app)
+            .required(&self.required)
+            .create_usage_with_title(&required)
     }
 
     fn gather_requires(&mut self, matcher: &ArgMatcher) {
@@ -294,12 +311,12 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                 };
 
                 for req in self.p.app.unroll_arg_requires(is_relevant, &arg.id) {
-                    self.p.required.insert(req);
+                    self.required.insert(req);
                 }
             } else if let Some(g) = self.p.app.find_group(name) {
                 debug!("Validator::gather_requires:iter:{:?}:group", name);
                 for r in &g.requires {
-                    self.p.required.insert(r.clone());
+                    self.required.insert(r.clone());
                 }
             }
         }
@@ -335,7 +352,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
             return Err(Error::unexpected_multiple_usage(
                 self.p.app,
                 a,
-                Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                Usage::new(self.p.app)
+                    .required(&self.required)
+                    .create_usage_with_title(&[]),
             ));
         }
         if let Some(max_occurs) = a.max_occurs {
@@ -350,7 +369,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                     a,
                     max_occurs,
                     occurs,
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
         }
@@ -379,7 +400,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                     } else {
                         total_num
                     },
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
         }
@@ -396,7 +419,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                         .expect(INVALID_UTF8)
                         .to_string(),
                     a.to_string(),
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
         }
@@ -409,7 +434,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                     a,
                     num,
                     ma.num_vals(),
-                    Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                    Usage::new(self.p.app)
+                        .required(&self.required)
+                        .create_usage_with_title(&[]),
                 ));
             }
             num == 0
@@ -426,20 +453,19 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                     .filter_map(PossibleValue::get_visible_name)
                     .collect::<Vec<_>>(),
                 a,
-                Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+                Usage::new(self.p.app)
+                    .required(&self.required)
+                    .create_usage_with_title(&[]),
             ));
         }
         Ok(())
     }
 
     fn validate_required(&mut self, matcher: &ArgMatcher) -> ClapResult<()> {
-        debug!(
+        debug!("Validator::validate_required: required={:?}", self.required);
-            "Validator::validate_required: required={:?}",
-            self.p.required
-        );
         self.gather_requires(matcher);
 
-        for arg_or_group in self.p.required.iter().filter(|r| !matcher.contains(r)) {
+        for arg_or_group in self.required.iter().filter(|r| !matcher.contains(r)) {
             debug!("Validator::validate_required:iter:aog={:?}", arg_or_group);
             if let Some(arg) = self.p.app.find(arg_or_group) {
                 debug!("Validator::validate_required:iter: This is an arg");
@@ -535,10 +561,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
         debug!("Validator::missing_required_error; incl={:?}", incl);
         debug!(
             "Validator::missing_required_error: reqs={:?}",
-            self.p.required
+            self.required
         );
 
-        let usg = Usage::new(self.p.app, &self.p.required);
+        let usg = Usage::new(self.p.app).required(&self.required);
 
         let req_args = usg.get_required_usage_from(&incl, Some(matcher), true);
 
@@ -552,9 +578,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
             .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
             .filter(|n| {
                 // Filter out the args we don't want to specify.
-                self.p.app.find(n).map_or(true, |a| {
+                self.p
-                    !a.is_hide_set() && !self.p.required.contains(&a.id)
+                    .app
-                })
+                    .find(n)
+                    .map_or(true, |a| !a.is_hide_set() && !self.required.contains(&a.id))
             })
             .cloned()
             .chain(incl)