Output from string/Unicode helpers broken when CapsLock is ON

[RedBearAK]

I'm trying to fix the bug that breaks the string and Unicode processor functions when CapsLock is "ON". First step was to make them into inner functions so they might be able to respond in real-time in different ways depending on the state of the CapsLock LED. That works fine. Now they no longer just create a static list once on startup. But, I'm failing to understand how to get access to the key context object (and thus the "capslock_on" property) inside either processor function. Or really anywhere in config_api.py.

There's something really fundamental about how the context object works (i.e., where it "comes from" and how it becomes accessible) that I've obviously never understood completely. The config file doesn't directly import the KeyContext class from key_context.py or do a "KeyContext(device)" anywhere, or define "ctx = whatever" anywhere, yet whenever you do a "lambda ctx" you can suddenly access all the different properties with things like "ctx.capslock_on".

I was able to use the "ctx" object in my matchProps() function with no difficulty, by passing it in as a parameter in the inner function. But when I try the same trick with the _unicode_keystrokes inner function in config_api.py, it acts like it has no idea what that parameter is.

So I'm kind of stuck until I can understand how to access the "capslock_on" property from within these inner functions, at the moment the Unicode or string processing function is "fired" by an input combo.

Been struggling with this for a while. Any kind of clue would be helpful.

[joshgoebel]

Context is only passed to keymap/modmap conditionals...

• https://github.com/joshgoebel/keyszer/blob/main/src/keyszer/transform.py#L239
• https://github.com/joshgoebel/keyszer/blob/main/src/keyszer/transform.py#L253

You can look at where commands are called:

https://github.com/joshgoebel/keyszer/blob/main/src/keyszer/transform.py#L551

Commands simply do not have access to this information. Context isn't currently even available that deep into the transform...

[RedBearAK]

Commands simply do not have access to this information. Context isn't currently even available that deep into the transform...

Oof. Without that information I don't see how we can get those functions to adapt to the state of the CapsLock LED. I gather this doesn't affect keyboard shortcut combos because the system ignores the CapsLock state in those cases, but when we're "typing" characters to output it really messes with things. Disables the Shift-Ctrl-u shortcut to initiate Unicode entry, and so on.

I will look at those locations, but do you have any other idea how this could be fixed? Or do I just need to look at passing context all the way through transform so that it's available to these functions when they get called?

Wait, maybe I can just use a wrapper from within config, and pass the context in that way? No, if it's only passed into the "when" part, does that mean it's also not available in "mappings"... Hmm... 🤔

[RedBearAK]

Do you think it's possible to do a real-time test within the processor function where I "assert" some normal letter keystroke and check whether it came out as lowercase or uppercase?

[joshgoebel]

What would you do if you had access to caps lock state? Output the opposite of what key was pressed (lower vs upper) to get around caps lock?

[RedBearAK]

For the Unicode function, I think the only thing that needs to be done during the creation of the list is to prepend and append a CapsLock keystroke, to reverse the state and allow the Shift+Ctrl+u shortcut to invoke the Unicode entry method. I don't think the method particularly cares whether the hex letters are upper or lower.

For the string processor, I'm not exactly sure what needs to be done yet. But most likely just something similar, adding a CapsLock keystroke at the beginning and end of the combo_list, to reverse the state during the output of the string.

I always use the Unicode function directly, but there is this inside the string processor:

            if ord(c) > 127:
                combos = unicode_keystrokes(ord(c))
                combo_list.extend(combos)

So it may actually be a little more complicated in the case where they interact. Have to make sure there won't be duplicate reversals of the CapsLock state.

I'm sure I can figure it all out, I just need to somehow get access to that dang CapsLock state.

[RedBearAK]

Oh, I think I kind of broke that part of the string processor, since both of the processor functions now return functions. 😛 I'll need to find some way around that. Global var, maybe?

  File "/home/kris/Documents/keyszer/src/keyszer/config_api.py", line 172, in _to_keystrokes
    combo_list.extend(combos)
