From a37e637e0766bb43f1068479c7c8ffd427037df3 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Wed, 19 Oct 2016 18:54:36 -0400 Subject: [PATCH] fix: fixes a bug that made determining when to auto-wrap long help messages inconsistent Closes #688 --- src/app/help.rs | 120 ++++++++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 61 deletions(-) diff --git a/src/app/help.rs b/src/app/help.rs index 0b22a277d98b..81f8d4b4f6a9 100644 --- a/src/app/help.rs +++ b/src/app/help.rs @@ -84,6 +84,8 @@ pub struct Help<'a> { term_w: usize, color: bool, cizer: Colorizer, + longest: usize, + force_next_line: bool, } // Public Functions @@ -111,6 +113,8 @@ impl<'a> Help<'a> { }, color: color, cizer: cizer, + longest: 0, + force_next_line: false, } } @@ -169,13 +173,13 @@ impl<'a> Help<'a> { fn write_args_unsorted<'b: 'd, 'c: 'd, 'd, I: 'd>(&mut self, args: I) -> io::Result<()> where I: Iterator> { - let mut longest = 0; + self.longest = 0; let mut arg_v = Vec::with_capacity(10); for arg in args.filter(|arg| { !(arg.is_set(ArgSettings::Hidden)) || arg.is_set(ArgSettings::NextLineHelp) }) { if arg.longest_filter() { - longest = cmp::max(longest, arg.to_string().len()); + self.longest = cmp::max(self.longest, arg.to_string().len()); } if !arg.is_set(ArgSettings::Hidden) { arg_v.push(arg) @@ -188,7 +192,7 @@ impl<'a> Help<'a> { } else { try!(self.writer.write(b"\n")); } - try!(self.write_arg(arg.as_base(), longest)); + try!(self.write_arg(arg.as_base())); } Ok(()) } @@ -198,13 +202,13 @@ impl<'a> Help<'a> { where I: Iterator> { debugln!("fn=write_args;"); - let mut longest = 0; + self.longest = 0; let mut ord_m = VecMap::new(); for arg in args.filter(|arg| { !(arg.is_set(ArgSettings::Hidden)) || arg.is_set(ArgSettings::NextLineHelp) }) { if arg.longest_filter() { - longest = cmp::max(longest, arg.to_string().len()); + self.longest = cmp::max(self.longest, arg.to_string().len()); } if !arg.is_set(ArgSettings::Hidden) { let btm = ord_m.entry(arg.disp_ord()).or_insert(BTreeMap::new()); @@ -219,7 +223,7 @@ impl<'a> Help<'a> { } else { try!(self.writer.write(b"\n")); } - try!(self.write_arg(arg.as_base(), longest)); + try!(self.write_arg(arg.as_base())); } } Ok(()) @@ -227,14 +231,13 @@ impl<'a> Help<'a> { /// Writes help for an argument to the wrapped stream. fn write_arg<'b, 'c>(&mut self, - arg: &ArgWithDisplay<'b, 'c>, - longest: usize) + arg: &ArgWithDisplay<'b, 'c>) -> io::Result<()> { debugln!("fn=write_arg;"); try!(self.short(arg)); - try!(self.long(arg, longest)); - try!(self.val(arg, longest)); - try!(self.help(arg, longest)); + try!(self.long(arg)); + let spec_vals = try!(self.val(arg)); + try!(self.help(arg, &*spec_vals)); Ok(()) } @@ -252,7 +255,7 @@ impl<'a> Help<'a> { } /// Writes argument's long command to the wrapped stream. - fn long<'b, 'c>(&mut self, arg: &ArgWithDisplay<'b, 'c>, longest: usize) -> io::Result<()> { + fn long<'b, 'c>(&mut self, arg: &ArgWithDisplay<'b, 'c>) -> io::Result<()> { debugln!("fn=long;"); if !arg.has_switch() { return Ok(()); @@ -271,20 +274,20 @@ impl<'a> Help<'a> { } try!(color!(self, "--{}", l, good)); if !self.next_line_help || !arg.is_set(ArgSettings::NextLineHelp) { - write_nspaces!(self.writer, (longest + 4) - (l.len() + 2)); + write_nspaces!(self.writer, (self.longest + 4) - (l.len() + 2)); } } else if !self.next_line_help || !arg.is_set(ArgSettings::NextLineHelp) { // 6 is tab (4) + -- (2) - write_nspaces!(self.writer, (longest + 6)); + write_nspaces!(self.writer, (self.longest + 6)); } Ok(()) } /// Writes argument's possible values to the wrapped stream. - fn val<'b, 'c>(&mut self, arg: &ArgWithDisplay<'b, 'c>, longest: usize) -> io::Result<()> { + fn val<'b, 'c>(&mut self, arg: &ArgWithDisplay<'b, 'c>) -> Result { debugln!("fn=val;"); if !arg.takes_value() { - return Ok(()); + return Ok(String::new()); } if let Some(vec) = arg.val_names() { let mut it = vec.iter().peekable(); @@ -314,18 +317,26 @@ impl<'a> Help<'a> { let spec_vals = self.spec_vals(arg); let h = arg.help().unwrap_or(""); + let h_w = str_width(h) + str_width(&*spec_vals); let nlh = self.next_line_help || arg.is_set(ArgSettings::NextLineHelp); - let width = self.term_w; - let taken = (longest + 12) + str_width(&*spec_vals); - let force_next_line = !nlh && width >= taken && - (taken as f32 / width as f32) > 0.40 && - str_width(h) > (width - taken); + let taken = self.longest + 12; + self.force_next_line = !nlh && self.term_w >= taken && + (taken as f32 / self.term_w as f32) > 0.40 && + h_w > (self.term_w - taken); + debug!("Has switch..."); if arg.has_switch() { - if !(nlh || force_next_line) { + sdebugln!("Yes"); + debugln!("force_next_line...{:?}", self.force_next_line); + debugln!("nlh...{:?}", nlh); + debugln!("taken...{}", taken); + debugln!("help_width > (width - taken)...{} > ({} - {})", h_w, self.term_w, taken); + debug!("next_line..."); + if !(nlh || self.force_next_line) { + sdebugln!("No"); let self_len = arg.to_string().len(); // subtract ourself - let mut spcs = longest - self_len; + let mut spcs = self.longest - self_len; // Since we're writing spaces from the tab point we first need to know if we // had a long and short, or just short if arg.long().is_some() { @@ -337,20 +348,24 @@ impl<'a> Help<'a> { } write_nspaces!(self.writer, spcs); + } else { + sdebugln!("Yes"); } - } else if !(nlh || force_next_line) { - write_nspaces!(self.writer, longest + 4 - (arg.to_string().len())); + } else if !(nlh || self.force_next_line) { + sdebugln!("No, and not next_line"); + write_nspaces!(self.writer, self.longest + 4 - (arg.to_string().len())); + } else { + sdebugln!("No"); } - Ok(()) + Ok(spec_vals) } fn write_before_after_help(&mut self, h: &str) -> io::Result<()> { debugln!("fn=before_help;"); let mut help = String::new(); // determine if our help fits or needs to wrap - let width = self.term_w; - debugln!("Term width...{}", width); - let too_long = str_width(h) >= width; + debugln!("Term width...{}", self.term_w); + let too_long = str_width(h) >= self.term_w; debug!("Too long..."); if too_long || h.contains("{n}") { @@ -359,7 +374,7 @@ impl<'a> Help<'a> { debugln!("help: {}", help); debugln!("help width: {}", str_width(&*help)); // Determine how many newlines we need to insert - debugln!("Usable space: {}", width); + debugln!("Usable space: {}", self.term_w); let longest_w = { let mut lw = 0; for l in help.split(' ').map(|s| str_width(s)) { @@ -370,7 +385,7 @@ impl<'a> Help<'a> { lw }; help = help.replace("{n}", "\n"); - wrap_help(&mut help, longest_w, width); + wrap_help(&mut help, longest_w, self.term_w); } else { sdebugln!("No"); } @@ -394,52 +409,36 @@ impl<'a> Help<'a> { } /// Writes argument's help to the wrapped stream. - fn help<'b, 'c>(&mut self, arg: &ArgWithDisplay<'b, 'c>, longest: usize) -> io::Result<()> { + fn help<'b, 'c>(&mut self, arg: &ArgWithDisplay<'b, 'c>, spec_vals: &str) -> io::Result<()> { debugln!("fn=help;"); - let spec_vals = self.spec_vals(arg); let mut help = String::new(); let h = arg.help().unwrap_or(""); let nlh = self.next_line_help || arg.is_set(ArgSettings::NextLineHelp); debugln!("Next Line...{:?}", nlh); - // determine if our help fits or needs to wrap - let width = self.term_w; - debugln!("Term width...{}", width); - - // We calculate with longest+12 since if it's already NLH we don't care - let taken = (longest + 12) + str_width(&*spec_vals); - let force_next_line = !nlh && width >= taken && - (taken as f32 / width as f32) > 0.40 && - str_width(h) > (width - taken); - debugln!("Force Next Line...{:?}", force_next_line); - debugln!("Force Next Line math (help_len > (width - flags/opts/spcs))...{} > ({} - {})", - str_width(h), - width, - taken); - - let spcs = if nlh || force_next_line { + let spcs = if nlh || self.force_next_line { 12 // "tab" * 3 } else { - longest + 12 + self.longest + 12 }; - let too_long = spcs + str_width(h) + str_width(&*spec_vals) >= width; + let too_long = spcs + str_width(h) + str_width(&*spec_vals) >= self.term_w; debugln!("Spaces: {}", spcs); // Is help on next line, if so then indent - if nlh || force_next_line { + if nlh || self.force_next_line { try!(write!(self.writer, "\n{}{}{}", TAB, TAB, TAB)); } debug!("Too long..."); - if too_long && spcs <= width || h.contains("{n}") { + if too_long && spcs <= self.term_w || h.contains("{n}") { sdebugln!("Yes"); help.push_str(h); help.push_str(&*spec_vals); debugln!("help: {}", help); debugln!("help width: {}", str_width(&*help)); // Determine how many newlines we need to insert - let avail_chars = width - spcs; + let avail_chars = self.term_w - spcs; debugln!("Usable space: {}", avail_chars); let longest_w = { let mut lw = 0; @@ -470,12 +469,12 @@ impl<'a> Help<'a> { } for part in help.lines().skip(1) { try!(write!(self.writer, "\n")); - if nlh || force_next_line { + if nlh || self.force_next_line { try!(write!(self.writer, "{}{}{}", TAB, TAB, TAB)); } else if arg.has_switch() { - write_nspaces!(self.writer, longest + 12); + write_nspaces!(self.writer, self.longest + 12); } else { - write_nspaces!(self.writer, longest + 8); + write_nspaces!(self.writer, self.longest + 8); } try!(write!(self.writer, "{}", part)); } @@ -606,12 +605,11 @@ impl<'a> Help<'a> { /// Writes help for subcommands of a Parser Object to the wrapped stream. fn write_subcommands(&mut self, parser: &Parser) -> io::Result<()> { debugln!("fn=write_subcommands;"); - let mut longest = 0; - + self.longest = 0; let mut ord_m = VecMap::new(); for sc in parser.subcommands.iter().filter(|s| !s.p.is_set(AppSettings::Hidden)) { let btm = ord_m.entry(sc.p.meta.disp_ord).or_insert(BTreeMap::new()); - longest = cmp::max(longest, sc.p.meta.name.len()); + self.longest = cmp::max(self.longest, sc.p.meta.name.len()); btm.insert(sc.p.meta.name.clone(), sc.clone()); } @@ -623,7 +621,7 @@ impl<'a> Help<'a> { } else { try!(self.writer.write(b"\n")); } - try!(self.write_arg(sc, longest)); + try!(self.write_arg(sc)); } } Ok(())