GNOME Bugzilla – Bug 613127
[MetaScreen] Keep num_workspaces key in sync with the actual workspace number
Last modified: 2010-05-14 11:34:04 UTC
Created attachment 156342 [details] [review] patch to keep prefernces key in sync with actual workspace count Changing the number of workspaces via an external pager relies on the gconf key; if a plugin adds or removes workspaces on the fly, we can get into a situation when the stale number stored by the preferences matches the new number requested by the pager, in which case the pager request becomes a nop. This commit ensures that when the meta_screen_append_new_workspace() or meta_screen_remove_workspace() functions are called, the stored value is updated accordingly.
we'd find this slightly annoying since we treat the number of workspaces as being a transient/per-session thing. but we already have to have code to remove the excess workspaces at startup since the gconf default is "4" anyway, so...
The other option here is to decouple the handling of the _NET_NUMBER_OF_DESKTOPS property form the gconf value, and only use the latter to as the initial value. That would also fix the problem of not being able to create an additional workspace.
Review of attachment 156342 [details] [review]: Hmm, the problem with this versus gnome-shell is that if you start gnome-shell you are going to lose all your workspaces when you go back to normal GNOME. One thing I've considered - for themes, button positions, etc, is that we should have a generic way of overriding Metacity preferences from a plugin. So redirecting and saying "for num_workspaces, don't use /apps/metacity/general/num_workspaces, use /apps/gnome-shell/num_workspaces." I'm not sure how hard that's going to be to implement (here it has the problem too that we probably read the preferences before we initialize the plugins. After all, the plugins to use are one of the GConf keys, though that's a bit busted from the point of view of the way we conceive of plugins now.) I guess I should look into it some. Leaving that aside, this patch makes a lot of sense to me. It doesn't make sense to sometimes sync the number of workspace to GConf and sometimes not. Arguably it's a bit messy that: - When the number of workspaces changes via client message, we adjust the GConf key and then wait for the notification to adjust the workspace count - When the number of workspaces changes otherwise, we adjust the workspace count, adjust the GConf key, and count on short-circuiting We can't make both work like the client message (since we want to do things like add a workspace, then put a window on it immediately). We could switch the property-change code path, but likely not worth it. One minor style comment, otherwise looks good to me, with the big caveat about it messing things up for GNOME Shell testers. Having the GConf key only be set initially will break the idea that Mutter is a drop-in for Metacity; it means that Mutter no longer works with an external pager in the way that window managers are supposed to in GNOME 2. I'm not really that concerned about keeping the ability to use Mutter with GNOME 2, but it's sort of nice that it does work currently. ::: src/core/screen.c @@ +1268,3 @@ GList *next = NULL; int index; + gint new_num; Shouldn't mix int and gint in successive lines. (Metacity/Mutter is pretty consistent about not using gint and the like, there are some mistakes but it's only a small fraction. I certainly approve of the avoidance of gint - people should realize that glib and gtk+ only use gint because of historical reasons.) More examples of gint usage below.
GConf key redirection implemented in bug 615586
Created attachment 158579 [details] [review] updated patch for _MUTTER_HINTS Update patch based on the review attached.
Comment on attachment 158579 [details] [review] updated patch for _MUTTER_HINTS Sorry, that patch was meant for bug 613123.
Patch pushed as fc9488211f689e7f8802345d753d70e5f5cd4025.