TypeError: 'function' object is not iterable

[RedBearAK]

Phew. 😓

Managed to fix it with a global variable that gets set inside unicode_keystrokes and an additional function call to unicode_keystrokes from the string processor. Although I'm not entirely sure why the additional function call () was necessary. But without it, nothing shows up in the global variable for the string processor to use.

        global unicode_combo_list

        for c in s:
            if ord(c) > 127:
                unicode_keystrokes(ord(c))()
                combo_list.extend(unicode_combo_list)

Anyway, as far as I can tell everything is working again. Embedded Unicode characters, extended ASCII, or either of the Unicode Python notations in a string are all coming out like they should.

ST("CapsLock 🌹 € ÿ \u2021 \U00002021 double-tapped.")
CapsLock 🌹 € ÿ ‡ ‡ double-tapped.

Now I just need some kind of method for checking the CapsLock state, and it shouldn't take long to wrap up the fix. Maybe I can just do some sort of tripwire keymap like the one in the Option-key special character setup, that's constantly calling a function that's checking the state and setting a global variable.

[RedBearAK]

Well, I'll have to come back to this and make sure everything is really nailed down, but it works. It's still a bit of a hack because it requires a "tripwire" keymap to send the context object into a new function in config_api, so that it can update a variable inside config_api to store the CapsLock state for each key press.

At first I tried to just put the function in my config file, but that didn't work to update the variable in a way that the processor functions would have access to the state of the variable. Apparently you can do this between modules, but I couldn't make it work that way.

Results are looking good:

String: 
ST("CapsLock 🌹 € ÿ \u2021 \U00002021 12345 !@#$% double-tapped.")

With CapsLock OFF:
CapsLock 🌹 € ÿ ‡ ‡ 12345 !@#$% double-tapped.

With CapsLock ON:
CapsLock 🌹 € ÿ ‡ ‡ 12345 !@#$% double-tapped.

I did decide to just go with reversing the use of "Shift" for the letters in strings, while the Unicode processor disables CapsLock and then re-enables. This way there is really no conflict between the two functions, whether you use the Unicode processor directly or access it by proxy through the string processor. Works either way.

Also had to split up the isalnum elif into isdigit and isalpha, so there's one more decision fork in the for loop for strings. CapsLock doesn't affect numbers and their shifted characters.

The tripwire when conditional is more complex than I was hoping it would be, but the important thing is that it works.

keymap("Tripwire to keep track of CapsLock state", {
    # Nothing needed inside, runs a function from "when" condition for each key press.
}, when = lambda ctx: store_capslock_state()(ctx))

And the function inside config_api:

capslock_is_on = False

def store_capslock_state():
    def _store_capslock_state(ctx):
        global capslock_is_on
        capslock_is_on = ctx.capslock_on
    return _store_capslock_state

[RedBearAK]

@joshgoebel

OK, even though at the moment this requires the tripwire keymap "hack" in the user's config file to actually do anything meaningful, I'm going to clean up the code changes to config_api (remove the extra debugging output) and submit it as a PR, because:

It's not harmful if nothing is updating the variable

The changes, if there is no tripwire keymap (or other method) sending the context into the global variable in config_api, will simply operate exactly as the current code operates. If CapsLock is OFF, everything will be fine, output normal. If CapsLock is ON and there is no tripwire, the output will be wrong in exactly the same way it is right now. So the situation will be no worse.

capslock_is_on = False

The variable is just initiated as False, so the current "CapsLock is OFF" behavior will result if nothing is touching the variable.

It makes changes that are necessary to fix the issue

The main thing is that the code changes convert the existing simple functions that were just returning keystroke lists, into compound functions that return active inner functions that will stay in memory, and can now adapt to the state of the global variable that will mirror the CapsLock state. It will work no matter what kind of method is developed in the future to update that variable, with or without the use of the tripwire keymap.

Also it implements the logic changes so that the "shift reversal" only applies to alphabetical keys that need it, and doesn't touch the numeric/digit characters. There is no conflict with the operation of the Unicode function and its need to disable the CapsLock ON state before issuing the Shift+Ctrl+u shortcut.

Doesn't harm troubleshooting too much

All the keystrokes that are created by the processing functions will still show as debugging output keystrokes n the log, so there really isn't much of an issue created by returning "opaque" function objects rather than lists. There wasn't really any logging going on with the existing simple functions anyway. Debugging output can always be added back into the inner processing functions if a problem crops up.

Future enhancement

I'm envisioning some part of the code, where the context is already available, calling on the store_capslock_state() function in config_api for every key press, thus keeping the global variable updated without the need for the tripwire keymap. The tripwire is really just a proof of concept to make sure all the logic changes in the processor functions are actually working, and that all seems to work perfectly.

My Option-key special characters now come out the same with CapsLock on or off. With the current code you'd just see the raw Unicode address for each character on a new line when CapsLock was on.

Option with CapsLock OFF:
œ ∑ é ® † ¥
Shift+Option with CapsLock OFF:
Œ „ ´ ‰ ˇ Á

Option with CapsLock ON:
œ ∑ é ® † ¥
Shift+Option with CapsLock ON:
Œ „ ´ ‰ ˇ Á

[joshgoebel]

Oh ,it's only key context, not state - it doesn't include WHICH key is pressed... I'm ok with that falling thru to the commands... so you'd really only need something like this I think...

   1   │ diff --git a/src/keyszer/transform.py b/src/keyszer/transform.py
   2   │ index 5002b31..8f8dc43 100644
   3   │ --- a/src/keyszer/transform.py
   4   │ +++ b/src/keyszer/transform.py
   5   │ @@ -462,7 +462,7 @@ def transform_key(key, action, ctx):
   6   │              if not _output.is_mod_pressed(ks.key):
   7   │                  ks.spent = True
   8   │          debug("spent modifiers", [_.key for _ in held if _.spent])
   9   │ -        reset_mode = handle_commands(keymap[combo], key, action, combo)
  10   │ +        reset_mode = handle_commands(keymap[combo], key, action, combo, ctx)
  11   │          if reset_mode:
  12   │              _active_keymaps = None
  13   │          return
  14   │ @@ -525,7 +525,7 @@ def auto_sticky(combo, input_combo):
  15   │  # ─── COMMAND PROCESSING ───────────────────────────────────────────────────────
  16   │
  17   │
  18   │ -def handle_commands(commands, key, action, input_combo=None):
  19   │ +def handle_commands(commands, key, action, input_combo=None, ctx):
  20   │      """
  21   │      returns: reset_mode (True/False) if this is True, _active_keymaps will be reset
  22   │      """
  23   │ @@ -548,7 +548,7 @@ def handle_commands(commands, key, action, input_combo=None):
  24   │          for command in commands:
  25   │              if callable(command):
  26   │                  # very likely we're just passing None forwards here but that OK
  27   │ -                reset_mode = handle_commands(command(), key, action)
  28   │ +                reset_mode = handle_commands(command(ctx), key, action, None, ctx)
  29   │                  # if the command wants to disable reset, lets propagate that
  30   │                  if reset_mode is False:
  31   │                      return False

So every command function gets the state passed to it - just like conditionals.

[RedBearAK]

I figured something relatively simple like this would allow it to work. I was about to start trying something similar, but this is extremely helpful.

So every command function gets the state passed to it - just like conditionals.

