joshgoebel / keyszer

a smart, flexible keymapper for X11 (a fork/reboot of xkeysnail )
Other
69 stars 15 forks source link

Let string and Unicode processors adapt to CapsLock LED state #129

Closed RedBearAK closed 1 year ago

RedBearAK commented 1 year ago

Changes

Updates config_api and transform to pass the context object to commands (filtering for whether or not the target function is able to accept the new context parameter), and allowing the string and Unicode processor helper functions to adapt their output to deal with CapsLock being enabled.

Should resolve issue #128

Checklist

joshgoebel commented 1 year ago

Probably need some tests for this code?

RedBearAK commented 1 year ago

Probably need some tests for this code?

I'm still pretty confused about how to setup the tests, but I can try. But other than... testing the ability to detect that CapsLock is on, I'm not sure what would be getting tested? That some sets of shifted and unshifted characters come out correctly when CapsLock is ON and when CapsLock is OFF?

RedBearAK commented 1 year ago

Also my environments have been having an inexplicable problem finding Python modules and methods even though the code is working fine when I run keyszer in the venv. I've made sure to install things like pytest and pytest_asyncio, but the results of running pytest in the venv are still just wall-to-wall red errors.

This is happening in both Fedora (the host) and Ubuntu (the VM guest).

So I won't have much luck trying to run any tests until I figure out what that's all about.

RedBearAK commented 1 year ago

OK, progress. Needs to be pytest . and I need to avoid naming any beta files with "test_" as a prefix.

There are a lot of warnings about "pytest.mark.looptime".

Then, failures:

FAILED tests/test_api_helpers.py::test_type_simple - assert [<Key.H: 35>, <Key.E: 18>, <Key.L: 38>, <Key.L: 38>, <Key.O: 24>, <Key.KEY_5: 6>] == <function to_US_keystrokes.<locals>._to_keystrokes at 0x7fe717259b40>
FAILED tests/test_api_helpers.py::test_type_simple_with_shift - assert [Shift-H, <Key.E: 18>, <Key.L: 38>, <Key.L: 38>, <Key.O: 24>] == <function to_US_keystrokes.<locals>._to_keystrokes at 0x7fe717259f30>
FAILED tests/test_api_helpers.py::test_type_unsupported_character - AttributeError: 'ExceptionInfo' object has no attribute 'message'
FAILED tests/test_api_helpers.py::test_type_extended_ascii - assert [Shift-Ctrl-U, <Key.F: 33>, <Key.F: 33>, <Key.ENTER: 28>] == <function to_US_keystrokes.<locals>._to_keystrokes at 0x7fe7170a2710>
FAILED tests/test_api_helpers.py::test_ascii_keys - assert [<Key.GRAVE: 41>, <Key.MINUS: 12>, <Key.EQUAL: 13>, <Key.LEFT_BRACE: 26>, <Key.RIGHT_BRACE: 27>, <Key.BACKSLASH: 43>, ...] == <function to_US_keystrokes.<locals>._to_keystr...
FAILED tests/test_api_helpers.py::test_ascii_with_shift_keys - assert [Shift-GRAVE, Shift-KEY_1, Shift-KEY_2, Shift-KEY_3, Shift-KEY_4, Shift-KEY_5, ...] == <function to_US_keystrokes.<locals>._to_keystrokes at 0x7fe7170a2b90>
FAILED tests/test_api_helpers.py::test_type_unicode - assert [Shift-Ctrl-U, <Key.KEY_1: 2>, <Key.F: 33>, <Key.KEY_3: 4>, <Key.KEY_8: 9>, <Key.KEY_9: 10>, ...] == <function to_US_keystrokes.<locals>._to_keystrokes at 0x7fe7170a2b00>
FAILED tests/test_api_helpers.py::test_uncode_keystrokes - assert [Shift-Ctrl-U, <Key.F: 33>, <Key.F: 33>, <Key.ENTER: 28>] == <function unicode_keystrokes.<locals>._unicode_keystrokes at 0x7fe717259bd0>
FAILED tests/test_basics.py::test_escape_next_key - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_basics.py::test_after_combo_should_lift_exerted_keys - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_basics.py::test_sticky_combo_should_lift_exerted_keys_before_combo - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_basics.py::test_sticky_combo_with_sticky_inkey_in_output_combo - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_basics.py::test_real_inputs_do_not_reexert_during_combo_sequence - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_basics.py::test_simple_to_keystrokes - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_helpers.py::test_wm_class_match_and_inverse - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_keymap_basics.py::test_OLD_API_multiple_keys_at_once - assert [(<Action.PRE...FT_CTRL: 29>)] == [(<Action.PRE...FT_CTRL: 29>)]
FAILED tests/test_keymap_basics.py::test_wm_conditional_as_argument - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_keymap_basics.py::test_multiple_keys_at_once - assert [(<Action.PRE...FT_CTRL: 29>)] == [(<Action.PRE...FT_CTRL: 29>)]
FAILED tests/test_keymap_basics.py::test_multiple_combos_without_releasing_all_nonsticky - assert [(<Action.PRE....K: 37>), ...] == [(<Action.PRE...FT: 42>), ...]
FAILED tests/test_modmap_cond.py::test_cond_modmap_wins_over_default_modmap - assert [(<Action.PRE....F: 33>), ...] == [(<Action.PRE....F: 33>), ...]
FAILED tests/test_modmap_cond.py::test_when_in_emacs - assert [(<Action.PRE....F: 33>), ...] == [(<Action.PRE....F: 33>), ...]
FAILED tests/test_modmap_cond.py::test_multiple_conditional_maps_can_match - assert [(<Action.PRE....G: 34>), ...] == [(<Action.PRE....G: 34>), ...]
FAILED tests/test_modmap_cond_multi.py::test_weird_salute_firefox - assert [(<Action.PRE... <Key.A: 30>)] == [(<Action.PRE...FT_CTRL: 29>)]
FAILED tests/test_nested_keymaps.py::test_nested_keymaps - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_nested_keymaps.py::test_trigger_immediately - AttributeError: 'MockKeyboard' object has no attribute 'leds'
FAILED tests/test_sticky_bind.py::test_single_key_auto_sticky - AttributeError: 'MockKeyboard' object has no attribute 'leds'

