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.

Sign up for GitHub

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

Release mapped modifiers after the original is released. Closes #70. #71

Merged
merged 6 commits into from
Apr 25, 2020
Merged

Release mapped modifiers after the original is released. Closes #70. #71

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d79a3bc
Added release mapped modifiers only when original modifier has been r…
rbreaves Apr 19, 2020
567cfe0
Added an exception for Shift to not undo mapped or the original modif…
rbreaves Apr 19, 2020
7ac29be
Updated release modifier fix to iterate and set var
rbreaves Apr 19, 2020
e495db8
Updated release modifier fix to support no new modifier remaps
rbreaves Apr 19, 2020
6cf21fe
Updated release modifier fix to unset mode_maps properly
rbreaves Apr 19, 2020
5cd22ed
Removed debug print statement
rbreaves Apr 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions xkeysnail/output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
_pressed_modifier_keys = set()
_pressed_keys = set()

def output_modifier_key():
return _pressed_modifier_keys

def update_modifier_key_pressed(key, action):
if key in Modifier.get_all_keys():
if action.is_pressed():
Expand Down Expand Up @@ -57,8 +60,11 @@ def send_combo(combo):
missing_modifiers.remove(modifier)

for modifier_key in extra_modifier_keys:
send_key_action(modifier_key, Action.RELEASE)
released_modifiers_keys.append(modifier_key)
# Do not release new modifier
# until original modifier is released
if modifier_key != str(modifier.get_key()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

UnboundLocalError: local variable 'modifier' referenced before assignment

Copy link
Contributor Author

@rbreaves rbreaves Apr 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm.. I am not really seeing or getting that error, but I will admit.. it seems like I should probably be having to define "modifier" within a loop, which I am not doing. Perhaps it only runs because of the prior for loop above it?

for modifier in combo.modifiers:

Is there a switch/argument I can run to get more debug like output when running it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though I am not sure how to generate the debug output you showed I went ahead and set it to iterate through the possible modifier keys. I am not 100% certain it covers all use cases though, nor previously to be honest. It does appear to work fine with my use cases though.

There is one minor issue with using the pass_through_key. If I hit f3 and then try to use another mapped key then that mapped key fails initially until I hit it a second time.

# the issue
K("f3"): pass_through_key, # cancel find_next

K("C-g"): K("f3"), # find_next
K("Super-C-g"): K("M-f3"), # Default - find_all_under

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The debug output I posted was just the exception that was spat out to the terminal that I started xkeysnail in. You can add print statements etc to help with debugging.

I just added in your most recent commit, and this patch seems broken. My bindings to allow C-v and M-v in Firefox for page down / page up no longer work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Admittedly, the branch I'm testing this on contains other patches I've been using for a while, so there could be other interactions. https://github.com/Lenbok/xkeysnail/tree/tmp-test-mod-release )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, with your latest commit the single key mappings are fixed.

It's still broken. E.g. I have added tmux-ish style bindings to firefox like this:

# Keybindings for Firefox/Chrome
define_keymap(re.compile("Firefox|Google-chrome"), {
    # [...]
    # ` YYY, like tmux equivalents
    K("GRAVE"): {
        # ` ` (backtick)
        K("GRAVE"): K("GRAVE"),
        # ` k (close tab)
        K("k"): K("C-w"),
        # ` K (undo close tab)
        K("Shift-k"): K("C-Shift-t"),
        # ` c (new tab)
        K("c"): K("C-t"),
        # ` C (new container tab)
        K("Shift-c"): K("C-DOT"),
        # ` n (next tab)
        K("n"): K("C-TAB"),
        # ` p (prev tab)
        K("p"): K("C-Shift-TAB"),
        # ` TAB (switch to most recent tab, requires "Most Recent Tab" add on)
        K("TAB"): K("C-Shift-key_1"),
        # ` 1 (switch to tab 1)
        K("key_1"): K("M-key_1"),
        K("key_2"): K("M-key_2"),
        K("key_3"): K("M-key_3"),
        K("key_4"): K("M-key_4"),
        K("key_5"): K("M-key_5"),
    }
}, "Firefox and Chrome")

When I press grave followed by c, to create a new tab, the underlying C-t is issued but the control modifier is stuck down. (i.e. subsequently pressing s opens the save dialog). Is the problem that none of the input keys actually involved the modifier being issued?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've actually been wondering about nested scenarios, but I also haven't really tested any non-modifier key scenarios either I don't think. I will have to look into this more later. It's getting late for me.

I've been testing with this set of hotkeys.
https://github.com/rbreaves/kinto/blob/dev/xkeysnail/kinto.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lenbok I updated my patch to include mode_maps or multiple stroke keybinds rather. I tested it against your keybindings and everything appeared to work except for Grave - p. I am not sure why, but even with xkeysnail not running I couldn't get C-Shift-Tab to work right off any ways, only after C-Tab starts the tab switching process.. so I dunno what was up with that. I guess adding C-Tab to the start of it might have fixed it for me, but I did not test that out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be a better way to handle setting when to mind multiple stroke keybinds, but I am basically just checking if _mode_map is anything but None, it is set when a multi stroke keybind is in play and if it is then I know to set a global boolean to remind the on_key function that it should manually release any mods set on the next go around.

Normally output.py would handle the release of mods that had been set, but I removed that bit of code completely as it interferes with proper remaps that would otherwise align properly with the length of the original modifier key being held.

Stuck mod keybinds are a risk I suppose with this change, but I think it is worth figuring out given the ability to remap keys like Alt and Ctrl-Tab can benefit due to the nature of how those keys work. Another alternative might be to create a similar but different K like function, maybe HK( ), to denote that keys being mapped via this function will attempt to Hold modifier keys until the original modifiers are released.

Copy link
Contributor Author

@rbreaves rbreaves Apr 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now running my patch on a chromebook and have 1 or 2 other users using it as part of my Kinto (mac keybinding) app. It is part of my Alpha branch over there, after a few more days of testing and if I work out my wordwise support for browsers I'll merge into Kinto's dev branch. At least that is my plan.

@Lenbok If you still have any issues please let me know.

Initially I was going to work on a separate branch and integrate caret input detection directly into xkeysnail, but it is such a niche feature I really can't see the benefit of having it in xkeysnail directly. It would probably just introduce bugs and I still can't get IBus to work fully under Arch w/ Gnome, so there is that too.

I do have another ticket open for config py file update detection, which would also be beneficial. For now I am handling that via inotify-tools in bash as I don't think it can be implemented the same way that hotplug (watch) command is and would require another library to allow for a non-blocking while loop, I think. I tried already but it didn't go anywhere and I am not wanting to add a new library just for that.

send_key_action(modifier_key, Action.RELEASE)
released_modifiers_keys.append(modifier_key)

pressed_modifier_keys = []
for modifier in missing_modifiers:
Expand All @@ -70,9 +76,6 @@ def send_combo(combo):

send_key_action(combo.key, Action.RELEASE)

for modifier in reversed(pressed_modifier_keys):
send_key_action(modifier, Action.RELEASE)

for modifier in reversed(released_modifiers_keys):
send_key_action(modifier, Action.PRESS)

Expand Down
9 changes: 8 additions & 1 deletion xkeysnail/transform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import itertools
from inspect import signature
from .key import Action, Combo, Key, Modifier
from .output import send_combo, send_key_action, send_key, is_pressed
from .output import send_combo, send_key_action, send_key, is_pressed, output_modifier_key

__author__ = 'zh'

Expand Down Expand Up @@ -365,9 +365,16 @@ def on_event(event, device_name, quiet):


def on_key(key, action, wm_class=None, quiet=False):
output_mods = output_modifier_key().copy()
if key in Modifier.get_all_keys():
update_pressed_modifier_keys(key, action)
send_key_action(key, action)
# Release mapped modifier only when physical mod
# is released
if str(key) != "Key.LEFT_SHIFT" and str(key) != "Key.RIGHT_SHIFT":
for output_key in output_mods:
update_pressed_modifier_keys(output_key, action)
send_key_action(output_key, action)
elif not action.is_pressed():
if is_pressed(key):
send_key_action(key, action)
Expand Down