sc/source/core/data/column.cxx | 122 +++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 65 deletions(-)
New commits: commit 157b804ba1dad4b857bd723c454ab907b623a980 Author: Kohei Yoshida <[email protected]> Date: Thu Nov 1 20:16:15 2012 -0400 Add comments to make it easier to follow this non-obvious code. Change-Id: Ib3d3e5b57799c22916845899839ddcc9a81e9b98 diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index 1321cd2..5debdd1 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -1484,6 +1484,7 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) SCSIZE i; Search( nStartRow, i); // i points to start row or position thereafter SCSIZE nStartPos = i; + // First, copy the cell instances to the new column. for ( ; i < maItems.size() && maItems[i].nRow <= nEndRow; ++i) { SCROW nRow = maItems[i].nRow; @@ -1531,15 +1532,16 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) nStartPos = (*it).first; nStopPos = (*it).second; for (i=nStartPos; i<nStopPos; ++i) - maItems[i].pCell = pNoteCell; + maItems[i].pCell = pNoteCell; // Assign the dumpy cell instance to all slots. for (i=nStartPos; i<nStopPos; ++i) { rAddress.SetRow( maItems[i].nRow ); pDocument->AreaBroadcast( aHint ); } + // Erase the slots containing pointers to the dummy cell instance. maItems.erase(maItems.begin() + nStartPos, maItems.begin() + nStopPos); } - pNoteCell->Delete(); + pNoteCell->Delete(); // Delete the dummy cell instance. } } commit f978013f9b8d5256aa029a3572d905bebb55c5f4 Author: Kohei Yoshida <[email protected]> Date: Thu Nov 1 20:11:22 2012 -0400 Remove the correct range, or else maItems would end up with invalid pointer. nStopPos is non-inclusive, and STL's erase() method also expects a non-inclusive end position (like any other STL methods do). It's wrong to -1 here which would end up not erasing the last element containing a pointer to the deleted cell instance. Change-Id: Ia3ef4469b50695038836ff7b9b48172256032786 diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index 14e01f7..1321cd2 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -1537,7 +1537,7 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) rAddress.SetRow( maItems[i].nRow ); pDocument->AreaBroadcast( aHint ); } - maItems.erase(maItems.begin() + nStartPos, maItems.begin() + nStopPos - 1); + maItems.erase(maItems.begin() + nStartPos, maItems.begin() + nStopPos); } pNoteCell->Delete(); } commit 1c2ef32a78e1b59a7cb49218d78cf5abc70c8518 Author: Kohei Yoshida <[email protected]> Date: Thu Nov 1 20:03:06 2012 -0400 Now this bConsecutive flag makes no sense. Treat as if this flag is always false. Change-Id: Ie1364ac54f95263aa316bf81f631e607843934d5 diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index b1f81af..14e01f7 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -1481,7 +1481,6 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) return; ::std::vector<SCROW> aRows; - bool bConsecutive = true; SCSIZE i; Search( nStartRow, i); // i points to start row or position thereafter SCSIZE nStartPos = i; @@ -1498,9 +1497,6 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) typedef ::std::pair<SCSIZE,SCSIZE> PosPair; typedef ::std::vector<PosPair> EntryPosPairs; EntryPosPairs aEntries; - if (bConsecutive) - aEntries.push_back( PosPair(nStartPos, nStopPos)); - else { bool bFirst = true; nStopPos = 0; commit 79979dd23104bf35aaa18bc3e80449fe5537499c Author: Kohei Yoshida <[email protected]> Date: Thu Nov 1 18:52:24 2012 -0400 This if statement is never true. SCROW nRow = maItems[i].nRow; and nRow will never be modified afterwards. So if (nRow != maItems[i].nRow) is never true. Change-Id: I4f867a704d50138aee8c5e7f37464880470098c2 diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index ffff657..b1f81af 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -1490,11 +1490,6 @@ void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) SCROW nRow = maItems[i].nRow; aRows.push_back( nRow); rCol.Insert( nRow, maItems[i].pCell); - if (nRow != maItems[i].nRow) - { // Listener inserted - bConsecutive = false; - Search( nRow, i); - } } SCSIZE nStopPos = i; if (nStartPos < nStopPos) commit 1f422ffb23747cdc16dbeca5d0222d66fbab383c Author: Kohei Yoshida <[email protected]> Date: Thu Nov 1 17:55:34 2012 -0400 Prefer early bailout to avoid big fat if block. Change-Id: I028606c41b1486349f96b62e0ddb071ad46e9e55 diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx index 3636f60..ffff657 100644 --- a/sc/source/core/data/column.cxx +++ b/sc/source/core/data/column.cxx @@ -1472,87 +1472,86 @@ void ScColumn::SwapCol(ScColumn& rCol) } } - void ScColumn::MoveTo(SCROW nStartRow, SCROW nEndRow, ScColumn& rCol) { pAttrArray->MoveTo(nStartRow, nEndRow, *rCol.pAttrArray); - if ( !maItems.empty() ) + if (maItems.empty()) + // No cells to move. + return; + + ::std::vector<SCROW> aRows; + bool bConsecutive = true; + SCSIZE i; + Search( nStartRow, i); // i points to start row or position thereafter + SCSIZE nStartPos = i; + for ( ; i < maItems.size() && maItems[i].nRow <= nEndRow; ++i) { - ::std::vector<SCROW> aRows; - bool bConsecutive = true; - SCSIZE i; - Search( nStartRow, i); // i points to start row or position thereafter - SCSIZE nStartPos = i; - for ( ; i < maItems.size() && maItems[i].nRow <= nEndRow; ++i) - { - SCROW nRow = maItems[i].nRow; - aRows.push_back( nRow); - rCol.Insert( nRow, maItems[i].pCell); - if (nRow != maItems[i].nRow) - { // Listener inserted - bConsecutive = false; - Search( nRow, i); - } + SCROW nRow = maItems[i].nRow; + aRows.push_back( nRow); + rCol.Insert( nRow, maItems[i].pCell); + if (nRow != maItems[i].nRow) + { // Listener inserted + bConsecutive = false; + Search( nRow, i); } - SCSIZE nStopPos = i; - if (nStartPos < nStopPos) + } + SCSIZE nStopPos = i; + if (nStartPos < nStopPos) + { + // Create list of ranges of cell entry positions + typedef ::std::pair<SCSIZE,SCSIZE> PosPair; + typedef ::std::vector<PosPair> EntryPosPairs; + EntryPosPairs aEntries; + if (bConsecutive) + aEntries.push_back( PosPair(nStartPos, nStopPos)); + else { - // Create list of ranges of cell entry positions - typedef ::std::pair<SCSIZE,SCSIZE> PosPair; - typedef ::std::vector<PosPair> EntryPosPairs; - EntryPosPairs aEntries; - if (bConsecutive) - aEntries.push_back( PosPair(nStartPos, nStopPos)); - else + bool bFirst = true; + nStopPos = 0; + for (::std::vector<SCROW>::const_iterator it( aRows.begin()); + it != aRows.end() && nStopPos < maItems.size(); ++it, + ++nStopPos) { - bool bFirst = true; - nStopPos = 0; - for (::std::vector<SCROW>::const_iterator it( aRows.begin()); - it != aRows.end() && nStopPos < maItems.size(); ++it, - ++nStopPos) + if (!bFirst && *it != maItems[nStopPos].nRow) { - if (!bFirst && *it != maItems[nStopPos].nRow) - { - aEntries.push_back( PosPair(nStartPos, nStopPos)); - bFirst = true; - } - if (bFirst && Search( *it, nStartPos)) - { - bFirst = false; - nStopPos = nStartPos; - } - } - if (!bFirst && nStartPos < nStopPos) aEntries.push_back( PosPair(nStartPos, nStopPos)); - } - // Broadcast changes - ScAddress aAdr( nCol, 0, nTab ); - ScHint aHint( SC_HINT_DYING, aAdr, NULL ); // areas only - ScAddress& rAddress = aHint.GetAddress(); - ScNoteCell* pNoteCell = new ScNoteCell; // Dummy like in DeleteRange - - // must iterate backwards, because indexes of following cells become invalid - for (EntryPosPairs::reverse_iterator it( aEntries.rbegin()); - it != aEntries.rend(); ++it) - { - nStartPos = (*it).first; - nStopPos = (*it).second; - for (i=nStartPos; i<nStopPos; ++i) - maItems[i].pCell = pNoteCell; - for (i=nStartPos; i<nStopPos; ++i) + bFirst = true; + } + if (bFirst && Search( *it, nStartPos)) { - rAddress.SetRow( maItems[i].nRow ); - pDocument->AreaBroadcast( aHint ); + bFirst = false; + nStopPos = nStartPos; } - maItems.erase(maItems.begin() + nStartPos, maItems.begin() + nStopPos - 1); } - pNoteCell->Delete(); + if (!bFirst && nStartPos < nStopPos) + aEntries.push_back( PosPair(nStartPos, nStopPos)); } + // Broadcast changes + ScAddress aAdr( nCol, 0, nTab ); + ScHint aHint( SC_HINT_DYING, aAdr, NULL ); // areas only + ScAddress& rAddress = aHint.GetAddress(); + ScNoteCell* pNoteCell = new ScNoteCell; // Dummy like in DeleteRange + + // must iterate backwards, because indexes of following cells become invalid + for (EntryPosPairs::reverse_iterator it( aEntries.rbegin()); + it != aEntries.rend(); ++it) + { + nStartPos = (*it).first; + nStopPos = (*it).second; + for (i=nStartPos; i<nStopPos; ++i) + maItems[i].pCell = pNoteCell; + for (i=nStartPos; i<nStopPos; ++i) + { + rAddress.SetRow( maItems[i].nRow ); + pDocument->AreaBroadcast( aHint ); + } + maItems.erase(maItems.begin() + nStartPos, maItems.begin() + nStopPos - 1); + } + pNoteCell->Delete(); } } - bool ScColumn::UpdateReference( UpdateRefMode eUpdateRefMode, SCCOL nCol1, SCROW nRow1, SCTAB nTab1, SCCOL nCol2, SCROW nRow2, SCTAB nTab2, SCsCOL nDx, SCsROW nDy, SCsTAB nDz, ScDocument* pUndoDoc ) _______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