Will try to start working through some of these.

joshgoebel commented 1 year ago

Or I could try and get the tests back to a baseline... I think we've been neglecting them as we added things... need to get them tied into CI better.

RedBearAK commented 1 year ago

I think the main thing with several of these is they will need to be passing the context object into the helper functions, so they get back the expected list of keystrokes like before.

Tried to deal with the mock keyboard with some help, but only managed to convert the error into a TypeError so far, by adding the leds attribute.

from evdev.ecodes import EV_KEY
from evdev.events import InputEvent
from evdev import UInput
from lib.xorg_mock import set_window

from keyszer.models.action import PRESS, RELEASE, REPEAT
from keyszer.transform import on_event

# class MockKeyboard():
class MockKeyboard(UInput):
    name = "generic keyboard"
    def __init__(self, events=[]):
        super().__init__(events=events)
        self.leds = 0

I'm afraid if I even have a chance at dealing with these it will take quite a while. Sorry I can't be of more assistance.

RedBearAK commented 1 year ago

Can we just merge this if it doesn't have any known problem and work on the test updates later?

RedBearAK commented 1 year ago

So, as mentioned in the related PR, I figured out how to get the tests passing again, with the new form of the processors that will return inner functions instead of lists.

Along with the changes in the other new PRs, I get all green across the board when running pytest . now (except for that one test marked "skip"). No errors, no warnings.

Oh, and I seem to have figured out how to get rid of the "no attribute named leds" errors, by fleshing out the mock keyboard device a bit, and giving it an leds() method that returns a list. That seems to be how leds() works for the real device. And those errors no longer appear.

RedBearAK commented 1 year ago

I don't get why this is the case, but GitHub has always claimed there is a "conflict" with this PR, and highlights only the change in the Unicode processor, even though the string processor has similar changes. It offers me a "Resolve conflicts" button, but here doesn't seem to be anything I can do on my end to actually resolve the "conflict". Which just seems to point to the contents of the Unicode function being changed.

Unfortunately, this "conflict" is also keeping me from re-syncing this branch with the more recent updates/merges to your main branch.

joshgoebel commented 1 year ago

The UI shows the conflict very clearly:

Screen Shot 2023-04-02 at 5 31 22 PM

The function seems to have gone very different directions in main vs in your branch... you'll have to pick the correct one, or merge them by hand...

joshgoebel commented 1 year ago

In this case (at a glance) it looks like your new version may just want to win and replace the old code, thus resolving the conflict.

RedBearAK commented 1 year ago

In this case (at a glance) it looks like your new version may just want to win and replace the old code, thus resolving the conflict.

I guess I was leaving a stray part of the "diff" in place or something, leading to the preview window showing the whole rest of the file as an error and not letting me actually "resolve" it. I thought it was something you had to fix from your side. I tried again and it let me click the "resolve" button.

Still don't have a clue why it only complained about that function and not the string function just above it that has similar changes (addition of the inner function). It never seemed to care about that one.

joshgoebel commented 1 year ago

that has similar changes

Conflicts are about what changed in both branches... not just what you changed. Conflicts happen when you have overlapping or incompatible changes between branches- and git doesn't know which should win...

RedBearAK commented 1 year ago

@joshgoebel

Bumping. As far as I know, the final state of this PR met with your approval, and merging was only held up due to the failing tests. Since those tests are now fixed, or at least have PRs that will fix them...

joshgoebel commented 1 year ago

Somehow this PR now includes a bunch of other things... if you reset it back to just the caps lock stuff I'll take another look.

RedBearAK commented 1 year ago

Well, that was annoying. More than one thing went wrong there. Anyway, I did a hard reset back to what should have been the last relevant commit specific to this fix, and then resolved the "conflict" again, favoring the new version of the Unicode function. Should be back to where it was when you deemed it complete enough to start checking on the test suite.

I'm not going to touch it, even to fast-forward to your current main, which has several new things merged since this was "done".

Not sure what happened, I was just trying to include this fix in some new branches of my own.

Thanks for letting me know.

RedBearAK commented 1 year ago

Oh, I guess just the process of fixing the "conflict" actually caused the system to automatically merge the recent changes to main into it. It's saying it's only "ahead" and no commits "behind" your main branch.

Other than fixing the conflict, I haven't messed with it, and don't plan to. I'll make a copy of the branch if I need to use it for something else.

RedBearAK commented 1 year ago

Thanks! 👍🏽 🥳 🎉