GNOME Bugzilla – Bug 615586
Allow redirecting preferences to a different GConf key
Last modified: 2010-04-13 17:56:05 UTC
In general, we want to share preferences with Metacity, but for GNOME Shell, in some cases we want to diverge and use different defaults. * We'd like people to be testing GNOME Shell with a GNOME Shell specific theme that integrates well with the black panel. * We want to eliminate the menu button by default * We want to disconnect the number of workspaces in GNOME Shell from the user's normal workspace count, since we handle workspace addition more dynamically. (bug 613127) This patch set adds meta_prefs_override_preference_location() which allows repointing specific preferences to a different GConf key. It doesn't currently handle key bindings; since key bindings are all exposed to the user in gnome-keybinding-properties, it probably would just be confusing to have redirections there. This patch set is somewhere between over-engineered and completely hacky.... I can't really think of a better way to do it. (I thought of having plugins statically declare what GConf keys they want to redirect in a structure in the plugin file, but that really doesn't fit in well with the current philosophy.) Eventually maybe we'll go to a libmutter and get rid of plugins, and this will all make much more sense.
Created attachment 158541 [details] [review] Add an explicit start() method for plugins Rather than using the plugin objects constructed() method for doing setup that requires the MetaScreen, add an explicit start() method that is called after the screen is set. The reason for this is that this allows plugin objects to be created early before the bulk of Metacity setup, which then allows plugins to affect how the setup happens. (For example, to change the way that preferences are loaded.) This is an incompatible change, since 'screen' is now not set in the constructed method, so the plugin API version is bumped.
Created attachment 158542 [details] [review] Add meta_prefs_override_preference_location() Allow a plugin to redirect preferences from one GConf location to another GConf location. This is useful for keys that need to be set differently in a plugin-managed environment (like GNOME Shell) as compared to in standalone Metacity. Overriding is implemented by overwriting the keys in the arrays of preferences; a list of the current overrides is stored to allow proper memory management when an override is itself overriden. (we need to know whether to free the old keys or not)
Created attachment 158543 [details] [review] Load one copy of plugins early Although multi-screen support has not been tested and probably doesn't fully work, the basic setup for multi-screen is that we have the same list of plugins for all screens, but a different instance of the plugins for each screen. To allow plugins to do setup that is screen independent and needs to occur early in the setup process, we identify a "default plugin manager" and load (but not start) that plugin manager's plugins immediately after we know our list of plugins. That plugin manager is then reused for the first screen we open and the plugins are started at that time. Separate plugin managers are loaded and started for any other screens we open. (A plugin could keep track of whether the screen-independent setup has been done in a static variable, or it could do everything in a way that is safe to do repeatedly.)
Bug 615592 has the trivial fix for gnome-shell to make it handle a separate start() method.
I am happy with doing it this way, thanks Owen.
Review of attachment 158542 [details] [review]: the code looks good. just minor comments ::: src/core/prefs.c @@ +258,3 @@ + * - these types all begin with a gchar* and a MetaPreference, + * so can be cast to MetaGenericPreference; we could factor out + * repated code in the handlers by taking advantage of this. typo "repated" the comment as a whole is sort of weird since it starts out talking specifically about MetaEnumPreference, and then (before actually getting to the definition of MetaEnumPreference) switches to talking generically about all preferences. also, "these types" doesn't include the new MetaPrefsOverriddenKey, though there's no division of any sort between the end of the Meta*Preference types and that. you could just remove the entire "Other things we could have done" section... @@ +1278,3 @@ + /* FIXME: Use MetaGenericPreference and save a bit of code duplication */ + + while (preference_update_handler[i]!=NULL) spacing around "!="
(In reply to comment #6) > Review of attachment 158542 [details] [review]: > > the code looks good. just minor comments > > ::: src/core/prefs.c > @@ +258,3 @@ > + * - these types all begin with a gchar* and a MetaPreference, > + * so can be cast to MetaGenericPreference; we could factor out > + * repated code in the handlers by taking advantage of this. > > typo "repated" > > the comment as a whole is sort of weird since it starts out talking > specifically about MetaEnumPreference, and then (before actually getting to the > definition of MetaEnumPreference) switches to talking generically about all > preferences. > Yeah, I've moved things about a bit now. > also, "these types" doesn't include the new MetaPrefsOverriddenKey, though > there's no division of any sort between the end of the Meta*Preference types > and that. I decided to move that type down with the GSList holding the structures to make clear that it was a different sort of thing. And added a comment explaining it. > you could just remove the entire "Other things we could have done" section... Actually, yeah, the ideas there weren't that compelling, or hard to figure out again, so I just removed most of the text. > @@ +1278,3 @@ > + /* FIXME: Use MetaGenericPreference and save a bit of code duplication */ > + > + while (preference_update_handler[i]!=NULL) > > spacing around "!=" Fixed.
Attachment 158541 [details] pushed as 97a9726 - Add an explicit start() method for plugins Attachment 158542 [details] pushed as 95b260f - Add meta_prefs_override_preference_location() Attachment 158543 [details] pushed as b77b0a3 - Load one copy of plugins early