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 679075 - keyboard: Apply XKB options
keyboard: Apply XKB options
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on: 679074
Blocks: 678171
 
 
Reported: 2012-06-28 16:29 UTC by Rui Matos
Modified: 2012-08-20 13:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
keyboard: Apply XKB options (2.57 KB, patch)
2012-06-28 16:29 UTC, Rui Matos
reviewed Details | Review
keyboard: Apply XKB options (3.22 KB, patch)
2012-08-17 01:13 UTC, Rui Matos
committed Details | Review

Description Rui Matos 2012-06-28 16:29:49 UTC
Patch attached.
Comment 1 Rui Matos 2012-06-28 16:29:51 UTC
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.
Comment 2 Bastien Nocera 2012-06-29 13:33:02 UTC
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.
Comment 3 Matthias Clasen 2012-07-03 19:37:16 UTC
(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.
Comment 4 John Stowers 2012-08-15 11:35:05 UTC
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?
Comment 5 Rui Matos 2012-08-16 13:57:11 UTC
(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.
Comment 6 Rui Matos 2012-08-17 01:13:16 UTC
Created attachment 221538 [details] [review]
keyboard: Apply XKB options

--
Comments addressed.
Comment 7 Bastien Nocera 2012-08-17 14:00:31 UTC
Review of attachment 221538 [details] [review]:

Looks good.
Comment 8 Rui Matos 2012-08-20 13:12:00 UTC
Attachment 221538 [details] pushed as 8709d69 - keyboard: Apply XKB options