Simplify Qt external editors (Linguist, Designer)

Remove unneeded extra classes, and unneeded parameters.

Change-Id: Ib4ceccd7d92bd8ca5de122e174f2579a2a8a9390
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

src/plugins/qmakeprojectmanager/externaleditors.cpp

@@ -61,27 +61,32 @@ static inline QString msgAppNotFound(const QString &id)
 }
 
 // -- Commands and helpers
-static QString linguistBinary()
+static QString linguistBinary(const QtSupport::BaseQtVersion *qtVersion)
 {
+    if (qtVersion)
+        return qtVersion->linguistCommand();
     return QLatin1String(Utils::HostOsInfo::isMacHost() ? "Linguist" : "linguist");
 }
 
-static QString designerBinary()
+static QString designerBinary(const QtSupport::BaseQtVersion *qtVersion)
 {
+    if (qtVersion)
+        return qtVersion->designerCommand();
     return QLatin1String(Utils::HostOsInfo::isMacHost() ? "Designer" : "designer");
 }
 
 // Mac: Change the call 'Foo.app/Contents/MacOS/Foo <filelist>' to
 // 'open -a Foo.app <filelist>'. doesn't support generic command line arguments
-static void createMacOpenCommand(QString *binary, QStringList *arguments)
+static ExternalQtEditor::LaunchData createMacOpenCommand(const ExternalQtEditor::LaunchData &data)
 {
-    const int appFolderIndex = binary->lastIndexOf(QLatin1String("/Contents/MacOS/"));
+    ExternalQtEditor::LaunchData openData = data;
+    const int appFolderIndex = data.binary.lastIndexOf(QLatin1String("/Contents/MacOS/"));
     if (appFolderIndex != -1) {
-        binary->truncate(appFolderIndex);
+        openData.binary = "open";
-        arguments->push_front(*binary);
+        openData.arguments = QStringList({ QString("-a"), data.binary.left(appFolderIndex) })
-        arguments->push_front(QLatin1String("-a"));
+                + data.arguments;
-        *binary = QLatin1String("open");
     }
+    return openData;
 }
 
 static const char designerIdC[] = "Qt.Designer";
@@ -94,14 +99,34 @@ static const char linguistDisplayName[] = QT_TRANSLATE_NOOP("OpenWith::Editors",
 ExternalQtEditor::ExternalQtEditor(Core::Id id,
                                    const QString &displayName,
                                    const QString &mimetype,
-                                   QObject *parent) :
+                                   const CommandForQtVersion &commandForQtVersion) :
-    Core::IExternalEditor(parent),
     m_mimeTypes(mimetype),
     m_id(id),
-    m_displayName(displayName)
+    m_displayName(displayName),
+    m_commandForQtVersion(commandForQtVersion)
 {
 }
 
+ExternalQtEditor *ExternalQtEditor::createLinguistEditor()
+{
+    return new ExternalQtEditor(linguistIdC,
+                                QLatin1String(linguistDisplayName),
+                                QLatin1String(ProjectExplorer::Constants::LINGUIST_MIMETYPE),
+                                linguistBinary);
+}
+
+ExternalQtEditor *ExternalQtEditor::createDesignerEditor()
+{
+    if (Utils::HostOsInfo::isMacHost()) {
+        return new ExternalQtEditor(designerIdC,
+                                    QLatin1String(designerDisplayName),
+                                    QLatin1String(ProjectExplorer::Constants::FORM_MIMETYPE),
+                                    designerBinary);
+    } else {
+        return new DesignerExternalEditor;
+    }
+}
+
 QStringList ExternalQtEditor::mimeTypes() const
 {
     return m_mimeTypes;
@@ -118,41 +143,43 @@ QString ExternalQtEditor::displayName() const
 }
 
 bool ExternalQtEditor::getEditorLaunchData(const QString &fileName,
-                                           QtVersionCommandAccessor commandAccessor,
+                                           LaunchData *data,
-                                           const QString &fallbackBinary,
-                                           const QStringList &additionalArguments,
-                                           bool useMacOpenCommand,
-                                           EditorLaunchData *data,
                                            QString *errorMessage) const
 {
     // Get the binary either from the current Qt version of the project or Path
     if (Project *project = SessionManager::projectForFile(Utils::FileName::fromString(fileName))) {
         if (const Target *target = project->activeTarget()) {
             if (const QtSupport::BaseQtVersion *qtVersion = QtSupport::QtKitInformation::qtVersion(target->kit())) {
-                data->binary = (qtVersion->*commandAccessor)();
+                data->binary = m_commandForQtVersion(qtVersion);
                 data->workingDirectory = project->projectDirectory().toString();
             }
         }
     }
     if (data->binary.isEmpty()) {
         data->workingDirectory.clear();
-        data->binary = Utils::SynchronousProcess::locateBinary(fallbackBinary);
+        data->binary = Utils::SynchronousProcess::locateBinary(m_commandForQtVersion(nullptr));
     }
     if (data->binary.isEmpty()) {
         *errorMessage = msgAppNotFound(id().toString());
         return false;
     }
     // Setup binary + arguments, use Mac Open if appropriate
-    data->arguments = additionalArguments;
     data->arguments.push_back(fileName);
-    if (Utils::HostOsInfo::isMacHost() && useMacOpenCommand)
+    if (Utils::HostOsInfo::isMacHost())
-        createMacOpenCommand(&(data->binary), &(data->arguments));
+        *data = createMacOpenCommand(*data);
     if (debug)
         qDebug() << Q_FUNC_INFO << '\n' << data->binary << data->arguments;
     return true;
 }
 
