[PATCH weston v2] clients: Add XKB compose key support

Daniel Stone daniel at fooishbar.org
Fri Oct 7 09:56:11 UTC 2016


Hi,

On 7 October 2016 at 05:24, Bryce Harrington <bryce at 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 <daniels at collabora.com>

Cheers,
Daniel


More information about the wayland-devel mailing list