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

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Fri Sep 21 17:21:25 PDT 2012


 sc/qa/unit/rangelst_test.cxx     |   87 +++++++
 sc/source/core/tool/rangelst.cxx |  442 +++++++++++++++++++++++++++------------
 2 files changed, 404 insertions(+), 125 deletions(-)

New commits:
commit cb7ee824dc0b9dcc2fd466f190945de01a9d1fa5
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Sep 21 20:20:18 2012 -0400

    Fixed all discovered errors and extra unit tests to cover them.
    
    Change-Id: I1f91d66caed7fd3839f68f1e4495f0c05ee49acc

diff --git a/sc/qa/unit/rangelst_test.cxx b/sc/qa/unit/rangelst_test.cxx
index 7079166..f6e1d53 100644
--- a/sc/qa/unit/rangelst_test.cxx
+++ b/sc/qa/unit/rangelst_test.cxx
@@ -22,8 +22,12 @@ public:
 
     void testDeleteArea_4Ranges();
     void testDeleteArea_3Ranges();
+    void testDeleteArea_3Ranges_Case2();
+    void testDeleteArea_3Ranges_Case3();
     void testDeleteArea_2Ranges();
     void testDeleteArea_2Ranges_Case2();
+    void testDeleteArea_2Ranges_Case3();
+    void testDeleteArea_2Ranges_Case4();
     void testDeleteArea_1Range();
     void testDeleteArea_0Ranges();
 
@@ -33,8 +37,12 @@ public:
     CPPUNIT_TEST_SUITE(Test);
     CPPUNIT_TEST(testDeleteArea_4Ranges);
     CPPUNIT_TEST(testDeleteArea_3Ranges);
+    CPPUNIT_TEST(testDeleteArea_3Ranges_Case2);
+    CPPUNIT_TEST(testDeleteArea_3Ranges_Case3);
     CPPUNIT_TEST(testDeleteArea_2Ranges);
     CPPUNIT_TEST(testDeleteArea_2Ranges_Case2);
+    CPPUNIT_TEST(testDeleteArea_2Ranges_Case3);
+    CPPUNIT_TEST(testDeleteArea_2Ranges_Case4);
     CPPUNIT_TEST(testDeleteArea_1Range);
     CPPUNIT_TEST(testDeleteArea_0Ranges);
     CPPUNIT_TEST(testUpdateReference_DeleteRow);
@@ -105,6 +113,47 @@ void Test::testDeleteArea_3Ranges()
     CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(28), aList.GetCellCount());
 }
 
+void Test::testDeleteArea_3Ranges_Case2()
+{
+    ScRangeList aList(ScRange(1,1,0,6,6,0));
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aList.size());
+    aList.DeleteArea(0,2,0,2,4,0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aList.size());
+
+    // Column 1-2 && Row 2-4 should not be in the range list. The rest should
+    // be in the list.
+    for (SCCOL nCol = 1; nCol <= 6; ++nCol)
+    {
+        for (SCROW nRow = 1; nRow <= 6; ++nRow)
+        {
+            if ((1 <= nCol && nCol <= 2) && (2 <= nRow && nRow <= 4))
+                CPPUNIT_ASSERT(!aList.In(ScRange(nCol, nRow, 0)));
+            else
+                CPPUNIT_ASSERT(aList.In(ScRange(nCol, nRow, 0)));
+        }
+    }
+}
+
+void Test::testDeleteArea_3Ranges_Case3()
+{
+    ScRangeList aList(ScRange(1,5,0,6,11,0));
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aList.size());
+    aList.DeleteArea(3,2,0,4,8,0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), aList.size());
+
+    // Column 3-4 && Row 5-8 should not be in the range list.
+    for (SCCOL nCol = 1; nCol <= 6; ++nCol)
+    {
+        for (SCROW nRow = 5; nRow <= 11; ++nRow)
+        {
+            if ((3 <= nCol && nCol <= 4) && (5 <= nRow && nRow <= 8))
+                CPPUNIT_ASSERT(!aList.In(ScRange(nCol, nRow, 0)));
+            else
+                CPPUNIT_ASSERT(aList.In(ScRange(nCol, nRow, 0)));
+        }
+    }
+}
+
 void Test::testDeleteArea_2Ranges()
 {
     ScRangeList aList(ScRange(0,0,0,5,5,5));
@@ -142,6 +191,44 @@ void Test::testDeleteArea_2Ranges_Case2()
     CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(4), aList.GetCellCount());
 }
 
+void Test::testDeleteArea_2Ranges_Case3()
+{
+    ScRangeList aList(ScRange(0,5,0,2,10,0));
+    aList.DeleteArea(2,3,0,3,7,0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aList.size());
+
+    // Column 2 Row 5-7 shouldn't be in the list.
+    for (SCCOL nCol = 0; nCol <= 2; ++nCol)
+    {
+        for (SCROW nRow = 5; nRow <= 10; ++nRow)
+        {
+            if (nCol == 2 && (5 <= nRow && nRow <= 7))
+                CPPUNIT_ASSERT(!aList.In(ScRange(nCol,nRow,0)));
+            else
+                CPPUNIT_ASSERT(aList.In(ScRange(nCol,nRow,0)));
+        }
+    }
+}
+
+void Test::testDeleteArea_2Ranges_Case4()
+{
+    ScRangeList aList(ScRange(2,3,0,4,7,0));
+    aList.DeleteArea(0,1,0,2,5,0);
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), aList.size());
+
+    // Column 2 Row 3-5 shouldn't be in the list.
+    for (SCCOL nCol = 2; nCol <= 4; ++nCol)
+    {
+        for (SCROW nRow = 3; nRow <= 7; ++nRow)
+        {
+            if (nCol == 2 && (3 <= nRow && nRow <= 5))
+                CPPUNIT_ASSERT(!aList.In(ScRange(nCol,nRow,0)));
+            else
+                CPPUNIT_ASSERT(aList.In(ScRange(nCol,nRow,0)));
+        }
+    }
+}
+
 void Test::testDeleteArea_1Range()
 {
     ScRangeList aList(ScRange(1,1,0,3,3,0));
diff --git a/sc/source/core/tool/rangelst.cxx b/sc/source/core/tool/rangelst.cxx
index b4b8732..96f7de5 100644
--- a/sc/source/core/tool/rangelst.cxx
+++ b/sc/source/core/tool/rangelst.cxx
@@ -517,6 +517,7 @@ bool handleOneRange( const ScRange& rDeleteRange, ScRange* p )
             // +------+ (xxx) = deleted region
 
             p->aStart.SetRow(nDeleteRow1+1);
+            return true;
         }
         else if (nRow2 <= nDeleteRow2)
         {
@@ -527,9 +528,8 @@ bool handleOneRange( const ScRange& rDeleteRange, ScRange* p )
             // +------+ (xxx) = deleted region
 
             p->aEnd.SetRow(nDeleteRow1-1);
+            return true;
         }
-
-        return true;
     }
     else if (checkForOneRange(nDeleteRow1, nDeleteRow2, nDeleteCol1, nDeleteCol2, nRow1, nRow2, nCol1, nCol2))
     {
@@ -543,6 +543,7 @@ bool handleOneRange( const ScRange& rDeleteRange, ScRange* p )
             // +--+--+ (xxx) = deleted region
 
             p->aStart.SetCol(nDeleteCol2+1);
+            return true;
         }
         else if (nCol2 <= nDeleteCol2)
         {
@@ -553,9 +554,8 @@ bool handleOneRange( const ScRange& rDeleteRange, ScRange* p )
             // +--+--+ (xxx) = deleted region
 
             p->aEnd.SetCol(nDeleteCol1-1);
+            return true;
         }
-
-        return true;
     }
     return false;
 }
@@ -600,7 +600,7 @@ bool handleTwoRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScRan
     SCROW nRow2 = aPEnd.Row();
     SCTAB nTab = aPStart.Tab();
 
-    if (nCol1 < nDeleteCol1 && nDeleteCol1 < nCol2 && nCol2 <= nDeleteCol2)
+    if (nCol1 < nDeleteCol1 && nDeleteCol1 <= nCol2 && nCol2 <= nDeleteCol2)
     {
         // column deleted :     |-------|
         // column original: |-------|
@@ -636,32 +636,30 @@ bool handleTwoRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScRan
             // |   2   |    (xxx) deleted region
             // +-------+
 
-            ScRange aNewRange( aPStart, ScAddress( nCol1-1, nRow2, nTab ) ); // 1 (TODO: looks wrong)
+            ScRange aNewRange( aPStart, ScAddress(nDeleteCol1-1, nRow2, nTab) ); // 1
             rNewRanges.push_back(aNewRange);
 
             p->aStart.SetRow(nDeleteRow2+1); // 2
             return true;
         }
     }
-    else if (nCol1 < nDeleteCol2 && nDeleteCol2 < nCol2 && nDeleteCol1 <= nCol1)
+    else if (nCol1 <= nDeleteCol2 && nDeleteCol2 < nCol2 && nDeleteCol1 <= nCol1)
     {
         // column deleted : |-------|
         // column original:     |-------|
-        if (nRow1 < nDeleteRow1 && nDeleteRow1 < nRow2)
+        if (nRow1 < nDeleteRow1 && nDeleteRow1 < nRow2 && nRow2 <= nDeleteRow2)
         {
-            // row deleted:     |------|        |--|
-            // row original: |------|    or  |--------|
+            // row deleted:     |------|
+            // row original: |------|
             //
-            //     +-------+          +-------+
-            //     |   1   |          |       |
-            // +-------+---+      +---+---+   |
-            // |xxxxxxx| 2 |  or  |xxxxxxx|   |
-            // |xxxxxxx+---+      +---+---+   |
-            // |xxxxxxx|              |       |
-            // +-------+              +-------+
+            //     +-------+
+            //     |   1   |
+            // +-------+---+
+            // |xxxxxxx| 2 |
+            // |xxxxxxx+---+
+            // |xxxxxxx|
+            // +-------+
             //  (xxx) deleted region
-            //
-            // TODO: Is this correct especially on the second case?
 
             ScRange aNewRange( ScAddress( nDeleteCol2+1, nDeleteRow1, nTab ), aPEnd ); // 2
             rNewRanges.push_back(aNewRange);
@@ -669,7 +667,7 @@ bool handleTwoRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScRan
             p->aEnd.SetRow(nDeleteRow1-1); // 1
             return true;
         }
-        else if (nRow1 < nDeleteRow2 && nDeleteRow2 < nRow2)
+        else if (nRow1 < nDeleteRow2 && nDeleteRow2 < nRow2 && nDeleteRow1 <= nRow1)
         {
             // row deleted:  |-------|
             // row original:     |--------|
@@ -682,7 +680,7 @@ bool handleTwoRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScRan
             //     |   2   |
             //     +-------+ (xxx) deleted region
 
-            ScRange aNewRange( nDeleteCol2 +1, nRow1, nTab, nDeleteCol2, nDeleteRow2, nTab ); // 1 (TODO: this doesn't look right)
+            ScRange aNewRange(nDeleteCol2+1, nRow1, nTab, nCol2, nDeleteRow2, nTab); // 1
             rNewRanges.push_back(aNewRange);
 
             p->aStart.SetRow(nDeleteRow2+1); // 2
@@ -808,10 +806,10 @@ bool handleThreeRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScR
             //     |  3   |   |
             //     +------+---+
 
-            ScRange aNewRange(aPStart, ScAddress(nDeleteCol2-1, nDeleteRow1-1, nTab)); // 1 (TODO: End column should be nDeleteCol2.)
+            ScRange aNewRange(aPStart, ScAddress(nDeleteCol2, nDeleteRow1-1, nTab)); // 1
             rNewRanges.push_back(aNewRange);
 
-            aNewRange = ScRange(nCol1, nDeleteRow2+1, nTab, nDeleteCol2+1, nRow2, nTab); // 3 (TODO: End column should be nDeleteCol2.)
+            aNewRange = ScRange(nCol1, nDeleteRow2+1, nTab, nDeleteCol2, nRow2, nTab); // 3
             rNewRanges.push_back(aNewRange);
 
             p->aStart.SetCol(nDeleteCol2+1); // 2
@@ -854,7 +852,7 @@ bool handleThreeRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScR
             ScRange aNewRange(aPStart, ScAddress(nDeleteCol1-1, nDeleteRow2, nTab)); // 1
             rNewRanges.push_back(aNewRange);
 
-            aNewRange = ScRange(nDeleteCol2+1, nCol1, nTab, nCol2, nDeleteRow2, nTab); // 2 (TODO: start column should be nDeleteCol2+1.)
+            aNewRange = ScRange(nDeleteCol2+1, nRow1, nTab, nCol2, nDeleteRow2, nTab); // 2
             rNewRanges.push_back( aNewRange );
 
             p->aStart.SetRow(nDeleteRow2+1); // 3
commit 5551cd0209981f71ea5fb252b791391a6427066e
Author: Kohei Yoshida <kohei.yoshida at gmail.com>
Date:   Fri Sep 21 15:12:40 2012 -0400

    Improve readability.
    
    * clarify with comments what the code intendes to do.
    * more consistent variable naming.
    * keep the direction of equality consistent (i.e. lhs always smaller than
      rhs). This helps the reader of the code intuitively see relative geometrical
      positions.
    
    In doing so, I've also marked what appear to be bugs with TODO markers.
    
    The logic of the original code is intentionally unchanged, in order to
    spot any unintended behavior changes introduced by this commit.
    
    Change-Id: I19c09a1f93017ab18c8fec0f1281899322b45053

diff --git a/sc/source/core/tool/rangelst.cxx b/sc/source/core/tool/rangelst.cxx
index 393d0f0..b4b8732 100644
--- a/sc/source/core/tool/rangelst.cxx
+++ b/sc/source/core/tool/rangelst.cxx
@@ -460,16 +460,31 @@ bool ScRangeList::UpdateReference(
 }
 
 namespace {
-    //
-        // r.aStart.X() <= p.aStart.X() && r.aEnd.X() >= p.aEnd.X()
-        // && ( r.aStart.Y() <= p.aStart.Y() || r.aEnd.Y() >= r.aEnd.Y() )
 
+/**
+ * Check if the deleting range cuts the test range exactly into a single
+ * piece.
+ *
+ * X = column ; Y = row
+ * +------+    +------+
+ * |xxxxxx|    |      |
+ * +------+ or +------+
+ * |      |    |xxxxxx|
+ * +------+    +------+
+ *
+ * X = row; Y = column
+ * +--+--+    +--+--+
+ * |xx|  |    |  |xx|
+ * |xx|  | or |  |xx|
+ * |xx|  |    |  |xx|
+ * +--+--+    +--+--+
+ * where xxx is the deleted region.
+ */
 template<typename X, typename Y>
-bool checkForOneRange( X rStartX, X rEndX, Y rStartY, Y rEndY,
-                    X pStartX, X pEndX, Y pStartY, Y pEndY )
+bool checkForOneRange(
+   X nDeleteX1, X nDeleteX2, Y nDeleteY1, Y nDeleteY2, X nX1, X nX2, Y nY1, Y nY2)
 {
-    if( rStartX <= pStartX && rEndX >= pEndX
-            && ( rStartY <= pStartY || rEndY >= pEndY ) )
+    if (nDeleteX1 <= nX1 && nX2 <= nDeleteX2 && (nDeleteY1 <= nY1 || nY2 <= nDeleteY2))
         return true;
 
     return false;
@@ -477,142 +492,271 @@ bool checkForOneRange( X rStartX, X rEndX, Y rStartY, Y rEndY,
 
 bool handleOneRange( const ScRange& rDeleteRange, ScRange* p )
 {
-    ScAddress rDelStart = rDeleteRange.aStart;
-    ScAddress rDelEnd = rDeleteRange.aEnd;
-    ScAddress rPStart = p->aStart;
-    ScAddress rPEnd = p->aEnd;
-    if(checkForOneRange(rDelStart.Col(), rDelEnd.Col(), rDelStart.Row(), rDelEnd.Row(),
-                rPStart.Col(), rPEnd.Col(), rPStart.Row(), rPEnd.Row()))
+    const ScAddress& rDelStart = rDeleteRange.aStart;
+    const ScAddress& rDelEnd = rDeleteRange.aEnd;
+    ScAddress aPStart = p->aStart;
+    ScAddress aPEnd = p->aEnd;
+    SCCOL nDeleteCol1 = rDelStart.Col();
+    SCCOL nDeleteCol2 = rDelEnd.Col();
+    SCROW nDeleteRow1 = rDelStart.Row();
+    SCROW nDeleteRow2 = rDelEnd.Row();
+    SCCOL nCol1 = aPStart.Col();
+    SCCOL nCol2 = aPEnd.Col();
+    SCROW nRow1 = aPStart.Row();
+    SCROW nRow2 = aPEnd.Row();
+
+    if (checkForOneRange(nDeleteCol1, nDeleteCol2, nDeleteRow1, nDeleteRow2, nCol1, nCol2, nRow1, nRow2))
     {
-        // X = Col
-        // Y = Row
-        if(rDelStart.Row() <= rPStart.Row())
+        // Deleting range fully overlaps the column range.  Adjust the row span.
+        if (nDeleteRow1 <= nRow1)
         {
-            p->aStart.SetRow(rDelEnd.Row()+1);
+            // +------+
+            // |xxxxxx|
+            // +------+
+            // |      |
+            // +------+ (xxx) = deleted region
+
+            p->aStart.SetRow(nDeleteRow1+1);
         }
-        else if(rDelEnd.Row() >= rPEnd.Row())
+        else if (nRow2 <= nDeleteRow2)
         {
-            p->aEnd.SetRow(rDelStart.Row()-1);
+            // +------+
+            // |      |
+            // +------+
+            // |xxxxxx|
+            // +------+ (xxx) = deleted region
+
+            p->aEnd.SetRow(nDeleteRow1-1);
         }
 
         return true;
     }
-    else if(checkForOneRange(rDelStart.Row(), rDelEnd.Row(), rDelStart.Col(), rDelEnd.Col(),
-                rPStart.Row(), rPEnd.Row(), rPStart.Col(), rPEnd.Col()))
+    else if (checkForOneRange(nDeleteRow1, nDeleteRow2, nDeleteCol1, nDeleteCol2, nRow1, nRow2, nCol1, nCol2))
     {
-        // X = Row
-        // Y = Col
-        if(rDelStart.Col() <= rPStart.Col())
-            p->aStart.SetCol(rDelEnd.Col()+1);
-        else if(rDelEnd.Col() >= rPEnd.Col())
-            p->aEnd.SetCol(rDelStart.Col()-1);
+        // Deleting range fully overlaps the row range.  Adjust the column span.
+        if (nDeleteCol1 <= nCol1)
+        {
+            // +--+--+
+            // |xx|  |
+            // |xx|  |
+            // |xx|  |
+            // +--+--+ (xxx) = deleted region
+
+            p->aStart.SetCol(nDeleteCol2+1);
+        }
+        else if (nCol2 <= nDeleteCol2)
+        {
+            // +--+--+
+            // |  |xx|
+            // |  |xx|
+            // |  |xx|
+            // +--+--+ (xxx) = deleted region
+
+            p->aEnd.SetCol(nDeleteCol1-1);
+        }
 
         return true;
     }
     return false;
 }
 
+/**
+ * Check if the deleting range cuts the test range in the middle, to
+ * separate it into exactly two pieces.
+ *
+ * Either
+ * +--------+    +--+-+--+
+ * |        |    |  |x|  |
+ * +--------+    |  |x|  |
+ * |xxxxxxxx| or |  |x|  |
+ * +--------+    |  |x|  |
+ * |        |    |  |x|  |
+ * +--------+    +--+-+--+
+ * where xxx is the deleted region.
+ */
 template<typename X, typename Y>
-bool checkForTwoRangesCase2( X rStartX, X rEndX, Y rStartY, Y rEndY,
-                    X pStartX, X pEndX, Y pStartY, Y pEndY )
+bool checkForTwoRangesCase2(
+   X nDeleteX1, X nDeleteX2, Y nDeleteY1, Y nDeleteY2, X nX1, X nX2, Y nY1, Y nY2)
 {
-    if(rStartY > pStartY && rStartX <= pStartX
-            && rEndY < pEndY && rEndX >= pEndX)
+    if (nY1 < nDeleteY1 && nDeleteY2 < nY2 && nDeleteX1 <= nX1 && nX2 <= nDeleteX2)
         return true;
 
     return false;
 }
 
-
 bool handleTwoRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScRange>& rNewRanges )
 {
-    ScAddress rDelStart = rDeleteRange.aStart;
-    ScAddress rDelEnd = rDeleteRange.aEnd;
-    ScAddress rPStart = p->aStart;
-    ScAddress rPEnd = p->aEnd;
-    SCCOL rStartCol = rDelStart.Col();
-    SCCOL rEndCol = rDelEnd.Col();
-    SCCOL pStartCol = rPStart.Col();
-    SCCOL pEndCol = rPEnd.Col();
-    SCROW rStartRow = rDelStart.Row();
-    SCROW rEndRow = rDelEnd.Row();
-    SCROW pStartRow = rPStart.Row();
-    SCROW pEndRow = rPEnd.Row();
-    SCTAB nTab = rPStart.Tab();
-    if(rStartCol > pStartCol && rStartCol < pEndCol && rEndCol >= pEndCol)
+    const ScAddress& rDelStart = rDeleteRange.aStart;
+    const ScAddress& rDelEnd = rDeleteRange.aEnd;
+    ScAddress aPStart = p->aStart;
+    ScAddress aPEnd = p->aEnd;
+    SCCOL nDeleteCol1 = rDelStart.Col();
+    SCCOL nDeleteCol2 = rDelEnd.Col();
+    SCROW nDeleteRow1 = rDelStart.Row();
+    SCROW nDeleteRow2 = rDelEnd.Row();
+    SCCOL nCol1 = aPStart.Col();
+    SCCOL nCol2 = aPEnd.Col();
+    SCROW nRow1 = aPStart.Row();
+    SCROW nRow2 = aPEnd.Row();
+    SCTAB nTab = aPStart.Tab();
+
+    if (nCol1 < nDeleteCol1 && nDeleteCol1 < nCol2 && nCol2 <= nDeleteCol2)
     {
-        if(rStartRow > pStartRow && rStartRow < pEndRow && rEndRow >= pEndRow)
+        // column deleted :     |-------|
+        // column original: |-------|
+        if (nRow1 < nDeleteRow1 && nDeleteRow1 < nRow2 && nRow2 <= nDeleteRow2)
         {
-            ScRange aNewRange( pStartCol, rStartRow, nTab, rStartCol-1, pEndRow, nTab );
+            // row deleted:     |------|
+            // row original: |------|
+            //
+            // +-------+
+            // |   1   |
+            // +---+---+---+
+            // | 2 |xxxxxxx|
+            // +---+xxxxxxx|
+            //     |xxxxxxx|
+            //     +-------+ (xxx) deleted region
+
+            ScRange aNewRange( nCol1, nDeleteRow1, nTab, nDeleteCol1-1, nRow2, nTab ); // 2
             rNewRanges.push_back(aNewRange);
 
-            p->aEnd.SetRow(rStartRow -1);
+            p->aEnd.SetRow(nDeleteRow1-1); // 1
             return true;
         }
-        else if(rEndRow > pStartRow && rEndRow < pEndRow && rStartRow <= pStartRow)
+        else if (nRow1 < nDeleteRow2 && nDeleteRow2 < nRow2 && nDeleteRow1 <= nRow1)
         {
-            ScRange aNewRange( rPStart, ScAddress( pStartCol -1, pEndRow, nTab ) );
+            // row deleted:  |------|
+            // row original:    |------|
+            //
+            //     +-------+
+            //     |xxxxxxx|
+            // +---+xxxxxxx|
+            // | 1 |xxxxxxx|
+            // +---+---+---+
+            // |   2   |    (xxx) deleted region
+            // +-------+
+
+            ScRange aNewRange( aPStart, ScAddress( nCol1-1, nRow2, nTab ) ); // 1 (TODO: looks wrong)
             rNewRanges.push_back(aNewRange);
 
-            p->aStart.SetRow(rEndRow+1);
+            p->aStart.SetRow(nDeleteRow2+1); // 2
             return true;
         }
     }
-    else if(rEndCol > pStartCol && rEndCol < pEndCol && rStartCol <= pStartCol)
+    else if (nCol1 < nDeleteCol2 && nDeleteCol2 < nCol2 && nDeleteCol1 <= nCol1)
     {
-        if(rStartRow > pStartRow && rStartRow < pEndRow)
+        // column deleted : |-------|
+        // column original:     |-------|
+        if (nRow1 < nDeleteRow1 && nDeleteRow1 < nRow2)
         {
-            ScRange aNewRange( ScAddress( rEndCol +1, rStartRow, nTab ), rPEnd );
+            // row deleted:     |------|        |--|
+            // row original: |------|    or  |--------|
+            //
+            //     +-------+          +-------+
+            //     |   1   |          |       |
+            // +-------+---+      +---+---+   |
+            // |xxxxxxx| 2 |  or  |xxxxxxx|   |
+            // |xxxxxxx+---+      +---+---+   |
+            // |xxxxxxx|              |       |
+            // +-------+              +-------+
+            //  (xxx) deleted region
+            //
+            // TODO: Is this correct especially on the second case?
+
+            ScRange aNewRange( ScAddress( nDeleteCol2+1, nDeleteRow1, nTab ), aPEnd ); // 2
             rNewRanges.push_back(aNewRange);
 
-            p->aEnd.SetRow(rStartRow-1);
+            p->aEnd.SetRow(nDeleteRow1-1); // 1
             return true;
         }
-        else if(rEndRow > pStartRow && rEndRow < pEndRow)
+        else if (nRow1 < nDeleteRow2 && nDeleteRow2 < nRow2)
         {
-            ScRange aNewRange( rEndCol +1, pStartRow, nTab, rEndCol, rEndRow, nTab );
+            // row deleted:  |-------|
+            // row original:     |--------|
+            //
+            // +-------+
+            // |xxxxxxx|
+            // |xxxxxxx+---+
+            // |xxxxxxx| 1 |
+            // +-------+---+
+            //     |   2   |
+            //     +-------+ (xxx) deleted region
+
+            ScRange aNewRange( nDeleteCol2 +1, nRow1, nTab, nDeleteCol2, nDeleteRow2, nTab ); // 1 (TODO: this doesn't look right)
             rNewRanges.push_back(aNewRange);
 
-            p->aStart.SetRow(rEndRow+1);
+            p->aStart.SetRow(nDeleteRow2+1); // 2
             return true;
         }
     }
-    else if(checkForTwoRangesCase2(rDelStart.Col(), rDelEnd.Col(), rDelStart.Row(), rDelEnd.Row(),
-                rPStart.Col(), rPEnd.Col(), rPStart.Row(), rPEnd.Row()))
+    else if (nRow1 < nDeleteRow1 && nDeleteRow2 < nRow2 && nDeleteCol1 <= nCol1 && nCol2 <= nDeleteCol2)
     {
-        ScRange aNewRange( rPStart, ScAddress( rPEnd.Col(), rDelStart.Row() -1, nTab ) );
+        // +--------+
+        // |   1    |
+        // +--------+
+        // |xxxxxxxx| (xxx) deleted region
+        // +--------+
+        // |   2    |
+        // +--------+
+
+        ScRange aNewRange( aPStart, ScAddress(nCol2, nDeleteRow1-1, nTab) ); // 1
         rNewRanges.push_back(aNewRange);
 
-        p->aStart.SetRow(rEndRow+1);
+        p->aStart.SetRow(nDeleteRow2+1); // 2
         return true;
     }
-    else if(checkForTwoRangesCase2(rDelStart.Row(), rDelEnd.Row(), rDelStart.Col(), rDelEnd.Col(),
-                rPStart.Row(), rPEnd.Row(), rPStart.Col(), rPEnd.Col()))
+    else if (nCol1 < nDeleteCol1 && nDeleteCol2 < nCol2 && nDeleteRow1 <= nRow1 && nRow2 <= nDeleteRow2)
     {
-        ScRange aNewRange( rPStart, ScAddress( rDelStart.Col() -1, rPEnd.Row(), nTab ) );
+        // +---+-+---+
+        // |   |x|   |
+        // |   |x|   |
+        // | 1 |x| 2 | (xxx) deleted region
+        // |   |x|   |
+        // |   |x|   |
+        // +---+-+---+
+
+        ScRange aNewRange( aPStart, ScAddress(nDeleteCol1-1, nRow2, nTab) ); // 1
         rNewRanges.push_back(aNewRange);
 
-        p->aStart.SetCol(rEndCol+1);
+        p->aStart.SetCol(nDeleteCol2+1); // 2
         return true;
     }
 
     return false;
 }
 
-        // r.aStart.X() > p.aStart.X() && r.aEnd.X() >= p.aEnd.X()
-        // && r.aStart.Y() > p.aStart.Y() && r.aEnd.Y() < p.aEnd.Y()
-        // or
-        // r.aStart.X() <= p.aStart.X() && r.aEnd.X() < p.aEnd.X()
-        // && r.aStart.Y() > p.aStart.Y() && r.aEnd.Y() < p.aEnd.Y()
+/**
+ * Check if any of the followings applies:
+ *
+ * X = column; Y = row
+ * +----------+           +----------+
+ * |          |           |          |
+ * |  +-------+---+    +--+-------+  |
+ * |  |xxxxxxxxxxx| or |xxxxxxxxxx|  |
+ * |  +-------+---+    +--+-------+  |
+ * |          |           |          |
+ * +----------+           +----------+
+ *
+ * X = row; Y = column
+ *     +--+
+ *     |xx|
+ * +---+xx+---+    +----------+
+ * |   |xx|   |    |          |
+ * |   |xx|   | or |   +--+   |
+ * |   +--+   |    |   |xx|   |
+ * |          |    |   |xx|   |
+ * +----------+    +---+xx+---+
+ *                     |xx|
+ *                     +--+     (xxx) deleted region
+ */
 template<typename X, typename Y>
-bool checkForThreeRanges( X rStartX, X rEndX, Y rStartY, Y rEndY,
-                                X pStartX, X pEndX, Y pStartY, Y pEndY )
+bool checkForThreeRanges(
+   X nDeleteX1, X nDeleteX2, Y nDeleteY1, Y nDeleteY2, X nX1, X nX2, Y nY1, Y nY2)
 {
-    if(rStartX > pStartX && rEndX >= pEndX
-            && rStartY > pStartY && rEndY < pEndY )
+    if (nX1 < nDeleteX1 && nX2 <= nDeleteX2 && nY1 < nDeleteY1 && nDeleteY2 < nY2)
         return true;
-    else if( rStartX <= pStartX && rEndX < pEndX
-            && rStartY > pStartY && rEndY < pEndY )
+
+    if (nDeleteX1 <= nX1 && nDeleteX2 < nX2 && nY1 < nDeleteY1 && nDeleteY2 < nY2)
         return true;
 
     return false;
@@ -620,66 +764,100 @@ bool checkForThreeRanges( X rStartX, X rEndX, Y rStartY, Y rEndY,
 
 bool handleThreeRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScRange>& rNewRanges )
 {
-    ScAddress rDelStart = rDeleteRange.aStart;
-    ScAddress rDelEnd = rDeleteRange.aEnd;
-    ScAddress rPStart = p->aStart;
-    ScAddress rPEnd = p->aEnd;
-    SCTAB nTab = rDelStart.Tab();
-    if(checkForThreeRanges(rDelStart.Col(), rDelEnd.Col(), rDelStart.Row(), rDelEnd.Row(),
-                rPStart.Col(), rPEnd.Col(), rPStart.Row(), rPEnd.Row()))
+    const ScAddress& rDelStart = rDeleteRange.aStart;
+    const ScAddress& rDelEnd = rDeleteRange.aEnd;
+    ScAddress aPStart = p->aStart;
+    ScAddress aPEnd = p->aEnd;
+    SCCOL nDeleteCol1 = rDelStart.Col();
+    SCCOL nDeleteCol2 = rDelEnd.Col();
+    SCROW nDeleteRow1 = rDelStart.Row();
+    SCROW nDeleteRow2 = rDelEnd.Row();
+    SCCOL nCol1 = aPStart.Col();
+    SCCOL nCol2 = aPEnd.Col();
+    SCROW nRow1 = aPStart.Row();
+    SCROW nRow2 = aPEnd.Row();
+    SCTAB nTab = aPStart.Tab();
+
+    if (checkForThreeRanges(nDeleteCol1, nDeleteCol2, nDeleteRow1, nDeleteRow2, nCol1, nCol2, nRow1, nRow2))
     {
-        if(rDelStart.Col() > rPStart.Col())
+        if (nCol1 < nDeleteCol1)
         {
-            SCCOL nCol1 = rDelStart.Col();
-
-            ScRange aNewRange( nCol1, rPStart.Row(), nTab, rPEnd.Col(), rDelStart.Row()-1, nTab);
+            // +---+------+
+            // |   |  2   |
+            // |   +------+---+
+            // | 1 |xxxxxxxxxx|
+            // |   +------+---+
+            // |   |  3   |
+            // +---+------+
+
+            ScRange aNewRange(nDeleteCol1, nRow1, nTab, nCol2, nDeleteRow1-1, nTab); // 2
             rNewRanges.push_back(aNewRange);
 
-            aNewRange = ScRange( ScAddress(nCol1, rDelEnd.Row()+1, nTab), rPEnd);
+            aNewRange = ScRange(ScAddress(nDeleteCol1, nDeleteRow2+1, nTab), aPEnd); // 3
             rNewRanges.push_back(aNewRange);
 
-            p->aEnd.SetCol(nCol1-1);
+            p->aEnd.SetCol(nDeleteCol1-1); // 1
         }
         else
         {
-            SCCOL nCol1 = rDelEnd.Col();
-
-            ScRange aNewRange( rPStart, ScAddress( nCol1 - 1, rDelStart.Row() -1, nTab ) );
+            //     +------+---+
+            //     |  1   |   |
+            // +---+------+   |
+            // |xxxxxxxxxx| 2 |
+            // +---+------+   |
+            //     |  3   |   |
+            //     +------+---+
+
+            ScRange aNewRange(aPStart, ScAddress(nDeleteCol2-1, nDeleteRow1-1, nTab)); // 1 (TODO: End column should be nDeleteCol2.)
             rNewRanges.push_back(aNewRange);
 
-            aNewRange = ScRange( rPStart.Col(), rDelEnd.Row() + 1, nTab, rDelEnd.Col() +1, rPEnd.Row(), nTab );
+            aNewRange = ScRange(nCol1, nDeleteRow2+1, nTab, nDeleteCol2+1, nRow2, nTab); // 3 (TODO: End column should be nDeleteCol2.)
             rNewRanges.push_back(aNewRange);
 
-            p->aStart.SetCol(nCol1+1);
+            p->aStart.SetCol(nDeleteCol2+1); // 2
         }
         return true;
     }
-    else if(checkForThreeRanges(rDelStart.Row(), rDelEnd.Row(), rDelStart.Col(), rDelEnd.Col(),
-                rPStart.Row(), rPEnd.Row(), rPStart.Col(), rPEnd.Col()))
+    else if (checkForThreeRanges(nDeleteRow1, nDeleteRow2, nDeleteCol1, nDeleteCol2, nRow1, nRow2, nCol1, nCol2))
     {
-        if(rDelStart.Row() > rPStart.Row())
+        if (nRow1 < nDeleteRow1)
         {
-            SCROW nRow1 = rDelStart.Row();
-
-            ScRange aNewRange( rPStart.Col(), nRow1, nTab, rDelStart.Col() -1, rPEnd.Row(), nTab );
+            // +----------+
+            // |    1     |
+            // +---+--+---+
+            // |   |xx|   |
+            // | 2 |xx| 3 |
+            // |   |xx|   |
+            // +---+xx+---+
+            //     |xx|
+            //     +--+
+
+            ScRange aNewRange(nCol1, nDeleteRow1, nTab, nDeleteCol1-1, nRow2, nTab); // 2
             rNewRanges.push_back( aNewRange );
 
-            aNewRange = ScRange( ScAddress(rDelEnd.Col() +1, nRow1, nTab), rPEnd );
+            aNewRange = ScRange(ScAddress(nDeleteCol2+1, nDeleteRow1, nTab), aPEnd); // 3
             rNewRanges.push_back( aNewRange );
 
-            p->aEnd.SetRow(nRow1-1);
+            p->aEnd.SetRow(nDeleteRow1-1); // 1
         }
         else
         {
-            SCROW nRow1 = rDelEnd.Row();
-
-            ScRange aNewRange( rPStart, ScAddress( rDelStart.Col() -1, nRow1, nTab ) );
+            //     +--+
+            //     |xx|
+            // +---+xx+---+
+            // | 1 |xx| 2 |
+            // |   |xx|   |
+            // +---+--+---+
+            // |    3     |
+            // +----------+
+
+            ScRange aNewRange(aPStart, ScAddress(nDeleteCol1-1, nDeleteRow2, nTab)); // 1
             rNewRanges.push_back(aNewRange);
 
-            aNewRange = ScRange( rDelEnd.Col() +1, rPStart.Col(), nTab, rPEnd.Col(), nRow1, nTab );
+            aNewRange = ScRange(nDeleteCol2+1, nCol1, nTab, nCol2, nDeleteRow2, nTab); // 2 (TODO: start column should be nDeleteCol2+1.)
             rNewRanges.push_back( aNewRange );
 
-            p->aStart.SetRow(nRow1+1);
+            p->aStart.SetRow(nDeleteRow2+1); // 3
         }
         return true;
     }
@@ -689,26 +867,42 @@ bool handleThreeRanges( const ScRange& rDeleteRange, ScRange* p, std::vector<ScR
 
 bool handleFourRanges( const ScRange& rDelRange, ScRange* p, std::vector<ScRange>& rNewRanges )
 {
-    ScAddress rDelStart = rDelRange.aStart;
-    ScAddress rDelEnd = rDelRange.aEnd;
-    ScAddress rPStart = p->aStart;
-    ScAddress rPEnd = p->aEnd;
-    if( rDelRange.aStart.Col() > p->aStart.Col() && rDelRange.aEnd.Col() < p->aEnd.Col()
-            && rDelRange.aStart.Row() > p->aStart.Row() && rDelRange.aEnd.Row() < p->aEnd.Row() )
+    const ScAddress& rDelStart = rDelRange.aStart;
+    const ScAddress& rDelEnd = rDelRange.aEnd;
+    ScAddress aPStart = p->aStart;
+    ScAddress aPEnd = p->aEnd;
+    SCCOL nDeleteCol1 = rDelStart.Col();
+    SCCOL nDeleteCol2 = rDelEnd.Col();
+    SCROW nDeleteRow1 = rDelStart.Row();
+    SCROW nDeleteRow2 = rDelEnd.Row();
+    SCCOL nCol1 = aPStart.Col();
+    SCCOL nCol2 = aPEnd.Col();
+    SCROW nRow1 = aPStart.Row();
+    SCROW nRow2 = aPEnd.Row();
+    SCTAB nTab = aPStart.Tab();
+
+    if (nCol1 < nDeleteCol1 && nDeleteCol2 < nCol2 && nRow1 < nDeleteRow1 && nDeleteRow2 < nRow2)
     {
-        SCTAB nTab = rDelStart.Tab();
-
-        ScRange aNewRange( ScAddress( rPStart.Col(), rDelEnd.Row()+1, nTab ), rPEnd );
+        // +---------------+
+        // |       1       |
+        // +---+-------+---+
+        // |   |xxxxxxx|   |
+        // | 2 |xxxxxxx| 3 |
+        // |   |xxxxxxx|   |
+        // +---+-------+---+
+        // |       4       |
+        // +---------------+
+
+        ScRange aNewRange(ScAddress(nCol1, nDeleteRow2+1, nTab), aPEnd); // 4
         rNewRanges.push_back( aNewRange );
 
-        aNewRange = ScRange( rPStart.Col(), rDelStart.Row(), nTab, rDelStart.Col() -1, rDelEnd.Row(), nTab );
+        aNewRange = ScRange(nCol1, nDeleteRow1, nTab, nDeleteCol1-1, nDeleteRow2, nTab); // 2
         rNewRanges.push_back( aNewRange );
 
-        aNewRange = ScRange( rDelEnd.Col() +1, rDelStart.Row(), nTab, rPEnd.Col(), rDelEnd.Row(), nTab );
+        aNewRange = ScRange(nDeleteCol2+1, nDeleteRow1, nTab, nCol2, nDeleteRow2, nTab); // 3
         rNewRanges.push_back( aNewRange );
 
-        rPEnd.SetRow(rDelStart.Row()-1);
-        p->aEnd = rPEnd;
+        p->aEnd.SetRow(nDeleteRow1-1); // 1
 
         return true;
     }


More information about the Libreoffice-commits mailing list