Keybindings config character uppercase

[xsebek]

I accidentally wrote a keybinding in uppercase, like Ctrl-T, and it turns out this means it parses successfully, with no errors or warnings, but simply does not work --- both Ctrl-t and Ctrl-Shift-t do nothing. I suspect it turns into a keybinding which is impossible to ever see, because Ctrl-Shift-t is not the same as Ctrl-T. Well, I don't know, perhaps it is possible to get Ctrl-T by turning on caps lock and then typing Ctrl-t? In any case it seems like some kind of warning here would be nice.

@byorgey in swarm-game/swarm#1979 (comment)

---

I am also confused by the behavior. Neither the Brick nor Vty documentation specifies what the case should be.

I suspect this code:

brick/src/Brick/Keybindings/Parse.hs

Lines 126 to 128 in 79c9317

	pKey t
	| T.length t == 1 =
	return (Vty.KChar $ T.last s)

Should use the lowercase t instead of the original s text:

        pKey t
          | [c] <- T.unpack t = return (Vty.KChar c)

Either way, I tried configuring a keybinding with Ctrl and Shift and could not get it to run:

; these do not seem to work
pause = S-C-P,S-C-p,C-P,S-P,S-p
; without shift it works
pause = C-p

[jtdaugherty]

Thanks for this report!

It turns out that it appears impossible to get an event from the terminal with an uppercase character when modifiers are present, such as Ctrl-T. Shift is not generally even recognized as a discrete modifier, so I'm frankly not sure why that's even in vty. (It's probably been there since long before I took over maintenance.)

The parser should normalize the character to lowercase whenever modifiers are present. So the patch is a little different; the first pKey case needs to conditionally normalize the key character to lowercase only if mods is non-empty. That normalization step probably needs to happen in at least one other place, too. I'll work on a patch.

[jtdaugherty]

The patch is now on master - 6929129. Care to give it a try?

[xsebek]

@jtdaugherty works for me - the uppercase versions now conflict with the lowercase ones. 👍

I am surprised that shift is not recognized as a modifier. Maybe it is recognized only on some terminals/platforms?
But that is a vty issue and it is not as important. 🙂