Resolves: tdf#92117 create only one Undo for all UnmergeCells() calls

... during DeleteCells() and InsertCells(), instead of one Undo per
UnmergeCells() call. And actually create Undo only if bRecord requested.

(cherry picked from commit 647e860435c781fbb111ae59bc70dc8e6776fed5)

 Conflicts:
	sc/source/ui/docshell/docfunc.cxx
	sc/source/ui/inc/docfunc.hxx
	sc/source/ui/inc/undoblk.hxx
	sc/source/ui/undo/undoblk.cxx

init ScUndoRemoveMerge with range, tdf#92117 follow-up

So the original selection is restored after Undo.

(cherry picked from commit 0ebe9fab18e732468d2b9d53dddf9f266411a0e5)

e549a0b62da469ee38270ae089ea5abf9a6868e3

Change-Id: I4f1747c3f42f36e16be81f989f0af5d048ba9d9f
Reviewed-on: https://gerrit.libreoffice.org/30297
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

9 files changed, 131 insertions, 58 deletions

diff --git a/sc/inc/markdata.hxx b/sc/inc/markdata.hxx
index 0392971f0a1c..60c04d076bb8 100644
--- a/sc/inc/markdata.hxx
+++ b/sc/inc/markdata.hxx
@@ -143,10 +143,12 @@ public:
     // iterators for table access
     typedef std::set<SCTAB>::iterator iterator;
     typedef std::set<SCTAB>::const_iterator const_iterator;
+    typedef std::set<SCTAB>::const_reverse_iterator const_reverse_iterator;
     iterator begin();
     iterator end();
     const_iterator begin() const;
     const_iterator end() const;
+    const_reverse_iterator rbegin() const;
 };
 
 #endif
diff --git a/sc/source/core/data/markdata.cxx b/sc/source/core/data/markdata.cxx
index 9cb5329b8531..defbef2778e5 100644
--- a/sc/source/core/data/markdata.cxx
+++ b/sc/source/core/data/markdata.cxx
@@ -814,4 +814,9 @@ ScMarkData::const_iterator ScMarkData::end() const
     return maTabMarked.end();
 }
 
