Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 612 #613

Merged
merged 5 commits into from
Dec 23, 2023
Merged

Fix 612 #613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
41728dc
Expand `orphan_lines_message_above_progress_bar` test
smoelius Dec 14, 2023
8ce0beb
Move `visual_line_count` and associated test to draw_target.rs
smoelius Dec 14, 2023
bbd910a
Change the type of `visual_line_count`'s `width` argument to `usize`
smoelius Dec 14, 2023
d68d862
Fix "type" error in multi.rs
smoelius Dec 14, 2023
2559ca2
Fix "type" error in draw_target.rs
smoelius Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 117 additions & 9 deletions src/draw_target.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,9 @@ const MAX_BURST: u8 = 20;
pub(crate) struct DrawState {
/// The lines to print (can contain ANSI codes)
pub(crate) lines: Vec<String>,
/// The number of lines that shouldn't be reaped by the next tick.
/// The number [`Self::lines`] entries that shouldn't be reaped by the next tick.
///
/// Note that this number may be different than the number of visual lines required to draw [`Self::lines`].
pub(crate) orphan_lines_count: usize,
/// True if we should move the cursor up when possible instead of clearing lines.
pub(crate) move_cursor: bool,
Expand Down Expand Up @@ -497,9 +499,11 @@ impl DrawState {
let term_height = term.height() as usize;
let term_width = term.width() as usize;
let len = self.lines.len();
debug_assert!(self.orphan_lines_count <= self.lines.len());
let orphan_visual_line_count =
visual_line_count(&self.lines[..self.orphan_lines_count], term_width);
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 @@ -516,12 +520,14 @@ impl DrawState {
// subtract with overflow later.
usize::max(terminal_len, 1)
};
// 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;
// Have all orphan lines been drawn?
if self.orphan_lines_count <= idx {
// If so, then `real_len` should be at least `orphan_visual_line_count`.
debug_assert!(orphan_visual_line_count <= real_len);
// Don't consider orphan lines when comparing to terminal height.
if real_len - orphan_visual_line_count + diff > term_height {
break;
}
}
real_len += diff;
if idx != 0 {
Expand All @@ -537,7 +543,7 @@ impl DrawState {
term.write_str(&" ".repeat(last_line_filler))?;

term.flush()?;
*last_line_count = real_len - self.orphan_lines_count + shift;
*last_line_count = real_len - orphan_visual_line_count + shift;
Ok(())
}

Expand All @@ -547,6 +553,21 @@ impl DrawState {
}
}

/// Calculate the number of visual lines in the given lines, after
/// accounting for line wrapping and non-printable characters.
pub(crate) fn visual_line_count(lines: &[impl AsRef<str>], width: usize) -> usize {
let mut real_lines = 0;
for line in lines {
let effective_line_length = console::measure_text_width(line.as_ref());
real_lines += usize::max(
(effective_line_length as f64 / width as f64).ceil() as usize,
1,
);
}

real_lines
}

#[cfg(test)]
mod tests {
use crate::{MultiProgress, ProgressBar, ProgressDrawTarget};
Expand All @@ -559,4 +580,91 @@ mod tests {
assert!(mp.is_hidden());
assert!(pb.is_hidden());
}

#[test]
fn real_line_count_test() {
#[derive(Debug)]
struct Case {
lines: &'static [&'static str],
expectation: usize,
width: usize,
}

let lines_and_expectations = [
Case {
lines: &["1234567890"],
expectation: 1,
width: 10,
},
Case {
lines: &["1234567890"],
expectation: 2,
width: 5,
},
Case {
lines: &["1234567890"],
expectation: 3,
width: 4,
},
Case {
lines: &["1234567890"],
expectation: 4,
width: 3,
},
Case {
lines: &["1234567890", "", "1234567890"],
expectation: 3,
width: 10,
},
Case {
lines: &["1234567890", "", "1234567890"],
expectation: 5,
width: 5,
},
Case {
lines: &["1234567890", "", "1234567890"],
expectation: 7,
width: 4,
},
Case {
lines: &["aaaaaaaaaaaaa", "", "bbbbbbbbbbbbbbbbb", "", "ccccccc"],
expectation: 8,
width: 7,
},
Case {
lines: &["", "", "", "", ""],
expectation: 5,
width: 6,
},
Case {
// These lines contain only ANSI escape sequences, so they should only count as 1 line
lines: &["\u{1b}[1m\u{1b}[1m\u{1b}[1m", "\u{1b}[1m\u{1b}[1m\u{1b}[1m"],
expectation: 2,
width: 5,
},
Case {
// These lines contain ANSI escape sequences and two effective chars, so they should only count as 1 line still
lines: &[
"a\u{1b}[1m\u{1b}[1m\u{1b}[1ma",
"a\u{1b}[1m\u{1b}[1m\u{1b}[1ma",
],
expectation: 2,
width: 5,
},
Case {
// These lines contain ANSI escape sequences and six effective chars, so they should count as 2 lines each
lines: &[
"aa\u{1b}[1m\u{1b}[1m\u{1b}[1mabcd",
"aa\u{1b}[1m\u{1b}[1m\u{1b}[1mabcd",
],
expectation: 4,
width: 5,
},
];

for case in lines_and_expectations.iter() {
let result = super::visual_line_count(case.lines, case.width);
assert_eq!(result, case.expectation, "case: {:?}", case);
}
}
}
113 changes: 8 additions & 105 deletions src/multi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use std::thread::panicking;
#[cfg(not(target_arch = "wasm32"))]
use std::time::Instant;

use crate::draw_target::{DrawState, DrawStateWrapper, LineAdjust, ProgressDrawTarget};
use crate::draw_target::{
visual_line_count, DrawState, DrawStateWrapper, LineAdjust, ProgressDrawTarget,
};
use crate::progress_bar::ProgressBar;
#[cfg(target_arch = "wasm32")]
use instant::Instant;
Expand Down Expand Up @@ -269,7 +271,7 @@ impl MultiState {
}

let width = match self.width() {
Some(width) => width as f64,
Some(width) => width as usize,
None => return Ok(()),
};

Expand Down Expand Up @@ -312,20 +314,20 @@ impl MultiState {
self.zombie_lines_count = 0;
}

let orphan_lines_count = visual_line_count(&self.orphan_lines, width);
force_draw |= orphan_lines_count > 0;
let orphan_visual_line_count = visual_line_count(&self.orphan_lines, width);
force_draw |= orphan_visual_line_count > 0;
let mut drawable = match self.draw_target.drawable(force_draw, now) {
Some(drawable) => drawable,
None => return Ok(()),
};

let mut draw_state = drawable.state();
draw_state.orphan_lines_count = orphan_lines_count;
draw_state.orphan_lines_count = self.orphan_lines.len();
draw_state.alignment = self.alignment;

if let Some(extra_lines) = &extra_lines {
draw_state.lines.extend_from_slice(extra_lines.as_slice());
draw_state.orphan_lines_count += visual_line_count(extra_lines, width);
draw_state.orphan_lines_count += extra_lines.len();
}

// Add lines from `ProgressBar::println` call.
Expand Down Expand Up @@ -519,18 +521,6 @@ enum InsertLocation {
Before(usize),
}

/// Calculate the number of visual lines in the given lines, after
/// accounting for line wrapping and non-printable characters.
fn visual_line_count(lines: &[impl AsRef<str>], width: f64) -> usize {
let mut real_lines = 0;
for line in lines {
let effective_line_length = console::measure_text_width(line.as_ref()) as f64;
real_lines += usize::max((effective_line_length / width).ceil() as usize, 1);
}

real_lines
}

#[cfg(test)]
mod tests {
use crate::{MultiProgress, ProgressBar, ProgressDrawTarget};
Expand Down Expand Up @@ -702,91 +692,4 @@ mod tests {
let pb = mp.add(ProgressBar::new(10));
mp.add(pb);
}

#[test]
fn real_line_count_test() {
#[derive(Debug)]
struct Case {
lines: &'static [&'static str],
expectation: usize,
width: f64,
}

let lines_and_expectations = [
Case {
lines: &["1234567890"],
expectation: 1,
width: 10.0,
},
Case {
lines: &["1234567890"],
expectation: 2,
width: 5.0,
},
Case {
lines: &["1234567890"],
expectation: 3,
width: 4.0,
},
Case {
lines: &["1234567890"],
expectation: 4,
width: 3.0,
},
Case {
lines: &["1234567890", "", "1234567890"],
expectation: 3,
width: 10.0,
},
Case {
lines: &["1234567890", "", "1234567890"],
expectation: 5,
width: 5.0,
},
Case {
lines: &["1234567890", "", "1234567890"],
expectation: 7,
width: 4.0,
},
Case {
lines: &["aaaaaaaaaaaaa", "", "bbbbbbbbbbbbbbbbb", "", "ccccccc"],
expectation: 8,
width: 7.0,
},
Case {
lines: &["", "", "", "", ""],
expectation: 5,
width: 6.0,
},
Case {
// These lines contain only ANSI escape sequences, so they should only count as 1 line
lines: &["\u{1b}[1m\u{1b}[1m\u{1b}[1m", "\u{1b}[1m\u{1b}[1m\u{1b}[1m"],
expectation: 2,
width: 5.0,
},
Case {
// These lines contain ANSI escape sequences and two effective chars, so they should only count as 1 line still
lines: &[
"a\u{1b}[1m\u{1b}[1m\u{1b}[1ma",
"a\u{1b}[1m\u{1b}[1m\u{1b}[1ma",
],
expectation: 2,
width: 5.0,
},
Case {
// These lines contain ANSI escape sequences and six effective chars, so they should count as 2 lines each
lines: &[
"aa\u{1b}[1m\u{1b}[1m\u{1b}[1mabcd",
"aa\u{1b}[1m\u{1b}[1m\u{1b}[1mabcd",
],
expectation: 4,
width: 5.0,
},
];

for case in lines_and_expectations.iter() {
let result = super::visual_line_count(case.lines, case.width);
assert_eq!(result, case.expectation, "case: {:?}", case);
}
}
}
17 changes: 17 additions & 0 deletions tests/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1794,6 +1794,23 @@ fn orphan_lines_message_above_progress_bar() {
Some(10),
ProgressDrawTarget::term_like(Box::new(in_mem.clone())),
);

orphan_lines_message_above_progress_bar_test(&pb, &in_mem);
}

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

let mp =
MultiProgress::with_draw_target(ProgressDrawTarget::term_like(Box::new(in_mem.clone())));

let pb = mp.add(ProgressBar::new(10));

orphan_lines_message_above_progress_bar_test(&pb, &in_mem);
}

fn orphan_lines_message_above_progress_bar_test(pb: &ProgressBar, in_mem: &InMemoryTerm) {
assert_eq!(in_mem.contents(), String::new());

for i in 0..=10 {
Expand Down