ProjectExplorer: Simplify IDevice extra device action setup

Function objects are easy nowadays.

Change-Id: Iec2b770b99d8f11b7a090fb3bd51af8aa60f6fe0
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/android/androiddevice.cpp

@@ -78,23 +78,6 @@ IDeviceWidget *AndroidDevice::createWidget()
     return nullptr;
 }
 
-QList<Core::Id> AndroidDevice::actionIds() const
-{
-    return QList<Core::Id>();
-}
-
-QString AndroidDevice::displayNameForActionId(Core::Id actionId) const
-{
-    Q_UNUSED(actionId)
-    return QString();
-}
-
-void AndroidDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    Q_UNUSED(actionId)
-    Q_UNUSED(parent)
-}
-
 bool AndroidDevice::canAutoDetectPorts() const
 {
     return true;

src/plugins/android/androiddevice.h

@@ -39,9 +39,6 @@ public:
 
     QString displayType() const override;
     ProjectExplorer::IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent = nullptr) override;
     bool canAutoDetectPorts() const override;
     ProjectExplorer::DeviceProcessSignalOperation::Ptr signalOperation() const override;
     Utils::OsType osType() const override;

src/plugins/baremetal/baremetaldevice.cpp

@@ -159,23 +159,6 @@ IDeviceWidget *BareMetalDevice::createWidget()
     return new BareMetalDeviceConfigurationWidget(sharedFromThis());
 }
 
-QList<Core::Id> BareMetalDevice::actionIds() const
-{
-    return QList<Core::Id>(); // no actions
-}
-
-QString BareMetalDevice::displayNameForActionId(Core::Id actionId) const
-{
-    QTC_ASSERT(actionIds().contains(actionId), return QString());
-    return QString();
-}
-
-void BareMetalDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    QTC_ASSERT(actionIds().contains(actionId), return);
-    Q_UNUSED(parent);
-}
-
 Utils::OsType BareMetalDevice::osType() const
 {
     return Utils::OsTypeOther;

src/plugins/baremetal/baremetaldevice.h

@@ -47,9 +47,6 @@ public:
     ~BareMetalDevice() override;
     QString displayType() const override;
     ProjectExplorer::IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent) override;
     Utils::OsType osType() const override;
     ProjectExplorer::IDevice::Ptr clone() const override;
 

src/plugins/ios/iosdevice.cpp

@@ -130,23 +130,6 @@ IDeviceWidget *IosDevice::createWidget()
     return nullptr;
 }
 
-QList<Core::Id> IosDevice::actionIds() const
-{
-    return QList<Core::Id>(); // add activation action?
-}
-
-QString IosDevice::displayNameForActionId(Core::Id actionId) const
-{
-    Q_UNUSED(actionId)
-    return QString();
-}
-
-void IosDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    Q_UNUSED(actionId)
-    Q_UNUSED(parent)
-}
-
 DeviceProcessSignalOperation::Ptr IosDevice::signalOperation() const
 {
     return DeviceProcessSignalOperation::Ptr();

src/plugins/ios/iosdevice.h

@@ -54,9 +54,6 @@ public:
 
     ProjectExplorer::IDevice::DeviceInfo deviceInformation() const override;
     ProjectExplorer::IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent = nullptr) override;
     ProjectExplorer::DeviceProcessSignalOperation::Ptr signalOperation() const override;
     QString displayType() const override;
 

src/plugins/ios/iossimulator.cpp

@@ -89,23 +89,6 @@ IDeviceWidget *IosSimulator::createWidget()
     return nullptr;
 }
 
-QList<Core::Id> IosSimulator::actionIds() const
-{
-    return QList<Core::Id>();
-}
-
-QString IosSimulator::displayNameForActionId(Core::Id actionId) const
-{
-    Q_UNUSED(actionId)
-    return QString();
-}
-
-void IosSimulator::executeAction(Core::Id actionId, QWidget *parent)
-{
-    Q_UNUSED(actionId)
-    Q_UNUSED(parent)
-}
-
 DeviceProcessSignalOperation::Ptr IosSimulator::signalOperation() const
 {
     return DeviceProcessSignalOperation::Ptr();

src/plugins/ios/iossimulator.h

@@ -69,9 +69,6 @@ public:
 
     QString displayType() const override;
     ProjectExplorer::IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent = nullptr) override;
     ProjectExplorer::DeviceProcessSignalOperation::Ptr signalOperation() const override;
     void fromMap(const QVariantMap &map) override;
     QVariantMap toMap() const override;

src/plugins/projectexplorer/devicesupport/desktopdevice.cpp

@@ -80,23 +80,6 @@ IDeviceWidget *DesktopDevice::createWidget()
     // range can be confusing to the user. Hence, disabling the widget for now.
 }
 
-QList<Core::Id> DesktopDevice::actionIds() const
-{
-    return QList<Core::Id>();
-}
-
-QString DesktopDevice::displayNameForActionId(Core::Id actionId) const
-{
-    Q_UNUSED(actionId);
-    return QString();
-}
-
-void DesktopDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    Q_UNUSED(actionId);
-    Q_UNUSED(parent);
-}
-
 bool DesktopDevice::canAutoDetectPorts() const
 {
     return true;

src/plugins/projectexplorer/devicesupport/desktopdevice.h

@@ -42,9 +42,6 @@ public:
 
     QString displayType() const override;
     IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent = nullptr) override;
     bool canAutoDetectPorts() const override;
     bool canCreateProcessModel() const override;
     DeviceProcessList *createProcessListModel(QObject *parent) const override;

src/plugins/projectexplorer/devicesupport/devicemanager.cpp

@@ -413,9 +413,6 @@ private:
     TestDevice(const TestDevice &other) = default;
     QString displayType() const override { return QLatin1String("blubb"); }
     IDeviceWidget *createWidget() override { return nullptr; }
-    QList<Core::Id> actionIds() const override { return QList<Core::Id>(); }
-    QString displayNameForActionId(Core::Id) const override { return QString(); }
-    void executeAction(Core::Id, QWidget *) override { }
     Ptr clone() const override { return Ptr(new TestDevice(*this)); }
     DeviceProcessSignalOperation::Ptr signalOperation() const override
     {

src/plugins/projectexplorer/devicesupport/devicesettingswidget.cpp

@@ -299,11 +299,19 @@ void DeviceSettingsWidget::currentDeviceChanged(int index)
         m_ui->buttonsLayout->insertWidget(m_ui->buttonsLayout->count() - 1, button);
     }
 
-    foreach (Id actionId, device->actionIds()) {
+    for (const IDevice::DeviceAction &deviceAction : device->deviceActions()) {
-        QPushButton * const button = new QPushButton(device->displayNameForActionId(actionId));
+        QPushButton * const button = new QPushButton(deviceAction.display);
         m_additionalActionButtons << button;
-        connect(button, &QAbstractButton::clicked, this,
+        connect(button, &QAbstractButton::clicked, this, [this, deviceAction] {
-                [this, actionId] { handleAdditionalActionRequest(actionId); });
+            const IDevice::Ptr device = m_deviceManager->mutableDevice(currentDevice()->id());
+            QTC_ASSERT(device, return);
+            updateDeviceFromUi();
+            deviceAction.execute(device, this);
+            // Widget must be set up from scratch, because the action could have
+            // changed random attributes.
+            currentDeviceChanged(currentIndex());
+        });
+
         m_ui->buttonsLayout->insertWidget(m_ui->buttonsLayout->count() - 1, button);
     }
 
@@ -322,17 +330,6 @@ void DeviceSettingsWidget::clearDetails()
     m_ui->autoDetectionValueLabel->clear();
 }
 
