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

Kohei Yoshida kohei.yoshida at gmail.com
Thu Mar 14 23:53:46 PDT 2013


 sc/inc/column.hxx               |    8 +
 sc/source/core/data/column.cxx  |   25 ++-
 sc/source/core/data/column2.cxx |   21 ++
 sc/source/core/data/column3.cxx |  292 +++++++++++++++++++---------------------
 4 files changed, 189 insertions(+), 157 deletions(-)

New commits:
commit 5b1d9145acd9dcb4d364f0bdc865c0464b3cdc80
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Mar 15 02:55:09 2013 -0400

    Delete cell segments using reverse iterator (as TODO comment said).
    
    This makes it much simpler, and less error-prone.
    
    Change-Id: I21dbe0d2bb4a71fc2ac738a5ffb03e4d959d91a5

diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index f09367b..6116780 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -165,12 +165,12 @@ void ScColumn::Delete( SCROW nRow )
         {
             pNoteCell->Delete();
             maItems.erase( maItems.begin() + nIndex);
+            maTextWidths.set_empty(nRow, nRow);
             // Should we free memory here (delta)? It'll be slower!
         }
         pCell->EndListeningTo( pDocument );
         pCell->Delete();
 
-        maTextWidths.set_empty(nRow, nRow);
         CellStorageModified();
     }
 }
@@ -360,9 +360,6 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
 
     ScHint aHint( SC_HINT_DYING, ScAddress( nCol, 0, nTab ), 0 );
 
-    SCROW nStartRow = maItems[nStartIndex].nRow;
-    SCROW nEndRow = maItems[nEndIndex].nRow;
-
     // cache all formula cells, they will be deleted at end of this function
     typedef ::std::vector< ScFormulaCell* > FormulaCellVector;
     FormulaCellVector aDelCells;
@@ -471,50 +468,40 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
     if (nFirst <= nEndIndex)
         aRemovedSegments.insert_back(nFirst, nEndIndex + 1, true);
 
-    // Remove segments from the column array, containing pDummyCell and formula
-    // cell pointers to be deleted.
-    { // own scope for variables
-        RemovedSegments_t::const_iterator aIt(aRemovedSegments.begin());
-        RemovedSegments_t::const_iterator aEnd(aRemovedSegments.end());
-        // The indexes in aRemovedSegments denote cell positions in the
-        // original array. But as we are shifting it from the left, we have
-        // to compensate for already performed shifts for latter segments.
-        // TODO: use reverse iterators instead
-        SCSIZE nShift(0);
-        SCSIZE nStartSegment(nStartIndex);
-        bool bRemoved = false;
-        for (;aIt != aEnd; ++aIt)
+    {
+        // Remove segments from the column array, containing pDummyCell and
+        // formula cell pointers to be deleted.
+
+        RemovedSegments_t::const_reverse_iterator it = aRemovedSegments.rbegin();
+        RemovedSegments_t::const_reverse_iterator itEnd = aRemovedSegments.rend();
+
+        std::vector<ColEntry>::iterator itErase, itEraseEnd;
+        SCSIZE nEndSegment = it->first; // should equal maItems.size(). Non-inclusive.
+        // Skip the first node.
+        for (++it; it != itEnd; ++it)
         {
-            if (aIt->second)
+            if (!it->second)
             {
-                // this segment removed
-                if (!bRemoved)
-                    nStartSegment = aIt->first;
-                    // The first of removes in a row sets start (they should be
-                    // alternating removed/notremoved anyway).
-                bRemoved = true;
+                // Don't remove this segment.
+                nEndSegment = it->first;
                 continue;
             }
 
-            if (bRemoved)
-            {
-                // this segment not removed, but previous segment(s) removed, move tail.
-                SCSIZE const nEndSegment(aIt->first);
-                memmove(
-                        &maItems[nStartSegment - nShift],
-                        &maItems[nEndSegment - nShift],
-                        (maItems.size() - nEndSegment) * sizeof(ColEntry));
-                nShift += nEndSegment - nStartSegment;
-                bRemoved = false;
-            }
+            // Remove this segment.
+            SCSIZE nStartSegment = it->first;
+            SCROW nStartRow = maItems[nStartSegment].nRow;
+            SCROW nEndRow = maItems[nEndSegment-1].nRow;
+
+            itErase = maItems.begin();
+            std::advance(itErase, nStartSegment);
+            itEraseEnd = maItems.begin();
+            std::advance(itEraseEnd, nEndSegment);
+            maItems.erase(itErase, itEraseEnd);
+
+            maTextWidths.set_empty(nStartRow, nEndRow);
+
+            nEndSegment = nStartSegment;
         }
-        // The last removed segment up to aItems.size() is discarded, there's
-        // nothing following to be moved.
-        if (bRemoved)
-            nShift += maItems.size() - nStartSegment;
-        maItems.erase(maItems.end() - nShift, maItems.end());
-        maTextWidths.set_empty(nStartRow, nEndRow);
-        CellStorageModified();
     }
 
     // *** delete all formula cells ***
@@ -1666,8 +1653,8 @@ void ScColumn::RemoveProtected( SCROW nStartRow, SCROW nEndRow )
                     }
                     delete pFormula;
 
-                    CellStorageModified();
                     SetTextWidth(maItems[nIndex].nRow, TEXTWIDTH_DIRTY);
+                    CellStorageModified();
                 }
                 ++nIndex;
             }
commit da91c606cb9de7ffb998ae46a0ba60cba855e3ac
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Mar 15 02:03:19 2013 -0400

    Don't forget to set text widths here too.
    
    Change-Id: Ieaa9bd0e4f8117e3bc7ceccc68d6f37daac61440

diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index e778f96..a53c8a8 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -879,12 +879,12 @@ void ScColumn::SwapRow(SCROW nRow1, SCROW nRow2)
             {
                 // remove ColEntry at old position
                 maItems.erase( maItems.begin() + nIndex1 );
+                maTextWidths.set_empty(nRow1, nRow1);
             }
 
             // Empty text width at the cell 1 position.  For now, we don't
             // transfer the old value to the cell 2 position since Insert() is
             // quite complicated.
-            maTextWidths.set_empty(nRow1, nRow1);
             CellStorageModified();
 
             // insert ColEntry at new position.
@@ -1284,6 +1284,7 @@ void ScColumn::CopyStaticToDocument(SCROW nRow1, SCROW nRow2, ScColumn& rDestCol
         itEnd = std::find_if(it, rDestCol.maItems.end(), FindAboveRow(nRow2));
         std::for_each(it, itEnd, DeleteCell());
         rDestCol.maItems.erase(it, itEnd);
+        rDestCol.maTextWidths.set_empty(nRow1, nRow2);
     }
 
     // Determine the range of cells in the original column that need to be copied.
@@ -1351,6 +1352,10 @@ void ScColumn::CopyStaticToDocument(SCROW nRow1, SCROW nRow2, ScColumn& rDestCol
     // destination column shouldn't have any cells within the specified range.
     it = std::find_if(rDestCol.maItems.begin(), rDestCol.maItems.end(), FindAboveRow(nRow2));
     rDestCol.maItems.insert(it, aCopied.begin(), aCopied.end());
+    it = aCopied.begin();
+    itEnd = aCopied.end();
+    for (; it != itEnd; ++it)
+        rDestCol.maTextWidths.set<unsigned short>(it->nRow, TEXTWIDTH_DIRTY);
 }
 
 void ScColumn::CopyToColumn(
commit 8d0e982a80aabcd10e02a4cce20cc603e8c33ad8
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Mar 15 00:54:12 2013 -0400

    Move some of the code to local functions to make it easier to read.
    
    Change-Id: Ib7ca5c04ec057dbce958d580ad3b7d52d19ed21f

diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index baaaacd..f09367b 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -303,6 +303,53 @@ void ScColumn::DeleteRow( SCROW nStartRow, SCSIZE nSize )
     pDocument->SetAutoCalc( bOldAutoCalc );
 }
 
+namespace {
+
+bool isDate(const ScDocument& rDoc, const ScColumn& rCol, SCROW nRow)
+{
+    sal_uLong nIndex = (sal_uLong)((SfxUInt32Item*)rCol.GetAttr(nRow, ATTR_VALUE_FORMAT))->GetValue();
+    short nType = rDoc.GetFormatTable()->GetType(nIndex);
+    return (nType == NUMBERFORMAT_DATE) || (nType == NUMBERFORMAT_TIME) || (nType == NUMBERFORMAT_DATETIME);
+}
+
+bool checkDeleteCellByFlag(
+    CellType eCellType, sal_uInt16 nDelFlag, const ScDocument& rDoc, const ScColumn& rCol, const ColEntry& rEntry)
+{
+    bool bDelete = false;
+
+    switch (eCellType)
+    {
+        case CELLTYPE_VALUE:
+        {
+            sal_uInt16 nValFlags = nDelFlag & (IDF_DATETIME|IDF_VALUE);
+            // delete values and dates?
+            bDelete = nValFlags == (IDF_DATETIME|IDF_VALUE);
+            // if not, decide according to cell number format
+            if (!bDelete && (nValFlags != 0))
+            {
+                bool bIsDate = isDate(rDoc, rCol, rEntry.nRow);
+                bDelete = nValFlags == (bIsDate ? IDF_DATETIME : IDF_VALUE);
+            }
+        }
+        break;
+        case CELLTYPE_STRING:
+        case CELLTYPE_EDIT:
+            bDelete = (nDelFlag & IDF_STRING) != 0;
+        break;
+        case CELLTYPE_FORMULA:
+            bDelete = (nDelFlag & IDF_FORMULA) != 0;
+        break;
+        case CELLTYPE_NOTE:
+            // do note delete note cell with broadcaster
+            bDelete = !rEntry.pCell->GetBroadcaster();
+        break;
+        default:; // added to avoid warnings
+    }
+
+    return bDelete;
+}
+
+}
 
 void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDelFlag )
 {
@@ -323,7 +370,7 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
 
     typedef mdds::flat_segment_tree<SCSIZE, bool> RemovedSegments_t;
     RemovedSegments_t aRemovedSegments(nStartIndex, maItems.size(), false);
-    SCSIZE nFirst(nStartIndex);
+    SCSIZE nFirst = nStartIndex;
 
     // dummy replacement for old cells, to prevent that interpreter uses old cell
     boost::scoped_ptr<ScNoteCell> pDummyCell(new ScNoteCell);
@@ -357,57 +404,27 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
         ScBaseCell* pOldCell = maItems[nIdx].pCell;
         CellType eCellType = pOldCell->GetCellType();
         if ((nDelFlag & IDF_CONTENTS) == IDF_CONTENTS)
+            // All cell types to be deleted.
             bDelete = true;
         else
         {
-            // decide whether to delete the cell object according to passed
-            // flags
-            switch ( eCellType )
-            {
-                case CELLTYPE_VALUE:
-                {
-                    sal_uInt16 nValFlags = nDelFlag & (IDF_DATETIME|IDF_VALUE);
-                    // delete values and dates?
-                    bDelete = nValFlags == (IDF_DATETIME|IDF_VALUE);
-                    // if not, decide according to cell number format
-                    if (!bDelete && (nValFlags != 0))
-                    {
-                        sal_uLong nIndex = (sal_uLong)((SfxUInt32Item*)GetAttr(
-                                    maItems[nIdx].nRow, ATTR_VALUE_FORMAT))->GetValue();
-                        short nType = pDocument->GetFormatTable()->GetType(nIndex);
-                        bool bIsDate = (nType == NUMBERFORMAT_DATE) ||
-                            (nType == NUMBERFORMAT_TIME) || (nType == NUMBERFORMAT_DATETIME);
-                        bDelete = nValFlags == (bIsDate ? IDF_DATETIME : IDF_VALUE);
-                    }
-                }
-                break;
-                case CELLTYPE_STRING:
-                case CELLTYPE_EDIT:
-                    bDelete = (nDelFlag & IDF_STRING) != 0;
-                break;
-                case CELLTYPE_FORMULA:
-                    bDelete = (nDelFlag & IDF_FORMULA) != 0;
-                break;
-                case CELLTYPE_NOTE:
-                    // do note delete note cell with broadcaster
-                    bDelete = !pOldCell->GetBroadcaster();
-                break;
-                default:; // added to avoid warnings
-            }
+            // Decide whether to delete the cell object according to passed
+            // flags.
+            bDelete = checkDeleteCellByFlag(eCellType, nDelFlag, *pDocument, *this, maItems[nIdx]);
         }
 
         if (bDelete)
         {
-            // try to create a replacement note cell, if note or broadcaster exists
+            // Try to create a replacement "note" cell if broadcaster exists.
             ScNoteCell* pNoteCell = NULL;
             SvtBroadcaster* pBC = pOldCell->GetBroadcaster();
             if (pBC && pBC->HasListeners())
             {
-                pNoteCell = new ScNoteCell( pBC );
-                // NOTE: the broadcaster here is transferred and released
-                // only if it has listeners! If it does not, it will simply
-                // be deleted when the cell is deleted and no replacement
-                // cell is created.
+                // NOTE: the broadcaster here is transferred and released only
+                // if it has listeners! If it does not, it will simply be
+                // deleted when the cell is deleted and no replacement cell is
+                // created.
+                pNoteCell = new ScNoteCell(pBC);
                 pOldCell->ReleaseBroadcaster();
             }
 
@@ -423,16 +440,16 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
             else
                 maItems[nIdx].pCell = pDummyCell.get();
 
-            // cache formula cells (will be deleted later), delete cell of other type
             if (eCellType == CELLTYPE_FORMULA)
             {
-                aDelCells.push_back( static_cast< ScFormulaCell* >( pOldCell ) );
+                // Cache formula cells (will be deleted later), delete cell of other type.
+                aDelCells.push_back(static_cast<ScFormulaCell*>(pOldCell));
             }
             else
             {
-                aHint.GetAddress().SetRow( nOldRow );
-                aHint.SetCell( pNoteCell ? pNoteCell : pOldCell );
-                pDocument->Broadcast( aHint );
+                aHint.GetAddress().SetRow(nOldRow);
+                aHint.SetCell(pNoteCell ? pNoteCell : pOldCell);
+                pDocument->Broadcast(aHint);
                 if (pNoteCell != pOldCell)
                 {
                     pOldCell->Delete();
commit ed75f60db067ccfe87e077b3bc6b2cb76f3a0c7b
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Mar 15 00:11:37 2013 -0400

    Reduce indentation level.
    
    Change-Id: Ia70d1dfde53e13e7d40e7cb8fa09dc401570aa93

diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index cc25641..baaaacd 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -330,9 +330,10 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
 
     for ( SCSIZE nIdx = nStartIndex; nIdx <= nEndIndex; ++nIdx )
     {
-        // all content is deleted and cell does not contain broadcaster
         if (((nDelFlag & IDF_CONTENTS) == IDF_CONTENTS) && !maItems[ nIdx ].pCell->GetBroadcaster())
         {
+            // all content is deleted and cell does not contain broadcaster
+
             ScBaseCell* pOldCell = maItems[ nIdx ].pCell;
             if (pOldCell->GetCellType() == CELLTYPE_FORMULA)
             {
@@ -348,110 +349,105 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
                 pDocument->Broadcast( aHint );
                 pOldCell->Delete();
             }
+            continue;
         }
+
         // delete some contents of the cells, or cells with broadcaster
+        bool bDelete = false;
+        ScBaseCell* pOldCell = maItems[nIdx].pCell;
+        CellType eCellType = pOldCell->GetCellType();
+        if ((nDelFlag & IDF_CONTENTS) == IDF_CONTENTS)
+            bDelete = true;
         else
         {
-            bool bDelete = false;
-            ScBaseCell* pOldCell = maItems[nIdx].pCell;
-            CellType eCellType = pOldCell->GetCellType();
-            if ((nDelFlag & IDF_CONTENTS) == IDF_CONTENTS)
-                bDelete = true;
-            else
+            // decide whether to delete the cell object according to passed
+            // flags
+            switch ( eCellType )
             {
-                // decide whether to delete the cell object according to passed
-                // flags
-                switch ( eCellType )
+                case CELLTYPE_VALUE:
                 {
-                    case CELLTYPE_VALUE:
-                        {
-                            sal_uInt16 nValFlags = nDelFlag & (IDF_DATETIME|IDF_VALUE);
-                            // delete values and dates?
-                            bDelete = nValFlags == (IDF_DATETIME|IDF_VALUE);
-                            // if not, decide according to cell number format
-                            if( !bDelete && (nValFlags != 0) )
-                            {
-                                sal_uLong nIndex = (sal_uLong)((SfxUInt32Item*)GetAttr(
-                                            maItems[nIdx].nRow, ATTR_VALUE_FORMAT ))->GetValue();
-                                short nType = pDocument->GetFormatTable()->GetType(nIndex);
-                                bool bIsDate = (nType == NUMBERFORMAT_DATE) ||
-                                    (nType == NUMBERFORMAT_TIME) || (nType == NUMBERFORMAT_DATETIME);
-                                bDelete = nValFlags == (bIsDate ? IDF_DATETIME : IDF_VALUE);
-                            }
-                        }
-                        break;
-
-                    case CELLTYPE_STRING:
-                    case CELLTYPE_EDIT:
-                        bDelete = (nDelFlag & IDF_STRING) != 0;
-                        break;
-
-                    case CELLTYPE_FORMULA:
-                        bDelete = (nDelFlag & IDF_FORMULA) != 0;
-                        break;
-
-                    case CELLTYPE_NOTE:
-                        // do note delete note cell with broadcaster
-                        bDelete = !pOldCell->GetBroadcaster();
-                        break;
-
-                    default:; // added to avoid warnings
+                    sal_uInt16 nValFlags = nDelFlag & (IDF_DATETIME|IDF_VALUE);
+                    // delete values and dates?
+                    bDelete = nValFlags == (IDF_DATETIME|IDF_VALUE);
+                    // if not, decide according to cell number format
+                    if (!bDelete && (nValFlags != 0))
+                    {
+                        sal_uLong nIndex = (sal_uLong)((SfxUInt32Item*)GetAttr(
+                                    maItems[nIdx].nRow, ATTR_VALUE_FORMAT))->GetValue();
+                        short nType = pDocument->GetFormatTable()->GetType(nIndex);
+                        bool bIsDate = (nType == NUMBERFORMAT_DATE) ||
+                            (nType == NUMBERFORMAT_TIME) || (nType == NUMBERFORMAT_DATETIME);
+                        bDelete = nValFlags == (bIsDate ? IDF_DATETIME : IDF_VALUE);
+                    }
                 }
+                break;
+                case CELLTYPE_STRING:
+                case CELLTYPE_EDIT:
+                    bDelete = (nDelFlag & IDF_STRING) != 0;
+                break;
+                case CELLTYPE_FORMULA:
+                    bDelete = (nDelFlag & IDF_FORMULA) != 0;
+                break;
+                case CELLTYPE_NOTE:
+                    // do note delete note cell with broadcaster
+                    bDelete = !pOldCell->GetBroadcaster();
+                break;
+                default:; // added to avoid warnings
             }
+        }
 
-            if (bDelete)
+        if (bDelete)
+        {
+            // try to create a replacement note cell, if note or broadcaster exists
+            ScNoteCell* pNoteCell = NULL;
+            SvtBroadcaster* pBC = pOldCell->GetBroadcaster();
+            if (pBC && pBC->HasListeners())
             {
-                // try to create a replacement note cell, if note or broadcaster exists
-                ScNoteCell* pNoteCell = NULL;
-                SvtBroadcaster* pBC = pOldCell->GetBroadcaster();
-                if (pBC && pBC->HasListeners())
-                {
-                    pNoteCell = new ScNoteCell( pBC );
-                    // NOTE: the broadcaster here is transferred and released
-                    // only if it has listeners! If it does not, it will simply
-                    // be deleted when the cell is deleted and no replacement
-                    // cell is created.
-                    pOldCell->ReleaseBroadcaster();
-                }
+                pNoteCell = new ScNoteCell( pBC );
+                // NOTE: the broadcaster here is transferred and released
+                // only if it has listeners! If it does not, it will simply
+                // be deleted when the cell is deleted and no replacement
+                // cell is created.
+                pOldCell->ReleaseBroadcaster();
+            }
 
-                // remove cell entry in cell item list
-                SCROW nOldRow = maItems[nIdx].nRow;
-                if (pNoteCell)
-                {
-                    // replace old cell with the replacement note cell
-                    maItems[nIdx].pCell = pNoteCell;
-                    // ... so it's not really deleted
-                    bDelete = false;
-                }
-                else
-                    maItems[nIdx].pCell = pDummyCell.get();
+            // remove cell entry in cell item list
+            SCROW nOldRow = maItems[nIdx].nRow;
+            if (pNoteCell)
+            {
+                // replace old cell with the replacement note cell
+                maItems[nIdx].pCell = pNoteCell;
+                // ... so it's not really deleted
+                bDelete = false;
+            }
+            else
+                maItems[nIdx].pCell = pDummyCell.get();
 
-                // cache formula cells (will be deleted later), delete cell of other type
-                if (eCellType == CELLTYPE_FORMULA)
-                {
-                    aDelCells.push_back( static_cast< ScFormulaCell* >( pOldCell ) );
-                }
-                else
+            // cache formula cells (will be deleted later), delete cell of other type
+            if (eCellType == CELLTYPE_FORMULA)
+            {
+                aDelCells.push_back( static_cast< ScFormulaCell* >( pOldCell ) );
+            }
+            else
+            {
+                aHint.GetAddress().SetRow( nOldRow );
+                aHint.SetCell( pNoteCell ? pNoteCell : pOldCell );
+                pDocument->Broadcast( aHint );
+                if (pNoteCell != pOldCell)
                 {
-                    aHint.GetAddress().SetRow( nOldRow );
-                    aHint.SetCell( pNoteCell ? pNoteCell : pOldCell );
-                    pDocument->Broadcast( aHint );
-                    if (pNoteCell != pOldCell)
-                    {
-                        pOldCell->Delete();
-                    }
+                    pOldCell->Delete();
                 }
             }
+        }
 
-            if (!bDelete)
-            {
-                // We just came to a non-deleted cell after a segment of
-                // deleted ones. So we need to remember the segment
-                // before moving on.
-                if (nFirst < nIdx)
-                    aRemovedSegments.insert_back(nFirst, nIdx, true);
-                nFirst = nIdx + 1;
-            }
+        if (!bDelete)
+        {
+            // We just came to a non-deleted cell after a segment of
+            // deleted ones. So we need to remember the segment
+            // before moving on.
+            if (nFirst < nIdx)
+                aRemovedSegments.insert_back(nFirst, nIdx, true);
+            nFirst = nIdx + 1;
         }
     }
     // there is a segment of deleted cells at the end
@@ -470,30 +466,30 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
         SCSIZE nShift(0);
         SCSIZE nStartSegment(nStartIndex);
         bool bRemoved = false;
-        while (aIt != aEnd)
+        for (;aIt != aEnd; ++aIt)
         {
             if (aIt->second)
-            { // this segment removed
+            {
+                // this segment removed
                 if (!bRemoved)
                     nStartSegment = aIt->first;
                     // The first of removes in a row sets start (they should be
                     // alternating removed/notremoved anyway).
                 bRemoved = true;
+                continue;
             }
-            else
-            { // this segment not removed
-                if (bRemoved)
-                { // previous segment(s) removed, move tail
-                    SCSIZE const nEndSegment(aIt->first);
-                    memmove(
-                            &maItems[nStartSegment - nShift],
-                            &maItems[nEndSegment - nShift],
-                            (maItems.size() - nEndSegment) * sizeof(ColEntry));
-                    nShift += nEndSegment - nStartSegment;
-                    bRemoved = false;
-                }
+
+            if (bRemoved)
+            {
+                // this segment not removed, but previous segment(s) removed, move tail.
+                SCSIZE const nEndSegment(aIt->first);
+                memmove(
+                        &maItems[nStartSegment - nShift],
+                        &maItems[nEndSegment - nShift],
+                        (maItems.size() - nEndSegment) * sizeof(ColEntry));
+                nShift += nEndSegment - nStartSegment;
+                bRemoved = false;
             }
-            ++aIt;
         }
         // The last removed segment up to aItems.size() is discarded, there's
         // nothing following to be moved.
commit ce81e00c0e4dcd4929bbaa9244f38e205e0d170d
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Thu Mar 14 22:57:56 2013 -0400

    Add integrity check after column cell storage is modified.
    
    Change-Id: I8d2bd7616e0428e4e881ef0dc1012c4973e636a9

diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index ca68e7f..666d067 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -28,6 +28,14 @@
 #include <set>
 #include <vector>
 
+#define DEBUG_COLUMN_STORAGE 1
+
+#if DEBUG_COLUMN_STORAGE
+#ifdef NDEBUG
+#undef NDEBUG
+#endif
+#endif
+
 #include <mdds/multi_type_vector.hpp>
 #include <mdds/multi_type_vector_trait.hpp>
 
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 9190b31..19e5e9c 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -56,6 +56,13 @@
 
 #include <math.h>
 
+#if DEBUG_COLUMN_STORAGE
+#include "columniterator.hxx"
+#include <iostream>
+using std::cout;
+using std::endl;
+#endif
+
 // -----------------------------------------------------------------------
 
 // factor from font size to optimal cell height (text width)
@@ -1384,6 +1391,20 @@ SCROW ScColumn::FindNextVisibleRowWithContent(SCROW nRow, bool bForward) const
 
 void ScColumn::CellStorageModified()
 {
+#if DEBUG_COLUMN_STORAGE
+    ScColumnTextWidthIterator aIter(*this, 0, MAXROW);
+    for (; aIter.hasCell(); aIter.next())
+    {
+        SCROW nRow = aIter.getPos();
+        ScBaseCell* pCell = GetCell(nRow);
+        if (!pCell)
+        {
+            cout << "Cell and text width storages are out of sync!" << endl;
+            cout.flush();
+            abort();
+        }
+    }
+#endif
 }
 
 unsigned short ScColumn::GetTextWidth(SCROW nRow) const
commit c5ca9a66c36cc30aea81be4e99f73d14777d27f9
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Thu Mar 14 22:35:43 2013 -0400

    Call CellStorageModified() *after* the text widths get modified.
    
    So that I can put some integrity check code in there to test the
    integrity of cell storage.
    
    Change-Id: I0cc141ea74c27db1a014390b7abf807220e7be9f

diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index 682afc6..e778f96 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -852,7 +852,6 @@ void ScColumn::SwapRow(SCROW nRow1, SCROW nRow2)
                 be performed (but keep broadcasters and notes at old position). */
             maItems[nIndex1].pCell = pCell2;
             maItems[nIndex2].pCell = pCell1;
-            CellStorageModified();
 
             SvtBroadcaster* pBC2 = pCell2->ReleaseBroadcaster();
             pCell1->TakeBroadcaster( pBC2 );
@@ -863,6 +862,7 @@ void ScColumn::SwapRow(SCROW nRow1, SCROW nRow2)
             unsigned short nVal2 = maTextWidths.get<unsigned short>(nRow2);
             maTextWidths.set<unsigned short>(nRow1, nVal2);
             maTextWidths.set<unsigned short>(nRow2, nVal1);
+            CellStorageModified();
         }
         else
         {
@@ -874,19 +874,18 @@ void ScColumn::SwapRow(SCROW nRow1, SCROW nRow2)
             {
                 // insert dummy note cell (without note) containing old broadcaster
                 maItems[nIndex1].pCell = pDummyCell;
-                CellStorageModified();
             }
             else
             {
                 // remove ColEntry at old position
                 maItems.erase( maItems.begin() + nIndex1 );
-                CellStorageModified();
             }
 
             // Empty text width at the cell 1 position.  For now, we don't
             // transfer the old value to the cell 2 position since Insert() is
             // quite complicated.
             maTextWidths.set_empty(nRow1, nRow1);
+            CellStorageModified();
 
             // insert ColEntry at new position.
             Insert( nRow2, pCell1 );
@@ -1013,14 +1012,14 @@ void ScColumn::SwapCell( SCROW nRow, ScColumn& rCol)
             pFmlaCell2->UpdateReference(URM_MOVE, aRange, -dx, 0, 0);
         }
 
-        CellStorageModified();
-        rCol.CellStorageModified();
-
         // Swap the text widths.
         unsigned short nVal1 = maTextWidths.get<unsigned short>(nRow);
         unsigned short nVal2 = rCol.maTextWidths.get<unsigned short>(nRow);
         maTextWidths.set<unsigned short>(nRow, nVal2);
         rCol.maTextWidths.set<unsigned short>(nRow, nVal1);
+
+        CellStorageModified();
+        rCol.CellStorageModified();
     }
     else
     {
@@ -1037,8 +1036,9 @@ void ScColumn::SwapCell( SCROW nRow, ScColumn& rCol)
             pFmlaCell1->UpdateReference(URM_MOVE, aRange, dx, 0, 0);
         }
 
-        CellStorageModified();
         maTextWidths.set_empty(nRow, nRow);
+        CellStorageModified();
+
         // We don't transfer the text width to the destination column because
         // of Insert()'s complexity.
 
@@ -1189,8 +1189,8 @@ void ScColumn::InsertRow( SCROW nStartRow, SCSIZE nSize )
 
     pDocument->SetAutoCalc( bOldAutoCalc );
 
-    CellStorageModified();
     maTextWidths.insert_empty(nStartRow, nSize);
+    CellStorageModified();
 }
 
 
@@ -1691,8 +1691,8 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol)
 
         if (bErased)
         {
-            CellStorageModified();
             maTextWidths.set_empty(nStartRow, nEndRow);
+            CellStorageModified();
         }
     }
 }
diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index 57b5370..cc25641 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -88,17 +88,16 @@ void ScColumn::Insert( SCROW nRow, ScBaseCell* pNewCell )
             }
             pOldCell->Delete();
             maItems[nIndex].pCell = pNewCell;
-            CellStorageModified();
         }
         else
         {
             maItems.insert(maItems.begin() + nIndex, ColEntry());
             maItems[nIndex].pCell = pNewCell;
             maItems[nIndex].nRow  = nRow;
-            CellStorageModified();
         }
 
         maTextWidths.set<unsigned short>(nRow, TEXTWIDTH_DIRTY);
+        CellStorageModified();
     }
     // When we insert from the Clipboard we still have wrong (old) References!
     // First they are rewired in CopyBlockFromClip via UpdateReference and the
@@ -142,8 +141,8 @@ void ScColumn::Append( SCROW nRow, ScBaseCell* pCell )
     maItems.back().pCell = pCell;
     maItems.back().nRow  = nRow;
 
-    CellStorageModified();
     maTextWidths.set<unsigned short>(nRow, TEXTWIDTH_DIRTY);
+    CellStorageModified();
 }
 
 
@@ -156,7 +155,6 @@ void ScColumn::Delete( SCROW nRow )
         ScBaseCell* pCell = maItems[nIndex].pCell;
         ScNoteCell* pNoteCell = new ScNoteCell;
         maItems[nIndex].pCell = pNoteCell; // Dummy for Interpret
-        CellStorageModified();
         pDocument->Broadcast( ScHint( SC_HINT_DYING,
             ScAddress( nCol, nRow, nTab ), pCell ) );
         if ( SvtBroadcaster* pBC = pCell->ReleaseBroadcaster() )
@@ -167,13 +165,13 @@ void ScColumn::Delete( SCROW nRow )
         {
             pNoteCell->Delete();
             maItems.erase( maItems.begin() + nIndex);
-            CellStorageModified();
             // Should we free memory here (delta)? It'll be slower!
         }
         pCell->EndListeningTo( pDocument );
         pCell->Delete();
 
         maTextWidths.set_empty(nRow, nRow);
+        CellStorageModified();
     }
 }
 
@@ -191,8 +189,8 @@ void ScColumn::DeleteAtIndex( SCSIZE nIndex )
     pCell->EndListeningTo( pDocument );
     pCell->Delete();
 
-    CellStorageModified();
     maTextWidths.set_empty(nRow, nRow);
+    CellStorageModified();
 }
 
 
@@ -202,8 +200,8 @@ void ScColumn::FreeAll()
         maItems[i].pCell->Delete();
     maItems.clear();
 
-    CellStorageModified();
     maTextWidths.clear();
+    CellStorageModified();
 }
 
 
@@ -502,8 +500,8 @@ void ScColumn::DeleteRange( SCSIZE nStartIndex, SCSIZE nEndIndex, sal_uInt16 nDe
         if (bRemoved)
             nShift += maItems.size() - nStartSegment;
         maItems.erase(maItems.end() - nShift, maItems.end());
-        CellStorageModified();
         maTextWidths.set_empty(nStartRow, nEndRow);
+        CellStorageModified();
     }
 
     // *** delete all formula cells ***
@@ -1430,8 +1428,8 @@ bool ScColumn::SetString( SCROW nRow, SCTAB nTabP, const String& rString,
 
                 pOldCell->Delete();
                 maItems[i].pCell = pNewCell; // Replace
+                maTextWidths.set<unsigned short>(nRow, TEXTWIDTH_DIRTY);
                 CellStorageModified();
-                SetTextWidth(nRow, TEXTWIDTH_DIRTY);
 
                 if ( pNewCell->GetCellType() == CELLTYPE_FORMULA )
                 {


More information about the Libreoffice-commits mailing list