-bool ExternalQtEditor::startEditorProcess(const EditorLaunchData &data, QString *errorMessage)
+bool ExternalQtEditor::startEditor(const QString &fileName, QString *errorMessage)
+{
+    LaunchData data;
+    return getEditorLaunchData(fileName, &data, errorMessage)
+            && startEditorProcess(data, errorMessage);
+}
+
+bool ExternalQtEditor::startEditorProcess(const LaunchData &data, QString *errorMessage)
 {
     if (debug)
         qDebug() << Q_FUNC_INFO << '\n' << data.binary << data.arguments << data.workingDirectory;
@@ -164,47 +191,12 @@ bool ExternalQtEditor::startEditorProcess(const EditorLaunchData &data, QString
     return true;
 }
 
-// --------------- LinguistExternalEditor
-LinguistExternalEditor::LinguistExternalEditor(QObject *parent) :
-       ExternalQtEditor(linguistIdC,
-                        QLatin1String(linguistDisplayName),
-                        QLatin1String(ProjectExplorer::Constants::LINGUIST_MIMETYPE),
-                        parent)
-{
-}
-
-bool LinguistExternalEditor::startEditor(const QString &fileName, QString *errorMessage)
-{
-    EditorLaunchData data;
-    return getEditorLaunchData(fileName, &QtSupport::BaseQtVersion::linguistCommand,
-                            linguistBinary(), QStringList(), true, &data, errorMessage)
-            && startEditorProcess(data, errorMessage);
-}
-
-// --------------- MacDesignerExternalEditor, using Mac 'open'
-MacDesignerExternalEditor::MacDesignerExternalEditor(QObject *parent) :
-       ExternalQtEditor(designerIdC,
-                        QLatin1String(designerDisplayName),
-                        QLatin1String(ProjectExplorer::Constants::FORM_MIMETYPE),
-                        parent)
-{
-}
-
-bool MacDesignerExternalEditor::startEditor(const QString &fileName, QString *errorMessage)
-{
-    EditorLaunchData data;
-    return getEditorLaunchData(fileName, &QtSupport::BaseQtVersion::designerCommand,
-                            designerBinary(), QStringList(), true, &data, errorMessage)
-        && startEditorProcess(data, errorMessage);
-    return false;
-}
-
 // --------------- DesignerExternalEditor with Designer Tcp remote control.
-DesignerExternalEditor::DesignerExternalEditor(QObject *parent) :
+DesignerExternalEditor::DesignerExternalEditor() :
     ExternalQtEditor(designerIdC,
                      QLatin1String(designerDisplayName),
                      QLatin1String(Designer::Constants::FORM_MIMETYPE),
-                     parent)
+                     designerBinary)
 {
 }
 
