[lmi-commits] [lmi] master 3f962fd 1/6: Make minimum margin a set_column_widths() argument

[Greg Chicares]

branch: master
commit 3f962fd377c2593b8bd8de235b6c011f8110d704
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Make minimum margin a set_column_widths() argument
    
    ISO 31-0 recommends 'thin space' as an SI thousands separator, so that
    would seem ideal as a perceptible minimum space between columns that
    have been formatted with commas as thousands separators. However, that
    character, U+2009, is not quite trivial to embed comprehensibly in C++
    code, and not every font offers it anyway. Fonts may also lack U+2008,
    'punctuation space', but certainly include a period, which has the same
    width by definition. Therefore, used the width of a period as the
    minimum "margin" (space between columns) for production.
    
    Incidentally rearranged the order of set_column_widths() arguments,
    putting the most plausibly defaulted arguments at the end.
---
 report_table.cpp       | 15 +++++++--------
 report_table.hpp       |  5 +++--
 report_table_test.cpp  | 42 ++++++++++++++++++++++++------------------
 wx_table_generator.cpp |  8 ++++++--
 4 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/report_table.cpp b/report_table.cpp
index 841aa4c..c030893 100644
--- a/report_table.cpp
+++ b/report_table.cpp
@@ -94,24 +94,23 @@ std::vector<int> apportion(std::vector<int> const& votes, 
int total_seats)
 /// distribute any excess width left over among elastic columns.
 ///
 /// Notes on arguments:
-///   max_table_width: page width - page margins
-///   desired_margin: maximum margin for each inelastic column
-///   minimum_margin: minimum margin for every column
 ///   all_columns: the width of each inelastic column reflects:
 ///    - the header width, and
 ///    - a mask like "999,999" (ideally, there would instead be a
 ///      quasi-global data structure mapping symbolic column names
 ///      to their corresponding headers and maximal widths)
+///   max_table_width: page width - page margins
+///   desired_margin: maximum margin for each inelastic column
+///   minimum_margin: minimum margin for every column
 
 void set_column_widths
-    (int                             max_table_width
+    (std::vector<table_column_info>& all_columns
+    ,int                             max_table_width
     ,int                             desired_margin
-    ,std::vector<table_column_info>& all_columns
+    ,int                             minimum_margin
     )
 {
-    // Soon this will become an argument.
-    int minimum_margin = 1;
-
+    LMI_ASSERT(minimum_margin <= desired_margin);
     int const cardinality = lmi::ssize(all_columns);
     int data_width = 0;
     int n_columns_to_show = 0;
diff --git a/report_table.hpp b/report_table.hpp
index fc7803b..d5fdbc2 100644
--- a/report_table.hpp
+++ b/report_table.hpp
@@ -106,9 +106,10 @@ class LMI_SO table_column_info
 std::vector<int> LMI_SO apportion(std::vector<int> const& votes, int seats);
 
 void LMI_SO set_column_widths
-    (int                             max_table_width
+    (std::vector<table_column_info>& all_columns
+    ,int                             max_table_width
     ,int                             desired_margin
-    ,std::vector<table_column_info>& all_columns
+    ,int                             minimum_margin
     );
 
 #endif // report_table_hpp
diff --git a/report_table_test.cpp b/report_table_test.cpp
index 573a243..53f6963 100644
--- a/report_table_test.cpp
+++ b/report_table_test.cpp
@@ -188,13 +188,13 @@ void report_table_test::test_generally()
 
     // Just enough room for all data with desired margins.
     v = bloat({1, 2, 3}, {0, 0, 0});
-    set_column_widths(12, 2, v);
+    set_column_widths(v, 12, 2, 1);
     expected = {3, 4, 5};
     BOOST_TEST(widths(v) == expected);
 
     // Same columns: same layout, even if page is much wider.
     v = bloat({1, 2, 3}, {0, 0, 0});
-    set_column_widths(99, 2, v);
+    set_column_widths(v, 99, 2, 1);
     BOOST_TEST(widths(v) == expected);
 
     // Same columns, but inadequate page width.
@@ -204,73 +204,79 @@ void report_table_test::test_generally()
     // of at least one point.
 
     v = bloat({1, 2, 3}, {0, 0, 0});
-    set_column_widths(11, 2, v);
+    set_column_widths(v, 11, 2, 1);
     expected = {3, 4, 4};
     BOOST_TEST(widths(v) == expected);
 
     // Just enough room for all data with minimum margins.
     v = bloat({1, 2, 3}, {0, 0, 0});
-    set_column_widths( 9, 2, v);
+    set_column_widths(v,  9, 2, 1);
     expected = {2, 3, 4};
     BOOST_TEST(widths(v) == expected);
 
     // Not enough room for all data with minimum margins.
     v = bloat({1, 2, 3}, {0, 0, 0});
     std::cout << "Expect a diagnostic (printing 2/3 columns):\n  ";
-    set_column_widths( 8, 2, v);
+    set_column_widths(v,  8, 2, 1);
     expected = {3, 4, 0};
     BOOST_TEST(widths(v) == expected);
 
     // Not enough room for all data, even with no margins at all.
     v = bloat({1, 2, 3}, {0, 0, 0});
     std::cout << "Expect a diagnostic (printing 2/3 columns):\n  ";
-    set_column_widths( 5, 2, v);
+    set_column_widths(v,  5, 2, 1);
     expected = {2, 3, 0};
     BOOST_TEST(widths(v) == expected);
 
     // Not enough room for even the first column.
     BOOST_TEST_THROW
-        (set_column_widths(1, 2, v)
+        (set_column_widths(v, 1, 2, 1)
         ,std::runtime_error
         ,"Not enough room for even the first column."
         );
 
+    // Minimum margin greater than one.
+    v = bloat({1, 2, 3}, {0, 0, 0});
+    set_column_widths(v, 16, 5, 3);
+    expected = {5, 5, 6};
+    BOOST_TEST(widths(v) == expected);
+
     // An elastic column occupies all available space not claimed by
     // inelastic columns...
     v = bloat({1, 2, 0, 3}, {0, 0, 1, 0});
-    set_column_widths(99, 2, v);
+    set_column_widths(v, 99, 2, 1);
     expected = {3, 4, (99-12), 5};
     BOOST_TEST(widths(v) == expected);
     // ...though its width might happen to be zero (PDF !! but see
     //   https://lists.nongnu.org/archive/html/lmi/2018-07/msg00049.html
     // which questions whether zero should be allowed):
     v = bloat({1, 2, 0, 3}, {0, 0, 1, 0});
-    set_column_widths(12, 2, v);
+    set_column_widths(v, 12, 2, 1);
     expected = {3, 4, 0, 5};
     BOOST_TEST(widths(v) == expected);
 
     // Multiple elastic columns apportion all unclaimed space among
     // themselves.
     v = bloat({0, 2, 0, 3}, {1, 0, 1, 0});
-    set_column_widths(99, 2, v);
+    set_column_widths(v, 99, 2, 1);
     expected = {45, 4, 45, 5};
     BOOST_TEST(widths(v) == expected);
 
     // Same, but with nonzero width specified for one elastic column.
     v = bloat({1, 2, 0, 3}, {1, 0, 1, 0});
-    set_column_widths(99, 2, v);
+    set_column_widths(v, 99, 2, 1);
     expected = {46, 4, 44, 5};
     BOOST_TEST(widths(v) == expected);
 
     // Elastic columns only.
     v = bloat({10, 20, 30}, {1, 1, 1});
-    set_column_widths(99, 2, v);
+    set_column_widths(v, 99, 2, 1);
     expected = {23, 33, 43};
     BOOST_TEST(widths(v) == expected);
 
     // Same columns, but all inelastic.
     v = bloat({10, 20, 30}, {0, 0, 0});
-    set_column_widths(99, 2, v);
+    set_column_widths(v, 99, 2, 1);
     expected = {12, 22, 32};
     BOOST_TEST(widths(v) == expected);
 }
@@ -299,7 +305,7 @@ void report_table_test::test_group_quote()
         ,{"", 67, oe_center, oe_inelastic}
         };
 
-    set_column_widths(total_width, default_margin, v);
+    set_column_widths(v, total_width, default_margin, 1);
 
     std::vector<int> const observed = widths(v);
     std::vector<int> const expected = {36, 129, 52, 62, 78, 81, 78, 81, 78, 
81};
@@ -332,7 +338,7 @@ void report_table_test::test_illustration()
         ,{"", 53, oe_right, oe_inelastic}
         };
 
-    set_column_widths(total_width, default_margin, v);
+    set_column_widths(v, total_width, default_margin, 1);
 
     std::vector<int> const observed = widths(v);
     std::vector<int> const expected = {38, 52, 67, 66, 45, 62, 62, 67};
@@ -358,7 +364,7 @@ void report_table_test::test_illustration()
         ,{"", 50, oe_right, oe_inelastic}
         };
 
