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
81286906d0b76a3b6c4443378877828290c3e5f0 "docx import fixes
for: redlines".

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

Change-Id: If81631bde49ee52a249f5ba1dd64ab8e92fffaf7

6 files changed, 28 insertions, 15 deletions

diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx
index ac5d017eda4c..3f42f32d98ea 100644
--- a/sw/inc/IDocumentRedlineAccess.hxx
+++ b/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;
diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx
index 1a62a0b04c64..1d9a24be3153 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -1,4 +1,3 @@
-
 /* -*- 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 )
diff --git a/sw/source/core/doc/doccomp.cxx b/sw/source/core/doc/doccomp.cxx
index 6dc6fb18db2d..1acbe7a9f306 100644
--- a/sw/source/core/doc/doccomp.cxx
+++ b/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;
diff --git a/sw/source/core/inc/DocumentRedlineManager.hxx b/sw/source/core/inc/DocumentRedlineManager.hxx
index eeea5f23ace9..9ba7e5bd7f1d 100644
--- a/sw/source/core/inc/DocumentRedlineManager.hxx
+++ b/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;
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index d974d093dcef..0f62c97737cb 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/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 );
 }
 
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx b/sw/source/core/unocore/unocrsrhelper.cxx
index 02df4ff1e85b..4b1925eb70cb 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/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();
 }