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 663425 - schemas: Add (shared) WM schemas
schemas: Add (shared) WM schemas
Status: RESOLVED FIXED
Product: gsettings-desktop-schemas
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gsettings-desktop-schemas-maint
gsettings-desktop-schemas-maint
Depends on:
Blocks: 621204 635378
 
 
Reported: 2011-11-04 18:50 UTC by Florian Müllner
Modified: 2011-11-11 16:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
schemas: Add (shared) wm schemas (117.71 KB, patch)
2011-11-04 18:51 UTC, Florian Müllner
needs-work Details | Review
schemas: Add (shared) wm schemas (74.06 KB, patch)
2011-11-11 01:56 UTC, Florian Müllner
reviewed Details | Review
schemas: Add (shared) wm schemas (36.16 KB, patch)
2011-11-11 16:08 UTC, Florian Müllner
none Details | Review
schemas: Add (shared) wm schemas (36.12 KB, patch)
2011-11-11 16:15 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2011-11-04 18:50:31 UTC
See patch.
Comment 1 Florian Müllner 2011-11-04 18:51:11 UTC
Created attachment 200704 [details] [review]
schemas: Add (shared) wm schemas

Metacity, Mutter and GNOME Shell share most of their settings;
rather than having the latters depend on Metacity as it's currently
the case, move the schemas to a shared module.
Comment 2 Bastien Nocera 2011-11-04 22:15:10 UTC
Review of attachment 200704 [details] [review]:

The shortcuts list needs some serious trimming of things we don't want to use in gnome-shell.

Is there any reason why this isn't in a module shared between mutter and metacity?
Comment 3 Christian Persch 2011-11-04 22:21:39 UTC
+    <key name="command-1" type="s">

Shouldn't this be "ay" instead of "s" since commands are in filename encoding, not UTF-8 ? Also, wouldn't it be better to allow an argv array (ie use "aay") instead of having to put all in one command line?
Comment 4 Bastien Nocera 2011-11-04 22:29:46 UTC
(In reply to comment #3)
> +    <key name="command-1" type="s">
> 
> Shouldn't this be "ay" instead of "s"

Nope, because it shouldn't even be there, because it duplicates functionality in gnome-settings-daemon (which still uses GConf, FWIW).
Comment 5 Florian Müllner 2011-11-04 22:59:53 UTC
(In reply to comment #2)
> Review of attachment 200704 [details] [review]:
> 
> The shortcuts list needs some serious trimming of things we don't want to use
> in gnome-shell.

If we add 'screenshot' and 'window-screenshot' to org.gnome.desktop.default-applications, the keybinding-commands schema could be killed entirely (and all run-command-N shortcuts be removed).

I can do some investigation about switch-foo-backward/cycle-foo-backward (as it duplicates functionality already provided by hardcoding shift for reversal), though I suspect that it is useful for a11y.

Other than that, I updated the shortcut files for gnome-control-center to only include {switch,move}-to-workspace-N for N=[1,4], but given that the cost of additional hidden shortcuts is close to zero, I'd hesitate to remove them.

(There are two "keybindings" which are actually for internal use only, I will remove those from the schema).

Does that sound like a plan?


> Is there any reason why this isn't in a module shared between mutter and
> metacity?

None except for "we don't have such a module yet" and "there already is a module for schemas shared between modules" :)

(That said, I do have a 'gnome-wm-schemas' lying around, which I started before going the gsettings-desktop-schemas route)
Comment 6 Bastien Nocera 2011-11-07 14:19:01 UTC
(In reply to comment #5)
> (In reply to comment #2)
> > Review of attachment 200704 [details] [review] [details]:
> > 
> > The shortcuts list needs some serious trimming of things we don't want to use
> > in gnome-shell.
> 
> If we add 'screenshot' and 'window-screenshot' to
> org.gnome.desktop.default-applications, the keybinding-commands schema could be
> killed entirely (and all run-command-N shortcuts be removed).

Given that mutter just launches gnome-screenshot when you use those shortcuts, is there any reason why we can't remove them, and have mutter pass those shortcuts to gnome-settings-daemon in the overview instead?

> I can do some investigation about switch-foo-backward/cycle-foo-backward (as it
> duplicates functionality already provided by hardcoding shift for reversal),
> though I suspect that it is useful for a11y.

Ok.

> Other than that, I updated the shortcut files for gnome-control-center to only
> include {switch,move}-to-workspace-N for N=[1,4], but given that the cost of
> additional hidden shortcuts is close to zero, I'd hesitate to remove them.

I don't mind going all the way up to 10.

> (There are two "keybindings" which are actually for internal use only, I will
> remove those from the schema).

Cool.

> Does that sound like a plan?
> 
> 
> > Is there any reason why this isn't in a module shared between mutter and
> > metacity?
> 
> None except for "we don't have such a module yet" and "there already is a
> module for schemas shared between modules" :)
> 
> (That said, I do have a 'gnome-wm-schemas' lying around, which I started before
> going the gsettings-desktop-schemas route)

Could you create a separate convert script? Having the other files in this module doesn't bother me.
Comment 7 Florian Müllner 2011-11-11 01:56:09 UTC
Created attachment 201204 [details] [review]
schemas: Add (shared) wm schemas

(In reply to comment #6)
> (In reply to comment #5)
> Given that mutter just launches gnome-screenshot when you use those shortcuts,
> is there any reason why we can't remove them, and have mutter pass those
> shortcuts to gnome-settings-daemon in the overview instead?

Killed all custom bindings, including taking screenshots (now in g-s-d) and launching terminal (can be added as custom screenshot in g-c-c).


> > I can do some investigation about switch-foo-backward/cycle-foo-backward (as it
> > duplicates functionality already provided by hardcoding shift for reversal),
> > though I suspect that it is useful for a11y.
> 
> Ok.

I tracked the feature down to http://git.gnome.org/browse/metacity/commit?id=eb647577c3a23b6 - the reasoning kinda makes sense, so I left those bindings in for now.


> > (There are two "keybindings" which are actually for internal use only, I will
> > remove those from the schema).
> 
> Cool.

Done.


> Could you create a separate convert script? Having the other files in this
> module doesn't bother me.

Done as well.
Comment 8 Bastien Nocera 2011-11-11 12:16:17 UTC
Review of attachment 201204 [details] [review]:

Bump glib-2.0 reqs in configure to make the CDATA parsing work?

Please remove all the useless and repetitive descriptions of how the accelerators should be formatted.

::: schemas/org.gnome.desktop.wm.preferences.gschema.xml.in.in
@@ +4,3 @@
+          gettext-domain="@GETTEXT_PACKAGE@">
+    <key name="mouse-button-modifier" type="s">
+      <default>'&lt;Alt&gt;'</default>

CDATA?

@@ +25,3 @@
+    </key>
+    <key name="button-layout" type="s">
+      <default>':minimize,maximize,close'</default>

Is that overridden by gnome-shell?

@@ +158,3 @@
+        the window behind all the others, and 'none' which will not do anything.
+      </_description>
+      <!-- For compatibility with GConf strings (Metacity 2.30) -->

Isn't this automatic?

@@ +243,3 @@
+    <key name="visual-bell-type"
+         enum="org.gnome.desktop.GDesktopVisualBellType">
+      <default>'fullscreen-flash'</default>

Can't we have a single key merging "visual-bell" and "visual-bell-type"?

@@ +265,3 @@
+      <default>false</default>
+      <_summary>
+        (Not implemented) Navigation works in terms of applications not windows

If it's not implemented, do we really need to carry a setting for it?
Comment 9 Florian Müllner 2011-11-11 16:08:52 UTC
Created attachment 201238 [details] [review]
schemas: Add (shared) wm schemas

(In reply to comment #8)
> Bump glib-2.0 reqs in configure to make the CDATA parsing work?

OK.


> Please remove all the useless and repetitive descriptions of how the
> accelerators should be formatted.

Sure.
(I was going to add shorter descriptions, but as I was merely copying the summary, I removed them altogether)


> ::: schemas/org.gnome.desktop.wm.preferences.gschema.xml.in.in
> @@ +4,3 @@
> +          gettext-domain="@GETTEXT_PACKAGE@">
> +    <key name="mouse-button-modifier" type="s">
> +      <default>'&lt;Alt&gt;'</default>
> 
> CDATA?

Woops, yeah.



> @@ +25,3 @@
> +    </key>
> +    <key name="button-layout" type="s">
> +      <default>':minimize,maximize,close'</default>
> 
> Is that overridden by gnome-shell?

Yes.


> @@ +158,3 @@
> +        the window behind all the others, and 'none' which will not do
> anything.
> +      </_description>
> +      <!-- For compatibility with GConf strings (Metacity 2.30) -->
> 
> Isn't this automatic?

Mmh, I don't think so - I'll investigate. For now, I've left it in.


> @@ +243,3 @@
> +    <key name="visual-bell-type"
> +         enum="org.gnome.desktop.GDesktopVisualBellType">
> +      <default>'fullscreen-flash'</default>
> 
> Can't we have a single key merging "visual-bell" and "visual-bell-type"?

Right.


> @@ +265,3 @@
> +      <default>false</default>
> +      <_summary>
> +        (Not implemented) Navigation works in terms of applications not
> windows
> 
> If it's not implemented, do we really need to carry a setting for it?

I was going to remove it, but Jon objected (see bug 591639)
Comment 10 Florian Müllner 2011-11-11 16:15:56 UTC
Created attachment 201241 [details] [review]
schemas: Add (shared) wm schemas

Removed G_DESKTOP_VISUAL_BELL_INVALID
Comment 11 Florian Müllner 2011-11-11 16:57:35 UTC
Attachment 201241 [details] pushed as 99cbaf3 - schemas: Add (shared) wm schemas

OK'ed on IRC, pushing.