[Libreoffice-commits] core.git: 2 commits - sc/inc sc/qa sc/source

Kohei Yoshida kohei.yoshida at collabora.com
Tue Feb 25 19:48:43 PST 2014


 sc/inc/formulacell.hxx                  |   12 -
 sc/inc/listenercontext.hxx              |   11 +
 sc/inc/refupdatecontext.hxx             |   18 ++
 sc/qa/unit/helper/qahelper.cxx          |   58 +++++++
 sc/qa/unit/helper/qahelper.hxx          |    6 
 sc/qa/unit/ucalc.cxx                    |   48 ------
 sc/qa/unit/ucalc.hxx                    |    2 
 sc/qa/unit/ucalc_sharedformula.cxx      |  107 ++++++++++++++
 sc/source/core/data/column.cxx          |  234 +++++++++++++++++++++++++++-----
 sc/source/core/data/formulacell.cxx     |   17 +-
 sc/source/core/data/listenercontext.cxx |   24 +++
 sc/source/core/tool/token.cxx           |   17 +-
 sc/source/ui/inc/undoblk.hxx            |    5 
 sc/source/ui/undo/undoblk.cxx           |   19 +-
 14 files changed, 470 insertions(+), 108 deletions(-)

New commits:
commit 79d03eb090a5f88863c1004ef8b7f483cbecb69d
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Tue Feb 25 12:53:33 2014 -0500

    fdo#75386: Totally fix reference update during range move.
    
    It was just not working at all due to multiple reasons.  The
    reference update needed to be reworked for formula groups such that
    the token array is adjusted only for the top cell but all formula cells
    still needed to be processed afterwards.  The bound check also needed
    to be done against the old range prior to the move, not the new range
    after the move.
    
    During undo, the paint had to be deferred until after the two calls to
    DoUndo() else the formula cells would get re-calculated before the
    values were placed back to their old positions, causing them to mis-
    calculate wrong values.
    
    Change-Id: Iba66f80a413e0539cac5ab619226cd6f7a04f317

diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index d3e8853..274548b 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -141,12 +141,6 @@ private:
         const sc::RefUpdateContext& rCxt, ScDocument* pUndoDoc, const ScAddress* pUndoCellPos );
 
     /**
-     * Update reference in response to cell move.
-     */
-    bool UpdateReferenceOnMove(
-        const sc::RefUpdateContext& rCxt, ScDocument* pUndoDoc, const ScAddress* pUndoCellPos );
-
-    /**
      * Update reference in response to cell copy-n-paste.
      */
     bool UpdateReferenceOnCopy(
@@ -252,6 +246,12 @@ public:
     bool UpdateReference(
         const sc::RefUpdateContext& rCxt, ScDocument* pUndoDoc = NULL, const ScAddress* pUndoCellPos = NULL );
 
+    /**
+     * Update reference in response to cell move.
+     */
+    bool UpdateReferenceOnMove(
+        const sc::RefUpdateContext& rCxt, ScDocument* pUndoDoc, const ScAddress* pUndoCellPos );
+
     void            TransposeReference();
     void            UpdateTranspose( const ScRange& rSource, const ScAddress& rDest,
                                         ScDocument* pUndoDoc );
diff --git a/sc/inc/listenercontext.hxx b/sc/inc/listenercontext.hxx
index 1e7d067..c0260ef 100644
--- a/sc/inc/listenercontext.hxx
+++ b/sc/inc/listenercontext.hxx
@@ -17,6 +17,7 @@
 #include <boost/scoped_ptr.hpp>
 
 class ScDocument;
+class ScTokenArray;
 
 namespace sc {
 
@@ -39,9 +40,17 @@ class EndListeningContext : boost::noncopyable
     ScDocument& mrDoc;
     ColumnSpanSet maSet;
     boost::scoped_ptr<ColumnBlockPositionSet> mpPosSet;
+    ScTokenArray* mpOldCode;
+    ScAddress maPosDelta; // Add this to get the old position prior to the move.
+
 public:
-    EndListeningContext(ScDocument& rDoc);
+    EndListeningContext(ScDocument& rDoc, ScTokenArray* pOldCode = NULL);
+
+    void setPositionDelta( const ScAddress& rDelta );
+
     ScDocument& getDoc();
+    ScTokenArray* getOldCode();
+    ScAddress getOldPosition( const ScAddress& rPos ) const;
 
     ColumnBlockPosition* getBlockPosition(SCTAB nTab, SCCOL nCol);
 
diff --git a/sc/inc/refupdatecontext.hxx b/sc/inc/refupdatecontext.hxx
index ba0beed..c8e52d8 100644
--- a/sc/inc/refupdatecontext.hxx
+++ b/sc/inc/refupdatecontext.hxx
@@ -55,7 +55,8 @@ struct RefUpdateContext
     /**
      * Range of cells that are about to be moved for insert/delete/move modes.
      * For copy mode, it's the destination range of cells that are about to be
-     * pasted.
+     * pasted.  When moving a range of cells, it's the destination range, not
+     * the source range.
      */
     ScRange maRange;
 
@@ -77,8 +78,23 @@ struct RefUpdateContext
 
 struct RefUpdateResult
 {
+    /**
+     * When this flag is true, the result of the formula needs to be
+     * re-calculated either because it contains a reference that's been
+     * deleted, or the size of a range reference has changed.
+     */
     bool mbValueChanged;
+
+    /**
+     * This flag indicates whether any reference in the token array has been
+     * modified.
+     */
     bool mbReferenceModified;
+
+    /**
+     * When this flag is true, it indicates that the token array contains a
+     * range name that's been updated.
+     */
     bool mbNameModified;
 
     RefUpdateResult();
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index 3768808..4b8d38c 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -41,6 +41,7 @@
 #include "scopetools.hxx"
 #include "sharedformula.hxx"
 #include "refupdatecontext.hxx"
+#include <listenercontext.hxx>
 
 #include <svl/poolcach.hxx>
 #include <svl/zforlist.hxx>
@@ -2328,6 +2329,67 @@ void ScColumn::MarkSubTotalCells( sc::ColumnSpanSet& rSet, SCROW nRow1, SCROW nR
 
 namespace {
 
+struct FormulaGroup
+{
+    struct {
+        ScFormulaCell* mpCell;   // non-shared formula cell
+        ScFormulaCell** mpCells; // pointer to the top formula cell in a shared group.
+    };
+    size_t mnRow;
+    size_t mnLength;
+    bool mbShared;
+
+    FormulaGroup( ScFormulaCell** pCells, size_t nRow, size_t nLength ) :
+        mpCells(pCells), mnRow(nRow), mnLength(nLength), mbShared(true) {}
+
+    FormulaGroup( ScFormulaCell* pCell, size_t nRow ) :
+        mpCell(pCell), mnRow(nRow), mnLength(0), mbShared(false) {}
+};
+
+class SharedTopFormulaCellPicker : std::unary_function<sc::CellStoreType::value_type, void>
+{
+public:
+    virtual ~SharedTopFormulaCellPicker() {}
+
+    void operator() ( sc::CellStoreType::value_type& node )
+    {
+        if (node.type != sc::element_type_formula)
+            return;
+
+        size_t nTopRow = node.position;
+
+        sc::formula_block::iterator itBeg = sc::formula_block::begin(*node.data);
+        sc::formula_block::iterator itEnd = sc::formula_block::end(*node.data);
+
+        // Only pick shared formula cells that are the top cells of their
+        // respective shared ranges.
+        for (sc::formula_block::iterator it = itBeg; it != itEnd; ++it)
+        {
+            ScFormulaCell* pCell = *it;
+            size_t nRow = nTopRow + std::distance(itBeg, it);
+            if (!pCell->IsShared())
+            {
+                processNonShared(pCell, nRow);
+                continue;
+            }
+
+            if (pCell->IsSharedTop())
+            {
+                ScFormulaCell** pp = &(*it);
+                processSharedTop(pp, nRow, pCell->GetSharedLength());
+
+                // Move to the last cell in the group, to get incremented to
+                // the next cell in the next iteration.
+                size_t nOffsetToLast = pCell->GetSharedLength() - 1;
+                std::advance(it, nOffsetToLast);
+            }
+        }
+    }
+
+    virtual void processNonShared( ScFormulaCell* /*pCell*/, size_t /*nRow*/ ) {}
+    virtual void processSharedTop( ScFormulaCell** /*ppCells*/, size_t /*nRow*/, size_t /*nLength*/ ) {}
+};
+
 class UpdateRefOnCopy
 {
     const sc::RefUpdateContext& mrCxt;
@@ -2358,67 +2420,162 @@ public:
     }
 };
 
-class UpdateRefOnNonCopy
+class UpdateRefOnNonCopy : std::unary_function<FormulaGroup, void>
 {
     SCCOL mnCol;
     SCROW mnTab;
-    const sc::RefUpdateContext& mrCxt;
+    const sc::RefUpdateContext* mpCxt;
     ScDocument* mpUndoDoc;
     bool mbUpdated;
 
+    void updateRefOnMove( FormulaGroup& rGroup )
+    {
+        if (!rGroup.mbShared)
+        {
+            ScAddress aUndoPos(mnCol, rGroup.mnRow, mnTab);
+            mbUpdated |= rGroup.mpCell->UpdateReferenceOnMove(*mpCxt, mpUndoDoc, &aUndoPos);
+            return;
+        }
+
+        // Update references of a formula group.
+        ScFormulaCell** pp = rGroup.mpCells;
+        ScFormulaCell** ppEnd = pp + rGroup.mnLength;
+        ScFormulaCell* pTop = *pp;
+        ScTokenArray* pCode = pTop->GetCode();
+        boost::scoped_ptr<ScTokenArray> pOldCode(pCode->Clone());
+
+        ScAddress aPos = pTop->aPos;
+        ScAddress aOldPos = aPos;
+
+        if (mpCxt->maRange.In(aPos))
+        {
+            // The cell is being moved or copied to a new position. The
+            // position has already been updated prior to this call.
+            // Determine its original position before the move which will be
+            // used to adjust relative references later.
+
+            aOldPos.Set(
+                aPos.Col() - mpCxt->mnColDelta,
+                aPos.Row() - mpCxt->mnRowDelta,
+                aPos.Tab() - mpCxt->mnTabDelta);
+        }
+
+        bool bRecalcOnMove = pCode->IsRecalcModeOnRefMove();
+        if (bRecalcOnMove)
+            bRecalcOnMove = aPos != aOldPos;
+
+        sc::RefUpdateResult aRes = pCode->AdjustReferenceOnMove(*mpCxt, aOldPos, aPos);
+        if (aRes.mbReferenceModified || bRecalcOnMove)
+        {
+            // Perform end-listening, start-listening, and dirtying on all
+            // formula cells in the group.
+
+            sc::StartListeningContext aStartCxt(mpCxt->mrDoc);
+
+            sc::EndListeningContext aEndCxt(mpCxt->mrDoc, pOldCode.get());
+            aEndCxt.setPositionDelta(
+                ScAddress(-mpCxt->mnColDelta, -mpCxt->mnRowDelta, -mpCxt->mnTabDelta));
+
+            for (; pp != ppEnd; ++pp)
+            {
+                ScFormulaCell* p = *pp;
+                p->EndListeningTo(aEndCxt);
+                p->StartListeningTo(aStartCxt);
+                p->SetDirty();
+            }
+
+            if (mpUndoDoc)
+            {
+                // Insert the old formula group into the undo document.
+                ScAddress aUndoPos = aOldPos;
+                ScFormulaCell* pFC = new ScFormulaCell(mpUndoDoc, aUndoPos, pOldCode->Clone());
+                ScFormulaCellGroupRef xGroup = pFC->CreateCellGroup(rGroup.mnLength, false);
+
+                mpUndoDoc->SetFormulaCell(aUndoPos, pFC);
+                aUndoPos.IncRow();
+                for (size_t i = 1; i < rGroup.mnLength; ++i, aUndoPos.IncRow())
+                {
+                    pFC = new ScFormulaCell(mpUndoDoc, aUndoPos, xGroup);
+                    mpUndoDoc->SetFormulaCell(aUndoPos, pFC);
+                }
+            }
+        }
+    }
+
 public:
     UpdateRefOnNonCopy(
-        SCCOL nCol, SCTAB nTab, const sc::RefUpdateContext& rCxt,
+        SCCOL nCol, SCTAB nTab, const sc::RefUpdateContext* pCxt,
         ScDocument* pUndoDoc) :
-        mnCol(nCol), mnTab(nTab), mrCxt(rCxt),
+        mnCol(nCol), mnTab(nTab), mpCxt(pCxt),
         mpUndoDoc(pUndoDoc), mbUpdated(false) {}
 
-    void operator() (size_t nRow, ScFormulaCell* pCell)
+    void operator() ( FormulaGroup& rGroup )
     {
-        ScAddress aUndoPos(mnCol, nRow, mnTab);
-        mbUpdated |= pCell->UpdateReference(mrCxt, mpUndoDoc, &aUndoPos);
+        if (mpCxt->meMode == URM_MOVE)
+        {
+            updateRefOnMove(rGroup);
+            return;
+        }
+
+        if (rGroup.mbShared)
+        {
+            ScAddress aUndoPos(mnCol, rGroup.mnRow, mnTab);
+            ScFormulaCell** pp = rGroup.mpCells;
+            ScFormulaCell** ppEnd = pp + rGroup.mnLength;
+            for (; pp != ppEnd; ++pp, aUndoPos.IncRow())
+            {
+                ScFormulaCell* p = *pp;
+                mbUpdated |= p->UpdateReference(*mpCxt, mpUndoDoc, &aUndoPos);
+            }
+        }
+        else
+        {
+            ScAddress aUndoPos(mnCol, rGroup.mnRow, mnTab);
+            mbUpdated |= rGroup.mpCell->UpdateReference(*mpCxt, mpUndoDoc, &aUndoPos);
+        }
     }
 
     bool isUpdated() const { return mbUpdated; }
 };
 
-class UpdateRefGroupBoundChecker : std::unary_function<sc::CellStoreType::value_type, void>
+class UpdateRefGroupBoundChecker : public SharedTopFormulaCellPicker
 {
     const sc::RefUpdateContext& mrCxt;
     std::vector<SCROW>& mrBounds;
+
 public:
     UpdateRefGroupBoundChecker(const sc::RefUpdateContext& rCxt, std::vector<SCROW>& rBounds) :
         mrCxt(rCxt), mrBounds(rBounds) {}
 
-    void operator() (const sc::CellStoreType::value_type& node)
+    virtual ~UpdateRefGroupBoundChecker() {}
+
+    virtual void processSharedTop( ScFormulaCell** ppCells, size_t /*nRow*/, size_t /*nLength*/ )
     {
-        if (node.type != sc::element_type_formula)
-            return;
+        // Check its tokens and record its reference boundaries.
+        ScFormulaCell& rCell = **ppCells;
+        const ScTokenArray& rCode = *rCell.GetCode();
+        rCode.CheckRelativeReferenceBounds(
+            mrCxt, rCell.aPos, rCell.GetSharedLength(), mrBounds);
+    }
+};
 
-        sc::formula_block::const_iterator it = sc::formula_block::begin(*node.data);
-        sc::formula_block::const_iterator itEnd = sc::formula_block::end(*node.data);
+class FormulaGroupPicker : public SharedTopFormulaCellPicker
+{
+    std::vector<FormulaGroup>& mrGroups;
 
-        // Only pick shared formula cells that are the top cells of their
-        // respective shared ranges.
-        for (; it != itEnd; ++it)
-        {
-            const ScFormulaCell& rCell = **it;
-            if (!rCell.IsShared())
-                continue;
+public:
+    FormulaGroupPicker( std::vector<FormulaGroup>& rGroups ) : mrGroups(rGroups) {}
 
-            if (rCell.IsSharedTop())
-            {
-                // Check its tokens and record its reference boundaries.
-                const ScTokenArray& rCode = *rCell.GetCode();
-                rCode.CheckRelativeReferenceBounds(
-                    mrCxt, rCell.aPos, rCell.GetSharedLength(), mrBounds);
+    virtual ~FormulaGroupPicker() {}
 
-                // Move to the last cell in the group, to get incremented to
-                // the next cell in the next iteration.
-                size_t nOffsetToLast = rCell.GetSharedLength() - 1;
-                std::advance(it, nOffsetToLast);
-            }
-        }
+    virtual void processNonShared( ScFormulaCell* pCell, size_t nRow )
+    {
+        mrGroups.push_back(FormulaGroup(pCell, nRow));
+    }
+
+    virtual void processSharedTop( ScFormulaCell** ppCells, size_t nRow, size_t nLength )
+    {
+        mrGroups.push_back(FormulaGroup(ppCells, nRow, nLength));
     }
 };
 
@@ -2451,8 +2608,8 @@ bool ScColumn::UpdateReference( sc::RefUpdateContext& rCxt, ScDocument* pUndoDoc
     if (rCxt.meMode == URM_COPY)
         return UpdateReferenceOnCopy(rCxt, pUndoDoc);
 
-    if (IsEmptyData())
-        // Cells in this column are all empty.
+    if (IsEmptyData() || pDocument->IsClipOrUndo())
+        // Cells in this column are all empty, or clip or undo doc. No update needed.
         return false;
 
     std::vector<SCROW> aBounds;
@@ -2490,8 +2647,13 @@ bool ScColumn::UpdateReference( sc::RefUpdateContext& rCxt, ScDocument* pUndoDoc
     // Do the actual splitting.
     sc::SharedFormulaUtil::splitFormulaCellGroups(maCells, aBounds);
 
-    UpdateRefOnNonCopy aHandler(nCol, nTab, rCxt, pUndoDoc);
-    sc::ProcessFormula(maCells, aHandler);
+    // Collect all formula groups.
+    std::vector<FormulaGroup> aGroups;
+    std::for_each(maCells.begin(), maCells.end(), FormulaGroupPicker(aGroups));
+
+    // Process all collected formula groups.
+    UpdateRefOnNonCopy aHandler(nCol, nTab, &rCxt, pUndoDoc);
+    aHandler = std::for_each(aGroups.begin(), aGroups.end(), aHandler);
     if (aHandler.isUpdated())
         rCxt.maRegroupCols.set(nTab, nCol);
 
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index aa6e8b1..0540e88 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -3932,7 +3932,7 @@ void ScFormulaCell::EndListeningTo( ScDocument* pDoc, ScTokenArray* pArr,
         {
             case svSingleRef:
             {
-                ScAddress aCell = t->GetSingleRef().toAbs(aPos);
+                ScAddress aCell = t->GetSingleRef().toAbs(aCellPos);
                 if (aCell.IsValid())
                     pDoc->EndListeningCell(aCell, this);
             }
@@ -3954,27 +3954,32 @@ void ScFormulaCell::EndListeningTo( sc::EndListeningContext& rCxt )
     ScDocument& rDoc = rCxt.getDoc();
     rDoc.SetDetectiveDirty(true);  // It has changed something
 
-    if (pCode->IsRecalcModeAlways())
+    ScTokenArray* pArr = rCxt.getOldCode();
+    ScAddress aCellPos = rCxt.getOldPosition(aPos);
+    if (!pArr)
+        pArr = pCode;
+
+    if (pArr->IsRecalcModeAlways())
     {
         rDoc.EndListeningArea(BCA_LISTEN_ALWAYS, this);
         return;
     }
 
-    pCode->Reset();
+    pArr->Reset();
     ScToken* t;
-    while ( ( t = static_cast<ScToken*>(pCode->GetNextReferenceRPN()) ) != NULL )
+    while ( ( t = static_cast<ScToken*>(pArr->GetNextReferenceRPN()) ) != NULL )
     {
         switch (t->GetType())
         {
             case svSingleRef:
             {
-                ScAddress aCell = t->GetSingleRef().toAbs(aPos);
+                ScAddress aCell = t->GetSingleRef().toAbs(aCellPos);
                 if (aCell.IsValid())
                     rDoc.EndListeningCell(rCxt, aCell, *this);
             }
             break;
             case svDoubleRef:
-                endListeningArea(this, rDoc, aPos, *t);
+                endListeningArea(this, rDoc, aCellPos, *t);
             break;
             default:
                 ;   // nothing
diff --git a/sc/source/core/data/listenercontext.cxx b/sc/source/core/data/listenercontext.cxx
index dc92346..3dfe5ed 100644
--- a/sc/source/core/data/listenercontext.cxx
+++ b/sc/source/core/data/listenercontext.cxx
@@ -52,14 +52,34 @@ ColumnBlockPosition* StartListeningContext::getBlockPosition(SCTAB nTab, SCCOL n
     return mpSet->getBlockPosition(nTab, nCol);
 }
 
-EndListeningContext::EndListeningContext(ScDocument& rDoc) :
-    mrDoc(rDoc), maSet(false), mpPosSet(new ColumnBlockPositionSet(rDoc)) {}
+EndListeningContext::EndListeningContext(ScDocument& rDoc, ScTokenArray* pOldCode) :
+    mrDoc(rDoc), maSet(false), mpPosSet(new ColumnBlockPositionSet(rDoc)),
+    mpOldCode(pOldCode), maPosDelta(0,0,0) {}
+
+void EndListeningContext::setPositionDelta( const ScAddress& rDelta )
+{
+    maPosDelta = rDelta;
+}
 
 ScDocument& EndListeningContext::getDoc()
 {
     return mrDoc;
 }
 
+ScTokenArray* EndListeningContext::getOldCode()
+{
+    return mpOldCode;
+}
+
+ScAddress EndListeningContext::getOldPosition( const ScAddress& rPos ) const
+{
+    ScAddress aOldPos = rPos;
+    aOldPos.IncCol(maPosDelta.Col());
+    aOldPos.IncRow(maPosDelta.Row());
+    aOldPos.IncTab(maPosDelta.Tab());
+    return aOldPos;
+}
+
 ColumnBlockPosition* EndListeningContext::getBlockPosition(SCTAB nTab, SCCOL nCol)
 {
     return mpPosSet->getBlockPosition(nTab, nCol);
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index 463fd06..7052049 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -3155,17 +3155,22 @@ void checkBounds(
     if (!rRef.IsRowRel())
         return;
 
+    ScRange aCheckRange = rCxt.maRange;
+    if (rCxt.meMode == URM_MOVE)
+        // Check bounds against the old range prior to the move.
+        aCheckRange.Move(-rCxt.mnColDelta, -rCxt.mnRowDelta, -rCxt.mnTabDelta);
+
     ScRange aAbs(rRef.toAbs(rPos));
     aAbs.aEnd.IncRow(nGroupLen-1);
-    if (!rCxt.maRange.Intersects(aAbs))
+    if (!aCheckRange.Intersects(aAbs))
         return;
 
     // Get the boundary row positions.
-    if (aAbs.aEnd.Row() < rCxt.maRange.aStart.Row())
+    if (aAbs.aEnd.Row() < aCheckRange.aStart.Row())
         // No intersections.
         return;
 
-    if (aAbs.aStart.Row() <= rCxt.maRange.aStart.Row())
+    if (aAbs.aStart.Row() <= aCheckRange.aStart.Row())
     {
         //    +-+ <---- top
         //    | |
@@ -3175,11 +3180,11 @@ void checkBounds(
         // +-------+
 
         // Add offset from the reference top to the cell position.
-        SCROW nOffset = rCxt.maRange.aStart.Row() - aAbs.aStart.Row();
+        SCROW nOffset = aCheckRange.aStart.Row() - aAbs.aStart.Row();
         rBounds.push_back(rPos.Row()+nOffset);
     }
 
-    if (aAbs.aEnd.Row() >= rCxt.maRange.aEnd.Row())
+    if (aAbs.aEnd.Row() >= aCheckRange.aEnd.Row())
     {
         // only check for end range
 
@@ -3191,7 +3196,7 @@ void checkBounds(
         //    +-+
 
         // Ditto.
-        SCROW nOffset = rCxt.maRange.aEnd.Row() + 1 - aAbs.aStart.Row();
+        SCROW nOffset = aCheckRange.aEnd.Row() + 1 - aAbs.aStart.Row();
         rBounds.push_back(rPos.Row()+nOffset);
     }
 }
diff --git a/sc/source/ui/inc/undoblk.hxx b/sc/source/ui/inc/undoblk.hxx
index d79e32c..29e82ac 100644
--- a/sc/source/ui/inc/undoblk.hxx
+++ b/sc/source/ui/inc/undoblk.hxx
@@ -241,6 +241,9 @@ public:
     virtual OUString GetComment() const;
 
 private:
+    sal_uInt16 mnPaintExtFlags;
+    ScRangeList maPaintRanges;
+
     ScRange         aSrcRange;
     ScRange         aDestRange;
     sal_uLong       nStartChangeAction;
@@ -249,7 +252,7 @@ private:
     bool            bKeepScenarioFlags;
 
     void            PaintArea( ScRange aRange, sal_uInt16 nExtFlags ) const;
-    void            DoUndo( ScRange aRange ) const;
+    void DoUndo( ScRange aRange );
 
     void            SetChangeTrack();
 };
diff --git a/sc/source/ui/undo/undoblk.cxx b/sc/source/ui/undo/undoblk.cxx
index 1fdf7d15..cd0c707 100644
--- a/sc/source/ui/undo/undoblk.cxx
+++ b/sc/source/ui/undo/undoblk.cxx
@@ -1214,7 +1214,7 @@ void ScUndoDragDrop::PaintArea( ScRange aRange, sal_uInt16 nExtFlags ) const
     pDocShell->PostPaint( aRange, nPaint, nExtFlags );
 }
 
-void ScUndoDragDrop::DoUndo( ScRange aRange ) const
+void ScUndoDragDrop::DoUndo( ScRange aRange )
 {
     ScDocument* pDoc = pDocShell->GetDocument();
 
@@ -1227,8 +1227,7 @@ void ScUndoDragDrop::DoUndo( ScRange aRange ) const
     ScRange aPaintRange = aRange;
     pDoc->ExtendMerge( aPaintRange );           // before deleting
 
-    sal_uInt16 nExtFlags = 0;
-    pDocShell->UpdatePaintExt( nExtFlags, aPaintRange );
+    pDocShell->UpdatePaintExt(mnPaintExtFlags, aPaintRange);
 
     // do not undo objects and note captions, they are handled via drawing undo
     sal_uInt16 nUndoFlags = (IDF_ALL & ~IDF_OBJECTS) | IDF_NOCAPTIONS;
@@ -1241,16 +1240,26 @@ void ScUndoDragDrop::DoUndo( ScRange aRange ) const
     aPaintRange.aEnd.SetCol( std::max( aPaintRange.aEnd.Col(), aRange.aEnd.Col() ) );
     aPaintRange.aEnd.SetRow( std::max( aPaintRange.aEnd.Row(), aRange.aEnd.Row() ) );
 
-    pDocShell->UpdatePaintExt( nExtFlags, aPaintRange );
-    PaintArea( aPaintRange, nExtFlags );
+    pDocShell->UpdatePaintExt(mnPaintExtFlags, aPaintRange);
+    maPaintRanges.Join(aPaintRange);
 }
 
 void ScUndoDragDrop::Undo()
 {
+    mnPaintExtFlags = 0;
+    maPaintRanges.RemoveAll();
+
     BeginUndo();
     DoUndo(aDestRange);
     if (bCut)
         DoUndo(aSrcRange);
+
+    for (size_t i = 0; i < maPaintRanges.size(); ++i)
+    {
+        const ScRange* p = maPaintRanges[i];
+        PaintArea(*p, mnPaintExtFlags);
+    }
+
     EndUndo();
     SFX_APP()->Broadcast( SfxSimpleHint( SC_HINT_AREALINKS_CHANGED ) );
 }
commit 3cb186f57d188ad17503d60cf2f8e6a4bbd8523c
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Tue Feb 25 12:16:45 2014 -0500

    fdo#75386: Add unit test for this.
    
    Change-Id: Iff2b5fd05eb098e83281c5ae9cedf309edde7250

diff --git a/sc/qa/unit/helper/qahelper.cxx b/sc/qa/unit/helper/qahelper.cxx
index 773b779..b875265 100644
--- a/sc/qa/unit/helper/qahelper.cxx
+++ b/sc/qa/unit/helper/qahelper.cxx
@@ -430,6 +430,64 @@ bool checkFormula(ScDocument& rDoc, const ScAddress& rPos, const char* pExpected
     return true;
 }
 
+bool checkFormulaPosition(ScDocument& rDoc, const ScAddress& rPos)
+{
+    OUString aStr(rPos.Format(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(SCA_VALID));
+        cerr << "Formula cell at " << aStr << " has incorrect position of " << aStr2 << endl;
+        return false;
+    }
+
+    return true;
+}
+
+bool checkFormulaPositions(
+    ScDocument& rDoc, SCTAB nTab, SCCOL nCol, const SCROW* pRows, size_t nRowCount)
+{
+    ScAddress aPos(nCol, 0, nTab);
+    for (size_t i = 0; i < nRowCount; ++i)
+    {
+        SCROW nRow = pRows[i];
+        aPos.SetRow(nRow);
+
+        if (!checkFormulaPosition(rDoc, aPos))
+        {
+            OUString aStr(aPos.Format(SCA_VALID));
+            cerr << "Formula cell position failed at " << aStr << "." << endl;
+            return false;
+        }
+    }
+    return true;
+}
+
+void clearFormulaCellChangedFlag( ScDocument& rDoc, const ScRange& rRange )
+{
+    const ScAddress& s = rRange.aStart;
+    const ScAddress& e = rRange.aEnd;
+    for (SCTAB nTab = s.Tab(); nTab <= e.Tab(); ++nTab)
+    {
+        for (SCCOL nCol = s.Col(); nCol <= e.Col(); ++nCol)
+        {
+            for (SCROW nRow = s.Row(); nRow <= e.Row(); ++nRow)
+            {
+                ScAddress aPos(nCol, nRow, nTab);
+                ScFormulaCell* pFC = rDoc.GetFormulaCell(aPos);
+                if (pFC)
+                    pFC->SetChanged(false);
+            }
+        }
+    }
+}
+
 bool isFormulaWithoutError(ScDocument& rDoc, const ScAddress& rPos)
 {
     ScFormulaCell* pFC = rDoc.GetFormulaCell(rPos);
diff --git a/sc/qa/unit/helper/qahelper.hxx b/sc/qa/unit/helper/qahelper.hxx
index 657f66b..33a93a6 100644
--- a/sc/qa/unit/helper/qahelper.hxx
+++ b/sc/qa/unit/helper/qahelper.hxx
@@ -120,6 +120,12 @@ SCQAHELPER_DLLPUBLIC ScRangeList getChartRanges(ScDocument& rDoc, const SdrOle2O
 
 SCQAHELPER_DLLPUBLIC bool checkFormula(ScDocument& rDoc, const ScAddress& rPos, const char* pExpected);
 
+SCQAHELPER_DLLPUBLIC bool checkFormulaPosition(ScDocument& rDoc, const ScAddress& rPos);
+SCQAHELPER_DLLPUBLIC bool checkFormulaPositions(
+    ScDocument& rDoc, SCTAB nTab, SCCOL nCol, const SCROW* pRows, size_t nRowCount);
+
+SCQAHELPER_DLLPUBLIC void clearFormulaCellChangedFlag( ScDocument& rDoc, const ScRange& rRange );
+
 /**
  * Check if the cell at specified position is a formula cell that doesn't
  * have an error value.
diff --git a/sc/qa/unit/ucalc.cxx b/sc/qa/unit/ucalc.cxx
index f4c1896..d5d70550 100644
--- a/sc/qa/unit/ucalc.cxx
+++ b/sc/qa/unit/ucalc.cxx
@@ -4036,48 +4036,6 @@ void Test::testSearchCells()
     m_pDoc->DeleteTab(0);
 }
 
-namespace {
-
-bool checkFormulaPosition(ScDocument& rDoc, const ScAddress& rPos)
-{
-    OUString aStr(rPos.Format(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(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(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");
@@ -4091,13 +4049,15 @@ void Test::testFormulaPosition()
 
     {
         SCROW aRows[] = { 0, 1, 3 };
-        checkFormulaPositions(*m_pDoc, aPos, aRows, SAL_N_ELEMENTS(aRows));
+        bool bRes = checkFormulaPositions(*m_pDoc, aPos.Tab(), aPos.Col(), aRows, SAL_N_ELEMENTS(aRows));
+        CPPUNIT_ASSERT(bRes);
     }
 
     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));
+        bool bRes = checkFormulaPositions(*m_pDoc, aPos.Tab(), aPos.Col(), aRows, SAL_N_ELEMENTS(aRows));
+        CPPUNIT_ASSERT(bRes);
     }
 
     m_pDoc->DeleteTab(0);
diff --git a/sc/qa/unit/ucalc.hxx b/sc/qa/unit/ucalc.hxx
index 28e0156..4f8810a 100644
--- a/sc/qa/unit/ucalc.hxx
+++ b/sc/qa/unit/ucalc.hxx
@@ -271,6 +271,7 @@ public:
     void testSharedFormulasRefUpdateCopySheets();
     void testSharedFormulasCopyPaste();
     void testSharedFormulaInsertColumn();
+    void testSharedFormulaMoveBlock();
     void testFormulaPosition();
 
     void testMixData();
@@ -442,6 +443,7 @@ public:
     CPPUNIT_TEST(testSharedFormulasRefUpdateCopySheets);
     CPPUNIT_TEST(testSharedFormulasCopyPaste);
     CPPUNIT_TEST(testSharedFormulaInsertColumn);
+    CPPUNIT_TEST(testSharedFormulaMoveBlock);
     CPPUNIT_TEST(testFormulaPosition);
     CPPUNIT_TEST(testMixData);
     CPPUNIT_TEST(testJumpToPrecedentsDependents);
diff --git a/sc/qa/unit/ucalc_sharedformula.cxx b/sc/qa/unit/ucalc_sharedformula.cxx
index a64398b..91885ea 100644
--- a/sc/qa/unit/ucalc_sharedformula.cxx
+++ b/sc/qa/unit/ucalc_sharedformula.cxx
@@ -15,6 +15,7 @@
 #include "clipparam.hxx"
 #include "undoblk.hxx"
 #include "scopetools.hxx"
+#include <docfunc.hxx>
 #include "svl/sharedstring.hxx"
 
 #include "formula/grammar.hxx"
@@ -738,4 +739,110 @@ void Test::testSharedFormulaInsertColumn()
     m_pDoc->DeleteTab(0);
 }
 
+void Test::testSharedFormulaMoveBlock()
+{
+    sc::AutoCalcSwitch aACSwitch(*m_pDoc, true); // turn on auto calc.
+    FormulaGrammarSwitch aFGSwitch(m_pDoc, formula::FormulaGrammar::GRAM_ENGLISH_XL_R1C1);
+
+    m_pDoc->InsertTab(0, "Test");
+
+    // Set values to A1:A3.
+    m_pDoc->SetValue(ScAddress(0,0,0), 1.0);
+    m_pDoc->SetValue(ScAddress(0,1,0), 2.0);
+    m_pDoc->SetValue(ScAddress(0,2,0), 3.0);
+
+    // Set formulas in B1:B3 to reference A1:A3.
+    m_pDoc->SetString(ScAddress(1,0,0), "=RC[-1]");
+    m_pDoc->SetString(ScAddress(1,1,0), "=RC[-1]");
+    m_pDoc->SetString(ScAddress(1,2,0), "=RC[-1]");
+
+    ScRange aFormulaRange(1,0,0,1,2,0);
+
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+
+    clearFormulaCellChangedFlag(*m_pDoc, aFormulaRange);
+
+    // Move A1:A3 to D1:D3.
+    ScDocFunc& rFunc = getDocShell().GetDocFunc();
+    rFunc.MoveBlock(ScRange(0,0,0,0,2,0), ScAddress(3,0,0), true, true, false, true);
+
+    // The result should stay the same.
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+
+    clearFormulaCellChangedFlag(*m_pDoc, aFormulaRange);
+
+    // Make sure these formula cells in B1:B3 have correct positions even after the move.
+    std::vector<SCROW> aRows;
+    aRows.push_back(0);
+    aRows.push_back(1);
+    aRows.push_back(2);
+    bool bRes = checkFormulaPositions(*m_pDoc, 0, 1, &aRows[0], aRows.size());
+    CPPUNIT_ASSERT(bRes);
+
+    SfxUndoManager* pUndoMgr = m_pDoc->GetUndoManager();
+    CPPUNIT_ASSERT(pUndoMgr);
+
+    // Undo and check the result.
+    pUndoMgr->Undo();
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+
+    clearFormulaCellChangedFlag(*m_pDoc, aFormulaRange);
+
+    // Redo and check the result.
+    pUndoMgr->Redo();
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+
+    // Clear the range and start over.
+    clearRange(m_pDoc, ScRange(0,0,0,MAXCOL,MAXROW,0));
+
+    // Set values 1,2,3,4,5 to A1:A5.
+    for (SCROW i = 0; i <= 4; ++i)
+        m_pDoc->SetValue(ScAddress(0,i,0), (i+1));
+
+    // Set formulas to B1:B5.
+    for (SCROW i = 0; i <= 4; ++i)
+        m_pDoc->SetString(ScAddress(1,i,0), "=RC[-1]");
+
+    // Check the initial formula results.
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+    CPPUNIT_ASSERT_EQUAL(4.0, m_pDoc->GetValue(ScAddress(1,3,0)));
+    CPPUNIT_ASSERT_EQUAL(5.0, m_pDoc->GetValue(ScAddress(1,4,0)));
+
+    // Move A1:A2 to D2:D3.
+    rFunc.MoveBlock(ScRange(0,0,0,0,1,0), ScAddress(3,1,0), true, true, false, true);
+
+    // Check the formula values again.  They should not change.
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+    CPPUNIT_ASSERT_EQUAL(4.0, m_pDoc->GetValue(ScAddress(1,3,0)));
+    CPPUNIT_ASSERT_EQUAL(5.0, m_pDoc->GetValue(ScAddress(1,4,0)));
+
+    pUndoMgr->Undo();
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+    CPPUNIT_ASSERT_EQUAL(4.0, m_pDoc->GetValue(ScAddress(1,3,0)));
+    CPPUNIT_ASSERT_EQUAL(5.0, m_pDoc->GetValue(ScAddress(1,4,0)));
+
+    pUndoMgr->Redo();
+    CPPUNIT_ASSERT_EQUAL(1.0, m_pDoc->GetValue(ScAddress(1,0,0)));
+    CPPUNIT_ASSERT_EQUAL(2.0, m_pDoc->GetValue(ScAddress(1,1,0)));
+    CPPUNIT_ASSERT_EQUAL(3.0, m_pDoc->GetValue(ScAddress(1,2,0)));
+    CPPUNIT_ASSERT_EQUAL(4.0, m_pDoc->GetValue(ScAddress(1,3,0)));
+    CPPUNIT_ASSERT_EQUAL(5.0, m_pDoc->GetValue(ScAddress(1,4,0)));
+
+    m_pDoc->DeleteTab(0);
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */


More information about the Libreoffice-commits mailing list