Created attachment 178332 [details] [review]
Teach meta_display_get_keybinding_action() about "Above_Tab" pseudo-keysym

Review of attachment 178332 [details] [review]:

I don't think quite works - if someone goes into the keyboard capplet and sets up a new keybinding, they'll get the "real" keysym (grave, whatever) rather than the ABOVE_TAB keysym attached to the binding, so I think get_keybinding_action() actually needs to check both ways.

I don't see how that can be done on get_keybinding_action().

Shouldn't that be considered a new bug on the keyboard capplet which could then put the "<Alt>Above_Tab" string into gconf using a copy of your from bug 635569?

For testing I've done

$ gconftool-2 -s /apps/metacity/global_keybindings/switch_group -t string "<Alt>Above_Tab"
$ gconftool-2 -s /apps/metacity/global_keybindings/switch_group_backward -t string "<Shift><Alt>Above_Tab"

and it actually works.

(In reply to comment #3)
> I don't see how that can be done on get_keybinding_action().

What's the problem? If the keysym is Above_Tab, check for a keybinding using Above_Tab, if that isn't found, check for a binding using XKeysymToKeyCode(). Might require a bit of code reorganization (maybe moving some code to a separate function.)

> Shouldn't that be considered a new bug on the keyboard capplet which could then
> put the "<Alt>Above_Tab" string into gconf using a copy of your from bug
> 635569?

Also have to consider older configurations - I'd rather we just made it work here.

Created attachment 178357 [details] [review]
Teach meta_display_get_keybinding_action() about "Above_Tab" pseudo-keysym

(In reply to comment #4)

You are right, of course, and I was being lazy :-)

It was actually easier to handle both cases than I thought after thinking a bit more about it.

Review of attachment 178357 [details] [review]:

OK to commit with a tiny tweak (just let me know if you don't have a GNOME git account and I'll commit it for you)

::: src/core/keybindings.c
@@ +536,3 @@
   binding = display_get_keybinding (display, keysym, keycode, mask);
 
+  if (! binding && keycode == meta_display_get_above_tab_keycode (display))

No space in '! binding'

Pushed with missing space fixed. Thanks!

Attachment 178357 [details] pushed as bbfc435 - Teach meta_display_get_keybinding_action() about "Above_Tab" pseudo-keysym