From 727be63dacaff6a8327f2c74852a7d907f4ca7e3 Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Thu, 15 Aug 2024 15:16:37 +0200 Subject: [PATCH] Utils: Allow range-for access to Environment dict Also refactor functions to use modern c++. Change-Id: I29aaf92800890048bc1af198cd1726cfe04d573e Reviewed-by: Cristian Adam --- src/libs/utils/environment.cpp | 13 ++- src/libs/utils/environmentmodel.cpp | 80 ++++++++----------- src/libs/utils/namevaluedictionary.cpp | 54 ++++++------- src/libs/utils/namevaluedictionary.h | 45 ++++++++--- src/libs/utils/namevalueitem.cpp | 18 ++--- .../cmakeprojectmanager/presetsmacros.cpp | 43 +++++----- 6 files changed, 127 insertions(+), 126 deletions(-) diff --git a/src/libs/utils/environment.cpp b/src/libs/utils/environment.cpp index 190cde2d2df..f9bd493edc5 100644 --- a/src/libs/utils/environment.cpp +++ b/src/libs/utils/environment.cpp @@ -60,10 +60,10 @@ EnvironmentItems Environment::diff(const Environment &other, bool checkAppendPre Environment::FindResult Environment::find(const QString &name) const { const NameValueDictionary &dict = resolved(); - const auto it = dict.constFind(name); - if (it == dict.constEnd()) + const auto it = dict.find(name); + if (it == dict.end()) return {}; - return Entry{it.key().name, it.value().first, it.value().second}; + return Entry{it.key(), it.value(), it.enabled()}; } void Environment::forEachEntry(const std::function &callBack) const @@ -124,11 +124,10 @@ QStringList Environment::toStringList() const QProcessEnvironment Environment::toProcessEnvironment() const { - const NameValueDictionary &dict = resolved(); QProcessEnvironment result; - for (auto it = dict.m_values.constBegin(); it != dict.m_values.constEnd(); ++it) { - if (it.value().second) - result.insert(it.key().name, expandedValueForKey(dict.key(it))); + for (const auto &[key, _, enabled] : resolved()) { + if (enabled) + result.insert(key, expandedValueForKey(key)); } return result; } diff --git a/src/libs/utils/environmentmodel.cpp b/src/libs/utils/environmentmodel.cpp index e81ee520d81..8e0458b429a 100644 --- a/src/libs/utils/environmentmodel.cpp +++ b/src/libs/utils/environmentmodel.cpp @@ -38,38 +38,29 @@ public: int findInChanges(const QString &name) const { - for (int i = 0; i < m_items.size(); ++i) - if (m_items.at(i).name.compare(name, - m_baseNameValueDictionary.nameCaseSensitivity()) == 0) { + for (int i = 0; i < m_items.size(); ++i) { + if (m_items.at(i).name.compare(name, m_baseNameValueDictionary.nameCaseSensitivity()) + == 0) { return i; } + } return -1; } int findInResultInsertPosition(const QString &name) const { - NameValueDictionary::const_iterator it; - int i = 0; - for (it = m_resultNameValueDictionary.constBegin(); - it != m_resultNameValueDictionary.constEnd(); - ++it, ++i) - if (it.key() > DictKey(name, m_resultNameValueDictionary.nameCaseSensitivity())) - return i; - return m_resultNameValueDictionary.size(); + const auto it = m_resultNameValueDictionary.find(name); + if (it == m_resultNameValueDictionary.end()) + return m_resultNameValueDictionary.size(); + return std::distance(m_resultNameValueDictionary.begin(), it); } int findInResult(const QString &name) const { - NameValueDictionary::const_iterator it; - int i = 0; - for (it = m_resultNameValueDictionary.constBegin(); - it != m_resultNameValueDictionary.constEnd(); - ++it, ++i) - if (m_resultNameValueDictionary.key(it) - .compare(name, m_resultNameValueDictionary.nameCaseSensitivity()) == 0) { - return i; - } - return -1; + const auto it = m_resultNameValueDictionary.find(name); + if (it == m_resultNameValueDictionary.end()) + return -1; + return std::distance(m_resultNameValueDictionary.begin(), it); } NameValueDictionary m_baseNameValueDictionary; @@ -88,8 +79,8 @@ EnvironmentModel::~EnvironmentModel() = default; QString EnvironmentModel::indexToVariable(const QModelIndex &index) const { - const auto it = std::next(d->m_resultNameValueDictionary.constBegin(), index.row()); - return d->m_resultNameValueDictionary.key(it); + const auto it = std::next(d->m_resultNameValueDictionary.begin(), index.row()); + return it.key(); } void EnvironmentModel::setBaseEnvironment(const Environment &env) @@ -133,14 +124,14 @@ QVariant EnvironmentModel::data(const QModelIndex &index, int role) const if (!index.isValid()) return QVariant(); - const auto resultIterator = std::next(d->m_resultNameValueDictionary.constBegin(), index.row()); + const auto it = std::next(d->m_resultNameValueDictionary.begin(), index.row()); switch (role) { case Qt::DisplayRole: case Qt::EditRole: case Qt::ToolTipRole: if (index.column() == 0) - return d->m_resultNameValueDictionary.key(resultIterator); + return it.key(); if (index.column() == 1) { // Do not return "" when editing a previously unset variable: if (role == Qt::EditRole) { @@ -148,7 +139,7 @@ QVariant EnvironmentModel::data(const QModelIndex &index, int role) const if (pos != -1 && d->m_items.at(pos).operation == EnvironmentItem::Unset) return QString(); } - QString value = d->m_resultNameValueDictionary.value(resultIterator); + QString value = it.value(); if (role == Qt::ToolTipRole && value.length() > 80) { if (currentEntryIsPathList(index)) { // For path lists, display one entry per line without separator @@ -166,13 +157,12 @@ QVariant EnvironmentModel::data(const QModelIndex &index, int role) const break; case Qt::FontRole: { QFont f; - f.setStrikeOut(!d->m_resultNameValueDictionary.isEnabled(resultIterator)); + f.setStrikeOut(!it.enabled()); return f; } case Qt::ForegroundRole: { const QPalette p = QGuiApplication::palette(); - return p.color(changes(d->m_resultNameValueDictionary.key(resultIterator)) - ? QPalette::Link : QPalette::Text); + return p.color(changes(it.key()) ? QPalette::Link : QPalette::Text); } } return QVariant(); @@ -237,13 +227,11 @@ bool EnvironmentModel::setData(const QModelIndex &index, const QVariant &value, // We are changing an existing value: const QString stringValue = value.toString(); if (changesPos != -1) { - const auto oldIt = d->m_baseNameValueDictionary.constFind(oldName); - const auto newIt = d->m_resultNameValueDictionary.constFind(oldName); + const auto oldIt = d->m_baseNameValueDictionary.find(oldName); + const auto newIt = d->m_resultNameValueDictionary.find(oldName); // We have already changed this value - if (oldIt != d->m_baseNameValueDictionary.constEnd() - && stringValue == d->m_baseNameValueDictionary.value(oldIt) - && d->m_baseNameValueDictionary.isEnabled(oldIt) - == d->m_resultNameValueDictionary.isEnabled(newIt)) { + if (oldIt != d->m_baseNameValueDictionary.end() && stringValue == oldIt.value() + && oldIt.enabled() == newIt.enabled()) { // ... and now went back to the base value d->m_items.removeAt(changesPos); } else { @@ -352,21 +340,19 @@ void EnvironmentModel::unsetVariable(const QString &name) void EnvironmentModel::toggleVariable(const QModelIndex &idx) { const QString name = indexToVariable(idx); - const auto newIt = d->m_resultNameValueDictionary.constFind(name); - QTC_ASSERT(newIt != d->m_resultNameValueDictionary.constEnd(), return); - const auto op = d->m_resultNameValueDictionary.isEnabled(newIt) - ? EnvironmentItem::SetDisabled : EnvironmentItem::SetEnabled; + const auto newIt = d->m_resultNameValueDictionary.find(name); + QTC_ASSERT(newIt != d->m_resultNameValueDictionary.begin(), return); + const auto op = newIt.enabled() ? EnvironmentItem::SetDisabled : EnvironmentItem::SetEnabled; const int changesPos = d->findInChanges(name); if (changesPos != -1) { - const auto oldIt = d->m_baseNameValueDictionary.constFind(name); - if (oldIt == d->m_baseNameValueDictionary.constEnd() - || oldIt.value().first != newIt.value().first) { + const auto oldIt = d->m_baseNameValueDictionary.find(name); + if (oldIt == d->m_baseNameValueDictionary.end() || oldIt.value() != newIt.value()) { d->m_items[changesPos].operation = op; } else { d->m_items.removeAt(changesPos); } } else { - d->m_items.append({name, d->m_resultNameValueDictionary.value(newIt), op}); + d->m_items.append({name, newIt.value(), op}); } d->updateResultNameValueDictionary(); emit dataChanged(index(idx.row(), 0), index(idx.row(), 1)); @@ -381,7 +367,7 @@ bool EnvironmentModel::isUnset(const QString &name) bool EnvironmentModel::isEnabled(const QString &name) const { - return d->m_resultNameValueDictionary.isEnabled(d->m_resultNameValueDictionary.constFind(name)); + return d->m_resultNameValueDictionary.find(name).enabled(); } bool EnvironmentModel::canReset(const QString &name) @@ -413,9 +399,9 @@ void EnvironmentModel::setUserChanges(const EnvironmentItems &items) if (d->m_baseNameValueDictionary.osType() == OsTypeWindows) { // NameValueDictionary variable names are case-insensitive under windows, but we still // want to preserve the case of pre-existing variables. - auto it = d->m_baseNameValueDictionary.constFind(name); - if (it != d->m_baseNameValueDictionary.constEnd()) - name = d->m_baseNameValueDictionary.key(it); + auto it = d->m_baseNameValueDictionary.find(name); + if (it != d->m_baseNameValueDictionary.end()) + name = it.key(); } } diff --git a/src/libs/utils/namevaluedictionary.cpp b/src/libs/utils/namevaluedictionary.cpp index d93298b3f15..d331bbcf6c7 100644 --- a/src/libs/utils/namevaluedictionary.cpp +++ b/src/libs/utils/namevaluedictionary.cpp @@ -31,20 +31,12 @@ NameValueDictionary::NameValueDictionary(const NameValuePairs &nameValues) NameValueMap::iterator NameValueDictionary::findKey(const QString &key) { - for (auto it = m_values.begin(); it != m_values.end(); ++it) { - if (key.compare(it.key().name, nameCaseSensitivity()) == 0) - return it; - } - return m_values.end(); + return m_values.find(DictKey(key, nameCaseSensitivity())); } NameValueMap::const_iterator NameValueDictionary::findKey(const QString &key) const { - for (auto it = m_values.constBegin(); it != m_values.constEnd(); ++it) { - if (key.compare(it.key().name, nameCaseSensitivity()) == 0) - return it; - } - return m_values.constEnd(); + return m_values.find(DictKey(key, nameCaseSensitivity())); } QStringList NameValueDictionary::toStringList() const @@ -87,11 +79,6 @@ QString NameValueDictionary::value(const QString &key) const return it != m_values.end() && it.value().second ? it.value().first : QString(); } -NameValueDictionary::const_iterator NameValueDictionary::constFind(const QString &name) const -{ - return findKey(name); -} - int NameValueDictionary::size() const { return m_values.size(); @@ -107,24 +94,26 @@ void NameValueDictionary::modify(const EnvironmentItems &items) EnvironmentItems NameValueDictionary::diff(const NameValueDictionary &other, bool checkAppendPrepend) const { - NameValueMap::const_iterator thisIt = constBegin(); - NameValueMap::const_iterator otherIt = other.constBegin(); + NameValueMap::const_iterator thisIt = m_values.begin(); + NameValueMap::const_iterator otherIt = other.m_values.begin(); EnvironmentItems result; - while (thisIt != constEnd() || otherIt != other.constEnd()) { - if (thisIt == constEnd()) { - result.append({other.key(otherIt), other.value(otherIt), - otherIt.value().second ? EnvironmentItem::SetEnabled : EnvironmentItem::SetDisabled}); + while (thisIt != m_values.end() || otherIt != other.m_values.end()) { + if (thisIt == m_values.end()) { + const auto enabled = otherIt.value().second ? EnvironmentItem::SetEnabled + : EnvironmentItem::SetDisabled; + result.append({otherIt.key().name, otherIt.value().first, enabled}); ++otherIt; - } else if (otherIt == other.constEnd()) { - result.append(EnvironmentItem(key(thisIt), QString(), EnvironmentItem::Unset)); + } else if (otherIt == other.m_values.end()) { + result.append(EnvironmentItem(thisIt.key().name, QString(), EnvironmentItem::Unset)); ++thisIt; } else if (thisIt.key() < otherIt.key()) { - result.append(EnvironmentItem(key(thisIt), QString(), EnvironmentItem::Unset)); + result.append(EnvironmentItem(thisIt.key().name, QString(), EnvironmentItem::Unset)); ++thisIt; } else if (thisIt.key() > otherIt.key()) { - result.append({other.key(otherIt), otherIt.value().first, - otherIt.value().second ? EnvironmentItem::SetEnabled : EnvironmentItem::SetDisabled}); + const auto enabled = otherIt.value().second ? EnvironmentItem::SetEnabled + : EnvironmentItem::SetDisabled; + result.append({otherIt.key().name, otherIt.value().first, enabled}); ++otherIt; } else { const QString &oldValue = thisIt.value().first; @@ -137,16 +126,19 @@ EnvironmentItems NameValueDictionary::diff(const NameValueDictionary &other, boo QString appended = newValue.right(newValue.size() - oldValue.size()); if (appended.startsWith(OsSpecificAspects::pathListSeparator(osType()))) appended.remove(0, 1); - result.append(EnvironmentItem(other.key(otherIt), appended, EnvironmentItem::Append)); + result.append( + EnvironmentItem(otherIt.key().name, appended, EnvironmentItem::Append)); } else if (checkAppendPrepend && newValue.endsWith(oldValue) && oldEnabled == newEnabled) { QString prepended = newValue.left(newValue.size() - oldValue.size()); if (prepended.endsWith(OsSpecificAspects::pathListSeparator(osType()))) prepended.chop(1); - result.append(EnvironmentItem(other.key(otherIt), prepended, EnvironmentItem::Prepend)); + result.append( + EnvironmentItem(otherIt.key().name, prepended, EnvironmentItem::Prepend)); } else { - result.append({other.key(otherIt), newValue, newEnabled - ? EnvironmentItem::SetEnabled : EnvironmentItem::SetDisabled}); + const auto enabled = newEnabled ? EnvironmentItem::SetEnabled + : EnvironmentItem::SetDisabled; + result.append({otherIt.key().name, newValue, enabled}); } } ++otherIt; @@ -158,7 +150,7 @@ EnvironmentItems NameValueDictionary::diff(const NameValueDictionary &other, boo bool NameValueDictionary::hasKey(const QString &key) const { - return findKey(key) != constEnd(); + return m_values.find(DictKey(key, nameCaseSensitivity())) != m_values.end(); } OsType NameValueDictionary::osType() const diff --git a/src/libs/utils/namevaluedictionary.h b/src/libs/utils/namevaluedictionary.h index 87d81249704..57b85d6b196 100644 --- a/src/libs/utils/namevaluedictionary.h +++ b/src/libs/utils/namevaluedictionary.h @@ -36,7 +36,38 @@ using NameValueMap = QMap>; class QTCREATOR_UTILS_EXPORT NameValueDictionary { public: - using const_iterator = NameValueMap::const_iterator; + class const_iterator + { + NameValueMap::const_iterator it; + + public: + const_iterator(NameValueMap::const_iterator it) + : it(it) + {} + // clang-format off + const_iterator &operator++() { ++it; return *this; } + const_iterator &operator++(int) { it++; return *this; } + const_iterator &operator--() { --it; return *this; } + const_iterator &operator--(int) { it--; return *this; } + // clang-format on + + bool operator==(const const_iterator &other) const { return it == other.it; } + bool operator!=(const const_iterator &other) const { return it != other.it; } + std::tuple operator*() const + { + return std::make_tuple(it.key().name, it.value().first, it.value().second); + } + + QString key() const { return it.key().name; } + QString value() const { return it.value().first; } + bool enabled() const { return it.value().second; } + + using difference_type = NameValueMap::const_iterator::difference_type; + using value_type = std::tuple; + using pointer = const value_type *; + using reference = const value_type &; + using iterator_category = NameValueMap::const_iterator::iterator_category; + }; explicit NameValueDictionary(OsType osType = HostOsInfo::hostOs()) : m_osType(osType) @@ -60,13 +91,9 @@ public: void clear(); int size() const; - QString key(const_iterator it) const { return it.key().name; } - QString value(const_iterator it) const { return it.value().first; } - bool isEnabled(const_iterator it) const { return it.value().second; } - - const_iterator constBegin() const { return m_values.constBegin(); } - const_iterator constEnd() const { return m_values.constEnd(); } - const_iterator constFind(const QString &name) const; + const const_iterator begin() const { return const_iterator(m_values.begin()); } + const const_iterator end() const { return const_iterator(m_values.end()); } + const const_iterator find(const QString &key) const { return const_iterator(findKey(key)); } friend bool operator!=(const NameValueDictionary &first, const NameValueDictionary &second) { @@ -81,7 +108,7 @@ public: protected: friend class Environment; NameValueMap::iterator findKey(const QString &key); - const_iterator findKey(const QString &key) const; + NameValueMap::const_iterator findKey(const QString &key) const; NameValueMap m_values; OsType m_osType; diff --git a/src/libs/utils/namevalueitem.cpp b/src/libs/utils/namevalueitem.cpp index cb34f8a5505..30e0eb8e925 100644 --- a/src/libs/utils/namevalueitem.cpp +++ b/src/libs/utils/namevalueitem.cpp @@ -112,9 +112,9 @@ static QString expand(const NameValueDictionary *dictionary, QString value) end = value.indexOf('}', i); if (end != -1) { const QString &key = value.mid(i + 2, end - i - 2); - NameValueDictionary::const_iterator it = dictionary->constFind(key); - if (it != dictionary->constEnd()) - value.replace(i, end - i + 1, it.value().first); + const NameValueDictionary::const_iterator it = dictionary->find(key); + if (it != dictionary->end()) + value.replace(i, end - i + 1, it.value()); ++replaceCount; QTC_ASSERT(replaceCount < 100, break); } @@ -137,9 +137,9 @@ void EnvironmentItem::apply(NameValueDictionary *dictionary, Operation op) const dictionary->unset(name); break; case Prepend: { - const NameValueDictionary::const_iterator it = dictionary->constFind(name); - if (it != dictionary->constEnd()) { - QString v = dictionary->value(it); + const NameValueDictionary::const_iterator it = dictionary->find(name); + if (it != dictionary->end()) { + QString v = it.value(); const QChar pathSep = HostOsInfo::pathListSeparator(); int sepCount = 0; if (v.startsWith(pathSep)) @@ -157,9 +157,9 @@ void EnvironmentItem::apply(NameValueDictionary *dictionary, Operation op) const } } break; case Append: { - const NameValueDictionary::const_iterator it = dictionary->constFind(name); - if (it != dictionary->constEnd()) { - QString v = dictionary->value(it); + const NameValueDictionary::const_iterator it = dictionary->find(name); + if (it != dictionary->end()) { + QString v = it.value(); const QChar pathSep = HostOsInfo::pathListSeparator(); int sepCount = 0; if (v.endsWith(pathSep)) diff --git a/src/plugins/cmakeprojectmanager/presetsmacros.cpp b/src/plugins/cmakeprojectmanager/presetsmacros.cpp index add697c3452..21342b4ab53 100644 --- a/src/plugins/cmakeprojectmanager/presetsmacros.cpp +++ b/src/plugins/cmakeprojectmanager/presetsmacros.cpp @@ -136,7 +136,7 @@ void expand(const PresetType &preset, Environment &env, const FilePath &sourceDi const Environment combinedEnv = getEnvCombined(preset.environment, env); const Environment parentEnv = env; - preset.environment->forEachEntry([&](const QString &key, QString value, bool enabled) { + for (auto [key, value, enabled] : preset.environment->resolved()) { if (!enabled) return; expandAllButEnv(preset, sourceDirectory, value); @@ -152,7 +152,7 @@ void expand(const PresetType &preset, Environment &env, const FilePath &sourceDi expandAllButEnv(preset, sourceDirectory, value); env.set(key, value); - }); + } } template @@ -161,28 +161,25 @@ void expand(const PresetType &preset, EnvironmentItems &envItems, const FilePath if (!preset.environment) return; - preset.environment->forEachEntry( - [&preset, - &sourceDirectory, - &envItems](const QString &key, QString value, bool enabled) { - if (!enabled) - return; - expandAllButEnv(preset, sourceDirectory, value); - value = expandMacroEnv("env", value, [&preset](const QString ¯oName) { - if (preset.environment->hasKey(macroName)) - return preset.environment->value(macroName); - return QString("${%1}").arg(macroName); - }); - - value = expandMacroEnv("penv", value, [](const QString ¯oName) { - return QString("${%1}").arg(macroName); - }); - - // Make sure to expand the CMake macros also for environment variables - expandAllButEnv(preset, sourceDirectory, value); - - envItems.emplace_back(Utils::EnvironmentItem(key, value)); + for (auto [key, value, enabled] : preset.environment->resolved()) { + if (!enabled) + return; + expandAllButEnv(preset, sourceDirectory, value); + value = expandMacroEnv("env", value, [&preset](const QString ¯oName) { + if (preset.environment->hasKey(macroName)) + return preset.environment->value(macroName); + return QString("${%1}").arg(macroName); }); + + value = expandMacroEnv("penv", value, [](const QString ¯oName) { + return QString("${%1}").arg(macroName); + }); + + // Make sure to expand the CMake macros also for environment variables + expandAllButEnv(preset, sourceDirectory, value); + + envItems.emplace_back(Utils::EnvironmentItem(key, value)); + } } template