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

stop processing key events twice on windows (#71) #73

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions src/ui/input_handler.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use anyhow::Result;
use crossterm::event::{poll, read, Event, KeyCode, KeyEvent, KeyModifiers};
use crossterm::event::{poll, read, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers};
use std::time::Duration;

use crate::app::Application;
Expand Down Expand Up @@ -38,12 +38,19 @@ impl InputHandler {
if poll(poll_timeout)? {
let read_event = read()?;
if let Event::Key(key_event) = read_event {
match self.input_mode {
InputMode::Normal => self.handle_key_in_normal_mode(key_event, app),
InputMode::TextInsertion => {
self.handle_key_in_text_insertion_mode(key_event, app)
// The following line needs to be amended if and when enabling the
// `KeyboardEnhancementFlags::REPORT_EVENT_TYPES` flag on unix.
let event_kind_enabled = cfg!(target_family = "windows");
let process_event = !event_kind_enabled || key_event.kind != KeyEventKind::Release;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we simply do this? I don't have a Windows machine to test this unfortunately.

let process_event = key_event.kind != KeyEventKind::Release;

Copy link
Contributor Author

@TeFiLeDo TeFiLeDo Feb 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be fine for windows, but I don't know about other platforms.

The crossterm documentation seems to suggest that the field should only be read:

If searched through the igrep code, but did not find any code enabling that flag. I could not find any default value for that field when the flag is disabled. Given that the mentioned issue also is about windows, I did not want to change the behavior on other platforms.

I could change the variable name to check_event_kind if you think that is more readable. Currently it is basically a weird form of conditional compilation. However, if someone enables that flag later on they only need to change that line to be on par with windows.


if process_event {
match self.input_mode {
InputMode::Normal => self.handle_key_in_normal_mode(key_event, app),
InputMode::TextInsertion => {
self.handle_key_in_text_insertion_mode(key_event, app)
}
InputMode::Keymap => self.handle_key_in_keymap_mode(key_event, app),
}
InputMode::Keymap => self.handle_key_in_keymap_mode(key_event, app),
}
}
}
Expand Down
Loading