ProjectExplorer: Fix "Open Terminal With Run Env" for remote targets

Amends 6fa474ead8.
Note that the semantics of this operation are a bit dubious, because it
is supposed to open the terminal at the location of the file
corresponding to the project node, which does not make sense for remote
targets. So for those, we use the run configuration's working directory
instead.

Change-Id: Ie60b54e441daab51e8d51fdfba1d4a71e6a33604
Reviewed-by: hjk <hjk@qt.io>

7 files changed, 98 insertions, 42 deletions

diff --git a/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp b/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp
index 3cd6590f20..a725658b9b 100644
--- a/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp
+++ b/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp
@@ -30,6 +30,8 @@
 #include "desktopdeviceconfigurationwidget.h"
 #include "desktopprocesssignaloperation.h"
 
+#include <coreplugin/fileutils.h>
+
 #include <projectexplorer/projectexplorerconstants.h>
 #include <projectexplorer/runcontrol.h>
 
@@ -58,6 +60,9 @@ DesktopDevice::DesktopDevice()
     const QString portRange =
             QString::fromLatin1("%1-%2").arg(DESKTOP_PORT_START).arg(DESKTOP_PORT_END);
     setFreePorts(Utils::PortList::fromString(portRange));
+    setOpenTerminal([](const Utils::Environment &env, const QString &workingDir) {
+        Core::FileUtils::openTerminal(workingDir, env);
+    });
 }
 
 IDevice::DeviceInfo DesktopDevice::deviceInformation() const
diff --git a/src/plugins/projectexplorer/devicesupport/idevice.cpp b/src/plugins/projectexplorer/devicesupport/idevice.cpp
index 0faade9f9b..86d783a6d9 100644
--- a/src/plugins/projectexplorer/devicesupport/idevice.cpp
+++ b/src/plugins/projectexplorer/devicesupport/idevice.cpp
@@ -153,6 +153,7 @@ public:
     QList<Utils::Icon> deviceIcons;
     QList<IDevice::DeviceAction> deviceActions;
     QVariantMap extraData;
+    IDevice::OpenTerminal openTerminal;
 };
 } // namespace Internal
 
@@ -162,6 +163,11 @@ IDevice::IDevice() : d(new Internal::IDevicePrivate)
 {
 }
 
+void IDevice::setOpenTerminal(const IDevice::OpenTerminal &openTerminal)
+{
+    d->openTerminal = openTerminal;
+}
+
 void IDevice::setupId(Origin origin, Core::Id id)
 {
     d->origin = origin;
@@ -169,6 +175,17 @@ void IDevice::setupId(Origin origin, Core::Id id)
     d->id = id.isValid() ? id : newId();
 }
 
+bool IDevice::canOpenTerminal() const
+{
+    return bool(d->openTerminal);
+}
+
+void IDevice::openTerminal(const Utils::Environment &env, const QString &workingDir) const
+{
+    QTC_ASSERT(canOpenTerminal(), return);
+    d->openTerminal(env, workingDir);
+}
+
 IDevice::~IDevice() = default;
 
 /*!
diff --git a/src/plugins/projectexplorer/devicesupport/idevice.h b/src/plugins/projectexplorer/devicesupport/idevice.h
index db750086a5..b263544000 100644
--- a/src/plugins/projectexplorer/devicesupport/idevice.h
+++ b/src/plugins/projectexplorer/devicesupport/idevice.h
@@ -120,6 +120,7 @@ public:
 // See cpp file for documentation.
 class PROJECTEXPLORER_EXPORT IDevice : public QEnableSharedFromThis<IDevice>
 {
+    friend class Internal::IDevicePrivate;
 public:
     using Ptr = QSharedPointer<IDevice>;
     using ConstPtr = QSharedPointer<const IDevice>;
@@ -218,9 +219,15 @@ public:
 
     void setupId(Origin origin, Core::Id id = Core::Id());
 
+    bool canOpenTerminal() const;
+    void openTerminal(const Utils::Environment &env, const QString &workingDir) const;
+
 protected:
     IDevice();
 
+    using OpenTerminal = std::function<void(const Utils::Environment &, const QString &)>;
+    void setOpenTerminal(const OpenTerminal &openTerminal);
+
 private:
     IDevice(const IDevice &) = delete;
     IDevice &operator=(const IDevice &) = delete;
diff --git a/src/plugins/projectexplorer/projectexplorer.cpp b/src/plugins/projectexplorer/projectexplorer.cpp
index 12eed68bd7..9d0e1181ad 100644
--- a/src/plugins/projectexplorer/projectexplorer.cpp
+++ b/src/plugins/projectexplorer/projectexplorer.cpp
@@ -268,11 +268,20 @@ static Utils::optional<Utils::Environment> buildEnv(const Project *project)
     return project->activeTarget()->activeBuildConfiguration()->environment();
 }
 
-static Utils::optional<Utils::Environment> runEnv(const Project *project)
+static bool canOpenTerminalWithRunEnv(const Project *project)
 {
-    if (!project || !project->activeTarget() || !project->activeTarget()->activeRunConfiguration())
-        return {};
-    return project->activeTarget()->activeRunConfiguration()->runnable().environment;
+    if (!project)
+        return false;
+    const Target * const target = project->activeTarget();
+    if (!target)
+        return false;
+    const RunConfiguration * const runConfig = target->activeRunConfiguration();
+    if (!runConfig)
+        return false;
+    IDevice::ConstPtr device = runConfig->runnable().device;
+    if (!device)
+        device = DeviceKitAspect::device(target->kit());
+    return device && device->canOpenTerminal();
 }
 
 static Target *activeTarget()
@@ -382,6 +391,7 @@ public:
     void updateUnloadProjectMenu();
     using EnvironmentGetter = std::function<Utils::optional<Utils::Environment>(const Project *project)>;
     void openTerminalHere(const EnvironmentGetter &env);
+    void openTerminalHereWithRunEnv();
 
     void invalidateProject(ProjectExplorer::Project *project);
 
@@ -1473,7 +1483,7 @@ bool ProjectExplorerPlugin::initialize(const QStringList &arguments, QString *er
 
     connect(dd->m_openTerminalHere, &QAction::triggered, dd, []() { dd->openTerminalHere(sysEnv); });
     connect(dd->m_openTerminalHereBuildEnv, &QAction::triggered, dd, []() { dd->openTerminalHere(buildEnv); });
-    connect(dd->m_openTerminalHereRunEnv, &QAction::triggered, dd, []() { dd->openTerminalHere(runEnv); });
+    connect(dd->m_openTerminalHereRunEnv, &QAction::triggered, dd, []() { dd->openTerminalHereWithRunEnv(); });
 
     connect(dd->m_filePropertiesAction, &QAction::triggered, this, []() {
                 const Node *currentNode = ProjectTree::currentNode();
@@ -3205,7 +3215,7 @@ void ProjectExplorerPluginPrivate::updateContextMenuActions()
 
         Project *project = ProjectTree::currentProject();
         m_openTerminalHereBuildEnv->setVisible(bool(buildEnv(project)));
-        m_openTerminalHereRunEnv->setVisible(bool(runEnv(project)));
+        m_openTerminalHereRunEnv->setVisible(canOpenTerminalWithRunEnv(project));
 
         if (pn && project) {
             if (pn == project->rootProjectNode()) {
@@ -3544,6 +3554,28 @@ void ProjectExplorerPluginPrivate::openTerminalHere(const EnvironmentGetter &env
     FileUtils::openTerminal(directoryFor(currentNode), environment.value());
 }
 
+void ProjectExplorerPluginPrivate::openTerminalHereWithRunEnv()
+{
+    const Node *currentNode = ProjectTree::currentNode();
+    QTC_ASSERT(currentNode, return);
+
+    const Project * const project = ProjectTree::projectForNode(currentNode);
+    QTC_ASSERT(project, return);
+    const Target * const target = project->activeTarget();
+    QTC_ASSERT(target, return);
+    const RunConfiguration * const runConfig = target->activeRunConfiguration();
+    QTC_ASSERT(runConfig, return);
+
+    const Runnable runnable = runConfig->runnable();
+    IDevice::ConstPtr device = runnable.device;
+    if (!device)
+        device = DeviceKitAspect::device(target->kit());
+    QTC_ASSERT(device && device->canOpenTerminal(), return);
+    const QString workingDir = device->type() == Constants::DESKTOP_DEVICE_TYPE
+            ? directoryFor(currentNode) : runnable.workingDirectory;
+    device->openTerminal(runnable.environment, workingDir);
+}
+
 void ProjectExplorerPluginPrivate::removeFile()
 {
     const Node *currentNode = ProjectTree::currentNode();
diff --git a/src/plugins/remotelinux/linuxdevice.cpp b/src/plugins/remotelinux/linuxdevice.cpp
index e6b22eb4bb..6e05739c01 100644
--- a/src/plugins/remotelinux/linuxdevice.cpp
+++ b/src/plugins/remotelinux/linuxdevice.cpp
@@ -196,9 +196,38 @@ LinuxDevice::LinuxDevice()
         }
     }});
 
+    setOpenTerminal([this](const Utils::Environment &env, const QString &workingDir) {
+        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();
+        runnable.environment = env;
+        runnable.workingDirectory = workingDir;
+
+        // It seems we cannot pass an environment to OpenSSH dynamically
+        // without specifying an executable.
+        if (env.size() > 0)
+            runnable.executable = "/bin/sh";
+
+        proc->setRunInTerminal(true);
+        proc->start(runnable);
+    });
+
     if (Utils::HostOsInfo::isAnyUnixHost()) {
         addDeviceAction({tr("Open Remote Shell"), [](const IDevice::Ptr &device, QWidget *) {
-            device.staticCast<LinuxDevice>()->startRemoteShell(Utils::Environment());
+            device->openTerminal(Utils::Environment(), QString());
         }});
     }
 }
@@ -268,36 +297,6 @@ bool LinuxDevice::supportsRSync() const
     return extraData("RemoteLinux.SupportsRSync").toBool();
 }
 
-void LinuxDevice::startRemoteShell(const Utils::Environment &env) const
-{
-    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();
-    runnable.environment = env;
-
-    // It seems we cannot pass an environment to OpenSSH dynamically
-    // without specifying an executable.
-    if (env.size() > 0)
-        runnable.executable = "/bin/sh";
-
-    proc->setRunInTerminal(true);
-    proc->start(runnable);
-}
-
-
 namespace Internal {
 
 // Factory
diff --git a/src/plugins/remotelinux/linuxdevice.h b/src/plugins/remotelinux/linuxdevice.h
index c8e8d8bf31..18faf4ae05 100644
--- a/src/plugins/remotelinux/linuxdevice.h
+++ b/src/plugins/remotelinux/linuxdevice.h
@@ -32,8 +32,6 @@
 
 #include <QCoreApplication>
 
-namespace Utils { class Environment; }
-
 namespace RemoteLinux {
 
 class REMOTELINUX_EXPORT LinuxDevice : public ProjectExplorer::IDevice
@@ -64,8 +62,6 @@ public:
     void setSupportsRsync(bool supportsRsync);
     bool supportsRSync() const;
 
-    void startRemoteShell(const Utils::Environment &env) const;
-
 protected:
     LinuxDevice();
 };
diff --git a/src/plugins/remotelinux/remotelinuxenvironmentaspectwidget.cpp b/src/plugins/remotelinux/remotelinuxenvironmentaspectwidget.cpp
index b244a5e129..0d94cf3ffa 100644
--- a/src/plugins/remotelinux/remotelinuxenvironmentaspectwidget.cpp
+++ b/src/plugins/remotelinux/remotelinuxenvironmentaspectwidget.cpp
@@ -78,7 +78,7 @@ RemoteLinuxEnvironmentAspectWidget::RemoteLinuxEnvironmentAspectWidget
         }
         const auto linuxDevice = device.dynamicCast<const LinuxDevice>();
         QTC_ASSERT(linuxDevice, return);
-        linuxDevice->startRemoteShell(env);
+        linuxDevice->openTerminal(env, QString());
     };
     envWidget()->setOpenTerminalFunc(openTerminalFunc);
 }