Hi, On 7 October 2016 at 05:24, Bryce Harrington <br...@osg.samsung.com> wrote: > @@ -3016,8 +3024,38 @@ keyboard_handle_keymap(void *data, struct wl_keyboard > *keyboard, > return; > } > > + /* Look up the appropriate locale, or use "C" as default */ > + locale = getenv("LC_ALL"); > + if (!locale) > + locale = "C"; > + > + /* Set up XKB compose table */ > + compose_table = > xkb_compose_table_new_from_locale(input->display->xkb_context, > + locale, > + > XKB_COMPOSE_COMPILE_NO_FLAGS);
This line is quite long. You can get it to exactly 80 characters like so (this style is quite common within Weston): compose_table = xkb_compose_table_new_from_locale(input->display->xkb_context, locale, XKB_COMPOSE_COMPILE_NO_FLAGS); > +/* Translate symbols appropriately if a compose sequence is being entered */ > +static xkb_keysym_t > +process_key_press(xkb_keysym_t sym, struct input *input) > +{ > + if (sym == XKB_KEY_NoSymbol) > + return sym; > + if (xkb_compose_state_feed(input->xkb.compose_state, sym) != > XKB_COMPOSE_FEED_ACCEPTED) > + return sym; Same here. > + switch (xkb_compose_state_get_status(input->xkb.compose_state)) { > + case XKB_COMPOSE_COMPOSING: > + return XKB_KEY_NoSymbol; > + case XKB_COMPOSE_COMPOSED: > + return > xkb_compose_state_get_one_sym(input->xkb.compose_state); > + case XKB_COMPOSE_CANCELLED: > + return XKB_KEY_NoSymbol; > + case XKB_COMPOSE_NOTHING: > + return sym; > + } > + > + return sym; This line is unreachable, and could also be handled with a 'default' case. The rest looks good to me though, and I like the split function, so with those fixed: Reviewed-by: Daniel Stone <dani...@collabora.com> Cheers, Daniel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel