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 673014 - keybindings: Use a GSettings object rather than a schema, to support extensions
keybindings: Use a GSettings object rather than a schema, to support extensions
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 673032 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-03-28 18:23 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2012-04-17 16:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
keybindings: Use a GSettings object rather than a schema, to support extensions (38.28 KB, patch)
2012-03-28 18:23 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
keybindings: Use a GSettings object rather than a schema, to support extensions (38.28 KB, patch)
2012-04-12 00:41 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
windowManager: Adapt to mutter API change (1.08 KB, patch)
2012-04-16 22:29 UTC, Florian Müllner
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2012-03-28 18:23:05 UTC
Extensions need to be able to supply a GSettings object so they can pass something
which has a GSchemaSettingsSource.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-03-28 18:23:07 UTC
Created attachment 210799 [details] [review]
keybindings: Use a GSettings object rather than a schema, to support extensions

If we want to support keybindings from extensions installed in the user's
directory, we can't take a schema, as the GSettings object needs to have
a special GSettingsSchemaSource.
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-03-29 00:00:21 UTC
*** Bug 673032 has been marked as a duplicate of this bug. ***
Comment 3 Tim Cuthbertson 2012-04-06 09:33:38 UTC
I've tested this patch against the latest mutter (3.4.0 in fedora 17 alpha) and my shellshape extension, which uses custom keybindings. Happy to report that it works as advertised, so I'd love to see this make it into mutter ASAP.
Comment 4 Giovanni Campagna 2012-04-11 17:38:10 UTC
Review of attachment 210799 [details] [review]:

::: src/core/prefs.c
@@ +1952,3 @@
+  if (pref->builtin)
+    {
+      if (g_object_get_data (settings, "changed-signal") != NULL)

== NULL ?
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-04-12 00:41:01 UTC
Created attachment 211887 [details] [review]
keybindings: Use a GSettings object rather than a schema, to support extensions

If we want to support keybindings from extensions installed in the user's
directory, we can't take a schema, as the GSettings object needs to have
a special GSettingsSchemaSource.



Nice catch!
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-04-16 19:02:04 UTC
OK, I tested with Shellshape to make sure that this patch works properly. It does. Shellshape has other issues, but they're not related to keybindings at all.
Comment 7 Florian Müllner 2012-04-16 22:29:33 UTC
Created attachment 212173 [details] [review]
windowManager: Adapt to mutter API change

Trivial (shell) change, but required by the previous patch
Comment 8 Florian Müllner 2012-04-16 22:29:43 UTC
Review of attachment 211887 [details] [review]:

Some minor style issues.

::: src/core/keybindings-private.h
@@ +75,2 @@
 gboolean meta_prefs_add_keybinding          (const char           *name,
+                                             GSettings            *settings,

I'd include gio/gio.h explicitly rather than relying on it being included from somewhere else before including keybindings-private.h

::: src/core/prefs.c
@@ +1962,3 @@
     {
+      char *changed_signal;
+      changed_signal = g_strdup_printf ("changed::%s", name);

Breaking up that line looks pointless
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-04-17 00:34:25 UTC
Comment on attachment 211887 [details] [review]
keybindings: Use a GSettings object rather than a schema, to support extensions

Attachment 211887 [details] pushed as 68321d9 - keybindings: Use a GSettings object rather than a schema, to support extensions


Pushed with suggested style fixes.
Comment 10 Jasper St. Pierre (not reading bugmail) 2012-04-17 00:34:45 UTC
Attachment 212173 [details] pushed as 7313172 - windowManager: Adapt to mutter API change


(Had a similar patch locally)