Skip to content

Commit

Permalink
Fix attempt to subtract with overflow (#582) (#586)
Browse files Browse the repository at this point in the history
* Add `orphan_lines` tests

* Don't consider orphan lines when comparing to terminal height

* Simplify tests (address Clippy warnings)

* Use wrapping strings in `orphan_lines_message_above_progress_bar`

* Fix comments
  • Loading branch information
smoelius committed Sep 20, 2023
1 parent 257d3ec commit 4461012
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/draw_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ impl DrawState {
let len = self.lines.len();
let mut real_len = 0;
let mut last_line_filler = 0;
debug_assert!(self.orphan_lines_count <= self.lines.len());
for (idx, line) in self.lines.iter().enumerate() {
let line_width = console::measure_text_width(line);
let diff = if line.is_empty() {
Expand All @@ -514,7 +515,11 @@ impl DrawState {
// subtract with overflow later.
usize::max(terminal_len, 1)
};
if real_len + diff > term_height {
// Don't consider orphan lines when comparing to terminal height.
debug_assert!(idx <= real_len);
if self.orphan_lines_count <= idx
&& real_len - self.orphan_lines_count + diff > term_height
{
break;
}
real_len += diff;
Expand Down
58 changes: 58 additions & 0 deletions tests/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1762,3 +1762,61 @@ Flush
"#
);
}

#[test]
fn orphan_lines() {
let in_mem = InMemoryTerm::new(10, 80);

let pb = ProgressBar::with_draw_target(
Some(10),
ProgressDrawTarget::term_like(Box::new(in_mem.clone())),
);
assert_eq!(in_mem.contents(), String::new());

for i in 0..=10 {
if i != 0 {
pb.inc(1);
}

let n = 5 + i;

pb.println("\n".repeat(n));
}

pb.finish();
}

#[test]
fn orphan_lines_message_above_progress_bar() {
let in_mem = InMemoryTerm::new(10, 80);

let pb = ProgressBar::with_draw_target(
Some(10),
ProgressDrawTarget::term_like(Box::new(in_mem.clone())),
);
assert_eq!(in_mem.contents(), String::new());

for i in 0..=10 {
if i != 0 {
pb.inc(1);
}

let n = 5 + i;

// Test with messages of differing numbers of lines. The messages have the form:
// n - 1 newlines followed by n * 11 dashes (`-`). The value of n ranges from 5
// (less than the terminal height) to 15 (greater than the terminal height). The
// number 11 is intentionally not a factor of the terminal width (80), but large
// enough that the strings of dashes eventually wrap.
pb.println(format!("{}{}", "\n".repeat(n - 1), "-".repeat(n * 11)));

// Check that the line above the progress bar is a string of dashes of length
// n * 11 mod the terminal width.
assert_eq!(
format!("{}", "-".repeat(n * 11 % 80)),
in_mem.contents().lines().rev().nth(1).unwrap(),
);
}

pb.finish();
}

0 comments on commit 4461012

Please sign in to comment.