From 6418eb2ddaa60da246a03951798fbb2dfdb864f6 Mon Sep 17 00:00:00 2001 From: hjk Date: Tue, 3 Aug 2021 15:12:24 +0200 Subject: [PATCH] Utils: Use EnvironmentChange instead of Environment in several places For path choosers this makes it easier to change the actual base retrospectively based on the device for the file path entered by the user. In other cases "end user code" only knows that something needs to be added to PATH to get a proper environment. This here lets this code to specify the change alone without bothering about the base environment this might be applied to. Change-Id: I726aaa2fd2feb0bee7158f601aac660b0ac6327b Reviewed-by: Eike Ziller --- src/libs/utils/aspects.cpp | 10 ++++----- src/libs/utils/aspects.h | 2 +- src/libs/utils/environment.cpp | 19 +++++++++++++---- src/libs/utils/environment.h | 6 ++++-- src/libs/utils/filepath.h | 1 + src/libs/utils/pathchooser.cpp | 21 +++++++++++-------- src/libs/utils/pathchooser.h | 3 ++- src/plugins/coreplugin/systemsettings.cpp | 7 +++---- .../projectexplorer/buildconfiguration.cpp | 4 ++-- .../customexecutablerunconfiguration.cpp | 7 ++++--- .../runconfigurationaspects.cpp | 12 +++++------ .../projectexplorer/runconfigurationaspects.h | 2 +- src/plugins/vcsbase/commonvcssettings.cpp | 7 +++---- 13 files changed, 59 insertions(+), 42 deletions(-) diff --git a/src/libs/utils/aspects.cpp b/src/libs/utils/aspects.cpp index 1be9b1b92d5..ddf41bc197b 100644 --- a/src/libs/utils/aspects.cpp +++ b/src/libs/utils/aspects.cpp @@ -642,7 +642,7 @@ public: QString m_placeHolderText; QString m_historyCompleterKey; PathChooser::Kind m_expectedKind = PathChooser::File; - Environment m_environment; + EnvironmentChange m_environmentChange; QPointer m_labelDisplay; QPointer m_lineEditDisplay; QPointer m_pathChooserDisplay; @@ -962,11 +962,11 @@ void StringAspect::setExpectedKind(const PathChooser::Kind expectedKind) d->m_pathChooserDisplay->setExpectedKind(expectedKind); } -void StringAspect::setEnvironment(const Environment &env) +void StringAspect::setEnvironmentChange(const EnvironmentChange &change) { - d->m_environment = env; + d->m_environmentChange = change; if (d->m_pathChooserDisplay) - d->m_pathChooserDisplay->setEnvironment(env); + d->m_pathChooserDisplay->setEnvironmentChange(change); } void StringAspect::setBaseFileName(const FilePath &baseFileName) @@ -1064,7 +1064,7 @@ void StringAspect::addToLayout(LayoutBuilder &builder) d->m_pathChooserDisplay->setHistoryCompleter(d->m_historyCompleterKey); if (d->m_validator) d->m_pathChooserDisplay->setValidationFunction(d->m_validator); - d->m_pathChooserDisplay->setEnvironment(d->m_environment); + d->m_pathChooserDisplay->setEnvironmentChange(d->m_environmentChange); d->m_pathChooserDisplay->setBaseDirectory(d->m_baseFileName); d->m_pathChooserDisplay->setOpenTerminalHandler(d->m_openTerminal); d->m_pathChooserDisplay->setFilePath(FilePath::fromString(displayedString)); diff --git a/src/libs/utils/aspects.h b/src/libs/utils/aspects.h index ee4219292a4..9124bb2ddf6 100644 --- a/src/libs/utils/aspects.h +++ b/src/libs/utils/aspects.h @@ -298,7 +298,7 @@ public: void setPlaceHolderText(const QString &placeHolderText); void setHistoryCompleter(const QString &historyCompleterKey); void setExpectedKind(const Utils::PathChooser::Kind expectedKind); - void setEnvironment(const Utils::Environment &env); + void setEnvironmentChange(const Utils::EnvironmentChange &change); void setBaseFileName(const Utils::FilePath &baseFileName); void setUndoRedoEnabled(bool readOnly); void setAcceptRichText(bool acceptRichText); diff --git a/src/libs/utils/environment.cpp b/src/libs/utils/environment.cpp index 908132fae84..db178852676 100644 --- a/src/libs/utils/environment.cpp +++ b/src/libs/utils/environment.cpp @@ -413,14 +413,18 @@ void EnvironmentChange::addUnsetValue(const QString &key) m_changeItems.append([key](Environment &env) { env.unset(key); }); } -void EnvironmentChange::addPrependToPath(const QString &value) +void EnvironmentChange::addPrependToPath(const QStringList &values) { - m_changeItems.append([value](Environment &env) { env.prependOrSetPath(value); }); + for (int i = values.size(); --i >= 0; ) { + const QString value = values.at(i); + m_changeItems.append([value](Environment &env) { env.prependOrSetPath(value); }); + } } -void EnvironmentChange::addAppendToPath(const QString &value) +void EnvironmentChange::addAppendToPath(const QStringList &values) { - m_changeItems.append([value](Environment &env) { env.appendOrSetPath(value); }); + for (const QString &value : values) + m_changeItems.append([value](Environment &env) { env.appendOrSetPath(value); }); } void EnvironmentChange::addModify(const NameValueItems &items) @@ -428,6 +432,13 @@ void EnvironmentChange::addModify(const NameValueItems &items) m_changeItems.append([items](Environment &env) { env.modify(items); }); } +EnvironmentChange EnvironmentChange::fromFixedEnvironment(const Environment &fixedEnv) +{ + EnvironmentChange change; + change.m_changeItems.append([fixedEnv](Environment &env) { env = fixedEnv; }); + return change; +} + void EnvironmentChange::applyToEnvironment(Environment &env) const { for (const Item &item : m_changeItems) diff --git a/src/libs/utils/environment.h b/src/libs/utils/environment.h index 0ed2d9b613c..c20ed67f7df 100644 --- a/src/libs/utils/environment.h +++ b/src/libs/utils/environment.h @@ -93,12 +93,14 @@ public: EnvironmentChange() = default; + static EnvironmentChange fromFixedEnvironment(const Environment &fixedEnv); + void applyToEnvironment(Environment &) const; void addSetValue(const QString &key, const QString &value); void addUnsetValue(const QString &key); - void addPrependToPath(const QString &value); - void addAppendToPath(const QString &value); + void addPrependToPath(const QStringList &values); + void addAppendToPath(const QStringList &values); void addModify(const NameValueItems &items); void addChange(const Item &item) { m_changeItems.append(item); } diff --git a/src/libs/utils/filepath.h b/src/libs/utils/filepath.h index 1e7fd4a88d1..b68609e31ce 100644 --- a/src/libs/utils/filepath.h +++ b/src/libs/utils/filepath.h @@ -47,6 +47,7 @@ class tst_fileutils; // This becomes a friend of Utils::FilePath for testing pri namespace Utils { class Environment; +class EnvironmentChange; class QTCREATOR_UTILS_EXPORT FilePath { diff --git a/src/libs/utils/pathchooser.cpp b/src/libs/utils/pathchooser.cpp index d4109175cbb..0d095c77930 100644 --- a/src/libs/utils/pathchooser.cpp +++ b/src/libs/utils/pathchooser.cpp @@ -191,7 +191,7 @@ public: FilePath m_initialBrowsePathOverride; QString m_defaultValue; FilePath m_baseDirectory; - Environment m_environment; + EnvironmentChange m_environmentChange; BinaryVersionToolTipEventFilter *m_binaryVersionToolTipEventFilter = nullptr; QList m_buttons; MacroExpander *m_macroExpander = globalMacroExpander(); @@ -209,18 +209,22 @@ FilePath PathChooserPrivate::expandedPath(const QString &input) const if (input.isEmpty()) return {}; - QString expandedInput = m_environment.expandVariables(input); - if (m_macroExpander) - expandedInput = m_macroExpander->expand(expandedInput); + FilePath path = FilePath::fromUserInput(input); + + Environment env = path.deviceEnvironment(); + m_environmentChange.applyToEnvironment(env); + path = env.expandVariables(path); + + if (m_macroExpander) + path = m_macroExpander->expand(path); - const FilePath path = FilePath::fromUserInput(expandedInput); if (path.isEmpty()) return path; switch (m_acceptingKind) { case PathChooser::Command: case PathChooser::ExistingCommand: { - FilePaths searchPaths = m_environment.path(); + FilePaths searchPaths = env.path(); searchPaths.append(m_baseDirectory); const FilePath expanded = path.searchOnDevice(searchPaths); return expanded.isEmpty() ? path : expanded; @@ -277,7 +281,6 @@ PathChooser::PathChooser(QWidget *parent) : setLayout(d->m_hLayout); setFocusProxy(d->m_lineEdit); setFocusPolicy(d->m_lineEdit->focusPolicy()); - setEnvironment(Environment::systemEnvironment()); d->m_lineEdit->setValidationFunction(defaultValidationFunction()); } @@ -328,10 +331,10 @@ FilePath PathChooser::baseDirectory() const return d->m_baseDirectory; } -void PathChooser::setEnvironment(const Environment &env) +void PathChooser::setEnvironmentChange(const EnvironmentChange &env) { QString oldExpand = filePath().toString(); - d->m_environment = env; + d->m_environmentChange = env; if (filePath().toString() != oldExpand) { triggerChanged(); emit rawPathChanged(rawPath()); diff --git a/src/libs/utils/pathchooser.h b/src/libs/utils/pathchooser.h index 06bfc07b85e..44346cf4d05 100644 --- a/src/libs/utils/pathchooser.h +++ b/src/libs/utils/pathchooser.h @@ -41,6 +41,7 @@ namespace Utils { class FancyLineEdit; class MacroExpander; class Environment; +class EnvironmentChange; class PathChooserPrivate; class QTCREATOR_UTILS_EXPORT PathChooser : public QWidget @@ -99,7 +100,7 @@ public: FilePath baseDirectory() const; void setBaseDirectory(const FilePath &base); - void setEnvironment(const Environment &env); + void setEnvironmentChange(const EnvironmentChange &change); /** Returns the suggested label title when used in a form layout. */ static QString label(); diff --git a/src/plugins/coreplugin/systemsettings.cpp b/src/plugins/coreplugin/systemsettings.cpp index 44e22c902b7..e6e25361f62 100644 --- a/src/plugins/coreplugin/systemsettings.cpp +++ b/src/plugins/coreplugin/systemsettings.cpp @@ -317,10 +317,9 @@ void SystemSettingsWidget::resetFileBrowser() void SystemSettingsWidget::updatePath() { - Environment env = Environment::systemEnvironment(); - QStringList toAdd = VcsManager::additionalToolsPath(); - env.appendOrSetPath(toAdd.join(HostOsInfo::pathListSeparator())); - m_ui.patchChooser->setEnvironment(env); + EnvironmentChange change; + change.addAppendToPath(VcsManager::additionalToolsPath()); + m_ui.patchChooser->setEnvironmentChange(change); } void SystemSettingsWidget::updateEnvironmentChangesLabel() diff --git a/src/plugins/projectexplorer/buildconfiguration.cpp b/src/plugins/projectexplorer/buildconfiguration.cpp index b7430714554..9610dacd333 100644 --- a/src/plugins/projectexplorer/buildconfiguration.cpp +++ b/src/plugins/projectexplorer/buildconfiguration.cpp @@ -213,12 +213,12 @@ BuildConfiguration::BuildConfiguration(Target *target, Utils::Id id) d->m_buildDirectoryAspect = addAspect(this); d->m_buildDirectoryAspect->setBaseFileName(target->project()->projectDirectory()); - d->m_buildDirectoryAspect->setEnvironment(environment()); + d->m_buildDirectoryAspect->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(environment())); d->m_buildDirectoryAspect->setMacroExpanderProvider([this] { return macroExpander(); }); connect(d->m_buildDirectoryAspect, &StringAspect::changed, this, &BuildConfiguration::emitBuildDirectoryChanged); connect(this, &BuildConfiguration::environmentChanged, this, [this] { - d->m_buildDirectoryAspect->setEnvironment(environment()); + d->m_buildDirectoryAspect->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(environment())); emit this->target()->buildEnvironmentChanged(this); }); diff --git a/src/plugins/projectexplorer/customexecutablerunconfiguration.cpp b/src/plugins/projectexplorer/customexecutablerunconfiguration.cpp index 720fab3a554..d39922c6d32 100644 --- a/src/plugins/projectexplorer/customexecutablerunconfiguration.cpp +++ b/src/plugins/projectexplorer/customexecutablerunconfiguration.cpp @@ -73,14 +73,15 @@ CustomExecutableRunConfiguration::CustomExecutableRunConfiguration(Target *targe exeAspect->setDisplayStyle(StringAspect::PathChooserDisplay); exeAspect->setHistoryCompleter("Qt.CustomExecutable.History"); exeAspect->setExpectedKind(PathChooser::ExistingCommand); - exeAspect->setEnvironment(envAspect->environment()); + exeAspect->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(envAspect->environment())); addAspect(); addAspect(); addAspect(); - connect(envAspect, &EnvironmentAspect::environmentChanged, - this, [exeAspect, envAspect] { exeAspect->setEnvironment(envAspect->environment()); }); + connect(envAspect, &EnvironmentAspect::environmentChanged, this, [exeAspect, envAspect] { + exeAspect->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(envAspect->environment())); + }); setDefaultDisplayName(defaultDisplayName()); } diff --git a/src/plugins/projectexplorer/runconfigurationaspects.cpp b/src/plugins/projectexplorer/runconfigurationaspects.cpp index ef0d16e2784..ef10cbc618e 100644 --- a/src/plugins/projectexplorer/runconfigurationaspects.cpp +++ b/src/plugins/projectexplorer/runconfigurationaspects.cpp @@ -195,9 +195,9 @@ void WorkingDirectoryAspect::addToLayout(LayoutBuilder &builder) if (m_envAspect) { connect(m_envAspect, &EnvironmentAspect::environmentChanged, m_chooser.data(), [this] { - m_chooser->setEnvironment(m_envAspect->environment()); + m_chooser->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(m_envAspect->environment())); }); - m_chooser->setEnvironment(m_envAspect->environment()); + m_chooser->setEnvironmentChange(EnvironmentChange::fromFixedEnvironment(m_envAspect->environment())); } builder.addItems({tr("Working directory:"), m_chooser.data(), m_resetButton.data()}); @@ -566,13 +566,13 @@ void ExecutableAspect::setExpectedKind(const PathChooser::Kind expectedKind) of paths is chosen as PathChooser::Command or PathChooser::ExistingCommand to \a env. - \sa Utils::StringAspect::setEnvironment() + \sa Utils::StringAspect::setEnvironmentChange() */ -void ExecutableAspect::setEnvironment(const Environment &env) +void ExecutableAspect::setEnvironmentChange(const EnvironmentChange &change) { - m_executable.setEnvironment(env); + m_executable.setEnvironmentChange(change); if (m_alternativeExecutable) - m_alternativeExecutable->setEnvironment(env); + m_alternativeExecutable->setEnvironmentChange(change); } /*! diff --git a/src/plugins/projectexplorer/runconfigurationaspects.h b/src/plugins/projectexplorer/runconfigurationaspects.h index ca37d063efc..00b44ceabf9 100644 --- a/src/plugins/projectexplorer/runconfigurationaspects.h +++ b/src/plugins/projectexplorer/runconfigurationaspects.h @@ -174,7 +174,7 @@ public: void setExecutablePathStyle(Utils::OsType osType); void setHistoryCompleter(const QString &historyCompleterKey); void setExpectedKind(const Utils::PathChooser::Kind expectedKind); - void setEnvironment(const Utils::Environment &env); + void setEnvironmentChange(const Utils::EnvironmentChange &change); void setDisplayStyle(Utils::StringAspect::DisplayStyle style); protected: diff --git a/src/plugins/vcsbase/commonvcssettings.cpp b/src/plugins/vcsbase/commonvcssettings.cpp index e927c225525..9d6bfe20322 100644 --- a/src/plugins/vcsbase/commonvcssettings.cpp +++ b/src/plugins/vcsbase/commonvcssettings.cpp @@ -155,10 +155,9 @@ CommonSettingsWidget::CommonSettingsWidget(CommonOptionsPage *page) void CommonSettingsWidget::updatePath() { - Environment env = Environment::systemEnvironment(); - QStringList toAdd = Core::VcsManager::additionalToolsPath(); - env.appendOrSetPath(toAdd.join(HostOsInfo::pathListSeparator())); - m_page->settings().sshPasswordPrompt.setEnvironment(env); + EnvironmentChange change; + change.addAppendToPath(Core::VcsManager::additionalToolsPath()); + m_page->settings().sshPasswordPrompt.setEnvironmentChange(change); } void CommonSettingsWidget::apply()