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

Adds checks for key 'press' on Windows for keyboard events #4

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

luqmanishere
Copy link
Contributor

crossterm emits events upon keyboard press and release, which causes duplicate events for the same key on Windows.

This PR adds checks for only a key press so only 1 event is returned on Windows.

I've yet to test this change on Linux, however it should not break as KeyEventKind is not read on Linux (bar special flags) and defaults to Press anyways.

Crossterm emits events upon keyboard press and release, which
causes duplicate events for the same key.
@TheEmeraldBee
Copy link
Owner

Thank you for writing this, I hadn't caught this as I write on mac.

This is great, however, we need to also check for repeat, as that can be emitted as well.

If you fix this, I can pull it in and release a tag!

@luqmanishere
Copy link
Contributor Author

Thanks for the reply. As I understand it, repeated key inputs (by holding it down) should work just fine since on Unix systems, it would just be repeated key presses. The Repeat and Release kinds on Unix is only used when the special flag is enabled via:

execute!(
    stdout,
    PushKeyboardEnhancementFlags(KeyboardEnhancementFlags::REPORT_EVENT_TYPES)
);

And only a select few terminals support this flag, notably kitty being the source of this protocol. Repeat (held down) inputs should be shown as KeyEventKind::Press over multiple queries to crossterm's event::read()

On Windows holding down the key works as expected (repeated KeyEventKind::Press), which can be tested with the custom_set example or any of crossterm's event reading examples.

Please advise me if there are any other changes needed.

@TheEmeraldBee
Copy link
Owner

Yep, wasn't sure how it worked on windows, in that case, thank you! I'll pull that in as soon as I get a chance to look at it closer.

@TheEmeraldBee TheEmeraldBee merged commit a6ef9d5 into TheEmeraldBee:master Apr 10, 2024
1 check passed
@TheEmeraldBee
Copy link
Owner

Implemented in version 0.5.1

@luqmanishere luqmanishere deleted the windows-duplicate branch April 10, 2024 17:10
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.

2 participants