Utils: Allow comments in environment items

This patch allows commenting environment changes or adding
comments to the batch edit widget.
To mark a line as comment prefix it with '##'.

Modifying the environment by using the batch edit mode
allows using '#' to disable variables.
Mis-using this to disable statements of the environment
items widget is tempting and other tools explicitly allow
it this way. But when doing so, the environment may get
some unforeseen modifications.
So, explicitly provide a mechanism for comments and be
more clear about this inside the documentation.

Change-Id: I6a58d0d00e996a3f886ec30e826cade324321818
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

doc/qtcreator/src/projects/creator-only/creator-projects-settings-environment.qdoc

@@ -141,6 +141,10 @@
 
     Use the following syntax to enter environment variable names and values:
     \c {<VARIABLE>=<VALUE>}.
+    To temporarily disable a variable, add a hash character (#) to the beginning
+    of the line.
+    \note Using this approach for a different statement (append, prepend, unset)
+    may result in unexpected changes of the environment.
 
     To remove a variable value from the environment, enter the variable name.
     For example, \c TEST sets the value of the \c TEST variable empty when
@@ -160,8 +164,8 @@
     following lines. However, you can remove a value after you have referred to
     it on an earlier line.
 
-    To temporarily disable a variable, add a hash character (#) to the beginning
+    To add a comment or disable any of the above actions, prefix it with two hash
-    of the line.
+    characters (##).
 
     \sa {Specify the environment for projects}, {Configure projects for building},
     {Configure projects for running}, {Use Qt Creator variables}

src/libs/utils/environmentmodel.cpp

@@ -397,7 +397,8 @@ EnvironmentItems EnvironmentModel::userChanges() const
 void EnvironmentModel::setUserChanges(const EnvironmentItems &items)
 {
     EnvironmentItems filtered = Utils::filtered(items, [](const EnvironmentItem &i) {
-        return i.name != "export " && !i.name.contains('=');
+        return i.operation == EnvironmentItem::Comment
+                || (i.name != "export " && !i.name.contains('='));
     });
     // We assume nobody is reordering the items here.
     if (filtered == d->m_items)

src/libs/utils/namevalueitem.cpp

@@ -19,6 +19,10 @@ EnvironmentItems EnvironmentItem::fromStringList(const QStringList &list)
 {
     EnvironmentItems result;
     for (const QString &string : list) {
+        if (string.startsWith("##")) {
+            result.append({string.mid(2), {}, EnvironmentItem::Comment});
+            continue;
+        }
         int pos = string.indexOf("+=");
         if (pos != -1) {
             result.append({string.left(pos), string.mid(pos + 2), EnvironmentItem::Append});
@@ -59,6 +63,8 @@ QStringList EnvironmentItem::toStringList(const EnvironmentItems &list)
             return QString('#' + item.name + '=' + item.value);
         case EnvironmentItem::SetEnabled:
             return QString(item.name + '=' + item.value);
+        case EnvironmentItem::Comment:
+            return QString("##" + item.name);
         }
         return QString();
     });
@@ -170,6 +176,8 @@ void EnvironmentItem::apply(NameValueDictionary *dictionary, Operation op) const
             apply(dictionary, SetEnabled);
         }
     } break;
+    case Comment: // ignore comments when applying to environment
+        break;
     }
 }
 
@@ -195,6 +203,9 @@ QDebug operator<<(QDebug debug, const EnvironmentItem &i)
     case EnvironmentItem::Append:
         debug << "append to \"" << i.name << "\":\"" << i.value << '"';
         break;
+    case EnvironmentItem::Comment:
+        debug << "comment:" << i.name;
+        break;
     }
     debug << ')';
     return debug;

src/libs/utils/namevalueitem.h

@@ -16,7 +16,7 @@ namespace Utils {
 class QTCREATOR_UTILS_EXPORT EnvironmentItem
 {
 public:
-    enum Operation : char { SetEnabled, Unset, Prepend, Append, SetDisabled };
+    enum Operation : char { SetEnabled, Unset, Prepend, Append, SetDisabled, Comment };
     EnvironmentItem() = default;
     EnvironmentItem(const QString &key, const QString &value, Operation operation = SetEnabled)
         : name(key)

src/libs/utils/namevaluesdialog.cpp

@@ -60,11 +60,12 @@ NameValueItemsWidget::NameValueItemsWidget(QWidget *parent)
     const QString helpText = Tr::tr(
         "Enter one environment variable per line.\n"
         "To set or change a variable, use VARIABLE=VALUE.\n"
+        "To disable a variable, prefix this line with \"#\".\n"
         "To append to a variable, use VARIABLE+=VALUE.\n"
         "To prepend to 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.\n"
-        "To disable a variable, prefix the line with \"#\".");
+        "Lines starting with \"##\" will be treated as comments.");
 
     m_editor = new Internal::TextEditHelper(this);
     auto layout = new QVBoxLayout(this);

src/plugins/projectexplorer/environmentwidget.cpp

@@ -363,7 +363,10 @@ void EnvironmentWidget::updateSummaryText()
         return;
     }
 
-    Utils::EnvironmentItems list = d->m_model->userChanges();
+    Utils::EnvironmentItems list
+            = Utils::filtered(d->m_model->userChanges(), [](const EnvironmentItem &it) {
+        return it.operation != Utils::EnvironmentItem::Comment;
+    });
     Utils::EnvironmentItem::sort(&list);
 
     QString text;
@@ -387,6 +390,8 @@ void EnvironmentWidget::updateSummaryText()
             case Utils::EnvironmentItem::SetDisabled:
                 text.append(Tr::tr("Set <a href=\"%1\"><b>%1</b></a> to <b>%2</b> [disabled]").arg(item.name.toHtmlEscaped(), item.value.toHtmlEscaped()));
                 break;
+            case Utils::EnvironmentItem::Comment:
+                break;
             }
         }
     }

tests/auto/environment/tst_environment.cpp

@@ -1,6 +1,7 @@
 // Copyright (C) 2016 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 
+#include <utils/algorithm.h>
 #include <utils/environment.h>
 
 #include <QtTest>
@@ -263,10 +264,18 @@ void tst_Environment::incrementalChanges()
 {
     const Environment origEnv({{"VAR1", "VALUE1"}, {"VAR2", "VALUE2"}, {"PATH", "/usr/bin"}});
     const EnvironmentItems changes({
+        {"VAR1", QString(), EnvironmentItem::Comment},
         {"VAR1", QString(), EnvironmentItem::Unset},
         {"VAR2", "VALUE2", EnvironmentItem::SetDisabled},
+        {"PATH+=/bin", QString(), EnvironmentItem::Comment},
         {"PATH", "/usr/local/bin", EnvironmentItem::Append},
-        {"PATH", "/tmp", EnvironmentItem::Prepend}});
+        {"PATH", "/tmp", EnvironmentItem::Prepend},
+        {"PATH=/opt/bin", QString(), EnvironmentItem::Comment}});
+
+    const QStringList changesStringList = EnvironmentItem::toStringList(changes);
+    const int comments = Utils::count(changesStringList,
+                                      [](const QString &line) { return line.startsWith("##"); });
+    QCOMPARE(comments, 3);
 
     // Check values after change application.
     Environment newEnv = origEnv;
@@ -290,7 +299,7 @@ void tst_Environment::incrementalChanges()
     QCOMPARE(newEnv2, origEnv);
 
     // Check conversion round-trips.
-    QCOMPARE(EnvironmentItem::fromStringList(EnvironmentItem::toStringList(changes)), changes);
+    QCOMPARE(EnvironmentItem::fromStringList(changesStringList), changes);
     QCOMPARE(EnvironmentItem::fromStringList(EnvironmentItem::toStringList(diff)), diff);
     QCOMPARE(EnvironmentItem::fromStringList(EnvironmentItem::toStringList(reverseDiff)), reverseDiff);
     QCOMPARE(EnvironmentItem::itemsFromVariantList(EnvironmentItem::toVariantList(changes)), changes);