From fb457594fd1d4473d783efa84119f126b76d27fd Mon Sep 17 00:00:00 2001
From: Zheng Lei <realthunder.dev@gmail.com>
Date: Thu, 9 Jun 2022 18:20:08 +0800
Subject: [PATCH] Spreadsheet: fix range checking (#6997)

* App: add option to normalize a Range

- To make sure the range starts from top left and ends with bottom right
corner.
- Default is to not normalize on construction for backward compatibility.
-  fix range checking in range binding
---
 src/App/Range.cpp                        | 32 +++++++++++++++++-------
 src/App/Range.h                          |  9 ++++---
 src/Mod/Spreadsheet/App/Sheet.cpp        | 11 ++++----
 src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp |  4 +--
 4 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/src/App/Range.cpp b/src/App/Range.cpp
index d9b19780a7..05a97294ad 100644
--- a/src/App/Range.cpp
+++ b/src/App/Range.cpp
@@ -37,7 +37,7 @@ using namespace App;
 const int App::CellAddress::MAX_ROWS = 16384;
 const int App::CellAddress::MAX_COLUMNS = 26 * 26 + 26;
 
-Range::Range(const char * range)
+Range::Range(const char * range, bool normalize)
 {
     std::string from;
     std::string to;
@@ -62,28 +62,42 @@ Range::Range(const char * range)
     row_end = end.row();
     col_end = end.col();
 
+    if (normalize)
+        this->normalize();
     row_curr = row_begin;
     col_curr = col_begin;
 }
 
-Range::Range(int _row_begin, int _col_begin, int _row_end, int _col_end)
-    : row_curr(_row_begin)
-    , col_curr(_col_begin)
-    , row_begin(_row_begin)
+Range::Range(int _row_begin, int _col_begin, int _row_end, int _col_end, bool normalize)
+    : row_begin(_row_begin)
     , col_begin(_col_begin)
     , row_end(_row_end)
     , col_end(_col_end)
 {
+    if (normalize)
+        this->normalize();
+    row_curr = row_begin;
+    col_curr = col_begin;
 }
 
-Range::Range(const CellAddress &from, const CellAddress &to)
-    : row_curr(from.row())
-    , col_curr(from.col())
-    , row_begin(from.row())
+Range::Range(const CellAddress &from, const CellAddress &to, bool normalize)
+    : row_begin(from.row())
     , col_begin(from.col())
     , row_end(to.row())
     , col_end(to.col())
 {
+    if (normalize)
+        this->normalize();
+    row_curr = row_begin;
+    col_curr = col_begin;
+}
+
+void Range::normalize()
+{
+    if (row_begin > row_end)
+        std::swap(row_begin, row_end);
+    if (col_begin > col_end)
+        std::swap(col_begin, col_end);
 }
 
 bool Range::next()
diff --git a/src/App/Range.h b/src/App/Range.h
index 9d339994d7..17f0bb4e33 100644
--- a/src/App/Range.h
+++ b/src/App/Range.h
@@ -117,14 +117,17 @@ protected:
 
 class AppExport Range {
 public:
-    Range(const char *range);
+    Range(const char *range, bool normalize=false);
 
-    Range(int _row_begin, int _col_begin, int _row_end, int _col_end);
+    Range(int _row_begin, int _col_begin, int _row_end, int _col_end, bool normalize=false);
 
-    Range(const CellAddress & from, const CellAddress & to);
+    Range(const CellAddress & from, const CellAddress & to, bool normalize=false);
 
     bool next();
 
+    /** Make sure the range starts from top left and ends with bottom right corner **/
+    void normalize();
+
     /** Current row */
     inline int row() const { return row_curr; }
 
diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp
index 89e0043be5..69dd1f81eb 100644
--- a/src/Mod/Spreadsheet/App/Sheet.cpp
+++ b/src/Mod/Spreadsheet/App/Sheet.cpp
@@ -830,9 +830,10 @@ Sheet::getCellBinding(Range &range,
                       ExpressionPtr *pEnd,
                       App::ObjectIdentifier *pTarget) const 
 {
+    range.normalize();
     do {
         CellAddress addr = *range;
-        for(auto &r : boundRanges) {
+        for(const auto &r : boundRanges) {
             if(addr.row()>=r.from().row()
                     && addr.row()<=r.to().row()
                     && addr.col()>=r.from().col()
@@ -885,20 +886,20 @@ void Sheet::updateBindings()
     std::set<Range> newRangeSet;
     std::set<Range> rangeSet;
     boundRanges.clear();
-    for(auto &v : ExpressionEngine.getExpressions()) {
+    for(const auto &v : ExpressionEngine.getExpressions()) {
         CellAddress from,to;
         if(!cells.isBindingPath(v.first,&from,&to))
             continue;
-        App::Range range(from,to);
+        App::Range range(from,to,true);
         if(!oldRangeSet.erase(range))
             newRangeSet.insert(range);
         rangeSet.insert(range);
     }
     boundRanges.reserve(rangeSet.size());
     boundRanges.insert(boundRanges.end(),rangeSet.begin(),rangeSet.end());
-    for(auto &range : oldRangeSet)
+    for(const auto &range : oldRangeSet)
         rangeUpdated(range);
-    for(auto &range : newRangeSet)
+    for(const auto &range : newRangeSet)
         rangeUpdated(range);
 }
 
diff --git a/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp b/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp
index f53b7aadcb..23c2f4dd57 100644
--- a/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp
+++ b/src/Mod/Spreadsheet/Gui/DlgBindSheet.cpp
@@ -188,8 +188,8 @@ void DlgBindSheet::accept()
         else {
             checkAddress(toEnd, toCellEnd, true);
             if (toCellStart.isValid()) {
-                App::Range fromRange(fromCellStart, fromCellEnd);
-                App::Range toRange(toCellStart, toCellEnd);
+                App::Range fromRange(fromCellStart, fromCellEnd, true);
+                App::Range toRange(toCellStart, toCellEnd, true);
                 if (fromRange.size() != toRange.size()) {
                     auto res = QMessageBox::warning(this, tr("Bind cells"),
                             tr("Source and target cell count mismatch. Partial binding may still work.\n\n"