[Libreoffice-commits] .: 5 commits - sc/source
Libreoffice Gerrit user
logerrit at kemper.freedesktop.org
Thu Nov 1 17:17:33 PDT 2012
sc/source/core/data/column.cxx | 122 +++++++++++++++++++----------------------
1 file changed, 57 insertions(+), 65 deletions(-)
New commits:
commit 157b804ba1dad4b857bd723c454ab907b623a980
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
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 <kohei.yoshida at gmail.com>
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 <kohei.yoshida at gmail.com>
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 <kohei.yoshida at gmail.com>
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 <kohei.yoshida at gmail.com>
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 )
More information about the Libreoffice-commits
mailing list