-    set_column_widths(total_width, default_margin, v);
+    set_column_widths(v, total_width, default_margin, 1);
 
     std::vector<int> const observed = widths(v);
     std::vector<int> const expected = {30, 28, 54, 36, 54, 54, 54, 54, 53, 53, 
53, 53};
@@ -385,7 +391,7 @@ void report_table_test::test_illustration()
         };
 
     std::cout << "Expect a diagnostic (printing 11/12 columns):\n  ";
-    set_column_widths(total_width, default_margin, v);
+    set_column_widths(v, total_width, default_margin, 1);
 
     // Today, two times the default margin is added to each column,
     // even though the data cannot fit.
@@ -396,7 +402,7 @@ void report_table_test::test_illustration()
 
 #if 0 // Doesn't throw today, but might someday.
     BOOST_TEST_THROW
-        (set_column_widths(total_width, default_margin, v)
+        (set_column_widths(v, total_width, default_margin, 1)
         ,std::runtime_error
         ,"Not enough space for all 12 columns."
         );
diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
index 234aed6..bac5acf 100644
--- a/wx_table_generator.cpp
+++ b/wx_table_generator.cpp
@@ -56,7 +56,9 @@ wx_table_generator::wx_table_generator
         {
         enroll_column(i);
         }
-    set_column_widths(total_width_, 2 * one_em_, all_columns_);
+    // Ideally this would be '&thinsp;' instead of '&puncsp;'.
+    int const one_puncsp = dc_.GetTextExtent(".").x;
+    set_column_widths(all_columns_, total_width_, 2 * one_em_, one_puncsp);
 
     // Set a pen with zero width to make grid lines thin,
     // and round cap style so that they combine seamlessly.
@@ -88,7 +90,9 @@ wx_table_generator::wx_table_generator
         {
         enroll_column(i);
         }
-    set_column_widths(total_width_, 2 * one_em_, all_columns_);
+    // Ideally this would be '&thinsp;' instead of '&puncsp;'.
+    int const one_puncsp = dc_.GetTextExtent(".").x;
+    set_column_widths(all_columns_, total_width_, 2 * one_em_, one_puncsp);
 
     dc_.SetPen(illustration_rule_color);
 }