Repository: pacak/bpaf
Pull request #340 (2024-02-04 09:55:19)

make `max_width` of help configurable
`bpaf` currently applies column wrapping at a hardcoded width of [100](https://github.com/pacak/bpaf/blob/a86daeca198254d1beae6a37c8c012e1df160c11/src/buffer/console.rs#L42) characters.

This PR aims to make this width configurable via the option parser.
Additionally it contributes a fix to the breaking logic where earlier only the current cursor position was used to determine, resulting in the trailing word overflowing the set width of 100.

ysndr: Draft, because I mean to add derive support and docs, but I'd like to gather a response to the general approach, before spending more time on it.
pacak: I'm not opposed to making it configurable even though I'm curious about the motivation.

Implementation looks about right.
ysndr: For once 100 characters might not be universally agreed upon (terminals commonly defaulting to 80, ssh reporting a term width of 80, etc).
Proposing to change bpafs default to another opinionated value seemed hypocritical.
Additionally, providing a config like this allows to adapt to the reported terminal width as well if so desired by the calling application.
pacak: Let me know if you need any help with that change :)
ysndr: Alright, derive support is in as well now, and fixed the tests that broke due to including the next word width in the breaking decision.

pacak: CI failure is not related to your changes. I guess I need to decide how much effort I'm willing to put into supporting rustc 1.56... Will try to fix it tomorrow-ish.

Patch:
diff --git a/bpaf_derive/src/td.rs b/bpaf_derive/src/td.rs
index 741e4a6..28ce23b 100644
--- a/bpaf_derive/src/td.rs
+++ b/bpaf_derive/src/td.rs
@@ -29,6 +29,7 @@ pub(crate) struct OptionsCfg {
     pub(crate) header: Option<Help>,
     pub(crate) usage: Option<Box<Expr>>,
     pub(crate) version: Option<Box<Expr>>,
+    pub(crate) max_width: Option<Box<Expr>>,
 }
 
 #[derive(Debug, Default)]
