From 49bca657d7d66ce75a49d2912d1fc86b3e89bcc4 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Mon, 5 Aug 2024 11:32:37 +0200 Subject: [PATCH] simplify IMark hierarchy (10) use more type-safe container for AnnotationMark, which also avoids some dynamic_cast Change-Id: I1b1d44f81f2a3d626a1f13fbe952eecd1ea0390a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171564 Tested-by: Jenkins Reviewed-by: Noel Grandin --- sw/Library_msword.mk | 1 + sw/inc/IDocumentMarkAccess.hxx | 11 +++-- sw/inc/txtannotationfld.hxx | 4 +- sw/source/core/crsr/swcrsr.cxx | 3 +- .../doc/DocumentContentOperationsManager.cxx | 1 + sw/source/core/doc/docbm.cxx | 48 ++++++++++++++----- sw/source/core/doc/docredln.cxx | 1 + sw/source/core/inc/MarkManager.hxx | 13 ++--- sw/source/core/txtnode/atrfld.cxx | 2 +- sw/source/core/unocore/unofield.cxx | 1 + sw/source/core/unocore/unoportenum.cxx | 7 +-- sw/source/filter/ww8/wrtw8nds.cxx | 3 +- 12 files changed, 63 insertions(+), 32 deletions(-) diff --git a/sw/Library_msword.mk b/sw/Library_msword.mk index 5427e2d3e858..7d9d0c7f80f8 100644 --- a/sw/Library_msword.mk +++ b/sw/Library_msword.mk @@ -29,6 +29,7 @@ $(eval $(call gb_Library_use_custom_headers,msword,\ $(eval $(call gb_Library_set_precompiled_header,msword,sw/inc/pch/precompiled_msword)) $(eval $(call gb_Library_set_include,msword,\ + -I$(SRCDIR)/sw/source/core/inc \ -I$(SRCDIR)/sw/source/filter/inc \ -I$(SRCDIR)/sw/inc \ $$(INCLUDE) \ diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx index 590d243189d7..60b2928067a1 100644 --- a/sw/inc/IDocumentMarkAccess.hxx +++ b/sw/inc/IDocumentMarkAccess.hxx @@ -30,6 +30,7 @@ class SwTextNode; class SwCursorShell; namespace sw::mark { + class AnnotationMark; class SaveBookmark; // FIXME: Ugly: SaveBookmark is a core-internal class, and should not be used in the interface class MarkBase; class Fieldmark; @@ -302,11 +303,11 @@ class IDocumentMarkAccess virtual void ClearFieldActivation() = 0; // Annotation Marks - virtual const_iterator getAnnotationMarksBegin() const = 0; - virtual const_iterator getAnnotationMarksEnd() const = 0; + virtual std::vector::const_iterator getAnnotationMarksBegin() const = 0; + virtual std::vector::const_iterator getAnnotationMarksEnd() const = 0; virtual sal_Int32 getAnnotationMarksCount() const = 0; - virtual const_iterator findAnnotationMark( const OUString& rName ) const = 0; - virtual sw::mark::MarkBase* getAnnotationMarkFor(const SwPosition& rPosition) const = 0; + virtual std::vector::const_iterator findAnnotationMark( const OUString& rName ) const = 0; + virtual sw::mark::AnnotationMark* getAnnotationMarkFor(const SwPosition& rPosition) const = 0; // handle and restore text ranges of annotations of tracked deletions // based on the helper bookmarks (which can survive I/O and hiding redlines) virtual ::sw::mark::Bookmark* makeAnnotationBookmark(const SwPaM& rPaM, @@ -320,7 +321,7 @@ class IDocumentMarkAccess @returns an iterator pointing to the mark, or pointing to getAnnotationMarksEnd() if nothing was found. */ - virtual const_iterator findFirstAnnotationStartsAfter(const SwPosition& rPos) const =0; + virtual std::vector::const_iterator findFirstAnnotationStartsAfter(const SwPosition& rPos) const =0; /** Returns the MarkType used to create the mark */ diff --git a/sw/inc/txtannotationfld.hxx b/sw/inc/txtannotationfld.hxx index a8acb2940e79..ac8cf1ff5d77 100644 --- a/sw/inc/txtannotationfld.hxx +++ b/sw/inc/txtannotationfld.hxx @@ -22,7 +22,7 @@ #include "txtfld.hxx" -namespace sw::mark { class MarkBase; } +namespace sw::mark { class AnnotationMark; } class SwTextAnnotationField final : public SwTextField { @@ -34,7 +34,7 @@ public: virtual ~SwTextAnnotationField() override; - ::sw::mark::MarkBase* GetAnnotationMark() const; + ::sw::mark::AnnotationMark* GetAnnotationMark() const; }; #endif diff --git a/sw/source/core/crsr/swcrsr.cxx b/sw/source/core/crsr/swcrsr.cxx index 9f01cb239bf1..bf68081de1fe 100644 --- a/sw/source/core/crsr/swcrsr.cxx +++ b/sw/source/core/crsr/swcrsr.cxx @@ -58,6 +58,7 @@ #include #include +#include using namespace ::com::sun::star::i18n; @@ -1494,7 +1495,7 @@ bool SwCursor::SelectWordWT( SwViewShell const * pViewShell, sal_Int16 nWordType { SetMark(); GetMark()->Assign(*pStartNode, nStartIndex); - if (sw::mark::MarkBase* pAnnotationMark = pMarksAccess->getAnnotationMarkFor(*GetPoint())) + if (sw::mark::AnnotationMark* pAnnotationMark = pMarksAccess->getAnnotationMarkFor(*GetPoint())) { // An annotation mark covers the selected word. Check // if it covers only the word: in that case we select diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx index 56b44e289e62..e95b3be234c1 100644 --- a/sw/source/core/doc/DocumentContentOperationsManager.cxx +++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx @@ -73,6 +73,7 @@ #include #include #include +#include #include #include #include diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx index 63ca42ca0e3a..6a82a22df15e 100644 --- a/sw/source/core/doc/docbm.cxx +++ b/sw/source/core/doc/docbm.cxx @@ -113,6 +113,32 @@ namespace return bCRFirst; // cross-ref sorts *before* } + // specialise to avoid loplugin:faileddyncast + template<> + bool lcl_MarkOrderingByStart(const AnnotationMark *const pFirst, + const AnnotationMark *const pSecond) + { + SwPosition const& rFirstStart(pFirst->GetMarkStart()); + SwPosition const& rSecondStart(pSecond->GetMarkStart()); + if (rFirstStart.GetNode() != rSecondStart.GetNode()) + { + return rFirstStart.GetNode() < rSecondStart.GetNode(); + } + const sal_Int32 nFirstContent = rFirstStart.GetContentIndex(); + const sal_Int32 nSecondContent = rSecondStart.GetContentIndex(); + if (nFirstContent != 0 || nSecondContent != 0) + { + return nFirstContent < nSecondContent; + } + SwContentNode const*const pFirstNode(rFirstStart.nContent.GetContentNode()); + SwContentNode const*const pSecondNode(rSecondStart.nContent.GetContentNode()); + if ((pFirstNode != nullptr) != (pSecondNode != nullptr)) + { // consistency with SwPosition::operator< + return pSecondNode != nullptr; + } + return false; // equal + } + template bool lcl_MarkOrderingByEnd(const MarkT *const pFirst, const MarkT *const pSecond) @@ -613,7 +639,7 @@ namespace sw::mark lcl_InsertMarkSorted(m_vFieldmarks, static_cast(pMark.get())); break; case IDocumentMarkAccess::MarkType::ANNOTATIONMARK: - lcl_InsertMarkSorted( m_vAnnotationMarks, pMark.get() ); + lcl_InsertMarkSorted( m_vAnnotationMarks, static_cast(pMark.get()) ); break; case IDocumentMarkAccess::MarkType::NAVIGATOR_REMINDER: case IDocumentMarkAccess::MarkType::DDE_BOOKMARK: @@ -1242,7 +1268,7 @@ namespace sw::mark case IDocumentMarkAccess::MarkType::ANNOTATIONMARK: { - auto const ppAnnotationMark = lcl_FindMark(m_vAnnotationMarks, *ppMark); + auto const ppAnnotationMark = lcl_FindMark(m_vAnnotationMarks, static_cast(*ppMark)); assert(ppAnnotationMark != m_vAnnotationMarks.end() && " - Annotation Mark not found in Annotation Mark container."); m_vAnnotationMarks.erase(ppAnnotationMark); @@ -1629,12 +1655,12 @@ namespace sw::mark Fieldmark* MarkManager::getFieldmarkBefore(const SwPosition& rPos, bool bLoop) const { return lcl_getMarkBefore(m_vFieldmarks, rPos, bLoop); } - IDocumentMarkAccess::const_iterator MarkManager::getAnnotationMarksBegin() const + std::vector::const_iterator MarkManager::getAnnotationMarksBegin() const { return m_vAnnotationMarks.begin(); } - IDocumentMarkAccess::const_iterator MarkManager::getAnnotationMarksEnd() const + std::vector::const_iterator MarkManager::getAnnotationMarksEnd() const { return m_vAnnotationMarks.end(); } @@ -1644,30 +1670,30 @@ namespace sw::mark return m_vAnnotationMarks.size(); } - IDocumentMarkAccess::const_iterator MarkManager::findAnnotationMark( const OUString& rName ) const + std::vector::const_iterator MarkManager::findAnnotationMark( const OUString& rName ) const { - return lcl_FindMarkByName( rName, m_vAnnotationMarks.begin(), m_vAnnotationMarks.end() ); + return lcl_FindMarkByName( rName, m_vAnnotationMarks.begin(), m_vAnnotationMarks.end() ); } - MarkBase* MarkManager::getAnnotationMarkFor(const SwPosition& rPos) const + AnnotationMark* MarkManager::getAnnotationMarkFor(const SwPosition& rPos) const { auto const pAnnotationMark = find_if( m_vAnnotationMarks.begin(), m_vAnnotationMarks.end(), - [&rPos] (const ::sw::mark::MarkBase *const pMark) { return pMark->IsCoveringPosition(rPos); } ); + [&rPos] (const ::sw::mark::AnnotationMark *const pMark) { return pMark->IsCoveringPosition(rPos); } ); if (pAnnotationMark == m_vAnnotationMarks.end()) return nullptr; return *pAnnotationMark; } // finds the first that is starting after - IDocumentMarkAccess::const_iterator MarkManager::findFirstAnnotationStartsAfter(const SwPosition& rPos) const + std::vector::const_iterator MarkManager::findFirstAnnotationStartsAfter(const SwPosition& rPos) const { return std::upper_bound( m_vAnnotationMarks.begin(), m_vAnnotationMarks.end(), rPos, - CompareIMarkStartsAfter()); + CompareIMarkStartsAfter()); } // create helper bookmark for annotations on tracked deletions @@ -1701,7 +1727,7 @@ namespace sw::mark (nPos = rBookmarkName.indexOf(S_ANNOTATION_BOOKMARK)) > -1 ) { ::sw::UndoGuard const undoGuard(m_rDoc.GetIDocumentUndoRedo()); - IDocumentMarkAccess::const_iterator pMark = findAnnotationMark(rBookmarkName.copy(0, nPos)); + auto pMark = findAnnotationMark(rBookmarkName.copy(0, nPos)); if ( pMark != getAnnotationMarksEnd() ) { const SwPaM aPam((**iter).GetMarkStart(), (**pMark).GetMarkEnd()); diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx index e4f1f0f1de13..f48b067ad9ea 100644 --- a/sw/source/core/doc/docredln.cxx +++ b/sw/source/core/doc/docredln.cxx @@ -64,6 +64,7 @@ #include #include +#include using namespace com::sun::star; diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx index a737662bb85f..593caeac9984 100644 --- a/sw/source/core/inc/MarkManager.hxx +++ b/sw/source/core/inc/MarkManager.hxx @@ -31,6 +31,7 @@ class SfxViewShell; namespace sw::mark { typedef std::unordered_map MarkBasenameMapUniqueOffset_t; + class AnnotationMark; class FieldmarkWithDropDownButton; class MarkManager final @@ -114,12 +115,12 @@ namespace sw::mark { void dumpAsXml(xmlTextWriterPtr pWriter) const; // Annotation Marks - virtual const_iterator getAnnotationMarksBegin() const override; - virtual const_iterator getAnnotationMarksEnd() const override; + virtual std::vector::const_iterator getAnnotationMarksBegin() const override; + virtual std::vector::const_iterator getAnnotationMarksEnd() const override; virtual sal_Int32 getAnnotationMarksCount() const override; - virtual const_iterator findAnnotationMark( const OUString& rName ) const override; - virtual sw::mark::MarkBase* getAnnotationMarkFor(const SwPosition& rPos) const override; - virtual const_iterator findFirstAnnotationStartsAfter(const SwPosition& rPos) const override; + virtual std::vector::const_iterator findAnnotationMark( const OUString& rName ) const override; + virtual sw::mark::AnnotationMark* getAnnotationMarkFor(const SwPosition& rPos) const override; + virtual std::vector::const_iterator findFirstAnnotationStartsAfter(const SwPosition& rPos) const override; virtual void assureSortedMarkContainers() const override; @@ -155,7 +156,7 @@ namespace sw::mark { mutable MarkBasenameMapUniqueOffset_t m_aMarkBasenameMapUniqueOffset; // container for annotation marks - container_t m_vAnnotationMarks; + std::vector m_vAnnotationMarks; SwDoc& m_rDoc; diff --git a/sw/source/core/txtnode/atrfld.cxx b/sw/source/core/txtnode/atrfld.cxx index 53846770f6d5..7155248d793f 100644 --- a/sw/source/core/txtnode/atrfld.cxx +++ b/sw/source/core/txtnode/atrfld.cxx @@ -782,7 +782,7 @@ SwTextAnnotationField::~SwTextAnnotationField() { } -::sw::mark::MarkBase* SwTextAnnotationField::GetAnnotationMark() const +::sw::mark::AnnotationMark* SwTextAnnotationField::GetAnnotationMark() const { auto pPostItField = dynamic_cast(GetFormatField().GetField()); assert(pPostItField); diff --git a/sw/source/core/unocore/unofield.cxx b/sw/source/core/unocore/unofield.cxx index bd4c2b89fbe9..1ba6b6d53de4 100644 --- a/sw/source/core/unocore/unofield.cxx +++ b/sw/source/core/unocore/unofield.cxx @@ -85,6 +85,7 @@ #include #include #include +#include #include using namespace ::com::sun::star; diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx index f7bdd63fa9f9..1a9bb475af1c 100644 --- a/sw/source/core/unocore/unoportenum.cxx +++ b/sw/source/core/unocore/unoportenum.cxx @@ -274,11 +274,8 @@ namespace ppMark != pCandidatesEnd; ++ppMark ) { - ::sw::mark::AnnotationMark* const pAnnotationMark = - dynamic_cast< ::sw::mark::AnnotationMark* >(*ppMark); - - if (!pAnnotationMark) - continue; + ::sw::mark::AnnotationMark* const pAnnotationMark = *ppMark; + assert(pAnnotationMark); const SwPosition& rStartPos = pAnnotationMark->GetMarkStart(); if (rStartPos.GetNode() != rOwnNode) diff --git a/sw/source/filter/ww8/wrtw8nds.cxx b/sw/source/filter/ww8/wrtw8nds.cxx index 4c0d273502ad..ccb3b1c0b6fd 100644 --- a/sw/source/filter/ww8/wrtw8nds.cxx +++ b/sw/source/filter/ww8/wrtw8nds.cxx @@ -110,6 +110,7 @@ #include #include #include +#include #include @@ -2068,7 +2069,7 @@ bool MSWordExportBase::GetAnnotationMarks( const SwWW8AttrIter& rAttrs, sal_Int3 const sal_Int32 nMarks = pMarkAccess->getAnnotationMarksCount(); for ( sal_Int32 i = 0; i < nMarks; i++ ) { - MarkBase* pMark = pMarkAccess->getAnnotationMarksBegin()[i]; + AnnotationMark* pMark = pMarkAccess->getAnnotationMarksBegin()[i]; // Only keep the bookmarks starting or ending in this node if ( pMark->GetMarkStart().GetNode() == rNd ||