Just guessing at the moment, but that means this should allow the context to be accessed in the processor functions without needing the new CapsLock LED status function and the global variable, correct? (In other words, I shouldn't need the tripwire keymap anymore?) I'll have to try it to be sure I actually understand what's happening here. But it looks very promising.

[joshgoebel]

Right, the internal state is just passed directly to commands - the state is maintained in the engine, not a global.

[RedBearAK]

VSCode is showing an error here, "Non-default argument follows default argument" from Pylance. Not sure if that actually has any real consequences, but the linting definitely doesn't like it.

def handle_commands(commands, key, action, input_combo=None, ctx):

Should be fine to move the default argument to the end, right?

Right, the internal state is just passed directly to commands - the state is maintained in the engine, not a global.

Thought so. Sounds good. There's definitely no need to know what key is being pressed. Just whether CapsLock is ON or OFF.

[joshgoebel]

Yeah, need to change the order.

[RedBearAK]

Yeah, need to change the order.

No problem.

But there is an exception when I try to run this.

Exception in callback receive_input(InputDevice('...input/event4')) at /home/kris/Documents/keyszer/src/keyszer/input.py:102
handle: <Handle receive_input(InputDevice('...input/event4')) at /home/kris/Documents/keyszer/src/keyszer/input.py:102>
Traceback (most recent call last):
  File "/usr/lib64/python3.10/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/kris/Documents/keyszer/src/keyszer/input.py", line 117, in receive_input
    on_event(event, device)
  File "/home/kris/Documents/keyszer/src/keyszer/transform.py", line 334, in on_event
    on_key(ks, context)
  File "/home/kris/Documents/keyszer/src/keyszer/transform.py", line 414, in on_key
    transform_key(key, action, context)
  File "/home/kris/Documents/keyszer/src/keyszer/transform.py", line 482, in transform_key
    reset_mode = handle_commands(keymap[combo], key, action, combo, ctx)
  File "/home/kris/Documents/keyszer/src/keyszer/transform.py", line 574, in handle_commands
    auto_sticky(command, input_combo)
  File "/home/kris/Documents/keyszer/src/keyszer/transform.py", line 536, in auto_sticky
    _sticky = simple_sticky(input_combo, combo)
  File "/home/kris/Documents/keyszer/src/keyszer/transform.py", line 502, in simple_sticky
    inmods = combo.modifiers
AttributeError: 'KeyContext' object has no attribute 'modifiers'

[joshgoebel]

You've got the param order wrong likely...

[RedBearAK]

I think this helped, but now I get other errors, including an error with my custom isDoubleTap() function, which is weird.

        reset_mode = handle_commands(keymap[combo], key, action, ctx, combo)

I'll have to update the processor functions to accept the "ctx" and see what happens.

[RedBearAK]

OK, that was a little bit complicated, but everything seems to be working, at least not having errors. Still need to actually use the "ctx" properly and see if that works.

A possible complication is that I had to add a "ctx" parameter to the inner function of my isDoubleTap, which is just a new custom function sitting in my config, to get it to stop complaining about receiving 1 positional argument when it expected 0 positional arguments.

Pretty sure this means that any custom function the user makes will now have to accept the "ctx" being passed to it, and the user will have to understand that from this relatively generic error message (and follow the traceback info):

TypeError: isDoubleTap.<locals>._isDoubleTap() takes 0 positional arguments but 1 was given

There is the traceback but a lot of users will find it difficult to actually understand how to fix the error.

Just trying to think if there is a way to make the usage of "ctx" in custom functions optional, or if this is the price that must be paid...

[joshgoebel]

Oh I wonder if that makes the commands REQUIRE an argument? If so that's annoying and we'd need to analyze them first to find out if they want an arg or not.

[RedBearAK]

Yeah, I think the command(ctx) makes it required for every command, and if I understand things correctly you could make the receiving function treat the parameter as optional on the receiving end with notation like my_function(ctx=None), but the parameter would still need to be designated in the parameter list for every custom function or the "too many arguments given" would always happen.

It's kind of fascinating that my whole config file didn't immediately blow up because the matchProps() function that's running everything now was always operating from within the when conditional and accepting ctx in the inner function anyway. At least now I finally understand where that context was originally coming from.

Anyway, looks like we have to dig into using something like inspect.signature to check what the function "wants" and avoid passing ctx if the function can't receive it? I can try to deal with that, but it will take some time. I've only tried to use inspect a couple of times for debugging.

[RedBearAK]

I’m kind of taking a break but started wondering if this could actually be as simple as just doing a try/except fork to catch the positional argument error and send the contextless version when there is an error.

Edit: Seems to work OK. Still getting context inside of the processing functions, but was able to remove the parameter from isDoubleTap() and it still works.

        for command in commands:
            if callable(command):
                # very likely we're just passing None forwards here but that OK
                try:
                    reset_mode = handle_commands(command(ctx), key, action, ctx)
                except:
                    reset_mode = handle_commands(command(), key, action, ctx)

🤞🏽 Fingers crossed that this is actually a good solution and all that needs to be done.

Oops, gotta remember to except only the specific error and not leave a generic except statement.

Still seems to work OK.

            if callable(command):
                # very likely we're just passing None forwards here but that OK
                try:
                    reset_mode = handle_commands(command(ctx), key, action, ctx)
                except TypeError:
                    reset_mode = handle_commands(command(), key, action, ctx)

[joshgoebel]

I think it's just an if/else around something like len(inspect.signature(command_fn).parameters)

[RedBearAK]

I think it's just an if/else around something like len(inspect.signature(command_fn).parameters)

Is there an advantage to that over the try/except block? More reliable for this specific issue?

That just shows the length (count) of the parameters, which is fine if it's zero. But if it's one or more I'll need to do a check for "ctx" being in the parameter list that comes out:

OrderedDict([('ctx', <Parameter "ctx">)])

I'll have to play around with this a bit.

[joshgoebel]

What would the purpose of any other arguments be? You can use closures to pass any values from an outer wrapper... so a command fn would either only take 0 params, or 1 context param, nothing else. No?

[RedBearAK]

What would the purpose of any other arguments be? You can use closures to pass any values from an outer wrapper... so a command fn would either only take 0 params, or 1 context param, nothing else. No?

Mmm... By "command fn" do you mean the inner functions? I haven't really grasped yet why the inner functions are the only ones that receive the "ctx" parameter. I did notice in the past that it couldn't be passed in via the outer function. My matchProps() has like a dozen parameters, but those are all in the outer function definition, with only "ctx" as a parameter on the inner function. So, I guess this may be overkill? (It does work at least.)

        for command in commands:
            if callable(command):
                # very likely we're just passing None forwards here but that OK
                cmd_param_cnt = len(inspect.signature(command).parameters)
                cmd_param_lst = inspect.signature(command).parameters
                ctx_param_ptn = '(\'ctx\', <Parameter "ctx">)'
                ctx_param_rgx = re.search(ctx_param_ptn, str(cmd_param_lst))

                if cmd_param_cnt == 0 or not ctx_param_rgx:
                    reset_mode = handle_commands(command(), key, action, ctx)
                elif ctx_param_rgx:
                    reset_mode = handle_commands(command(ctx), key, action, ctx)

I thought maybe someone at some point would have some sort of reason to have more than one parameter in an inner function, but if that's never going to be the case I can pare this down to just the len() check...

[RedBearAK]

Alright, if you think this will be good enough, I'll finish modifying the processor functions to work with ctx directly, and then put the changes to both files together into a PR. Should completely solve the problem end-to-end.

        for command in commands:
            if callable(command):
                # very likely we're just passing None forwards here but that OK
                cmd_param_cnt = len(inspect.signature(command).parameters)
                if cmd_param_cnt == 0:
                    reset_mode = handle_commands(command(), key, action, ctx)
                else:
                    reset_mode = handle_commands(command(ctx), key, action, ctx)

[RedBearAK]

@joshgoebel

Submitted PR #129. Let me know if you see some sort of problem with it, or want something changed.

Oh, and thanks for the help and suggestions. Really made this a lot easier.

[RedBearAK]

Bumping this. As far as I know, the fix is complete and working perfectly. See something that should be done differently?

[RedBearAK]

This should be resolved with the merge of PR #129. Closing.

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 👏🏽