Hi, > > Hey, > > I wonder if we always want to do this kind of synchronization > automatically, or if it should be optional? This would mean a new > property on SpiceGtkSession which would be checked before sending the > layout. I agree with you, it would be better to have it optional. > > On Wed, Nov 05, 2014 at 03:20:33PM +0100, Pavel Grunt wrote: > > The keyboard description of the client will be sent to the guest > > when the connection is established. > > > > RFE: https://bugs.freedesktop.org/show_bug.cgi?id=85332 > > --- > > v3: > > - wrong g_return_val_if_fail replaced by g_warn_if_fail > > v2: > > - reduced indentation level > > - removed checking all the channels > > --- > > gtk/spice-gtk-session-priv.h | 1 + > > gtk/spice-gtk-session.c | 63 > > ++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 64 insertions(+) > > > > diff --git a/gtk/spice-gtk-session-priv.h b/gtk/spice-gtk-session-priv.h > > index 91304b2..571f91f 100644 > > --- a/gtk/spice-gtk-session-priv.h > > +++ b/gtk/spice-gtk-session-priv.h > > @@ -28,6 +28,7 @@ gboolean spice_gtk_session_get_read_only(SpiceGtkSession > > *self); > > void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self); > > void spice_gtk_session_set_pointer_grabbed(SpiceGtkSession *self, gboolean > > grabbed); > > gboolean spice_gtk_session_get_pointer_grabbed(SpiceGtkSession *self); > > +void spice_gtk_session_sync_keyboard_layout(SpiceGtkSession *self); > > > > G_END_DECLS > > > > diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c > > index 52ad597..02de129 100644 > > --- a/gtk/spice-gtk-session.c > > +++ b/gtk/spice-gtk-session.c > > @@ -178,6 +178,13 @@ static void > > spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio > > } > > } > > > > +static void keymap_layout_changed(GdkKeymap *keymap, gpointer data) > > +{ > > + SpiceGtkSession *self = data; > > + > > + spice_gtk_session_sync_keyboard_layout(self); > > +} > > + > > static void keymap_modifiers_changed(GdkKeymap *keymap, gpointer data) > > { > > SpiceGtkSession *self = data; > > @@ -207,6 +214,8 @@ static void spice_gtk_session_init(SpiceGtkSession > > *self) > > G_CALLBACK(clipboard_owner_change), self); > > spice_g_signal_connect_object(keymap, "state-changed", > > G_CALLBACK(keymap_modifiers_changed), > > self, 0); > > + spice_g_signal_connect_object(keymap, "keys-changed", > > + G_CALLBACK(keymap_layout_changed), self, > > 0); > > } > > > > static GObject * > > @@ -1001,6 +1010,15 @@ static void channel_new(SpiceSession *session, > > SpiceChannel *channel, > > G_CALLBACK(clipboard_request), self); > > g_signal_connect(channel, "main-clipboard-selection-release", > > G_CALLBACK(clipboard_release), self); > > + gboolean agent_connected; > > I don't know if we have rules regarding declaring variables in the > middle of a block, this is some C99 thing which some module prefer to > avoid in case they are built with a C89 compiler. > > > + g_object_get(channel, "agent-connected", &agent_connected, NULL); > > + if (agent_connected) { > > + spice_gtk_session_sync_keyboard_layout(self); > > + } else { > > + g_signal_connect_swapped(channel, "notify::agent-connected", > > + > > G_CALLBACK(spice_gtk_session_sync_keyboard_layout), > > + self); > > + } > > } > > if (SPICE_IS_INPUTS_CHANNEL(channel)) { > > spice_g_signal_connect_object(channel, "inputs-modifiers", > > @@ -1195,3 +1213,48 @@ gboolean > > spice_gtk_session_get_pointer_grabbed(SpiceGtkSession *self) > > > > return self->priv->pointer_grabbed; > > } > > + > > +static gchar *get_keyboard_description(void) > > +{ > > + gchar *keyboard_description = NULL; > > +#ifdef HAVE_X11_XKBLIB_H > > + Display *dpy; > > + XkbDescPtr xkb; > > + int major = XkbMajorVersion, minor = XkbMinorVersion; > > + > > + dpy = XkbOpenDisplay(NULL, NULL, NULL, &major, &minor, NULL); > > + g_return_val_if_fail(dpy != NULL, NULL); > > + > > + xkb = XkbGetKeyboard(dpy, XkbAllComponentsMask, XkbUseCoreKbd); > > + g_warn_if_fail(xkb != NULL); > > + if (xkb != NULL) { > > + keyboard_description = g_strdelimit( > > + g_strdup_printf("%s", XGetAtomName(dpy, xkb->names->symbols)), > > "_",'+'); > > It seems you need to call XFree on the value returned by XGetAtomName. > This has the added advantage of splitting this very long line with > multiple function calls ;) > > > + XkbFreeKeyboard(xkb, XkbAllComponentsMask, True); > > + } > > + XCloseDisplay(dpy); > > +#elif defined(G_OS_WIN32) > > + gchar *locale, *region; > > + locale = g_win32_getlocale(); > > I'd feel more comfortable with a g_return_val_if_fail (g_strrstr(locale, > "_"), NULL); > here. > > > + region = g_ascii_strdown(g_strrstr(locale, "_")+1, -1); > > + g_free(locale); > > + keyboard_description = g_strdup_printf("pc+%s", region); > > + g_free(region); > > +#endif > > Maybe add > #else > g_debug("get_keyboard_description unimplemented on this platform"); > #endif > so that we know about it? (or g_warning for something more invasive) > I would prefer g_warning (user has to enable this behavior, so he should be warned that it is not working). > Looks all good otherwise! > > Christophe > Thank you for the review, I will send a new version. Pavel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel