small improvements to variable names and comments

tertsdiepraam · tertsdiepraam · commit b2d435bc8161 · 2025-04-09T23:11:15.000+02:00
diff --git a/src/lib.rs b/src/lib.rs
@@ -79,7 +79,7 @@ pub struct GridOptions {
 #[derive(PartialEq, Eq, Debug)]
 struct Dimensions {
     /// The number of lines in the grid.
-    num_lines: usize,
+    num_rows: usize,
 
     /// The width of each column in the grid. The length of this vector serves
     /// as the number of columns.
@@ -120,7 +120,7 @@ impl<T: AsRef<str>> Grid<T> {
             widths,
             widest_cell_width,
             dimensions: Dimensions {
-                num_lines: 0,
+                num_rows: 0,
                 widths: Vec::new(),
             },
         };
@@ -140,7 +140,7 @@ impl<T: AsRef<str>> Grid<T> {
 
     /// The number of rows this display takes up.
     pub fn row_count(&self) -> usize {
-        self.dimensions.num_lines
+        self.dimensions.num_rows
     }
 
     /// The width of each column
@@ -172,7 +172,7 @@ impl<T: AsRef<str>> Grid<T> {
         }
 
         Dimensions {
-            num_lines,
+            num_rows: num_lines,
             widths: column_widths,
         }
     }
@@ -181,7 +181,7 @@ impl<T: AsRef<str>> Grid<T> {
         if self.cells.len() == 1 {
             let cell_widths = self.widths[0];
             return Dimensions {
-                num_lines: 1,
+                num_rows: 1,
                 widths: vec![cell_widths],
             };
         }
@@ -191,36 +191,41 @@ impl<T: AsRef<str>> Grid<T> {
         // If it exceeds terminal's width, return, since it is impossible to fit.
         if widest_column > self.options.width {
             return Dimensions {
-                num_lines: self.cells.len(),
+                num_rows: self.cells.len(),
                 widths: vec![self.widest_cell_width],
             };
         }
 
-        // Calculate minimum number of columns with the widest column size.
+        // Calculate the number of columns if all columns had the size of the largest
+        // column. This is a lower bound on the number of columns.
         let min_columns = self
             .cells
             .len()
             .min((self.options.width + self.options.filling.width()) / widest_column);
-        // Caculate minimum number of rows.
-        let min_lines = div_ceil(self.cells.len(), min_columns);
+
+        // Calculate maximum number of lines and columns.
+        let max_rows = div_ceil(self.cells.len(), min_columns);
 
         // This is a potential dimension, which can definitely fit all of the cells.
-        let mut potential_dimension = self.compute_dimensions(min_lines, min_columns);
-        // If all of the cells can be fit on one line, return.
-        if min_lines == 1 {
+        let mut potential_dimension = self.compute_dimensions(max_rows, min_columns);
+
+        // If all of the cells can be fit on one line, return immediately.
+        if max_rows == 1 {
             return potential_dimension;
         }
 
         // Try to increase number of columns, to see if new dimension can still fit.
-        for num_columns in min_columns + 1.. {
-            let new_width = (num_columns - 1) * self.options.filling.width();
-            if new_width > self.options.width {
+        for num_columns in min_columns + 1..self.cells.len() {
+            let Some(adjusted_width) = self
+                .options
+                .width
+                .checked_sub((num_columns - 1) * self.options.filling.width())
+            else {
                 break;
-            }
-
+            };
             let num_rows = div_ceil(self.cells.len(), num_columns);
             let new_dimension = self.compute_dimensions(num_rows, num_columns);
-            if new_dimension.widths.iter().sum::<usize>() <= self.options.width - new_width {
+            if new_dimension.widths.iter().sum::<usize>() <= adjusted_width {
                 potential_dimension = new_dimension;
             }
         }
@@ -252,7 +257,7 @@ impl<T: AsRef<str>> fmt::Display for Grid<T> {
         // get exactly right.
         let padding = " ".repeat(self.widest_cell_width + self.options.filling.width());
 
-        for y in 0..self.dimensions.num_lines {
+        for y in 0..self.dimensions.num_rows {
             // Current position on the line.
             let mut cursor: usize = 0;
             for x in 0..self.dimensions.widths.len() {
@@ -261,7 +266,7 @@ impl<T: AsRef<str>> fmt::Display for Grid<T> {
                 let (current, offset) = match self.options.direction {
                     Direction::LeftToRight => (y * self.dimensions.widths.len() + x, 1),
                     Direction::TopToBottom => {
-                        (y + self.dimensions.num_lines * x, self.dimensions.num_lines)
+                        (y + self.dimensions.num_rows * x, self.dimensions.num_rows)
                     }
                 };
 
diff --git a/tests/test.rs b/tests/test.rs
@@ -345,6 +345,26 @@ fn use_max_possible_width() {
     assert_eq!(grid.row_count(), 2);
 }
 
+#[test]
+fn dont_use_max_possible_width() {
+    let grid = Grid::new(
+        vec![
+            "test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8", "test9",
+            "test10", "test11",
+        ],
+        GridOptions {
+            filling: Filling::Text("||".to_string()),
+            direction: Direction::TopToBottom,
+            width: 69,
+        },
+    );
+
+    let bits = "test1||test3||test5||test7||test9 ||test11\ntest2||test4||test6||test8||test10\n";
+
+    assert_eq!(grid.to_string(), bits);
+    assert_eq!(grid.row_count(), 2);
+}
+
 #[test]
 fn use_minimal_optimal_lines() {
     let grid = Grid::new(
@@ -362,6 +382,8 @@ fn use_minimal_optimal_lines() {
 
 #[test]
 fn weird_column_edge_case() {
+    // Here, 5 columns fit while fewer columns don't. So if we exit too early
+    // while increasing columns, we don't find the optimal solution.
     let grid = Grid::new(
         vec!["0", "1", "222222222", "333333333", "4", "5", "6", "7", "8"],
         GridOptions {
@@ -376,6 +398,7 @@ fn weird_column_edge_case() {
         1  333333333  5  7\n\
     ";
 
+    println!("{grid}");
     assert_eq!(grid.to_string(), expected);
 }
 
