Created attachment 190907 [details] [review]
keyboard: Fix should_show_key() for GSettings

KeyListEntries may specify a condition which determines whether the
binding should be displayed. Currently this is only supported for
entries specified in GConf, so add support for GSettings entries.

Created attachment 190908 [details] [review]
keyboard: Improve support for conditions in GSettings entries

For GConf entries, the "key" attribute specifies the full location
of the setting, however for GSettings, the location is determined
by the combination of key name and schema. To support conditions
which use a comparison key from a different schema than the setting
in question, add support for an optional "key-schema" attribute.

Oh, one thing that crossed my mind while working on this - a quick grep suggests that conditions are only used by metacity/mutter keybindings, and only to show/hide bindings depending on the number of workspaces. It's obviously crazy to show a huge list of actions á la "Switch to workspace 42", but I still wonder whether those conditions still make sense with the number of workspaces changing dynamically in GNOME Shell ...

(In reply to comment #3)
> Oh, one thing that crossed my mind while working on this - a quick grep
> suggests that conditions are only used by metacity/mutter keybindings, and only
> to show/hide bindings depending on the number of workspaces. It's obviously
> crazy to show a huge list of actions á la "Switch to workspace 42", but I still
> wonder whether those conditions still make sense with the number of workspaces
> changing dynamically in GNOME Shell ...

They only really make sense for metacity. They might make some sense for mutter/gnome-shell, but only up to a certain amount of workspaces. Alt+1, Alt+2, fine, but what after Alt+9 (or Alt+0)?

Review of attachment 190907 [details] [review]:

::: panels/keyboard/keyboard-shortcuts.c
@@ -167,3 @@
-  /* FIXME: We'll need to change that when metacity/mutter
-   * uses GSettings */
-  g_assert (entry->type == CC_KEYBOARD_ITEM_TYPE_GCONF);

What about the GCONF_DIR type?

@@ +177,3 @@
+
+      settings = g_settings_new (entry->schema);
+  else

I'm really not certain that will work as you expect when the actual key is in a different location to the key representing the number of workspaces.

As you mentioned earlier, I would really rather we nuked this horror. Under gnome-shell or metacity, always show shortcuts for the first 10 workspaces, and be done with that.

Created attachment 191007 [details] [review]
keyboard: Fix should_show_key() for GSettings

(In reply to comment #5)
> What about the GCONF_DIR type?

Ooops. Fixed.


> I'm really not certain that will work as you expect when the actual key is in a
> different location to the key representing the number of workspaces.

It doesn't, that's why there's a second patch ...

We won't be using this, as we'll be removing conditions altogether.

The only thing it was used for was showing workspace-number-dependent keyboard shortcuts. With gnome-shell's dynamic workspaces, it makes little sense.

We'll remove the supporting code from the control-center when metacity's keybindings have been updated.