From e71c1d51040567f1b8b5fc5ceb73b935d21f7fac Mon Sep 17 00:00:00 2001 From: Mikkel Kjeldsen Date: Sun, 1 Oct 2023 16:37:40 +0200 Subject: [PATCH] Ignore overridden --width args It is a common pattern to allow a command line option to be specified multiple times, later occurrences taking precedence, such that command lines can be easily but safely extended just by appending. Although this pattern has no obvious utility to commitmsgfmt, neither does rejecting it serve any useful purpose at all, while at the same time making a specification of the CLI more complex. Remove the incidental constraint that the "--width" option cannot be repeated. Incidentally, that constraint already does not apply to the generated "-h" and "-V" options so now the CLI is internally consistent. --- CHANGELOG.md | 4 ++ doc/commitmsgfmt.1.adoc | 3 +- src/main.rs | 120 +++++++++++++++++++++++++++++++++++++--- 3 files changed, 118 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19fdc44..2016df9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ `commitmsgfmt` formats commit messages. It reflows and wraps text, with special understanding of patterns often seen in commit messages. +## Unreleased + +- If `--width` is specified multiple times, ignore all but the last occurrence. + ## 1.5.0 - 2022-07-30 - Fix an edge case where footnote references followed by punctuation would diff --git a/doc/commitmsgfmt.1.adoc b/doc/commitmsgfmt.1.adoc index 1b4c41c..13f1ce5 100644 --- a/doc/commitmsgfmt.1.adoc +++ b/doc/commitmsgfmt.1.adoc @@ -49,7 +49,8 @@ Output version information and exit. *-w*, *--width* _NUM_:: -Specify the max allowed width of body text. Default 72. +Specify the max allowed width of body text. Default 72. If specified multiple +times only the last occurrence is used. == Details diff --git a/src/main.rs b/src/main.rs index d4c6d6d..2ee3b56 100644 --- a/src/main.rs +++ b/src/main.rs @@ -80,7 +80,12 @@ impl Config { fn new(m: &ArgMatches) -> CliResult { use std::str::FromStr; - let width = m.value_of("width").map(i32::from_str).unwrap()?; + let width = m + .values_of("width") + .unwrap() + .last() + .map(i32::from_str) + .unwrap()?; if width < 1 { return Err(CliError::ArgWidthOutOfBounds(width)); @@ -123,6 +128,8 @@ fn main() { .short("w") .long("width") .takes_value(true) + .number_of_values(1) + .multiple(true) .default_value("72") .hide_default_value(true) .help("The message body max paragraph width. Default: 72.") @@ -214,8 +221,8 @@ mod tests { use std::process::Command; use std::process::Stdio; - fn cargo_run_cmd(w: &str) -> Vec { - let mut cmd: Vec = Vec::with_capacity(8); + fn cargo_run_cmd() -> Vec { + let mut cmd: Vec = Vec::with_capacity(6); cmd.push("cargo".to_owned()); cmd.push("run".to_owned()); cmd.push("--quiet".to_owned()); @@ -226,20 +233,26 @@ mod tests { } cmd.push("--".to_owned()); - cmd.push("--width".to_owned()); - cmd.push(w.to_owned()); cmd } - fn target_binary_with_width(w: &str) -> Command { - let cargo_run = cargo_run_cmd(w); + fn target_binary() -> Command { + let cargo_run = cargo_run_cmd(); let mut cmd = Command::new(&cargo_run[0]); cmd.args(&cargo_run[1..]); cmd } + fn target_binary_with_width(w: &str) -> Command { + let mut cmd = target_binary(); + cmd.arg("--width"); + cmd.arg(w); + + cmd + } + #[test] fn to_utf8_converts_utf_8_bytes_to_utf_8() { let some_utf_8_str = "å"; @@ -295,6 +308,19 @@ mod tests { assert!(err.contains("invalid digit")); } + #[test] + fn width_with_multiple_values_exits_with_code_1() { + let mut cmd = target_binary(); + cmd.args(&["-w", "1", "2"]); + + let output = cmd.output().expect("run debug binary"); + + assert_eq!(1, output.status.code().unwrap()); + + let err = String::from_utf8_lossy(&output.stderr); + assert!(err.contains(r#"Found argument '2'"#)); + } + #[test] fn width_1_wraps_body_at_width_1_and_exits_successfully() { use std::io::Write; @@ -331,6 +357,81 @@ y assert!(err.is_empty()); } + #[test] + fn width_short_form_is_w() { + use std::io::Write; + + let mut cmd = target_binary(); + cmd.args(&["-w", "1"]); + let mut cmd: Child = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .expect("spawn debug binary"); + + cmd.stdin + .as_mut() + .expect("child stdin") + .write_all(b"subject\nb o d y") + .expect("write to child stdin"); + + let output = cmd.wait_with_output().expect("run debug binary"); + + assert!(output.status.success()); + + let out = String::from_utf8_lossy(&output.stdout); + assert_eq!( + "subject + +b +o +d +y +", + out + ); + + let err = String::from_utf8_lossy(&output.stderr); + assert!(err.is_empty()); + } + + #[test] + fn only_last_specified_width_matters() { + use std::io::Write; + + let mut cmd = target_binary_with_width("string"); + cmd.args(&["-w", "1"]); + cmd.args(&["-w", "100"]); + + let mut cmd: Child = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .expect("spawn debug binary"); + + cmd.stdin + .as_mut() + .expect("child stdin") + .write_all(b"subject\nb o d y") + .expect("write to child stdin"); + + let output = cmd.wait_with_output().expect("run debug binary"); + + assert!(output.status.success()); + + let out = String::from_utf8_lossy(&output.stdout); + assert_eq!( + "subject + +b o d y +", + out + ); + + let err = String::from_utf8_lossy(&output.stderr); + assert!(err.is_empty()); + } + // Sometime after the release of v1.2.0, external changes to Travis CI have // caused this test to begin failing consistently. Anecdotally, it likewise // has a higher failure rate on Ubuntu 19.10 than in the past. @@ -394,6 +495,9 @@ y }) .unwrap_or("head"); + let mut cargo_run_cmd = cargo_run_cmd(); + cargo_run_cmd.push("--width".into()); + cargo_run_cmd.push("72".into()); let output = Command::new("bash") .args(&[ "-c", @@ -402,7 +506,7 @@ y set -o pipefail {} < Cargo.lock | {} -n0", - cargo_run_cmd("72").join(" "), + cargo_run_cmd.join(" "), gnu_head ), ])