desktop/source/lib/init.cxx | 4 +- include/sfx2/lokhelper.hxx | 17 +++++++--- sfx2/source/view/lokhelper.cxx | 69 ++++++++++++++++++++++++----------------- sw/source/core/crsr/crsrsh.cxx | 2 - 4 files changed, 56 insertions(+), 36 deletions(-)
New commits: commit 44f6329330e8c8f000cb310b04ac13777e8bfff3 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Fri Jul 24 11:33:46 2020 -0400 Commit: Tor Lillqvist <t...@collabora.com> CommitDate: Fri Nov 20 14:34:00 2020 +0100 sfx2: lok: refactor notifications and const correctness This reduces the stringification and reuses the notificaiton helpers to reduce code duplication. Change-Id: Icf9f9c50f3eeee61a0ded741d39fed37cfcc8da1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99972 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Ashod Nakashian <a...@collabora.com> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106221 Tested-by: Jenkins Reviewed-by: Tor Lillqvist <t...@collabora.com> diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx index 8f82d577f3cc..6fe7347e9b70 100644 --- a/desktop/source/lib/init.cxx +++ b/desktop/source/lib/init.cxx @@ -2195,7 +2195,7 @@ static LibreOfficeKitDocument* lo_documentLoadWithOptions(LibreOfficeKit* pThis, LibLibreOffice_Impl* pLib = static_cast<LibLibreOffice_Impl*>(pThis); pLib->maLastExceptionMsg.clear(); - OUString aURL(getAbsoluteURL(pURL)); + const OUString aURL(getAbsoluteURL(pURL)); if (aURL.isEmpty()) { pLib->maLastExceptionMsg = "Filename to load was not provided."; @@ -2291,7 +2291,7 @@ static LibreOfficeKitDocument* lo_documentLoadWithOptions(LibreOfficeKit* pThis, LibLODocument_Impl* pDocument = new LibLODocument_Impl(xComponent, nDocumentIdCounter++); - // Do we know that after loading the document, its initial view is the "current" view? + // After loading the document, its initial view is the "current" view. SfxLokHelper::setDocumentIdOfView(pDocument->mnDocumentId); if (pLib->mpCallback) { diff --git a/include/sfx2/lokhelper.hxx b/include/sfx2/lokhelper.hxx index 79052a0d6e04..2aad968681b8 100644 --- a/include/sfx2/lokhelper.hxx +++ b/include/sfx2/lokhelper.hxx @@ -51,7 +51,7 @@ public: /// Set a view shell as current one. static void setView(int nId); /// Get the currently active view. - static int getView(SfxViewShell* pViewShell = nullptr); + static int getView(const SfxViewShell* pViewShell = nullptr); /// Get the number of views of the current object shell. static std::size_t getViewsCount(); /// Get viewIds of views of the current object shell. @@ -72,17 +72,24 @@ public: static LOKDeviceFormFactor getDeviceFormFactor(); /// Set the device form factor that should be used for a new view. static void setDeviceFormFactor(const OUString& rDeviceFormFactor); + /// Iterate over any view shell, except pThisViewShell, passing it to the f function. template<typename ViewShellType, typename FunctionType> static void forEachOtherView(ViewShellType* pThisViewShell, FunctionType f); + /// Invoke the LOK callback of all other views showing the same document as pThisView, with a payload of rKey-rPayload. - static void notifyOtherViews(SfxViewShell* pThisView, int nType, const OString& rKey, const OString& rPayload); + static void notifyOtherViews(const SfxViewShell* pThisView, int nType, const OString& rKey, + const OString& rPayload); /// Invoke the LOK callback of all views except pThisView, with a JSON payload created from the given property tree. - static void notifyOtherViews(SfxViewShell* pThisView, int nType, const boost::property_tree::ptree& rTree); + static void notifyOtherViews(const SfxViewShell* pThisView, int nType, + const boost::property_tree::ptree& rTree); /// Same as notifyOtherViews(), but works on a selected "other" view, not on all of them. - static void notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, const OString& rKey, const OString& rPayload); + static void notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView, + int nType, const OString& rKey, const OString& rPayload); /// Same as notifyOtherViews(), the property-tree version, but works on a selected "other" view, not on all of them. - static void notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, const boost::property_tree::ptree& rTree); + static void notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView, + int nType, const boost::property_tree::ptree& rTree); + /// Emits a LOK_CALLBACK_STATE_CHANGED static void sendUnoStatus(const SfxViewShell* pShell, const SfxPoolItem* pItem); /// Emits a LOK_CALLBACK_WINDOW diff --git a/sfx2/source/view/lokhelper.cxx b/sfx2/source/view/lokhelper.cxx index fa5b7e0bcb2d..1240209d9fb2 100644 --- a/sfx2/source/view/lokhelper.cxx +++ b/sfx2/source/view/lokhelper.cxx @@ -74,7 +74,7 @@ LOKDeviceFormFactor g_deviceFormFactor = LOKDeviceFormFactor::UNKNOWN; int SfxLokHelper::createView() { SfxViewFrame* pViewFrame = SfxViewFrame::GetFirst(); - if (!pViewFrame) + if (pViewFrame == nullptr) return -1; SfxViewShell* pPrevViewShell = SfxViewShell::Current(); ViewShellDocId nId; @@ -83,7 +83,7 @@ int SfxLokHelper::createView() SfxRequest aRequest(pViewFrame, SID_NEWWINDOW); pViewFrame->ExecView_Impl(aRequest); SfxViewShell* pViewShell = SfxViewShell::Current(); - if (!pViewShell) + if (pViewShell == nullptr) return -1; if (pPrevViewShell) pViewShell->SetDocId(nId); @@ -93,15 +93,15 @@ int SfxLokHelper::createView() void SfxLokHelper::destroyView(int nId) { SfxApplication* pApp = SfxApplication::Get(); - if (!pApp) + if (pApp == nullptr) return; - int nViewShellId = nId; + const ViewShellId nViewShellId(nId); SfxViewShellArr_Impl& rViewArr = pApp->GetViewShells_Impl(); - for (SfxViewShell* pViewShell : rViewArr) + for (const SfxViewShell* pViewShell : rViewArr) { - if (static_cast<sal_Int32>(pViewShell->GetViewShellId()) == nViewShellId) + if (pViewShell->GetViewShellId() == nViewShellId) { SfxViewFrame* pViewFrame = pViewShell->GetViewFrame(); SfxRequest aRequest(pViewFrame, SID_CLOSEWIN); @@ -114,15 +114,15 @@ void SfxLokHelper::destroyView(int nId) void SfxLokHelper::setView(int nId) { SfxApplication* pApp = SfxApplication::Get(); - if (!pApp) + if (pApp == nullptr) return; - int nViewShellId = nId; + const ViewShellId nViewShellId(nId); SfxViewShellArr_Impl& rViewArr = pApp->GetViewShells_Impl(); - for (SfxViewShell* pViewShell : rViewArr) + for (const SfxViewShell* pViewShell : rViewArr) { - if (static_cast<sal_Int32>(pViewShell->GetViewShellId()) == nViewShellId) + if (pViewShell->GetViewShellId() == nViewShellId) { DisableCallbacks dc; @@ -146,7 +146,7 @@ void SfxLokHelper::setView(int nId) } -int SfxLokHelper::getView(SfxViewShell* pViewShell) +int SfxLokHelper::getView(const SfxViewShell* pViewShell) { if (!pViewShell) pViewShell = SfxViewShell::Current(); @@ -291,38 +291,50 @@ static OString lcl_escapeQuotes(const OString &rStr) return aBuf.makeStringAndClear(); } -static OString lcl_generateJSON(SfxViewShell* pView, const boost::property_tree::ptree& rTree) +static OString lcl_generateJSON(const SfxViewShell* pView, const boost::property_tree::ptree& rTree) { + assert(pView != nullptr && "pView must be valid"); boost::property_tree::ptree aMessageProps = rTree; aMessageProps.put("viewId", SfxLokHelper::getView(pView)); aMessageProps.put("part", pView->getPart()); std::stringstream aStream; boost::property_tree::write_json(aStream, aMessageProps, false /* pretty */); - return OString(aStream.str().c_str()).trim(); + const std::string aString = aStream.str(); + return OString(aString.c_str(), aString.size()).trim(); } -void SfxLokHelper::notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, const OString& rKey, const OString& rPayload) +static inline OString lcl_generateJSON(const SfxViewShell* pView, const OString& rKey, + const OString& rPayload) { + assert(pView != nullptr && "pView must be valid"); + return OStringLiteral("{ \"viewId\": \"") + OString::number(SfxLokHelper::getView(pView)) + + "\", \"part\": \"" + OString::number(pView->getPart()) + "\", \"" + rKey + "\": \"" + + lcl_escapeQuotes(rPayload) + "\" }"; +} + +void SfxLokHelper::notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView, + int nType, const OString& rKey, const OString& rPayload) +{ + assert(pThisView != nullptr && "pThisView must be valid"); if (DisableCallbacks::disabled()) return; - OString aPayload = OStringLiteral("{ \"viewId\": \"") + OString::number(SfxLokHelper::getView(pThisView)) + - "\", \"part\": \"" + OString::number(pThisView->getPart()) + - "\", \"" + rKey + "\": \"" + lcl_escapeQuotes(rPayload) + "\" }"; - + const OString aPayload = lcl_generateJSON(pThisView, rKey, rPayload); pOtherView->libreOfficeKitViewCallback(nType, aPayload.getStr()); } -void SfxLokHelper::notifyOtherView(SfxViewShell* pThisView, SfxViewShell const* pOtherView, int nType, - const boost::property_tree::ptree& rTree) +void SfxLokHelper::notifyOtherView(const SfxViewShell* pThisView, SfxViewShell const* pOtherView, + int nType, const boost::property_tree::ptree& rTree) { + assert(pThisView != nullptr && "pThisView must be valid"); if (DisableCallbacks::disabled()) return; pOtherView->libreOfficeKitViewCallback(nType, lcl_generateJSON(pThisView, rTree).getStr()); } -void SfxLokHelper::notifyOtherViews(SfxViewShell* pThisView, int nType, const OString& rKey, const OString& rPayload) +void SfxLokHelper::notifyOtherViews(const SfxViewShell* pThisView, int nType, const OString& rKey, + const OString& rPayload) { if (DisableCallbacks::disabled()) return; @@ -337,7 +349,8 @@ void SfxLokHelper::notifyOtherViews(SfxViewShell* pThisView, int nType, const OS } } -void SfxLokHelper::notifyOtherViews(SfxViewShell* pThisView, int nType, const boost::property_tree::ptree& rTree) +void SfxLokHelper::notifyOtherViews(const SfxViewShell* pThisView, int nType, + const boost::property_tree::ptree& rTree) { if (SfxLokHelper::getViewsCount() <= 1 || DisableCallbacks::disabled()) return; @@ -399,26 +412,26 @@ void SfxLokHelper::notifyWindow(const SfxViewShell* pThisView, const OUString& rAction, const std::vector<vcl::LOKPayloadItem>& rPayload) { - assert(pThisView); + assert(pThisView != nullptr && "pThisView must be valid"); if (SfxLokHelper::getViewsCount() <= 0 || nLOKWindowId == 0 || DisableCallbacks::disabled()) return; OStringBuffer aPayload; - aPayload.append("{ \"id\": \"").append(OString::number(nLOKWindowId)).append("\""); - aPayload.append(", \"action\": \"").append(OUStringToOString(rAction, RTL_TEXTENCODING_UTF8)).append("\""); + aPayload.append("{ \"id\": \"").append(OString::number(nLOKWindowId)).append('"'); + aPayload.append(", \"action\": \"").append(OUStringToOString(rAction, RTL_TEXTENCODING_UTF8)).append('"'); for (const auto& rItem: rPayload) { if (!rItem.first.isEmpty() && !rItem.second.isEmpty()) { aPayload.append(", \"").append(rItem.first).append("\": \"") - .append(rItem.second).append("\""); + .append(rItem.second).append('"'); } } - aPayload.append("}"); + aPayload.append('}'); - auto s = aPayload.makeStringAndClear(); + const OString s = aPayload.makeStringAndClear(); pThisView->libreOfficeKitViewCallback(LOK_CALLBACK_WINDOW, s.getStr()); } diff --git a/sw/source/core/crsr/crsrsh.cxx b/sw/source/core/crsr/crsrsh.cxx index 4e89e80d711b..1aba8c3ac22f 100644 --- a/sw/source/core/crsr/crsrsh.cxx +++ b/sw/source/core/crsr/crsrsh.cxx @@ -2394,7 +2394,7 @@ void SwCursorShell::ShowCursor() if (comphelper::LibreOfficeKit::isActive()) { - OString aPayload = OString::boolean(m_bSVCursorVis); + const OString aPayload = OString::boolean(m_bSVCursorVis); GetSfxViewShell()->libreOfficeKitViewCallback(LOK_CALLBACK_CURSOR_VISIBLE, aPayload.getStr()); SfxLokHelper::notifyOtherViews(GetSfxViewShell(), LOK_CALLBACK_VIEW_CURSOR_VISIBLE, "visible", aPayload); } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits