EarTrumpet
EarTrumpet copied to clipboard
[wip] Allow Windows key in shortcuts (Discussion #703)
Took a crack at #703. Wasn't sure if I should open a separate issue since there's already a request in discussions so sending this PR
The main wrinkle with using the Windows key as a modifier is that it's not actually one according to the Windows API. This means that it doesn't have a nice value to use as a bitmask like Keys.Control
, Keys.Alt
, and Keys.Shift
do. In fact there's a warning in the System.Windows.Forms.Keys
docs about this:
Do not use the values in this enumeration for combined bitwise operations. The values in the enumeration are not mutually exclusive.
Looking at the actual values on that page, the modifiers' bits won't overlap with anything else while LWin
and RWin
overlap with many other keys:
Key Dec Hex
--------------------------
Control 131072 0x20000
Alt 262144 0x40000
Shift 65536 0x10000
LWin 91 0x0005B
RWin 92 0x0005C
A 65 0x00041
B 66 0x00042
C 67 0x00043
(etc.)
This means HotkeyData.ToString
needs to be careful not to bitwise or a non-modifier key with LWin
or RWin
when calling KeysConverter.ConvertToString
.
For some reason that I haven't managed to figure out, KeysConverter.ConvertToString(Keys.Control)
would return "Control+None"
and make the strings in the settings page ugly. I removed the extra "+None"
but I'm not sure why it was doing that in the first place.
Also, since there's no combined Win
key, I decided to handle RWin
as if it were LWin
. They could be split out but I felt like that wasn't a good idea because it breaks consistency with Ctrl
/Alt
/Shift
. Leaving the string in the settings page with "LWin"
so that it gets localized by KeysConverter
even though it also could represent the RWin
key.
@iamevn Can you re-target this PR to dev
branch and rebase?
Thanks for taking a look! I think I accidentally clicked something wrong in github and it hid your review comment but I rebased onto current dev and it looks right to me.
Thanks!