Core: Return Result<> from IEditorFactory::startEditor()

Change-Id: Ibca35716497ea62e92957db115130a841aff360a
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/coreplugin/editormanager/editormanager.cpp

@@ -929,8 +929,10 @@ IEditor *EditorManagerPrivate::openEditor(EditorView *view, const FilePath &file
             QTC_ASSERT(factory->isExternalEditor(),
                        factory = factories.isEmpty() ? nullptr : factories.takeFirst();
                        continue);
-            if (factory->startEditor(filePath, &errorString))
+            const Result<> res = factory->startEditor(filePath);
+            if (res)
                 break;
+            errorString = res.error();
         }
 
         if (errorString.isEmpty())
@@ -3293,13 +3295,13 @@ bool EditorManager::openExternalEditor(const FilePath &filePath, Id editorId)
 
     if (!ee)
         return false;
-    QString errorMessage;
+
     QApplication::setOverrideCursor(QCursor(Qt::WaitCursor));
-    const bool ok = ee->startEditor(filePath, &errorMessage);
+    const Result<> res = ee->startEditor(filePath);
     QApplication::restoreOverrideCursor();
-    if (!ok)
+    if (!res)
-        QMessageBox::critical(ICore::dialogParent(), ::Core::Tr::tr("Opening File"), errorMessage);
+        QMessageBox::critical(ICore::dialogParent(), ::Core::Tr::tr("Opening File"), res.error());
-    return ok;
+    return res.has_value();
 }
 
 /*!

src/plugins/coreplugin/editormanager/ieditorfactory.cpp

@@ -298,10 +298,10 @@ IEditor *IEditorFactory::createEditor() const
 
     \sa setEditorStarter()
 */
-bool IEditorFactory::startEditor(const FilePath &filePath, QString *errorMessage)
+Result<> IEditorFactory::startEditor(const FilePath &filePath)
 {
-    QTC_ASSERT(m_starter, return false);
+    QTC_ASSERT(m_starter, return ResultError(ResultAssert));
-    return m_starter(filePath, errorMessage);
+    return m_starter(filePath);
 }
 
 /*!
@@ -322,8 +322,6 @@ void IEditorFactory::setEditorCreator(const std::function<IEditor *()> &creator)
 }
 
 /*!
-    \fn void Core::IEditorFactory::setEditorStarter(const std::function<bool(const Utils::FilePath &, QString *)> &starter);
-
     Sets the function that is used to open a file for a given \c FilePath to
     \a starter.
 
@@ -332,7 +330,7 @@ void IEditorFactory::setEditorCreator(const std::function<IEditor *()> &creator)
 
     This is mutually exclusive with the use of setEditorCreator().
 */
-void IEditorFactory::setEditorStarter(const std::function<bool(const FilePath &, QString *)> &starter)
+void IEditorFactory::setEditorStarter(const std::function<Result<>(const FilePath &)> &starter)
 {
     QTC_CHECK(!m_starter);
     QTC_CHECK(!m_creator);

src/plugins/coreplugin/editormanager/ieditorfactory.h

@@ -6,6 +6,7 @@
 #include "../core_global.h"
 
 #include <utils/id.h>
+#include <utils/result.h>
 
 #include <QStringList>
 
@@ -42,7 +43,7 @@ public:
     bool isExternalEditor() const;
 
     IEditor *createEditor() const;
-    bool startEditor(const Utils::FilePath &filePath, QString *errorMessage);
+    Utils::Result<> startEditor(const Utils::FilePath &filePath);
 
 protected:
     IEditorFactory();
@@ -52,14 +53,14 @@ protected:
     void setMimeTypes(const QStringList &mimeTypes) { m_mimeTypes = mimeTypes; }
     void addMimeType(const QString &mimeType) { m_mimeTypes.append(mimeType); }
     void setEditorCreator(const std::function<IEditor *()> &creator);
-    void setEditorStarter(const std::function<bool(const Utils::FilePath &, QString *)> &starter);
+    void setEditorStarter(const std::function<Utils::Result<>(const Utils::FilePath &)> &starter);
 
 private:
     Utils::Id m_id;
     QString m_displayName;
     QStringList m_mimeTypes;
     std::function<IEditor *()> m_creator;
-    std::function<bool(const Utils::FilePath &, QString *)> m_starter;
+    std::function<Utils::Result<>(const Utils::FilePath &)> m_starter;
 };
 
 } // namespace Core

