Merge branch 'Issue484/fix-calculate-dimension-when-not-skipping-nulls' of https://github.com/imgspc/xlnt into imgspc-Issue484/fix-calculate-dimension-when-not-skipping-nulls

include/xlnt/worksheet/worksheet.hpp

@@ -433,9 +433,11 @@ public:
     column_t highest_column_or_props() const;
 
     /// <summary>
-    /// Returns a range_reference pointing to the full range of non-empty cells in the worksheet.
+    /// Returns a range_reference pointing to the full range of cells in the worksheet.
+    /// If skip_null is true (default), empty cells (For example if the first row or column is empty)
+    /// will not be included in upper left bound of range.
     /// </summary>
-    range_reference calculate_dimension() const;
+    range_reference calculate_dimension(bool skip_null=true) const;
 
     // cell merge
 

source/worksheet/worksheet.cpp

@@ -577,7 +577,7 @@ column_t worksheet::highest_column_or_props() const
     return highest;
 }
 
-range_reference worksheet::calculate_dimension() const
+range_reference worksheet::calculate_dimension(bool skip_null) const
 {
     // partially optimised version of:
     // return range_reference(lowest_column(), lowest_row_or_props(),
@@ -588,11 +588,16 @@ range_reference worksheet::calculate_dimension() const
         return range_reference(constants::min_column(), constants::min_row(),
             constants::min_column(), constants::min_row());
     }
-    row_t min_row_prop = constants::max_row();
+
+    // if skip_null = false, min row = min_row() and min column = min_column()
+    // in order to include first empty rows and columns
+    row_t min_row_prop = skip_null? constants::max_row() : constants::min_row();
     row_t max_row_prop = constants::min_row();
     for (const auto &row_prop : d_->row_properties_)
     {
+        if(skip_null){
             min_row_prop = std::min(min_row_prop, row_prop.first);
+        }
         max_row_prop = std::max(max_row_prop, row_prop.first);
     }
     if (d_->cell_map_.empty())
@@ -601,15 +606,17 @@ range_reference worksheet::calculate_dimension() const
             constants::min_column(), max_row_prop);
     }
     // find min and max row/column in cell map
-    column_t min_col = constants::max_column();
+    column_t min_col = skip_null? constants::max_column() : constants::min_column();
     column_t max_col = constants::min_column();
     row_t min_row = min_row_prop;
     row_t max_row = max_row_prop;
     for (auto &c : d_->cell_map_)
     {
+        if(skip_null){
             min_col = std::min(min_col, c.second.column_);
-        max_col = std::max(max_col, c.second.column_);
             min_row = std::min(min_row, c.second.row_);
+        }
+        max_col = std::max(max_col, c.second.column_);
         max_row = std::max(max_row, c.second.row_);
     }
     return range_reference(min_col, min_row, max_col, max_row);
@@ -717,22 +724,22 @@ row_t worksheet::next_row() const
 
 xlnt::range worksheet::rows(bool skip_null)
 {
-    return xlnt::range(*this, calculate_dimension(), major_order::row, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null), major_order::row, skip_null);
 }
 
 const xlnt::range worksheet::rows(bool skip_null) const
 {
-    return xlnt::range(*this, calculate_dimension(), major_order::row, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null), major_order::row, skip_null);
 }
 
 xlnt::range worksheet::columns(bool skip_null)
 {
-    return xlnt::range(*this, calculate_dimension(), major_order::column, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null), major_order::column, skip_null);
 }
 
 const xlnt::range worksheet::columns(bool skip_null) const
 {
-    return xlnt::range(*this, calculate_dimension(), major_order::column, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null), major_order::column, skip_null);
 }
 
 /*

tests/worksheet/worksheet_test_suite.cpp

@@ -21,8 +21,6 @@
 // @license: http://www.opensource.org/licenses/mit-license.php
 // @author: see AUTHORS file
 
-#include <iostream>
-
 #include <xlnt/cell/cell.hpp>
 #include <xlnt/cell/hyperlink.hpp>
 #include <xlnt/workbook/workbook.hpp>
@@ -114,6 +112,7 @@ public:
         register_test(test_insert_delete_moves_merges);
         register_test(test_hidden_sheet);
         register_test(test_xlsm_read_write);
+        register_test(test_issue_484);
     }
 
     void test_new_worksheet()
@@ -143,8 +142,10 @@ public:
         auto ws = wb.active_sheet();
 
         xlnt_assert_equals("A1:A1", ws.calculate_dimension());
+        xlnt_assert_equals("A1:A1", ws.calculate_dimension(false));
         ws.cell("B12").value("AAA");
         xlnt_assert_equals("B12:B12", ws.calculate_dimension());
+        xlnt_assert_equals("A1:B12", ws.calculate_dimension(false));
     }
 
     void test_fill_rows()
@@ -1639,5 +1640,22 @@ public:
             xlnt_assert_equals(rows[1][1].value<int>(), 1500);
         }
     }
+
+    void test_issue_484()
+    {
+        // Include first empty rows/columns in column dimensions
+        // if skip_null is false.
+        xlnt::workbook wb;
+        auto ws = wb.active_sheet();
+        
+        ws.cell("B12").value("AAA");
+        
+        xlnt_assert_equals("B12:B12", ws.rows(true).reference());
+        xlnt_assert_equals("A1:B12", ws.rows(false).reference());
+        
+        xlnt_assert_equals("B12:B12", ws.columns(true).reference());
+        xlnt_assert_equals("A1:B12", ws.columns(false).reference());
+    }
 };
+
 static worksheet_test_suite x;