+ScMarkData::const_reverse_iterator ScMarkData::rbegin() const
+{
+    return maTabMarked.rbegin();
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/ui/docshell/docfunc.cxx b/sc/source/ui/docshell/docfunc.cxx
index 6fe32df1e233..8aacaad8f163 100644
--- a/sc/source/ui/docshell/docfunc.cxx
+++ b/sc/source/ui/docshell/docfunc.cxx
@@ -1790,6 +1790,7 @@ bool ScDocFunc::InsertCells( const ScRange& rRange, const ScMarkData* pTabMark,
     OUString aUndo = ScGlobal::GetRscString( STR_UNDO_INSERTCELLS );
     if (bRecord)
         rDocShell.GetUndoManager()->EnterListAction( aUndo, aUndo );
+    std::unique_ptr<ScUndoRemoveMerge> pUndoRemoveMerge;
 
     itr = aMark.begin();
     for (; itr != itrEnd && nTabCount; ++itr)
@@ -1897,12 +1898,19 @@ bool ScDocFunc::InsertCells( const ScRange& rRange, const ScMarkData* pTabMark,
 
                 if( !qIncreaseRange.empty() )
                 {
+                    if (bRecord && !pUndoRemoveMerge)
+                    {
+                        ScDocument* pUndoDoc = new ScDocument( SCDOCMODE_UNDO );
+                        pUndoDoc->InitUndo( &rDoc, *aMark.begin(), *aMark.rbegin());
+                        pUndoRemoveMerge.reset( new ScUndoRemoveMerge( &rDocShell, rRange, pUndoDoc ));
+                    }
+
                     for( ::std::vector<ScRange>::const_iterator iIter( qIncreaseRange.begin()); iIter != qIncreaseRange.end(); ++iIter )
                     {
                         ScRange aRange( *iIter );
                         if( rDoc.HasAttrib( aRange, HASATTR_OVERLAPPED | HASATTR_MERGED ) )
                         {
-                            UnmergeCells( aRange, true );
+                            UnmergeCells( aRange, bRecord, pUndoRemoveMerge.get() );
                         }
                     }
                 }
@@ -1918,6 +1926,11 @@ bool ScDocFunc::InsertCells( const ScRange& rRange, const ScMarkData* pTabMark,
         }
     }
 
+    if (bRecord && pUndoRemoveMerge)
+    {
+        rDocShell.GetUndoManager()->AddUndoAction( pUndoRemoveMerge.release());
+    }
+
     switch (eCmd)
     {
         case INS_CELLSDOWN:
@@ -2218,6 +2231,7 @@ bool ScDocFunc::DeleteCells( const ScRange& rRange, const ScMarkData* pTabMark,
     OUString aUndo = ScGlobal::GetRscString( STR_UNDO_DELETECELLS );
     if (bRecord)
         rDocShell.GetUndoManager()->EnterListAction( aUndo, aUndo );
+    std::unique_ptr<ScUndoRemoveMerge> pUndoRemoveMerge;
 
     itr = aMark.begin();
     for (; itr != itrEnd && *itr < nTabCount; ++itr)
@@ -2327,12 +2341,19 @@ bool ScDocFunc::DeleteCells( const ScRange& rRange, const ScMarkData* pTabMark,
 
                 if( !qDecreaseRange.empty() )
                 {
+                    if (bRecord && !pUndoRemoveMerge)
+                    {
+                        ScDocument* pUndoDoc = new ScDocument( SCDOCMODE_UNDO );
+                        pUndoDoc->InitUndo( &rDoc, *aMark.begin(), *aMark.rbegin());
+                        pUndoRemoveMerge.reset( new ScUndoRemoveMerge( &rDocShell, rRange, pUndoDoc ));
+                    }
+
                     for( ::std::vector<ScRange>::const_iterator iIter( qDecreaseRange.begin()); iIter != qDecreaseRange.end(); ++iIter )
                     {
                         ScRange aRange( *iIter );
                         if( rDoc.HasAttrib( aRange, HASATTR_OVERLAPPED | HASATTR_MERGED ) )
                         {
-                            UnmergeCells( aRange, true );
+                            UnmergeCells( aRange, bRecord, pUndoRemoveMerge.get() );
                         }
                     }
                 }
@@ -2347,6 +2368,11 @@ bool ScDocFunc::DeleteCells( const ScRange& rRange, const ScMarkData* pTabMark,
         }
     }
 
+    if (bRecord && pUndoRemoveMerge)
+    {
+        rDocShell.GetUndoManager()->AddUndoAction( pUndoRemoveMerge.release());
+    }
+
     //      ausfuehren
 
     WaitObject aWait( ScDocShell::GetActiveDialogParent() );      // wichtig wegen TrackFormulas bei UpdateReference
@@ -2794,7 +2820,7 @@ bool ScDocFunc::MoveBlock( const ScRange& rSource, const ScAddress& rDestPos,
 
     // skipped rows and merged cells don't mix
     if ( !bIncludeFiltered && pClipDoc->HasClipFilteredRows() )
-        UnmergeCells( aPasteDest, false );
+        UnmergeCells( aPasteDest, false, nullptr );
 
     bool bDestHeight = AdjustRowHeight(
                             ScRange( 0,nDestRow,nDestTab, MAXCOL,nDestEndRow,nDestEndTab ),
@@ -4777,17 +4803,17 @@ bool ScDocFunc::MergeCells( const ScCellMergeOption& rOption, bool bContents, bo
     return true;
 }
 
-bool ScDocFunc::UnmergeCells( const ScRange& rRange, bool bRecord )
+bool ScDocFunc::UnmergeCells( const ScRange& rRange, bool bRecord, ScUndoRemoveMerge* pUndoRemoveMerge )
 {
     ScCellMergeOption aOption(rRange.aStart.Col(), rRange.aStart.Row(), rRange.aEnd.Col(), rRange.aEnd.Row());
     SCTAB nTab1 = rRange.aStart.Tab(), nTab2 = rRange.aEnd.Tab();
     for (SCTAB i = nTab1; i <= nTab2; ++i)
         aOption.maTabs.insert(i);
 
-    return UnmergeCells(aOption, bRecord);
+    return UnmergeCells(aOption, bRecord, pUndoRemoveMerge);
 }
 
-bool ScDocFunc::UnmergeCells( const ScCellMergeOption& rOption, bool bRecord )
+bool ScDocFunc::UnmergeCells( const ScCellMergeOption& rOption, bool bRecord, ScUndoRemoveMerge* pUndoRemoveMerge )
 {
     using ::std::set;
 
@@ -4801,7 +4827,8 @@ bool ScDocFunc::UnmergeCells( const ScCellMergeOption& rOption, bool bRecord )
     if (bRecord && !rDoc.IsUndoEnabled())
         bRecord = false;
 
-    ScDocument* pUndoDoc = nullptr;
+    ScDocument* pUndoDoc = (pUndoRemoveMerge ? pUndoRemoveMerge->GetUndoDoc() : nullptr);
+    assert( pUndoDoc || !pUndoRemoveMerge );
     for (set<SCTAB>::const_iterator itr = rOption.maTabs.begin(), itrEnd = rOption.maTabs.end();
           itr != itrEnd; ++itr)
     {
@@ -4844,8 +4871,17 @@ bool ScDocFunc::UnmergeCells( const ScCellMergeOption& rOption, bool bRecord )
 
     if (bRecord)
     {
-        rDocShell.GetUndoManager()->AddUndoAction(
-            new ScUndoRemoveMerge( &rDocShell, rOption, pUndoDoc ) );
+        if (pUndoRemoveMerge)
+        {
+            // If pUndoRemoveMerge was passed, the caller is responsible for
+            // adding it to Undo. Just add the current option.
+            pUndoRemoveMerge->AddCellMergeOption( rOption);
+        }
+        else
+        {
+            rDocShell.GetUndoManager()->AddUndoAction(
+                    new ScUndoRemoveMerge( &rDocShell, rOption, pUndoDoc ) );
+        }
     }
     aModificator.SetDocumentModified();
 
diff --git a/sc/source/ui/inc/docfunc.hxx b/sc/source/ui/inc/docfunc.hxx
index b7709294b7c4..deb7b45d995f 100644
--- a/sc/source/ui/inc/docfunc.hxx
+++ b/sc/source/ui/inc/docfunc.hxx
@@ -45,6 +45,7 @@ class ScTableProtection;
 struct ScCellMergeOption;
 class ScConditionalFormat;
 class ScConditionalFormatList;
+class ScUndoRemoveMerge;
 
 namespace sc {
 
@@ -189,8 +190,8 @@ public:
 
     bool            MergeCells( const ScCellMergeOption& rOption, bool bContents,
                                         bool bRecord, bool bApi );
-    bool            UnmergeCells( const ScRange& rRange, bool bRecord );
-    bool            UnmergeCells( const ScCellMergeOption& rOption, bool bRecord );
+    bool            UnmergeCells( const ScRange& rRange, bool bRecord, ScUndoRemoveMerge* pUndoRemoveMerge );
+    bool            UnmergeCells( const ScCellMergeOption& rOption, bool bRecord, ScUndoRemoveMerge* pUndoRemoveMerge );
 
     void            SetNewRangeNames( ScRangeName* pNewRanges, bool bModifyDoc = true, SCTAB nTab = -1 );     // takes ownership of pNewRanges //nTab = -1 for local range names
     void            ModifyRangeNames( const ScRangeName& rNewRanges, SCTAB nTab = -1 );
diff --git a/sc/source/ui/inc/undoblk.hxx b/sc/source/ui/inc/undoblk.hxx
index 3187163aefd3..dd236f3a911a 100644
--- a/sc/source/ui/inc/undoblk.hxx
+++ b/sc/source/ui/inc/undoblk.hxx
@@ -902,6 +902,9 @@ public:
                     ScUndoRemoveMerge( ScDocShell* pNewDocShell,
                                        const ScCellMergeOption& rOption,
                                        ScDocument* pNewUndoDoc );
+                    ScUndoRemoveMerge( ScDocShell* pNewDocShell,
+                                       const ScRange& rRange,
+                                       ScDocument* pNewUndoDoc );
     virtual         ~ScUndoRemoveMerge();
 
     virtual void    Undo() override;
@@ -911,11 +914,14 @@ public:
 
     virtual OUString GetComment() const override;
 
+    ScDocument*     GetUndoDoc();
+    void            AddCellMergeOption( const ScCellMergeOption& rOption );
+
 private:
     void            SetCurTab();
 
-    ScCellMergeOption maOption;
-    ScDocument*     pUndoDoc;
+    std::vector<ScCellMergeOption> maOptions;
+    ScDocument*                    pUndoDoc;
 };
 
 class ScUndoBorder: public ScBlockUndo
diff --git a/sc/source/ui/undo/undoblk.cxx b/sc/source/ui/undo/undoblk.cxx
index 97d7e6fc4699..e42ca42f7b8f 100644
--- a/sc/source/ui/undo/undoblk.cxx
+++ b/sc/source/ui/undo/undoblk.cxx
@@ -1398,7 +1398,7 @@ void ScUndoDragDrop::Redo()
 
     // skipped rows and merged cells don't mix
     if ( !bIncludeFiltered && pClipDoc->HasClipFilteredRows() )
-        pDocShell->GetDocFunc().UnmergeCells( aDestRange, false );
+        pDocShell->GetDocFunc().UnmergeCells( aDestRange, false, nullptr );
 
     for (nTab=aDestRange.aStart.Tab(); nTab<=aDestRange.aEnd.Tab(); nTab++)
     {
@@ -2108,7 +2108,14 @@ bool ScUndoRemoveBreaks::CanRepeat(SfxRepeatTarget& rTarget) const
 ScUndoRemoveMerge::ScUndoRemoveMerge( ScDocShell* pNewDocShell,
                                       const ScCellMergeOption& rOption, ScDocument* pNewUndoDoc ) :
     ScBlockUndo( pNewDocShell, rOption.getFirstSingleRange(), SC_UNDO_SIMPLE ),
-    maOption(rOption),
+    pUndoDoc( pNewUndoDoc )
+{
+    maOptions.push_back( rOption);
+}
+
+ScUndoRemoveMerge::ScUndoRemoveMerge( ScDocShell* pNewDocShell,
+                                      const ScRange& rRange, ScDocument* pNewUndoDoc ) :
+    ScBlockUndo( pNewDocShell, rRange, SC_UNDO_SIMPLE ),
     pUndoDoc( pNewUndoDoc )
 {
 }
@@ -2123,6 +2130,16 @@ OUString ScUndoRemoveMerge::GetComment() const
     return ScGlobal::GetRscString( STR_UNDO_REMERGE );  // "remove merge"
 }
 
+ScDocument* ScUndoRemoveMerge::GetUndoDoc()
+{
+    return pUndoDoc;
+}
+
+void ScUndoRemoveMerge::AddCellMergeOption( const ScCellMergeOption& rOption )
+{
+    maOptions.push_back( rOption);
+}
+
 void ScUndoRemoveMerge::Undo()
 {
     using ::std::set;
@@ -2133,25 +2150,28 @@ void ScUndoRemoveMerge::Undo()
     ScTabViewShell* pViewShell = ScTabViewShell::GetActiveViewShell();
 
     ScDocument& rDoc = pDocShell->GetDocument();
-    for (set<SCTAB>::const_iterator itr = maOption.maTabs.begin(), itrEnd = maOption.maTabs.end();
-          itr != itrEnd; ++itr)
+    for (const auto & rOption : maOptions)
     {
-        OSL_ENSURE(pUndoDoc, "NULL pUndoDoc!");
-        if (!pUndoDoc)
-            continue;
-        // There is no need to extend merge area because it's already been extended.
-        ScRange aRange = maOption.getSingleRange(*itr);
-        rDoc.DeleteAreaTab(aRange, InsertDeleteFlags::ATTRIB);
-        pUndoDoc->CopyToDocument(aRange, InsertDeleteFlags::ATTRIB, false, &rDoc);
-
-        bool bDidPaint = false;
-        if ( pViewShell )
+        for (set<SCTAB>::const_iterator itr = rOption.maTabs.begin(), itrEnd = rOption.maTabs.end();
+                itr != itrEnd; ++itr)
         {
-            pViewShell->SetTabNo(*itr);
-            bDidPaint = pViewShell->AdjustRowHeight(maOption.mnStartRow, maOption.mnEndRow);
+            OSL_ENSURE(pUndoDoc, "NULL pUndoDoc!");
+            if (!pUndoDoc)
+                continue;
+            // There is no need to extend merge area because it's already been extended.
+            ScRange aRange = rOption.getSingleRange(*itr);
+            rDoc.DeleteAreaTab(aRange, InsertDeleteFlags::ATTRIB);
+            pUndoDoc->CopyToDocument(aRange, InsertDeleteFlags::ATTRIB, false, &rDoc);
+
+            bool bDidPaint = false;
+            if ( pViewShell )
+            {
+                pViewShell->SetTabNo(*itr);
+                bDidPaint = pViewShell->AdjustRowHeight(rOption.mnStartRow, rOption.mnEndRow);
+            }
+            if (!bDidPaint)
+                ScUndoUtil::PaintMore(pDocShell, aRange);
         }
-        if (!bDidPaint)
-            ScUndoUtil::PaintMore(pDocShell, aRange);
     }
 
     EndUndo();
@@ -2167,36 +2187,39 @@ void ScUndoRemoveMerge::Redo()
     ScDocument& rDoc = pDocShell->GetDocument();
     ScTabViewShell* pViewShell = ScTabViewShell::GetActiveViewShell();
 
-    for (set<SCTAB>::const_iterator itr = maOption.maTabs.begin(), itrEnd = maOption.maTabs.end();
-          itr != itrEnd; ++itr)
+    for (const auto & rOption : maOptions)
     {
-        SCTAB nTab = *itr;
-        // There is no need to extend merge area because it's already been extended.
-        ScRange aRange = maOption.getSingleRange(nTab);
+        for (set<SCTAB>::const_iterator itr = rOption.maTabs.begin(), itrEnd = rOption.maTabs.end();
+                itr != itrEnd; ++itr)
+        {
+            SCTAB nTab = *itr;
+            // There is no need to extend merge area because it's already been extended.
+            ScRange aRange = rOption.getSingleRange(nTab);
 
-        const SfxPoolItem& rDefAttr = rDoc.GetPool()->GetDefaultItem( ATTR_MERGE );
-        ScPatternAttr aPattern( rDoc.GetPool() );
-        aPattern.GetItemSet().Put( rDefAttr );
-        rDoc.ApplyPatternAreaTab( maOption.mnStartCol, maOption.mnStartRow,
-                                   maOption.mnEndCol, maOption.mnEndRow, nTab,
-                                   aPattern );
+            const SfxPoolItem& rDefAttr = rDoc.GetPool()->GetDefaultItem( ATTR_MERGE );
+            ScPatternAttr aPattern( rDoc.GetPool() );
+            aPattern.GetItemSet().Put( rDefAttr );
+            rDoc.ApplyPatternAreaTab( rOption.mnStartCol, rOption.mnStartRow,
+                    rOption.mnEndCol, rOption.mnEndRow, nTab,
+                    aPattern );
 
-        rDoc.RemoveFlagsTab( maOption.mnStartCol, maOption.mnStartRow,
-                              maOption.mnEndCol, maOption.mnEndRow, nTab,
-                              ScMF::Hor | ScMF::Ver );
+            rDoc.RemoveFlagsTab( rOption.mnStartCol, rOption.mnStartRow,
+                    rOption.mnEndCol, rOption.mnEndRow, nTab,
+                    ScMF::Hor | ScMF::Ver );
 
-        rDoc.ExtendMerge(aRange, true);
+            rDoc.ExtendMerge(aRange, true);
 
-        //  Paint
+            //  Paint
 
-        bool bDidPaint = false;
-        if ( pViewShell )
-        {
-            pViewShell->SetTabNo(nTab);
-            bDidPaint = pViewShell->AdjustRowHeight(maOption.mnStartRow, maOption.mnEndRow);
+            bool bDidPaint = false;
+            if ( pViewShell )
+            {
+                pViewShell->SetTabNo(nTab);
+                bDidPaint = pViewShell->AdjustRowHeight(rOption.mnStartRow, rOption.mnEndRow);
+            }
+            if (!bDidPaint)
+                ScUndoUtil::PaintMore(pDocShell, aRange);
         }
-        if (!bDidPaint)
-            ScUndoUtil::PaintMore(pDocShell, aRange);
     }
 
     EndRedo();
diff --git a/sc/source/ui/unoobj/cellsuno.cxx b/sc/source/ui/unoobj/cellsuno.cxx
index de019bfac9ad..522d4ae0cc1d 100644
--- a/sc/source/ui/unoobj/cellsuno.cxx
+++ b/sc/source/ui/unoobj/cellsuno.cxx
@@ -5374,7 +5374,7 @@ void SAL_CALL ScCellRangeObj::merge( sal_Bool bMerge ) throw(uno::RuntimeExcepti
         if ( bMerge )
             pDocSh->GetDocFunc().MergeCells( aMergeOption, false, true, true );
         else
-            pDocSh->GetDocFunc().UnmergeCells( aMergeOption, true );
+            pDocSh->GetDocFunc().UnmergeCells( aMergeOption, true, nullptr );
 
         //! Fehler abfangen?
     }
diff --git a/sc/source/ui/view/viewfun2.cxx b/sc/source/ui/view/viewfun2.cxx
index b9a423048921..7e2e8b316708 100644
--- a/sc/source/ui/view/viewfun2.cxx
+++ b/sc/source/ui/view/viewfun2.cxx
@@ -1188,7 +1188,7 @@ bool ScViewFunc::RemoveMerge()
         }
         while (bExtended);
 
-        bool bOk = pDocSh->GetDocFunc().UnmergeCells(aOption, true/*bRecord*/ );
+        bool bOk = pDocSh->GetDocFunc().UnmergeCells(aOption, true/*bRecord*/, nullptr);
         aExtended = aOption.getFirstSingleRange();
         MarkRange( aExtended );
 
diff --git a/sc/source/ui/view/viewfun3.cxx b/sc/source/ui/view/viewfun3.cxx
index f798690cb8a2..95e1177b40f8 100644
--- a/sc/source/ui/view/viewfun3.cxx
+++ b/sc/source/ui/view/viewfun3.cxx
@@ -1201,7 +1201,7 @@ bool ScViewFunc::PasteFromClip( InsertDeleteFlags nFlags, ScDocument* pClipDoc,
                     ScRange aRange(nCol, nRow1, nStartTab);
                     pDoc->ExtendOverlapped(aRange);
                     pDoc->ExtendMerge(aRange, true);
-                    rDocFunc.UnmergeCells(aRange, bRecord);
+                    rDocFunc.UnmergeCells(aRange, bRecord, nullptr /*TODO: should pass combined UndoDoc if bRecord*/);
                 }
             }
         }
@@ -1302,7 +1302,7 @@ bool ScViewFunc::PasteFromClip( InsertDeleteFlags nFlags, ScDocument* pClipDoc,
 
     // skipped rows and merged cells don't mix
     if ( !bIncludeFiltered && pClipDoc->HasClipFilteredRows() )
-        rDocFunc.UnmergeCells( aUserRange, false );
+        rDocFunc.UnmergeCells( aUserRange, false, nullptr );
 
     pDoc->ExtendMergeSel( nStartCol, nStartRow, nEndCol, nEndRow, aFilteredMark, true );    // refresh
                                                                                     // new range