Docker: Simplify settings access

Change-Id: I0ecbba9d29b041b06dac26159be702a38db42185
Reviewed-by: Marcus Tillmanns <marcus.tillmanns@qt.io>

src/plugins/docker/dockerapi.cpp

@@ -22,8 +22,7 @@ namespace Docker::Internal {
 
 DockerApi *s_instance{nullptr};
 
-DockerApi::DockerApi(DockerSettings *settings)
+DockerApi::DockerApi()
-    : m_settings(settings)
 {
     s_instance = this;
 }
@@ -103,7 +102,7 @@ std::optional<bool> DockerApi::isDockerDaemonAvailable(bool async)
 
 FilePath DockerApi::dockerClient()
 {
-    return m_settings->dockerBinaryPath();
+    return settings().dockerBinaryPath();
 }
 
 } // Docker::Internal

src/plugins/docker/dockerapi.h

@@ -20,7 +20,7 @@ class DockerApi : public QObject
     Q_OBJECT
 
 public:
-    DockerApi(DockerSettings *settings);
+    DockerApi();
 
     static DockerApi *instance();
 
@@ -40,7 +40,6 @@ private:
 
     std::optional<bool> m_dockerDaemonAvailable;
     QMutex m_daemonCheckGuard;
-    DockerSettings *m_settings;
 };
 
 } // Docker::Internal

src/plugins/docker/dockerdevice.cpp

@@ -90,16 +90,15 @@ namespace Docker::Internal {
 class ContainerShell : public Utils::DeviceShell
 {
 public:
-    ContainerShell(DockerSettings *settings, const QString &containerId, const FilePath &devicePath)
+    ContainerShell(const QString &containerId, const FilePath &devicePath)
-        : m_settings(settings)
+        : m_containerId(containerId)
-        , m_containerId(containerId)
         , m_devicePath(devicePath)
     {}
 
 private:
     void setupShellProcess(Process *shellProcess) final
     {
-        shellProcess->setCommand({m_settings->dockerBinaryPath(),
+        shellProcess->setCommand({settings().dockerBinaryPath(),
                                   {"container", "start", "-i", "-a", m_containerId}});
     }
 
@@ -111,7 +110,6 @@ private:
     }
 
 private:
-    DockerSettings *m_settings;
     QString m_containerId;
     FilePath m_devicePath;
 };
@@ -133,10 +131,9 @@ public:
 class DockerDevicePrivate : public QObject
 {
 public:
-    DockerDevicePrivate(DockerDevice *parent, DockerSettings *settings, DockerDeviceData data)
+    DockerDevicePrivate(DockerDevice *parent, DockerDeviceData data)
         : q(parent)
         , m_data(std::move(data))
-        , m_settings(settings)
     {}
 
     ~DockerDevicePrivate() { stopCurrentContainer(); }
@@ -152,7 +149,6 @@ public:
     QString containerId() { return m_container; }
     DockerDeviceData data() { return m_data; }
     void setData(const DockerDeviceData &data);
-    DockerSettings *settings() { return m_settings; }
 
     QString repoAndTag() const { return m_data.repoAndTag(); }
     QString repoAndTagEncoded() const { return m_data.repoAndTagEncoded(); }
@@ -190,7 +186,6 @@ public:
 
     DockerDevice *const q;
     DockerDeviceData m_data;
-    DockerSettings *m_settings;
 
     struct TemporaryMountInfo
     {
@@ -414,8 +409,8 @@ QString DockerDeviceFileAccess::mapToDevicePath(const QString &hostPath) const
     return newPath;
 }
 
-DockerDevice::DockerDevice(DockerSettings *settings, const DockerDeviceData &data)
+DockerDevice::DockerDevice(const DockerDeviceData &data)
-    : d(new DockerDevicePrivate(this, settings, data))
+    : d(new DockerDevicePrivate(this, data))
 {
     setFileAccess(&d->m_fileAccess);
     setDisplayType(Tr::tr("Docker"));
@@ -492,13 +487,10 @@ CommandLine DockerDevicePrivate::withDockerExecCmd(const CommandLine &cmd,
                                                    bool withPty,
                                                    bool withMarker)
 {
-    if (!m_settings)
-        return {};
-
     if (!updateContainerAccess())
         return {};
 
-    CommandLine dockerCmd{m_settings->dockerBinaryPath(), {"exec"}};
+    CommandLine dockerCmd{settings().dockerBinaryPath(), {"exec"}};
 
     if (interactive)
         dockerCmd.addArg("-i");
@@ -538,8 +530,6 @@ CommandLine DockerDevicePrivate::withDockerExecCmd(const CommandLine &cmd,
 
 void DockerDevicePrivate::stopCurrentContainer()
 {
-    if (!m_settings)
-        return;
     if (m_container.isEmpty())
         return;
     if (!DockerApi::isDockerDaemonAvailable(false).value_or(false))
@@ -555,7 +545,7 @@ void DockerDevicePrivate::stopCurrentContainer()
     }
 
     Process proc;
-    proc.setCommand({m_settings->dockerBinaryPath(), {"container", "stop", m_container}});
+    proc.setCommand({settings().dockerBinaryPath(), {"container", "stop", m_container}});
 
     m_container.clear();
 
@@ -660,7 +650,7 @@ bool DockerDevicePrivate::isImageAvailable() const
 {
     Process proc;
     proc.setCommand(
-        {m_settings->dockerBinaryPath(),
+        {settings().dockerBinaryPath(),
          {"image", "list", m_data.repoAndTag(), "--format", "{{.Repository}}:{{.Tag}}"}});
     proc.runBlocking();
     if (proc.result() != ProcessResult::FinishedWithSuccess)
@@ -674,15 +664,12 @@ bool DockerDevicePrivate::isImageAvailable() const
 
 bool DockerDevicePrivate::createContainer()
 {
-    if (!m_settings)
-        return false;
-
     if (!isImageAvailable())
         return false;
 
     const QString display = HostOsInfo::isLinuxHost() ? QString(":0")
                                                       : QString("host.docker.internal:0");
-    CommandLine dockerCreate{m_settings->dockerBinaryPath(),
+    CommandLine dockerCreate{settings().dockerBinaryPath(),
                              {"create",
                               "-i",
                               "--rm",
@@ -734,7 +721,7 @@ bool DockerDevicePrivate::startContainer()
     if (!createContainer())
         return false;
 
-    m_shell = std::make_unique<ContainerShell>(m_settings, m_container, q->rootPath());
+    m_shell = std::make_unique<ContainerShell>(m_container, q->rootPath());
 
     connect(m_shell.get(), &DeviceShell::done, this, [this](const ProcessResultData &resultData) {
         if (m_shell)
@@ -979,9 +966,8 @@ public:
 class DockerDeviceSetupWizard final : public QDialog
 {
 public:
-    DockerDeviceSetupWizard(DockerSettings *settings)
+    DockerDeviceSetupWizard()
         : QDialog(ICore::dialogParent())
-        , m_settings(settings)
     {
         setWindowTitle(Tr::tr("Docker Image Selection"));
         resize(800, 600);
@@ -1050,7 +1036,7 @@ public:
         connect(m_buttons, &QDialogButtonBox::rejected, this, &QDialog::reject);
         m_buttons->button(QDialogButtonBox::Ok)->setEnabled(false);
 
-        CommandLine cmd{m_settings->dockerBinaryPath(),
+        CommandLine cmd{settings().dockerBinaryPath(),
                         {"images", "--format", "{{.ID}}\\t{{.Repository}}\\t{{.Tag}}\\t{{.Size}}"}};
         m_log->append(Tr::tr("Running \"%1\"\n").arg(cmd.toUserOutput()));
 
@@ -1106,7 +1092,7 @@ public:
             m_proxyModel->mapToSource(selectedRows.front()));
         QTC_ASSERT(item, return {});
 
-        auto device = DockerDevice::create(m_settings, *item);
+        auto device = DockerDevice::create(*item);
 
         return device;
     }
@@ -1117,7 +1103,6 @@ public:
     SortFilterModel *m_proxyModel = nullptr;
     QTextBrowser *m_log = nullptr;
     QDialogButtonBox *m_buttons;
-    DockerSettings *m_settings;
 
     Process *m_process = nullptr;
     QString m_selectedId;
@@ -1125,19 +1110,19 @@ public:
 
 // Factory
 
-DockerDeviceFactory::DockerDeviceFactory(DockerSettings *settings)
+DockerDeviceFactory::DockerDeviceFactory()
     : IDeviceFactory(Constants::DOCKER_DEVICE_TYPE)
 {
     setDisplayName(Tr::tr("Docker Device"));
     setIcon(QIcon());
-    setCreator([settings] {
+    setCreator([] {
-        DockerDeviceSetupWizard wizard(settings);
+        DockerDeviceSetupWizard wizard;
         if (wizard.exec() != QDialog::Accepted)
             return IDevice::Ptr();
         return wizard.device();
     });
-    setConstructionFunction([settings, this] {
+    setConstructionFunction([this] {
-        auto device = DockerDevice::create(settings, {});
+        auto device = DockerDevice::create({});
         QMutexLocker lk(&m_deviceListMutex);
         m_existingDevices.push_back(device);
         return device;
@@ -1193,7 +1178,6 @@ Environment DockerDevicePrivate::environment()
 void DockerDevicePrivate::shutdown()
 {
     m_isShutdown = true;
-    m_settings = nullptr;
     stopCurrentContainer();
 }
 

src/plugins/docker/dockerdevice.h

@@ -58,14 +58,14 @@ public:
     using Ptr = QSharedPointer<DockerDevice>;
     using ConstPtr = QSharedPointer<const DockerDevice>;
 
-    explicit DockerDevice(DockerSettings *settings, const DockerDeviceData &data);
+    explicit DockerDevice(const DockerDeviceData &data);
     ~DockerDevice();
 
     void shutdown();
 
-    static Ptr create(DockerSettings *settings, const DockerDeviceData &data)
+    static Ptr create(const DockerDeviceData &data)
     {
-        return Ptr(new DockerDevice(settings, data));
+        return Ptr(new DockerDevice(data));
     }
 
     ProjectExplorer::IDeviceWidget *createWidget() override;
@@ -114,7 +114,7 @@ private:
 class DockerDeviceFactory final : public ProjectExplorer::IDeviceFactory
 {
 public:
-    DockerDeviceFactory(DockerSettings *settings);
+    DockerDeviceFactory();
 
     void shutdownExistingDevices();
 

src/plugins/docker/dockerplugin.cpp

@@ -6,7 +6,6 @@
 #include "dockerapi.h"
 #include "dockerconstants.h"
 #include "dockerdevice.h"
-#include "dockersettings.h"
 
 #include <projectexplorer/projectexplorerconstants.h>
 
@@ -22,13 +21,13 @@ namespace Docker::Internal {
 class DockerPluginPrivate
 {
 public:
-    ~DockerPluginPrivate() {
+    ~DockerPluginPrivate()
+    {
         m_deviceFactory.shutdownExistingDevices();
     }
 
-    DockerSettings m_settings;
+    DockerDeviceFactory m_deviceFactory;
-    DockerDeviceFactory m_deviceFactory{&m_settings};
+    DockerApi m_dockerApi;
-    DockerApi m_dockerApi{&m_settings};
 };
 
 DockerPlugin::DockerPlugin()

src/plugins/docker/dockersettings.cpp

@@ -6,6 +6,8 @@
 #include "dockerconstants.h"
 #include "dockertr.h"
 
+#include <coreplugin/dialogs/ioptionspage.h>
+
 #include <projectexplorer/projectexplorerconstants.h>
 
 #include <utils/hostosinfo.h>
@@ -15,12 +17,16 @@ using namespace Utils;
 
 namespace Docker::Internal {
 
+DockerSettings &settings()
+{
+    static DockerSettings theSettings;
+    return theSettings;
+}
+
 DockerSettings::DockerSettings()
 {
+    setAutoApply(false);
     setSettingsGroup(Constants::DOCKER);
-    setId(Docker::Constants::DOCKER_SETTINGS_ID);
-    setDisplayName(Tr::tr("Docker"));
-    setCategory(ProjectExplorer::Constants::DEVICE_SETTINGS_CATEGORY);
 
     setLayouter([this] {
         using namespace Layouting;
@@ -52,4 +58,18 @@ DockerSettings::DockerSettings()
     readSettings();
 }
 
+class DockerSettingsPage final : public Core::IOptionsPage
+{
+public:
+    DockerSettingsPage()
+    {
+        setId(Docker::Constants::DOCKER_SETTINGS_ID);
+        setDisplayName(Tr::tr("Docker"));
+        setCategory(ProjectExplorer::Constants::DEVICE_SETTINGS_CATEGORY);
+        setSettingsProvider([] { return &settings(); });
+    }
+};
+
+const DockerSettingsPage settingsPage;
+
 } // Docker::Internal

src/plugins/docker/dockersettings.h

@@ -3,11 +3,11 @@
 
 #pragma once
 
-#include <coreplugin/dialogs/ioptionspage.h>
+#include <utils/aspects.h>
 
 namespace Docker::Internal {
 
-class DockerSettings final : public Core::PagedSettings
+class DockerSettings final : public Utils::AspectContainer
 {
 public:
     DockerSettings();
@@ -15,4 +15,6 @@ public:
     Utils::FilePathAspect dockerBinaryPath{this};
 };
 
+DockerSettings &settings();
+
 } // Docker::Internal