EnvironmentItem: Introduce operation enumeration

Extend operations to handle prepend/append which can be optionally
determined by diff(). This allows cleanly implementing
the MSVC toolchain setup.

Amends c7a84634

Change-Id: Ida08d8f5e00cf5f78c20ea8d08c531b1ed22c015
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

src/libs/utils/environment.cpp

@@ -28,6 +28,7 @@
 #include "algorithm.h"
 #include "qtcassert.h"
 
+#include <QDebug>
 #include <QDir>
 #include <QProcessEnvironment>
 #include <QSet>
@@ -58,6 +59,15 @@ Q_GLOBAL_STATIC(SystemEnvironment, staticSystemEnvironment)
 
 namespace Utils {
 
+enum : char
+{
+#ifdef Q_OS_WIN
+    pathSepC = ';'
+#else
+    pathSepC = ':'
+#endif
+};
+
 void EnvironmentItem::sort(QList<EnvironmentItem> *list)
 {
     Utils::sort(*list, &EnvironmentItem::name);
@@ -68,14 +78,10 @@ QList<EnvironmentItem> EnvironmentItem::fromStringList(const QStringList &list)
     QList<EnvironmentItem> result;
     for (const QString &string : list) {
         int pos = string.indexOf('=', 1);
-        if (pos == -1) {
-            EnvironmentItem item(string, QString());
-            item.unset = true;
-            result.append(item);
-        } else {
-            EnvironmentItem item(string.left(pos), string.mid(pos+1));
-            result.append(item);
-        }
+        if (pos == -1)
+            result.append(EnvironmentItem(string, QString(), EnvironmentItem::Unset));
+        else
+            result.append(EnvironmentItem(string.left(pos), string.mid(pos + 1)));
     }
     return result;
 }
@@ -84,7 +90,7 @@ QStringList EnvironmentItem::toStringList(const QList<EnvironmentItem> &list)
 {
     QStringList result;
     for (const EnvironmentItem &item : list) {
-        if (item.unset)
+        if (item.operation == EnvironmentItem::Unset)
             result << QString(item.name);
         else
             result << QString(item.name + '=' + item.value);
@@ -92,6 +98,110 @@ QStringList EnvironmentItem::toStringList(const QList<EnvironmentItem> &list)
     return result;
 }
 
+static QString expand(const Environment *e, QString value)
+{
+    int replaceCount = 0;
+    for (int i = 0; i < value.size(); ++i) {
+        if (value.at(i) == '$') {
+            if ((i + 1) < value.size()) {
+                const QChar &c = value.at(i+1);
+                int end = -1;
+                if (c == '(')
+                    end = value.indexOf(')', i);
+                else if (c == '{')
+                    end = value.indexOf('}', i);
+                if (end != -1) {
+                    const QString &name = value.mid(i + 2, end - i - 2);
+                    Environment::const_iterator it = e->constFind(name);
+                    if (it != e->constEnd())
+                        value.replace(i, end - i + 1, it.value());
+                    ++replaceCount;
+                    QTC_ASSERT(replaceCount < 100, break);
+                }
+            }
+        }
+    }
+    return value;
+}
+
+QDebug operator<<(QDebug debug, const EnvironmentItem &i)
+{
+    QDebugStateSaver saver(debug);
+    debug.noquote();
+    debug.nospace();
+    debug << "EnvironmentItem(";
+    switch (i.operation) {
+    case Utils::EnvironmentItem::Set:
+        debug << "set \"" << i.name << "\" to \"" << i.value << '"';
+        break;
+    case Utils::EnvironmentItem::Unset:
+        debug << "unset \"" << i.name << '"';
+        break;
+    case Utils::EnvironmentItem::Prepend:
+        debug << "prepend to \"" << i.name << "\":\"" << i.value << '"';
+        break;
+    case Utils::EnvironmentItem::Append:
+        debug << "append to \"" << i.name << "\":\"" << i.value << '"';
+        break;
+    }
+    debug << ')';
+    return debug;
+}
+
+void EnvironmentItem::apply(Environment *e, Operation op) const
+{
+    switch (op) {
+    case Set:
+        e->set(name, expand(e, value));
+        break;
+    case Unset:
+        e->unset(name);
+        break;
+    case Prepend: {
+        const Environment::const_iterator it = e->constFind(name);
+        if (it != e->constEnd()) {
+            QString v = it.value();
+            const QChar pathSep{QLatin1Char(pathSepC)};
+            int sepCount = 0;
+            if (v.startsWith(pathSep))
+                ++sepCount;
+            if (value.endsWith(pathSep))
+                ++sepCount;
+            if (sepCount == 2)
+                v.remove(0, 1);
+            else if (sepCount == 0)
+                v.prepend(pathSep);
+            v.prepend(expand(e, value));
+            e->set(name, v);
+        } else {
+            apply(e, Set);
+        }
+    }
+        break;
+    case Append: {
+        const Environment::const_iterator it = e->constFind(name);
+        if (it != e->constEnd()) {
+            QString v = it.value();
+            const QChar pathSep{QLatin1Char(pathSepC)};
+            int sepCount = 0;
+            if (v.endsWith(pathSep))
+                ++sepCount;
+            if (value.startsWith(pathSep))
+                ++sepCount;
+            if (sepCount == 2)
+                v.chop(1);
+            else if (sepCount == 0)
+                v.append(pathSep);
+            v.append(expand(e, value));
+            e->set(name, v);
+        } else {
+            apply(e, Set);
+        }
+    }
+        break;
+    }
+}
+
 Environment::Environment(const QStringList &env, OsType osType) : m_osType(osType)
 {
     for (const QString &s : env) {
@@ -367,40 +477,12 @@ int Environment::size() const
 void Environment::modify(const QList<EnvironmentItem> & list)
 {
     Environment resultEnvironment = *this;
-    for (const EnvironmentItem &item : list) {
-        if (item.unset) {
-            resultEnvironment.unset(item.name);
-        } else {
-            // TODO use variable expansion
-            QString value = item.value;
-            int replaceCount = 0;
-            for (int i=0; i < value.size(); ++i) {
-                if (value.at(i) == '$') {
-                    if ((i + 1) < value.size()) {
-                        const QChar &c = value.at(i+1);
-                        int end = -1;
-                        if (c == '(')
-                            end = value.indexOf(')', i);
-                        else if (c == '{')
-                            end = value.indexOf('}', i);
-                        if (end != -1) {
-                            const QString &name = value.mid(i+2, end-i-2);
-                            Environment::const_iterator it = constFind(name);
-                            if (it != constEnd())
-                                value.replace(i, end-i+1, it.value());
-                            ++replaceCount;
-                            QTC_ASSERT(replaceCount < 100, break);
-                        }
-                    }
-                }
-            }
-            resultEnvironment.set(item.name, value);
-        }
-    }
+    for (const EnvironmentItem &item : list)
+        item.apply(&resultEnvironment);
     *this = resultEnvironment;
 }
 
-QList<EnvironmentItem> Environment::diff(const Environment &other) const
+QList<EnvironmentItem> Environment::diff(const Environment &other, bool checkAppendPrepend) const
 {
     QMap<QString, QString>::const_iterator thisIt = constBegin();
     QMap<QString, QString>::const_iterator otherIt = other.constBegin();
@@ -411,21 +493,34 @@ QList<EnvironmentItem> Environment::diff(const Environment &other) const
             result.append(EnvironmentItem(otherIt.key(), otherIt.value()));
             ++otherIt;
         } else if (otherIt == other.constEnd()) {
-            EnvironmentItem item(thisIt.key(), QString());
-            item.unset = true;
-            result.append(item);
+            result.append(EnvironmentItem(thisIt.key(), QString(), EnvironmentItem::Unset));
             ++thisIt;
         } else if (thisIt.key() < otherIt.key()) {
-            EnvironmentItem item(thisIt.key(), QString());
-            item.unset = true;
-            result.append(item);
+            result.append(EnvironmentItem(thisIt.key(), QString(), EnvironmentItem::Unset));
             ++thisIt;
         } else if (thisIt.key() > otherIt.key()) {
             result.append(EnvironmentItem(otherIt.key(), otherIt.value()));
             ++otherIt;
         } else {
-            if (thisIt.value() != otherIt.value())
-                result.append(EnvironmentItem(otherIt.key(), otherIt.value()));
+            const QString &oldValue = thisIt.value();
+            const QString &newValue = otherIt.value();
+            if (oldValue != newValue) {
+                if (checkAppendPrepend && newValue.startsWith(oldValue)) {
+                    QString appended = newValue.right(newValue.size() - oldValue.size());
+                    if (appended.startsWith(QLatin1Char(pathSepC)))
+                        appended.remove(0, 1);
+                    result.append(EnvironmentItem(otherIt.key(), appended,
+                                                  EnvironmentItem::Append));
+                } else if (checkAppendPrepend && newValue.endsWith(oldValue)) {
+                    QString prepended = newValue.left(newValue.size() - oldValue.size());
+                    if (prepended.endsWith(QLatin1Char(pathSepC)))
+                        prepended.chop(1);
+                    result.append(EnvironmentItem(otherIt.key(), prepended,
+                                                  EnvironmentItem::Prepend));
+                } else {
+                    result.append(EnvironmentItem(otherIt.key(), newValue));
+                }
+            }
             ++otherIt;
             ++thisIt;
         }

src/libs/utils/environment.h

@@ -34,31 +34,42 @@
 
 #include <functional>
 
+QT_FORWARD_DECLARE_CLASS(QDebug)
 QT_FORWARD_DECLARE_CLASS(QProcessEnvironment)
 
 namespace Utils {
+class Environment;
 
 class QTCREATOR_UTILS_EXPORT EnvironmentItem
 {
 public:
-    EnvironmentItem(const QString &n, const QString &v)
-            : name(n), value(v), unset(false)
+    enum Operation { Set, Unset, Prepend, Append };
+
+    EnvironmentItem(const QString &n, const QString &v, Operation op = Set)
+            : name(n), value(v), operation(op)
     {}
 
+    void apply(Environment *e) const { apply(e, operation); }
+
     QString name;
     QString value;
-    bool unset;
+    Operation operation;
 
     bool operator==(const EnvironmentItem &other) const
     {
-        return unset == other.unset && name == other.name && value == other.value;
+        return operation == other.operation && name == other.name && value == other.value;
     }
 
     static void sort(QList<EnvironmentItem> *list);
     static QList<EnvironmentItem> fromStringList(const QStringList &list);
     static QStringList toStringList(const QList<EnvironmentItem> &list);
+
+private:
+    void apply(Environment *e, Operation op) const;
 };
 
+QTCREATOR_UTILS_EXPORT QDebug operator<<(QDebug debug, const EnvironmentItem &i);
+
 class QTCREATOR_UTILS_EXPORT Environment
 {
 public:
@@ -78,7 +89,7 @@ public:
     void unset(const QString &key);
     void modify(const QList<EnvironmentItem> &list);
     /// Return the Environment changes necessary to modify this into the other environment.
-    QList<EnvironmentItem> diff(const Environment &other) const;
+    QList<EnvironmentItem> diff(const Environment &other, bool checkAppendPrepend = false) const;
     bool hasKey(const QString &key) const;
 
     QString userName() const;

src/libs/utils/environmentmodel.cpp

@@ -44,7 +44,7 @@ public:
         // Add removed variables again and mark them as "<UNSET>" so
         // that the user can actually see those removals:
         foreach (const EnvironmentItem &item, m_items) {
-            if (item.unset)
+            if (item.operation == EnvironmentItem::Unset)
                 m_resultEnvironment.set(item.name, EnvironmentModel::tr("<UNSET>"));
         }
     }
@@ -230,7 +230,7 @@ bool EnvironmentModel::setData(const QModelIndex &index, const QVariant &value,
             } else {
                 // ... and changed it again
                 d->m_items[changesPos].value = stringValue;
-                d->m_items[changesPos].unset = false;
+                d->m_items[changesPos].operation = EnvironmentItem::Set;
             }
         } else {
             // Add a new change item:
@@ -267,7 +267,7 @@ QModelIndex EnvironmentModel::addVariable(const EnvironmentItem &item)
         Q_ASSERT(changePos >= 0);
         // Do not insert a line here as we listed the variable as <UNSET> before!
         Q_ASSERT(d->m_items.at(changePos).name == item.name);
-        Q_ASSERT(d->m_items.at(changePos).unset);
+        Q_ASSERT(d->m_items.at(changePos).operation == EnvironmentItem::Unset);
         Q_ASSERT(d->m_items.at(changePos).value.isEmpty());
         d->m_items[changePos] = item;
         emit dataChanged(index(insertPos, 0, QModelIndex()), index(insertPos, 1, QModelIndex()));
@@ -320,16 +320,14 @@ void EnvironmentModel::unsetVariable(const QString &name)
     // look in d->m_items for the variable
     int pos = d->findInChanges(name);
     if (pos != -1) {
-        d->m_items[pos].unset = true;
+        d->m_items[pos].operation = EnvironmentItem::Unset;
         d->m_items[pos].value.clear();
         d->updateResultEnvironment();
         emit dataChanged(index(row, 0, QModelIndex()), index(row, 1, QModelIndex()));
         emit userChangesChanged();
         return;
     }
-    EnvironmentItem item(name, QString());
-    item.unset = true;
-    d->m_items.append(item);
+    d->m_items.append(EnvironmentItem(name, QString(), EnvironmentItem::Unset));
     d->updateResultEnvironment();
     emit dataChanged(index(row, 0, QModelIndex()), index(row, 1, QModelIndex()));
     emit userChangesChanged();
@@ -339,7 +337,7 @@ bool EnvironmentModel::canUnset(const QString &name)
 {
     int pos = d->findInChanges(name);
     if (pos != -1)
-        return d->m_items.at(pos).unset;
+        return d->m_items.at(pos).operation == EnvironmentItem::Unset;
     else
         return false;
 }

src/plugins/debugger/gdb/gdbengine.cpp

@@ -4046,7 +4046,7 @@ void GdbEngine::setEnvironmentVariables()
     Environment sysEnv = Environment::systemEnvironment();
     Environment runEnv = runParameters().inferior.environment;
     foreach (const EnvironmentItem &item, sysEnv.diff(runEnv)) {
-        if (item.unset)
+        if (item.operation == EnvironmentItem::Unset)
             runCommand({"unset environment " + item.name});
         else
             runCommand({"-gdb-set environment " + item.name + '='

src/plugins/projectexplorer/environmentwidget.cpp

@@ -275,7 +275,7 @@ void EnvironmentWidget::updateSummaryText()
     foreach (const Utils::EnvironmentItem &item, list) {
         if (item.name != Utils::EnvironmentModel::tr("<VARIABLE>")) {
             text.append(QLatin1String("<br>"));
-            if (item.unset)
+            if (item.operation == Utils::EnvironmentItem::Unset)
                 text.append(tr("Unset <a href=\"%1\"><b>%1</b></a>").arg(item.name.toHtmlEscaped()));
             else
                 text.append(tr("Set <a href=\"%1\"><b>%1</b></a> to <b>%2</b>").arg(item.name.toHtmlEscaped(), item.value.toHtmlEscaped()));

src/plugins/projectexplorer/msvctoolchain.cpp

@@ -529,23 +529,10 @@ QList<Utils::EnvironmentItem> MsvcToolChain::environmentModifications() const
         }
     }
 
-    QList<Utils::EnvironmentItem> diff = inEnv.diff(outEnv);
+    QList<Utils::EnvironmentItem> diff = inEnv.diff(outEnv, true);
     for (int i = diff.size() - 1; i >= 0; --i) {
         if (diff.at(i).name.startsWith(QLatin1Char('='))) { // Exclude "=C:", "=EXITCODE"
             diff.removeAt(i);
-        } else {
-            // Fix the append/prepend cases to "FOO=${FOO};newValue" (see Environment::modify)
-            Utils::EnvironmentItem &e = diff[i];
-            if (!e.unset) {
-                const auto oldIt = inEnv.constFind(e.name);
-                if (oldIt != inEnv.constEnd()) {
-                    const int index = e.value.indexOf(oldIt.value());
-                    if (index != -1) {
-                        e.value.replace(index, oldIt.value().size(),
-                                        QStringLiteral("${") + e.name + QLatin1Char('}'));
-                    }
-                }
-            }
         }
     }