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

Wrong KeyEvent reported to yazi #917

Open
SchuhBaum opened this issue Aug 22, 2024 · 5 comments
Open

Wrong KeyEvent reported to yazi #917

SchuhBaum opened this issue Aug 22, 2024 · 5 comments

Comments

@SchuhBaum
Copy link

Describe the bug
See sxyazi/yazi#1534.
It seems that crossterm reports ctrl+alt+\ instead of just \ when pressing ctrl+alt+ß on a german keyboard. This creates an issue because yazi destincts between \ and ctrl+alt+\.

To Reproduce
See sxyazi/yazi#1534.
Let me know if there is a way to check that indeed this dependency is at fault. The reported steps from the link use yazi.

Expected behavior
The keybinding ctrl+alt+ß is translated to the key event with KeyCode::Char('\\') without modifiers.

OS
Windows

Terminal/Console
Powershell

@joshka
Copy link
Collaborator

joshka commented Aug 22, 2024

See #685

@joshka joshka closed this as completed Aug 22, 2024
@joshka
Copy link
Collaborator

joshka commented Aug 24, 2024

I'm going to reopen this one after doing a bit more reading as it's a little more complex than just a missing key.

On a German keyboard layout (I'm probably assuming / generalizing here), there is a third layer of keys on the standard keyboard which are accessed via either the AltGr key (which is often the right Alt button), or the Ctrl+Alt key combination on keyboards that don't do automatic AltGr translation.

On the layout, the ß key is to the right of the 0 key on the keyboard. This key has a \ in its third layer.
So while both the control and alt keys are held down, these are used to instead select the layer not to modify the key.

This likely is something that would only be fixed by implementing a fairly deep fundamental change to interacting with the console in crossterm using virtual terminal methods instead of the more legacy low level methods. I'm not sure this would be reasonable to implement as a change in crossterm, but it's worth discussing. The main problem would be that the new methods don't continue to return the window size events etc, but they would handle stripping AltGr / Alt+Ctrl automatically.

@joshka joshka reopened this Aug 24, 2024
@SchuhBaum
Copy link
Author

How feasible is it to just throw away AltGr / Ctrl+Alt when the detected code isn't a control code (see the TODO section below)?
Or to add a KeyCode category or some variable to the key event that says, hey we found a control code.

_ => {
            let utf16 = key_event.u_char;
            match utf16 {
                0x00..=0x1f => {
                    log_to_file(&format!("TEMP: Case 1 (control code or no u_char)."));

                    // Some key combinations generate either no u_char value or generate control
                    // codes. To deliver back a KeyCode::Char(...) event we want to know which
                    // character the key normally maps to on the user's keyboard layout.
                    // The keys that intentionally generate control codes (ESC, ENTER, TAB, etc.)
                    // are handled by their virtual key codes above.
                    get_char_for_key(key_event).map(KeyCode::Char)
                }
                surrogate @ 0xD800..=0xDFFF => {
                    return Some(WindowsKeyEvent::Surrogate(surrogate));
                }
                unicode_scalar_value => {
                    log_to_file(&format!("TEMP: Case 3 (unicode scalar value)."));

                    // TODO: Throw away ctrl+alt modifiers if present???
                    // Context: Ctrl+alt indicate control codes. But on some keyboard layouts they
                    //          can be used to access a third layer of characters instead.
                    //          This way the returned key event can be used to infer if case 1 or 3 is
                    //          present.

                    // This KeyCode can differ from the returned KeyCode.
                    log_to_file(&format!("TEMP: {:?}", get_char_for_key(key_event).map(KeyCode::Char)));

                    // Unwrap is safe: We tested for surrogate values above and those are the only
                    // u16 values that are invalid when directly interpreted as unicode scalar
                    // values.
                    let ch = std::char::from_u32(unicode_scalar_value as u32).unwrap();
                    Some(KeyCode::Char(ch))
                }
            }
        }

@joshka
Copy link
Collaborator

joshka commented Aug 24, 2024

Not feasible at all. Ctrl+Alt+\ is a perfectly reasonable shortcut (which requires holding Fn as well on a Mac).
There's nothing really that would disambiguate good modifiers from bad (except the locale). Are there characters on the keyboard you'd expect to be interpreted as Ctrl+Alt rather than the third layer key?

Running the event-read example:

Event: Key(KeyEvent { code: Char('4'), modifiers: KeyModifiers(CONTROL), kind: Press, state: KeyEventState(0x0) })
Event: Key(KeyEvent { code: Char('\\'), modifiers: KeyModifiers(CONTROL | ALT), kind: Press, state: KeyEventState(0x0) })

@SchuhBaum
Copy link
Author

Are there characters on the keyboard you'd expect to be interpreted as Ctrl+Alt rather than the third layer key?

Yes. There are many keys without a third layer. Ctrl+Alt needs to do both.

(I don't really want to argue about details. Whatever works is fine. You can ignore this:
The copied source code was from the Windows branch. Assuming Windows is used. There is a match utf16 statement with three mutually exclusive cases. The KeyEvent is not created yet. Your example would trigger the first case (i.e. control code or no u_char) and not the TODO case.)

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

No branches or pull requests

2 participants