[Libreoffice-commits] core.git: Branch 'feature/formula-core-rework' - sc/qa sc/source

[Kohei Yoshida]

 sc/qa/unit/ucalc.cxx           |   72 +++++++++++++++++++++++++++++++++++++++++
 sc/source/core/data/column.cxx |   36 --------------------
 2 files changed, 73 insertions(+), 35 deletions(-)

New commits:
commit 9b0905350a005c397f790a72b175cb20169c93cf
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Wed Jun 26 23:06:02 2013 -0400

    No need to increment formula row positions in InsertRow().
    
    UpdateReference() which gets called before InsertRow() moves the formula
    positions.
    
    Change-Id: I6d00607a1a1b4463f69bb58610f6ba41871e4475

diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index 36c3628..c66f3c0 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -237,6 +237,7 @@ public:
     void testUpdateReference();
     void testSearchCells();
     void testSharedFormulas();
+    void testFormulaPosition();
 
     /**
      * Make sure the sheet streams are invalidated properly.
@@ -354,6 +355,7 @@ public:
     CPPUNIT_TEST(testUpdateReference);
     CPPUNIT_TEST(testSearchCells);
     CPPUNIT_TEST(testSharedFormulas);
+    CPPUNIT_TEST(testFormulaPosition);
     CPPUNIT_TEST(testJumpToPrecedentsDependents);
     CPPUNIT_TEST(testSetBackgroundColor);
     CPPUNIT_TEST(testRenameTable);
@@ -6405,6 +6407,76 @@ void Test::testSharedFormulas()
 
 namespace {
 
+bool checkFormulaPosition(ScDocument& rDoc, const ScAddress& rPos)
+{
+    OUString aStr;
+    rPos.Format(aStr, SCA_VALID);
+    const ScFormulaCell* pFC = rDoc.GetFormulaCell(rPos);
+    if (!pFC)
+    {
+        cerr << "Formula cell expected at " << aStr << " but not found." << endl;
+        return false;
+    }
+
+    if (pFC->aPos != rPos)
+    {
+        OUString aStr2;
+        pFC->aPos.Format(aStr2, SCA_VALID);
+        cerr << "Formula cell at " << aStr << " has incorrect position of " << aStr2 << endl;
+        return false;
+    }
+
+    return true;
+}
+
+void checkFormulaPositions(ScDocument& rDoc, const ScAddress& rPos, const SCROW* pRows, size_t nRowCount)
+{
+    ScAddress aPos = rPos;
+    for (size_t i = 0; i < nRowCount; ++i)
+    {
+        SCROW nRow = pRows[i];
+        aPos.SetRow(nRow);
+
+        if (!checkFormulaPosition(rDoc, aPos))
+        {
+            OUString aStr;
+            aPos.Format(aStr, SCA_VALID);
+            std::ostringstream os;
+            os << "Formula cell position failed at " << aStr;
+            CPPUNIT_FAIL(os.str().c_str());
+        }
+    }
+}
+
+}
+
+void Test::testFormulaPosition()
+{
+    m_pDoc->InsertTab(0, "Test");
+
+    ScAddress aPos(0,0,0); // A1
+    m_pDoc->SetString(aPos, "=ROW()");
+    aPos.IncRow(); // A2
+    m_pDoc->SetString(aPos, "=ROW()");
+    aPos.SetRow(3); // A4;
+    m_pDoc->SetString(aPos, "=ROW()");
+
+    {
+        SCROW aRows[] = { 0, 1, 3 };
+        checkFormulaPositions(*m_pDoc, aPos, aRows, SAL_N_ELEMENTS(aRows));
+    }
+
+    m_pDoc->InsertRow(0,0,0,0,1,5); // Insert 5 rows at A2.
+    {
+        SCROW aRows[] = { 0, 6, 8 };
+        checkFormulaPositions(*m_pDoc, aPos, aRows, SAL_N_ELEMENTS(aRows));
+    }
+
+    m_pDoc->DeleteTab(0);
+}
+
+namespace {
+
 bool hasRange(const std::vector<ScTokenRef>& rRefTokens, const ScRange& rRange)
 {
     std::vector<ScTokenRef>::const_iterator it = rRefTokens.begin(), itEnd = rRefTokens.end();
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index de330f5..26edaa9 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -1233,43 +1233,9 @@ void ScColumn::InsertRow( SCROW nStartRow, SCSIZE nSize )
     maCellTextAttrs.insert_empty(nStartRow, nSize);
     maCellTextAttrs.resize(MAXROWCOUNT);
 
-    sc::CellStoreType::position_type aPos = maCells.position(nStartRow);
-    sc::CellStoreType::iterator it = maCells.insert_empty(aPos.first, nStartRow, nSize);
+    maCells.insert_empty(nStartRow, nSize);
     maCells.resize(MAXROWCOUNT);
 
-    sc::AutoCalcSwitch aSwitch(*pDocument, false);
-
-    // Get the position of the first affected cell.
-    aPos = maCells.position(it, nStartRow+nSize);
-    it = aPos.first;
-
-    // Update the positions of all affected formula cells.
-    if (it->type == sc::element_type_formula)
-    {
-        sc::formula_block::iterator itf = sc::formula_block::begin(*it->data);
-        sc::formula_block::iterator itfEnd = sc::formula_block::end(*it->data);
-        std::advance(itf, aPos.second);
-        for (; itf != itfEnd; ++itf)
-        {
-            ScFormulaCell& rCell = **itf;
-            rCell.aPos.IncRow(nSize);
-        }
-    }
-
-    for (++it; it != maCells.end(); ++it)
-    {
-        if (it->type != sc::element_type_formula)
-            continue;
-
-        sc::formula_block::iterator itf = sc::formula_block::begin(*it->data);
-        sc::formula_block::iterator itfEnd = sc::formula_block::end(*it->data);
-        for (; itf != itfEnd; ++itf)
-        {
-            ScFormulaCell* pCell = *itf;
-            pCell->aPos.IncRow(nSize);
-        }
-    }
-
     CellStorageModified();
 
     // We *probably* don't need to broadcast here since the parent call seems