Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support key release events for windows. #745

Merged
merged 5 commits into from
Jan 28, 2023

Conversation

TimonPost
Copy link
Member

@TimonPost TimonPost commented Jan 11, 2023

@matkaas
Copy link
Contributor

matkaas commented Jan 13, 2023

Hi @TimonPost

I rather moved it than added it. The check used to happen here:
0fcce0d#diff-0fc06b629fbd961b93f1888ab8546e9089e7359747d2fbe9e8b7aabd30983603L35

And I moved it where you linked:
0fcce0d#diff-0fc06b629fbd961b93f1888ab8546e9089e7359747d2fbe9e8b7aabd30983603R115

The reason I moved it one function call further in was that I needed to check the (Alt) key release for alt+numpad key combinations:
0fcce0d#diff-0fc06b629fbd961b93f1888ab8546e9089e7359747d2fbe9e8b7aabd30983603R94

I see no problem with removing it in principle, but beware that the alt+numpad key events from the terminal are sorta weird. The generated character gets delivered as part of the Alt key release event. I don't think the case here:
https://github.com/crossterm-rs/crossterm/pull/745/files#diff-0fc06b629fbd961b93f1888ab8546e9089e7359747d2fbe9e8b7aabd30983603R224
will ever happen for key_down == true.
(In particular also because we have a check for that in the if-expression of the block 0fcce0d#diff-0fc06b629fbd961b93f1888ab8546e9089e7359747d2fbe9e8b7aabd30983603R96).

Since the alt+numpad character events are synthetically generated, they don't really have a press+release event pair, so it's hard to say how that is best handled. Perhaps synthesize both the press and release at the same time? Or simply accept that the character only gets a release event. It's an intrinsic problem anyhow that press+release events are not guaranteed to be delivered to applications in pairs, e.g. if a user presses down a key and moves focus away from the window before releasing the key, so that the press and release events go to two different windows. I also think the keyboard repeat function (holding down a key to repeatedly trigger it) will deliver several press events and one final release event, so not in pairs. I'm not sure if that applies to terminal applications though, since they're really a text-based protocol.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support key release events
2 participants