Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking changes of mod key release in xkeysnail 0.3.0 #74

Closed
ncaq opened this issue Apr 27, 2020 · 14 comments · Fixed by #87, paulie-g/xkeysnail#1 or trueneu/xkeysnail#1
Closed

Breaking changes of mod key release in xkeysnail 0.3.0 #74

ncaq opened this issue Apr 27, 2020 · 14 comments · Fixed by #87, paulie-g/xkeysnail#1 or trueneu/xkeysnail#1

Comments

@ncaq
Copy link

ncaq commented Apr 27, 2020

Breaking changes in xkeysnail 0.3.0.

The behavior is very different when you operate the keyboard in the following order.

press Ctrl
press Alt
release Ctrl
press W

xkeysnail 0.2.0: perform Alt-w
xkeysnail 0.3.0: perform w

This behavior is especially problematic when you are operating Emacs.
I perform C-SPCC-eM-w.
When performing an operation such as, I have to be careful not to release the Ctrl key before hitting the Alt key but w will be typed.

I think this breaking change because
Release mapped modifiers after the original is released. closes #70. by rbreaves - Pull Request #71 - mooz/xkeysnail
I'm not sure why this change was necessary in the first place when I look at the issue.

I'd like to ask @rbreaves what his intentions were in making the change to fix the disruptive change while maintaining the significance of the change.


original strings

xkeysnail 0.3.0の破壊的変更について.

以下の順にキーボードを操作したとき動作が大きく異なります.

press Ctrl
press Alt
release Ctrl
press w

xkeysnail 0.2.0: perform Alt-w
xkeysnail 0.3.0: perform w

この動作は特にEmacsを操作している時に問題になります.
C-SPCC-eM-w
のような操作をした時に,
Altキーを押す前にCtrlキーを離さないように慎重に気をつけないとwが入力されてしまいます.

私はこの破壊的変更が
Release mapped modifiers after the original is released. Closes #70. by rbreaves · Pull Request #71 · mooz/xkeysnail
によって引き起こされたと思っていますが,
そもそもこの変更が何故必要だったのかissueを見てもよく分かりませんでした.

この変更の意義を保ったまま破壊的変更を修正するために @rbreaves に変更の意図を聞きたいです.

@rbreaves
Copy link
Contributor

I’ll take a look at what needs to be changed to fix it. I submitted the change so that Alt-Tab & Ctrl-Tab shortcuts could be properly remapped.

The prior behavior of quickly emitting hotkeys instead of mimicking the actual modifier release is fine in many cases but not for hotkeys that need a held key as it’s the held key that will keep Window switching active (Alt-tab). Ctrl-tab is more specific to the app, but without a held modifier in some apps the tab switching will break if most recently used is the default behavior, you’ll only switch between 2 tabs instead of the entire list of tabs.

@rbreaves
Copy link
Contributor

Can you share w/ me your emacs config.py shortcuts file so I can make sure I know that I’ve resolved it?

@ncaq
Copy link
Author

ncaq commented Apr 29, 2020

@rbreaves
My xkeysnail config is ncaq/.xkeysnail: My xkeysnail config.
But I do not apply special setting xkeysnail for Emacs.
xkeysnail drop modifier for all application.

@rbreaves
Copy link
Contributor

Ok, thank you @ncaq . I will try and work through this tomorrow or some time this week. I will either fix it in the K( ) function or propose a new one such as HK( ) for held key and honestly I thought about doing that, but given my specific needs and use cases I hadn't tested it as thoroughly as I'd have liked.

Perhaps we can come up with some sort of unit tests or another to verify that new features or features I write don't break existing functionality lol.

@rbreaves
Copy link
Contributor

And that is interesting @ncaq, I will use xbindkeys -mk to monitor the difference then and see what I need to change to have things passing through the same as it does under 0.2.

@rbreaves
Copy link
Contributor

rbreaves commented May 1, 2020

So I guess I shouldn't have admitted guilt so quickly @ncaq lol. It appears the breakage occurred by @kevinslashslash. I am not saying my own changes may not need to be modified slightly, but the issue existed before my own commit.

The breaking commit

git diff 56df0b9..8c4fadb

My commit changes

git diff e3dcedd~..e3dcedd

To figure this out I just used xbindkeys -mk and checked out what happens on those combos before and after my changes and saw that it behaved the exact same. Only when I went further back did I see the behavior you described.


The log description with the breaking change.

commit 8c4fadb
Author: Kevin Barry
Date: Sat Mar 23 02:16:29 2019 -0500

Fix modifier pressed/released handling

Previous send_combo would release/press modifiers unnecessarily
and sometimes trigger double-tap actions in apps.
This was likely introduced when handling of independent handling of
left/right modifiers was introduced.
Now modifiers are conservatively pressed/released.

commit 56df0b9
Author: Masafumi Oyamada
Date: Sun Jan 13 14:59:16 2019 +0900

Extract device operations into methods

@rbreaves
Copy link
Contributor

rbreaves commented May 1, 2020

Despite not having introduced the bug/behavior I have submitted a new commit/PR that I believe resolves the issues @ncaq. PR #76

My changes were significant enough that it really didn't matter who introduced it or why imo. The specific areas I had modified already were still the areas that needed modifying to correct the behavior. Unless someone can demonstrate a use case scenario where a key could get stuck I believe this behavior has been fixed.

@ncaq
Copy link
Author

ncaq commented May 2, 2020

I'm sorry.
I run git bisect result is

d79a3bc086a6635ffc3c8f8ef41c646b6f563407 is the first bad commit
commit d79a3bc086a6635ffc3c8f8ef41c646b6f563407
Author: Ben Reaves <[email protected]>
Date:   Sat Apr 18 19:37:50 2020 -0500
    Added release mapped modifiers only when original modifier has been released
 xkeysnail/output.py    | 13 ++++++++-----
 xkeysnail/transform.py |  8 +++++++-
 2 files changed, 15 insertions(+), 6 deletions(-)

So I miss find the commit.

@rbreaves
Copy link
Contributor

rbreaves commented May 2, 2020

Interesting, I have never seen or used git bisect before.. seems I am still learning new things about git all the time 😅. I tend to just use gitk or the git diff tool when I am inspecting things and it was fine to call me out earlier. @Lenbok had certainly found some issues with nested key remaps that needed fixing before the last merge.

@yoshinari-nomura
Copy link

In my case, after e3dcedd, i.e. merging PR #71, escape_next_key doesn't work well.

With this simple configuration:

import re
from xkeysnail.transform import *

define_keymap(None, {
    # Escape
    K("C-q"): escape_next_key,
}, "Test")

C-qC-p no longer passes C-p to the application (p is passed instead).
You have to release all keys after C-q and then press C-p to pass C-p.

git bisect reports that first bad commit: [6cf21fe]

Updated release modifier fix to unset mode_maps properly

Unfortunately, PR #76 does not solve my problem.

@yoshinari-nomura
Copy link

I noticed that when I pushed down the C-qC-p sequence very quickly, it worked as expected.
It seems that some kind of timeout process may not be working well.

Lenbok added a commit to Lenbok/xkeysnail that referenced this issue May 21, 2020
Fixes mooz#74, mooz#80, mooz#81

This reverts commit e3dcedd, reversing
changes made to 123432b.
@shicks
Copy link

shicks commented Sep 17, 2020

Is anyone looking into this anymore? At this point I've reverted to 0.2.0 to avoid it, but it would be nice to get updates.

@joshgoebel
Copy link

@ncaq Just to be clear here your issue here wasn't with any type of remapping, but just with the basic pass-thru behavior breaking your keystroke chains, yes? I'm trying to understand all the issues here to see if we can have our cake and eat it also - have a release that handles both cases without requiring any custom patches.

@ncaq
Copy link
Author

ncaq commented May 26, 2022

@joshgoebel
I'm not too sure I understand the question properly.
Yes, the problem was that the combination with Ctrl and Alt was broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants