From ca89d1b8a62c800f9a336685a353768a90fd303d Mon Sep 17 00:00:00 2001 From: hjk Date: Wed, 5 Feb 2020 15:50:46 +0100 Subject: [PATCH] DiffEditor: Pimpl plugin The standard pattern, and allows to drop the QObject parent on the editor factory which gets into the way of de-QObject-ifing the IEditorFactory hierarchy. Change-Id: I5c6a50f8075a99592ed4459b417ca8ba7471ae96 Reviewed-by: Christian Stenger --- src/plugins/diffeditor/diffeditorfactory.cpp | 3 +- src/plugins/diffeditor/diffeditorfactory.h | 2 +- src/plugins/diffeditor/diffeditorplugin.cpp | 80 +++++++++++++------- src/plugins/diffeditor/diffeditorplugin.h | 24 ++---- 4 files changed, 61 insertions(+), 48 deletions(-) diff --git a/src/plugins/diffeditor/diffeditorfactory.cpp b/src/plugins/diffeditor/diffeditorfactory.cpp index c02199c6293..12a1d58ff9f 100644 --- a/src/plugins/diffeditor/diffeditorfactory.cpp +++ b/src/plugins/diffeditor/diffeditorfactory.cpp @@ -36,8 +36,7 @@ namespace DiffEditor { namespace Internal { -DiffEditorFactory::DiffEditorFactory(QObject *parent) - : IEditorFactory(parent) +DiffEditorFactory::DiffEditorFactory() { setId(Constants::DIFF_EDITOR_ID); setDisplayName(QCoreApplication::translate("DiffEditorFactory", Constants::DIFF_EDITOR_DISPLAY_NAME)); diff --git a/src/plugins/diffeditor/diffeditorfactory.h b/src/plugins/diffeditor/diffeditorfactory.h index 64795b1971d..3f9fbfac645 100644 --- a/src/plugins/diffeditor/diffeditorfactory.h +++ b/src/plugins/diffeditor/diffeditorfactory.h @@ -33,7 +33,7 @@ namespace Internal { class DiffEditorFactory : public Core::IEditorFactory { public: - explicit DiffEditorFactory(QObject *parent); + DiffEditorFactory(); }; } // namespace Internal diff --git a/src/plugins/diffeditor/diffeditorplugin.cpp b/src/plugins/diffeditor/diffeditorplugin.cpp index a4372ae0b1e..ff405524e35 100644 --- a/src/plugins/diffeditor/diffeditorplugin.cpp +++ b/src/plugins/diffeditor/diffeditorplugin.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include @@ -47,6 +46,7 @@ #include #include +#include "texteditor/texteditoractionhandler.h" #include #include @@ -54,6 +54,7 @@ #include using namespace Core; +using namespace TextEditor; using namespace Utils; namespace DiffEditor { @@ -421,10 +422,7 @@ static TextEditor::TextDocument *currentTextDocument() EditorManager::currentDocument()); } -DiffEditorServiceImpl::DiffEditorServiceImpl(QObject *parent) : - QObject(parent) -{ -} +DiffEditorServiceImpl::DiffEditorServiceImpl() = default; void DiffEditorServiceImpl::diffFiles(const QString &leftFileName, const QString &rightFileName) { @@ -458,11 +456,28 @@ void DiffEditorServiceImpl::diffModifiedFiles(const QStringList &fileNames) document->reload(); } -bool DiffEditorPlugin::initialize(const QStringList &arguments, QString *errorMessage) +class DiffEditorPluginPrivate : public QObject { - Q_UNUSED(arguments) - Q_UNUSED(errorMessage) + Q_DECLARE_TR_FUNCTIONS(DiffEditor::Internal::DiffEditorPlugin) +public: + DiffEditorPluginPrivate(); + + void updateDiffCurrentFileAction(); + void updateDiffOpenFilesAction(); + void diffCurrentFile(); + void diffOpenFiles(); + void diffExternalFiles(); + + QAction *m_diffCurrentFileAction = nullptr; + QAction *m_diffOpenFilesAction = nullptr; + + DiffEditorFactory editorFactory; + DiffEditorServiceImpl service; +}; + +DiffEditorPluginPrivate::DiffEditorPluginPrivate() +{ //register actions ActionContainer *toolsContainer = ActionManager::actionContainer(Core::Constants::M_TOOLS); @@ -474,51 +489,43 @@ bool DiffEditorPlugin::initialize(const QStringList &arguments, QString *errorMe m_diffCurrentFileAction = new QAction(tr("Diff Current File"), this); Command *diffCurrentFileCommand = ActionManager::registerAction(m_diffCurrentFileAction, "DiffEditor.DiffCurrentFile"); diffCurrentFileCommand->setDefaultKeySequence(QKeySequence(useMacShortcuts ? tr("Meta+H") : tr("Ctrl+H"))); - connect(m_diffCurrentFileAction, &QAction::triggered, this, &DiffEditorPlugin::diffCurrentFile); + connect(m_diffCurrentFileAction, &QAction::triggered, this, &DiffEditorPluginPrivate::diffCurrentFile); diffContainer->addAction(diffCurrentFileCommand); m_diffOpenFilesAction = new QAction(tr("Diff Open Files"), this); Command *diffOpenFilesCommand = ActionManager::registerAction(m_diffOpenFilesAction, "DiffEditor.DiffOpenFiles"); diffOpenFilesCommand->setDefaultKeySequence(QKeySequence(useMacShortcuts ? tr("Meta+Shift+H") : tr("Ctrl+Shift+H"))); - connect(m_diffOpenFilesAction, &QAction::triggered, this, &DiffEditorPlugin::diffOpenFiles); + connect(m_diffOpenFilesAction, &QAction::triggered, this, &DiffEditorPluginPrivate::diffOpenFiles); diffContainer->addAction(diffOpenFilesCommand); QAction *diffExternalFilesAction = new QAction(tr("Diff External Files..."), this); Command *diffExternalFilesCommand = ActionManager::registerAction(diffExternalFilesAction, "DiffEditor.DiffExternalFiles"); - connect(diffExternalFilesAction, &QAction::triggered, this, &DiffEditorPlugin::diffExternalFiles); + connect(diffExternalFilesAction, &QAction::triggered, this, &DiffEditorPluginPrivate::diffExternalFiles); diffContainer->addAction(diffExternalFilesCommand); connect(EditorManager::instance(), &EditorManager::currentEditorChanged, - this, &DiffEditorPlugin::updateDiffCurrentFileAction); + this, &DiffEditorPluginPrivate::updateDiffCurrentFileAction); connect(EditorManager::instance(), &EditorManager::currentDocumentStateChanged, - this, &DiffEditorPlugin::updateDiffCurrentFileAction); + this, &DiffEditorPluginPrivate::updateDiffCurrentFileAction); connect(EditorManager::instance(), &EditorManager::editorOpened, - this, &DiffEditorPlugin::updateDiffOpenFilesAction); + this, &DiffEditorPluginPrivate::updateDiffOpenFilesAction); connect(EditorManager::instance(), &EditorManager::editorsClosed, - this, &DiffEditorPlugin::updateDiffOpenFilesAction); + this, &DiffEditorPluginPrivate::updateDiffOpenFilesAction); connect(EditorManager::instance(), &EditorManager::documentStateChanged, - this, &DiffEditorPlugin::updateDiffOpenFilesAction); + this, &DiffEditorPluginPrivate::updateDiffOpenFilesAction); updateDiffCurrentFileAction(); updateDiffOpenFilesAction(); - - new DiffEditorFactory(this); - new DiffEditorServiceImpl(this); - - return true; } -void DiffEditorPlugin::extensionsInitialized() -{ } - -void DiffEditorPlugin::updateDiffCurrentFileAction() +void DiffEditorPluginPrivate::updateDiffCurrentFileAction() { auto textDocument = currentTextDocument(); const bool enabled = textDocument && textDocument->isModified(); m_diffCurrentFileAction->setEnabled(enabled); } -void DiffEditorPlugin::updateDiffOpenFilesAction() +void DiffEditorPluginPrivate::updateDiffOpenFilesAction() { const bool enabled = Utils::anyOf(DocumentModel::openedDocuments(), [](IDocument *doc) { return doc->isModified() && qobject_cast(doc); @@ -526,7 +533,7 @@ void DiffEditorPlugin::updateDiffOpenFilesAction() m_diffOpenFilesAction->setEnabled(enabled); } -void DiffEditorPlugin::diffCurrentFile() +void DiffEditorPluginPrivate::diffCurrentFile() { auto textDocument = currentTextDocument(); if (!textDocument) @@ -551,7 +558,7 @@ void DiffEditorPlugin::diffCurrentFile() document->reload(); } -void DiffEditorPlugin::diffOpenFiles() +void DiffEditorPluginPrivate::diffOpenFiles() { const QString documentId = Constants::DIFF_EDITOR_PLUGIN + QLatin1String(".DiffOpenFiles"); @@ -567,7 +574,7 @@ void DiffEditorPlugin::diffOpenFiles() document->reload(); } -void DiffEditorPlugin::diffExternalFiles() +void DiffEditorPluginPrivate::diffExternalFiles() { const QString fileName1 = QFileDialog::getOpenFileName(ICore::dialogParent(), tr("Select First File for Diff"), @@ -599,6 +606,21 @@ void DiffEditorPlugin::diffExternalFiles() document->reload(); } +DiffEditorPlugin::~DiffEditorPlugin() +{ + delete d; +} + +bool DiffEditorPlugin::initialize(const QStringList &arguments, QString *errorMessage) +{ + d = new DiffEditorPluginPrivate; + + Q_UNUSED(arguments) + Q_UNUSED(errorMessage) + + return true; +} + } // namespace Internal } // namespace DiffEditor diff --git a/src/plugins/diffeditor/diffeditorplugin.h b/src/plugins/diffeditor/diffeditorplugin.h index 7ec3e2d392e..68179469863 100644 --- a/src/plugins/diffeditor/diffeditorplugin.h +++ b/src/plugins/diffeditor/diffeditorplugin.h @@ -30,10 +30,6 @@ #include #include -QT_FORWARD_DECLARE_CLASS(QAction) - -namespace Core { class IEditor; } - namespace DiffEditor { namespace Internal { @@ -41,31 +37,27 @@ class DiffEditorServiceImpl : public QObject, public Core::DiffService { Q_OBJECT Q_INTERFACES(Core::DiffService) + public: - explicit DiffEditorServiceImpl(QObject *parent = nullptr); + DiffEditorServiceImpl(); void diffFiles(const QString &leftFileName, const QString &rightFileName) override; void diffModifiedFiles(const QStringList &fileNames) override; }; -class DiffEditorPlugin : public ExtensionSystem::IPlugin +class DiffEditorPlugin final : public ExtensionSystem::IPlugin { Q_OBJECT Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QtCreatorPlugin" FILE "DiffEditor.json") public: - bool initialize(const QStringList &arguments, QString *errorMessage = nullptr) override; - void extensionsInitialized() override; + ~DiffEditorPlugin() final; + + bool initialize(const QStringList &arguments, QString *errorMessage) final; + void extensionsInitialized() final {}; private: - void updateDiffCurrentFileAction(); - void updateDiffOpenFilesAction(); - void diffCurrentFile(); - void diffOpenFiles(); - void diffExternalFiles(); - - QAction *m_diffCurrentFileAction = nullptr; - QAction *m_diffOpenFilesAction = nullptr; + class DiffEditorPluginPrivate *d = nullptr; #ifdef WITH_TESTS private slots: