GNOME Bugzilla – Bug 679075
keyboard: Apply XKB options
Last modified: 2012-08-20 13:12:04 UTC
Patch attached.
Created attachment 217547 [details] [review] keyboard: Apply XKB options The new 'xkb-options' key contains a list of XKB options to apply together with XKB layouts and variants.
Review of attachment 217547 [details] [review]: ::: plugins/keyboard/gsd-keyboard-manager.c @@ +379,3 @@ + gchar *string; + + opt_string = g_strjoinv (",", options); Won't that append a comma if there's only one option? I would also rather you reimplemented g_strjoinv() to use malloc() directly (look at the implementation of g_strjoinv, there's plenty of shortcuts you can take knowing the length of the separator for example). If you're not willing to do that now, please add a FIXME comment. @@ +392,3 @@ + XkbRF_VarDefsRec *xkb_var_defs) +{ + GsdKeyboardManagerPrivate *priv = manager->priv; You're using this only once, just use manager->priv directly in the function call.
(In reply to comment #2) > Review of attachment 217547 [details] [review]: > > ::: plugins/keyboard/gsd-keyboard-manager.c > @@ +379,3 @@ > + gchar *string; > + > + opt_string = g_strjoinv (",", options); > > Won't that append a comma if there's only one option? No, it only inserts the separator between the strings.
Ping? Rui? I'd like to add some of the esoteric options to gnome-tweak-tool (x11 zap, caps lock modifier, etc) How far away is this from being ready?
(In reply to comment #4) > Ping? Rui? Hey, sorry for the delay. This patch here isn't going to change much besides addressing the review points. Please see bug 682004 for API to get at the XKB options. The gsettings schema patch is in bug 679074. Your input there would be great, thanks.
Created attachment 221538 [details] [review] keyboard: Apply XKB options -- Comments addressed.
Review of attachment 221538 [details] [review]: Looks good.
Attachment 221538 [details] pushed as 8709d69 - keyboard: Apply XKB options