From a235d338375f66f6abe58fc4f303abee6d65a716 Mon Sep 17 00:00:00 2001 From: hjk Date: Wed, 30 Jan 2019 19:13:58 +0100 Subject: [PATCH] ProjectExplorer: Remove BuildConfigurationFactory::priority It's essentially an intrusive hack that's used only to prioritize Ios+Qmake over plain Qmake. The effect is now achieved by an arguably equally evil dependency on the construction order of BuildConfiguration factories. It can be argued, however, that this is a feature as it allows user plugins to intentionally override core functionality by using the standard setup pattern and depending on the to-be- overridden plugin. Change-Id: Ic1efa305daf3ca19a880d2a7ccb40e2768d8f57c Reviewed-by: Eike Ziller --- src/plugins/ios/iosbuildconfiguration.cpp | 1 - .../projectexplorer/buildconfiguration.cpp | 86 ++++++------------- .../projectexplorer/buildconfiguration.h | 8 -- .../tests/qmlprofilerdetailsrewriter_test.cpp | 5 -- 4 files changed, 25 insertions(+), 75 deletions(-) diff --git a/src/plugins/ios/iosbuildconfiguration.cpp b/src/plugins/ios/iosbuildconfiguration.cpp index cf5298e7220..3f6de58d386 100644 --- a/src/plugins/ios/iosbuildconfiguration.cpp +++ b/src/plugins/ios/iosbuildconfiguration.cpp @@ -145,7 +145,6 @@ IosBuildConfigurationFactory::IosBuildConfigurationFactory() registerBuildConfiguration(QmakeProjectManager::Constants::QMAKE_BC_ID); addSupportedTargetDeviceType(Constants::IOS_DEVICE_TYPE); addSupportedTargetDeviceType(Constants::IOS_SIMULATOR_TYPE); - setBasePriority(1); } } // namespace Internal diff --git a/src/plugins/projectexplorer/buildconfiguration.cpp b/src/plugins/projectexplorer/buildconfiguration.cpp index 9b29f628334..5297ec7c84d 100644 --- a/src/plugins/projectexplorer/buildconfiguration.cpp +++ b/src/plugins/projectexplorer/buildconfiguration.cpp @@ -325,7 +325,8 @@ static QList g_buildConfigurationFactories; BuildConfigurationFactory::BuildConfigurationFactory() { - g_buildConfigurationFactories.append(this); + // Note: Order matters as first-in-queue wins. + g_buildConfigurationFactories.prepend(this); } BuildConfigurationFactory::~BuildConfigurationFactory() @@ -333,11 +334,6 @@ BuildConfigurationFactory::~BuildConfigurationFactory() g_buildConfigurationFactories.removeOne(this); } -int BuildConfigurationFactory::priority(const Target *parent) const -{ - return canHandle(parent) ? m_basePriority : -1; -} - const QList BuildConfigurationFactory::reportIssues(ProjectExplorer::Kit *kit, const QString &projectPath, const QString &buildDir) const { @@ -363,44 +359,27 @@ bool BuildConfigurationFactory::supportsTargetDeviceType(Core::Id id) const return m_supportedTargetDeviceTypes.contains(id); } -int BuildConfigurationFactory::priority(const Kit *k, const QString &projectPath) const -{ - QTC_ASSERT(!m_supportedProjectMimeTypeName.isEmpty(), return -1); - if (k && Utils::mimeTypeForFile(projectPath).matchesName(m_supportedProjectMimeTypeName) - && supportsTargetDeviceType(DeviceTypeKitInformation::deviceTypeId(k))) { - return m_basePriority; - } - return -1; -} - // setup BuildConfigurationFactory *BuildConfigurationFactory::find(const Kit *k, const QString &projectPath) { - BuildConfigurationFactory *factory = nullptr; - int priority = -1; - for (BuildConfigurationFactory *i : g_buildConfigurationFactories) { - int iPriority = i->priority(k, projectPath); - if (iPriority > priority) { - factory = i; - priority = iPriority; - } + QTC_ASSERT(k, return nullptr); + const Core::Id deviceType = DeviceTypeKitInformation::deviceTypeId(k); + for (BuildConfigurationFactory *factory : g_buildConfigurationFactories) { + if (Utils::mimeTypeForFile(projectPath).matchesName(factory->m_supportedProjectMimeTypeName) + && factory->supportsTargetDeviceType(deviceType)) + return factory; } - return factory; + return nullptr; } // create BuildConfigurationFactory * BuildConfigurationFactory::find(Target *parent) { - BuildConfigurationFactory *factory = nullptr; - int priority = -1; - for (BuildConfigurationFactory *i : g_buildConfigurationFactories) { - int iPriority = i->priority(parent); - if (iPriority > priority) { - factory = i; - priority = iPriority; - } + for (BuildConfigurationFactory *factory : g_buildConfigurationFactories) { + if (factory->canHandle(parent)) + return factory; } - return factory; + return nullptr; } void BuildConfigurationFactory::setSupportedProjectType(Core::Id id) @@ -418,11 +397,6 @@ void BuildConfigurationFactory::addSupportedTargetDeviceType(Core::Id id) m_supportedTargetDeviceTypes.append(id); } -void BuildConfigurationFactory::setBasePriority(int basePriority) -{ - m_basePriority = basePriority; -} - bool BuildConfigurationFactory::canHandle(const Target *target) const { if (m_supportedProjectType.isValid() && m_supportedProjectType != target->project()->id()) @@ -456,32 +430,22 @@ BuildConfiguration *BuildConfigurationFactory::create(Target *parent, const Buil BuildConfiguration *BuildConfigurationFactory::restore(Target *parent, const QVariantMap &map) { - BuildConfigurationFactory *factory = nullptr; - int priority = -1; - for (BuildConfigurationFactory *i : g_buildConfigurationFactories) { - if (!i->canHandle(parent)) + const Core::Id id = idFromMap(map); + for (BuildConfigurationFactory *factory : g_buildConfigurationFactories) { + QTC_ASSERT(factory->m_creator, return nullptr); + if (!factory->canHandle(parent)) continue; - const Core::Id id = idFromMap(map); - if (!id.name().startsWith(i->m_buildConfigId.name())) + if (!id.name().startsWith(factory->m_buildConfigId.name())) continue; - int iPriority = i->priority(parent); - if (iPriority > priority) { - factory = i; - priority = iPriority; + BuildConfiguration *bc = factory->m_creator(parent); + QTC_ASSERT(bc, return nullptr); + if (!bc->fromMap(map)) { + delete bc; + bc = nullptr; } + return bc; } - - if (!factory) - return nullptr; - - QTC_ASSERT(factory->m_creator, return nullptr); - BuildConfiguration *bc = factory->m_creator(parent); - QTC_ASSERT(bc, return nullptr); - if (!bc->fromMap(map)) { - delete bc; - bc = nullptr; - } - return bc; + return nullptr; } BuildConfiguration *BuildConfigurationFactory::clone(Target *parent, diff --git a/src/plugins/projectexplorer/buildconfiguration.h b/src/plugins/projectexplorer/buildconfiguration.h index 09abdaa5105..c40a9d82a62 100644 --- a/src/plugins/projectexplorer/buildconfiguration.h +++ b/src/plugins/projectexplorer/buildconfiguration.h @@ -128,15 +128,10 @@ protected: ~BuildConfigurationFactory() override; public: - // The priority is negative if this factory cannot create anything for the target. - // It is 0 for the "default" factory that wants to handle the target. - // Add 100 for each specialization. - virtual int priority(const Target *parent) const; // List of build information that can be used to create a new build configuration via // "Add Build Configuration" button. const QList allAvailableBuilds(const Target *parent) const; - virtual int priority(const Kit *k, const QString &projectPath) const; // List of build information that can be used to initially set up a new build configuration. const QList allAvailableSetups(const Kit *k, const QString &projectPath) const; @@ -162,7 +157,6 @@ protected: void setSupportedProjectMimeTypeName(const QString &mimeTypeName); void addSupportedTargetDeviceType(Core::Id id); void setDefaultDisplayName(const QString &defaultDisplayName); - void setBasePriority(int basePriority); using BuildConfigurationCreator = std::function; @@ -183,8 +177,6 @@ private: QList m_supportedTargetDeviceTypes; QString m_supportedProjectMimeTypeName; IssueReporter m_issueReporter; - - int m_basePriority = 0; // Use higher numbers (1, 2, ...) for higher priorities. }; } // namespace ProjectExplorer diff --git a/src/plugins/qmlprofiler/tests/qmlprofilerdetailsrewriter_test.cpp b/src/plugins/qmlprofiler/tests/qmlprofilerdetailsrewriter_test.cpp index 927c397aa86..9a2dede6571 100644 --- a/src/plugins/qmlprofiler/tests/qmlprofilerdetailsrewriter_test.cpp +++ b/src/plugins/qmlprofiler/tests/qmlprofilerdetailsrewriter_test.cpp @@ -85,11 +85,6 @@ public: { return {}; } - - int priority(const ProjectExplorer::Kit *, const QString &) const final - { - return 0; - } }; QmlProfilerDetailsRewriterTest::QmlProfilerDetailsRewriterTest(QObject *parent) : QObject(parent)