Nicer serialization of configuration options

Summary:
The previous approach of having an array of object that
hold information about the current config group and the config keys
and values was not really scalable nor intuitive.

Now, the config is an object that has the group names as keys,
and each group has its own configuration (key, value) pairs.

The config groups are identified as a unix-like path

Reviewers: #plasma, mart

Reviewed By: mart

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D2516

shell/scripting/scriptengine_v1.cpp

@@ -86,9 +86,35 @@ namespace {
         const QScriptValue &array;
     };
 
-    #define SCRIPT_FOREACH(Variable, Array) \
+    #define SCRIPT_ARRAY_FOREACH(Variable, Array) \
         ScriptArray_forEach_Helper(Array) + [&] (const QScriptValue &Variable)
 
+    class ScriptObject_forEach_Helper {
+    public:
+        ScriptObject_forEach_Helper(const QScriptValue &object)
+            : object(object)
+        {
+        }
+
+        // operator + is commonly used for these things
+        // to avoid having the lambda inside the parenthesis
+        template <typename Function>
+        void operator+ (Function function) const
+        {
+            QScriptValueIterator it(object);
+            while (it.hasNext()) {
+                it.next();
+                function(it.name(), it.value());
+            }
+        }
+
+    private:
+        const QScriptValue &object;
+    };
+
+    #define SCRIPT_OBJECT_FOREACH(Key, Value, Array) \
+        ScriptObject_forEach_Helper(Array) + [&] (const QString &Key, const QScriptValue &Value)
+
     // Case insensitive comparison of two strings
     template <typename StringType>
     inline bool matches(const QString &object, const StringType &string)
@@ -264,27 +290,19 @@ QScriptValue ScriptEngine::V1::activities(QScriptContext *context, QScriptEngine
 template <typename Object>
 void loadSerializedConfigs(Object *object, const QScriptValue &configs)
 {
-    SCRIPT_FOREACH(config, configs) {
+    SCRIPT_OBJECT_FOREACH(escapedGroup, config, configs) {
         // If the config group is set, pass it on to the containment
-        auto currentConfigGroup = config.property("currentConfigGroup");
-        if (currentConfigGroup.isArray()) {
-            QStringList groups;
-            SCRIPT_FOREACH(group, currentConfigGroup) {
-                groups << group.toString();
-            };
-            object->setCurrentConfigGroup(groups);
-            // qDebug() << "DESERIALIZATION: currentConfigGroup = " << groups;
+        QStringList groups = escapedGroup.split('/', QString::SkipEmptyParts);
+        for (QString &group: groups) {
+            group = QUrl::fromPercentEncoding(group.toUtf8());
         }
+        qDebug() << "Config group" << groups;
+        object->setCurrentConfigGroup(groups);
 
         // Read other properties and set the configuration
-        QScriptValueIterator it(config);
-        while (it.hasNext()) {
-            it.next();
-            if (it.name() == "currentConfigGroup") continue;
-
-            object->writeConfig(it.name(), it.value().toVariant());
-            // qDebug() << "DESERIALIZATION: writeConfig(...) " << it.name() << it.value().toVariant();
-        }
+        SCRIPT_OBJECT_FOREACH(key, value, config) {
+            object->writeConfig(key, value.toVariant());
+        };
     };
 }
 
@@ -310,7 +328,7 @@ QScriptValue ScriptEngine::V1::loadSerializedLayout(QScriptContext *context, QSc
     // qDebug() << "DESKTOP DESERIALIZATION: Loading desktops...";
 
     int count = 0;
-    SCRIPT_FOREACH(desktopData, data.property("desktops")) {
+    SCRIPT_ARRAY_FOREACH(desktopData, data.property("desktops")) {
         // If the template has more desktops than we do, ignore them
         if (count >= desktops.size()) return;
 
@@ -325,7 +343,7 @@ QScriptValue ScriptEngine::V1::loadSerializedLayout(QScriptContext *context, QSc
         loadSerializedConfigs(desktop, desktopData.property("config"));
 
         // After the config, we want to load the applets
-        SCRIPT_FOREACH(appletData, desktopData.property("applets")) {
+        SCRIPT_ARRAY_FOREACH(appletData, desktopData.property("applets")) {
             // qDebug() << "DESKTOP DESERIALIZATION: Applet: " << appletData.toString();
 
             // TODO: It would be nicer to be able to call addWidget directly
@@ -352,7 +370,7 @@ QScriptValue ScriptEngine::V1::loadSerializedLayout(QScriptContext *context, QSc
 
     // qDebug() << "PANEL DESERIALIZATION: Loading panels...";
 
-    SCRIPT_FOREACH(panelData, data.property("panels")) {
+    SCRIPT_ARRAY_FOREACH(panelData, data.property("panels")) {
         const auto panel = qobject_cast<Panel *>(env->createContainment(
             QStringLiteral("Panel"), QStringLiteral("org.kde.panel")));
 
@@ -366,7 +384,7 @@ QScriptValue ScriptEngine::V1::loadSerializedLayout(QScriptContext *context, QSc
         loadSerializedConfigs(panel, panelData.property("config"));
 
         // Now dealing with the applets
-        SCRIPT_FOREACH(appletData, panelData.property("applets")) {
+        SCRIPT_ARRAY_FOREACH(appletData, panelData.property("applets")) {
             // qDebug() << "PANEL DESERIALIZATION: Applet: " << appletData.toString();
 
             // TODO: It would be nicer to be able to call addWidget directly

shell/shellcorona.cpp

@@ -29,6 +29,7 @@
 #include <QMenu>
 #include <QQmlContext>
 #include <QDBusConnection>
+#include <QUrl>
 
 #include <QJsonObject>
 #include <QJsonDocument>
@@ -316,11 +317,12 @@ void ShellCorona::setShell(const QString &shell)
 }
 
 
-QJsonArray dumpconfigGroupJS(const KConfigGroup &rootGroup)
+QJsonObject dumpconfigGroupJS(const KConfigGroup &rootGroup)
 {
-    QJsonArray result;
+    QJsonObject result;
 
     QStringList hierarchy;
+    QStringList escapedHierarchy;
     QList<KConfigGroup> groups{rootGroup};
     QSet<QString> visitedNodes;
 
@@ -333,6 +335,10 @@ QJsonArray dumpconfigGroupJS(const KConfigGroup &rootGroup)
             QStringLiteral("plugin")
         };
 
+    auto groupID = [&escapedHierarchy]() {
+        return '/' + escapedHierarchy.join('/');
+    };
+
     // Perform a depth-first tree traversal for config groups
     while (!groups.isEmpty()) {
         KConfigGroup cg = groups.last();
@@ -341,27 +347,21 @@ QJsonArray dumpconfigGroupJS(const KConfigGroup &rootGroup)
         //FIXME: name is not enough
 
         hierarchy.clear();
+        escapedHierarchy.clear();
         while (parentCg.isValid() && parentCg.name() != rootGroup.name()) {
-            hierarchy.prepend(parentCg.name());
+            const auto name = parentCg.name();
+            hierarchy.prepend(name);
+            escapedHierarchy.prepend(QString::fromUtf8(QUrl::toPercentEncoding(name.toUtf8())));
             parentCg = parentCg.parent();
         }
 
-        visitedNodes.insert(hierarchy.join(QChar()));
+        visitedNodes.insert(groupID());
         groups.pop_back();
 
         QJsonObject configGroupJson;
 
         if (!cg.keyList().isEmpty()) {
             //TODO: this is conditional if applet or containment
-            if (hierarchy.length() > 0) {
-                QJsonArray currentConfigGroup;
-
-                foreach (const QString &item, hierarchy) {
-                    currentConfigGroup << item;
-                }
-
-                configGroupJson.insert("currentConfigGroup", currentConfigGroup);
-            }
 
             const auto map = cg.entryMap();
             auto i = map.cbegin();
@@ -375,14 +375,14 @@ QJsonArray dumpconfigGroupJS(const KConfigGroup &rootGroup)
 
         foreach (const QString &groupName, cg.groupList()) {
             if (groupName == QStringLiteral("Applets") ||
-                visitedNodes.contains(hierarchy.join(QChar()) + groupName)) {
+                visitedNodes.contains(groupID() + '/' + groupName)) {
                 continue;
             }
             groups << KConfigGroup(&cg, groupName);
         }
 
         if (!configGroupJson.isEmpty()) {
-            result << configGroupJson;
+            result.insert(groupID(), configGroupJson);
         }
     }