> > - 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
