[PATCH libxkbcommon] Add xkb_key_get_action API

[Daniel Stone]

Hi Ran,

On 24 March 2012 15:03, Ran Benita <ran234 at gmail.com> wrote:
> I like the API, it's clear and extensible, and the filter-pipeline
> implementation is also nice (I'd never think of something like that..).

Ha, in fairness, it's not actually original, mostly just ripped from
the server code. ;)

> I've ported some of my code to use it, and it mostly worked. The few
> things I have remaining are:
> - Handling of group lock actions (I tried a simple patch for that, see
>  the following pull request).

Great, thanks. :)

> - Some way to update the state according to a given led state, i.e. say
>  the app is starting with the Caps Lock led on, I'd like to update the
>  modifier state according to the indicator map. Right now there are
>  only getters in the new API.

This should be fairly straightforward for most LEDs, which only
reflect the state of a single locked modifier or group.  For LEDs
which reflect multiple states though, which modifiers/groups do you
choose, and for LEDs which reflect effective state, which state
component do you choose?

I'm tempted to say that we only deal with the straightforward case,
and don't bother with the other case at all, so then your call would
look like:

{
    xkb_led_mask_t led_mask = (1 << xkb_map_led_get_index(xkb, "Caps Lock"));
    xkb_led_index_t led;

    xkb_state_update_led_mask(state, led_mask);
    for (led = 0; led < xkb_map_num_leds(xkb); led++)
        set_led(xkb_state_led_index_is_active(state, led)));
}

That way xkbcommon could make a best-effort attempt to get the state
in sync with the LEDs, and the user could then update the LEDs to
match the new state.

If that sounds sensible I'll knock that up too.

> - Some help with handling key repeat events. I'm not sure what the scope
>  of this (if any) should be, but there is some stuff like the key
>  repeat control and per key repeat. Should any of this be exposed?

Yeah, I'm really not sure what the answer is here.  I'm tempted to say
that we should probably add an extra out parameter to
xkb_state_update_key, being the repeat time, but it seems a bit icky.
There's also the AccessX stuff to consider (sticky keys, debouncing,
etc), which should also pass a notification back to the application to
let it know what's happened.  My current thought is to put that in the
state and let applications clear it, so perhaps repeats could be put
there as well?

I guess the alternative is to change xkb_state_update_key to return an
int: -1 meaning that the key should not be sent (e.g. due to AccessX
slowkeys/debouncing), 0 meaning that the key should be sent and does
not repeat, and a positive value specifying the time in milliseconds
until the key should repeat.

Ideas?

> - Related, maybe the "down" parameter can be changed to be a little more
>  descriptive? Maybe an enum or even a flag mask. I think boolean
>  parameters usually don't go well, and are also pretty meaningless on
>  the caller's side, e.g:
>        xkb_state_update_key(state, keycode, 1);
>  versus
>        xkb_state_update_key(state, keycode, XKB_KEY_PRESSED);
>  or something more suitable.

Good call, I've done that now, thanks.

> Again, it's pretty nice and helpful; thanks for doing it :)

No, again, thank you. :)

>> At the moment, I can't see a use for multiple actions per keypress,
>> whereas there's a definite need for symbols, as Unicode has been
>> pushing back on adding precomposed symbols, so some keymaps have
>> symbols which cannot be represented in a single symbol; at the moment,
>> they're working around it by having a placeholder symbol which
>> triggers an entry in the Compose map with multiple symbols.
>
> Interesting. I think many wrapping APIs work by only sending one event
> per key press; those will need to be adapted to respond with multiple
> events.

Most other window systems seem to allow a single keypress to generate
multiple keysyms, so for now it's just a matter of applications
ignoring it if they can't support it, I guess.

Cheers,
Daniel