[Libreoffice-commits] core.git: sw/source

[Bjoern Michaelsen]

 sw/source/core/unocore/unotext.cxx |   35 +++++++++++------------------------
 1 file changed, 11 insertions(+), 24 deletions(-)

New commits:
commit 7320a375143a9ecda0515f38b35cba38679d60ea
Author: Bjoern Michaelsen <bjoern.michaelsen at libreoffice.org>
Date:   Wed Sep 20 01:34:09 2017 +0200

    remove confused error-handling
    
    - tracking the error state in a bool
    - which gets pushed by reference to subfunctions
    - and signals back error state from a bazzilion different location
    - in a long operation that is not grouped into one undo
    - and then haphazardly triggers an indiscriminate Undo
    - and in addition throws an excpetion
    
    More likely than not leaves the client with an undefined half-reverted
    state, which an impossible situation to properly recover. This either
    has to be done properly or not at all.
    
    Change-Id: I677f87e78c32f4d93c8d21e9df7130a2595da891
    Reviewed-on: https://gerrit.libreoffice.org/42510
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Björn Michaelsen <bjoern.michaelsen at libreoffice.org>

diff --git a/sw/source/core/unocore/unotext.cxx b/sw/source/core/unocore/unotext.cxx
index 3298d00c7eea..646a802b61de 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -114,8 +114,7 @@ public:
     void ConvertCell(
             const uno::Sequence< uno::Reference< text::XTextRange > > & rCell,
             std::vector<SwNodeRange> & rRowNodes,
-            SwNodeRange *const pLastCell,
-            bool & rbExcept);
+            SwNodeRange *const pLastCell);
 
 };
 
@@ -1781,8 +1780,7 @@ static bool lcl_SimilarPosition( const sal_Int32 nPos1, const sal_Int32 nPos2 )
 void SwXText::Impl::ConvertCell(
     const uno::Sequence< uno::Reference< text::XTextRange > > & rCell,
     std::vector<SwNodeRange> & rRowNodes,
-    SwNodeRange *const pLastCell,
-    bool & rbExcept)
+    SwNodeRange *const pLastCell)
 {
     if (rCell.getLength() != 2)
     {
@@ -1848,15 +1846,13 @@ void SwXText::Impl::ConvertCell(
             }
             if (nOpenNodeBlock < 0)
             {
-                rbExcept = true;
-                break;
+                throw lang::IllegalArgumentException();
             }
             ++aCellIndex;
         }
         if (nOpenNodeBlock != 0)
         {
-            rbExcept = true;
-            return;
+            throw lang::IllegalArgumentException();
         }
     }
 
@@ -1885,7 +1881,7 @@ void SwXText::Impl::ConvertCell(
             // same node as predecessor then equal nContent?
             if (0 != aStartCellPam.Start()->nContent.GetIndex())
             {
-                rbExcept = true;
+                throw lang::IllegalArgumentException();
             }
             else
             {
@@ -1916,7 +1912,7 @@ void SwXText::Impl::ConvertCell(
         }
         else
         {
-            rbExcept = true;
+            throw lang::IllegalArgumentException();
         }
     }
     // now check if there's a need to insert another paragraph break
@@ -2113,9 +2109,7 @@ SwXText::convertToTable(
     const uno::Sequence< uno::Sequence< uno::Reference< text::XTextRange > > >*
         pTableRanges = rTableRanges.getConstArray();
     std::vector< std::vector<SwNodeRange> > aTableNodes;
-    bool bExcept = false;
-    for (sal_Int32 nRow = 0; !bExcept && (nRow < rTableRanges.getLength());
-            ++nRow)
+    for (sal_Int32 nRow = 0; nRow < rTableRanges.getLength(); ++nRow)
     {
         std::vector<SwNodeRange> aRowNodes;
         const uno::Sequence< uno::Reference< text::XTextRange > >* pRow =
@@ -2124,11 +2118,10 @@ SwXText::convertToTable(
 
         if (0 == nCells) // this would lead to no pLastCell below
         {                // and make it impossible to detect node gaps
-            bExcept = true;
-            break;
+            throw lang::IllegalArgumentException();
         }
 
-        for (sal_Int32 nCell = 0; !bExcept && nCell < nCells; ++nCell)
+        for (sal_Int32 nCell = 0; nCell < nCells; ++nCell)
         {
             SwNodeRange *const pLastCell(
                 (nCell == 0)
@@ -2136,18 +2129,12 @@ SwXText::convertToTable(
                         ? nullptr
                         : &*aTableNodes.rbegin()->rbegin())
                     : &*aRowNodes.rbegin());
-            m_pImpl->ConvertCell(pRow[nCell], aRowNodes, pLastCell, bExcept);
+            m_pImpl->ConvertCell(pRow[nCell], aRowNodes, pLastCell);
         }
-        assert(bExcept || !aRowNodes.empty());
+        assert(!aRowNodes.empty());
         aTableNodes.push_back(aRowNodes);
     }
 
-    if(bExcept)
-    {
-        m_pImpl->m_pDoc->GetIDocumentUndoRedo().Undo();
-        throw lang::IllegalArgumentException();
-    }
-
     std::vector< TableColumnSeparators >
         aRowSeparators(rRowProperties.getLength());
     std::vector<VerticallyMergedCell> aMergedCells;