@@ -225,10 +217,9 @@ void DesignerExternalEditor::processTerminated(const QString &binary)
 
 bool DesignerExternalEditor::startEditor(const QString &fileName, QString *errorMessage)
 {
-    EditorLaunchData data;
+    LaunchData data;
     // Find the editor binary
-    if (!getEditorLaunchData(fileName, &QtSupport::BaseQtVersion::designerCommand,
+    if (!getEditorLaunchData(fileName, &data, errorMessage)) {
-                            designerBinary(), QStringList(), false, &data, errorMessage)) {
         return false;
     }
     // Known one?

src/plugins/qmakeprojectmanager/externaleditors.h

@@ -52,64 +52,46 @@ class ExternalQtEditor : public Core::IExternalEditor
     Q_OBJECT
 
 public:
-    virtual QStringList mimeTypes() const;
+    static ExternalQtEditor *createLinguistEditor();
-    virtual Core::Id id() const;
+    static ExternalQtEditor *createDesignerEditor();
-    virtual QString displayName() const;
 
-protected:
+    QStringList mimeTypes() const override;
-    // Member function pointer for a QtVersion function return a string (command)
+    Core::Id id() const override;
-    typedef QString (QtSupport::BaseQtVersion::*QtVersionCommandAccessor)() const;
+    QString displayName() const override;
+
+    bool startEditor(const QString &fileName, QString *errorMessage) override;
 
     // Data required to launch the editor
-    struct EditorLaunchData {
+    struct LaunchData {
         QString binary;
         QStringList arguments;
         QString workingDirectory;
     };
 
-    explicit ExternalQtEditor(Core::Id id,
+protected:
+    // Member function pointer for a QtVersion function return a string (command)
+    using CommandForQtVersion = std::function<QString(const QtSupport::BaseQtVersion *)>;
+
+    ExternalQtEditor(Core::Id id,
                      const QString &displayName,
                      const QString &mimetype,
-                              QObject *parent = 0);
+                     const CommandForQtVersion &commandForQtVersion);
 
     // Try to retrieve the binary of the editor from the Qt version,
     // prepare arguments accordingly (Mac "open" if desired)
     bool getEditorLaunchData(const QString &fileName,
-                             QtVersionCommandAccessor commandAccessor,
+                             LaunchData *data,
-                             const QString &fallbackBinary,
-                             const QStringList &additionalArguments,
-                             bool useMacOpenCommand,
-                             EditorLaunchData *data,
                              QString *errorMessage) const;
 
     // Create and start a detached GUI process executing in the background.
     // Set the project environment if there is one.
-    bool startEditorProcess(const EditorLaunchData &data, QString *errorMessage);
+    bool startEditorProcess(const LaunchData &data, QString *errorMessage);
 
 private:
     const QStringList m_mimeTypes;
     const Core::Id m_id;
     const QString m_displayName;
-};
+    const CommandForQtVersion m_commandForQtVersion;
-
-// Qt Linguist
-class LinguistExternalEditor : public ExternalQtEditor
-{
-    Q_OBJECT
-public:
-    explicit LinguistExternalEditor(QObject *parent = 0);
-    virtual bool startEditor(const QString &fileName, QString *errorMessage);
-};
-
-// Qt Designer on Mac: Make use of the Mac's 'open' mechanism to
-// ensure files are opened in the same (per version) instance.
-
-class MacDesignerExternalEditor : public ExternalQtEditor
-{
-    Q_OBJECT
-public:
-    explicit MacDesignerExternalEditor(QObject *parent = 0);
-    virtual bool startEditor(const QString &fileName, QString *errorMessage);
 };
 
 /* Qt Designer on the remaining platforms: Uses Designer's own
@@ -120,9 +102,9 @@ class DesignerExternalEditor : public ExternalQtEditor
 {
     Q_OBJECT
 public:
-    explicit DesignerExternalEditor(QObject *parent = 0);
+    DesignerExternalEditor();
 
-    virtual bool startEditor(const QString &fileName, QString *errorMessage);
+    bool startEditor(const QString &fileName, QString *errorMessage) override;
 
 private:
     void processTerminated(const QString &binary);

src/plugins/qmakeprojectmanager/qmakeprojectmanagerplugin.cpp

@@ -111,11 +111,8 @@ bool QmakeProjectManagerPlugin::initialize(const QStringList &arguments, QString
     addAutoReleasedObject(new QmakeBuildConfigurationFactory);
     addAutoReleasedObject(new DesktopQmakeRunConfigurationFactory);
 
-    if (Utils::HostOsInfo::isMacHost())
+    addAutoReleasedObject(ExternalQtEditor::createDesignerEditor());
-        addAutoReleasedObject(new MacDesignerExternalEditor);
+    addAutoReleasedObject(ExternalQtEditor::createLinguistEditor());
-    else
-        addAutoReleasedObject(new DesignerExternalEditor);
-    addAutoReleasedObject(new LinguistExternalEditor);
 
     addAutoReleasedObject(new ProFileEditorFactory);