src/plugins/coreplugin/editormanager/systemeditor.cpp

@@ -25,17 +25,13 @@ public:
         setDisplayName(Tr::tr("System Editor"));
         setMimeTypes({Utils::Constants::OCTET_STREAM_MIMETYPE});
 
-        setEditorStarter([](const FilePath &filePath, QString *errorMessage) {
+        setEditorStarter([](const FilePath &filePath) -> Result<> {
-            Q_UNUSED(errorMessage)
             QUrl url;
             url.setPath(filePath.toUrlishString());
             url.setScheme(QLatin1String("file"));
-            if (!QDesktopServices::openUrl(url)) {
+            if (!QDesktopServices::openUrl(url))
-                if (errorMessage)
+                return ResultError(Tr::tr("Could not open URL %1.").arg(url.toString()));
-                    *errorMessage = Tr::tr("Could not open URL %1.").arg(url.toString());
+            return ResultOk;
-                return false;
-            }
-            return true;
         });
     }
 };

src/plugins/qmlprojectmanager/qmlprojectplugin.cpp

@@ -205,9 +205,9 @@ public:
         setId("Qt.QtDesignStudio");
         setDisplayName(Tr::tr("Qt Design Studio"));
         setMimeTypes({Utils::Constants::QMLUI_MIMETYPE});
-        setEditorStarter([](const FilePath &filePath, [[maybe_unused]] QString *errorMessage) {
+        setEditorStarter([](const FilePath &filePath) {
             openInQds(filePath);
-            return true;
+            return ResultOk;
         });
     }
 };

src/plugins/qtsupport/externaleditors.cpp

@@ -163,10 +163,9 @@ static QString findFirstCommand(const QList<QtSupport::QtVersion *> &qtVersions,
     return QString();
 }
 
-static bool getEditorLaunchData(const CommandForQtVersion &commandForQtVersion,
+static Result<> getEditorLaunchData(const CommandForQtVersion &commandForQtVersion,
                                     const FilePath &filePath,
-                                LaunchData *data,
+                                    LaunchData *data)
-                                QString *errorMessage)
 {
     // Check in order for Qt version with the binary:
     // - active kit of project
@@ -174,10 +173,9 @@ static bool getEditorLaunchData(const CommandForQtVersion &commandForQtVersion,
     // - default kit
     // - any other kit
     // As fallback check PATH
-    if (!KitManager::waitForLoaded()) {
+    if (!KitManager::waitForLoaded())
-        *errorMessage = Tr::tr("Could not load kits in a reasonable amount of time.");
+        return ResultError(Tr::tr("Could not load kits in a reasonable amount of time."));
-        return false;
+
-    }
     data->workingDirectory.clear();
     QList<QtSupport::QtVersion *> qtVersionsToCheck; // deduplicated after being filled
     if (const Project *project = ProjectManager::projectForFile(filePath)) {
@@ -202,9 +200,8 @@ static bool getEditorLaunchData(const CommandForQtVersion &commandForQtVersion,
     }
 
     if (data->binary.isEmpty()) {
-        *errorMessage = Tr::tr("The application \"%1\" could not be found.")
+        return ResultError(Tr::tr("The application \"%1\" could not be found.")
-            .arg(filePath.toUserOutput());
+            .arg(filePath.toUserOutput()));
-        return false;
     }
 
     // Setup binary + arguments, use Mac Open if appropriate
@@ -213,18 +210,17 @@ static bool getEditorLaunchData(const CommandForQtVersion &commandForQtVersion,
         *data = createMacOpenCommand(*data);
     if (debug)
         qDebug() << Q_FUNC_INFO << '\n' << data->binary << data->arguments;
-    return true;
+    return ResultOk;
 }
 
-static bool startEditorProcess(const LaunchData &data, QString *errorMessage)
+Result<> startEditorProcess(const LaunchData &data)
 {
     if (debug)
         qDebug() << Q_FUNC_INFO << '\n' << data.binary << data.arguments << data.workingDirectory;
     const CommandLine cmd{FilePath::fromString(data.binary), data.arguments};
-    if (Process::startDetached(cmd, data.workingDirectory))
+    if (!Process::startDetached(cmd, data.workingDirectory))
-        return true;
+        return  ResultError(Tr::tr("Unable to start \"%1\".").arg(cmd.toUserOutput()));
-    *errorMessage = Tr::tr("Unable to start \"%1\".").arg(cmd.toUserOutput());
+    return ResultOk;
-    return false;
 }
 
 // ExternalDesignerEditorFactory with Designer Tcp remote control.
@@ -258,15 +254,15 @@ public:
         setDisplayName(::Core::Tr::tr("Qt Widgets Designer"));
         setMimeTypes({Utils::Constants::FORM_MIMETYPE});
 
-        setEditorStarter([guard](const FilePath &filePath, QString *errorMessage) {
+        setEditorStarter([guard](const FilePath &filePath) -> Result<> {
             LaunchData data;
 
             // Find the editor binary
-            if (!getEditorLaunchData(designerBinary, filePath, &data, errorMessage))
+            if (const Result<> res = getEditorLaunchData(designerBinary, filePath, &data); !res)
-                return false;
+                return res;
 
             if (HostOsInfo::isMacHost())
-                return startEditorProcess(data, errorMessage);
+                return startEditorProcess(data);
 
             /* Qt Widgets Designer on the remaining platforms: Uses Designer's own
             * Tcp-based communication mechanism to ensure all files are opened
@@ -280,18 +276,16 @@ public:
                     qDebug() << Q_FUNC_INFO << "\nWriting to socket:" << data.binary << filePath;
                 QTcpSocket *socket = it.value();
                 if (!socket->write(filePath.toUrlishString().toUtf8() + '\n')) {
-                    *errorMessage = Tr::tr("Qt Widgets Designer is not responding (%1).")
+                    return ResultError(Tr::tr("Qt Widgets Designer is not responding (%1).")
-                                        .arg(socket->errorString());
+                                        .arg(socket->errorString()));
-                    return false;
                 }
-                return true;
+                return ResultOk;
             }
             // No process yet. Create socket & launch the process
             QTcpServer server;
-            if (!server.listen(QHostAddress::LocalHost)) {
+            if (!server.listen(QHostAddress::LocalHost))
-                *errorMessage = Tr::tr("Unable to create server socket: %1").arg(server.errorString());
+                return ResultError(Tr::tr("Unable to create server socket: %1").arg(server.errorString()));
-                return false;
+
-            }
             const quint16 port = server.serverPort();
             if (debug)
                 qDebug() << Q_FUNC_INFO << "\nLaunching server:" << port << data.binary << filePath;
@@ -300,8 +294,9 @@ public:
             data.arguments.push_front(QString::number(port));
             data.arguments.push_front(QLatin1String("-client"));
 
-            if (!startEditorProcess(data, errorMessage))
+            if (const Result<> res = startEditorProcess(data); !res)
-                return false;
+                return res;
+
             // Insert into cache if socket is created, else try again next time
             if (server.waitForNewConnection(3000)) {
                 QTcpSocket *socket = server.nextPendingConnection();
@@ -312,7 +307,7 @@ public:
                 QObject::connect(socket, &QAbstractSocket::disconnected, guard, mapSlot);
                 QObject::connect(socket, &QAbstractSocket::errorOccurred, guard, mapSlot);
             }
-            return true;
+            return ResultOk;
         });
     }
 };
@@ -339,10 +334,11 @@ public:
         setId("Qt.Linguist");
         setDisplayName(::Core::Tr::tr("Qt Linguist"));
         setMimeTypes({Utils::Constants::LINGUIST_MIMETYPE});
-        setEditorStarter([](const FilePath &filePath, QString *errorMessage) {
+        setEditorStarter([](const FilePath &filePath) {
             LaunchData data;
-            return getEditorLaunchData(linguistBinary, filePath, &data, errorMessage)
+            if (const Result<> res = getEditorLaunchData(linguistBinary, filePath, &data); !res)
-                   && startEditorProcess(data, errorMessage);
+                return res;
+            return startEditorProcess(data);
         });
     }
 };