ahk icon indicating copy to clipboard operation
ahk copied to clipboard

Consider adding a curly bracket ("{}") check for the Key class

Open hiroshi-ya opened this issue 1 year ago • 2 comments

I'm working on a project that allows its users to customize their key configurations. Here's a snippet:

def move_forward(forwardKey : str) # forwardKey is a valid key
  ahk.send(forwardKey)
  ahk.key_down(forwardKey)
  sleep(1)
  ahk.key_up(forwardKey)

Since key strings such as 'up' and 'shift' need curly brackets ({}) when passed to ahk.send(), I simply give a pair of curly brackets to the forwardKey string no matter what they are (e.g. 'a' becomes '{a}', 'up' becomes '{up}').

Problem is, the Key.DOWN/UP property also automatically adds a pair of curly brackets, e.g.:

from ahk import AHK, keys
k = keys.Key(key_name='{up}')
print(k.DOWN) # prints {{up} down}

So, in the move_forward() function,

  1. if forwardKey == 'up', then ahk.send(forwardKey) will not work as expected (sends 'u' and 'p' instead of the up arrow key);
  2. if forwardKey == '{up}', then ahk.key_down(forwardKey) will not work because it's equivalent to ahk.send('{{up} down}').

This PR aims to fix this issue - probably not very gracefully :/

hiroshi-ya avatar Feb 22 '24 17:02 hiroshi-ya

Coverage Status

coverage: 76.247% (-0.009%) from 76.256% when pulling ac9a2ad51a2b5ab617bc712001a7f61d5cec906f on hiroshi-ya:Key.DOWN/UP-bracket-check into f2d38c90c789f44ac6e59eb4556e31708ab3365f on spyoungtech:main.

coveralls avatar Feb 22 '24 17:02 coveralls

Thanks for this. Yeah, there's some known issues like this around the keys module when used in various ways. It's not documented in the README partially for this reason. For example, it may produce incorrect strings for use as hotkey triggers.

Likely, the keys module will be rewritten to support a more well-defined usage across various use cases.

In the meantime, we can of course improve the current implementation. I'll submit a small review and will be happy to merge with those changes.

spyoungtech avatar Feb 24 '24 02:02 spyoungtech