Let users disable environment entries

It's helpful to be able to temporarily disable environment variables, as
opposed to having to remove (and then re-add) them entirely.

Fixes: QTCREATORBUG-20984
Change-Id: Ib0d287035b9357507c4c19faaf3a1517382506b5
Reviewed-by: hjk <hjk@qt.io>

9 files changed, 145 insertions, 84 deletions

diff --git a/src/libs/clangsupport/processcreator.cpp b/src/libs/clangsupport/processcreator.cpp
index 22b52793e3..e0147bda6b 100644
--- a/src/libs/clangsupport/processcreator.cpp
+++ b/src/libs/clangsupport/processcreator.cpp
@@ -173,8 +173,10 @@ QProcessEnvironment ProcessCreator::processEnvironment() const
     }
 
     const Utils::Environment &env = m_environment;
-    for (auto it = env.constBegin(); it != env.constEnd(); ++it)
-        processEnvironment.insert(it.key(), it.value());
+    for (auto it = env.constBegin(); it != env.constEnd(); ++it) {
+        if (env.isEnabled(it))
+            processEnvironment.insert(it.key(), env.value(it));
+    }
 
     return processEnvironment;
 }
diff --git a/src/libs/utils/environment.cpp b/src/libs/utils/environment.cpp
index cee6713254..15751e9c2e 100644
--- a/src/libs/utils/environment.cpp
+++ b/src/libs/utils/environment.cpp
@@ -68,8 +68,10 @@ static NameValueMap::const_iterator findKey(const NameValueMap &input,
 QProcessEnvironment Environment::toProcessEnvironment() const
 {
     QProcessEnvironment result;
-    for (auto it = m_values.constBegin(); it != m_values.constEnd(); ++it)
-        result.insert(it.key(), it.value());
+    for (auto it = m_values.constBegin(); it != m_values.constEnd(); ++it) {
+        if (it.value().second)
+            result.insert(it.key(), it.value().first);
+    }
     return result;
 }
 
@@ -90,12 +92,12 @@ void Environment::appendOrSet(const QString &key, const QString &value, const QS
     QTC_ASSERT(!key.contains('='), return );
     auto it = findKey(m_values, m_osType, key);
     if (it == m_values.end()) {
-        m_values.insert(key, value);
+        m_values.insert(key, qMakePair(value, true));
     } else {
         // Append unless it is already there
         const QString toAppend = sep + value;
-        if (!it.value().endsWith(toAppend))
-            it.value().append(toAppend);
+        if (!it.value().first.endsWith(toAppend))
+            it.value().first.append(toAppend);
     }
 }
 
@@ -104,12 +106,12 @@ void Environment::prependOrSet(const QString &key, const QString &value, const Q
     QTC_ASSERT(!key.contains('='), return );
     auto it = findKey(m_values, m_osType, key);
     if (it == m_values.end()) {
-        m_values.insert(key, value);
+        m_values.insert(key, qMakePair(value, true));
     } else {
         // Prepend unless it is already there
         const QString toPrepend = value + sep;
-        if (!it.value().startsWith(toPrepend))
-            it.value().prepend(toPrepend);
+        if (!it.value().first.startsWith(toPrepend))
+            it.value().first.prepend(toPrepend);
     }
 }
 
@@ -346,8 +348,8 @@ QString Environment::expandVariables(const QString &input) const
                 if (vStart > 0) {
                     const_iterator it = findKey(m_values, m_osType, result.mid(vStart, i - vStart - 1));
                     if (it != m_values.constEnd()) {
-                        result.replace(vStart - 1, i - vStart + 1, *it);
-                        i = vStart - 1 + it->length();
+                        result.replace(vStart - 1, i - vStart + 1, it->first);
+                        i = vStart - 1 + it->first.length();
                         vStart = -1;
                     } else {
                         vStart = i;
@@ -380,8 +382,8 @@ QString Environment::expandVariables(const QString &input) const
                 if (c == '}') {
                     const_iterator it = m_values.constFind(result.mid(vStart, i - 1 - vStart));
                     if (it != constEnd()) {
-                        result.replace(vStart - 2, i - vStart + 2, *it);
-                        i = vStart - 2 + it->length();
+                        result.replace(vStart - 2, i - vStart + 2, it->first);
+                        i = vStart - 2 + it->first.length();
                     }
                     state = BASE;
                 }
@@ -389,8 +391,8 @@ QString Environment::expandVariables(const QString &input) const
                 if (!c.isLetterOrNumber() && c != '_') {
                     const_iterator it = m_values.constFind(result.mid(vStart, i - vStart - 1));
                     if (it != constEnd()) {
-                        result.replace(vStart - 1, i - vStart, *it);
-                        i = vStart - 1 + it->length();
+                        result.replace(vStart - 1, i - vStart, it->first);
+                        i = vStart - 1 + it->first.length();
                     }
                     state = BASE;
                 }
@@ -399,7 +401,7 @@ QString Environment::expandVariables(const QString &input) const
         if (state == VARIABLE) {
             const_iterator it = m_values.constFind(result.mid(vStart));
             if (it != constEnd())
-                result.replace(vStart - 1, result.length() - vStart + 1, *it);
+                result.replace(vStart - 1, result.length() - vStart + 1, it->first);
         }
     }
     return result;
diff --git a/src/libs/utils/environmentdialog.cpp b/src/libs/utils/environmentdialog.cpp
index e9c9fd620f..88311854ec 100644
--- a/src/libs/utils/environmentdialog.cpp
+++ b/src/libs/utils/environmentdialog.cpp
@@ -47,7 +47,8 @@ Utils::optional<EnvironmentItems> EnvironmentDialog::getEnvironmentItems(
         tr("Enter one environment variable per line.\n"
            "To set or change a variable, use VARIABLE=VALUE.\n"
            "Existing variables can be referenced in a VALUE with ${OTHER}.\n"
-           "To clear a variable, put its name on a line with nothing else on it."));
+           "To clear a variable, put its name on a line with nothing else on it.\n"
+           "To disable a variable, prefix the line with \"#\""));
 }
 
 } // namespace Utils
diff --git a/src/libs/utils/namevaluedictionary.cpp b/src/libs/utils/namevaluedictionary.cpp
index 84402eaa1f..40a18513cf 100644
--- a/src/libs/utils/namevaluedictionary.cpp
+++ b/src/libs/utils/namevaluedictionary.cpp
@@ -79,19 +79,22 @@ NameValueDictionary::NameValueDictionary(const NameValuePairs &nameValues)
 QStringList NameValueDictionary::toStringList() const
 {
     QStringList result;
-    for (auto it = m_values.constBegin(); it != m_values.constEnd(); ++it)
-        result.append(it.key() + '=' + it.value());
+    for (auto it = m_values.constBegin(); it != m_values.constEnd(); ++it) {
+        if (it.value().second)
+            result.append(it.key() + '=' + it.value().first);
+    }
     return result;
 }
 
-void NameValueDictionary::set(const QString &key, const QString &value)
+void NameValueDictionary::set(const QString &key, const QString &value, bool enabled)
 {
     QTC_ASSERT(!key.contains('='), return );
     auto it = findKey(m_values, m_osType, key);
+    const auto valuePair = qMakePair(value, enabled);
     if (it == m_values.end())
-        m_values.insert(key, value);
+        m_values.insert(key, valuePair);
     else
-        it.value() = value;
+        it.value() = valuePair;
 }
 
 void NameValueDictionary::unset(const QString &key)
@@ -110,7 +113,7 @@ void NameValueDictionary::clear()
 QString NameValueDictionary::value(const QString &key) const
 {
     const auto it = findKey(m_values, m_osType, key);
-    return it != m_values.end() ? it.value() : QString();
+    return it != m_values.end() && it.value().second ? it.value().first : QString();
 }
 
 NameValueDictionary::const_iterator NameValueDictionary::constFind(const QString &name) const
@@ -146,8 +149,9 @@ NameValueItems NameValueDictionary::diff(const NameValueDictionary &other, bool
 
     NameValueItems result;
     while (thisIt != constEnd() || otherIt != other.constEnd()) {
-        if (thisIt == constEnd()) {
-            result.append(NameValueItem(otherIt.key(), otherIt.value()));
+        if (thisIt == constEnd() || thisIt.key() > otherIt.key()) {
+            result.append({otherIt.key(), otherIt.value().first,
+                otherIt.value().second ? NameValueItem::SetEnabled : NameValueItem::SetDisabled});
             ++otherIt;
         } else if (otherIt == other.constEnd()) {
             result.append(NameValueItem(thisIt.key(), QString(), NameValueItem::Unset));
@@ -155,25 +159,27 @@ NameValueItems NameValueDictionary::diff(const NameValueDictionary &other, bool
         } else if (thisIt.key() < otherIt.key()) {
             result.append(NameValueItem(thisIt.key(), QString(), NameValueItem::Unset));
             ++thisIt;
-        } else if (thisIt.key() > otherIt.key()) {
-            result.append(NameValueItem(otherIt.key(), otherIt.value()));
-            ++otherIt;
         } else {
-            const QString &oldValue = thisIt.value();
-            const QString &newValue = otherIt.value();
+            const QString &oldValue = thisIt.value().first;
+            const QString &newValue = otherIt.value().first;
+            const bool oldEnabled = thisIt.value().second;
+            const bool newEnabled = otherIt.value().second;
             if (oldValue != newValue) {
-                if (checkAppendPrepend && newValue.startsWith(oldValue)) {
+                if (checkAppendPrepend && newValue.startsWith(oldValue)
+                        && oldEnabled == newEnabled) {
                     QString appended = newValue.right(newValue.size() - oldValue.size());
                     if (appended.startsWith(QLatin1Char(pathSepC)))
                         appended.remove(0, 1);
                     result.append(NameValueItem(otherIt.key(), appended, NameValueItem::Append));
-                } else if (checkAppendPrepend && newValue.endsWith(oldValue)) {
+                } else if (checkAppendPrepend && newValue.endsWith(oldValue)
+                           && oldEnabled == newEnabled) {
                     QString prepended = newValue.left(newValue.size() - oldValue.size());
                     if (prepended.endsWith(QLatin1Char(pathSepC)))
                         prepended.chop(1);
                     result.append(NameValueItem(otherIt.key(), prepended, NameValueItem::Prepend));
                 } else {
-                    result.append(NameValueItem(otherIt.key(), newValue));
+                    result.append({otherIt.key(), newValue, newEnabled
+                            ? NameValueItem::SetEnabled : NameValueItem::SetDisabled});
                 }
             }
             ++otherIt;
diff --git a/src/libs/utils/namevaluedictionary.h b/src/libs/utils/namevaluedictionary.h
index af414a0c62..acb9800745 100644
--- a/src/libs/utils/namevaluedictionary.h
+++ b/src/libs/utils/namevaluedictionary.h
@@ -33,7 +33,7 @@ namespace Utils {
 
 using NameValuePair = std::pair<QString, QString>;
 using NameValuePairs = QVector<NameValuePair>;
-using NameValueMap = QMap<QString, QString>;
+using NameValueMap = QMap<QString, QPair<QString, bool>>;
 
 class QTCREATOR_UTILS_EXPORT NameValueDictionary
 {
@@ -48,7 +48,7 @@ public:
 
     QStringList toStringList() const;
     QString value(const QString &key) const;
-    void set(const QString &key, const QString &value);
+    void set(const QString &key, const QString &value, bool enabled = true);
     void unset(const QString &key);
     void modify(const NameValueItems &items);
     /// Return the KeyValueDictionary changes necessary to modify this into the other environment.
@@ -61,15 +61,13 @@ public:
     void clear();
     int size() const;
 
-    QString key(NameValueDictionary::const_iterator it) const { return it.key(); }
+    QString key(const_iterator it) const { return it.key(); }
+    QString value(const_iterator it) const { return it.value().first; }
+    bool isEnabled(const_iterator it) const { return it.value().second; }
 
-    QString value(NameValueDictionary::const_iterator it) const { return it.value(); }
-
-    NameValueDictionary::const_iterator constBegin() const { return m_values.constBegin(); }
-
-    NameValueDictionary::const_iterator constEnd() const { return m_values.constEnd(); }
-
-    NameValueDictionary::const_iterator constFind(const QString &name) const;
+    const_iterator constBegin() const { return m_values.constBegin(); }
+    const_iterator constEnd() const { return m_values.constEnd(); }
+    const_iterator constFind(const QString &name) const;
 
     friend bool operator!=(const NameValueDictionary &first, const NameValueDictionary &second)
     {
diff --git a/src/libs/utils/namevalueitem.cpp b/src/libs/utils/namevalueitem.cpp
index 6be972201c..0ca80f8083 100644
--- a/src/libs/utils/namevalueitem.cpp
+++ b/src/libs/utils/namevalueitem.cpp
@@ -42,10 +42,17 @@ NameValueItems NameValueItem::fromStringList(const QStringList &list)
     NameValueItems result;
     for (const QString &string : list) {
         int pos = string.indexOf('=', 1);
-        if (pos == -1)
+        if (pos == -1) {
             result.append(NameValueItem(string, QString(), NameValueItem::Unset));
-        else
-            result.append(NameValueItem(string.left(pos), string.mid(pos + 1)));
+            continue;
+        }
+        const int hashPos = string.indexOf('#');
+        if (hashPos != -1 && hashPos < pos) {
+            result.append({string.mid(hashPos + 1, pos - hashPos - 1), string.mid(pos + 1),
+                           NameValueItem::SetDisabled});
+        } else {
+            result.append({string.left(pos), string.mid(pos + 1)});
+        }
     }
     return result;
 }
@@ -55,7 +62,8 @@ QStringList NameValueItem::toStringList(const NameValueItems &list)
     return Utils::transform<QStringList>(list, [](const NameValueItem &item) {
         if (item.operation == NameValueItem::Unset)
             return QString(item.name);
-        return QString(item.name + '=' + item.value);
+        return QString((item.operation == NameValueItem::SetDisabled ? "#" : "")
+                       + item.name + '=' + item.value);
     });
 }
 
@@ -103,7 +111,7 @@ static QString expand(const NameValueDictionary *dictionary, QString value)
                     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());
+                        value.replace(i, end - i + 1, it.value().first);
                     ++replaceCount;
                     QTC_ASSERT(replaceCount < 100, break);
                 }
@@ -124,16 +132,19 @@ enum : char {
 void NameValueItem::apply(NameValueDictionary *dictionary, Operation op) const
 {
     switch (op) {
-    case Set:
+    case SetEnabled:
         dictionary->set(name, expand(dictionary, value));
         break;
+    case SetDisabled:
+        dictionary->set(name, expand(dictionary, value), false);
+        break;
     case Unset:
         dictionary->unset(name);
         break;
     case Prepend: {
         const NameValueDictionary::const_iterator it = dictionary->constFind(name);
         if (it != dictionary->constEnd()) {
-            QString v = it.value();
+            QString v = dictionary->value(it);
             const QChar pathSep{QLatin1Char(pathSepC)};
             int sepCount = 0;
             if (v.startsWith(pathSep))
@@ -147,13 +158,13 @@ void NameValueItem::apply(NameValueDictionary *dictionary, Operation op) const
             v.prepend(expand(dictionary, value));
             dictionary->set(name, v);
         } else {
-            apply(dictionary, Set);
+            apply(dictionary, SetEnabled);
         }
     } break;
     case Append: {
         const NameValueDictionary::const_iterator it = dictionary->constFind(name);
         if (it != dictionary->constEnd()) {
-            QString v = it.value();
+            QString v = dictionary->value(it);
             const QChar pathSep{QLatin1Char(pathSepC)};
             int sepCount = 0;
             if (v.endsWith(pathSep))
@@ -167,7 +178,7 @@ void NameValueItem::apply(NameValueDictionary *dictionary, Operation op) const
             v.append(expand(dictionary, value));
             dictionary->set(name, v);
         } else {
-            apply(dictionary, Set);
+            apply(dictionary, SetEnabled);
         }
     } break;
     }
@@ -180,9 +191,12 @@ QDebug operator<<(QDebug debug, const NameValueItem &i)
     debug.nospace();
     debug << "KeyValueItem(";
     switch (i.operation) {
-    case NameValueItem::Set:
+    case NameValueItem::SetEnabled:
         debug << "set \"" << i.name << "\" to \"" << i.value << '"';
         break;
+    case NameValueItem::SetDisabled:
+        debug << "set \"" << i.name << "\" to \"" << i.value << '"' << "[disabled]";
+        break;
     case NameValueItem::Unset:
         debug << "unset \"" << i.name << '"';
         break;
diff --git a/src/libs/utils/namevalueitem.h b/src/libs/utils/namevalueitem.h
index 3ed4cc3cb3..1ec203115a 100644
--- a/src/libs/utils/namevalueitem.h
+++ b/src/libs/utils/namevalueitem.h
@@ -37,9 +37,9 @@ namespace Utils {
 class QTCREATOR_UTILS_EXPORT NameValueItem
 {
 public:
-    enum Operation : char { Set, Unset, Prepend, Append };
+    enum Operation : char { SetEnabled, Unset, Prepend, Append, SetDisabled };
     NameValueItem() = default;
-    NameValueItem(const QString &key, const QString &value, Operation operation = Set)
+    NameValueItem(const QString &key, const QString &value, Operation operation = SetEnabled)
         : name(key)
         , value(value)
         , operation(operation)
diff --git a/src/libs/utils/namevaluemodel.cpp b/src/libs/utils/namevaluemodel.cpp
index 2a310d6b61..e4f0149567 100644
--- a/src/libs/utils/namevaluemodel.cpp
+++ b/src/libs/utils/namevaluemodel.cpp
@@ -28,11 +28,15 @@
 #include <utils/algorithm.h>
 #include <utils/hostosinfo.h>
 #include <utils/namevaluedictionary.h>
+#include <utils/qtcassert.h>
 
+#include <QBrush>
+#include <QColor>
 #include <QFont>
 #include <QString>
 
 namespace Utils {
+
 namespace Internal {
 
 class NameValueModelPrivate
@@ -142,19 +146,21 @@ QVariant NameValueModel::data(const QModelIndex &index, int role) const
     if (!index.isValid())
         return QVariant();
 
-    if (role == Qt::DisplayRole || role == Qt::EditRole || role == Qt::ToolTipRole) {
-        if (index.column() == 0) {
-            return d->m_resultNameValueDictionary.key(d->m_resultNameValueDictionary.constBegin()
-                                                      + index.row());
-        } else if (index.column() == 1) {
+    const auto resultIterator = d->m_resultNameValueDictionary.constBegin() + index.row();
+    switch (role) {
+    case Qt::DisplayRole:
+    case Qt::EditRole:
+    case Qt::ToolTipRole:
+        if (index.column() == 0)
+            return d->m_resultNameValueDictionary.key(resultIterator);
+        if (index.column() == 1) {
             // Do not return "<UNSET>" when editing a previously unset variable:
             if (role == Qt::EditRole) {
                 int pos = d->findInChanges(indexToVariable(index));
-                if (pos >= 0)
+                if (pos != -1)
                     return d->m_items.at(pos).value;
             }
-            QString value = d->m_resultNameValueDictionary.value(
-                d->m_resultNameValueDictionary.constBegin() + index.row());
+            QString value = d->m_resultNameValueDictionary.value(resultIterator);
             if (role == Qt::ToolTipRole && value.length() > 80) {
                 // Use html to enable text wrapping
                 value = value.toHtmlEscaped();
@@ -163,16 +169,15 @@ QVariant NameValueModel::data(const QModelIndex &index, int role) const
             }
             return value;
         }
+        break;
+    case Qt::FontRole: {
+        QFont f;
+        f.setStrikeOut(!d->m_resultNameValueDictionary.isEnabled(resultIterator));
+        return f;
     }
-    if (role == Qt::FontRole) {
-        // check whether this name value item variable exists in d->m_items
-        if (changes(d->m_resultNameValueDictionary.key(d->m_resultNameValueDictionary.constBegin()
-                                                       + index.row()))) {
-            QFont f;
-            f.setBold(true);
-            return QVariant(f);
-        }
-        return QFont();
+    case Qt::ForegroundRole:
+        return changes(d->m_resultNameValueDictionary.key(resultIterator))
+                ? QBrush(Qt::blue) : QBrush();
     }
     return QVariant();
 }
@@ -236,15 +241,20 @@ bool NameValueModel::setData(const QModelIndex &index, const QVariant &value, in
         // 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);
             // We have already changed this value
-            if (d->m_baseNameValueDictionary.hasKey(oldName)
-                && stringValue == d->m_baseNameValueDictionary.value(oldName)) {
+            if (oldIt != d->m_baseNameValueDictionary.constEnd()
+                    && stringValue == d->m_baseNameValueDictionary.value(oldIt)
+                    && d->m_baseNameValueDictionary.isEnabled(oldIt)
+                            == d->m_resultNameValueDictionary.isEnabled(newIt)) {
                 // ... and now went back to the base value
                 d->m_items.removeAt(changesPos);
             } else {
                 // ... and changed it again
                 d->m_items[changesPos].value = stringValue;
-                d->m_items[changesPos].operation = NameValueItem::Set;
+                if (d->m_items[changesPos].operation == NameValueItem::Unset)
+                    d->m_items[changesPos].operation = NameValueItem::SetEnabled;
             }
         } else {
             // Add a new change item:
@@ -346,13 +356,39 @@ void NameValueModel::unsetVariable(const QString &name)
     emit userChangesChanged();
 }
 
-bool NameValueModel::canUnset(const QString &name)
+void NameValueModel::toggleVariable(const QModelIndex &idx)
 {
-    int pos = d->findInChanges(name);
-    if (pos != -1)
-        return d->m_items.at(pos).operation == NameValueItem::Unset;
-    else
-        return false;
+    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)
+            ? NameValueItem::SetDisabled : NameValueItem::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) {
+            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->updateResultNameValueDictionary();
+    emit dataChanged(index(idx.row(), 0), index(idx.row(), 1));
+    emit userChangesChanged();
+}
+
+bool NameValueModel::isUnset(const QString &name)
+{
+    const int pos = d->findInChanges(name);
+    return pos == -1 ? false : d->m_items.at(pos).operation == NameValueItem::Unset;
+}
+
+bool NameValueModel::isEnabled(const QString &name) const
+{
+    return d->m_resultNameValueDictionary.isEnabled(d->m_resultNameValueDictionary.constFind(name));
 }
 
 bool NameValueModel::canReset(const QString &name)
diff --git a/src/libs/utils/namevaluemodel.h b/src/libs/utils/namevaluemodel.h
index 96811fec68..ede319a013 100644
--- a/src/libs/utils/namevaluemodel.h
+++ b/src/libs/utils/namevaluemodel.h
@@ -59,7 +59,9 @@ public:
     QModelIndex addVariable(const NameValueItem &item);
     void resetVariable(const QString &name);
     void unsetVariable(const QString &name);
-    bool canUnset(const QString &name);
+    void toggleVariable(const QModelIndex &index);
+    bool isUnset(const QString &name);
+    bool isEnabled(const QString &name) const;
     bool canReset(const QString &name);
     QString indexToVariable(const QModelIndex &index) const;
     QModelIndex variableToIndex(const QString &name) const;