-void DeviceSettingsWidget::handleAdditionalActionRequest(Id actionId)
-{
-    const IDevice::Ptr device = m_deviceManager->mutableDevice(currentDevice()->id());
-    QTC_ASSERT(device, return);
-    updateDeviceFromUi();
-    device->executeAction(actionId, this);
-
-    // Widget must be set up from scratch, because the action could have changed random attributes.
-    currentDeviceChanged(currentIndex());
-}
-
 void DeviceSettingsWidget::handleProcessListRequested()
 {
     QTC_ASSERT(currentDevice()->canCreateProcessModel(), return);

src/plugins/projectexplorer/devicesupport/devicesettingswidget.h

@@ -65,7 +65,6 @@ private:
     void handleProcessListRequested();
 
     void initGui();
-    void handleAdditionalActionRequest(Core::Id actionId);
     void displayCurrent();
     void setDeviceInfoWidgetsEnabled(bool enable);
     IDevice::ConstPtr currentDevice() const;

src/plugins/projectexplorer/devicesupport/idevice.cpp

@@ -44,6 +44,15 @@
 #include <QString>
 #include <QUuid>
 
+/*!
+ * \class ProjectExplorer::IDevice::DeviceAction
+ * \brief The DeviceAction class describes an action that can be run on a device.
+ *
+ * The description consists of a human-readable string that will be displayed
+ * on a button which, when clicked, executes a functor, and the functor itself.
+ * This is typically some sort of dialog or wizard, so \a parent widget is provided.
+ */
+
 /*!
  * \class ProjectExplorer::IDevice
  * \brief The IDevice class is the base class for all devices.
@@ -76,23 +85,11 @@
  */
 
 /*!
- * \fn QStringList ProjectExplorer::IDevice::actionIds() const
+ * \fn void ProjectExplorer::IDevice::addDeviceAction(const DeviceAction &deviceAction)
- * Returns a list of ids representing actions that can be run on this device.
+ * Adds an actions that can be run on this device.
  * These actions will be available in the \gui Devices options page.
  */
 
-/*!
- * \fn QString ProjectExplorer::IDevice::displayNameForActionId(Core::Id actionId) const
- * A human-readable string for \a actionId. Will be displayed on a button which,
- *        when clicked, starts the respective action.
- */
-
-/*!
- * \fn void ProjectExplorer::IDevice::executeAction(Core::Id actionId, QWidget *parent) const
- * Executes the action specified by \a actionId. This is typically done via some
- * sort of dialog or wizard, so \a parent widget is provided.
- */
-
 /*!
  * \fn ProjectExplorer::IDevice::Ptr ProjectExplorer::IDevice::clone() const
  * Creates an identical copy of a device object.
@@ -153,6 +150,7 @@ public:
     QString qmlsceneCommand;
 
     QList<Utils::Icon> deviceIcons;
+    QList<IDevice::DeviceAction> deviceActions;
     QVariantMap extraData;
 };
 } // namespace Internal
@@ -252,6 +250,16 @@ bool IDevice::isCompatibleWith(const Kit *k) const
     return DeviceTypeKitInformation::deviceTypeId(k) == type();
 }
 
+void IDevice::addDeviceAction(const DeviceAction &deviceAction)
+{
+    d->deviceActions.append(deviceAction);
+}
+
+const QList<IDevice::DeviceAction> IDevice::deviceActions() const
+{
+    return d->deviceActions;
+}
+
 PortsGatheringMethod::Ptr IDevice::portsGatheringMethod() const
 {
     return PortsGatheringMethod::Ptr();

src/plugins/projectexplorer/devicesupport/idevice.h

@@ -153,9 +153,13 @@ public:
 
     virtual QString displayType() const = 0;
     virtual IDeviceWidget *createWidget() = 0;
-    virtual QList<Core::Id> actionIds() const = 0;
+
-    virtual QString displayNameForActionId(Core::Id actionId) const = 0;
+    struct DeviceAction {
-    virtual void executeAction(Core::Id actionId, QWidget *parent = nullptr) = 0;
+        QString display;
+        std::function<void(const IDevice::Ptr &device, QWidget *parent)> execute;
+    };
+    void addDeviceAction(const DeviceAction &deviceAction);
+    const QList<DeviceAction> deviceActions() const;
 
     // Devices that can auto detect ports need not return a ports gathering method. Such devices can
     // obtain a free port on demand. eg: Desktop device.

src/plugins/qnx/qnxdevice.cpp

@@ -50,7 +50,6 @@ namespace Qnx {
 using namespace Internal;
 
 const char QnxVersionKey[] = "QnxVersion";
-const char DeployQtLibrariesActionId [] = "Qnx.Qnx.DeployQtLibrariesAction";
 
 class QnxPortsGatheringMethod : public PortsGatheringMethod
 {
@@ -79,19 +78,26 @@ class QnxPortsGatheringMethod : public PortsGatheringMethod
 };
 
 QnxDevice::QnxDevice()
-    : RemoteLinux::LinuxDevice()
-    , m_versionNumber(0)
 {
+    init();
 }
 
 QnxDevice::QnxDevice(const QString &name, Core::Id type, MachineType machineType, Origin origin, Core::Id id)
     : RemoteLinux::LinuxDevice(name, type, machineType, origin, id)
-    , m_versionNumber(0)
 {
+    init();
 }
 
 QnxDevice::QnxDevice(const QnxDevice &other) = default;
 
+void QnxDevice::init()
+{
+    addDeviceAction({tr("Deploy Qt libraries..."), [](const IDevice::Ptr &device, QWidget *parent) {
+        QnxDeployQtLibrariesDialog dialog(device, parent);
+        dialog.exec();
+    }});
+}
+
 QnxDevice::Ptr QnxDevice::create()
 {
     return Ptr(new QnxDevice);
@@ -190,33 +196,6 @@ DeviceProcess *QnxDevice::createProcess(QObject *parent) const
     return new QnxDeviceProcess(sharedFromThis(), parent);
 }
 
-QList<Core::Id> QnxDevice::actionIds() const
-{
-    QList<Core::Id> actions = RemoteLinux::LinuxDevice::actionIds();
-    actions << Core::Id(DeployQtLibrariesActionId);
-    return actions;
-}
-
-QString QnxDevice::displayNameForActionId(Core::Id actionId) const
-{
-    if (actionId == Core::Id(DeployQtLibrariesActionId))
-        return tr("Deploy Qt libraries...");
-
-    return RemoteLinux::LinuxDevice::displayNameForActionId(actionId);
-}
-
-void QnxDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    const QnxDevice::ConstPtr device =
-            sharedFromThis().staticCast<const QnxDevice>();
-    if (actionId == Core::Id(DeployQtLibrariesActionId)) {
-        QnxDeployQtLibrariesDialog dialog(device, parent);
-        dialog.exec();
-    } else {
-        RemoteLinux::LinuxDevice::executeAction(actionId, parent);
-    }
-}
-
 DeviceProcessSignalOperation::Ptr QnxDevice::signalOperation() const
 {
     return DeviceProcessSignalOperation::Ptr(

src/plugins/qnx/qnxdevice.h

@@ -51,10 +51,6 @@ public:
     ProjectExplorer::DeviceTester *createDeviceTester() const override;
     ProjectExplorer::DeviceProcess *createProcess(QObject *parent) const override;
 
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent) override;
-
     QString displayType() const override;
     Utils::OsType osType() const override;
 
@@ -74,8 +70,9 @@ protected:
 
 private:
     void updateVersionNumber() const;
+    void init();
 
-    mutable int m_versionNumber;
+    mutable int m_versionNumber = 0;
 };
 
 } // namespace Qnx

src/plugins/remotelinux/linuxdevice.cpp

@@ -52,9 +52,6 @@ namespace RemoteLinux {
 const char Delimiter0[] = "x--";
 const char Delimiter1[] = "---";
 
-
-static Core::Id openShellActionId() { return "RemoteLinux.OpenShellAction"; }
-
 static QString visualizeNull(QString s)
 {
     return s.replace(QLatin1Char('\0'), QLatin1String("<null>"));
@@ -185,60 +182,6 @@ IDeviceWidget *LinuxDevice::createWidget()
     return new GenericLinuxDeviceConfigurationWidget(sharedFromThis());
 }
 
-QList<Core::Id> LinuxDevice::actionIds() const
-{
-    QList<Core::Id> ids({Core::Id(Constants::GenericDeployKeyToDeviceActionId)});
-    if (Utils::HostOsInfo::isAnyUnixHost())
-        ids << openShellActionId();
-    return ids;
-}
-
-QString LinuxDevice::displayNameForActionId(Core::Id actionId) const
-{
-    QTC_ASSERT(actionIds().contains(actionId), return QString());
-
-    if (actionId == Constants::GenericDeployKeyToDeviceActionId)
-        return tr("Deploy Public Key...");
-    if (actionId == openShellActionId())
-        return tr("Open Remote Shell");
-    return QString(); // Can't happen.
-}
-
-void LinuxDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    QTC_ASSERT(actionIds().contains(actionId), return);
-
-    if (actionId == Constants::GenericDeployKeyToDeviceActionId) {
-        const LinuxDevice::ConstPtr device = sharedFromThis().staticCast<const LinuxDevice>();
-        QDialog * const d = PublicKeyDeploymentDialog::createDialog(device, parent);
-        if (d)
-            d->exec();
-        delete d;
-        return;
-    }
-    if (actionId == openShellActionId()) {
-        DeviceProcess * const proc = createProcess(nullptr);
-        QObject::connect(proc, &DeviceProcess::finished, [proc] {
-            if (!proc->errorString().isEmpty()) {
-                Core::MessageManager::write(tr("Error running remote shell: %1")
-                                            .arg(proc->errorString()),
-                                            Core::MessageManager::ModeSwitch);
-            }
-            proc->deleteLater();
-        });
-        QObject::connect(proc, &DeviceProcess::error, [proc] {
-            Core::MessageManager::write(tr("Error starting remote shell."),
-                                        Core::MessageManager::ModeSwitch);
-            proc->deleteLater();
-        });
-        Runnable runnable;
-        runnable.device = sharedFromThis().staticCast<const LinuxDevice>();
-        proc->setRunInTerminal(true);
-        proc->start(runnable);
-        return;
-    }
-}
-
 Utils::OsType LinuxDevice::osType() const
 {
     return Utils::OsTypeLinux;
@@ -249,10 +192,44 @@ LinuxDevice::LinuxDevice(const QString &name, Core::Id type, MachineType machine
     : IDevice(type, origin, machineType, id)
 {
     setDisplayName(name);
+    init();
 }
 
 LinuxDevice::LinuxDevice(const LinuxDevice &other) = default;
 
+void LinuxDevice::init()
+{
+    addDeviceAction({tr("Deploy Public Key..."), [](const IDevice::Ptr &device, QWidget *parent) {
+        if (auto d = PublicKeyDeploymentDialog::createDialog(device, parent)) {
+            d->exec();
+            delete d;
+        }
+    }});
+
+    if (Utils::HostOsInfo::isAnyUnixHost()) {
+        addDeviceAction({tr("Open Remote Shell"), [](const IDevice::Ptr &device, QWidget *) {
+            DeviceProcess * const proc = device->createProcess(nullptr);
+            QObject::connect(proc, &DeviceProcess::finished, [proc] {
+                if (!proc->errorString().isEmpty()) {
+                    Core::MessageManager::write(tr("Error running remote shell: %1")
+                                                .arg(proc->errorString()),
+                                                Core::MessageManager::ModeSwitch);
+                }
+                proc->deleteLater();
+            });
+            QObject::connect(proc, &DeviceProcess::error, [proc] {
+                Core::MessageManager::write(tr("Error starting remote shell."),
+                                            Core::MessageManager::ModeSwitch);
+                proc->deleteLater();
+            });
+            Runnable runnable;
+            runnable.device = device;
+            proc->setRunInTerminal(true);
+            proc->start(runnable);
+        }});
+    }
+}
+
 LinuxDevice::Ptr LinuxDevice::create()
 {
     return Ptr(new LinuxDevice);

src/plugins/remotelinux/linuxdevice.h

@@ -48,9 +48,6 @@ public:
 
     QString displayType() const override;
     ProjectExplorer::IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent) override;
     Utils::OsType osType() const override;
     ProjectExplorer::IDevice::Ptr clone() const override;
 
@@ -69,13 +66,14 @@ public:
     bool supportsRSync() const;
 
 protected:
-    LinuxDevice() = default;
+    LinuxDevice() { init(); }
     LinuxDevice(const QString &name, Core::Id type,
                              MachineType machineType, Origin origin, Core::Id id);
     LinuxDevice(const LinuxDevice &other);
 
 private:
     LinuxDevice &operator=(const LinuxDevice &);
+    void init();
 };
 
 } // namespace RemoteLinux

src/plugins/remotelinux/remotelinux_constants.h

@@ -30,8 +30,6 @@ namespace Constants {
 
 const char GenericLinuxOsType[] = "GenericLinuxOsType";
 
-const char GenericDeployKeyToDeviceActionId[] = "RemoteLinux.GenericDeployKeyToDeviceAction";
-
 const char EMBEDDED_LINUX_QT[] = "RemoteLinux.EmbeddedLinuxQt";
 
 } // Constants

src/plugins/winrt/winrtdevice.cpp

@@ -77,23 +77,6 @@ IDeviceWidget *WinRtDevice::createWidget()
     return nullptr;
 }
 
-QList<Core::Id> WinRtDevice::actionIds() const
-{
-    return QList<Core::Id>();
-}
-
-QString WinRtDevice::displayNameForActionId(Core::Id actionId) const
-{
-    Q_UNUSED(actionId);
-    return QString();
-}
-
-void WinRtDevice::executeAction(Core::Id actionId, QWidget *parent)
-{
-    Q_UNUSED(actionId);
-    Q_UNUSED(parent);
-}
-
 DeviceProcessSignalOperation::Ptr WinRtDevice::signalOperation() const
 {
     class WinRtDesktopSignalOperation : public DesktopProcessSignalOperation

src/plugins/winrt/winrtdevice.h

@@ -39,9 +39,6 @@ public:
 
     QString displayType() const override;
     ProjectExplorer::IDeviceWidget *createWidget() override;
-    QList<Core::Id> actionIds() const override;
-    QString displayNameForActionId(Core::Id actionId) const override;
-    void executeAction(Core::Id actionId, QWidget *parent) override;
     ProjectExplorer::DeviceProcessSignalOperation::Ptr signalOperation() const override;
     void fromMap(const QVariantMap &map) override;
     QVariantMap toMap() const override;