fix: Fix saving stringlist in Settings via script

We can't use a QVariant, as it does not support list.
We have to take a QJSValue, and transform it in a QVariant ?!

Author: narnaud

docs/API/knut/settings.md

@@ -17,7 +17,7 @@ import Knut
 | | Name |
 |-|-|
 |bool |**[hasValue](#hasValue)**(string path)|
-|variant |**[setValue](#setValue)**(string path, variant value)|
+|variant |**[setValue](#setValue)**(string path, var value)|
 |variant |**[value](#value)**(string path, variant defaultValue = null)|
 
 ## Detailed Description
@@ -42,7 +42,7 @@ Returns true if Knut is currently in a test, and false otherwise
 
 Returns true if the project settings has a settings `path`.
 
-#### <a name="setValue"></a>variant **setValue**(string path, variant value)
+#### <a name="setValue"></a>variant **setValue**(string path, var value)
 
 Adds a new value `value` to the project settings at the given `path`. Returns `true` if the operation succeeded.
 

src/core/settings.cpp

@@ -175,10 +175,10 @@ QVariant Settings::value(QString path, const QVariant &defaultValue) const
 }
 
 /*!
- * \qmlmethod variant Settings::setValue(string path, variant value)
+ * \qmlmethod variant Settings::setValue(string path, var value)
  * Adds a new value `value` to the project settings at the given `path`. Returns `true` if the operation succeeded.
  */
-bool Settings::setValue(QString path, const QVariant &value)
+bool Settings::setValue(QString path, const QJSValue &value)
 {
     LOG("Settings::setValue", path, value);
 
@@ -190,34 +190,47 @@ bool Settings::setValue(QString path, const QVariant &value)
     if (!path.startsWith('/'))
         path.prepend('/');
 
-    auto jsonPath = nlohmann::json::json_pointer(path.toStdString());
-    switch (static_cast<QMetaType::Type>(value.typeId())) {
+    const auto jsonPath = nlohmann::json::json_pointer(path.toStdString());
+
+    const auto var = value.toVariant();
+
+    switch (static_cast<QMetaType::Type>(var.typeId())) {
     case QMetaType::Bool:
-        m_settings[jsonPath] = value.toBool();
-        m_projectSettings[jsonPath] = value.toBool();
+        m_settings[jsonPath] = var.toBool();
+        m_projectSettings[jsonPath] = var.toBool();
         break;
     case QMetaType::Int:
     case QMetaType::LongLong:
-        m_settings[jsonPath] = value.toInt();
-        m_projectSettings[jsonPath] = value.toInt();
+        m_settings[jsonPath] = var.toInt();
+        m_projectSettings[jsonPath] = var.toInt();
         break;
     case QMetaType::UInt:
     case QMetaType::ULongLong:
-        m_settings[jsonPath] = value.toUInt();
-        m_projectSettings[jsonPath] = value.toUInt();
+        m_settings[jsonPath] = var.toUInt();
+        m_projectSettings[jsonPath] = var.toUInt();
         break;
     case QMetaType::Double:
-        m_settings[jsonPath] = value.toDouble();
-        m_projectSettings[jsonPath] = value.toDouble();
+        m_settings[jsonPath] = var.toDouble();
+        m_projectSettings[jsonPath] = var.toDouble();
         break;
     case QMetaType::QString:
-        m_settings[jsonPath] = value.toString();
+        m_settings[jsonPath] = var.toString();
         m_projectSettings[jsonPath] = value.toString();
         break;
     case QMetaType::QStringList:
-        m_settings[jsonPath] = value.toStringList();
-        m_projectSettings[jsonPath] = value.toStringList();
+        m_settings[jsonPath] = var.toStringList();
+        m_projectSettings[jsonPath] = var.toStringList();
         break;
+    case QMetaType::QVariantList: {
+        // A stringlist from QML will have a QVariantList meta type
+        const QStringList list = var.toStringList();
+        if (list.size() == var.toList().size()) {
+            m_settings[jsonPath] = list;
+            m_projectSettings[jsonPath] = list;
+        } else {
+            spdlog::error("Settings::setValue {} in {} - only string lists are supported", value.toString(), path);
+        }
+    } break;
     default:
         spdlog::error("Settings::setValue {} in {} - value type not handled", value.toString(), path);
         return false;

src/core/settings.h

@@ -14,6 +14,7 @@
 #include "utils/json.h"
 #include "utils/log.h"
 
+#include <QJSValue>
 #include <QObject>
 #include <QStringList>
 #include <QTimer>
@@ -110,7 +111,7 @@ class Settings : public QObject
     bool hasLsp() const;
 
 public slots:
-    bool setValue(QString path, const QVariant &value);
+    bool setValue(QString path, const QJSValue &value);
 
 signals:
     void settingsLoaded();

test_data/tst_settings.qml

@@ -45,4 +45,5 @@ Script {
         verify(Settings.hasValue("/numbers"));
         compare(Settings.value("/numbers"), undefined);
     }
+
 }

test_data/tst_settings/setValue/knut.json.expected

@@ -28,7 +28,10 @@
     "rc": {
         "dialog_scalex": 2.0
     },
+    "thisisabooltest": true,
+    "thisisadoubletest": 3.5,
     "thisisanothertest": 10,
+    "thisisastringtest": "test",
     "thisisatest": [
         "This",
         "is",

test_data/tst_settings_rw.qml

@@ -0,0 +1,30 @@
+/*
+  This file is part of Knut.
+
+  SPDX-FileCopyrightText: 2024 Klarälvdalens Datakonsult AB, a KDAB Group company <[email protected]>
+
+  SPDX-License-Identifier: GPL-3.0-only
+
+  Contact KDAB at <[email protected]> for commercial licensing options.
+*/
+
+import QtQuick
+import Knut
+
+Script {
+    // We want to test the writing of settings without any project, that's why it's in a different QML file
+    function test_setSettings() {
+        verify(Settings.setValue("/int_val", 10))
+        compare(Settings.value("/int_val"), 10)
+
+        verify(Settings.setValue("double_val", 3.5))
+        compare(Settings.value("double_val"), 3.5)
+
+        verify(Settings.setValue("boolean/val", true))
+        compare(Settings.value("boolean/val"), true)
+
+        verify(Settings.setValue("/stringlistval", ["a", "b", "c"]))
+        compare(Settings.value("/stringlistval"), ["a", "b", "c"])
+    }
+
+}

tests/tst_knut.cpp

@@ -76,6 +76,7 @@ class TestKnut : public QObject
 
 private slots:
     KNUT_TEST(settings)
+    KNUT_TEST(settings_rw)
     KNUT_TEST(dir)
     KNUT_TEST(fileinfo)
     KNUT_TEST(utils)

tests/tst_settings.cpp

@@ -97,15 +97,21 @@ private slots:
         settings.setValue("/thisisatest", test);
         QCOMPARE(settings.value<QStringList>("/thisisatest"), test);
 
-        QVariant var(10);
-        settings.setValue("/thisisanothertest", var);
+        QJSValue varInt(10);
+        settings.setValue("/thisisanothertest", varInt);
         QCOMPARE(settings.value("/thisisanothertest").toInt(), 10);
 
-        QVariant v;
-        QVERIFY(!settings.setValue("/shouldnotwork", v));
+        QJSValue varString("test");
+        settings.setValue("/thisisastringtest", varString);
+        QCOMPARE(settings.value("/thisisastringtest").toString(), "test");
 
-        QRect r;
-        QVERIFY(!settings.setValue("/shouldnotwork", QVariant(r)));
+        QJSValue varDouble(3.5);
+        settings.setValue("/thisisadoubletest", varDouble);
+        QCOMPARE(settings.value("/thisisadoubletest").toDouble(), 3.5);
+
+        QJSValue varBool(true);
+        settings.setValue("/thisisabooltest", varBool);
+        QCOMPARE(settings.value("/thisisabooltest").toBool(), true);
 
         // There's only one signal, as the save is done asynchronously
         settingsSaved.wait();