From d308b86847abad84855d8703c6870245ec725d60 Mon Sep 17 00:00:00 2001 From: hjk Date: Fri, 7 Jun 2024 14:59:00 +0200 Subject: [PATCH] Utils: Centralize aspect macro expansion setup and handling Let each aspect have a macro expander, and let aspect-owned lineedits use this for expansion. Change-Id: Ifa6f5a678cf81c169643e4145f41e69eafedeb93 Reviewed-by: Marcus Tillmanns --- src/libs/utils/aspects.cpp | 82 ++++++++++--------- src/libs/utils/aspects.h | 12 ++- src/libs/utils/macroexpander.h | 1 - .../cmakebuildconfiguration.cpp | 3 - .../cmakeprojectmanager/cmakeprocess.cpp | 1 + src/plugins/cppeditor/clangdsettings.cpp | 7 +- src/plugins/debugger/commonoptionspage.cpp | 1 - src/plugins/debugger/gdb/gdbsettings.cpp | 2 - .../projectexplorer/buildconfiguration.cpp | 1 - .../buildpropertiessettings.cpp | 1 - src/plugins/projectexplorer/buildstep.cpp | 16 +--- src/plugins/projectexplorer/buildstep.h | 3 - src/plugins/projectexplorer/copystep.cpp | 4 - src/plugins/projectexplorer/processstep.cpp | 2 - .../projectexplorer/projectconfiguration.cpp | 1 + .../projectexplorer/workspaceproject.cpp | 2 - .../remotelinux/customcommanddeploystep.cpp | 2 - 17 files changed, 60 insertions(+), 81 deletions(-) diff --git a/src/libs/utils/aspects.cpp b/src/libs/utils/aspects.cpp index 13db6a1ecef..e6b8267c4ed 100644 --- a/src/libs/utils/aspects.cpp +++ b/src/libs/utils/aspects.cpp @@ -10,6 +10,7 @@ #include "guard.h" #include "iconbutton.h" #include "layoutbuilder.h" +#include "macroexpander.h" #include "passworddialog.h" #include "pathchooser.h" #include "pathlisteditor.h" @@ -95,6 +96,7 @@ public: BaseAspect::DataCloner m_dataCloner; QList m_dataExtractors; + MacroExpander *m_expander = globalMacroExpander(); QUndoStack *m_undoStack = nullptr; }; @@ -724,6 +726,27 @@ QVariant BaseAspect::fromSettingsValue(const QVariant &val) const return d->m_fromSettings ? d->m_fromSettings(val) : val; } +void BaseAspect::setMacroExpander(MacroExpander *expander) +{ + d->m_expander = expander; +} + +MacroExpander *BaseAspect::macroExpander() const +{ + return d->m_expander; +} + +void BaseAspect::addMacroExpansion(QWidget *w) +{ + if (!d->m_expander) + return; + const auto chooser = new VariableChooser(w); + chooser->addSupportedWidget(w); + chooser->addMacroExpanderProvider([this] { return d->m_expander; }); + if (auto pathChooser = qobject_cast(w)) + pathChooser->setMacroExpander(d->m_expander); +} + namespace Internal { class BoolAspectPrivate @@ -887,7 +910,6 @@ public: Qt::TextElideMode m_elideMode = Qt::ElideNone; QString m_placeHolderText; Key m_historyCompleterKey; - MacroExpanderProvider m_expanderProvider; StringAspect::ValueAcceptor m_valueAcceptor; std::optional m_validator; @@ -1119,16 +1141,6 @@ void StringAspect::setAcceptRichText(bool acceptRichText) emit acceptRichTextChanged(acceptRichText); } -void StringAspect::setMacroExpanderProvider(const MacroExpanderProvider &expanderProvider) -{ - d->m_expanderProvider = expanderProvider; -} - -void StringAspect::setUseGlobalMacroExpander() -{ - d->m_expanderProvider = &globalMacroExpander; -} - void StringAspect::setUseResetButton() { d->m_useResetButton = true; @@ -1149,14 +1161,6 @@ void StringAspect::addToLayout(Layout &parent) { d->m_checkerImpl.addToLayoutFirst(parent); - const auto useMacroExpander = [this](QWidget *w) { - if (!d->m_expanderProvider) - return; - const auto chooser = new VariableChooser(w); - chooser->addSupportedWidget(w); - chooser->addMacroExpanderProvider(d->m_expanderProvider); - }; - const QString displayedString = d->m_displayFilter ? d->m_displayFilter(volatileValue()) : volatileValue(); @@ -1164,6 +1168,7 @@ void StringAspect::addToLayout(Layout &parent) case PasswordLineEditDisplay: case LineEditDisplay: { auto lineEditDisplay = createSubWidget(); + addMacroExpansion(lineEditDisplay); lineEditDisplay->setPlaceholderText(d->m_placeHolderText); if (!d->m_historyCompleterKey.isEmpty()) lineEditDisplay->setHistoryCompleter(d->m_historyCompleterKey); @@ -1197,7 +1202,6 @@ void StringAspect::addToLayout(Layout &parent) } addLabeledItem(parent, lineEditDisplay); - useMacroExpander(lineEditDisplay); if (d->m_useResetButton) { auto resetButton = createSubWidget(Tr::tr("Reset")); resetButton->setEnabled(lineEditDisplay->text() != defaultValue()); @@ -1255,6 +1259,7 @@ void StringAspect::addToLayout(Layout &parent) } case TextEditDisplay: { auto textEditDisplay = createSubWidget(); + addMacroExpansion(textEditDisplay); textEditDisplay->setPlaceholderText(d->m_placeHolderText); textEditDisplay->setUndoRedoEnabled(false); textEditDisplay->setAcceptRichText(d->m_acceptRichText); @@ -1273,7 +1278,6 @@ void StringAspect::addToLayout(Layout &parent) } addLabeledItem(parent, textEditDisplay); - useMacroExpander(textEditDisplay); bufferToGui(); connect(this, &StringAspect::acceptRichTextChanged, @@ -1323,8 +1327,10 @@ void StringAspect::addToLayout(Layout &parent) QString StringAspect::expandedValue() const { - if (!m_internal.isEmpty() && d->m_expanderProvider) - return d->m_expanderProvider()->expand(m_internal); + if (!m_internal.isEmpty()) { + if (auto expander = macroExpander()) + return expander->expand(m_internal); + } return m_internal; } @@ -1406,7 +1412,6 @@ public: PathChooser::Kind m_expectedKind = PathChooser::File; Environment m_environment; QPointer m_pathChooserDisplay; - MacroExpanderProvider m_expanderProvider; FilePath m_baseFileName; StringAspect::ValueAcceptor m_valueAcceptor; std::optional m_validator; @@ -1453,8 +1458,10 @@ FilePath FilePathAspect::operator()() const FilePath FilePathAspect::expandedValue() const { const auto value = TypedAspect::value(); - if (!value.isEmpty() && d->m_expanderProvider) - return FilePath::fromUserInput(d->m_expanderProvider()->expand(value)); + if (!value.isEmpty()) { + if (auto expander = macroExpander()) + return FilePath::fromUserInput(expander->expand(value)); + } return FilePath::fromUserInput(value); } @@ -1586,17 +1593,10 @@ void FilePathAspect::addToLayout(Layouting::Layout &parent) { d->m_checkerImpl.addToLayoutFirst(parent); - const auto useMacroExpander = [this](QWidget *w) { - if (!d->m_expanderProvider) - return; - const auto chooser = new VariableChooser(w); - chooser->addSupportedWidget(w); - chooser->addMacroExpanderProvider(d->m_expanderProvider); - }; - const QString displayedString = d->m_displayFilter ? d->m_displayFilter(value()) : value(); d->m_pathChooserDisplay = createSubWidget(); + addMacroExpansion(d->m_pathChooserDisplay->lineEdit()); d->m_pathChooserDisplay->setExpectedKind(d->m_expectedKind); if (!d->m_historyCompleterKey.isEmpty()) d->m_pathChooserDisplay->setHistoryCompleter(d->m_historyCompleterKey); @@ -1621,7 +1621,6 @@ void FilePathAspect::addToLayout(Layouting::Layout &parent) d->m_pathChooserDisplay->lineEdit()->setPlaceholderText(d->m_placeHolderText); d->m_checkerImpl.updateWidgetFromCheckStatus(this, d->m_pathChooserDisplay.data()); addLabeledItem(parent, d->m_pathChooserDisplay); - useMacroExpander(d->m_pathChooserDisplay->lineEdit()); connect(d->m_pathChooserDisplay, &PathChooser::validChanged, this, &FilePathAspect::validChanged); bufferToGui(); if (isAutoApply() && d->m_autoApplyOnEditingFinished) { @@ -1765,11 +1764,6 @@ void FilePathAspect::setHistoryCompleter(const Key &historyCompleterKey) d->m_pathChooserDisplay->setHistoryCompleter(historyCompleterKey); } -void FilePathAspect::setMacroExpanderProvider(const MacroExpanderProvider &expanderProvider) -{ - d->m_expanderProvider = expanderProvider; -} - void FilePathAspect::validateInput() { if (d->m_pathChooserDisplay) @@ -3237,6 +3231,14 @@ void AspectContainer::setUndoStack(QUndoStack *undoStack) aspect->setUndoStack(undoStack); } +void AspectContainer::setMacroExpander(MacroExpander *expander) +{ + BaseAspect::setMacroExpander(expander); + + for (BaseAspect *aspect : std::as_const(d->m_items)) + aspect->setMacroExpander(expander); +} + bool AspectContainer::equals(const AspectContainer &other) const { // FIXME: Expensive, but should not really be needed in a fully aspectified world. diff --git a/src/libs/utils/aspects.h b/src/libs/utils/aspects.h index 8faeaf7a9e3..60a198c1f7b 100644 --- a/src/libs/utils/aspects.h +++ b/src/libs/utils/aspects.h @@ -7,7 +7,6 @@ #include "guiutils.h" #include "id.h" #include "infolabel.h" -#include "macroexpander.h" #include "pathchooser.h" #include "qtcsettings.h" #include "store.h" @@ -36,6 +35,7 @@ namespace Utils { class AspectContainer; class BoolAspect; class CheckableDecider; +class MacroExpander; namespace Internal { class AspectContainerPrivate; @@ -202,6 +202,9 @@ public: // This is expensive. Do not use without good reason void writeToSettingsImmediatly() const; + void setMacroExpander(MacroExpander *expander); + MacroExpander *macroExpander() const; + signals: void changed(); void volatileValueChanged(); @@ -219,6 +222,8 @@ protected: virtual void handleGuiChanged(); + void addMacroExpansion(QWidget *w); + QLabel *createLabel(); void addLabeledItem(Layouting::Layout &parent, QWidget *widget); @@ -607,8 +612,6 @@ public: void setPlaceHolderText(const QString &placeHolderText); void setHistoryCompleter(const Key &historyCompleterKey); void setAcceptRichText(bool acceptRichText); - void setMacroExpanderProvider(const MacroExpanderProvider &expanderProvider); - void setUseGlobalMacroExpander(); void setUseResetButton(); void setValidationFunction(const FancyLineEdit::ValidationFunction &validator); void setAutoApplyOnEditingFinished(bool applyOnEditingFinished); @@ -685,7 +688,6 @@ public: void setValidationFunction(const FancyLineEdit::ValidationFunction &validator); void setDisplayFilter(const std::function &displayFilter); void setHistoryCompleter(const Key &historyCompleterKey); - void setMacroExpanderProvider(const MacroExpanderProvider &expanderProvider); void setShowToolTipOnLabel(bool show); void setAutoApplyOnEditingFinished(bool applyOnEditingFinished); @@ -975,6 +977,8 @@ public: bool isDirty() override; void setUndoStack(QUndoStack *undoStack) override; + void setMacroExpander(MacroExpander *expander); + template T *aspect() const { for (BaseAspect *aspect : aspects()) diff --git a/src/libs/utils/macroexpander.h b/src/libs/utils/macroexpander.h index adde4db1d3f..fe98d46872f 100644 --- a/src/libs/utils/macroexpander.h +++ b/src/libs/utils/macroexpander.h @@ -5,7 +5,6 @@ #include "utils_global.h" -#include #include #include diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp index 274ebd8f83f..696ce44c1e2 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp @@ -1463,12 +1463,9 @@ CMakeBuildConfiguration::CMakeBuildConfiguration(Target *target, Id id) buildTypeAspect.setDisplayStyle(StringAspect::LineEditDisplay); buildTypeAspect.setDefaultValue("Unknown"); - initialCMakeArguments.setMacroExpanderProvider([this] { return macroExpander(); }); - additionalCMakeOptions.setSettingsKey("CMake.Additional.Options"); additionalCMakeOptions.setLabelText(Tr::tr("Additional CMake options:")); additionalCMakeOptions.setDisplayStyle(StringAspect::LineEditDisplay); - additionalCMakeOptions.setMacroExpanderProvider([this] { return macroExpander(); }); macroExpander()->registerVariable(DEVELOPMENT_TEAM_FLAG, Tr::tr("The CMake flag for the development team"), diff --git a/src/plugins/cmakeprojectmanager/cmakeprocess.cpp b/src/plugins/cmakeprojectmanager/cmakeprocess.cpp index 618c42892f2..3df6a6bbcb6 100644 --- a/src/plugins/cmakeprojectmanager/cmakeprocess.cpp +++ b/src/plugins/cmakeprojectmanager/cmakeprocess.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include diff --git a/src/plugins/cppeditor/clangdsettings.cpp b/src/plugins/cppeditor/clangdsettings.cpp index daa78aad7eb..78cce6df1c0 100644 --- a/src/plugins/cppeditor/clangdsettings.cpp +++ b/src/plugins/cppeditor/clangdsettings.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -207,17 +208,17 @@ FilePath ClangdSettings::clangdFilePath() const return fallbackClangdFilePath(); } -FilePath ClangdSettings::projectIndexPath(const Utils::MacroExpander &expander) const +FilePath ClangdSettings::projectIndexPath(const MacroExpander &expander) const { return FilePath::fromUserInput(expander.expand(m_data.projectIndexPathTemplate)); } -FilePath ClangdSettings::sessionIndexPath(const Utils::MacroExpander &expander) const +FilePath ClangdSettings::sessionIndexPath(const MacroExpander &expander) const { return FilePath::fromUserInput(expander.expand(m_data.sessionIndexPathTemplate)); } -bool ClangdSettings::sizeIsOkay(const Utils::FilePath &fp) const +bool ClangdSettings::sizeIsOkay(const FilePath &fp) const { return !sizeThresholdEnabled() || sizeThresholdInKb() * 1024 >= fp.fileSize(); } diff --git a/src/plugins/debugger/commonoptionspage.cpp b/src/plugins/debugger/commonoptionspage.cpp index 5739845fe65..b7c56541b82 100644 --- a/src/plugins/debugger/commonoptionspage.cpp +++ b/src/plugins/debugger/commonoptionspage.cpp @@ -266,7 +266,6 @@ LocalsAndExpressionsSettings::LocalsAndExpressionsSettings() extraDumperCommands.setSettingsKey(debugModeGroup, "GdbCustomDumperCommands"); extraDumperCommands.setDisplayStyle(StringAspect::TextEditDisplay); - extraDumperCommands.setUseGlobalMacroExpander(); extraDumperCommands.setToolTip("

" + Tr::tr("Python commands entered here will be executed after built-in " "debugging helpers have been loaded and fully initialized. You can " diff --git a/src/plugins/debugger/gdb/gdbsettings.cpp b/src/plugins/debugger/gdb/gdbsettings.cpp index e88e9f4f8a9..9ca93263069 100644 --- a/src/plugins/debugger/gdb/gdbsettings.cpp +++ b/src/plugins/debugger/gdb/gdbsettings.cpp @@ -127,7 +127,6 @@ GdbSettings::GdbSettings() gdbStartupCommands.setSettingsKey(debugModeGroup, "GdbStartupCommands"); gdbStartupCommands.setDisplayStyle(StringAspect::TextEditDisplay); - gdbStartupCommands.setUseGlobalMacroExpander(); gdbStartupCommands.setToolTip("

" + Tr::tr( "GDB commands entered here will be executed after " "GDB has been started, but before the debugged program is started or " @@ -136,7 +135,6 @@ GdbSettings::GdbSettings() gdbPostAttachCommands.setSettingsKey(debugModeGroup, "GdbPostAttachCommands"); gdbPostAttachCommands.setDisplayStyle(StringAspect::TextEditDisplay); - gdbPostAttachCommands.setUseGlobalMacroExpander(); gdbPostAttachCommands.setToolTip("

