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

Receiving release Enter command upon application start. #752

Open
C-BJ opened this issue Feb 3, 2023 · 8 comments · May be fixed by #778
Open

Receiving release Enter command upon application start. #752

C-BJ opened this issue Feb 3, 2023 · 8 comments · May be fixed by #778

Comments

@C-BJ
Copy link

C-BJ commented Feb 3, 2023

Describe the bug
We had a strange problem developing REPL for the Erg language: erg-lang/erg#362

When we run a program through the terminal, we get an Enter event first, and then whatever we type(type only once), it gets two of these input events

This bug only appears at 0.26.0, not 0.25.0
This issue only appears on Windows (not WSL)

OS

  • Windows10 / 11
@aschey
Copy link
Contributor

aschey commented Feb 3, 2023

Looks like this was caused by this PR: #745 and the fix is to check the KeyEventKind first:

if let Event::Key(key) = event {
    if key.kind == KeyEventKind::Press {
        ...
    }
}

@TimonPost
Copy link
Member

TimonPost commented Feb 3, 2023

Thats possible, does it have the KeyEvent:::kind set?

@dsherret
Copy link
Contributor

dsherret commented Mar 11, 2023

This bug only occurs in crossterm 0.26.0 and above (I found this issue while creating my bug, so posting my template below):

To Reproduce

pub fn main() {
  crossterm::terminal::enable_raw_mode().unwrap();
  let result = crossterm::event::read();
  eprintln!("RESULT: {:?}", result);
}

This will immedately return the enter key press:

> cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target\debug\rust-test.exe`
RESULT: Ok(Key(KeyEvent { code: Enter, modifiers: NONE, kind: Release, state: NONE }))

Expected behavior

It should wait for a key press.

OS
Windows 10.0.19045 Build 19045

Terminal/Console

Windows terminal

@lesleyrs
Copy link
Contributor

lesleyrs commented Apr 6, 2023

I just did some testing on various terminals and shells and the incorrect enter "release" event does not appear on command prompt as long as you are using conhost instead of wezterm or windows terminal.

No clue why this happens, needs to be looked in to. Powershell on conhost has the release event on launch as well.

Edit: it's quite funny how command prompt on conhost is the only one that actually works perfectly with the modern input. Windows terminal is the least functional right now in terms of input microsoft/terminal#8440.

@TimonPost
Copy link
Member

TimonPost commented Apr 8, 2023

Interesting finding! So this issue seems to have to things at play:

  • The release event kind introduced in 0.26 was unexpected and caused some confusion.
  • There is an enter key code being send Ok(Key(KeyEvent { code: Enter, modifiers: KeyModifiers(0x0), kind: Release, state: KeyEventState(0x0) }))

I think I will disable the first feature by default as its not cross platform functionality, rather only for kitty and windows at the moment. So it should likely be an opt in over an default feature.

The second issue was introduced by #745 where I removed:

  if !key_event.key_down {
        return None;
    }

I wasn't sure what the purpose of this was but likely this was the reason it exited in the code. It early returns if the key was down whereafter it does a check on virtual key codes. I think it might be to do with the event press when you start the application from the command line and press enter to run the command.

@TimonPost TimonPost changed the title Strange input event on windows Receiving release Enter command upon application start. Apr 8, 2023
@TimonPost TimonPost linked a pull request Apr 8, 2023 that will close this issue
@lesleyrs
Copy link
Contributor

lesleyrs commented Apr 8, 2023

Does kitty protocol actually make release events work on linux? I thought it was only used for extra modifier keys.

Last time when I disabled win32 input and enabled kitty in wezterm there were no releases.

@TimonPost
Copy link
Member

TimonPost commented Apr 8, 2023

You also have to enable the KeyboardEnhancementFlags::REPORT_EVENT_TYPES in order for the kitty protocol to fire them. And it might be that not every kitty protocol implementation has this correctly implemented.

see: https://sw.kovidgoyal.net/kitty/keyboard-protocol/#key-codes

@lesleyrs
Copy link
Contributor

lesleyrs commented Apr 9, 2023

@TimonPost It seems wezterm got it fixed just today lol wez/wezterm#3220 (comment)

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 a pull request may close this issue.

5 participants