From 4a1283a62ef25e169c58183c10528171f1eccc16 Mon Sep 17 00:00:00 2001 From: Martin Geisler Date: Sun, 2 Jan 2022 21:35:02 +0100 Subject: [PATCH] Change penalties to non-negative numbers The optimization problem solved by the optimal-fit algorithm is fundamentally a minimization problem. It is therefore not sensible to allow negative penalties since all penalties are there to discourage certain features: * `nline_penalty` discourages breaks with more lines than necessary, * `overflow_penalty` discourages lines longer than the line width, * `short_last_line_penalty` discourages short last lines, * `hyphen_penalty` discourages hyphenation Making this change surfaces the overflow bug behind #247 and #416. This will be fixed next via #421 and this commit can be seen as a way of simplifying that PR. --- examples/wasm/src/lib.rs | 16 ++++++++-------- examples/wasm/www/index.html | 6 +++--- fuzz/fuzz_targets/wrap_optimal_fit.rs | 8 ++++---- src/lib.rs | 11 +++++++++++ src/wrap_algorithms/optimal_fit.rs | 12 ++++++------ 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/examples/wasm/src/lib.rs b/examples/wasm/src/lib.rs index 50fb405d..59de964e 100644 --- a/examples/wasm/src/lib.rs +++ b/examples/wasm/src/lib.rs @@ -250,22 +250,22 @@ pub enum WasmWrapAlgorithm { #[wasm_bindgen] #[derive(Copy, Clone, Debug, Default)] pub struct WasmOptimalFit { - pub nline_penalty: i32, - pub overflow_penalty: i32, + pub nline_penalty: usize, + pub overflow_penalty: usize, pub short_last_line_fraction: usize, - pub short_last_line_penalty: i32, - pub hyphen_penalty: i32, + pub short_last_line_penalty: usize, + pub hyphen_penalty: usize, } #[wasm_bindgen] impl WasmOptimalFit { #[wasm_bindgen(constructor)] pub fn new( - nline_penalty: i32, - overflow_penalty: i32, + nline_penalty: usize, + overflow_penalty: usize, short_last_line_fraction: usize, - short_last_line_penalty: i32, - hyphen_penalty: i32, + short_last_line_penalty: usize, + hyphen_penalty: usize, ) -> WasmOptimalFit { WasmOptimalFit { nline_penalty, diff --git a/examples/wasm/www/index.html b/examples/wasm/www/index.html index 50fe3bf8..d9296bf0 100644 --- a/examples/wasm/www/index.html +++ b/examples/wasm/www/index.html @@ -97,7 +97,7 @@

Textwrap WebAssembly Demo

- +
@@ -109,13 +109,13 @@

Textwrap WebAssembly Demo

- +
- +
diff --git a/fuzz/fuzz_targets/wrap_optimal_fit.rs b/fuzz/fuzz_targets/wrap_optimal_fit.rs index d73adad0..2dd89c16 100644 --- a/fuzz/fuzz_targets/wrap_optimal_fit.rs +++ b/fuzz/fuzz_targets/wrap_optimal_fit.rs @@ -6,11 +6,11 @@ use textwrap::wrap_algorithms::{wrap_optimal_fit, OptimalFit}; #[derive(Arbitrary, Debug)] struct Penalties { - nline_penalty: i32, - overflow_penalty: i32, + nline_penalty: usize, + overflow_penalty: usize, short_last_line_fraction: usize, - short_last_line_penalty: i32, - hyphen_penalty: i32, + short_last_line_penalty: usize, + hyphen_penalty: usize, } impl Into for Penalties { diff --git a/src/lib.rs b/src/lib.rs index 37520d8f..1be49af8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1392,7 +1392,18 @@ mod tests { } #[test] + #[cfg(not(feature = "smawk"))] + fn max_width() { + // No overflow for the first-fit wrap algorithm. + assert_eq!(wrap("foo bar", usize::max_value()), vec!["foo bar"]); + } + + #[test] + #[cfg(feature = "smawk")] + #[should_panic(expected = "attempt to multiply with overflow")] fn max_width() { + // The optimal-fit algorithm overflows for extreme line + // widths. See #247 and #416 for details.. assert_eq!(wrap("foo bar", usize::max_value()), vec!["foo bar"]); } diff --git a/src/wrap_algorithms/optimal_fit.rs b/src/wrap_algorithms/optimal_fit.rs index 74bd6d31..e046d587 100644 --- a/src/wrap_algorithms/optimal_fit.rs +++ b/src/wrap_algorithms/optimal_fit.rs @@ -24,7 +24,7 @@ use crate::wrap_algorithms::WrapAlgorithm; pub struct OptimalFit { /// Per-line penalty. This is added for every line, which makes it /// expensive to output more lines than the minimum required. - pub nline_penalty: i32, + pub nline_penalty: usize, /// Per-character cost for lines that overflow the target line width. /// @@ -67,7 +67,7 @@ pub struct OptimalFit { /// character. If it overflows by more than one character, the /// overflow penalty will quickly outgrow the cost of the gap, as /// seen above. - pub overflow_penalty: i32, + pub overflow_penalty: usize, /// When should the a single word on the last line be considered /// "too short"? @@ -128,10 +128,10 @@ pub struct OptimalFit { /// Penalty for a last line with a single short word. /// /// Set this to zero if you do not want to penalize short last lines. - pub short_last_line_penalty: i32, + pub short_last_line_penalty: usize, /// Penalty for lines ending with a hyphen. - pub hyphen_penalty: i32, + pub hyphen_penalty: usize, } impl OptimalFit { @@ -308,12 +308,12 @@ pub fn wrap_optimal_fit<'a, 'b, T: Fragment>( // Next, we add a penalty depending on the line length. if line_width > target_width { // Lines that overflow get a hefty penalty. - let overflow = (line_width - target_width) as i32; + let overflow = line_width - target_width; cost += overflow * penalties.overflow_penalty; } else if j < fragments.len() { // Other lines (except for the last line) get a milder // penalty which depend on the size of the gap. - let gap = (target_width - line_width) as i32; + let gap = target_width - line_width; cost += gap * gap; } else if i + 1 == j && line_width < target_width / penalties.short_last_line_fraction { // The last line can have any size gap, but we do add a