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 <christian.stenger@qt.io>

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));

src/plugins/diffeditor/diffeditorfactory.h

@@ -33,7 +33,7 @@ namespace Internal {
 class DiffEditorFactory : public Core::IEditorFactory
 {
 public:
-    explicit DiffEditorFactory(QObject *parent);
+    DiffEditorFactory();
 };
 
 } // namespace Internal

src/plugins/diffeditor/diffeditorplugin.cpp

@@ -35,7 +35,6 @@
 #include <QFutureWatcher>
 #include <QMenu>
 #include <QTextCodec>
-#include <QtPlugin>
 
 #include <coreplugin/actionmanager/actioncontainer.h>
 #include <coreplugin/actionmanager/actionmanager.h>
@@ -47,6 +46,7 @@
 
 #include <texteditor/textdocument.h>
 #include <texteditor/texteditor.h>
+#include "texteditor/texteditoractionhandler.h"
 
 #include <utils/algorithm.h>
 #include <utils/differ.h>
@@ -54,6 +54,7 @@
 #include <utils/qtcassert.h>
 
 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<TextEditor::TextDocument *>(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
 

src/plugins/diffeditor/diffeditorplugin.h

@@ -30,10 +30,6 @@
 #include <coreplugin/diffservice.h>
 #include <extensionsystem/iplugin.h>
 
-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: