tdf#109267 sw: fix confusing return value of AppendRedline()

AppendRedline() has a boolen return value which is rather
unclear and confusing: most callers don't even check it, but
SaveMergeRedline::InsertRedline() expects "true" to mean that
its redline hasn't been deleted, whereas makeRedline()
expects "true" to mean that the redline was somehow "valid",
even if it has been deleted and merged with an existing one.

The "bMerged" variable in AppendRedline(), which is the source
of the confusion, was introduced with commit
81286906 "docx import fixes
for: redlines".

Split these differing expectations into different return
values of a new enum type.

Change-Id: If81631bde49ee52a249f5ba1dd64ab8e92fffaf7

sw/inc/IDocumentRedlineAccess.hxx

@@ -159,15 +159,23 @@ public:
 
     virtual bool IsInRedlines(const SwNode& rNode) const = 0;
 
+    enum class AppendResult { IGNORED, MERGED, APPENDED };
     /** Append a new redline
 
-        @param pPtr
+        @param pNewRedl redline to insert
 
         @param bCallDelete
+            if set, then for a new DELETE redline that is inserted so that it
+            overlaps an existing INSERT redline with the same author, the
+            overlapping range is deleted, i.e. the new DELETE removes
+            existing INSERT for that range
 
         @returns
+            APPENDED if pNewRedl is still alive and was appended
+            MERGED if pNewRedl was deleted but has been merged with existing one
+            IGNORED if pNewRedl was deleted and ignored/invalid
     */
-    virtual bool AppendRedline(/*[in]*/SwRangeRedline* pPtr, /*[in]*/bool bCallDelete) = 0;
+    virtual AppendResult AppendRedline(/*[in]*/SwRangeRedline* pNewRedl, /*[in]*/bool bCallDelete) = 0;
 
     virtual bool AppendTableRowRedline(/*[in]*/SwTableRowRedline* pPtr, /*[in]*/bool bCallDelete) = 0;
     virtual bool AppendTableCellRedline(/*[in]*/SwTableCellRedline* pPtr, /*[in]*/bool bCallDelete) = 0;

sw/source/core/doc/DocumentRedlineManager.cxx

-
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /*
  * This file is part of the LibreOffice project.
@@ -730,7 +729,8 @@ Behaviour of Delete-Redline:
                                           other Insert is overlapped by
                                           the Delete
 */
-bool DocumentRedlineManager::AppendRedline( SwRangeRedline* pNewRedl, bool bCallDelete )
+IDocumentRedlineAccess::AppendResult
+DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const bCallDelete)
 {
     bool bMerged = false;
     CHECK_REDLINE( *this )
@@ -792,7 +792,7 @@ bool DocumentRedlineManager::AppendRedline( SwRangeRedline* pNewRedl, bool bCall
             ( pNewRedl->GetContentIdx() == nullptr ) )
         {   // Do not insert empty redlines
             delete pNewRedl;
-            return false;
+            return AppendResult::IGNORED;
         }
         bool bCompress = false;
         SwRedlineTable::size_type n = 0;
@@ -1706,7 +1706,9 @@ bool DocumentRedlineManager::AppendRedline( SwRangeRedline* pNewRedl, bool bCall
     }
     CHECK_REDLINE( *this )
 
-    return ( nullptr != pNewRedl ) || bMerged;
+    return (nullptr != pNewRedl)
+        ? AppendResult::APPENDED
+        : ((bMerged) ? AppendResult::MERGED : AppendResult::IGNORED);
 }
 
 bool DocumentRedlineManager::AppendTableRowRedline( SwTableRowRedline* pNewRedl, bool )

sw/source/core/doc/doccomp.cxx

@@ -1751,7 +1751,9 @@ void CompareData::SetRedlinesToDoc( bool bUseDocInfo )
         }
 
         do {
-            if( rDoc.getIDocumentRedlineAccess().AppendRedline( new SwRangeRedline( aRedlnData, *pTmp ), true) &&
+            if (IDocumentRedlineAccess::AppendResult::APPENDED ==
+                    rDoc.getIDocumentRedlineAccess().AppendRedline(
+                        new SwRangeRedline(aRedlnData, *pTmp), true) &&
                 rDoc.GetIDocumentUndoRedo().DoesUndo())
             {
                 SwUndo *const pUndo(new SwUndoCompDoc( *pTmp, true ));
@@ -2034,7 +2036,8 @@ sal_uInt16 SaveMergeRedline::InsertRedline(SwPaM* pLastDestRedline)
             ? new SwUndoCompDoc( *pDestRedl ) : nullptr;
 
         // now modify doc: append redline, undo (and count)
-        bool bRedlineAccepted = pDoc->getIDocumentRedlineAccess().AppendRedline( pDestRedl, true );
+        IDocumentRedlineAccess::AppendResult const result(
+            pDoc->getIDocumentRedlineAccess().AppendRedline(pDestRedl, true));
         if( pUndo )
         {
             pDoc->GetIDocumentUndoRedo().AppendUndo( pUndo );
@@ -2043,7 +2046,7 @@ sal_uInt16 SaveMergeRedline::InsertRedline(SwPaM* pLastDestRedline)
 
         // if AppendRedline has deleted our redline, we may not keep a
         // reference to it
-        if( ! bRedlineAccepted )
+        if (IDocumentRedlineAccess::AppendResult::APPENDED != result)
             pDestRedl = nullptr;
     }
     return nIns;

sw/source/core/inc/DocumentRedlineManager.hxx

@@ -51,7 +51,7 @@ public:
 
     virtual bool IsInRedlines(const SwNode& rNode) const override;
 
-    virtual bool AppendRedline(/*[in]*/SwRangeRedline* pPtr, /*[in]*/bool bCallDelete) override;
+    virtual AppendResult AppendRedline(/*[in]*/SwRangeRedline* pPtr, /*[in]*/bool bCallDelete) override;
 
     virtual bool AppendTableRowRedline(/*[in]*/SwTableRowRedline* pPtr, /*[in]*/bool bCallDelete) override;
     virtual bool AppendTableCellRedline(/*[in]*/SwTableCellRedline* pPtr, /*[in]*/bool bCallDelete) override;

sw/source/core/undo/undobj.cxx

@@ -1372,9 +1372,9 @@ void SwRedlineSaveData::RedlineToDoc( SwPaM const & rPam )
     if (rDoc.GetDocShell() && (!pRedl->GetComment().isEmpty()) )
         rDoc.GetDocShell()->Broadcast(SwRedlineHint());
 
-    bool const bSuccess = rDoc.getIDocumentRedlineAccess().AppendRedline( pRedl, true );
-    assert(bSuccess); // SwRedlineSaveData::RedlineToDoc: insert redline failed
-    (void) bSuccess; // unused in non-debug
+    auto const result(rDoc.getIDocumentRedlineAccess().AppendRedline(pRedl, true));
+    assert(result != IDocumentRedlineAccess::AppendResult::IGNORED); // SwRedlineSaveData::RedlineToDoc: insert redline failed
+    (void) result; // unused in non-debug
     rDoc.getIDocumentRedlineAccess().SetRedlineFlags_intern( eOld );
 }
 

sw/source/core/unocore/unocrsrhelper.cxx

@@ -1267,9 +1267,9 @@ void makeRedline( SwPaM const & rPaM,
     pRedline->SetExtraData( pRedlineExtraData );
 
     pRedlineAccess->SetRedlineFlags_intern(RedlineFlags::On);
-    bool bRet = pRedlineAccess->AppendRedline( pRedline, false );
+    auto const result(pRedlineAccess->AppendRedline(pRedline, false));
     pRedlineAccess->SetRedlineFlags_intern( nPrevMode );
-    if( !bRet )
+    if (IDocumentRedlineAccess::AppendResult::IGNORED == result)
         throw lang::IllegalArgumentException();
 }