From 52751636454177eab810ffd66c25057714e2aaa7 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Mon, 26 Sep 2022 16:29:11 +0200 Subject: [PATCH 01/13] fix: Dynamic calculation where set width = content width --- .github/workflows/test.yml | 33 ++++++++--------- CHANGELOG.md | 7 ++++ Cargo.toml | 3 +- src/utils/arrangement/dynamic.rs | 20 ++++++++-- tests/all/content_arrangement_test.rs | 53 ++++++++++++++++++++++++--- 5 files changed, 88 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eb8f488..8039b73 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,17 +4,17 @@ on: push: branches: [main] paths: - - '.github/workflows/test.yml' - - '**.rs' - - 'Cargo.toml' - - 'Cargo.lock' + - ".github/workflows/test.yml" + - "**.rs" + - "Cargo.toml" + - "Cargo.lock" pull_request: branches: [main] paths: - - '.github/workflows/test.yml' - - '**.rs' - - 'Cargo.toml' - - 'Cargo.lock' + - ".github/workflows/test.yml" + - "**.rs" + - "Cargo.toml" + - "Cargo.lock" jobs: test: @@ -23,11 +23,11 @@ jobs: strategy: matrix: target: - - x86_64-unknown-linux-gnu - - x86_64-pc-windows-msvc - - x86_64-apple-darwin - - wasm32-wasi - toolchain: [stable, nightly, 1.59] + - x86_64-unknown-linux-gnu + - x86_64-pc-windows-msvc + - x86_64-apple-darwin + - wasm32-wasi + toolchain: [stable, nightly, 1.62] include: - target: x86_64-unknown-linux-gnu os: ubuntu-latest @@ -42,10 +42,9 @@ jobs: os: macos-latest minimal_setup: false -# minimal_setup: This is needed for targets that don't support our dev dependencies. -# It also excludes the default features, i.e. [tty]. -# For instance, "wasm32-wasi" is such a target. - + # minimal_setup: This is needed for targets that don't support our dev dependencies. + # It also excludes the default features, i.e. [tty]. + # For instance, "wasm32-wasi" is such a target. steps: - name: Checkout code uses: actions/checkout@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index d15618c..cce14e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,15 @@ # Changelog + All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [6.1.1] - unreleased + +### Fixed + +- Fixed an issue where dynamic arrangement failed when setting the table to the exact width of the content [#90](https://github.com/Nukesor/comfy-table/issues/90). + ## [6.1.0] - 2022-08-28 ### Added diff --git a/Cargo.toml b/Cargo.toml index f98f6f4..3553a22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,7 @@ documentation = "https://docs.rs/comfy-table/" license = "MIT" keywords = ["terminal", "table", "unicode"] readme = "README.md" -rust-version = "1.59" +rust-version = "1.62" edition = "2021" [badges] @@ -49,3 +49,4 @@ doc-comment = "0.3" proptest = "1" criterion = "0.4" rand = "0.8" +rstest = "0.15" diff --git a/src/utils/arrangement/dynamic.rs b/src/utils/arrangement/dynamic.rs index 3b45517..88af54f 100644 --- a/src/utils/arrangement/dynamic.rs +++ b/src/utils/arrangement/dynamic.rs @@ -45,7 +45,7 @@ pub fn arrange( // Step 2-4. // Find all columns that require less space than the average. // Returns the remaining available width and the amount of remaining columns that need handling - let (mut remaining_width, mut remaining_columns) = find_columns_less_than_average( + let (mut remaining_width, mut remaining_columns) = find_columns_that_fit_into_average( table, infos, table_width, @@ -189,7 +189,7 @@ fn available_content_width( /// /// Returns: /// `(remaining_width: usize, remaining_columns: u16)` -fn find_columns_less_than_average( +fn find_columns_that_fit_into_average( table: &Table, infos: &mut DisplayInfos, table_width: usize, @@ -247,6 +247,12 @@ fn find_columns_less_than_average( let info = ColumnDisplayInfo::new(column, width); infos.insert(column.index, info); + #[cfg(feature = "debug")] + println!( + "dynamic::find_columns_that_fit_into_average: Fixed column {} via MaxWidth constraint with size {}", + column.index, width + ); + // Continue with new recalculated width remaining_width = remaining_width.saturating_sub(width.into()); remaining_columns -= 1; @@ -259,12 +265,18 @@ fn find_columns_less_than_average( } } - // The column has a smaller max_content_width than the average space. + // The column has a smaller or equal max_content_width than the average space. // Fix the width to max_content_width and mark it as checked - if usize::from(max_column_width) < average_space { + if usize::from(max_column_width) <= average_space { let info = ColumnDisplayInfo::new(column, max_column_width); infos.insert(column.index, info); + #[cfg(feature = "debug")] + println!( + "dynamic::find_columns_that_fit_into_average: Fixed column {} as it's smaller than average with size {}", + column.index, max_column_width + ); + // Continue with new recalculated width remaining_width = remaining_width.saturating_sub(max_column_width.into()); remaining_columns -= 1; diff --git a/tests/all/content_arrangement_test.rs b/tests/all/content_arrangement_test.rs index 4f92789..36c0df1 100644 --- a/tests/all/content_arrangement_test.rs +++ b/tests/all/content_arrangement_test.rs @@ -11,8 +11,8 @@ fn assert_table_lines(table: &Table, count: usize) { } } -#[test] /// Test the robustnes of the dynamic table arangement. +#[test] fn simple_dynamic_table() { let mut table = Table::new(); table.set_header(&vec!["Header1", "Header2", "Head"]) @@ -71,9 +71,9 @@ fn simple_dynamic_table() { assert_eq!("\n".to_string() + &table.to_string(), expected); } -#[test] /// Individual rows can be configured to have a max height. /// Everything beyond that line height should be truncated. +#[test] fn table_with_truncate() { let mut table = Table::new(); let mut first_row: Row = Row::from(vec![ @@ -127,11 +127,11 @@ fn table_with_truncate() { assert_eq!("\n".to_string() + &table.to_string(), expected); } -#[test] /// This table checks the scenario, where a column has a big max_width, but a lot of the assigned /// space doesn't get used after splitting the lines. This happens mostly when there are /// many long words in a single column. /// The remaining space should rather be distributed to other cells. +#[test] fn distribute_space_after_split() { let mut table = Table::new(); table @@ -158,9 +158,9 @@ fn distribute_space_after_split() { assert_eq!("\n".to_string() + &table.to_string(), expected); } -#[test] /// A single column get's split and a lot of the available isn't used afterward. /// The remaining space should be cut away, making the table more compact. +#[test] fn unused_space_after_split() { let mut table = Table::new(); table @@ -182,8 +182,8 @@ fn unused_space_after_split() { assert_eq!("\n".to_string() + &table.to_string(), expected); } -#[test] /// The full width of a table should be used, even if the space isn't used. +#[test] fn dynamic_full_width_after_split() { let mut table = Table::new(); table @@ -205,10 +205,10 @@ fn dynamic_full_width_after_split() { assert_eq!("\n".to_string() + &table.to_string(), expected); } -#[test] /// This table checks the scenario, where a column has a big max_width, but a lot of the assigned /// space isn't used after splitting the lines. /// The remaining space should rather distributed between all cells. +#[test] fn dynamic_full_width() { let mut table = Table::new(); table @@ -228,3 +228,44 @@ fn dynamic_full_width() { assert_table_lines(&table, 80); assert_eq!("\n".to_string() + &table.to_string(), expected); } + +/// Test that a table is displayed in its full width, if the `table.width` is set to the exact +/// width the table has, if it's fully expanded. +/// +/// The same should be the case for values that're larget than this width. +#[rstest::rstest] +fn dynamic_exact_width() { + let header = vec!["a\n---\ni64", "b\n---\ni64", "b_squared\n---\nf64"]; + let rows = vec![ + vec!["1", "2", "4.0"], + vec!["3", "4", "16.0"], + vec!["5", "6", "36.0"], + ]; + + for width in 25..40 { + let mut table = Table::new(); + let table = table + .load_preset(comfy_table::presets::UTF8_FULL) + .set_content_arrangement(ContentArrangement::Dynamic) + .set_width(width); + + table.set_header(header.clone()).add_rows(rows.clone()); + + println!("{table}"); + let expected = " +┌─────┬─────┬───────────┐ +│ a ┆ b ┆ b_squared │ +│ --- ┆ --- ┆ --- │ +│ i64 ┆ i64 ┆ f64 │ +╞═════╪═════╪═══════════╡ +│ 1 ┆ 2 ┆ 4.0 │ +├╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤ +│ 3 ┆ 4 ┆ 16.0 │ +├╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤ +│ 5 ┆ 6 ┆ 36.0 │ +└─────┴─────┴───────────┘"; + println!("{expected}"); + assert_table_lines(&table, 25); + assert_eq!("\n".to_string() + &table.to_string(), expected); + } +} From 0f0a33d56726f747456e00707409b5ef04a5e5b7 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Mon, 26 Sep 2022 17:21:18 +0200 Subject: [PATCH 02/13] fix: Respect header in last optimization step --- CHANGELOG.md | 5 ++ src/table.rs | 76 +++++++++++++++++++++++++-- src/utils/arrangement/dynamic.rs | 69 ++++++++++++++++++------ tests/all/constraints_test.rs | 3 ++ tests/all/content_arrangement_test.rs | 59 ++++++++++++++++----- tests/all/mod.rs | 9 ++++ 6 files changed, 186 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cce14e5..28234c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Fixed - Fixed an issue where dynamic arrangement failed when setting the table to the exact width of the content [#90](https://github.com/Nukesor/comfy-table/issues/90). +- The header size is now properly respected in the final optimization step [#90](https://github.com/Nukesor/comfy-table/issues/90). + Previously, this wasn't the case and lead to weird formatting behavior when both of the following were true + - Dynamic content adjustment was active. + - The table didn't fit into the the available space. + - The header of a row was longer than its content. ## [6.1.0] - 2022-08-28 diff --git a/src/table.rs b/src/table.rs index 4b21940..6989be1 100644 --- a/src/table.rs +++ b/src/table.rs @@ -560,6 +560,37 @@ impl Table { } } + /// Get a mutable iterator over cells of a column, including the header cell. + /// The header cell will be the very first cell returned. + /// The iterator returns a nested Option>, since there might be + /// rows that are missing this specific Cell. + /// + /// ``` + /// use comfy_table::Table; + /// let mut table = Table::new(); + /// table.set_header(&vec!["A", "B"]); + /// table.add_row(&vec!["First", "Second"]); + /// table.add_row(&vec!["Third"]); + /// table.add_row(&vec!["Fourth", "Fifth"]); + /// + /// // Create an iterator over the second column + /// let mut cell_iter = table.column_cells_with_header_iter(1); + /// assert_eq!(cell_iter.next().unwrap().unwrap().content(), "B"); + /// assert_eq!(cell_iter.next().unwrap().unwrap().content(), "Second"); + /// assert!(cell_iter.next().unwrap().is_none()); + /// assert_eq!(cell_iter.next().unwrap().unwrap().content(), "Fifth"); + /// assert!(cell_iter.next().is_none()); + /// ``` + pub fn column_cells_with_header_iter(&self, column_index: usize) -> ColumnCellsWithHeaderIter { + ColumnCellsWithHeaderIter { + header_checked: false, + header: &self.header, + rows: &self.rows, + column_index, + row_index: 0, + } + } + /// Reference to a specific row pub fn row(&self, index: usize) -> Option<&Row> { self.rows.get(index) @@ -679,12 +710,47 @@ impl<'a> Iterator for ColumnCellIter<'a> { if let Some(row) = self.rows.get(self.row_index) { self.row_index += 1; - // Check if the row has the requested column. - if let Some(cell) = row.cells.get(self.column_index) { - return Some(Some(cell)); - } + // Return the cell (if it exists). + return Some(row.cells.get(self.column_index)); + } + + None + } +} + +/// An iterator over cells of a specific column. +/// A dedicated struct is necessary, as data is usually handled by rows and thereby stored in +/// `Table::rows`. This type is returned by [Table::column_cells_iter]. +pub struct ColumnCellsWithHeaderIter<'a> { + header_checked: bool, + header: &'a Option, + rows: &'a [Row], + column_index: usize, + row_index: usize, +} + +impl<'a> Iterator for ColumnCellsWithHeaderIter<'a> { + type Item = Option<&'a Cell>; + fn next(&mut self) -> Option> { + // Get the header as the first cell + if !self.header_checked { + self.header_checked = true; + + return match self.header { + Some(header) => { + // Return the cell (if it exists). + Some(header.cells.get(self.column_index)) + } + None => Some(None), + }; + } + + // Check if there's a next row + if let Some(row) = self.rows.get(self.row_index) { + self.row_index += 1; - return Some(None); + // Return the cell (if it exists). + return Some(row.cells.get(self.column_index)); } None diff --git a/src/utils/arrangement/dynamic.rs b/src/utils/arrangement/dynamic.rs index 88af54f..a88fc99 100644 --- a/src/utils/arrangement/dynamic.rs +++ b/src/utils/arrangement/dynamic.rs @@ -40,7 +40,9 @@ pub fn arrange( available_content_width(table, infos, visible_columns, table_width); #[cfg(feature = "debug")] - println!("Table width: {table_width}, Start remaining width {remaining_width}"); + println!( + "dynamic::arrange: Table width: {table_width}, Start remaining width {remaining_width}" + ); // Step 2-4. // Find all columns that require less space than the average. @@ -93,8 +95,8 @@ pub fn arrange( #[cfg(feature = "debug")] { - println!("After optimize: {infos:#?}",); - println!("Remaining width {remaining_width}, column {remaining_columns}",); + println!("dynamic::arrange: After optimize: {infos:#?}",); + println!("dynamic::arrange: Remaining width {remaining_width}, column {remaining_columns}",); } // Early exit and one branch of Part 7. @@ -107,7 +109,7 @@ pub fn arrange( { use_full_width(infos, remaining_width); #[cfg(feature = "debug")] - println!("After full width: {infos:#?}"); + println!("dynamic::arrange: After full width: {infos:#?}"); } return; } @@ -121,7 +123,7 @@ pub fn arrange( distribute_remaining_space(&table.columns, infos, remaining_width, remaining_columns); #[cfg(feature = "debug")] - println!("After distribute: {infos:#?}"); + println!("dynamic::arrange: After distribute: {infos:#?}"); } /// Step 1 @@ -249,8 +251,8 @@ fn find_columns_that_fit_into_average( #[cfg(feature = "debug")] println!( - "dynamic::find_columns_that_fit_into_average: Fixed column {} via MaxWidth constraint with size {}", - column.index, width + "dynamic::find_columns_that_fit_into_average: Fixed column {} via MaxWidth constraint with size {}, as it's bigger than average {}", + column.index, width, average_space ); // Continue with new recalculated width @@ -273,8 +275,8 @@ fn find_columns_that_fit_into_average( #[cfg(feature = "debug")] println!( - "dynamic::find_columns_that_fit_into_average: Fixed column {} as it's smaller than average with size {}", - column.index, max_column_width + "dynamic::find_columns_that_fit_into_average: Fixed column {} with size {}, as it's smaller than average {}", + column.index, max_column_width, average_space ); // Continue with new recalculated width @@ -294,10 +296,13 @@ fn find_columns_that_fit_into_average( /// Step 5 /// -/// Determine, whether there are any columns that have a higher LowerBoundary than the currently -/// available average width. +/// Determine, whether there are any columns that are allowed to occupy more width than the current +/// `average_space` via a [LowerBoundary] constraint. +/// +/// These columns will then get fixed to the width specified in the [LowerBoundary] constraint. /// -/// These columns will then get fixed to the specified amount. +/// I.e. if a column has to have at least 10 characters, but the average width left for a column is +/// only 6, we fix the column to this 10 character minimum! fn enforce_lower_boundary_constraints( table: &Table, infos: &mut DisplayInfos, @@ -337,6 +342,12 @@ fn enforce_lower_boundary_constraints( let info = ColumnDisplayInfo::new(column, width); infos.insert(column.index, info); + #[cfg(feature = "debug")] + println!( + "dynamic::enforce_lower_boundary_constraints: Fixed column {} to min constraint width {}", + column.index, width + ); + // Continue with new recalculated width remaining_width = remaining_width.saturating_sub(width.into()); remaining_columns -= 1; @@ -379,6 +390,12 @@ fn optimize_space_after_split( // Calculate the average space that remains for each column. let mut average_space = remaining_width / remaining_columns; + #[cfg(feature = "debug")] + println!( + "dynamic::optimize_space_after_split: Start with average_space {}", + average_space + ); + // Do this as long as we find a smaller column while found_smaller { found_smaller = false; @@ -390,6 +407,12 @@ fn optimize_space_after_split( let longest_line = longest_line_after_split(average_space, column, table); + #[cfg(feature = "debug")] + println!( + "dynamic::optimize_space_after_split: Longest line after split for column {} is {}", + column.index, longest_line + ); + // If there's a considerable amount space left after splitting, we freeze the column and // set its content width to the calculated post-split width. let remaining_space = average_space.saturating_sub(longest_line); @@ -404,6 +427,12 @@ fn optimize_space_after_split( break; } average_space = remaining_width / remaining_columns; + + #[cfg(feature = "debug")] + println!( + "dynamic::optimize_space_after_split: average_space is now {}", + average_space + ); found_smaller = true; } } @@ -424,12 +453,11 @@ fn longest_line_after_split(average_space: usize, column: &Column, table: &Table let mut column_lines = Vec::new(); // Iterate - for cell in table.column_cells_iter(column.index) { + for cell in table.column_cells_with_header_iter(column.index) { // Only look at rows that actually contain this cell. - let cell = if let Some(cell) = cell { - cell - } else { - continue; + let cell = match cell { + Some(cell) => cell, + None => continue, }; let delimiter = delimiter(table, column, cell); @@ -443,6 +471,13 @@ fn longest_line_after_split(average_space: usize, column: &Column, table: &Table for line in cell.content.iter() { if line.width() > average_space { let mut splitted = split_line(line, &info, delimiter); + + #[cfg(feature = "debug")] + println!( + "dynamic::longest_line_after_split: Splitting line with width {}. Original:\n {}\nSplitted:\n {:?}", + line.width(), line, splitted + ); + column_lines.append(&mut splitted); } else { column_lines.push(line.into()); diff --git a/tests/all/constraints_test.rs b/tests/all/constraints_test.rs index 99ae140..e531bf5 100644 --- a/tests/all/constraints_test.rs +++ b/tests/all/constraints_test.rs @@ -3,6 +3,8 @@ use comfy_table::Width::*; use comfy_table::*; use pretty_assertions::assert_eq; +use super::assert_table_line_width; + fn get_constraint_table() -> Table { let mut table = Table::new(); table @@ -225,6 +227,7 @@ fn max_100_percentage() { | smol | +--------------------------------------+"; println!("{expected}"); + assert_table_line_width(&table, 40); assert_eq!("\n".to_string() + &table.to_string(), expected); } diff --git a/tests/all/content_arrangement_test.rs b/tests/all/content_arrangement_test.rs index 36c0df1..a1ca4b7 100644 --- a/tests/all/content_arrangement_test.rs +++ b/tests/all/content_arrangement_test.rs @@ -3,13 +3,8 @@ use pretty_assertions::assert_eq; use comfy_table::ColumnConstraint::*; use comfy_table::Width::*; use comfy_table::{ContentArrangement, Row, Table}; -use unicode_width::UnicodeWidthStr; -fn assert_table_lines(table: &Table, count: usize) { - for line in table.lines() { - assert_eq!(line.width(), count); - } -} +use super::assert_table_line_width; /// Test the robustnes of the dynamic table arangement. #[test] @@ -67,7 +62,7 @@ fn simple_dynamic_table() { | | stuff | | +--------+-------+------+"; println!("{expected}"); - assert!(table.lines().all(|line| line.width() == 25)); + assert_table_line_width(&table, 25); assert_eq!("\n".to_string() + &table.to_string(), expected); } @@ -123,7 +118,7 @@ fn table_with_truncate() { | the middle ... | text | | +----------------+--------+-------+"; println!("{expected}"); - assert_table_lines(&table, 35); + assert_table_line_width(&table, 35); assert_eq!("\n".to_string() + &table.to_string(), expected); } @@ -154,7 +149,7 @@ fn distribute_space_after_split() { +-----------------------------------------+-----------------------------+------+"; println!("{expected}"); - assert_table_lines(&table, 80); + assert_table_line_width(&table, 80); assert_eq!("\n".to_string() + &table.to_string(), expected); } @@ -178,7 +173,7 @@ fn unused_space_after_split() { | anotherverylongtext | +---------------------+"; println!("{expected}"); - assert_table_lines(&table, 23); + assert_table_line_width(&table, 23); assert_eq!("\n".to_string() + &table.to_string(), expected); } @@ -201,7 +196,7 @@ fn dynamic_full_width_after_split() { | anotherverylongtexttesttestaa | +------------------------------------------------+"; println!("{expected}"); - assert_table_lines(&table, 50); + assert_table_line_width(&table, 50); assert_eq!("\n".to_string() + &table.to_string(), expected); } @@ -225,7 +220,7 @@ fn dynamic_full_width() { | This is a short line | small | smol | +-----------------------------------+----------------------+-------------------+"; println!("{expected}"); - assert_table_lines(&table, 80); + assert_table_line_width(&table, 80); assert_eq!("\n".to_string() + &table.to_string(), expected); } @@ -265,7 +260,45 @@ fn dynamic_exact_width() { │ 5 ┆ 6 ┆ 36.0 │ └─────┴─────┴───────────┘"; println!("{expected}"); - assert_table_lines(&table, 25); + assert_table_line_width(&table, 25); assert_eq!("\n".to_string() + &table.to_string(), expected); } } + +/// Test that the formatting works as expected, if the table is slightly smaller than the max width +/// of the table. +#[rstest::rstest] +fn dynamic_slightly_smaller() { + let header = vec!["a\n---\ni64", "b\n---\ni64", "b_squared\n---\nf64"]; + let rows = vec![ + vec!["1", "2", "4.0"], + vec!["3", "4", "16.0"], + vec!["5", "6", "36.0"], + ]; + + let mut table = Table::new(); + let table = table + .load_preset(comfy_table::presets::UTF8_FULL) + .set_content_arrangement(ContentArrangement::Dynamic) + .set_width(24); + + table.set_header(header.clone()).add_rows(rows.clone()); + + println!("{table}"); + let expected = " +┌─────┬─────┬──────────┐ +│ a ┆ b ┆ b_square │ +│ --- ┆ --- ┆ d │ +│ i64 ┆ i64 ┆ --- │ +│ ┆ ┆ f64 │ +╞═════╪═════╪══════════╡ +│ 1 ┆ 2 ┆ 4.0 │ +├╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌┤ +│ 3 ┆ 4 ┆ 16.0 │ +├╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌┤ +│ 5 ┆ 6 ┆ 36.0 │ +└─────┴─────┴──────────┘"; + println!("{expected}"); + assert_table_line_width(&table, 24); + assert_eq!("\n".to_string() + &table.to_string(), expected); +} diff --git a/tests/all/mod.rs b/tests/all/mod.rs index a1029f8..5082401 100644 --- a/tests/all/mod.rs +++ b/tests/all/mod.rs @@ -1,3 +1,6 @@ +use comfy_table::Table; +use unicode_width::UnicodeWidthStr; + mod alignment_test; #[cfg(feature = "tty")] mod combined_test; @@ -13,3 +16,9 @@ mod simple_test; #[cfg(feature = "tty")] mod styling_test; mod utf_8_characters; + +pub fn assert_table_line_width(table: &Table, count: usize) { + for line in table.lines() { + assert_eq!(line.width(), count); + } +} From a8f13c75f4d9d7e55d9543c6dbff7e5dd498b686 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Thu, 13 Oct 2022 23:04:45 +0200 Subject: [PATCH 03/13] docs: Remove some confusing statements --- src/style/column.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/style/column.rs b/src/style/column.rs index 751f4b9..639d050 100644 --- a/src/style/column.rs +++ b/src/style/column.rs @@ -10,7 +10,8 @@ pub enum ColumnConstraint { /// This will completely hide a column. Hidden, /// Force the column to be as long as it's content. - /// Use with caution! This can easily break your table, if the column's content is overly long. + /// Use with caution! This can easily mess up your table formatting, + /// if a column's content is overly long. ContentWidth, /// Enforce a absolute width for a column. Absolute(Width), @@ -25,13 +26,10 @@ pub enum ColumnConstraint { #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum Width { /// A fixed amount of characters. - /// This can be used to specify an upper/lower boundary as well as a fixed size for the column. Fixed(u16), /// A width equivalent to a certain percentage of the available width. /// Values above 100 will be automatically reduced to 100. /// - /// This can be used to specify an upper/lower boundary as well as a fixed size for the column. - /// /// **Warning:** This option will be ignored if: /// - you aren't using one of ContentArrangement::{Dynamic, DynamicFullWidth} /// - the width of the table/terminal cannot be determined. From b5696a826e0390836547f87aac80ad2f18882eaf Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Thu, 13 Oct 2022 23:06:37 +0200 Subject: [PATCH 04/13] refactor: Fix some variable names --- src/utils/arrangement/dynamic.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/utils/arrangement/dynamic.rs b/src/utils/arrangement/dynamic.rs index a88fc99..82425fb 100644 --- a/src/utils/arrangement/dynamic.rs +++ b/src/utils/arrangement/dynamic.rs @@ -196,11 +196,11 @@ fn find_columns_that_fit_into_average( infos: &mut DisplayInfos, table_width: usize, mut remaining_width: usize, - visible_coulumns: usize, + visible_columns: usize, max_content_widths: &[u16], ) -> (usize, usize) { let mut found_smaller = true; - let mut remaining_columns = count_remaining_columns(visible_coulumns, infos); + let mut remaining_columns = count_remaining_columns(visible_columns, infos); while found_smaller { found_smaller = false; @@ -233,16 +233,16 @@ fn find_columns_that_fit_into_average( table, &column.constraint, Some(table_width), - visible_coulumns, + visible_columns, ) { // Max/Min constraints always include padding! let average_space_with_padding = average_space + usize::from(column.padding_width()); - let column_width_with_padding = max_column_width + column.padding_width(); + let width_with_padding = max_column_width + column.padding_width(); // Check that both conditions mentioned above are met. if usize::from(max_width) <= average_space_with_padding - && column_width_with_padding >= max_width + && width_with_padding >= max_width { // Save the calculated info, this column has been handled. let width = absolute_width_with_padding(column, max_width); @@ -258,6 +258,7 @@ fn find_columns_that_fit_into_average( // Continue with new recalculated width remaining_width = remaining_width.saturating_sub(width.into()); remaining_columns -= 1; + if remaining_columns == 0 { break; } From 61816f68f7e3d6a6401f7f0fcdaf4a425b233e8f Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Thu, 13 Oct 2022 23:06:59 +0200 Subject: [PATCH 05/13] test: Add failing polars test case --- tests/all/content_arrangement_test.rs | 48 +++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/all/content_arrangement_test.rs b/tests/all/content_arrangement_test.rs index a1ca4b7..870c57c 100644 --- a/tests/all/content_arrangement_test.rs +++ b/tests/all/content_arrangement_test.rs @@ -1,3 +1,5 @@ +use comfy_table::ColumnConstraint; +use comfy_table::Width; use pretty_assertions::assert_eq; use comfy_table::ColumnConstraint::*; @@ -302,3 +304,49 @@ fn dynamic_slightly_smaller() { assert_table_line_width(&table, 24); assert_eq!("\n".to_string() + &table.to_string(), expected); } + +/// This failed on a python integration test case in the polars project. +/// This a regression test. +#[rstest::rstest] +fn polar_python_test_tbl_width_chars() { + let header = vec![ + "a really long col\n---\ni64", + "b\n---\nstr", + "this is 10\n---\ni64", + ]; + let rows = vec![ + vec!["1", "", "4"], + vec!["2", "this is a string value that will...", "5"], + vec!["3", "null", "6"], + ]; + + let mut table = Table::new(); + let table = table + .load_preset(comfy_table::presets::UTF8_FULL) + .set_content_arrangement(ContentArrangement::Dynamic) + .set_width(100) + .set_header(header) + .add_rows(rows) + .set_constraints(vec![ + ColumnConstraint::LowerBoundary(Width::Fixed(12)), + ColumnConstraint::LowerBoundary(Width::Fixed(5)), + ColumnConstraint::LowerBoundary(Width::Fixed(10)), + ]); + + println!("{table}"); + let expected = " +┌───────────────────┬─────────────────────────────────────┬────────────┐ +│ a really long col ┆ b ┆ this is 10 │ +│ --- ┆ --- ┆ --- │ +│ i64 ┆ str ┆ i64 │ +╞═══════════════════╪═════════════════════════════════════╪════════════╡ +│ 1 ┆ ┆ 4 │ +├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤ +│ 2 ┆ this is a string value that will... ┆ 5 │ +├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤ +│ 3 ┆ null ┆ 6 │ +└───────────────────┴─────────────────────────────────────┴────────────┘"; + println!("{expected}"); + assert_table_line_width(&table, 72); + assert_eq!("\n".to_string() + &table.to_string(), expected); +} From a524a268493f0d0d38300cca74aaff12e6fed322 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Thu, 13 Oct 2022 23:07:25 +0200 Subject: [PATCH 06/13] fix: Wrong lower boundary constraint logic This bug has been introduced in commit bee764d, when doing the refactoring on how column content widths were calculated. The old logic worked with the full width including paddings, the new logic however only worked with the content width. Due to this, columns were sometimes pinned to a width that was too small for their actual content. --- CHANGELOG.md | 1 + src/utils/arrangement/constraint.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28234c0..e329f5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Dynamic content adjustment was active. - The table didn't fit into the the available space. - The header of a row was longer than its content. +- Fix wrong LowerBoundary calculation. This was introduced in commit bee764d, when this logic was refactored. [#90](https://github.com/Nukesor/comfy-table/issues/90). ## [6.1.0] - 2022-08-28 diff --git a/src/utils/arrangement/constraint.rs b/src/utils/arrangement/constraint.rs index 47ede74..cf3b404 100644 --- a/src/utils/arrangement/constraint.rs +++ b/src/utils/arrangement/constraint.rs @@ -45,7 +45,8 @@ pub fn evaluate( if let Some(min_width) = min(table, &column.constraint, table_width, visible_columns) { // In case a min_width is specified, we may already fix the size of the column. // We do this, if we know that the content is smaller than the min size. - if max_content_width <= min_width { + let max_width = max_content_width + column.padding_width(); + if max_width <= min_width { let width = absolute_width_with_padding(column, min_width); let info = ColumnDisplayInfo::new(column, width); infos.insert(column.index, info); From 24bdac5f114c704d31b03652b1345dc1ac9e0878 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 14:53:26 +0200 Subject: [PATCH 07/13] refactor: Don't pass table_width through everything --- src/utils/arrangement/constraint.rs | 16 +++++----------- src/utils/arrangement/disabled.rs | 2 +- src/utils/arrangement/dynamic.rs | 27 +++++++-------------------- src/utils/arrangement/mod.rs | 1 - 4 files changed, 13 insertions(+), 33 deletions(-) diff --git a/src/utils/arrangement/constraint.rs b/src/utils/arrangement/constraint.rs index cf3b404..b71cfad 100644 --- a/src/utils/arrangement/constraint.rs +++ b/src/utils/arrangement/constraint.rs @@ -13,7 +13,6 @@ use crate::{Column, Table}; /// - The Column is supposed to be hidden. pub fn evaluate( table: &Table, - table_width: Option, visible_columns: usize, infos: &mut DisplayInfos, column: &Column, @@ -25,9 +24,7 @@ pub fn evaluate( infos.insert(column.index, info); } Some(Absolute(width)) => { - if let Some(width) = - absolute_value_from_width(table, width, visible_columns, table_width) - { + if let Some(width) = absolute_value_from_width(table, width, visible_columns) { // The column should get always get a fixed width. let width = absolute_width_with_padding(column, width); let info = ColumnDisplayInfo::new(column, width); @@ -42,7 +39,7 @@ pub fn evaluate( _ => {} } - if let Some(min_width) = min(table, &column.constraint, table_width, visible_columns) { + if let Some(min_width) = min(table, &column.constraint, visible_columns) { // In case a min_width is specified, we may already fix the size of the column. // We do this, if we know that the content is smaller than the min size. let max_width = max_content_width + column.padding_width(); @@ -64,7 +61,6 @@ pub fn evaluate( pub fn min( table: &Table, constraint: &Option, - table_width: Option, visible_columns: usize, ) -> Option { let constraint = if let Some(constraint) = constraint { @@ -75,7 +71,7 @@ pub fn min( match constraint { LowerBoundary(width) | Boundaries { lower: width, .. } => { - absolute_value_from_width(table, width, visible_columns, table_width) + absolute_value_from_width(table, width, visible_columns) } _ => None, } @@ -91,7 +87,6 @@ pub fn min( pub fn max( table: &Table, constraint: &Option, - table_width: Option, visible_columns: usize, ) -> Option { let constraint = if let Some(constraint) = constraint { @@ -102,7 +97,7 @@ pub fn max( match constraint { UpperBoundary(width) | Boundaries { upper: width, .. } => { - absolute_value_from_width(table, width, visible_columns, table_width) + absolute_value_from_width(table, width, visible_columns) } _ => None, } @@ -113,13 +108,12 @@ pub fn absolute_value_from_width( table: &Table, width: &Width, visible_columns: usize, - table_width: Option, ) -> Option { match width { Width::Fixed(width) => Some(*width), Width::Percentage(percent) => { // Don't return a value, if we cannot determine the current table width. - let table_width = table_width?; + let table_width = table.width().map(usize::from)?; // Enforce at most 100% let percent = std::cmp::min(*percent, 100u16); diff --git a/src/utils/arrangement/disabled.rs b/src/utils/arrangement/disabled.rs index e99bc77..a3c9f79 100644 --- a/src/utils/arrangement/disabled.rs +++ b/src/utils/arrangement/disabled.rs @@ -20,7 +20,7 @@ pub fn arrange( let mut width = max_content_widths[column.index]; // Reduce the width, if a column has longer content than the specified MaxWidth constraint. - if let Some(max_width) = constraint::max(table, &column.constraint, None, visible_columns) { + if let Some(max_width) = constraint::max(table, &column.constraint, visible_columns) { if max_width < width { width = absolute_width_with_padding(column, max_width); } diff --git a/src/utils/arrangement/dynamic.rs b/src/utils/arrangement/dynamic.rs index 82425fb..1d06235 100644 --- a/src/utils/arrangement/dynamic.rs +++ b/src/utils/arrangement/dynamic.rs @@ -50,7 +50,6 @@ pub fn arrange( let (mut remaining_width, mut remaining_columns) = find_columns_that_fit_into_average( table, infos, - table_width, remaining_width, visible_columns, max_content_widths, @@ -64,7 +63,6 @@ pub fn arrange( (remaining_width, remaining_columns) = enforce_lower_boundary_constraints( table, infos, - table_width, remaining_width, remaining_columns, visible_columns, @@ -194,7 +192,6 @@ fn available_content_width( fn find_columns_that_fit_into_average( table: &Table, infos: &mut DisplayInfos, - table_width: usize, mut remaining_width: usize, visible_columns: usize, max_content_widths: &[u16], @@ -229,12 +226,7 @@ fn find_columns_that_fit_into_average( // two conditions are met: // - The average remaining space is bigger then the MaxWidth constraint. // - The actual max content of the column is bigger than the MaxWidth constraint. - if let Some(max_width) = constraint::max( - table, - &column.constraint, - Some(table_width), - visible_columns, - ) { + if let Some(max_width) = constraint::max(table, &column.constraint, visible_columns) { // Max/Min constraints always include padding! let average_space_with_padding = average_space + usize::from(column.padding_width()); @@ -307,7 +299,6 @@ fn find_columns_that_fit_into_average( fn enforce_lower_boundary_constraints( table: &Table, infos: &mut DisplayInfos, - table_width: usize, mut remaining_width: usize, mut remaining_columns: usize, visible_columns: usize, @@ -321,16 +312,12 @@ fn enforce_lower_boundary_constraints( } // Check whether the column has a LowerBoundary constraint. - let min_width = if let Some(min_width) = constraint::min( - table, - &column.constraint, - Some(table_width), - visible_columns, - ) { - min_width - } else { - continue; - }; + let min_width = + if let Some(min_width) = constraint::min(table, &column.constraint, visible_columns) { + min_width + } else { + continue; + }; // Only proceed if the average spaces is smaller than the specified lower boundary. if average_space >= min_width.into() { diff --git a/src/utils/arrangement/mod.rs b/src/utils/arrangement/mod.rs index abe1040..b5ff18e 100644 --- a/src/utils/arrangement/mod.rs +++ b/src/utils/arrangement/mod.rs @@ -26,7 +26,6 @@ pub(crate) fn arrange_content(table: &Table) -> Vec { if column.constraint.is_some() { constraint::evaluate( table, - table_width, visible_columns, &mut infos, column, From b5f725e62d9f5bb2509f6df3b48b4fec58bded14 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 14:53:51 +0200 Subject: [PATCH 08/13] refactor: Remove unnecessary rstest statements --- tests/all/content_arrangement_test.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/all/content_arrangement_test.rs b/tests/all/content_arrangement_test.rs index 870c57c..3964e9d 100644 --- a/tests/all/content_arrangement_test.rs +++ b/tests/all/content_arrangement_test.rs @@ -230,7 +230,7 @@ fn dynamic_full_width() { /// width the table has, if it's fully expanded. /// /// The same should be the case for values that're larget than this width. -#[rstest::rstest] +#[test] fn dynamic_exact_width() { let header = vec!["a\n---\ni64", "b\n---\ni64", "b_squared\n---\nf64"]; let rows = vec![ @@ -269,7 +269,7 @@ fn dynamic_exact_width() { /// Test that the formatting works as expected, if the table is slightly smaller than the max width /// of the table. -#[rstest::rstest] +#[test] fn dynamic_slightly_smaller() { let header = vec!["a\n---\ni64", "b\n---\ni64", "b_squared\n---\nf64"]; let rows = vec![ @@ -307,7 +307,7 @@ fn dynamic_slightly_smaller() { /// This failed on a python integration test case in the polars project. /// This a regression test. -#[rstest::rstest] +#[test] fn polar_python_test_tbl_width_chars() { let header = vec![ "a really long col\n---\ni64", From 6cb3addb81e48d5e008c9d019d88237f3cc2e0c8 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 14:54:22 +0200 Subject: [PATCH 09/13] tests: Add empty table test --- tests/all/constraints_test.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/all/constraints_test.rs b/tests/all/constraints_test.rs index e531bf5..c52e79d 100644 --- a/tests/all/constraints_test.rs +++ b/tests/all/constraints_test.rs @@ -335,3 +335,23 @@ fn min_max_boundary() { println!("{expected}"); assert_eq!("\n".to_string() + &table.to_string(), expected); } + +#[rstest::rstest] +#[case(ContentArrangement::Dynamic)] +#[case(ContentArrangement::Disabled)] +/// Empty table with zero width constraint. +fn empty_table(#[case] arrangement: ContentArrangement) { + let mut table = Table::new(); + table + .add_row(vec![""]) + .set_content_arrangement(arrangement) + .set_constraints(vec![Absolute(Fixed(0))]); + + println!("{table}"); + let expected = " ++---+ +| | ++---+"; + println!("{expected}"); + assert_eq!("\n".to_string() + &table.to_string(), expected); +} From d0600aff882822e41f22a287a6d2a20241ddff9a Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 15:58:58 +0200 Subject: [PATCH 10/13] fix: Mutability on &mut self for Table::column_iter --- CHANGELOG.md | 1 + src/table.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e329f5b..dbce8a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - The table didn't fit into the the available space. - The header of a row was longer than its content. - Fix wrong LowerBoundary calculation. This was introduced in commit bee764d, when this logic was refactored. [#90](https://github.com/Nukesor/comfy-table/issues/90). +- `Table::column_iter` no longer requires a `&mut self`, but only `&self`. ## [6.1.0] - 2022-08-28 diff --git a/src/table.rs b/src/table.rs index 6989be1..8227b79 100644 --- a/src/table.rs +++ b/src/table.rs @@ -502,7 +502,7 @@ impl Table { } /// Iterator over all columns - pub fn column_iter(&mut self) -> Iter { + pub fn column_iter(&self) -> Iter { self.columns.iter() } From 619df9695b8a149f9cfbe27123b01fb991a139ef Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 17:00:22 +0200 Subject: [PATCH 11/13] add: table::content_arrangement() --- CHANGELOG.md | 4 ++++ src/table.rs | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbce8a1..f28d77e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix wrong LowerBoundary calculation. This was introduced in commit bee764d, when this logic was refactored. [#90](https://github.com/Nukesor/comfy-table/issues/90). - `Table::column_iter` no longer requires a `&mut self`, but only `&self`. +### Added + +- Expose current ContentArrangement for table via `table.content_arrangement`. + ## [6.1.0] - 2022-08-28 ### Added diff --git a/src/table.rs b/src/table.rs index 8227b79..af19c1e 100644 --- a/src/table.rs +++ b/src/table.rs @@ -212,6 +212,11 @@ impl Table { self } + /// Get the current content arrangement of the table. + pub fn content_arrangement(&self) -> ContentArrangement { + self.arrangement.clone() + } + /// Set the delimiter used to split text in all cells. /// /// A custom delimiter on a cell in will overwrite the column's delimiter.\ From b29e1371cabd85f5b18e3bdc4f522473edf38df8 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 17:01:23 +0200 Subject: [PATCH 12/13] change: Expose some util functions via integration_test feature --- Cargo.toml | 2 ++ src/lib.rs | 6 ++++++ src/utils/arrangement/mod.rs | 6 +++--- src/utils/mod.rs | 4 ++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3553a22..46c1b2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,8 @@ default = ["tty"] tty = ["crossterm"] # This flag is for library debugging only! debug = [] +# This feature is used to expose internal functionality for integration testing. +integration_test= [] [dependencies] crossterm = { version = "0.25", optional = true } diff --git a/src/lib.rs b/src/lib.rs index 8220eb0..6728248 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,12 @@ mod column; mod row; mod style; mod table; +#[cfg(feature = "integration_test")] +/// We publicly expose the internal [utils] module for our integration tests. +/// There's some logic we need from inside here. +/// The API inside of this isn't considered stable and shouldnt' be used. +pub mod utils; +#[cfg(not(feature = "integration_test"))] mod utils; pub use crate::cell::{Cell, Cells}; diff --git a/src/utils/arrangement/mod.rs b/src/utils/arrangement/mod.rs index b5ff18e..772e349 100644 --- a/src/utils/arrangement/mod.rs +++ b/src/utils/arrangement/mod.rs @@ -4,16 +4,16 @@ use super::ColumnDisplayInfo; use crate::style::ContentArrangement; use crate::table::Table; -mod constraint; +pub mod constraint; mod disabled; mod dynamic; -mod helper; +pub mod helper; type DisplayInfos = BTreeMap; /// Determine the width of each column depending on the content of the given table. /// The results uses Option, since users can choose to hide columns. -pub(crate) fn arrange_content(table: &Table) -> Vec { +pub fn arrange_content(table: &Table) -> Vec { let table_width = table.width().map(usize::from); let mut infos = BTreeMap::new(); diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 3d62d55..61fb250 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,5 +1,5 @@ -mod arrangement; -mod formatting; +pub mod arrangement; +pub mod formatting; use crate::style::{CellAlignment, ColumnConstraint}; use crate::{Column, Table}; From 286ff73da4f486c80861419bd12dcf9e8b216de2 Mon Sep 17 00:00:00 2001 From: Arne Beer Date: Fri, 14 Oct 2022 17:09:02 +0200 Subject: [PATCH 13/13] Reintroduce proptest suite The proptest suite now contains extensive tests that ensure constraints will be respected. --- .github/workflows/test.yml | 2 +- tests/all/property_test.proptest-regressions | 3 + tests/all/property_test.rs | 266 +++++++++++++++++-- 3 files changed, 241 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8039b73..a73256e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -61,7 +61,7 @@ jobs: if: ${{ !matrix.minimal_setup }} - name: cargo test - run: cargo test --target=${{ matrix.target }} + run: cargo test --target=${{ matrix.target }} --features=integration_test if: ${{ !matrix.minimal_setup }} - name: cargo build without default features and without dev dependencies diff --git a/tests/all/property_test.proptest-regressions b/tests/all/property_test.proptest-regressions index 891ca29..e5097f3 100644 --- a/tests/all/property_test.proptest-regressions +++ b/tests/all/property_test.proptest-regressions @@ -10,3 +10,6 @@ cc a2dac653cf48baabb2b2e22f97084bc1b199692f2574f966efefb35a1b024f43 # shrinks to cc d23c0c7ebc36ec192a9532fd94c128e35560c07255add62c1f9dd1459b3366ba # shrinks to table = Table { columns: [Column { index: 0, padding: (1, 1), cell_alignment: None, max_content_width: 0, constraint: Some(MinPercentage(98)) }], style: {TopBorder: '─', HeaderLines: '═', TopBorderIntersections: '┬', RightBorder: '│', BottomLeftCorner: '╰', TopRightCorner: '╮', TopLeftCorner: '╭', BottomRightCorner: '╯', BottomBorder: '─', BottomBorderIntersections: '┴', RightHeaderIntersection: '╡', LeftBorder: '│', LeftHeaderIntersection: '╞', VerticalLines: '┆', MiddleHeaderIntersections: '╪', LeftBorderIntersections: '├', MiddleIntersections: '┼', HorizontalLines: '╌', RightBorderIntersections: '┤'}, header: None, rows: [Row { index: Some(0), cells: [Cell { content: [""], alignment: None, fg: None, bg: None, attributes: [] }] }], arrangement: Disabled, no_tty: false, table_width: Some(669), enforce_styling: false } cc d7a48cda89aeb0fce5cede6f45c8387e2df00cb3bf305ecefb5d48c13074faf8 # shrinks to table = Table { columns: [], style: {HeaderLines: '═', RightBorder: '│', BottomBorder: '─', LeftHeaderIntersection: '╞', BottomBorderIntersections: '┴', BottomLeftCorner: '╰', TopRightCorner: '╮', MiddleHeaderIntersections: '╪', MiddleIntersections: '┼', TopLeftCorner: '╭', LeftBorderIntersections: '├', TopBorder: '─', HorizontalLines: '╌', TopBorderIntersections: '┬', RightHeaderIntersection: '╡', LeftBorder: '│', VerticalLines: '┆', RightBorderIntersections: '┤', BottomRightCorner: '╯'}, header: None, rows: [Row { index: Some(0), cells: [], max_height: None }], arrangement: DynamicFullWidth, delimiter: None, no_tty: false, table_width: Some(2), enforce_styling: false } cc d5305ce51fb8e961cc04af56393095c991b9ad218ac62b4f85e6abf218b8b2ad # shrinks to table = Table { columns: [Column { index: 0, padding: (1, 1), delimiter: None, cell_alignment: None, max_content_width: 0, constraint: Some(Percentage(40)) }, Column { index: 1, padding: (1, 1), delimiter: None, cell_alignment: None, max_content_width: 30, constraint: Some(MinPercentage(0)) }], style: {LeftHeaderIntersection: '╞', MiddleHeaderIntersections: '╪', MiddleIntersections: '┼', BottomBorder: '─', RightBorder: '│', TopRightCorner: '╮', VerticalLines: '┆', HorizontalLines: '╌', TopLeftCorner: '╭', BottomLeftCorner: '╰', LeftBorder: '│', HeaderLines: '═', BottomRightCorner: '╯', RightHeaderIntersection: '╡', BottomBorderIntersections: '┴', TopBorderIntersections: '┬', LeftBorderIntersections: '├', RightBorderIntersections: '┤', TopBorder: '─'}, header: None, rows: [Row { index: Some(0), cells: [Cell { content: [""], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }, Cell { content: ["¡\u{0} 0\u{0}a0¡a¡¡¡0 ¡a ¡¡a¡¡¡ ¡¡¡¡"], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }], max_height: None }], arrangement: Dynamic, delimiter: None, no_tty: false, table_width: Some(58), enforce_styling: false } +cc 0d4d3573fbe99ac650d858c7a306f8c2ed7cf5fb5998f2c30495bb37849c2c5b # shrinks to mut table = Table { columns: [Column { index: 0, padding: (1, 1), delimiter: None, cell_alignment: None, constraint: Some(Absolute(Percentage(0))) }], style: {MiddleHeaderIntersections: '=', HorizontalLines: '-', BottomBorderIntersections: '+', BottomRightCorner: '+', RightBorderIntersections: '|', RightBorder: '|', BottomBorder: '-', MiddleIntersections: '+', TopRightCorner: '+', LeftHeaderIntersection: '+', TopLeftCorner: '+', BottomLeftCorner: '+', VerticalLines: '|', TopBorderIntersections: '+', LeftBorder: '|', TopBorder: '-', HeaderLines: '=', LeftBorderIntersections: '|', RightHeaderIntersection: '+'}, header: None, rows: [Row { index: Some(0), cells: [Cell { content: [""], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }], max_height: None }], arrangement: DynamicFullWidth, delimiter: None, no_tty: false, use_stderr: false, width: None, enforce_styling: false, style_text_only: false }, table_width = 0 +cc 5e2562ec5b8d70abf05ea49ba65a178f086f1f3ed9251506f7b3e6bbb9091015 # shrinks to mut table = Table { columns: [Column { index: 0, padding: (1, 1), delimiter: None, cell_alignment: None, constraint: None }, Column { index: 1, padding: (1, 1), delimiter: None, cell_alignment: None, constraint: None }, Column { index: 2, padding: (1, 1), delimiter: None, cell_alignment: None, constraint: Some(UpperBoundary(Percentage(0))) }], style: {BottomBorderIntersections: '+', MiddleIntersections: '+', HorizontalLines: '-', RightHeaderIntersection: '+', VerticalLines: '|', TopLeftCorner: '+', BottomLeftCorner: '+', LeftBorder: '|', HeaderLines: '=', RightBorderIntersections: '|', TopBorder: '-', MiddleHeaderIntersections: '=', TopRightCorner: '+', RightBorder: '|', LeftBorderIntersections: '|', BottomRightCorner: '+', BottomBorder: '-', TopBorderIntersections: '+', LeftHeaderIntersection: '+'}, header: None, rows: [Row { index: Some(0), cells: [], max_height: None }, Row { index: Some(1), cells: [], max_height: None }, Row { index: Some(2), cells: [], max_height: None }, Row { index: Some(3), cells: [], max_height: None }, Row { index: Some(4), cells: [], max_height: None }, Row { index: Some(5), cells: [], max_height: None }, Row { index: Some(6), cells: [], max_height: None }, Row { index: Some(7), cells: [Cell { content: [""], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }, Cell { content: [""], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }, Cell { content: [""], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }], max_height: None }], arrangement: Dynamic, delimiter: None, no_tty: false, use_stderr: false, width: None, enforce_styling: false, style_text_only: false }, table_width = 351 +cc ac6d94f68bcd585b1ffff6b3b3e10b625d79ad96ec317ad147601c68bee5aa99 # shrinks to mut table = Table { columns: [Column { index: 0, padding: (1, 1), delimiter: None, cell_alignment: None, constraint: Some(UpperBoundary(Percentage(127))) }, Column { index: 1, padding: (1, 1), delimiter: None, cell_alignment: Some(Left), constraint: Some(LowerBoundary(Percentage(23))) }, Column { index: 2, padding: (1, 1), delimiter: None, cell_alignment: Some(Left), constraint: Some(ContentWidth) }], style: {TopRightCorner: '+', RightHeaderIntersection: '+', LeftBorderIntersections: '|', RightBorderIntersections: '|', BottomRightCorner: '+', RightBorder: '|', MiddleHeaderIntersections: '=', TopLeftCorner: '+', TopBorderIntersections: '+', MiddleIntersections: '+', BottomBorderIntersections: '+', LeftHeaderIntersection: '+', BottomLeftCorner: '+', HorizontalLines: '-', TopBorder: '-', BottomBorder: '-', LeftBorder: '|', VerticalLines: '|', HeaderLines: '='}, header: None, rows: [Row { index: Some(0), cells: [], max_height: None }, Row { index: Some(1), cells: [], max_height: None }, Row { index: Some(2), cells: [Cell { content: [""], delimiter: None, alignment: None, fg: None, bg: None, attributes: [] }, Cell { content: ["rz_m___Dksx_c__KdvK__fh"], delimiter: None, alignment: Some(Right), fg: None, bg: None, attributes: [] }, Cell { content: ["i_swmNSsyuOtR_UfklUqR_"], delimiter: None, alignment: Some(Center), fg: None, bg: None, attributes: [] }], max_height: None }, Row { index: Some(3), cells: [Cell { content: ["xJ_iEc_IGix____hf_GPlKlnqKZr_"], delimiter: None, alignment: Some(Left), fg: None, bg: None, attributes: [] }], max_height: None }], arrangement: Dynamic, delimiter: None, no_tty: false, use_stderr: false, width: None, enforce_styling: false, style_text_only: false }, table_width = 77 diff --git a/tests/all/property_test.rs b/tests/all/property_test.rs index 1c49715..3e6435d 100644 --- a/tests/all/property_test.rs +++ b/tests/all/property_test.rs @@ -1,6 +1,4 @@ use ::proptest::prelude::*; -use comfy_table::modifiers::UTF8_ROUND_CORNERS; -use comfy_table::presets::UTF8_FULL; use comfy_table::ColumnConstraint::*; use comfy_table::Width::*; use comfy_table::*; @@ -80,7 +78,15 @@ fn columns_and_rows() -> impl Strategy< cell_alignments.push(cell_alignment()); } // Add a strategy that creates random cell content with a length of 0 to column_count - rows.push(::proptest::collection::vec(".*", 0..column_count as usize)); + // TODO: Move this back to ".*" once we properly handle multi-space UTF-8 chars. + // UTF-8 characters completely break table alignment in edge-case situations (e.g. 1 space columns). + // UTF-8 characters can be multiple characters wide, which conflicts with the 1 space + // column fallback, as well as fixed-width-, percental- and max-column-constraints. + // As a result, we cannot check this with proptest, as this is inherently broken. + rows.push(::proptest::collection::vec( + "[A-Za-z_]*", + 0..column_count as usize, + )); } let mut constraints = Vec::new(); let mut column_alignments = Vec::new(); @@ -93,13 +99,17 @@ fn columns_and_rows() -> impl Strategy< }) } +/// We test the Row::max_height with a few values. +fn table_width() -> impl Strategy { + 0..1000u16 +} + prop_compose! { /// The ultimate test /// This creates a table from a combination of all "random" selectors above. fn table() (arrangement in content_arrangement(), max_height in max_height(), - table_width in 0..1000u16, (rows, constraints, cell_alignments, column_alignments) in columns_and_rows()) -> Table { let mut table = Table::new(); @@ -140,10 +150,7 @@ prop_compose! { } - table.set_width(table_width) - .set_content_arrangement(arrangement) - .load_preset(UTF8_FULL) - .apply_modifier(UTF8_ROUND_CORNERS); + table.set_content_arrangement(arrangement); table } } @@ -155,29 +162,230 @@ proptest! { config })] #[test] - fn random_tables(table in table()) { + fn random_tables(mut table in table(), table_width in table_width()) { + table.set_width(table_width); + // Make sure the table builds without any panics - let _formatted = table.to_string(); - - // Ensure that all lines have the same lenght. - // This check has been disabled for now. - // UTF-8 characters completely break table alignment in edge-case situations (e.g. 1 space columns). - // UTF-8 characters can be multiple characters wide, which conflicts with the 1 space - // column fallback, as well as fixed-width-, percental- and max-column-constraints. - // As a result, we cannot check this with proptest, as this is inherently broken. - // - // let lines: Vec<&str> = formatted.split_terminator('\n').collect(); - // - // let mut line_iter = lines.iter(); - // let line_length = if let Some(line) = line_iter.next() { - // line.width() - // } else { - // 0 - // }; - //for line in line_iter { - // if line.width() != line_length { - // return Err(TestCaseError::Fail("Each line of a printed table has to have the same length!".into())) + let formatted = table.to_string(); + + // We'll take a look at each individual line to ensure they all share some properties. + let lines: Vec = formatted.split_terminator('\n').map(|line| line.to_owned()).collect(); + let mut line_iter = lines.iter(); + + // ----- Table width check ------ + + // Get the length of the very first line. + // We're lateron going to ensure, that all lines have the same length. + let line_length = if let Some(line) = line_iter.next() { + line.len() + } else { + 0 + }; + + for line in line_iter { + // Make sure all lines have the same length + if line.len() != line_length { + return build_error(&formatted, "Each line of a printed table has to have the same length!"); + } + } + + // TODO: This is a bit tricky. + // A table can be larger than the specified width, if the user forces it to be + // larger. + // Make sure that the table is within its width, if arrangement isn't enabled. + //match content_arrangement{ + // ContentArrangement::Disabled => (), + // _ => { + // let expected_max = table.width().unwrap(); + // let actual = line_length; + // if actual > expected_max.into() { + // return build_error( + // &formatted, + // &format!("Expected table to be smaller than line length!\n\ + // Actual: {actual}, Expected max: {expected_max}\n\ + // Arrangement: {content_arrangement:?}" + // )); + // } // } //} + + #[cfg(feature = "integration_test")] + // Only run this test, if the `integration_test` is enabled. + // Without this flag, we don't have access to some util functions in comfy_table, that + // aren't by default. + enforce_constraints(&table, formatted, lines)? + } +} + +fn build_error(table: &str, context: &str) -> Result<(), TestCaseError> { + Err(TestCaseError::Fail( + format!("\n{context}:\n{table}\n").into(), + )) +} + +/// Enforce that Column constraints are enforced as expected in `Dynamic` mode. +#[cfg(feature = "integration_test")] +fn enforce_constraints( + table: &Table, + formatted: String, + lines: Vec, +) -> Result<(), TestCaseError> { + let content_arrangement = table.content_arrangement(); + // Don't run the following for disabled or full-width arrangement. + // These constraints kind of mess with all kinds of assertions we can make, which is why we + // skip them. + match content_arrangement { + ContentArrangement::Dynamic => (), + _ => return Ok(()), + } + + // Extract the constraints for each table + // Also remove hidden columns + let constraints: Vec> = table + .column_iter() + .map(|col| col.constraint().cloned()) + .filter(|constraint| { + if let Some(ColumnConstraint::Hidden) = constraint { + false + } else { + true + } + }) + .collect(); + + let line_iter = lines.iter(); + + for line in line_iter { + // Split the line along the column delimiter. + // This allows us to ensure that each column is inside its constraints. + let line_parts: Vec = line.split('|').map(|col| col.to_string()).collect(); + + // Skip the line if there're fewer vertical delimiters than columns + borders. + // If that's the case, we're currently looking at a border or a delimiter line. + if line_parts.len() < (constraints.len() + 2) { + continue; + } + + // The left and right borders will produce empty strings, let's filter those out. + let line_parts: Vec = line_parts + .into_iter() + .filter(|part| !part.is_empty()) + .collect(); + + for (index, (part, constraint)) in line_parts.iter().zip(constraints.iter()).enumerate() { + let constraint = match constraint { + Some(constraint) => constraint, + // No constraint, we're good to go. + None => continue, + }; + // Get the actual length of the part. + let actual = part.len(); + + match constraint { + ColumnConstraint::Hidden => panic!("This shouldn't happen"), + // No need to check, if the column can be as wide as the content. + ColumnConstraint::ContentWidth => continue, + // Absolute width is defined. + ColumnConstraint::Absolute(absolute) => { + let mut expected = absolute_width(&table, absolute); + // The minimal amount of chars per column (with default padding) + // is 3 chars. 2 padding + 1 char content. + if expected < 3 { + expected = 3; + } + if actual != expected.into() { + return build_error( + &formatted, + &format!( + "Column {index} for should have absolute width of {expected}.\n\ + Actual width is {actual}.\n\ + {absolute:?} for line '{line}', part '{part}'" + ), + ); + } + } + ColumnConstraint::LowerBoundary(lower) => { + let expected_lower = absolute_width(&table, lower); + if actual < expected_lower.into() { + return build_error( + &formatted, + &format!( + "Column {index} has a lower bound of {expected_lower}.\n\ + Actual width is {actual}.\n\ + {lower:?} for line '{line}', part '{part}'" + ), + ); + } + } + ColumnConstraint::UpperBoundary(upper) => { + let mut expected_upper = absolute_width(&table, upper); + // The minimal amount of chars per column (with default padding) + // is 3 chars. 2 padding + 1 char content. + if expected_upper < 3 { + expected_upper = 3; + } + + if actual > expected_upper.into() { + return build_error( + &formatted, + &format!( + "Column {index} has a upper bound of {expected_upper}.\n\ + Actual width is {actual}.\n\ + {upper:?} for line '{line}', part '{part}'" + ), + ); + } + } + ColumnConstraint::Boundaries { lower, upper } => { + let expected_lower = absolute_width(&table, lower); + let mut expected_upper = absolute_width(&table, upper); + // The minimal amount of chars per column (with default padding) + // is 3 chars. 2 padding + 1 char content. + if expected_upper < 3 { + expected_upper = 3; + } + + if actual < expected_lower.into() { + return build_error( + &formatted, + &format!( + "Column {index} has a lower bound of {expected_lower}.\n\ + Actual width is {actual}.\n\ + {lower:?} for line '{line}', part '{part}'" + ), + ); + } + + if actual > expected_upper.into() { + return build_error( + &formatted, + &format!( + "Column {index} has a upper bound of {expected_upper}.\n\ + Actual width is {actual}.\n\ + {upper:?} for line '{line}', part '{part}'" + ), + ); + } + } + } + } } + + Ok(()) +} + +/// Resolve an absolute value from a given boundary +#[cfg(feature = "integration_test")] +pub fn absolute_width(table: &Table, width: &Width) -> u16 { + let visible_columns = table + .column_iter() + .filter(|column| !column.is_hidden()) + .count(); + + comfy_table::utils::arrangement::constraint::absolute_value_from_width( + table, + width, + visible_columns, + ) + .expect("Expected table to have a width") }