Improve SyncthingProcess and SyncthingLauncher

* Add documentation
* Remove leftovers
* Ensure all members are initialized
* Improve coding style

connector/syncthingprocess.cpp

@@ -8,6 +8,17 @@ namespace Data {
 
 SyncthingProcess *SyncthingProcess::s_mainInstance = nullptr;
 
+/*!
+ * \class SyncthingProcess
+ * \brief The SyncthingProcess class starts a Syncthing instance or additional tools as an external process.
+ *
+ * This class is actually not Syncthing-specific. It is just an extension of QProcess for some use-cases within
+ * Syncthing Tray.
+ */
+
+/*!
+ * \brief Constructs a new Syncthing process.
+ */
 SyncthingProcess::SyncthingProcess(QObject *parent)
     : QProcess(parent)
     , m_manuallyStopped(false)
@@ -21,6 +32,9 @@ SyncthingProcess::SyncthingProcess(QObject *parent)
     connect(&m_killTimer, &QTimer::timeout, this, &SyncthingProcess::confirmKill);
 }
 
+/*!
+ * \brief Splits the given arguments similar to how a shell would split it. So whitespaces are considered seperators unless quotes are used.
+ */
 QStringList SyncthingProcess::splitArguments(const QString &arguments)
 {
     enum { Any, Quote, Slash, Space } lastInput = Any;
@@ -89,6 +103,9 @@ QStringList SyncthingProcess::splitArguments(const QString &arguments)
     return result;
 }
 
+/*!
+ * \brief Stops the currently running process. If it has been stopped, starts the specified \a program with the specified \a arguments.
+ */
 void SyncthingProcess::restartSyncthing(const QString &program, const QStringList &arguments)
 {
     if (!isRunning()) {
@@ -102,6 +119,9 @@ void SyncthingProcess::restartSyncthing(const QString &program, const QStringLis
     terminate();
 }
 
+/*!
+ * \brief Starts the specified \a program with the specified \a arguments.
+ */
 void SyncthingProcess::startSyncthing(const QString &program, const QStringList &arguments)
 {
     if (isRunning()) {
@@ -112,6 +132,9 @@ void SyncthingProcess::startSyncthing(const QString &program, const QStringList
     start(program, arguments, QProcess::ReadOnly);
 }
 
+/*!
+ * \brief Stops the currently running process gracefully. If it doesn't stop after 3 seconds, attempts to kill the process.
+ */
 void SyncthingProcess::stopSyncthing()
 {
     if (!isRunning()) {
@@ -122,6 +145,9 @@ void SyncthingProcess::stopSyncthing()
     terminate();
 }
 
+/*!
+ * \brief Kills the currently running process.
+ */
 void SyncthingProcess::killSyncthing()
 {
     if (!isRunning()) {

connector/syncthingprocess.h

@@ -49,31 +49,43 @@ private:
     static SyncthingProcess *s_mainInstance;
 };
 
+/// \brief Returns whether the process is running.
 inline bool SyncthingProcess::isRunning() const
 {
     return state() != QProcess::NotRunning;
 }
 
+/// \brief Returns the last time when QProcess::started() has been emitted.
 inline CppUtilities::DateTime SyncthingProcess::activeSince() const
 {
     return m_activeSince;
 }
 
+/// \brief Checks whether the process already runs for the specified number of seconds.
 inline bool SyncthingProcess::isActiveFor(unsigned int atLeastSeconds) const
 {
     return !m_activeSince.isNull() && (CppUtilities::DateTime::gmtNow() - m_activeSince).totalSeconds() > atLeastSeconds;
 }
 
+/// \brief Returns whether the process has been manually stopped via SyncthingProcess::stopSyncthing(), SyncthingProcess::killSyncthing()
+/// or SyncthingProcess::restartSyncthing().
+/// \remarks Resetted on SyncthingProcess::startSyncthing() and SyncthingProcess::restartSyncthing().
 inline bool SyncthingProcess::isManuallyStopped() const
 {
     return m_manuallyStopped;
 }
 
+/*!
+ * \brief Returns the "main" instance assigned via SyncthingProcess::setMainInstance().
+ */
 inline SyncthingProcess *SyncthingProcess::mainInstance()
 {
     return s_mainInstance;
 }
 
+/*!
+ * \brief Sets the "main" instance.
+ */
 inline void SyncthingProcess::setMainInstance(SyncthingProcess *mainInstance)
 {
     s_mainInstance = mainInstance;

widgets/misc/syncthinglauncher.cpp

@@ -15,9 +15,21 @@ namespace Data {
 
 SyncthingLauncher *SyncthingLauncher::s_mainInstance = nullptr;
 
+/*!
+ * \class SyncthingLauncher
+ * \brief The SyncthingLauncher class starts a Syncthing instance either as an external process or using a library version of Syncthing.
+ * \remarks
+ * - This is *not* strictly a singleton class. However, one instance is supposed to be the "main instance" (see SyncthingLauncher::setMainInstance()).
+ * - A SyncthingLauncher instance can only launch one Syncthing instance at a time.
+ * - Using Syncthing as library is still under development and must be explicitely enabled by setting the CMake variable USE_LIBSYNCTHING.
+ */
+
+/*!
+ * \brief Constructs a new Syncthing launcher.
+ */
 SyncthingLauncher::SyncthingLauncher(QObject *parent)
     : QObject(parent)
-    , m_useLibSyncthing(false)
+    , m_manuallyStopped(true)
 {
     connect(&m_process, &SyncthingProcess::readyRead, this, &SyncthingLauncher::handleProcessReadyRead);
     connect(&m_process, static_cast<void (SyncthingProcess::*)(int exitCode, QProcess::ExitStatus exitStatus)>(&SyncthingProcess::finished), this,
@@ -27,6 +39,9 @@ SyncthingLauncher::SyncthingLauncher(QObject *parent)
     connect(&m_process, &SyncthingProcess::confirmKill, this, &SyncthingLauncher::confirmKill);
 }
 
+/*!
+ * \brief Returns whether the built-in Syncthing library is available.
+ */
 bool SyncthingLauncher::isLibSyncthingAvailable()
 {
 #ifdef SYNCTHINGWIDGETS_USE_LIBSYNCTHING
@@ -38,7 +53,9 @@ bool SyncthingLauncher::isLibSyncthingAvailable()
 
 /*!
  * \brief Launches a Syncthing instance using the specified \a arguments.
- * \remarks To use the internal library, leave \a program empty. Otherwise it must be the path the external Syncthing executable.
+ * \remarks
+ * - Does nothing if already running an instance.
+ * - To use the internal library, leave \a program empty. Otherwise it must be the path the external Syncthing executable.
  */
 void SyncthingLauncher::launch(const QString &program, const QStringList &arguments)
 {
@@ -46,22 +63,33 @@ void SyncthingLauncher::launch(const QString &program, const QStringList &argume
         return;
     }
     m_manuallyStopped = false;
+
+    // start external process
     if (!program.isEmpty()) {
         m_process.startSyncthing(program, arguments);
-    } else {
+        return;
-        vector<string> utf8Arguments{ "-no-restart", "-no-browser" };
-        utf8Arguments.reserve(utf8Arguments.size() + static_cast<size_t>(arguments.size()));
-        for (const auto &arg : arguments) {
-            const auto utf8Data(arg.toUtf8());
-            utf8Arguments.emplace_back(utf8Data.data(), utf8Data.size());
-        }
-        m_future = QtConcurrent::run(
-            this, static_cast<void (SyncthingLauncher::*)(const std::vector<std::string> &)>(&SyncthingLauncher::runLibSyncthing), utf8Arguments);
     }
+
+    // use libsyncthing
+    vector<string> utf8Arguments{ "-no-restart", "-no-browser" };
+    utf8Arguments.reserve(utf8Arguments.size() + static_cast<size_t>(arguments.size()));
+    for (const auto &arg : arguments) {
+        const auto utf8Data(arg.toUtf8());
+        utf8Arguments.emplace_back(utf8Data.data(), utf8Data.size());
+    }
+    m_future = QtConcurrent::run(
+        this, static_cast<void (SyncthingLauncher::*)(const std::vector<std::string> &)>(&SyncthingLauncher::runLibSyncthing), utf8Arguments);
 }
 
+/*!
+ * \brief Launches a Syncthing instance according to the specified \a launcherSettings.
+ * \remarks Does nothing if already running an instance.
+ */
 void SyncthingLauncher::launch(const Settings::Launcher &launcherSettings)
 {
+    if (isRunning()) {
+        return;
+    }
     if (!launcherSettings.useLibSyncthing && launcherSettings.syncthingPath.isEmpty()) {
         emit errorOccurred(QProcess::FailedToStart);
         return;
@@ -72,6 +100,7 @@ void SyncthingLauncher::launch(const Settings::Launcher &launcherSettings)
 
 /*!
  * \brief Launches a Syncthing instance using the internal library with the specified \a runtimeOptions.
+ * \remarks Does nothing if already running an instance.
  */
 void SyncthingLauncher::launch(const LibSyncthing::RuntimeOptions &runtimeOptions)
 {
@@ -194,10 +223,4 @@ void SyncthingLauncher::runLibSyncthing(const std::vector<string> &arguments)
 #endif
 }
 
-SyncthingLauncher &syncthingLauncher()
-{
-    static SyncthingLauncher launcher;
-    return launcher;
-}
-
 } // namespace Data

widgets/misc/syncthinglauncher.h

@@ -23,7 +23,6 @@ class SYNCTHINGWIDGETS_EXPORT SyncthingLauncher : public QObject {
     Q_PROPERTY(bool running READ isRunning NOTIFY runningChanged)
     Q_PROPERTY(CppUtilities::DateTime activeSince READ activeSince)
     Q_PROPERTY(bool manuallyStopped READ isManuallyStopped)
-    Q_PROPERTY(bool useLibSyncthing READ isUseLibSyncthing WRITE setUseLibSyncthing)
 
 public:
     explicit SyncthingLauncher(QObject *parent = nullptr);
@@ -32,7 +31,6 @@ public:
     CppUtilities::DateTime activeSince() const;
     bool isActiveFor(unsigned int atLeastSeconds) const;
     bool isManuallyStopped() const;
-    bool isUseLibSyncthing() const;
     static bool isLibSyncthingAvailable();
     static SyncthingLauncher *mainInstance();
     static void setMainInstance(SyncthingLauncher *mainInstance);
@@ -45,7 +43,6 @@ Q_SIGNALS:
     void errorOccurred(QProcess::ProcessError error);
 
 public Q_SLOTS:
-    void setUseLibSyncthing(bool useLibSyncthing);
     void launch(const QString &program, const QStringList &arguments);
     void launch(const Settings::Launcher &launcherSettings);
     void launch(const LibSyncthing::RuntimeOptions &runtimeOptions);
@@ -69,11 +66,13 @@ private:
     static SyncthingLauncher *s_mainInstance;
 };
 
+/// \brief Returns whether Syncthing is running.
 inline bool SyncthingLauncher::isRunning() const
 {
     return m_process.isRunning() || m_future.isRunning();
 }
 
+/// \brief Returns when the Syncthing instance has been started.
 inline CppUtilities::DateTime SyncthingLauncher::activeSince() const
 {
     if (m_process.isRunning()) {
@@ -84,32 +83,28 @@ inline CppUtilities::DateTime SyncthingLauncher::activeSince() const
     return CppUtilities::DateTime();
 }
 
+/// \brief Checks whether Syncthing is already running for the specified number of seconds.
 inline bool SyncthingLauncher::isActiveFor(unsigned int atLeastSeconds) const
 {
     const auto activeSince(this->activeSince());
     return !activeSince.isNull() && (CppUtilities::DateTime::gmtNow() - activeSince).totalSeconds() > atLeastSeconds;
 }
 
+/// \brief Returns whether the Syncthing instance has been manually stopped using SyncthingLauncher::terminate()
+/// or SyncthingLauncher::kill().
+/// \remarks This is resetted when calling SyncthingLauncher::launch().
 inline bool SyncthingLauncher::isManuallyStopped() const
 {
     return m_manuallyStopped;
 }
 
-inline bool SyncthingLauncher::isUseLibSyncthing() const
+/// \brief Returns the SyncthingLauncher instance previously assigned via SyncthingLauncher::setMainInstance().
-{
-    return m_useLibSyncthing;
-}
-
-inline void SyncthingLauncher::setUseLibSyncthing(bool useLibSyncthing)
-{
-    m_useLibSyncthing = useLibSyncthing;
-}
-
 inline SyncthingLauncher *SyncthingLauncher::mainInstance()
 {
     return s_mainInstance;
 }
 
+/// \brief Sets the "main" SyncthingLauncher instance and SyncthingProcess::mainInstance() if not already assigned.
 inline void SyncthingLauncher::setMainInstance(SyncthingLauncher *mainInstance)
 {
     if ((s_mainInstance = mainInstance) && !SyncthingProcess::mainInstance()) {
@@ -117,8 +112,6 @@ inline void SyncthingLauncher::setMainInstance(SyncthingLauncher *mainInstance)
     }
 }
 
-SyncthingLauncher SYNCTHINGWIDGETS_EXPORT &syncthingLauncher();
-
 } // namespace Data
 
 #endif // SYNCTHINGWIDGETS_SYNCTHINGLAUNCHER_H