" + Tr::tr( "GDB commands entered here will be executed after " "GDB has successfully attached to remote targets.

" diff --git a/src/plugins/projectexplorer/buildconfiguration.cpp b/src/plugins/projectexplorer/buildconfiguration.cpp index a69133b2190..6cc025d8770 100644 --- a/src/plugins/projectexplorer/buildconfiguration.cpp +++ b/src/plugins/projectexplorer/buildconfiguration.cpp @@ -194,7 +194,6 @@ BuildConfiguration::BuildConfiguration(Target *target, Utils::Id id) d->m_buildDirectoryAspect.setBaseFileName(target->project()->projectDirectory()); d->m_buildDirectoryAspect.setEnvironment(environment()); - d->m_buildDirectoryAspect.setMacroExpanderProvider([this] { return macroExpander(); }); connect(&d->m_buildDirectoryAspect, &StringAspect::changed, this, &BuildConfiguration::emitBuildDirectoryChanged); connect(this, &BuildConfiguration::environmentChanged, this, [this] { diff --git a/src/plugins/projectexplorer/buildpropertiessettings.cpp b/src/plugins/projectexplorer/buildpropertiessettings.cpp index ff7c1808518..ebfe8782151 100644 --- a/src/plugins/projectexplorer/buildpropertiessettings.cpp +++ b/src/plugins/projectexplorer/buildpropertiessettings.cpp @@ -59,7 +59,6 @@ BuildPropertiesSettings::BuildPropertiesSettings() "The default value can be set using the environment variable " "%1") .arg(Constants::QTC_DEFAULT_BUILD_DIRECTORY_TEMPLATE)); - buildDirectoryTemplate.setUseGlobalMacroExpander(); buildDirectoryTemplate.setUseResetButton(); separateDebugInfo.setSettingsKey("ProjectExplorer/Settings/SeparateDebugInfo"); diff --git a/src/plugins/projectexplorer/buildstep.cpp b/src/plugins/projectexplorer/buildstep.cpp index a290fc0c69a..19eb143d245 100644 --- a/src/plugins/projectexplorer/buildstep.cpp +++ b/src/plugins/projectexplorer/buildstep.cpp @@ -85,6 +85,9 @@ BuildStep::BuildStep(BuildStepList *bsl, Id id) : ProjectConfiguration(bsl->target(), id) , m_stepList(bsl) { + if (auto bc = buildConfiguration()) + setMacroExpander(bc->macroExpander()); + connect(this, &ProjectConfiguration::displayNameChanged, this, &BuildStep::updateSummary); } @@ -120,12 +123,8 @@ QWidget *BuildStep::createConfigWidget() form.flush(); } } - auto widget = form.emerge(); - if (m_addMacroExpander) - VariableChooser::addSupportForChildWidgets(widget, macroExpander()); - - return widget; + return form.emerge(); } void BuildStep::fromMap(const Store &map) @@ -196,13 +195,6 @@ BuildConfiguration::BuildType BuildStep::buildType() const return BuildConfiguration::Unknown; } -MacroExpander *BuildStep::macroExpander() const -{ - if (auto bc = buildConfiguration()) - return bc->macroExpander(); - return globalMacroExpander(); -} - QString BuildStep::fallbackWorkingDirectory() const { if (buildConfiguration()) diff --git a/src/plugins/projectexplorer/buildstep.h b/src/plugins/projectexplorer/buildstep.h index ed07d763e7a..4fd01f0eb33 100644 --- a/src/plugins/projectexplorer/buildstep.h +++ b/src/plugins/projectexplorer/buildstep.h @@ -52,7 +52,6 @@ public: BuildSystem *buildSystem() const; BuildConfiguration::BuildType buildType() const; - Utils::MacroExpander *macroExpander() const; enum class OutputFormat { Stdout, Stderr, // These are for forwarded output from external tools @@ -99,7 +98,6 @@ protected: void setWidgetExpandedByDefault(bool widgetExpandedByDefault); void setImmutable(bool immutable) { m_immutable = immutable; } void setSummaryUpdater(const std::function &summaryUpdater); - void addMacroExpander() { m_addMacroExpander = true; } void setSummaryText(const QString &summaryText); DeployConfiguration *deployConfiguration() const; @@ -119,7 +117,6 @@ private: bool m_enabled = true; bool m_immutable = false; bool m_widgetExpandedByDefault = true; - bool m_addMacroExpander = false; std::optional m_wasExpanded; std::function m_summaryUpdater; diff --git a/src/plugins/projectexplorer/copystep.cpp b/src/plugins/projectexplorer/copystep.cpp index 4f9d94e2eeb..812fcb31348 100644 --- a/src/plugins/projectexplorer/copystep.cpp +++ b/src/plugins/projectexplorer/copystep.cpp @@ -25,13 +25,9 @@ public: { m_sourceAspect.setSettingsKey(SOURCE_KEY); m_sourceAspect.setLabelText(Tr::tr("Source:")); - m_sourceAspect.setMacroExpanderProvider([this] { return macroExpander(); }); m_targetAspect.setSettingsKey(TARGET_KEY); m_targetAspect.setLabelText(Tr::tr("Target:")); - m_targetAspect.setMacroExpanderProvider([this] { return macroExpander(); }); - - addMacroExpander(); } protected: diff --git a/src/plugins/projectexplorer/processstep.cpp b/src/plugins/projectexplorer/processstep.cpp index 782eadf91df..18547fcb015 100644 --- a/src/plugins/projectexplorer/processstep.cpp +++ b/src/plugins/projectexplorer/processstep.cpp @@ -60,8 +60,6 @@ public: setupProcessParameters(¶m); return param.summary(display); }); - - addMacroExpander(); } private: diff --git a/src/plugins/projectexplorer/projectconfiguration.cpp b/src/plugins/projectexplorer/projectconfiguration.cpp index 33285507453..423ed95479e 100644 --- a/src/plugins/projectexplorer/projectconfiguration.cpp +++ b/src/plugins/projectexplorer/projectconfiguration.cpp @@ -6,6 +6,7 @@ #include "target.h" #include +#include #include using namespace ProjectExplorer; diff --git a/src/plugins/projectexplorer/workspaceproject.cpp b/src/plugins/projectexplorer/workspaceproject.cpp index 2a22c65c4be..5b0a92ba6c9 100644 --- a/src/plugins/projectexplorer/workspaceproject.cpp +++ b/src/plugins/projectexplorer/workspaceproject.cpp @@ -180,8 +180,6 @@ public: executable.setLabelText(Tr::tr("Executable:")); executable.setReadOnly(true); executable.setValue(bti.targetFilePath); - executable.setMacroExpanderProvider( - [this]() -> MacroExpander * { return const_cast(macroExpander()); }); auto argumentsAsString = [this]() { return CommandLine{ diff --git a/src/plugins/remotelinux/customcommanddeploystep.cpp b/src/plugins/remotelinux/customcommanddeploystep.cpp index 97826fde5e9..2850e59f0cf 100644 --- a/src/plugins/remotelinux/customcommanddeploystep.cpp +++ b/src/plugins/remotelinux/customcommanddeploystep.cpp @@ -31,8 +31,6 @@ public: commandLine.setHistoryCompleter("RemoteLinuxCustomCommandDeploymentStep.History"); setInternalInitializer([this] { return isDeploymentPossible(); }); - - addMacroExpander(); } expected_str isDeploymentPossible() const final;