[PATCH] xkb: Support NoLock and NoUnlock for LockControls

wettstein509 at solnet.ch wettstein509 at solnet.ch
Wed Feb 19 09:32:00 PST 2014


> > -        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.

You are right, it is safer.

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

I trust you on that...

> > +            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).

You are right.  XkbComputeControlsNotify compares old and new enabled
controls, so it should be smart enough.

Thanks for wading through all this.  I will send a revised patch.

Andreas


More information about the xorg-devel mailing list