[PATCH] xkb: Support NoLock and NoUnlock for LockControls

[Ran Benita]

On Sat, Feb 15, 2014 at 05:37:08PM +0100, Andreas Wettstein wrote:
> The XKB protocol specification demands support for these flags.
> 
> Signed-off-by: Andreas Wettstein <wettstein509 at solnet.ch>
> ---
>  xkb/xkbActions.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
> index 89360df..84190df 100644
> --- a/xkb/xkbActions.c
> +++ b/xkb/xkbActions.c
> @@ -688,14 +688,22 @@ _XkbFilterControls(XkbSrvInfoPtr xkbi,
>          filter->keycode = keycode;
>          filter->active = 1;
>          filter->filterOthers = 0;
> -        change = XkbActionCtrls(&pAction->ctrls);
> +        change = (XkbActionCtrls(&pAction->ctrls) & ~ctrls->enabled_ctrls);

I think this slightly changes the semantics for SetControls, in that if
you SetControls with something that is already set, all the stuff below
used to happen, and now it won't. I think you should not change it just
to be safe.

>          filter->priv = change;
>          filter->filter = _XkbFilterControls;
>          filter->upAction = *pAction;
>  
>          if (pAction->type == XkbSA_LockControls) {
> -            filter->priv = (ctrls->enabled_ctrls & change);
> -            change &= ~ctrls->enabled_ctrls;
> +            if (pAction->ctrls.flags & XkbSA_LockNoUnlock)
> +                filter->priv = 0;

I'd prefer if you did the NoUnlock on the release branch of the if, less
magical this way.

> +            else
> +                /* The protocol specification says we should unlock controls
> +                   are NOT currently enabled.  As this does not make any sense,
> +                   let's assume this is a typo and do the opposite. */
> +                filter->priv = (XkbActionCtrls(&pAction->ctrls) & ctrls->enabled_ctrls);
> +
> +            if (pAction->ctrls.flags & XkbSA_LockNoLock)
> +                change = 0;

And this I'd move inside the 'if (change)'' part, make it only protect the
'enabled_ctrls |= change'. Again so all the other stuff there will
continue to happen; the XkbComputeControlsNotify ought to be smart
enough to know whether to send a notify event or not (though I haven't
looked closely).

Ran

>          }
>  
>          if (change) {
> -- 
> 1.8.3.1
>