From 341132dd0a01f8226bd5d1f6385755d83a8dfb86 Mon Sep 17 00:00:00 2001 From: hjk Date: Fri, 26 Mar 2021 16:54:40 +0100 Subject: [PATCH] CMake: Code cosmetics - use newer approach to settings page layout building - no SHOUTING enums - namespaces - proper dialog parent for message Change-Id: I7adfb7487d502b8fd706c4a0c5645f4d2153e39c Reviewed-by: Cristian Adam --- .../builddirparameters.cpp | 4 +- .../cmakebuildconfiguration.cpp | 8 +- .../cmakeprojectmanager/cmakebuildsystem.cpp | 57 +++++------ .../cmakekitinformation.cpp | 2 +- .../cmakespecificsettings.cpp | 96 +++++++------------ .../cmakespecificsettings.h | 35 ++----- 6 files changed, 73 insertions(+), 129 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/builddirparameters.cpp b/src/plugins/cmakeprojectmanager/builddirparameters.cpp index 005a87cabfc..dc71a5eb62a 100644 --- a/src/plugins/cmakeprojectmanager/builddirparameters.cpp +++ b/src/plugins/cmakeprojectmanager/builddirparameters.cpp @@ -85,8 +85,8 @@ BuildDirParameters::BuildDirParameters(CMakeBuildConfiguration *bc) environment.set("ICECC", "no"); CMakeSpecificSettings *settings = CMakeProjectPlugin::projectTypeSpecificSettings(); - if (!settings->ninjaPath().isEmpty()) { - const Utils::FilePath setting = settings->ninjaPath(); + if (!settings->ninjaPath.filePath().isEmpty()) { + const Utils::FilePath setting = settings->ninjaPath.filePath(); const Utils::FilePath path = setting.toFileInfo().isFile() ? setting.parentDir() : setting; environment.appendOrSetPath(path.toString()); } diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp index 86826432401..00011d1be9d 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp @@ -188,10 +188,10 @@ CMakeBuildSettingsWidget::CMakeBuildSettingsWidget(CMakeBuildConfiguration *bc) auto clearCMakeConfiguration = new QPushButton(tr("Re-configure with Initial Parameters")); connect(clearCMakeConfiguration, &QPushButton::clicked, this, [bc]() { auto *settings = CMakeProjectPlugin::projectTypeSpecificSettings(); - bool doNotAsk{!settings->askBeforeReConfigureInitialParams()}; + bool doNotAsk = !settings->askBeforeReConfigureInitialParams.value(); if (!doNotAsk) { QDialogButtonBox::StandardButton reply = Utils::CheckableMessageBox::question( - nullptr, + Core::ICore::dialogParent(), tr("Re-configure with Initial Parameters"), tr("Clear CMake configuration and configure with initial parameters?"), tr("Do not ask again"), @@ -199,7 +199,7 @@ CMakeBuildSettingsWidget::CMakeBuildSettingsWidget(CMakeBuildConfiguration *bc) QDialogButtonBox::Yes | QDialogButtonBox::No, QDialogButtonBox::Yes); - settings->setAskBeforeReConfigureInitialParams(!doNotAsk); + settings->askBeforeReConfigureInitialParams.setValue(!doNotAsk); settings->writeSettings(Core::ICore::settings()); if (reply != QDialogButtonBox::Yes) { @@ -803,7 +803,7 @@ static QStringList defaultInitialCMakeArguments(const Kit *k, const QString buil = Internal::CMakeProjectPlugin::projectTypeSpecificSettings(); // Package manager - if (settings->packageManagerAutoSetup()) + if (settings->packageManagerAutoSetup.value()) initialArgs.append(QString::fromLatin1("-DCMAKE_PROJECT_INCLUDE_BEFORE:PATH=%1") .arg("%{IDE:ResourcePath}/package-manager/auto-setup.cmake")); diff --git a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp index 4a249f188d8..b1486ef2f29 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp @@ -70,16 +70,15 @@ #include #include #include -#include #include using namespace ProjectExplorer; using namespace Utils; -namespace { +namespace CMakeProjectManager { +namespace Internal { -void copySourcePathToClipboard(Utils::optional srcPath, - const ProjectExplorer::ProjectNode *node) +static void copySourcePathToClipboard(Utils::optional srcPath, const ProjectNode *node) { QClipboard *clip = QGuiApplication::clipboard(); @@ -87,7 +86,7 @@ void copySourcePathToClipboard(Utils::optional srcPath, clip->setText(QDir::cleanPath(projDir.relativeFilePath(srcPath.value()))); } -void noAutoAdditionNotify(const QStringList &filePaths, const ProjectExplorer::ProjectNode *node) +static void noAutoAdditionNotify(const QStringList &filePaths, const ProjectNode *node) { Utils::optional srcPath{}; @@ -99,13 +98,12 @@ void noAutoAdditionNotify(const QStringList &filePaths, const ProjectExplorer::P } if (srcPath) { - CMakeProjectManager::Internal::CMakeSpecificSettings *settings - = CMakeProjectManager::Internal::CMakeProjectPlugin::projectTypeSpecificSettings(); - switch (settings->afterAddFileSetting()) { - case CMakeProjectManager::Internal::ASK_USER: { + CMakeSpecificSettings *settings = CMakeProjectPlugin::projectTypeSpecificSettings(); + switch (settings->afterAddFileSetting.value()) { + case AskUser: { bool checkValue{false}; - QDialogButtonBox::StandardButton reply = Utils::CheckableMessageBox::question( - nullptr, + QDialogButtonBox::StandardButton reply = CheckableMessageBox::question( + Core::ICore::dialogParent(), QMessageBox::tr("Copy to Clipboard?"), QMessageBox::tr("Files are not automatically added to the " "CMakeLists.txt file of the CMake project." @@ -116,37 +114,30 @@ void noAutoAdditionNotify(const QStringList &filePaths, const ProjectExplorer::P QDialogButtonBox::Yes); if (checkValue) { if (QDialogButtonBox::Yes == reply) - settings->setAfterAddFileSetting( - CMakeProjectManager::Internal::AfterAddFileAction::COPY_FILE_PATH); + settings->afterAddFileSetting.setValue(CopyFilePath); else if (QDialogButtonBox::No == reply) - settings->setAfterAddFileSetting( - CMakeProjectManager::Internal::AfterAddFileAction::NEVER_COPY_FILE_PATH); + settings->afterAddFileSetting.setValue(NeverCopyFilePath); settings->writeSettings(Core::ICore::settings()); } - if (QDialogButtonBox::Yes == reply) { + if (QDialogButtonBox::Yes == reply) copySourcePathToClipboard(srcPath, node); - } + break; } - case CMakeProjectManager::Internal::COPY_FILE_PATH: { + case CopyFilePath: { copySourcePathToClipboard(srcPath, node); break; } - case CMakeProjectManager::Internal::NEVER_COPY_FILE_PATH: + case NeverCopyFilePath: break; } } } -} // namespace - -namespace CMakeProjectManager { -namespace Internal { - static Q_LOGGING_CATEGORY(cmakeBuildSystemLog, "qtc.cmake.buildsystem", QtWarningMsg); // -------------------------------------------------------------------- @@ -180,7 +171,7 @@ CMakeBuildSystem::CMakeBuildSystem(CMakeBuildConfiguration *bc) return isIgnored; }); - m_treeScanner.setTypeFactory([](const Utils::MimeType &mimeType, const Utils::FilePath &fn) { + m_treeScanner.setTypeFactory([](const MimeType &mimeType, const FilePath &fn) { auto type = TreeScanner::genericFileType(mimeType, fn); if (type == FileType::Unknown) { if (mimeType.isValid()) { @@ -377,7 +368,7 @@ QString CMakeBuildSystem::reparseParametersString(int reparseFlags) void CMakeBuildSystem::writeConfigurationIntoBuildDirectory() { - const Utils::MacroExpander *expander = cmakeBuildConfiguration()->macroExpander(); + const MacroExpander *expander = cmakeBuildConfiguration()->macroExpander(); const FilePath buildDir = workDirectory(m_parameters); QTC_ASSERT(buildDir.exists(), return ); @@ -551,7 +542,7 @@ void CMakeBuildSystem::clearCMakeCache() for (const FilePath &path : pathsToDelete) { if (path.exists()) - Utils::FileUtils::removeRecursively(path); + FileUtils::removeRecursively(path); } } @@ -609,7 +600,7 @@ void CMakeBuildSystem::updateProjectData() QSet res; QStringList apps; for (const auto &target : qAsConst(m_buildTargets)) { - if (target.targetType == CMakeProjectManager::DynamicLibraryType) { + if (target.targetType == DynamicLibraryType) { res.insert(target.executable.parentDir().toString()); apps.push_back(target.executable.toUserOutput()); } @@ -889,7 +880,7 @@ void CMakeBuildSystem::wireUpConnections() FilePath CMakeBuildSystem::workDirectory(const BuildDirParameters ¶meters) { - const Utils::FilePath bdir = parameters.buildDirectory; + const FilePath bdir = parameters.buildDirectory; const CMakeTool *cmake = parameters.cmakeTool(); // use the build directory if it already exists anyhow @@ -910,7 +901,7 @@ FilePath CMakeBuildSystem::workDirectory(const BuildDirParameters ¶meters) auto tmpDirIt = m_buildDirToTempDir.find(bdir); if (tmpDirIt == m_buildDirToTempDir.end()) { auto ret = m_buildDirToTempDir.emplace( - std::make_pair(bdir, std::make_unique("qtc-cmake-XXXXXXXX"))); + std::make_pair(bdir, std::make_unique("qtc-cmake-XXXXXXXX"))); QTC_ASSERT(ret.second, return bdir); tmpDirIt = ret.first; @@ -920,7 +911,7 @@ FilePath CMakeBuildSystem::workDirectory(const BuildDirParameters ¶meters) return bdir; } } - return Utils::FilePath::fromString(tmpDirIt->second->path()); + return FilePath::fromString(tmpDirIt->second->path()); } void CMakeBuildSystem::stopParsingAndClearState() @@ -1158,11 +1149,11 @@ DeploymentData CMakeBuildSystem::deploymentData() const return result; } -QList CMakeBuildSystem::findExtraCompilers() +QList CMakeBuildSystem::findExtraCompilers() { qCDebug(cmakeBuildSystemLog) << "Finding Extra Compilers: start."; - QList extraCompilers; + QList extraCompilers; const QList factories = ExtraCompilerFactory::extraCompilerFactories(); qCDebug(cmakeBuildSystemLog) << "Finding Extra Compilers: Got factories."; diff --git a/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp b/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp index 1a338af1a39..f008d51b466 100644 --- a/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp +++ b/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp @@ -655,7 +655,7 @@ QVariant CMakeGeneratorKitAspect::defaultValue(const Kit *k) const Internal::CMakeSpecificSettings *settings = Internal::CMakeProjectPlugin::projectTypeSpecificSettings(); - if (settings->ninjaPath().isEmpty()) { + if (settings->ninjaPath.filePath().isEmpty()) { Utils::Environment env = Utils::Environment::systemEnvironment(); k->addToEnvironment(env); return !env.searchInPath("ninja").isEmpty(); diff --git a/src/plugins/cmakeprojectmanager/cmakespecificsettings.cpp b/src/plugins/cmakeprojectmanager/cmakespecificsettings.cpp index de11b911b40..8c00c4f5a35 100644 --- a/src/plugins/cmakeprojectmanager/cmakespecificsettings.cpp +++ b/src/plugins/cmakeprojectmanager/cmakespecificsettings.cpp @@ -25,8 +25,6 @@ #include "cmakespecificsettings.h" -#include - #include #include @@ -41,82 +39,56 @@ CMakeSpecificSettings::CMakeSpecificSettings() setSettingsGroup("CMakeSpecificSettings"); setAutoApply(false); - registerAspect(&m_afterAddFileToProjectSetting); - m_afterAddFileToProjectSetting.setSettingsKey("ProjectPopupSetting"); - m_afterAddFileToProjectSetting.setDefaultValue(AfterAddFileAction::ASK_USER); - m_afterAddFileToProjectSetting.addOption(tr("Ask about copying file paths")); - m_afterAddFileToProjectSetting.addOption(tr("Do not copy file paths")); - m_afterAddFileToProjectSetting.addOption(tr("Copy file paths")); - m_afterAddFileToProjectSetting.setToolTip(tr("Determines whether file paths are copied " + registerAspect(&afterAddFileSetting); + afterAddFileSetting.setSettingsKey("ProjectPopupSetting"); + afterAddFileSetting.setDefaultValue(AfterAddFileAction::AskUser); + afterAddFileSetting.addOption(tr("Ask about copying file paths")); + afterAddFileSetting.addOption(tr("Do not copy file paths")); + afterAddFileSetting.addOption(tr("Copy file paths")); + afterAddFileSetting.setToolTip(tr("Determines whether file paths are copied " "to the clipboard for pasting to the CMakeLists.txt file when you " "add new files to CMake projects.")); - registerAspect(&m_ninjaPath); - m_ninjaPath.setSettingsKey("NinjaPath"); + registerAspect(&ninjaPath); + ninjaPath.setSettingsKey("NinjaPath"); - registerAspect(&m_packageManagerAutoSetup); - m_packageManagerAutoSetup.setSettingsKey("PackageManagerAutoSetup"); - m_packageManagerAutoSetup.setDefaultValue(true); - m_packageManagerAutoSetup.setLabelText(tr("Package manager auto setup")); - m_packageManagerAutoSetup.setToolTip(tr("Add the CMAKE_PROJECT_INCLUDE_BEFORE variable " + registerAspect(&packageManagerAutoSetup); + packageManagerAutoSetup.setSettingsKey("PackageManagerAutoSetup"); + packageManagerAutoSetup.setDefaultValue(true); + packageManagerAutoSetup.setLabelText(tr("Package manager auto setup")); + packageManagerAutoSetup.setToolTip(tr("Add the CMAKE_PROJECT_INCLUDE_BEFORE variable " "pointing to a CMake script that will install dependencies from the conanfile.txt, " "conanfile.py, or vcpkg.json file from the project source directory.")); - registerAspect(&m_askBeforeReConfigureInitialParams); - m_askBeforeReConfigureInitialParams.setSettingsKey("AskReConfigureInitialParams"); - m_askBeforeReConfigureInitialParams.setDefaultValue(true); - m_askBeforeReConfigureInitialParams.setLabelText(tr("Ask before re-configuring with " + registerAspect(&askBeforeReConfigureInitialParams); + askBeforeReConfigureInitialParams.setSettingsKey("AskReConfigureInitialParams"); + askBeforeReConfigureInitialParams.setDefaultValue(true); + askBeforeReConfigureInitialParams.setLabelText(tr("Ask before re-configuring with " "initial parameters")); } -// CMakeSpecificSettingWidget - -class CMakeSpecificSettingWidget final : public Core::IOptionsPageWidget -{ - Q_DECLARE_TR_FUNCTIONS(CMakeProjectManager::Internal::CMakeSpecificSettingWidget) - -public: - explicit CMakeSpecificSettingWidget(CMakeSpecificSettings *settings); - - void apply() final; - -private: - CMakeSpecificSettings *m_settings; -}; - -CMakeSpecificSettingWidget::CMakeSpecificSettingWidget(CMakeSpecificSettings *settings) - : m_settings(settings) -{ - CMakeSpecificSettings &s = *m_settings; - using namespace Layouting; - - Column { - Group { - Title(tr("Adding Files")), - s.m_afterAddFileToProjectSetting - }, - s.m_packageManagerAutoSetup, - s.m_askBeforeReConfigureInitialParams, - Stretch(), - }.attachTo(this); -} - -void CMakeSpecificSettingWidget::apply() -{ - if (m_settings->isDirty()) { - m_settings->apply(); - m_settings->writeSettings(Core::ICore::settings()); - } -} - // CMakeSpecificSettingsPage CMakeSpecificSettingsPage::CMakeSpecificSettingsPage(CMakeSpecificSettings *settings) { setId("CMakeSpecificSettings"); - setDisplayName(CMakeSpecificSettingWidget::tr("CMake")); + setDisplayName(CMakeSpecificSettings::tr("CMake")); setCategory(ProjectExplorer::Constants::BUILD_AND_RUN_SETTINGS_CATEGORY); - setWidgetCreator([settings] { return new CMakeSpecificSettingWidget(settings); }); + setSettings(settings); + + setLayouter([settings](QWidget *widget) { + CMakeSpecificSettings &s = *settings; + using namespace Layouting; + Column { + Group { + Title(CMakeSpecificSettings::tr("Adding Files")), + s.afterAddFileSetting + }, + s.packageManagerAutoSetup, + s.askBeforeReConfigureInitialParams, + Stretch(), + }.attachTo(widget); + }); } } // Internal diff --git a/src/plugins/cmakeprojectmanager/cmakespecificsettings.h b/src/plugins/cmakeprojectmanager/cmakespecificsettings.h index 753d3c9950f..1c673fdc66f 100644 --- a/src/plugins/cmakeprojectmanager/cmakespecificsettings.h +++ b/src/plugins/cmakeprojectmanager/cmakespecificsettings.h @@ -33,43 +33,24 @@ namespace CMakeProjectManager { namespace Internal { enum AfterAddFileAction : int { - ASK_USER, - COPY_FILE_PATH, - NEVER_COPY_FILE_PATH + AskUser, + CopyFilePath, + NeverCopyFilePath }; -class CMakeSpecificSettings : public Utils::AspectContainer +class CMakeSpecificSettings final : public Utils::AspectContainer { Q_DECLARE_TR_FUNCTIONS(CMakeProjectManager::Internal::CMakeSpecificSettings) public: CMakeSpecificSettings(); - void setAfterAddFileSetting(AfterAddFileAction settings) { - m_afterAddFileToProjectSetting.setValue(settings); - } - AfterAddFileAction afterAddFileSetting() const { - return static_cast(m_afterAddFileToProjectSetting.value()); - } - - Utils::FilePath ninjaPath() const { return m_ninjaPath.filePath(); } - - void setPackageManagerAutoSetup(bool checked) { m_packageManagerAutoSetup.setValue(checked); } - bool packageManagerAutoSetup() const { return m_packageManagerAutoSetup.value(); } - - void setAskBeforeReConfigureInitialParams(bool doAsk) { m_askBeforeReConfigureInitialParams.setValue(doAsk); } - bool askBeforeReConfigureInitialParams() const { return m_askBeforeReConfigureInitialParams.value(); } - -private: - friend class CMakeSpecificSettingWidget; - - Utils::SelectionAspect m_afterAddFileToProjectSetting; - Utils::StringAspect m_ninjaPath; - Utils::BoolAspect m_packageManagerAutoSetup; - Utils::BoolAspect m_askBeforeReConfigureInitialParams; + Utils::SelectionAspect afterAddFileSetting; + Utils::StringAspect ninjaPath; + Utils::BoolAspect packageManagerAutoSetup; + Utils::BoolAspect askBeforeReConfigureInitialParams; }; - class CMakeSpecificSettingsPage final : public Core::IOptionsPage { public: