Merge pull request #115 from Nukesor/development

Several formatting fixes and other fixes

CHANGELOG.md

@@ -10,6 +10,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 - Fix a panic when working with extreme paddings, where `(padding.left + padding.right) > u16::MAX`.
 - Fix a panic when working with extremely long content, where `(content_width + padding) > u16::MAX`.
+- Properly enforce lower boundery constraints.
+  Previously, "normal" columns were allocated before lower boundaries were respected.
+  This could lead to scenarios, where the table would grow beyond the specified size, when there was a lower boundary.
+- Fix calculation of column widths for empty columns.
+  The minimum content width for a column is `1` char, but the `column_max_content_widths` function on the table returned a `0` width for fully empty columns.
+  This resulted in tables becoming larger than specified if there were any empty columns.
+
+## Misc
+
+- Extend property tests, which lead to the discovery some bugs.
 
 ## [7.0.0] - 2023-06-06
 

src/style/column.rs

@@ -16,8 +16,13 @@ pub enum ColumnConstraint {
     /// Enforce a absolute width for a column.
     Absolute(Width),
     /// Specify a lower boundary, either fixed or as percentage of the total width.
+    /// A column with this constraint will be at least as wide as specified.
+    /// If the content isn't as long as that boundary, it will be padded.
+    /// If the column has longer content and is allowed to grow, the column may take more space.
     LowerBoundary(Width),
     /// Specify a upper boundary, either fixed or as percentage of the total width.
+    /// A column with this constriant will be at most as wide as specified.
+    /// The column may be smaller than that width.
     UpperBoundary(Width),
     /// Specify both, an upper and a lower boundary.
     Boundaries { lower: Width, upper: Width },

src/table.rs

@@ -682,7 +682,9 @@ impl Table {
             // Get the max width for each cell of the row
             let row_max_widths = row.max_content_widths();
             for (index, width) in row_max_widths.iter().enumerate() {
-                let width = (*width).try_into().unwrap_or(u16::MAX);
+                let mut width = (*width).try_into().unwrap_or(u16::MAX);
+                // A column's content is at least 1 char wide.
+                width = std::cmp::max(1, width);
 
                 // Set a new max, if the current cell is the longest for that column.
                 let current_max = max_widths[index];

src/utils/arrangement/dynamic.rs

@@ -10,13 +10,13 @@ use crate::{Column, Table};
 /// Try to find the best fit for a given content and table_width
 ///
 /// 1. Determine the amount of available space after applying fixed columns, padding, and borders.
-/// 2. Check if there are any columns that require less space than the average
-///    remaining space for the remaining columns. (This includes the MaxWidth constraint).
-/// 3. Take those columns, fix their size and add the surplus in space to the remaining space.
-/// 4. Repeat step 2-3 until no columns with smaller size than average remaining space are left.
-/// 5. Now that we know how much space we have to work with, we have to check again for
+/// 2. Now that we know how much space we have to work with, we have to check again for
 ///    LowerBoundary constraints. If there are any columns that have a higher LowerBoundary,
 ///    we have to fix that column to this size.
+/// 3. Check if there are any columns that require less space than the average
+///    remaining space for the remaining columns. (This includes the MaxWidth constraint).
+/// 4. Take those columns, fix their size and add the surplus in space to the remaining space.
+/// 5. Repeat step 2-3 until no columns with smaller size than average remaining space are left.
 /// 6. At this point, the remaining spaces is equally distributed between all columns.
 ///    It get's a little tricky now. Check the documentation of [optimize_space_after_split]
 ///    for more information.
@@ -36,26 +36,18 @@ pub fn arrange(
 
     // Step 1
     // Find out how much space there is left.
-    let remaining_width: usize =
+    let mut remaining_width: usize =
         available_content_width(table, infos, visible_columns, table_width);
+    let mut remaining_columns = count_remaining_columns(visible_columns, infos);
 
     #[cfg(feature = "debug")]
     println!(
         "dynamic::arrange: Table width: {table_width}, Start remaining width {remaining_width}"
     );
+    #[cfg(feature = "debug")]
+    println!("dynamic::arrange: Max content widths: {max_content_widths:#?}");
 
-    // 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_that_fit_into_average(
-        table,
-        infos,
-        remaining_width,
-        visible_columns,
-        max_content_widths,
-    );
-
-    // Step 5.
+    // Step 2.
     //
     // Iterate through all undecided columns and enforce LowerBoundary constraints, if they're
     // bigger than the current average space.
@@ -69,6 +61,18 @@ pub fn arrange(
         );
     }
 
+    // Step 3-5.
+    // 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_that_fit_into_average(
+        table,
+        infos,
+        remaining_width,
+        remaining_columns,
+        visible_columns,
+        max_content_widths,
+    );
+
     #[cfg(feature = "debug")]
     {
         println!("After less than average: {infos:#?}");
@@ -193,11 +197,11 @@ fn find_columns_that_fit_into_average(
     table: &Table,
     infos: &mut DisplayInfos,
     mut remaining_width: usize,
+    mut remaining_columns: usize,
     visible_columns: usize,
     max_content_widths: &[u16],
 ) -> (usize, usize) {
     let mut found_smaller = true;
-    let mut remaining_columns = count_remaining_columns(visible_columns, infos);
     while found_smaller {
         found_smaller = false;
 

src/utils/formatting/content_split/mod.rs

@@ -95,22 +95,19 @@ pub fn split_line(line: &str, info: &ColumnDisplayInfo, delimiter: char) -> Vec<
             // Only add delimiter, if we're not on a fresh line
             if !new_line {
                 current_line.push(delimiter);
-                //remaining_width = remaining_width.saturating_sub(1);
             }
 
             let (mut next, mut remaining) = split_long_word(remaining_width, &next);
 
-            // TODO: This is a pretty hefty hack, but it's needed for now.
+            // This is a ugly hack, but it's needed for now.
             //
-            // Scenario: We try to split a word that doesn't fit into the current line.
-            // It's supposed to be a new line, with a width of 1. However, the next char in line
-            // is a multi-character UTF-8 symbol.
+            // Scenario: The current column has to have a width of 1 and we work with a new line.
+            // However, the next char is a multi-character UTF-8 symbol.
             //
-            // Since a, for instance, two-character wide symbol doesn't fit into a 1-character
-            // column, this code would loop endlessly. (There's no legitimate way to split that
-            // word.)
+            // Since a multi-character wide symbol doesn't fit into a 1-character column,
+            // this code would loop endlessly. (There's no legitimate way to split that character.)
             // Hence, we have to live with the fact, that this line will look broken, as we put a
-            // two-character wide symbol into it.
+            // two-character wide symbol into it, despite the line being formatted for 1 character.
             if new_line && next.is_empty() {
                 let mut chars = remaining.chars();
                 next.push(chars.next().unwrap());

tests/all/add_predicate.rs

@@ -32,7 +32,7 @@ fn add_predicate_single_true() {
 |                      | add some             |                        |
 |                      | multi line stuff     |                        |
 +----------------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -61,7 +61,7 @@ fn add_predicate_single_false() {
 +================================================================+
 | This is a text | This is another text | This is the third text |
 +----------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -98,7 +98,7 @@ fn add_predicate_single_mixed() {
 |                      | add some             |                        |
 |                      | multi line stuff     |                        |
 +----------------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -127,7 +127,7 @@ fn add_predicate_single_wrong_row_count() {
 +================================================================+
 | This is a text | This is another text | This is the third text |
 +----------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -161,7 +161,7 @@ fn add_predicate_multi_true() {
 |                      | add some             |                        |
 |                      | multi line stuff     |                        |
 +----------------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -190,7 +190,7 @@ fn add_predicate_multi_false() {
 +================================================================+
 | This is a text | This is another text | This is the third text |
 +----------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -232,7 +232,7 @@ fn add_predicate_multi_mixed() {
 |                      | add some             |                        |
 |                      | multi line stuff     |                        |
 +----------------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -261,5 +261,5 @@ fn add_predicate_multi_wrong_rows_count() {
 +================================================================+
 | This is a text | This is another text | This is the third text |
 +----------------+----------------------+------------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }

tests/all/alignment_test.rs

@@ -44,5 +44,5 @@ fn cell_alignment() {
 |---------------------+---------------------+---------------------|
 | Left                |        Center       |               Right |
 +---------------------+---------------------+---------------------+";
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }

tests/all/combined_test.rs

@@ -52,5 +52,5 @@ fn combined_features() {
 │                     ┆ atshoulddynamicallywrap       ┆                        │
 └─────────────────────┴───────────────────────────────┴────────────────────────┘";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }

tests/all/constraints_test.rs

@@ -53,7 +53,7 @@ fn fixed_max_min_constraints() {
 |          | stuff  |          |
 +----------+--------+----------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 
     // Now try this again when using dynamic content arrangement
     // The table tries to arrange to 28 characters,
@@ -98,7 +98,7 @@ fn fixed_max_min_constraints() {
 |          | f  |          |
 +----------+----+----------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -121,7 +121,7 @@ fn unnecessary_max_min_constraints() {
 |      | multi line stuff     |                        |
 +------+----------------------+------------------------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 
     // Now test for dynamic content arrangement
     table.set_content_arrangement(ContentArrangement::Dynamic);
@@ -137,7 +137,7 @@ fn unnecessary_max_min_constraints() {
 |      | multi line stuff     |                        |
 +------+----------------------+------------------------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -175,7 +175,7 @@ fn constraints_bigger_than_table_width() {
 | l |                              |                        |
 +---+------------------------------+------------------------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -205,7 +205,7 @@ fn percentage() {
 |       | stuff         |              |
 +-------+---------------+--------------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -228,7 +228,7 @@ fn max_100_percentage() {
 +--------------------------------------+";
     println!("{expected}");
     assert_table_line_width(&table, 40);
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -261,7 +261,7 @@ fn percentage_second() {
 |              | stuff    |          |
 +--------------+----------+----------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -294,7 +294,7 @@ fn max_percentage() {
 |      | stuff    |          |
 +------+----------+----------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[test]
@@ -333,7 +333,7 @@ fn min_max_boundary() {
 |                  | stuff         |          |
 +------------------+---------------+----------+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }
 
 #[rstest::rstest]
@@ -353,5 +353,5 @@ fn empty_table(#[case] arrangement: ContentArrangement) {
 |   |
 +---+";
     println!("{expected}");
-    assert_eq!("\n".to_string() + &table.to_string(), expected);
+    assert_eq!(expected, "\n".to_string() + &table.to_string());
 }