From 41728dcf2da2d321927090d05b645d0be99a2177 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 14 Dec 2023 05:18:52 -0500 Subject: [PATCH 1/5] Expand `orphan_lines_message_above_progress_bar` test The test currenty fails. --- tests/render.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/render.rs b/tests/render.rs index a891e72f..bd275af7 100644 --- a/tests/render.rs +++ b/tests/render.rs @@ -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 { From 8ce0beb8966a9e61e8c8edaa41d382f2caa9b9a9 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 14 Dec 2023 05:23:01 -0500 Subject: [PATCH 2/5] Move `visual_line_count` and associated test to draw_target.rs --- src/draw_target.rs | 99 +++++++++++++++++++++++++++++++++++++++++++ src/multi.rs | 103 ++------------------------------------------- 2 files changed, 102 insertions(+), 100 deletions(-) diff --git a/src/draw_target.rs b/src/draw_target.rs index 5f9b66c9..b88ed3f6 100644 --- a/src/draw_target.rs +++ b/src/draw_target.rs @@ -547,6 +547,18 @@ 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], 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}; @@ -559,4 +571,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: 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); + } + } } diff --git a/src/multi.rs b/src/multi.rs index a02159a3..84fedc6c 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -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; @@ -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], 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}; @@ -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); - } - } } From bbd910a7803bf26527571632f93c156386ff134e Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 14 Dec 2023 05:27:22 -0500 Subject: [PATCH 3/5] Change the type of `visual_line_count`'s `width` argument to `usize` --- src/draw_target.rs | 35 +++++++++++++++++++---------------- src/multi.rs | 2 +- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/draw_target.rs b/src/draw_target.rs index b88ed3f6..e1a3e417 100644 --- a/src/draw_target.rs +++ b/src/draw_target.rs @@ -549,11 +549,14 @@ 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], width: f64) -> usize { +pub(crate) fn visual_line_count(lines: &[impl AsRef], width: usize) -> 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); + 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 @@ -578,60 +581,60 @@ mod tests { struct Case { lines: &'static [&'static str], expectation: usize, - width: f64, + width: usize, } let lines_and_expectations = [ Case { lines: &["1234567890"], expectation: 1, - width: 10.0, + width: 10, }, Case { lines: &["1234567890"], expectation: 2, - width: 5.0, + width: 5, }, Case { lines: &["1234567890"], expectation: 3, - width: 4.0, + width: 4, }, Case { lines: &["1234567890"], expectation: 4, - width: 3.0, + width: 3, }, Case { lines: &["1234567890", "", "1234567890"], expectation: 3, - width: 10.0, + width: 10, }, Case { lines: &["1234567890", "", "1234567890"], expectation: 5, - width: 5.0, + width: 5, }, Case { lines: &["1234567890", "", "1234567890"], expectation: 7, - width: 4.0, + width: 4, }, Case { lines: &["aaaaaaaaaaaaa", "", "bbbbbbbbbbbbbbbbb", "", "ccccccc"], expectation: 8, - width: 7.0, + width: 7, }, Case { lines: &["", "", "", "", ""], expectation: 5, - width: 6.0, + 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.0, + width: 5, }, Case { // These lines contain ANSI escape sequences and two effective chars, so they should only count as 1 line still @@ -640,7 +643,7 @@ mod tests { "a\u{1b}[1m\u{1b}[1m\u{1b}[1ma", ], expectation: 2, - width: 5.0, + width: 5, }, Case { // These lines contain ANSI escape sequences and six effective chars, so they should count as 2 lines each @@ -649,7 +652,7 @@ mod tests { "aa\u{1b}[1m\u{1b}[1m\u{1b}[1mabcd", ], expectation: 4, - width: 5.0, + width: 5, }, ]; diff --git a/src/multi.rs b/src/multi.rs index 84fedc6c..a1167ccd 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -271,7 +271,7 @@ impl MultiState { } let width = match self.width() { - Some(width) => width as f64, + Some(width) => width as usize, None => return Ok(()), }; From d68d862efc12524b2af56f63823adeb8e48d2ff3 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 14 Dec 2023 05:44:19 -0500 Subject: [PATCH 4/5] Fix "type" error in multi.rs The `orphan_lines_message_above_progress_bar` tests now pass. But the `multi_progress_println_terminal_wrap` test fails. --- src/draw_target.rs | 4 +++- src/multi.rs | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/draw_target.rs b/src/draw_target.rs index e1a3e417..49f5ab8f 100644 --- a/src/draw_target.rs +++ b/src/draw_target.rs @@ -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, - /// 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, diff --git a/src/multi.rs b/src/multi.rs index a1167ccd..967e0f0c 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -314,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. From 2559ca2ab726724486570a330474bbfa6380c5c4 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Thu, 14 Dec 2023 05:54:58 -0500 Subject: [PATCH 5/5] Fix "type" error in draw_target.rs The `multi_progress_println_terminal_wrap` test now passes. --- src/draw_target.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/draw_target.rs b/src/draw_target.rs index 49f5ab8f..003dfd5c 100644 --- a/src/draw_target.rs +++ b/src/draw_target.rs @@ -499,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() { @@ -518,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 { @@ -539,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(()) }