[Libreoffice-commits] core.git: Branch 'private/mst/sw_redlinehide_2' - sw/inc sw/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Fri Sep 7 11:19:16 UTC 2018


 sw/inc/redline.hxx                                      |    7 +
 sw/source/core/doc/DocumentContentOperationsManager.cxx |  105 ++++++++++------
 sw/source/core/doc/docredln.cxx                         |   41 ++++--
 3 files changed, 109 insertions(+), 44 deletions(-)

New commits:
commit c0cbc39529f89537575d96419aa41312c60e27e7
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Sep 7 12:35:54 2018 +0200
Commit:     Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Fri Sep 7 13:07:18 2018 +0200

    sw_redlinehide_2: fix frame updates for SwUndoRedlineDelete
    
    There is a special case where the redline that is inserted doesn't have
    a valid range and then it's split up inside AppendRedline(), in
    SwRedlineTable::InsertWithValidRanges().  This happens for example
    with a selection that includes a section start node but not its end node
    (or vice versa).
    
    This breaks the UpdateFramesForRemoveDeleteRedline() /
    UpdateFramesForAddDeleteRedline() because they assume that the given
    range is one SwRangeRedline; the result is duplicate text frames.
    
    This could be worked around by iterating the SwRangeRedline in the given
    PaM, but for the Remove case there is the additional complication that
    the redlines are actually removed by the time the function is called.
    
    So rework the implementation of DeleteAndJoinWithRedlineImpl()
    to call the part of InsertWithValidRanges() that does the splitting
    into multiple redlines (extracted into new sw::GetAllValidRanges())
    and create multiple SwUndoRedlineDelete actions, each of which now
    updates the frames properly.
    
    Also clean up the horrible group-undo code to group before inserting,
    instead of after-the-fact cleanup.
    
    Change-Id: Ia279910e0c74edabe56b0c4cf87dbbad688179da

diff --git a/sw/inc/redline.hxx b/sw/inc/redline.hxx
index 40d8377fcb54..d66d8b4a8ec6 100644
--- a/sw/inc/redline.hxx
+++ b/sw/inc/redline.hxx
@@ -340,6 +340,13 @@ class SW_DLLPUBLIC SwRedlineHint : public SfxHint
 {
 };
 
+
+namespace sw {
+
+std::vector<SwRangeRedline*> GetAllValidRanges(std::unique_ptr<SwRangeRedline> p);
+
+} // namespace sw
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index dc45775cbf25..46ee7b26f894 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -3571,10 +3571,32 @@ bool DocumentContentOperationsManager::DeleteAndJoinWithRedlineImpl( SwPaM & rPa
 {
     assert(m_rDoc.getIDocumentRedlineAccess().IsRedlineOn());
 
-    SwUndoRedlineDelete* pUndo = nullptr;
     RedlineFlags eOld = m_rDoc.getIDocumentRedlineAccess().GetRedlineFlags();
     m_rDoc.GetDocumentRedlineManager().checkRedlining( eOld );
 
+    if (*rPam.GetPoint() == *rPam.GetMark())
+    {
+        return false; // do not add empty redlines
+    }
+
+    std::vector<SwRangeRedline*> redlines;
+    {
+        auto pRedline(o3tl::make_unique<SwRangeRedline>(nsRedlineType_t::REDLINE_DELETE, rPam));
+        if (pRedline->HasValidRange())
+        {
+            redlines.push_back(pRedline.release());
+        }
+        else // sigh ... why is such a selection even possible...
+        {    // split it up so we get one SwUndoRedlineDelete per inserted RL
+            redlines = GetAllValidRanges(std::move(pRedline));
+        }
+    }
+
+    if (redlines.empty())
+    {
+        return false;
+    }
+
     auto & rDMA(*m_rDoc.getIDocumentMarkAccess());
     std::vector<std::unique_ptr<SwUndo>> MarkUndos;
     for (auto iter = rDMA.getAnnotationMarksBegin();
@@ -3604,6 +3626,7 @@ bool DocumentContentOperationsManager::DeleteAndJoinWithRedlineImpl( SwPaM & rPa
         }
     }
 
+    std::vector<std::unique_ptr<SwUndoRedlineDelete>> undos;
     if (m_rDoc.GetIDocumentUndoRedo().DoesUndo())
     {
         /* please don't translate -- for cultural reasons this comment is protected
@@ -3611,47 +3634,61 @@ bool DocumentContentOperationsManager::DeleteAndJoinWithRedlineImpl( SwPaM & rPa
         //JP 06.01.98: MUSS noch optimiert werden!!!
         m_rDoc.getIDocumentRedlineAccess().SetRedlineFlags(
             RedlineFlags::On | RedlineFlags::ShowInsert | RedlineFlags::ShowDelete);
-        pUndo = new SwUndoRedlineDelete( rPam, SwUndoId::DELETE );
-        const SwRewriter aRewriter = pUndo->GetRewriter();
-        m_rDoc.GetIDocumentUndoRedo().StartUndo( SwUndoId::DELETE, &aRewriter );
-        for (auto& it : MarkUndos)
+        for (SwRangeRedline * pRedline : redlines)
+        {
+            assert(pRedline->HasValidRange());
+            undos.emplace_back(o3tl::make_unique<SwUndoRedlineDelete>(
+                        *pRedline, SwUndoId::DELETE));
+        }
+        const SwRewriter aRewriter = undos.front()->GetRewriter();
+        // can only group a single undo action
+        if (MarkUndos.empty() && undos.size() == 1
+            && m_rDoc.GetIDocumentUndoRedo().DoesGroupUndo())
+        {
+            SwUndo * const pLastUndo( m_rDoc.GetUndoManager().GetLastUndo() );
+            SwUndoRedlineDelete *const pUndoRedlineDel(dynamic_cast<SwUndoRedlineDelete*>(pLastUndo));
+            bool const bMerged = pUndoRedlineDel
+                && pUndoRedlineDel->CanGrouping(*undos.front());
+            if (!bMerged)
+            {
+                m_rDoc.GetIDocumentUndoRedo().AppendUndo(undos.front().release());
+            }
+            undos.clear(); // prevent unmatched EndUndo
+        }
+        else
         {
-            m_rDoc.GetIDocumentUndoRedo().AppendUndo(it.release());
+            m_rDoc.GetIDocumentUndoRedo().StartUndo(SwUndoId::DELETE, &aRewriter);
+            for (auto& it : MarkUndos)
+            {
+                m_rDoc.GetIDocumentUndoRedo().AppendUndo(it.release());
+            }
+            for (auto & it : undos)
+            {
+                m_rDoc.GetIDocumentUndoRedo().AppendUndo(it.release());
+            }
         }
-        m_rDoc.GetIDocumentUndoRedo().AppendUndo( pUndo );
     }
 
-    if (*rPam.GetPoint() != *rPam.GetMark())
-        m_rDoc.getIDocumentRedlineAccess().AppendRedline( new SwRangeRedline( nsRedlineType_t::REDLINE_DELETE, rPam ), true );
+    for (SwRangeRedline *const pRedline : redlines)
+    {
+        // note: 1. the pRedline can still be merged & deleted
+        //       2. the impl. can even DeleteAndJoin the range => no plain PaM
+        std::shared_ptr<SwUnoCursor> const pCursor(m_rDoc.CreateUnoCursor(*pRedline->GetMark()));
+        pCursor->SetMark();
+        *pCursor->GetPoint() = *pRedline->GetPoint();
+        m_rDoc.getIDocumentRedlineAccess().AppendRedline(pRedline, true);
+        // sw_redlinehide: 2 reasons why this is needed:
+        // 1. it's the first redline in node => RedlineDelText was sent but ignored
+        // 2. redline spans multiple nodes => must merge text frames
+        sw::UpdateFramesForAddDeleteRedline(*pCursor);
+    }
     m_rDoc.getIDocumentState().SetModified();
 
-    // sw_redlinehide: 2 reasons why this is needed:
-    // 1. it's the first redline in node => RedlineDelText was sent but ignored
-    // 2. redline spans multiple nodes => must merge text frames
-    sw::UpdateFramesForAddDeleteRedline(rPam);
-
-    if (pUndo)
+    if (m_rDoc.GetIDocumentUndoRedo().DoesUndo())
     {
-        m_rDoc.GetIDocumentUndoRedo().EndUndo( SwUndoId::EMPTY, nullptr );
-        // ??? why the hell is the AppendUndo not below the
-        // CanGrouping, so this hideous cleanup wouldn't be necessary?
-        // bah, this is redlining, probably changing this would break it...
-        if (m_rDoc.GetIDocumentUndoRedo().DoesGroupUndo())
+        if (!undos.empty())
         {
-            SwUndo * const pLastUndo( m_rDoc.GetUndoManager().GetLastUndo() );
-            SwUndoRedlineDelete *const pUndoRedlineDel( dynamic_cast<SwUndoRedlineDelete*>(pLastUndo) );
-            if (pUndoRedlineDel)
-            {
-                bool const bMerged = pUndoRedlineDel->CanGrouping( *pUndo );
-                if (bMerged)
-                {
-                    ::sw::UndoGuard const undoGuard( m_rDoc.GetIDocumentUndoRedo() );
-                    SwUndo const*const pDeleted = m_rDoc.GetUndoManager().RemoveLastUndo();
-                    OSL_ENSURE( pDeleted == pUndo, "DeleteAndJoinWithRedlineImpl: "
-                        "undo removed is not undo inserted?" );
-                    delete pDeleted;
-                }
-            }
+            m_rDoc.GetIDocumentUndoRedo().EndUndo(SwUndoId::EMPTY, nullptr);
         }
         //JP 06.01.98: MUSS noch optimiert werden!!!
         m_rDoc.getIDocumentRedlineAccess().SetRedlineFlags( eOld );
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index 6fe4e815dc69..716cdfc6c2d7 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -450,10 +450,12 @@ bool SwRedlineTable::Insert(SwRangeRedlinePtr& p, size_type& rP)
     return InsertWithValidRanges( p, &rP );
 }
 
-bool SwRedlineTable::InsertWithValidRanges(SwRangeRedlinePtr& p, size_type* pInsPos)
+namespace sw {
+
+std::vector<SwRangeRedline*> GetAllValidRanges(std::unique_ptr<SwRangeRedline> p)
 {
+    std::vector<SwRangeRedline*> ret;
     // Create valid "sub-ranges" from the Selection
-    bool bAnyIns = false;
     SwPosition* pStt = p->Start(),
               * pEnd = pStt == p->GetPoint() ? p->GetMark() : p->GetPoint();
     SwPosition aNewStt( *pStt );
@@ -470,7 +472,6 @@ bool SwRedlineTable::InsertWithValidRanges(SwRangeRedlinePtr& p, size_type* pIns
     }
 
     SwRangeRedline* pNew = nullptr;
-    size_type nInsPos;
 
     if( aNewStt < *pEnd )
         do {
@@ -538,14 +539,10 @@ bool SwRedlineTable::InsertWithValidRanges(SwRangeRedlinePtr& p, size_type* pIns
 #endif
 
             if( *pNew->GetPoint() != *pNew->GetMark() &&
-                pNew->HasValidRange() &&
-                Insert( pNew, nInsPos ) )
+                pNew->HasValidRange())
             {
-                pNew->CallDisplayFunc(nInsPos);
-                bAnyIns = true;
+                ret.push_back(pNew);
                 pNew = nullptr;
-                if( pInsPos && *pInsPos < nInsPos )
-                    *pInsPos = nInsPos;
             }
 
             if( aNewStt >= *pEnd ||
@@ -557,7 +554,31 @@ bool SwRedlineTable::InsertWithValidRanges(SwRangeRedlinePtr& p, size_type* pIns
         } while( aNewStt < *pEnd );
 
     delete pNew;
-    delete p;
+    p.reset();
+    return ret;
+}
+
+} // namespace sw
+
+bool SwRedlineTable::InsertWithValidRanges(SwRangeRedlinePtr& p, size_type* pInsPos)
+{
+    bool bAnyIns = false;
+    std::vector<SwRangeRedline*> const redlines(
+            GetAllValidRanges(std::unique_ptr<SwRangeRedline>(p)));
+    for (SwRangeRedline * pRedline : redlines)
+    {
+        assert(pRedline->HasValidRange());
+        size_type nInsPos;
+        if (Insert(pRedline, nInsPos))
+        {
+            pRedline->CallDisplayFunc(nInsPos);
+            bAnyIns = true;
+            if (pInsPos && *pInsPos < nInsPos)
+            {
+                *pInsPos = nInsPos;
+            }
+        }
+    }
     p = nullptr;
     return bAnyIns;
 }


More information about the Libreoffice-commits mailing list