Merge lp:~larsu/gsettings-qt/keep-propertymap-synced into lp:gsettings-qt

[Lars Karlitski]

Don't write values into the QQmlPropertyMap when the write failed

This makes sure that the property map doesn't get out of sync with the gsettings database.

Don't write values into the QQmlPropertyMap when the write failed

This makes sure that the property map doesn't get out of sync with the gsettings database.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:41
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~larsu/gsettings-qt/keep-propertymap-synced/+merge/183191/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/gsettings-qt-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gsettings-qt-saucy-amd64-ci/16

Click here to trigger a rebuild:
http://s-jenkins:8080/job/gsettings-qt-ci/16/rebuild

review: Needs Fixing (continuous-integration)

[Sebastien Bacher (seb128)]

looks fine to me, I'm not 100% sure it makes sense to have 2 different apis for a such small difference, but at the same time I don't like changing an api already in use so I guess that makes sense...

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'GSettings/gsettings-qml.cpp'
--- GSettings/gsettings-qml.cpp	2013-08-28 17:29:34 +0000
+++ GSettings/gsettings-qml.cpp	2013-08-30 14:26:58 +0000
@@ -134,8 +134,14 @@
 
 QVariant GSettingsQml::updateValue(const QString& key, const QVariant &value)
 {
-    if (priv->settings)
-        priv->settings->set(key, value);
+    if (priv->settings == NULL)
+        return QVariant();
 
-    return value;
+    if (priv->settings->trySet(key, value)) {
+        return value;
+    }
+    else {
+        qWarning("unable to set key '%s' to value '%s'", key.toUtf8().constData(), value.toString().toUtf8().constData());
+        return priv->settings->get(key);
+    }
 }

=== modified file 'src/qgsettings.cpp'
--- src/qgsettings.cpp	2013-08-28 17:29:34 +0000
+++ src/qgsettings.cpp	2013-08-30 14:26:58 +0000
@@ -71,17 +71,26 @@
 
 void QGSettings::set(const QString &key, const QVariant &value)
 {
+    if (!this->trySet(key, value))
+        qWarning("unable to set key '%s' to value '%s'", key.toUtf8().constData(), value.toString().toUtf8().constData());
+}
+
+bool QGSettings::trySet(const QString &key, const QVariant &value)
+{
     gchar *gkey = unqtify_name(key);
+    bool success = false;
 
     /* fetch current value to find out the exact type */
     GVariant *cur = g_settings_get_value(priv->settings, gkey);
 
     GVariant *new_value = qconf_types_collect_from_variant(g_variant_get_type (cur), value);
     if (new_value)
-        g_settings_set_value(priv->settings, gkey, new_value);
+        success = g_settings_set_value(priv->settings, gkey, new_value);
 
     g_free(gkey);
     g_variant_unref (cur);
+
+    return success;
 }
 
 QStringList QGSettings::keys() const

=== modified file 'src/qgsettings.h'
--- src/qgsettings.h	2013-08-28 17:29:34 +0000
+++ src/qgsettings.h	2013-08-30 14:26:58 +0000
@@ -67,6 +67,18 @@
     void set(const QString &key, const QVariant &value);
 
     /**
+     * @brief Sets the value at key to value
+     * @key The key for which to set the value
+     * @value The value to set
+     *
+     * Behaves just like ::set(key, value), but returns false instead of
+     * printing a warning if the key couldn't be set.
+     *
+     * @return whether the key was set
+     */
+    bool trySet(const QString &key, const QVariant &value);
+
+    /**
      * \brief Retrieves the list of avaliable keys
      */
     QStringList keys() const;

=== modified file 'tests/tst_GSettings.qml'
--- tests/tst_GSettings.qml	2013-08-29 11:32:58 +0000
+++ tests/tst_GSettings.qml	2013-08-30 14:26:58 +0000
@@ -45,6 +45,13 @@
     compare(settings.testStringList, ['six']);
     settings.testStringList = [];
     compare(settings.testStringList, []);
+
+    settings.testEnum = 'two';
+    compare(settings.testEnum, 'two');
+
+    // test whether writing an out-of-range key doesn't work
+    settings.testEnum = 'notanumber';
+    compare(settings.testEnum, 'two');
   }
 
   function test_changed() {