[lmi-commits] [lmi] master e30fcc9 01/10: Transplant code that adds bilateral column margins

[Greg Chicares]

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

    Transplant code that adds bilateral column margins
    
    It's questionable whether presumptive margins should always be added
    when they might soon need to be removed. Therefore, moved the code
    that adds them into compute_column_widths(), where they are potentially
    removed. This improves the separation of concerns and makes a planned
    refactoring easier.
---
 wx_table_generator.cpp | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/wx_table_generator.cpp b/wx_table_generator.cpp
index ffaa46d..58de18e 100644
--- a/wx_table_generator.cpp
+++ b/wx_table_generator.cpp
@@ -477,9 +477,6 @@ LMI_ASSERT(std::size_t(h / lh) == 1u + 
count_newlines(z.header));
                 {
                 // Greater of header width and 'widest_text' width.
                 width = std::max(w, dc().GetTextExtent(z.widest_text).x);
-                // PDF !! Reconsider whether margin should be added here,
-                // because compute_column_widths() may need to remove it.
-                width += 2 * column_margin();
                 }
                 break;
             case oe_elastic:
@@ -515,12 +512,24 @@ LMI_ASSERT(std::size_t(h / lh) == 1u + 
count_newlines(z.header));
 ///    quasi-global data structure mapping symbolic column names
 ///    to their corresponding headers and maximal widths)
 ///  - the header width
-///  - the bilateral margins that have already been added
+///  - PDF !! the bilateral margins added as a first step below
 /// The margins may be slightly reduced by this function to make
 /// everything fit when it otherwise wouldn't.
 
 void wx_table_generator::compute_column_widths()
 {
+    // PDF !! Unconditionally add bilateral margins even though they
+    // may conditionally be removed below. This is a questionable
+    // design decision; if it is later reversed, then remove the
+    // comment about it above the implementation.
+    for(auto& i : all_columns_)
+        {
+        if(!i.is_hidden() && !i.is_elastic())
+            {
+            i.col_width_ += 2 * column_margin();
+            }
+        }
+
     // Number of non-hidden columns.
     int number_of_columns = 0;