| Message ID | 87zjlsz61o.fsf@solnet.ch |
|---|---|
| State | Accepted |
| Commit | 5447ac45bc090e8f3269979af4db55f619c5f278 |
| Headers | show |
diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c index 1443498..da2e242 100644 --- a/xkb/xkbActions.c +++ b/xkb/xkbActions.c @@ -181,6 +181,7 @@ _XkbFilterSetState(XkbSrvInfoPtr xkbi, XkbFilterPtr filter, unsigned keycode, XkbAction *pAction) { if (filter->keycode == 0) { /* initial press */ + AccessXCancelRepeatKey(xkbi, keycode); filter->keycode = keycode; filter->active = 1; filter->filterOthers = ((pAction->mods.mask & XkbSA_ClearLocks) != 0); @@ -354,6 +355,9 @@ static int _XkbFilterLockState(XkbSrvInfoPtr xkbi, XkbFilterPtr filter, unsigned keycode, XkbAction *pAction) { + if (filter->keycode == 0) /* initial press */ + AccessXCancelRepeatKey(xkbi, keycode); + if (pAction && (pAction->type == XkbSA_LockGroup)) { if (pAction->group.flags & XkbSA_GroupAbsolute) xkbi->state.locked_group = XkbSAGroup(&pAction->group); @@ -678,6 +682,7 @@ _XkbFilterControls(XkbSrvInfoPtr xkbi, ctrls = xkbi->desc->ctrls; old = *ctrls; if (filter->keycode == 0) { /* initial press */ + AccessXCancelRepeatKey(xkbi, keycode); filter->keycode = keycode; filter->active = 1; filter->filterOthers = 0;
On Sat, Feb 15, 2014 at 05:36:51PM +0100, Andreas Wettstein wrote: > The autorepeat for these actions was not correctly implemented, as the key > repeat would be mistakenly interpreted as key releases. Rather than fixing > this, this change simply disables autorepeat for Set/Lock actions, for two > reasons: > > - Autorepeating Set/Lock keys make complicate the interactions of actions. > > - Autorepeating Set/Lock keys have no apparent benefit, but hurt in the real > world for layouts such as de(neo): Neo has a Level5 shift on the LSGT key, > and a Level5 lock on Level5 of the same key. This is unusable if LSGT > autorepeats. However, disabling autorepeat for key LSGT completely is not > ideal for users that have a "usual" layout besides Neo, where LSGT carries > symbols. I'm not too familiar with this and the other unreviewed patches, since the stuff they touch were either removed or non-relevant for libxkbcommon. So I'd need to find some time to figure them out. In the mean time I hope someone will apply the rest. I've stolen some of them for libxkbcommon, btw. [also if you don't mind some more procedural comments: it helps if you number the patches in the series, i.e. 1/5 v10 etc. That way we can refer to them by number. Also a cover letter (PATCH 0) with some general blurb is sometimes nice for comments on the entire series]. Thanks, Ran > Signed-off-by: Andreas Wettstein <wettstein509@solnet.ch> > --- > xkb/xkbActions.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c > index 1443498..da2e242 100644 > --- a/xkb/xkbActions.c > +++ b/xkb/xkbActions.c > @@ -181,6 +181,7 @@ _XkbFilterSetState(XkbSrvInfoPtr xkbi, > XkbFilterPtr filter, unsigned keycode, XkbAction *pAction) > { > if (filter->keycode == 0) { /* initial press */ > + AccessXCancelRepeatKey(xkbi, keycode); > filter->keycode = keycode; > filter->active = 1; > filter->filterOthers = ((pAction->mods.mask & XkbSA_ClearLocks) != 0); > @@ -354,6 +355,9 @@ static int > _XkbFilterLockState(XkbSrvInfoPtr xkbi, > XkbFilterPtr filter, unsigned keycode, XkbAction *pAction) > { > + if (filter->keycode == 0) /* initial press */ > + AccessXCancelRepeatKey(xkbi, keycode); > + > if (pAction && (pAction->type == XkbSA_LockGroup)) { > if (pAction->group.flags & XkbSA_GroupAbsolute) > xkbi->state.locked_group = XkbSAGroup(&pAction->group); > @@ -678,6 +682,7 @@ _XkbFilterControls(XkbSrvInfoPtr xkbi, > ctrls = xkbi->desc->ctrls; > old = *ctrls; > if (filter->keycode == 0) { /* initial press */ > + AccessXCancelRepeatKey(xkbi, keycode); > filter->keycode = keycode; > filter->active = 1; > filter->filterOthers = 0; > -- > 1.8.3.1 >
This patch has not been reviewed yet. Can someone please have a look? It is fairly small. Thank you, Andreas
Hi, On 23 March 2014 16:57, <wettstein509@solnet.ch> wrote: > This patch has not been reviewed yet. Can someone please have a look? > It is fairly small. This is definitely something we want. If you add the same for latches: Reviewed-by: Daniel Stone <daniel@fooishbar.org> Cheers, Daniel > Thank you, > > Andreas > _______________________________________________ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel
Hello Daniel, > This is definitely something we want. If you add the same for > latches: Latches already disable autorepeat. Andreas
The autorepeat for these actions was not correctly implemented, as the key repeat would be mistakenly interpreted as key releases. Rather than fixing this, this change simply disables autorepeat for Set/Lock actions, for two reasons: - Autorepeating Set/Lock keys make complicate the interactions of actions. - Autorepeating Set/Lock keys have no apparent benefit, but hurt in the real world for layouts such as de(neo): Neo has a Level5 shift on the LSGT key, and a Level5 lock on Level5 of the same key. This is unusable if LSGT autorepeats. However, disabling autorepeat for key LSGT completely is not ideal for users that have a "usual" layout besides Neo, where LSGT carries symbols. Signed-off-by: Andreas Wettstein <wettstein509@solnet.ch> --- xkb/xkbActions.c | 5 +++++ 1 file changed, 5 insertions(+)