After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 643767 - goffice should support using gsettings.
goffice should support using gsettings.
Status: RESOLVED FIXED
Product: libgoffice
Classification: Other
Component: General
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Jody Goldberg
Jody Goldberg
Depends on:
Blocks: 622558
 
 
Reported: 2011-03-03 10:37 UTC by Jean Bréfort
Modified: 2012-03-09 15:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch. (17.66 KB, patch)
2011-03-03 10:37 UTC, Jean Bréfort
none Details | Review
Fixed patch (3.17 KB, patch)
2011-03-03 17:18 UTC, Jean Bréfort
committed Details | Review

Description Jean Bréfort 2011-03-03 10:37:27 UTC
Created attachment 182330 [details] [review]
Proposed patch.

The summary says it all
Comment 1 Morten Welinder 2011-03-03 14:12:28 UTC
Looks good to me.  Please commit.
Comment 2 Jean Bréfort 2011-03-03 14:25:36 UTC
We don't wait until we branch? Looks a bit adventurous to me. I know we did even more dangerous things in the past,but ...
Comment 3 Morten Welinder 2011-03-03 16:00:51 UTC
Unless I am mistaken, none of the code will be compiled by default.

Maybe toss an "EXPERIMENTAL" note into conf_msg.
Comment 4 Jean Bréfort 2011-03-03 16:09:10 UTC
It is the default if gconf is not there. I might change that too.
Comment 5 Christian Persch 2011-03-03 16:58:27 UTC
+			node->path = g_strconcat ("/apps/", key, NULL);
+			node->id = g_strconcat ("org.gnome.", formatted, NULL);

It's a gsettings recommendation to use /org/gnome/foo/bar style paths instead of /apps/foo/bar.

+	if (g_variant_is_of_type (value, G_VARIANT_TYPE_STRING))
+		value_string = g_strdup (g_variant_get_string (value, NULL));
+	else if (g_variant_is_of_type (value, G_VARIANT_TYPE_INT32))
+		value_string = g_strdup_printf ("%i", g_variant_get_int32 (value));
+	else if (g_variant_is_of_type (value, G_VARIANT_TYPE_INT32))
+		value_string = g_strdup_printf ("%f", g_variant_get_double (value));
+	else if (g_variant_is_of_type (value, G_VARIANT_TYPE_INT32))
+		value_string = g_strdup (go_locale_boolean_name (g_variant_get_boolean (value)));
+	else
+		value_string = g_strdup ("ERROR FIXME");

You could switch on g_variant_classify() instead of this if..else..else cascade.
Comment 6 Jean Bréfort 2011-03-03 17:15:13 UTC
(In reply to comment #5)
> +            node->path = g_strconcat ("/apps/", key, NULL);
> +            node->id = g_strconcat ("org.gnome.", formatted, NULL);
> 
> It's a gsettings recommendation to use /org/gnome/foo/bar style paths instead
> of /apps/foo/bar.

The path is NOT for gsettings. It's there to be compatible with the other backends.

> You could switch on g_variant_classify() instead of this if..else..else
> cascade.

Thanks for this information. Also INT32 was tested three times. Oops.
Comment 7 Jean Bréfort 2011-03-03 17:18:44 UTC
Created attachment 182369 [details] [review]
Fixed patch

With this patch the GSettings backend will be enabled only if explicitly requested. Will commit that shortly along with the gschema files for gnumeric.
Comment 8 Jean Bréfort 2011-03-03 17:44:19 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.
Comment 9 Andreas J. Guelzow 2011-03-03 17:45:05 UTC
Why are we using "org.gnome."? Should we be using "org.gnumeric."?