[PATCH weston] clients: Add XKB compose key support

[Bryce Harrington]

On Thu, Oct 06, 2016 at 01:06:42PM +0100, Daniel Stone wrote:
> Hi,
> 
> On 6 October 2016 at 01:18, Bryce Harrington <bryce at osg.samsung.com> wrote:
> > This adds single-symbol compose support using libxkbcommon's compose
> > functionality.  E.g., assuming you have the right alt key defined as
> > your compose key, typing <RAlt>+i+' will produce í, and <RAlt>+y+= will
> > produce ¥.  This makes compose key work for weston-editor,
> > weston-terminal, weston-eventdemo, and any other clients that use
> > Weston's window.* routines for accepting and managing keyboard input.
> >
> > Compose sequences are loaded from the system's standard tables.  As
> > well, libxkbcommon will transparently load custom sequences from the
> > user's ~/.XCompose file.
> >
> > Note that due to limitations in Weston's key handler interface, only
> 
> *toytoolkit's
> 
> > @@ -3101,6 +3139,25 @@ keyboard_handle_key(void *data, struct wl_keyboard *keyboard,
> >                    state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> >                 window_close(window);
> >         } else if (window->key_handler) {
> > +               if ((sym != XKB_KEY_NoSymbol) &&
> > +                   (state == WL_KEYBOARD_KEY_STATE_PRESSED) &&
> > +                   (xkb_compose_state_feed(input->xkb.compose_state, sym) == XKB_COMPOSE_FEED_ACCEPTED)) {
> 
> Hm, this is kind of a hairy conditional.

I could split the switch statement out into a separate function, like
this:

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;

        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;
}

With that, then the stanza in keyboard_handle_key() looks cleaner:

        if (sym == XKB_KEY_F5 && input->modifiers == MOD_ALT_MASK) {
                if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
                        window_set_maximized(window,
                        !window->maximized);
        } else if (sym == XKB_KEY_F11 &&
                   window->fullscreen_handler &&
                   state == WL_KEYBOARD_KEY_STATE_PRESSED) {
                window->fullscreen_handler(window, window->user_data);
        } else if (sym == XKB_KEY_F4 &&
                   input->modifiers == MOD_ALT_MASK &&
                   state == WL_KEYBOARD_KEY_STATE_PRESSED) {
                window_close(window);
        } else if (window->key_handler) {
                if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
                        sym = process_key_press(sym, input);

                (*window->key_handler)(window, input, time, key,
                                       sym, state, window->user_data);
        }

What do you think of that approach?

> > +                       switch (xkb_compose_state_get_status(input->xkb.compose_state))
> > +                       {
> 
> Brace on same line.
> 
> > +                       case XKB_COMPOSE_COMPOSING:
> > +                               sym = XKB_KEY_NoSymbol;
> > +                               break;
> > +
> > +                       case XKB_COMPOSE_COMPOSED:
> > +                               sym = xkb_compose_state_get_one_sym(input->xkb.compose_state);
> > +                               break;
> > +
> > +                       case XKB_COMPOSE_CANCELLED:
> > +                       case XKB_COMPOSE_NOTHING:
> > +                               break;
> 
> CANCELLED should also produce NoSymbol, but NOTHING is fine to pass through.
> 
> Cheers,
> Daniel

Bryce