Valgrind: Simplify suppression file interface

The global settings and per-project settings looked the same,
but behaved quite differently: The per-project one were a kind
of diff against the global one.

Besides having "issues" when keeping the temporary and permanent places
where relevant parts of the data were kept (settings, manual-applied
global, auto-applied local settings), the concept was not clear in the
UI at all.

This here takes the simple way out: Either local, or global, no diffs.

Change-Id: I90439cd20067ab60b88372f1cb03eeef8c2e42d3
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/valgrind/memchecktool.cpp

@@ -996,7 +996,8 @@ void MemcheckToolPrivate::setupRunner(MemcheckToolRunner *runTool)
 
     m_errorView->setDefaultSuppressionFile(dir + name + ".supp");
 
-    foreach (const QString &file, runTool->suppressionFiles()) {
+    const QStringList suppressionFiles = runTool->suppressionFiles();
+    for (const QString &file : suppressionFiles) {
         QAction *action = m_filterMenu->addAction(FilePath::fromString(file).fileName());
         action->setToolTip(file);
         connect(action, &QAction::triggered, this, [file] {

src/plugins/valgrind/valgrindsettings.cpp

@@ -52,14 +52,6 @@ namespace Internal {
 // SuppressionAspect
 //
 
-// This is somewhat unusual, as it looks the same in Global Settings and Project
-// settings, but behaves differently (Project only stores a diff) depending on
-// context.
-
-const char globalSuppressionKey[] = "Analyzer.Valgrind.SupressionFiles";
-const char removedProjectSuppressionKey[] = "Analyzer.Valgrind.RemovedSuppressionFiles";
-const char addedProjectSuppressionKey[] = "Analyzer.Valgrind.AddedSuppressionFiles";
-
 class SuppressionAspectPrivate : public QObject
 {
     Q_DECLARE_TR_FUNCTIONS(Valgrind::Internal::ValgrindConfigWidget)
@@ -79,24 +71,13 @@ public:
     QPointer<QListView> entryList;
 
     QStandardItemModel m_model; // The volatile value of this aspect.
-
-    QStringList globalSuppressionFiles; // Real value, only used for global settings
-
-    QStringList removedProjectSuppressionFiles; // Part of real value for project settings
-    QStringList addedProjectSuppressionFiles; // Part of real value for project settings
 };
 
 void SuppressionAspect::addSuppressionFile(const QString &suppression)
 {
-    if (d->isGlobal) {
+    QStringList val = value();
-        d->globalSuppressionFiles.append(suppression);
+    val.append(suppression);
-    } else {
+    setValue(val);
-        const QStringList globalSuppressions = ValgrindGlobalSettings::instance()->suppressions.value();
-        if (!d->addedProjectSuppressionFiles.contains(suppression))
-            d->addedProjectSuppressionFiles.append(suppression);
-        d->removedProjectSuppressionFiles.removeAll(suppression);
-    }
-    setVolatileValue(value());
 }
 
 void SuppressionAspectPrivate::slotAddSuppression()
@@ -152,6 +133,7 @@ void SuppressionAspectPrivate::slotSuppressionSelectionChanged()
 SuppressionAspect::SuppressionAspect(bool global)
 {
     d = new SuppressionAspectPrivate(this, global);
+    setSettingsKey("Analyzer.Valgrind.SuppressionFiles");
 }
 
 SuppressionAspect::~SuppressionAspect()
@@ -161,34 +143,12 @@ SuppressionAspect::~SuppressionAspect()
 
 QStringList SuppressionAspect::value() const
 {
-    // Note: BaseAspect::d->value is /not/ used.
+    return BaseAspect::value().toStringList();
-    if (d->isGlobal)
-        return d->globalSuppressionFiles;
-
-    QStringList ret = ValgrindGlobalSettings::instance()->suppressions.value();
-    for (const QString &s : d->removedProjectSuppressionFiles)
-        ret.removeAll(s);
-    ret.append(d->addedProjectSuppressionFiles);
-    return ret;
 }
 
 void SuppressionAspect::setValue(const QStringList &val)
 {
-    if (d->isGlobal) {
+    BaseAspect::setValue(val);
-        d->globalSuppressionFiles = val;
-    } else {
-        const QStringList globals = ValgrindGlobalSettings::instance()->suppressions.value();
-        d->addedProjectSuppressionFiles.clear();
-        for (const QString &s : val) {
-            if (!globals.contains(s))
-                d->addedProjectSuppressionFiles.append(s);
-        }
-        d->removedProjectSuppressionFiles.clear();
-        for (const QString &s : globals) {
-            if (!val.contains(s))
-                d->removedProjectSuppressionFiles.append(s);
-        }
-    }
 }
 
 void SuppressionAspect::addToLayout(LayoutBuilder &builder)
@@ -202,7 +162,7 @@ void SuppressionAspect::addToLayout(LayoutBuilder &builder)
     d->addEntry = new QPushButton(tr("Add..."));
     d->removeEntry = new QPushButton(tr("Remove"));
 
-    d->entryList = new QListView;
+    d->entryList = createSubWidget<QListView>();
     d->entryList->setModel(&d->m_model);
     d->entryList->setSelectionMode(QAbstractItemView::MultiSelection);
 
@@ -216,37 +176,21 @@ void SuppressionAspect::addToLayout(LayoutBuilder &builder)
     builder.addItem(Column { new QLabel(tr("Suppression files:")), Stretch() });
     Row group {
         d->entryList.data(),
-                Column { d->addEntry.data(), d->removeEntry.data(), Stretch() }
+        Column { d->addEntry.data(), d->removeEntry.data(), Stretch() }
     };
     builder.addItem(Span { 2, group });
+
+    setVolatileValue(value());
 }
 
 void SuppressionAspect::fromMap(const QVariantMap &map)
 {
-    if (d->isGlobal) {
+    BaseAspect::fromMap(map);
-        d->globalSuppressionFiles = map.value(globalSuppressionKey).toStringList();
-    } else {
-        d->addedProjectSuppressionFiles = map.value(addedProjectSuppressionKey).toStringList();
-        d->removedProjectSuppressionFiles = map.value(removedProjectSuppressionKey).toStringList();
-    }
-    setVolatileValue(value());
 }
 
 void SuppressionAspect::toMap(QVariantMap &map) const
 {
-    auto save = [&map](const QStringList &data, const QString &key) {
+    BaseAspect::toMap(map);
-        if (data.isEmpty())
-            map.remove(key);
-        else
-            map.insert(key, data);
-    };
-
-    if (d->isGlobal) {
-        save(d->globalSuppressionFiles, globalSuppressionKey);
-    } else {
-        save(d->addedProjectSuppressionFiles, addedProjectSuppressionKey);
-        save(d->removedProjectSuppressionFiles, removedProjectSuppressionKey);
-    }
 }
 
 QVariant SuppressionAspect::volatileValue() const
@@ -267,21 +211,6 @@ void SuppressionAspect::setVolatileValue(const QVariant &val)
         d->m_model.appendRow(new QStandardItem(file));
 }
 
-void SuppressionAspect::cancel()
-{
-    setVolatileValue(value());
-}
-
-void SuppressionAspect::apply()
-{
-    setValue(volatileValue().toStringList());
-}
-
-void SuppressionAspect::finish()
-{
-    setVolatileValue(value()); // Clean up m_model content
-}
-
 //////////////////////////////////////////////////////////////////
 //
 // ValgrindBaseSettings

src/plugins/valgrind/valgrindsettings.h

@@ -57,10 +57,6 @@ public:
     QVariant volatileValue() const final;
     void setVolatileValue(const QVariant &val) final;
 
-    void cancel() final;
-    void apply() final;
-    void finish() final;
-
     void addSuppressionFile(const QString &suppressionFile);
 
 private: