From 66371198ecbc47e3eae87f63997eb0c189dc83be Mon Sep 17 00:00:00 2001 From: hjk Date: Wed, 15 Jan 2020 17:48:50 +0100 Subject: [PATCH] BareMetal: Use the plain creation and fromMap(toMap()) for clones This allows to drop the hierarchy of copy constructors and makes the setup more similar to what various other factory types in the ProjectExplorer use nowadays. This slightly changes clone() behavior in some cases. If that's not considered harmless, we can introduce some 'polishAfterClone' or similar. Change-Id: I935a1f4165bb88e517c136bf4594087aedfdd760 Reviewed-by: Denis Shienkov Reviewed-by: Christian Stenger --- .../debugserverproviderssettingspage.cpp | 37 ++++++++++--------- .../gdb/eblinkgdbserverprovider.cpp | 21 ----------- .../gdb/eblinkgdbserverprovider.h | 4 +- .../gdb/jlinkgdbserverprovider.cpp | 5 --- .../debugservers/gdb/jlinkgdbserverprovider.h | 3 +- .../gdb/openocdgdbserverprovider.cpp | 5 --- .../gdb/openocdgdbserverprovider.h | 1 - .../gdb/stlinkutilgdbserverprovider.cpp | 16 -------- .../gdb/stlinkutilgdbserverprovider.h | 4 +- .../baremetal/idebugserverprovider.cpp | 13 +++---- src/plugins/baremetal/idebugserverprovider.h | 15 ++++---- 11 files changed, 35 insertions(+), 89 deletions(-) diff --git a/src/plugins/baremetal/debugserverproviderssettingspage.cpp b/src/plugins/baremetal/debugserverproviderssettingspage.cpp index d1dafb2e9ea..dfb5a456c9f 100644 --- a/src/plugins/baremetal/debugserverproviderssettingspage.cpp +++ b/src/plugins/baremetal/debugserverproviderssettingspage.cpp @@ -282,10 +282,10 @@ public: void removeProvider(); void updateState(); - void createProvider(IDebugServerProviderFactory *f); +private: + void addProviderToModel(IDebugServerProvider *p); QModelIndex currentIndex() const; -public: DebugServerProviderModel m_model; QItemSelectionModel *m_selectionModel = nullptr; QTreeView *m_providerView = nullptr; @@ -357,11 +357,24 @@ DebugServerProvidersSettingsWidget::DebugServerProvidersSettingsWidget() for (const auto f : DebugServerProviderManager::factories()) { const auto action = new QAction(addMenu); action->setText(f->displayName()); - connect(action, &QAction::triggered, this, [this, f] { createProvider(f); }); + connect(action, &QAction::triggered, this, [this, f] { addProviderToModel(f->create()); }); addMenu->addAction(action); } - connect(m_cloneButton, &QAbstractButton::clicked, this, [this] { createProvider(nullptr); }); + connect(m_cloneButton, &QAbstractButton::clicked, this, [this] { + if (const IDebugServerProvider *old = m_model.provider(currentIndex())) { + QString id = old->id(); + for (const auto f : DebugServerProviderManager::factories()) { + if (id.startsWith(f->id())) { + IDebugServerProvider *p = f->create(); + p->fromMap(old->toMap()); + p->setDisplayName(tr("Clone of %1").arg(old->displayName())); + p->resetId(); + addProviderToModel(p); + } + } + } + }); m_addButton->setMenu(addMenu); @@ -387,21 +400,9 @@ void DebugServerProvidersSettingsWidget::providerSelectionChanged() updateState(); } -void DebugServerProvidersSettingsWidget::createProvider(IDebugServerProviderFactory *f) +void DebugServerProvidersSettingsWidget::addProviderToModel(IDebugServerProvider *provider) { - IDebugServerProvider *provider = nullptr; - if (!f) { - const IDebugServerProvider *old = m_model.provider(currentIndex()); - if (!old) - return; - provider = old->clone(); - } else { - provider = f->create(); - } - - if (!provider) - return; - + QTC_ASSERT(provider, return); m_model.markForAddition(provider); m_selectionModel->select(m_model.indexForProvider(provider), diff --git a/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.cpp b/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.cpp index 3c408dcef9a..0a6e76e0386 100644 --- a/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.cpp +++ b/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.cpp @@ -71,22 +71,6 @@ EBlinkGdbServerProvider::EBlinkGdbServerProvider() setTypeDisplayName(EBlinkGdbServerProviderFactory::tr("EBlink")); } -EBlinkGdbServerProvider::EBlinkGdbServerProvider( - const EBlinkGdbServerProvider &other) - : GdbServerProvider(other) - , m_executableFile(other.m_executableFile) - , m_verboseLevel(0) - , m_interfaceType(other.m_interfaceType) - , m_deviceScript(other.m_deviceScript) - , m_interfaceResetOnConnect(other.m_interfaceResetOnConnect) - , m_interfaceSpeed(other.m_interfaceSpeed) - , m_interfaceExplicidDevice(other.m_interfaceExplicidDevice) - , m_targetName(other.m_targetName) - , m_targetDisableStack(other.m_targetDisableStack) - , m_gdbShutDownAfterDisconnect(other.m_gdbShutDownAfterDisconnect) - , m_gdbNotUseCache(other.m_gdbNotUseCache){ -} - QString EBlinkGdbServerProvider::defaultInitCommands() { return {"monitor reset halt\n" @@ -184,11 +168,6 @@ bool EBlinkGdbServerProvider::isValid() const } } -GdbServerProvider *EBlinkGdbServerProvider::clone() const -{ - return new EBlinkGdbServerProvider(*this); -} - QVariantMap EBlinkGdbServerProvider::toMap() const { QVariantMap data = GdbServerProvider::toMap(); diff --git a/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.h b/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.h index e60ce408356..33a708e6200 100644 --- a/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.h +++ b/src/plugins/baremetal/debugservers/gdb/eblinkgdbserverprovider.h @@ -50,7 +50,6 @@ public: bool operator==(const IDebugServerProvider &other) const final; GdbServerProviderConfigWidget *configurationWidget() final; - GdbServerProvider *clone() const final; QString channelString() const final; Utils::CommandLine command() const final; @@ -59,8 +58,7 @@ public: bool isValid() const final; private: - explicit EBlinkGdbServerProvider(); - explicit EBlinkGdbServerProvider(const EBlinkGdbServerProvider &); + EBlinkGdbServerProvider(); static QString defaultInitCommands(); static QString defaultResetCommands(); diff --git a/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.cpp b/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.cpp index b93c69a2ec9..5f289d262a4 100644 --- a/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.cpp +++ b/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.cpp @@ -141,11 +141,6 @@ bool JLinkGdbServerProvider::isValid() const return true; } -GdbServerProvider *JLinkGdbServerProvider::clone() const -{ - return new JLinkGdbServerProvider(*this); -} - QVariantMap JLinkGdbServerProvider::toMap() const { QVariantMap data = GdbServerProvider::toMap(); diff --git a/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.h b/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.h index c955bcd63eb..c8936315b3c 100644 --- a/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.h +++ b/src/plugins/baremetal/debugservers/gdb/jlinkgdbserverprovider.h @@ -47,7 +47,6 @@ public: bool operator==(const IDebugServerProvider &other) const final; GdbServerProviderConfigWidget *configurationWidget() final; - GdbServerProvider *clone() const final; QString channelString() const final; Utils::CommandLine command() const final; @@ -56,7 +55,7 @@ public: bool isValid() const final; private: - explicit JLinkGdbServerProvider(); + JLinkGdbServerProvider(); static QString defaultInitCommands(); static QString defaultResetCommands(); diff --git a/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.cpp b/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.cpp index caf4ecd274b..48e2c59c2bc 100644 --- a/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.cpp +++ b/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.cpp @@ -148,11 +148,6 @@ bool OpenOcdGdbServerProvider::isValid() const return true; } -GdbServerProvider *OpenOcdGdbServerProvider::clone() const -{ - return new OpenOcdGdbServerProvider(*this); -} - QVariantMap OpenOcdGdbServerProvider::toMap() const { QVariantMap data = GdbServerProvider::toMap(); diff --git a/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.h b/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.h index 60e328661b5..4ea11e1046a 100644 --- a/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.h +++ b/src/plugins/baremetal/debugservers/gdb/openocdgdbserverprovider.h @@ -47,7 +47,6 @@ public: bool operator==(const IDebugServerProvider &other) const final; GdbServerProviderConfigWidget *configurationWidget() final; - GdbServerProvider *clone() const final; QString channelString() const final; Utils::CommandLine command() const final; diff --git a/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.cpp b/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.cpp index 166cdb3be70..347f0801da4 100644 --- a/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.cpp +++ b/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.cpp @@ -63,17 +63,6 @@ StLinkUtilGdbServerProvider::StLinkUtilGdbServerProvider() setTypeDisplayName(StLinkUtilGdbServerProviderFactory::tr("ST-LINK Utility")); } -StLinkUtilGdbServerProvider::StLinkUtilGdbServerProvider( - const StLinkUtilGdbServerProvider &other) - : GdbServerProvider(other) - , m_executableFile(other.m_executableFile) - , m_verboseLevel(0) - , m_extendedMode(false) - , m_resetBoard(true) - , m_transport(RawUsb) -{ -} - QString StLinkUtilGdbServerProvider::defaultInitCommands() { return {"load\n"}; @@ -141,11 +130,6 @@ bool StLinkUtilGdbServerProvider::isValid() const return true; } -GdbServerProvider *StLinkUtilGdbServerProvider::clone() const -{ - return new StLinkUtilGdbServerProvider(*this); -} - QVariantMap StLinkUtilGdbServerProvider::toMap() const { QVariantMap data = GdbServerProvider::toMap(); diff --git a/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.h b/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.h index 9bad7c932db..e48116fcc73 100644 --- a/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.h +++ b/src/plugins/baremetal/debugservers/gdb/stlinkutilgdbserverprovider.h @@ -50,7 +50,6 @@ public: bool operator==(const IDebugServerProvider &other) const final; GdbServerProviderConfigWidget *configurationWidget() final; - GdbServerProvider *clone() const final; QString channelString() const final; Utils::CommandLine command() const final; @@ -59,8 +58,7 @@ public: bool isValid() const final; private: - explicit StLinkUtilGdbServerProvider(); - explicit StLinkUtilGdbServerProvider(const StLinkUtilGdbServerProvider &); + StLinkUtilGdbServerProvider(); static QString defaultInitCommands(); static QString defaultResetCommands(); diff --git a/src/plugins/baremetal/idebugserverprovider.cpp b/src/plugins/baremetal/idebugserverprovider.cpp index abe35db30b6..1788f8df705 100644 --- a/src/plugins/baremetal/idebugserverprovider.cpp +++ b/src/plugins/baremetal/idebugserverprovider.cpp @@ -61,14 +61,6 @@ IDebugServerProvider::IDebugServerProvider(const QString &id) { } -IDebugServerProvider::IDebugServerProvider(const IDebugServerProvider &provider) - : m_id(createId(provider.m_id)) -{ - m_displayName = QCoreApplication::translate( - "BareMetal::IDebugServerProvider", "Clone of %1") - .arg(provider.displayName()); -} - IDebugServerProvider::~IDebugServerProvider() { const QSet devices = m_devices; @@ -189,6 +181,11 @@ void IDebugServerProvider::providerUpdated() DebugServerProviderManager::notifyAboutUpdate(this); } +void IDebugServerProvider::resetId() +{ + m_id = createId(m_id); +} + bool IDebugServerProvider::fromMap(const QVariantMap &data) { m_id = data.value(idKeyC).toString(); diff --git a/src/plugins/baremetal/idebugserverprovider.h b/src/plugins/baremetal/idebugserverprovider.h index 3094454d93c..34167772964 100644 --- a/src/plugins/baremetal/idebugserverprovider.h +++ b/src/plugins/baremetal/idebugserverprovider.h @@ -61,6 +61,11 @@ class IDebugServerProviderConfigWidget; class IDebugServerProvider { +protected: + explicit IDebugServerProvider(const QString &id); + IDebugServerProvider(const IDebugServerProvider &provider) = delete; + IDebugServerProvider &operator=(const IDebugServerProvider &provider) = delete; + public: virtual ~IDebugServerProvider(); @@ -81,9 +86,8 @@ public: virtual IDebugServerProviderConfigWidget *configurationWidget() = 0; - virtual IDebugServerProvider *clone() const = 0; - virtual QVariantMap toMap() const; + virtual bool fromMap(const QVariantMap &data); virtual bool aboutToRun(Debugger::DebuggerRunTool *runTool, QString &errorMessage) const = 0; @@ -96,16 +100,12 @@ public: void unregisterDevice(BareMetalDevice *device); protected: - explicit IDebugServerProvider(const QString &id); - explicit IDebugServerProvider(const IDebugServerProvider &provider); - void setTypeDisplayName(const QString &typeDisplayName); void setEngineType(Debugger::DebuggerEngineType engineType); void setSettingsKeyBase(const QString &settingsBase); void providerUpdated(); - - virtual bool fromMap(const QVariantMap &data); + void resetId(); QString m_id; mutable QString m_displayName; @@ -115,6 +115,7 @@ protected: Debugger::DebuggerEngineType m_engineType = Debugger::NoEngineType; QSet m_devices; + friend class DebugServerProvidersSettingsWidget; friend class IDebugServerProviderConfigWidget; friend class IDebugServerProviderFactory; };