Remove empty rows/columns from the range returned by worksheet::rows(true) (#8)

Note that the iterator already skipped those empty rows/cells, but those
rows/columns were included in the returned range.reference(). If someone
is interested in the old range_reference (i.e. the envolope of all
non-empty cells or rows with properties), calculate_dimension(true) can
be called directly.

Note that Google Sheets sometimes add empty rows (with only properties
being defined). This results in a different range being returned by
rows/columns before or after the file is saved by Google Sheets.

Note that the (old) default behaviour of calculate_dimension is kept, as
this is how the dimensions are stored in the worksheet. Changing this
behaviour results in failed unit tests.

Fixes #5

Author: doomlaur

include/xlnt/worksheet/worksheet.hpp

@@ -229,27 +229,31 @@ class XLNT_API worksheet
     /// <summary>
     /// Returns a range encompassing all cells in this sheet which will
     /// be iterated upon in row-major order. If skip_null is true (default),
+    /// the range does not contain empty rows/columns at its boundaries and
     /// empty rows and cells will be skipped during iteration of the range.
     /// </summary>
     class range rows(bool skip_null = true);
 
     /// <summary>
     /// Returns a range encompassing all cells in this sheet which will
     /// be iterated upon in row-major order. If skip_null is true (default),
+    /// the range does not contain empty rows/columns at its boundaries and
     /// empty rows and cells will be skipped during iteration of the range.
     /// </summary>
     const class range rows(bool skip_null = true) const;
 
     /// <summary>
     /// Returns a range ecompassing all cells in this sheet which will
     /// be iterated upon in column-major order. If skip_null is true (default),
+    /// the range does not contain empty rows/columns at its boundaries and
     /// empty columns and cells will be skipped during iteration of the range.
     /// </summary>
     class range columns(bool skip_null = true);
 
     /// <summary>
     /// Returns a range ecompassing all cells in this sheet which will
     /// be iterated upon in column-major order. If skip_null is true (default),
+    /// the range does not contain empty rows/columns at its boundaries and
     /// empty columns and cells will be skipped during iteration of the range.
     /// </summary>
     const class range columns(bool skip_null = true) const;
@@ -436,8 +440,9 @@ class XLNT_API 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.
+    /// If skip_row_props is false (default), rows with only properties being defined will be returned too.
     /// </summary>
-    range_reference calculate_dimension(bool skip_null=true) const;
+    range_reference calculate_dimension(bool skip_null=true, bool skip_row_props=false) 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(bool skip_null) const
+range_reference worksheet::calculate_dimension(bool skip_null, bool skip_row_props) const
 {
     // partially optimised version of:
     // return range_reference(lowest_column(), lowest_row_or_props(),
@@ -593,12 +593,15 @@ range_reference worksheet::calculate_dimension(bool skip_null) const
     // 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_row_props)
     {
-        if(skip_null){
-            min_row_prop = std::min(min_row_prop, row_prop.first);
+        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);
         }
-        max_row_prop = std::max(max_row_prop, row_prop.first);
     }
     if (d_->cell_map_.empty())
     {
@@ -724,22 +727,22 @@ row_t worksheet::next_row() const
 
 xlnt::range worksheet::rows(bool skip_null)
 {
-    return xlnt::range(*this, calculate_dimension(skip_null), major_order::row, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null, skip_null), major_order::row, skip_null);
 }
 
 const xlnt::range worksheet::rows(bool skip_null) const
 {
-    return xlnt::range(*this, calculate_dimension(skip_null), major_order::row, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null, skip_null), major_order::row, skip_null);
 }
 
 xlnt::range worksheet::columns(bool skip_null)
 {
-    return xlnt::range(*this, calculate_dimension(skip_null), major_order::column, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null, skip_null), major_order::column, skip_null);
 }
 
 const xlnt::range worksheet::columns(bool skip_null) const
 {
-    return xlnt::range(*this, calculate_dimension(skip_null), major_order::column, skip_null);
+    return xlnt::range(*this, calculate_dimension(skip_null, skip_null), major_order::column, skip_null);
 }
 
 /*

tests/worksheet/worksheet_test_suite.cpp

@@ -111,6 +111,7 @@ class worksheet_test_suite : public test_suite
         register_test(test_hidden_sheet);
         register_test(test_xlsm_read_write);
         register_test(test_issue_484);
+        register_test(test_issue_5_empty_bottom_rows);
     }
 
     void test_new_worksheet()
@@ -1674,6 +1675,21 @@ class worksheet_test_suite : public test_suite
         xlnt_assert_equals("B12:B12", ws.columns(true).reference());
         xlnt_assert_equals("A1:B12", ws.columns(false).reference());
     }
+
+    void test_issue_5_empty_bottom_rows()
+    {
+        xlnt::workbook wb;
+        auto ws = wb.active_sheet();
+
+        ws.cell("B12").value("AAA");
+        ws.row_properties(15).height = 1.0;
+
+        xlnt_assert_equals("B12:B12", ws.rows(true).reference());
+        xlnt_assert_equals("A1:B15", ws.rows(false).reference());
+
+        xlnt_assert_equals("B12:B12", ws.columns(true).reference());
+        xlnt_assert_equals("A1:B15", ws.columns(false).reference());
+    }
 };
 
 static worksheet_test_suite x;