@@ -239,6 +240,9 @@ impl Parse for TopInfo {
             } else if kw == "help" {
                 let help = parse_arg(input)?;
                 with_command(&kw, command.as_mut(), |cfg| cfg.help = Some(help))?;
+            } else if kw == "max_width" {
+                let max_width = parse_arg(input)?;
+                with_options(&kw, options.as_mut(), |opt| opt.max_width = Some(max_width))?;
             } else if let Some(pd) = PostDecor::parse(input, &kw)? {
                 attrs.push(pd);
             } else {
diff --git a/bpaf_derive/src/top.rs b/bpaf_derive/src/top.rs
index a8f3f92..94063db 100644
--- a/bpaf_derive/src/top.rs
+++ b/bpaf_derive/src/top.rs
@@ -190,6 +190,7 @@ impl ToTokens for Top {
                     descr,
                     footer,
                     header,
+                    max_width,
                 } = options;
 
                 let version = version.as_ref().map(|v| quote!(.version(#v)));
@@ -197,6 +198,7 @@ impl ToTokens for Top {
                 let descr = descr.as_ref().map(|v| quote!(.descr(#v)));
                 let footer = footer.as_ref().map(|v| quote!(.footer(#v)));
                 let header = header.as_ref().map(|v| quote!(.header(#v)));
+                let max_width = max_width.as_ref().map(|v| quote!(.max_width(#v)));
 
                 let CommandCfg {
                     name,
@@ -221,6 +223,7 @@ impl ToTokens for Top {
                         #header
                         #footer
                         #usage
+                        #max_width
                         .command(#name)
                         #(#short)*
                         #(#long)*
@@ -238,6 +241,7 @@ impl ToTokens for Top {
                     descr,
                     footer,
                     header,
+                    max_width
                 } = options;
                 let body = match cargo_helper {
                     Some(cargo) => quote!(::bpaf::cargo_helper(#cargo, #body)),
@@ -249,6 +253,8 @@ impl ToTokens for Top {
                 let descr = descr.as_ref().map(|v| quote!(.descr(#v)));
                 let footer = footer.as_ref().map(|v| quote!(.footer(#v)));
                 let header = header.as_ref().map(|v| quote!(.header(#v)));
+                let max_width = max_width.as_ref().map(|v| quote!(.max_width(#v)));
+
                 quote! {
                     #vis fn #generate() -> ::bpaf::OptionParser<#ty> {
                         #[allow(unused_imports)]
@@ -261,6 +267,7 @@ impl ToTokens for Top {
                         #header
                         #footer
                         #usage
+                        #max_width
                     }
                 }
             }
diff --git a/src/buffer.rs b/src/buffer.rs
index d5e651d..00d6ed9 100644
--- a/src/buffer.rs
+++ b/src/buffer.rs
@@ -15,6 +15,7 @@ mod manpage;
 mod splitter;
 
 pub(crate) use self::console::Color;
+
 #[cfg(feature = "docgen")]
 pub use manpage::Section;
 
diff --git a/src/buffer/console.rs b/src/buffer/console.rs
index f87dc50..99cd003 100644
--- a/src/buffer/console.rs
+++ b/src/buffer/console.rs
@@ -39,7 +39,7 @@ use super::{
 use super::Style;
 
 const MAX_TAB: usize = 24;
-const MAX_WIDTH: usize = 100;
+pub(crate) const MAX_WIDTH: usize = 100;
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 /// Default to dull color if colors are enabled,
@@ -133,11 +133,11 @@ impl Doc {
     /// difference for rendered help message, otherwise you can pass `true`.
     #[must_use]
     pub fn monochrome(&self, full: bool) -> String {
-        self.render_console(full, Color::Monochrome)
+        self.render_console(full, Color::Monochrome, MAX_WIDTH)
     }
 
     #[allow(clippy::too_many_lines)] // it's a big ass match statement
-    pub(crate) fn render_console(&self, full: bool, color: Color) -> String {
+    pub(crate) fn render_console(&self, full: bool, color: Color, max_width: usize) -> String {
         let mut res = String::new();
         let mut tabstop = 0;
         let mut byte_pos = 0;
@@ -208,7 +208,7 @@ impl Doc {
                                     if pending_blank_line && !res.ends_with("\n\n") {
                                         res.push('\n');
                                     }
-                                    if char_pos > MAX_WIDTH {
+                                    if char_pos + s.len() > max_width {
                                         char_pos = 0;
                                         res.truncate(res.trim_end().len());
                                         res.push('\n');
diff --git a/src/error.rs b/src/error.rs
index 7535a3f..9193dff 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -3,8 +3,7 @@ use std::ops::Range;
 use crate::{
     args::{Arg, State},
     buffer::{Block, Color, Doc, Style, Token},
-    item::Item,
-    item::ShortLong,
+    item::{Item, ShortLong},
     meta_help::Metavar,
     meta_youmean::{Suggestion, Variant},
     Meta,
@@ -202,20 +201,24 @@ impl ParseFailure {
         }
     }
 
-    /// Run an action appropriate to the failure and produce the exit code
-    ///
-    /// Prints a message to `stdout` or `stderr` and returns the exit code
+    /// Returns the exit code for the failure
     #[allow(clippy::must_use_candidate)]
     pub fn exit_code(self) -> i32 {
+        match self {
+            Self::Stdout(..) | Self::Completion(..) => 0,
+            Self::Stderr(..) => 1,
+        }
+    }
+
+    /// Prints a message to `stdout` or `stderr` appropriate to the failure.
+    pub fn print_mesage(&self, max_width: usize) {
         let color = Color::default();
         match self {
             ParseFailure::Stdout(msg, full) => {
-                println!("{}", msg.render_console(full, color));
-                0
+                println!("{}", msg.render_console(*full, color, max_width));
             }
             ParseFailure::Completion(s) => {
                 print!("{}", s);
-                0
             }
             ParseFailure::Stderr(msg) => {
                 #[allow(unused_mut)]
@@ -231,8 +234,7 @@ impl ParseFailure {
                     color.push_str(Style::Invalid, &mut error, "Error: ");
                 }
 
-                eprintln!("{}{}", error, msg.render_console(true, color));
-                1
+                eprintln!("{}{}", error, msg.render_console(true, color, max_width));
             }
         }
     }
diff --git a/src/info.rs b/src/info.rs
index af98a15..5cdf2ba 100644
--- a/src/info.rs
+++ b/src/info.rs
@@ -27,6 +27,7 @@ pub struct Info {
     pub help_arg: NamedArg,
     pub version_arg: NamedArg,
     pub help_if_no_args: bool,
+    pub max_width: usize,
 }
 
 impl Default for Info {
@@ -42,6 +43,7 @@ impl Default for Info {
                 .long("version")
                 .help("Prints version information"),
             help_if_no_args: false,
+            max_width: 100,
         }
     }
 }
@@ -87,7 +89,10 @@ impl<T> OptionParser<T> {
     {
         match self.run_inner(Args::current_args()) {
             Ok(t) => t,
-            Err(err) => std::process::exit(err.exit_code()),
+            Err(err) => {
+                err.print_mesage(self.info.max_width);
+                std::process::exit(err.exit_code())
+            }
         }
     }
 
@@ -647,6 +652,19 @@ impl<T> OptionParser<T> {
         self.info.help_if_no_args = true;
         self
     }
+
+    /// Set the width of the help message printed to the terminal upon failure
+    ///
+    /// By default, the help message is printed with a width of 100 characters.
+    /// This method allows to change where the help message is wrapped.
+    ///
+    /// Setting the max width too low may negatively affect the readability of the help message.
+    /// Also, the alignment padding of broken lines is always applied.
+    #[must_use]
+    pub fn max_width(mut self, width: usize) -> Self {
+        self.info.max_width = width;
+        self
+    }
 }
 
 impl Info {


Test patch:
diff --git a/tests/errors.rs b/tests/errors.rs
index 4ee4fc9..9337a07 100644
--- a/tests/errors.rs
+++ b/tests/errors.rs
@@ -181,7 +181,7 @@ fn should_not_split_adjacent_ambig_options() {
     assert_eq!(r, expected);
 
     let r = parser.run_inner(&["-ahello"]).unwrap_err().unwrap_stderr();
-    let expected = "app supports `-a` as both an option and an option-argument, try to split `-ahello` into individual options\n(-a -h ..) or use `-a=hello` syntax to disambiguate";
+    let expected = "app supports `-a` as both an option and an option-argument, try to split `-ahello` into individual\noptions (-a -h ..) or use `-a=hello` syntax to disambiguate";
     assert_eq!(r, expected);
 
     // this one is okay, try to parse -a as argument - it fails because "hello" is not a number, then
diff --git a/tests/meta_youmean.rs b/tests/meta_youmean.rs
index cacc6a0..cf439de 100644
--- a/tests/meta_youmean.rs
+++ b/tests/meta_youmean.rs
@@ -15,7 +15,7 @@ fn ambiguity() {
     let parser = construct!([a0, a1]).to_options();
 
     let r = parser.run_inner(&["-aaaaaa"]).unwrap_err().unwrap_stderr();
-    assert_eq!(r, "app supports `-a` as both an option and an option-argument, try to split `-aaaaaa` into individual options\n(-a -a ..) or use `-a=aaaaa` syntax to disambiguate");
+    assert_eq!(r, "app supports `-a` as both an option and an option-argument, try to split `-aaaaaa` into individual\noptions (-a -a ..) or use `-a=aaaaa` syntax to disambiguate");
 
     let r = parser.run_inner(&["-b"]).unwrap_err().unwrap_stderr();
     // single char typos are too random
diff --git a/tests/tests.rs b/tests/tests.rs
index 8226fde..87702c6 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1441,7 +1441,7 @@ fn suggestion_for_equals_1() {
         .unwrap_stderr();
     assert_eq!(
         r,
-        "`--par` requires an argument `P`, got a flag `--bar=baz`, try `--par=--bar=baz` to use it as an argument"
+        "`--par` requires an argument `P`, got a flag `--bar=baz`, try `--par=--bar=baz` to use it as an\nargument"
     );
 }