[PATCH 2/2] compositor: add a way to change the keyboard leds

[Pekka Paalanen]

On Thu, 21 Aug 2014 08:37:17 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> Last nitpick, sorry ...
> 
> On Wednesday, August 20, 2014, Giulio Camuffo <giuliocamuffo at gmail.com>
> wrote:
> >
> > +       mods_depressed =
> > xkb_state_serialize_mods(keyboard->xkb_state.state,
> > +                                               XKB_STATE_DEPRESSED);
> > +       mods_latched = xkb_state_serialize_mods(keyboard->xkb_state.state,
> > +                                               XKB_STATE_LATCHED);
> > +       mods_locked = xkb_state_serialize_mods(keyboard->xkb_state.state,
> > +                                               XKB_STATE_LOCKED);
> > +       group = xkb_state_serialize_group(keyboard->xkb_state.state,
> > +                                      XKB_STATE_EFFECTIVE);
> > +
> > +       num = (1 << keyboard->xkb_info->mod2_mod);
> > +       caps = (1 << keyboard->xkb_info->caps_mod);
> 
> 
> I still don't really believe the non-xkbcommon build should exist, but
> shouldn't all this be #ifdef'ed?

Looks like that to me, too, we have #ifdef ENABLE_XKBCOMMON it seems,
and input.c is full of it. So a simple ifdef around this function would
be needed.

FWIW, I think the --disable-xkbcommon build is broken anyway,
compositor.h uses e.g. xkb_mod_index_t and unconditionally includes
xkbcommon.h, so if xkbcommon is not in the standard system path, weston
won't build at all. But apparently it works for runtime to not link to
libxkbcommon.so, so let's keep it that way.

I just wonder...

weston_keyboard_set_leds()... but that also changes the XKB state to
match the leds, right?

Should it be called weston_keyboard_set_locks() or something?

The "compositor: add an option to set the default numlock value" is
fine by me, and the "evdev/libinput: sync the leds of keyboards with
the xkb state" you both already seemed to agree to leave for later.


Thanks,
pq