LSP: split out settings specific for std io clients

Change-Id: I25eebfc15a9d0d5665e57137f016e54a069ca849
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/languageclient/baseclient.cpp

@@ -499,6 +499,11 @@ bool BaseClient::isSupportedMimeType(const QString &mimeType) const
     return m_supportedMimeTypes.isEmpty() || m_supportedMimeTypes.contains(mimeType);
 }
 
+bool BaseClient::needsRestart(const BaseSettings *) const
+{
+    return false;
+}
+
 bool BaseClient::reset()
 {
     if (!m_restartsLeft)
@@ -756,6 +761,11 @@ StdIOClient::~StdIOClient()
     Utils::SynchronousProcess::stopProcess(m_process);
 }
 
+bool StdIOClient::needsRestart(const StdIOSettings *settings)
+{
+    return m_executable != settings->m_executable || m_arguments != settings->m_arguments;
+}
+
 bool StdIOClient::start()
 {
     m_process.start();
@@ -771,11 +781,6 @@ void StdIOClient::setWorkingDirectory(const QString &workingDirectory)
     m_process.setWorkingDirectory(workingDirectory);
 }
 
-bool StdIOClient::matches(const BaseSettings *setting)
-{
-    return setting->m_executable == m_executable && setting->m_arguments == m_arguments;
-}
-
 void StdIOClient::sendData(const QByteArray &data)
 {
     if (m_process.state() != QProcess::Running) {

src/plugins/languageclient/baseclient.h

@@ -112,8 +112,9 @@ public:
 
     Core::Id id() const { return m_id; }
 
+    bool needsRestart(const BaseSettings *) const;
+
     virtual bool start() { return true; }
-    virtual bool matches(const BaseSettings * /*setting*/) { return false; }
     virtual bool reset();
 
     void log(const QString &message,
@@ -175,12 +176,12 @@ public:
     StdIOClient &operator=(const StdIOClient &) = delete;
     StdIOClient &operator=(StdIOClient &&) = delete;
 
+    bool needsRestart(const StdIOSettings *settings);
+
     bool start() override;
 
     void setWorkingDirectory(const QString &workingDirectory);
 
-    bool matches(const BaseSettings *setting) override;
-
 protected:
     void sendData(const QByteArray &data) final;
     QProcess m_process;

src/plugins/languageclient/languageclientmanager.cpp

@@ -167,6 +167,7 @@ void LanguageClientManager::removeMarks(const Core::Id &id)
 
 void LanguageClientManager::startClient(BaseClient *client)
 {
+    QTC_ASSERT(client, return);
     if (managerInstance->m_shuttingDown) {
         managerInstance->clientFinished(client);
         return;

src/plugins/languageclient/languageclientsettings.cpp

@@ -23,8 +23,10 @@
 **
 ****************************************************************************/
 
-#include "languageclientmanager.h"
 #include "languageclientsettings.h"
+
+#include "baseclient.h"
+#include "languageclientmanager.h"
 #include "languageclient_global.h"
 
 #include <coreplugin/icore.h>
@@ -71,8 +73,9 @@ public:
     bool setData(const QModelIndex &index, const QVariant &value, int role) override;
     Qt::ItemFlags flags(const QModelIndex &index) const override;
 
-    void reset(const QList<BaseSettings *> &settings);
+    void reset(const QList<StdIOSettings *> &settings);
-    QList<BaseSettings *> settings() const { return m_settings; }
+    QList<StdIOSettings *> settings() const { return m_settings; }
+    QList<StdIOSettings *> removed() const { return m_removed; }
 
     enum Columns {
         DisplayNameColumn = 0,
@@ -84,7 +87,8 @@ public:
     };
 
 private:
-    QList<BaseSettings *> m_settings; // owned
+    QList<StdIOSettings *> m_settings; // owned
+    QList<StdIOSettings *> m_removed;
 };
 
 class LanguageClientSettingsPageWidget : public QWidget
@@ -115,7 +119,7 @@ public:
 
 private:
     LanguageClientSettingsModel m_model;
-    QList<BaseSettings *> m_settings; // owned
+    QList<StdIOSettings *> m_settings; // owned
     QPointer<LanguageClientSettingsPageWidget> m_widget;
 };
 
@@ -229,43 +233,33 @@ QWidget *LanguageClientSettingsPage::widget()
 void LanguageClientSettingsPage::apply()
 {
     qDeleteAll(m_settings);
-    m_settings = Utils::transform(m_model.settings(), [](const BaseSettings *other){
+    m_settings = Utils::transform(m_model.settings(), [](const StdIOSettings *other){
-        return new BaseSettings(*other);
+        return dynamic_cast<StdIOSettings *>(other->copy());
     });
     LanguageClientSettings::toSettings(Core::ICore::settings(), m_settings);
 
-    const QVector<BaseClient *> interfaces(LanguageClientManager::clients());
+    QList<StdIOSettings *> restarts = Utils::filtered(m_settings, &StdIOSettings::needsRestart);
-    QVector<BaseClient *> toShutdown;
+    for (auto setting : restarts + m_model.removed()) {
-    QList<BaseSettings *> toStart = m_settings;
+        if (auto client = setting->m_client) {
-    // check currently registered interfaces
+            if (client->reachable())
-    for (auto interface : interfaces) {
+                client->shutdown();
-        auto setting = Utils::findOr(m_settings, nullptr,
+            else
-                                     [interface](const BaseSettings *setting){
+                LanguageClientManager::deleteClient(client);
-            return interface->matches(setting);
-        });
-        if (setting && setting->isValid() && setting->m_enabled) {
-            toStart.removeAll(setting);
-            if (!interface->isSupportedMimeType(setting->m_mimeType))
-                interface->setSupportedMimeType({setting->m_mimeType});
-        } else {
-            toShutdown << interface;
         }
     }
-    for (auto interface : toShutdown) {
+    for (StdIOSettings *setting : restarts) {
-        if (interface->reachable())
+        if (setting && setting->isValid() && setting->m_enabled) {
-            interface->shutdown();
+            if (auto client = setting->createClient()) {
-        else
+                setting->m_client = client;
-            LanguageClientManager::deleteClient(interface);
+                LanguageClientManager::startClient(client);
-    }
+            }
-    for (auto setting : toStart) {
+        }
-        if (setting && setting->isValid() && setting->m_enabled)
-            LanguageClientManager::startClient(setting->createClient());
     }
+    m_model.reset(m_settings);
 }
 
 void LanguageClientSettingsPage::finish()
 {
-    m_model.reset(m_settings);
 }
 
 LanguageClientSettingsModel::~LanguageClientSettingsModel()
@@ -277,7 +271,7 @@ QVariant LanguageClientSettingsModel::data(const QModelIndex &index, int role) c
 {
     if (!index.isValid())
         return QVariant();
-    BaseSettings *setting = m_settings[index.row()];
+    StdIOSettings *setting = m_settings[index.row()];
     QTC_ASSERT(setting, return false);
     if (role == Qt::DisplayRole || role == Qt::EditRole) {
         switch (index.column()) {
@@ -314,7 +308,7 @@ bool LanguageClientSettingsModel::removeRows(int row, int count, const QModelInd
     const int end = qMin(row + count - 1, int(m_settings.size()) - 1);
     beginRemoveRows(parent, row, end);
     for (auto i = end; i >= row; --i)
-        delete m_settings.takeAt(i);
+        m_removed << m_settings.takeAt(i);
     endRemoveRows();
     return true;
 }
@@ -325,7 +319,7 @@ bool LanguageClientSettingsModel::insertRows(int row, int count, const QModelInd
         return false;
     beginInsertRows(parent, row, row + count - 1);
     for (int i = 0; i < count; ++i)
-        m_settings.insert(row + i, new BaseSettings());
+        m_settings.insert(row + i, new StdIOSettings());
     endInsertRows();
     return true;
 }
@@ -334,23 +328,31 @@ bool LanguageClientSettingsModel::setData(const QModelIndex &index, const QVaria
 {
     if (!index.isValid())
         return false;
-    BaseSettings *setting = m_settings[index.row()];
+    StdIOSettings *setting = m_settings[index.row()];
     QTC_ASSERT(setting, return false);
     if (role == Qt::DisplayRole || role == Qt::EditRole) {
+        const QString strVal(value.toString());
+        QString *settingsValue = nullptr;
         switch (index.column()) {
-        case DisplayNameColumn: setting->m_name = value.toString(); break;
+        case DisplayNameColumn: settingsValue = &setting->m_name; break;
-        case MimeTypeColumn: setting->m_mimeType = value.toString(); break;
+        case MimeTypeColumn: settingsValue = &setting->m_mimeType; break;
-        case ExecutableColumn: setting->m_executable = value.toString(); break;
+        case ExecutableColumn: settingsValue = &setting->m_executable; break;
-        case ArgumentsColumn: setting->m_arguments = value.toString(); break;
+        case ArgumentsColumn: settingsValue = &setting->m_arguments; break;
-        default:
+        }
-            return false;
+        if (settingsValue) {
+            if (strVal != *settingsValue) {
+                *settingsValue = value.toString();
+                emit dataChanged(index, index, { Qt::EditRole, Qt::DisplayRole });
+            }
+            return true;
         }
-        emit dataChanged(index, index, { Qt::EditRole, Qt::DisplayRole });
-        return true;
     }
+
     if (role == Qt::CheckStateRole && index.column() == EnabledColumn) {
-        setting->m_enabled = value.toBool();
+        if (setting->m_enabled != value.toBool()) {
-        emit dataChanged(index, index, { Qt::CheckStateRole });
+            setting->m_enabled = !setting->m_enabled;
+            emit dataChanged(index, index, { Qt::CheckStateRole });
+        }
         return true;
     }
     return false;
@@ -364,28 +366,31 @@ Qt::ItemFlags LanguageClientSettingsModel::flags(const QModelIndex &index) const
     return defaultFlags;
 }
 
-void LanguageClientSettingsModel::reset(const QList<BaseSettings *> &settings)
+void LanguageClientSettingsModel::reset(const QList<StdIOSettings *> &settings)
 {
     beginResetModel();
     qDeleteAll(m_settings);
-    m_settings = Utils::transform(settings, [](const BaseSettings *setting){
+    qDeleteAll(m_removed);
-        return new BaseSettings(*setting);
+    m_removed.clear();
+    m_settings = Utils::transform(settings, [](const StdIOSettings *other){
+        return dynamic_cast<StdIOSettings *>(other->copy());
     });
     endResetModel();
 }
 
-bool BaseSettings::isValid()
+bool BaseSettings::needsRestart() const
 {
-    return !m_name.isEmpty() && !m_executable.isEmpty() && QFile::exists(m_executable);
+    return m_client ? !m_enabled || m_client->needsRestart(this) : m_enabled;
 }
 
-BaseClient *BaseSettings::createClient()
+bool BaseSettings::isValid() const
 {
-    auto client = new StdIOClient(m_executable, m_arguments);
+    return !m_name.isEmpty();
-    client->setName(m_name);
+}
-    if (m_mimeType != noLanguageFilter)
+
-        client->setSupportedMimeType({m_mimeType});
+BaseClient *BaseSettings::createClient() const
-    return client;
+{
+    return nullptr;
 }
 
 QVariantMap BaseSettings::toMap() const
@@ -394,8 +399,6 @@ QVariantMap BaseSettings::toMap() const
     map.insert(nameKey, m_name);
     map.insert(enabledKey, m_enabled);
     map.insert(mimeTypeKey, m_mimeType);
-    map.insert(executableKey, m_executable);
-    map.insert(argumentsKey, m_arguments);
     return map;
 }
 
@@ -404,8 +407,6 @@ void BaseSettings::fromMap(const QVariantMap &map)
     m_name = map[nameKey].toString();
     m_enabled = map[enabledKey].toBool();
     m_mimeType = map[mimeTypeKey].toString();
-    m_executable = map[executableKey].toString();
-    m_arguments = map[argumentsKey].toString();
 }
 
 void LanguageClientSettings::init()
@@ -414,12 +415,12 @@ void LanguageClientSettings::init()
     settingsPage.init();
 }
 
-QList<BaseSettings *> LanguageClientSettings::fromSettings(QSettings *settingsIn)
+QList<StdIOSettings *> LanguageClientSettings::fromSettings(QSettings *settingsIn)
 {
     settingsIn->beginGroup(settingsGroupKey);
     auto variants = settingsIn->value(clientsKey).toList();
     auto settings = Utils::transform(variants, [](const QVariant& var){
-        auto settings = new BaseSettings();
+        auto settings = new StdIOSettings();
         settings->fromMap(var.toMap());
         return settings;
     });
@@ -427,14 +428,52 @@ QList<BaseSettings *> LanguageClientSettings::fromSettings(QSettings *settingsIn
     return settings;
 }
 
-void LanguageClientSettings::toSettings(QSettings *settings, const QList<BaseSettings *> &languageClientSettings)
+void LanguageClientSettings::toSettings(QSettings *settings, const QList<StdIOSettings *> &languageClientSettings)
 {
     settings->beginGroup(settingsGroupKey);
     settings->setValue(clientsKey, Utils::transform(languageClientSettings,
-                                                    [](const BaseSettings *setting){
+                                                    [](const StdIOSettings *setting){
         return QVariant(setting->toMap());
     }));
     settings->endGroup();
 }
 
+bool StdIOSettings::needsRestart() const
+{
+    if (BaseSettings::needsRestart())
+        return true;
+    if (auto stdIOClient = qobject_cast<StdIOClient *>(m_client))
+        return stdIOClient->needsRestart(this);
+    return false;
+}
+
+bool StdIOSettings::isValid() const
+{
+    return BaseSettings::isValid() && !m_executable.isEmpty();
+}
+
+BaseClient *StdIOSettings::createClient() const
+{
+    auto client = new StdIOClient(m_executable, m_arguments);
+    client->setName(m_name);
+    if (m_mimeType != noLanguageFilter)
+        client->setSupportedMimeType({m_mimeType});
+    return client;
+}
+
+QVariantMap StdIOSettings::toMap() const
+{
+    QVariantMap map = BaseSettings::toMap();
+    map.insert(executableKey, m_executable);
+    map.insert(argumentsKey, m_arguments);
+    return map;
+}
+
+void StdIOSettings::fromMap(const QVariantMap &map)
+{
+    BaseSettings::fromMap(map);
+    m_executable = map[executableKey].toString();
+    m_arguments = map[argumentsKey].toString();
+}
+
 } // namespace LanguageClient

src/plugins/languageclient/languageclientsettings.h

@@ -41,35 +41,69 @@ class BaseSettings
 {
 public:
     BaseSettings() = default;
-    BaseSettings(const QString &name, bool enabled, const QString &mimeTypeName,
+    BaseSettings(const QString &name, bool enabled, const QString &mimeTypeName)
-                 const QString &executable, const QString &arguments)
         : m_name(name)
         , m_enabled(enabled)
         , m_mimeType(mimeTypeName)
-        , m_executable(executable)
-        , m_arguments(arguments)
     {}
+
+    virtual ~BaseSettings() = default;
+
     QString m_name = QString("New Language Server");
     bool m_enabled = true;
     QString m_mimeType = QLatin1String(noLanguageFilter);
+    QPointer<BaseClient> m_client; // not owned
+
+    virtual BaseSettings *copy() const { return new BaseSettings(*this); }
+    virtual bool needsRestart() const;
+    virtual bool isValid() const ;
+    virtual BaseClient *createClient() const;
+    virtual QVariantMap toMap() const;
+    virtual void fromMap(const QVariantMap &map);
+
+protected:
+    BaseSettings(const BaseSettings &other) = default;
+    BaseSettings(BaseSettings &&other) = default;
+    BaseSettings &operator=(const BaseSettings &other) = default;
+    BaseSettings &operator=(BaseSettings &&other) = default;
+};
+
+class StdIOSettings : public BaseSettings
+{
+public:
+    StdIOSettings() = default;
+    StdIOSettings(const QString &name, bool enabled, const QString &mimeTypeName,
+                  const QString &executable, const QString &arguments)
+        : BaseSettings(name, enabled, mimeTypeName)
+        , m_executable(executable)
+        , m_arguments(arguments)
+    {}
+
+    ~StdIOSettings() override = default;
+
     QString m_executable;
     QString m_arguments;
 
-    bool isValid();
+    BaseSettings *copy() const override { return new StdIOSettings(*this); }
+    bool needsRestart() const override;
+    bool isValid() const override;
+    BaseClient *createClient() const override;
+    QVariantMap toMap() const override;
+    void fromMap(const QVariantMap &map) override;
 
-    BaseClient *createClient();
+protected:
-
+    StdIOSettings(const StdIOSettings &other) = default;
-    QVariantMap toMap() const;
+    StdIOSettings(StdIOSettings &&other) = default;
-    void fromMap(const QVariantMap &map);
+    StdIOSettings &operator=(const StdIOSettings &other) = default;
+    StdIOSettings &operator=(StdIOSettings &&other) = default;
 };
 
 class LanguageClientSettings
 {
 public:
     static void init();
-    static QList<BaseSettings *> fromSettings(QSettings *settings);
+    static QList<StdIOSettings *> fromSettings(QSettings *settings);
-    static void toSettings(QSettings *settings, const QList<BaseSettings *> &languageClientSettings);
+    static void toSettings(QSettings *settings, const QList<StdIOSettings *> &languageClientSettings);
-
 };
 
 } // namespace LanguageClient