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

Characters from keys with modifiers are ignored in input fields #1534

Closed
SchuhBaum opened this issue Aug 22, 2024 · 14 comments
Closed

Characters from keys with modifiers are ignored in input fields #1534

SchuhBaum opened this issue Aug 22, 2024 · 14 comments
Labels
bug Something isn't working upstream

Comments

@SchuhBaum
Copy link

SchuhBaum commented Aug 22, 2024

What system are you running Yazi on?

Windows

What terminal are you running Yazi in?

Powershell

yazi --debug output

Yazi
    Version: 0.3.1 (VERGEN_IDEMPOTENT_OUTPUT 2024-08-22)
    Debug  : true
    OS     : windows-x86_64 (windows)

Ya
    Version: 0.3.1 (VERGEN_IDEMPOTENT_OUTPUT 2024-08-22)

Emulator
    Emulator.via_env: ("", "")
    Emulator.via_csi: Ok(Unknown([]))
    Emulator.detect : Unknown([])

Adapter
    Adapter.matches: Chafa

Desktop
    XDG_SESSION_TYPE: None
    WAYLAND_DISPLAY : None
    DISPLAY         : None

SSH
    shared.in_ssh_connection: false

WSL
    /proc/sys/fs/binfmt_misc/WSLInterop: false

Variables
    SHELL              : None
    EDITOR             : None
    VISUAL             : None
    YAZI_FILE_ONE      : None
    YAZI_CONFIG_HOME   : None

Text Opener
    default: Some(Opener { run: "nvim \"%*\"", block: true, orphan: false, desc: "nvim", for_: None, spread: true })
    block  : Some(Opener { run: "nvim \"%*\"", block: true, orphan: false, desc: "nvim", for_: None, spread: true })

Multiplexers
    TMUX               : false
    tmux version       : program not found
    ZELLIJ_SESSION_NAME: None
    Zellij version     : program not found

Dependencies
    file             : 5.45
    ueberzugpp       : program not found
    ffmpegthumbnailer: program not found
    magick           : program not found
    fzf              : 0.51.0
    fd               : 10.1.0
    rg               : 14.1.0
    chafa            : program not found
    zoxide           : 0.9.2
    7z               : 24.05
    7zz              : program not found
    jq               : program not found


--------------------------------------------------
When reporting a bug, please also upload the `yazi.log` log file - only upload the most recent content by time.
You can find it in the "C:\\Users\\andre\\AppData\\Roaming\\yazi\\state" directory.

Did you try the latest nightly build to see if the problem got fixed?

No, and I'll explain why below

Describe the bug

Characters that require modifiers (like ctrl, alt) are ignored in input fields like "cd --interactive". The german keyboard layout requires modifiers for some characters. Examples are @ (ctrl+alt+q), \ (ctrl+alt+ß) and ~ (ctrl+alt++). These characters are filtered out when pasting text as well ("C:\hello" becomes "C:hello").

Minimal reproducer

  1. Use a keyboard with german layout. (Or maybe just change the keyboard layout in Windows. The key ß is located in the number row between 0 and ´.)
  2. Open cd --interactive by pressing g<space>.
  3. Press ctrl+alt+ß for the character \.
  4. Notice that no character is inserted and the input field stays empty.

Anything else?

No response

@SchuhBaum SchuhBaum added the bug Something isn't working label Aug 22, 2024
@sxyazi
Copy link
Owner

sxyazi commented Aug 22, 2024

Please apply this patch and build Yazi in debug mode, then check the log file to see what key was reported

--- yazi/yazi-fm/src/app/app.rs
+++ yazi/yazi-fm/src/app/app_new.rs
@@ -81,7 +81,10 @@
 	fn dispatch_render(&mut self) { NEED_RENDER.store(true, Ordering::Relaxed); }
 
 	#[inline]
-	fn dispatch_key(&mut self, key: KeyEvent) { Router::new(self).route(Key::from(key)); }
+	fn dispatch_key(&mut self, key: KeyEvent) {
+		tracing::error!(?key);
+		Router::new(self).route(Key::from(key));
+	}
 
 	#[inline]
 	fn dispatch_paste(&mut self, str: String) {

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Aug 22, 2024
@SchuhBaum
Copy link
Author

SchuhBaum commented Aug 22, 2024

From the logs:

  �[2m2024-08-22T15:45:58.492737Z�[0m �[31mERROR�[0m �[1;31myazi::app::app�[0m�[31m: �[1;31mkey�[0m�[31m: KeyEvent { code: Char('\\'), modifiers: KeyModifiers(CONTROL | ALT), kind: Press, state: KeyEventState(0x0) }�[0m
    �[2;3mat�[0m yazi-fm\src\app\app.rs:86

From the source code:
The function plain() is used to filter out characters when modifiers are pressed as well.

impl Key {
	#[inline]
	pub fn plain(&self) -> Option<char> {
		match self.code {
			KeyCode::Char(c) if !self.ctrl && !self.alt && !self.super_ => Some(c),
			_ => None,
		}
	}
}

@github-actions github-actions bot removed the waiting on op Waiting for more information from the original poster label Aug 22, 2024
@sxyazi
Copy link
Owner

sxyazi commented Aug 22, 2024

Based on the logs, it seems that crossterm reported an incorrect key event. The ctrl+alt+ß combination should be reported as \ instead of ctrl+alt+\. We can't remove the checks for ctrl and alt in the plain() function because that would affect the normal behavior of ctrl+alt+\.

I don't think there's anything Yazi can do about this, as Yazi simply receives the events from crossterm as they are without any additional processing, I suggest reporting this issue to crossterm so their devs can investigate it further.

@sxyazi sxyazi closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2024
@joshka
Copy link

joshka commented Aug 22, 2024

Hey there - @sxyazi. To help avoid users adding more issues on the crossterm repo about keyboard bindings, there's a central issue for these at crossterm-rs/crossterm#685. That issue contains debugging and more information about what terminals support what.

@sxyazi
Copy link
Owner

sxyazi commented Aug 22, 2024

Hey @joshka, thanks for linking to the issue!

It seems that the problem described there is a bit different - it's not about missing key combinations; instead, it's reporting incorrect key combinations, specifically wrong modifiers. Does this still apply to that issue?

@joshka
Copy link

joshka commented Aug 22, 2024

I think so, but I could be wrong.
The best bet though is to ensure that key has some meaningful value in the user's terminal outside of crossterm, and then spend some time on the debugging steps mentioned in the linked issue. Wrong modifiers is just a specialization of the problem that terminal input codes are non-standardized and many places where they will differ. Adding non PC-101 layouts in the mix makes this even more difficult.

@SchuhBaum
Copy link
Author

The ctrl+alt+ß combination should be reported as \ instead of ctrl+alt+\.

This might be wrong. Other combinations like shift+a are reported as shift+A and not just A. Therefore, the function plain(..) needs to account for that. It does that by ignoring the shift modifier.

Crossterm could handle these cases because it has a function that can return the character based on the keyboard layout. So it can determine that ß is the base character when ctrl+alt+\ is reported. But it is likely intended that modifiers are not consumed when the character changes.
Yazi already handles cases where Windows reports a shift modifier but Unix does not. It seems that it is not crossterm's responsibility to change the way that Windows reports modifiers.

@sxyazi
Copy link
Owner

sxyazi commented Aug 24, 2024

If I understand correctly, the functions of Shift and Ctrl / Alt are different. Shift is used to produce the uppercase form of printable characters.

For example, pressing Shift + a gives you A. I don't quite get why crossterm would report it as Shift + A, I haven't read through crossterm's code, but I guess it's easier to implement this way. In practice, though, this isn't really a problem.

Note the term I mentioned, "printable characters". This means A will show up in the input, which is fine.

However, Ctrl and Alt have a different purpose. They're used to generate control characters. For instance, Ctrl + a produces a character with an ASCII code of 1 (SOH), which is not printable, so it won't show up in the input, and that's correct behavior.

❯ showkey
Type any key to see the sequence it sends.
Terminate with your shell interrupt <CTL-C=ETX> or quit <FS> character.
<CTL-A=SOH>

The issue here is that I'm still unsure how to handle real ctrl+alt+\ if we ignore ctrl/alt in the plain() method. It's important to note that this method is a general-purpose one. In your case, it's ctrl+alt+\, but it affects all ctrl/alt combinations, like ctrl+a, alt+a, ctrl+alt+a, and so on.

@SchuhBaum
Copy link
Author

Wouldn't the real ctrl+alt+\ produce a control character instead of a printable character? The problem is that printable characters are being ignored.

Crossterm is consistent with what plain Powershell reports. It looks more like a Windows / Powershell behavior.
1

@sxyazi
Copy link
Owner

sxyazi commented Aug 24, 2024

Wouldn't the real ctrl+alt+\ produce a control character instead of a printable character? The problem is that printable characters are being ignored.

I'm a bit confused. Are you saying that ctrl+alt+\ produces a control character or a printable character? If it's a control character, then it won't be printable — it won't appear in the input but can be used as a shortcut key like these:

# Delete
{ on = "<Backspace>", run = "backspace", desc = "Delete the character before the cursor" },
{ on = "<Delete>", run = "backspace --under", desc = "Delete the character under the cursor" },
{ on = "<C-h>", run = "backspace", desc = "Delete the character before the cursor" },
{ on = "<C-d>", run = "backspace --under", desc = "Delete the character under the cursor" },
# Kill
{ on = "<C-u>", run = "kill bol", desc = "Kill backwards to the BOL" },
{ on = "<C-k>", run = "kill eol", desc = "Kill forwards to the EOL" },
{ on = "<C-w>", run = "kill backward", desc = "Kill backwards to the start of the current word" },
{ on = "<A-d>", run = "kill forward", desc = "Kill forwards to the end of the current word" },

@SchuhBaum
Copy link
Author

Let me clarify. I used the same notation for key combinations and key events. ctrl+alt+ß is the key combination. ß is the physical key (like a and 0). The resulting key event is ctrl+alt+\. Here, ctrl+alt does not produce a control character but instead the printable character \.

On an english keyboard, this might be different. In that case, \ is a physical key, right? So the key combination ctrl+alt+\ might produce a control character as you mentioned. The resulting key event should be different.

2

@sxyazi
Copy link
Owner

sxyazi commented Aug 24, 2024

Well, this brings us back to the original issue: Yazi is unable to distinguish between them. Yazi can't tell whether the user pressed ctrl+alt+ß or ctrl+alt+\ because crossterm reports both as ctrl+alt+\.

When the key event already includes ctrl+alt, it is no longer a printable character. My keyboard doesn't have ß, but pressing ctrl+alt+\ doesn't produce any valid character, whether in TUI or GUI applications.

If it were a printable character, it should be \, this would be the ideal behavior for crossterm. However, if for some reason I don't know, crossterm can't handle it that way, there should at least be some way for Yazi to know that the original key pressed by the user was ctrl+alt+ß - I noticed that the Oem4 shown in your screenshot above might serve as this indicator, but crossterm doesn't provide it.

@joshka
Copy link

joshka commented Aug 24, 2024

The missing thing (TIL) is that AltGr (often the right Alt key on a german keyboard layout) often gets translated to Ctrl+Alt, and is used to select the third layer of keys on a keyboard (in this case \). Thus Ctrl+Alt+ß should really be interpreted and returned by crossterm as just \ without any modifiers, but in reality that's difficult because crossterm uses the older low level console comands in windows.

It might be possible that crossterm could detect this based on the active keyboard locale and do the same thing as the newer methods do, but it would require some pretty deep digging into the windows api stuff to understand what's right to do for this.

Copy link

I'm going to lock this issue because it has been closed for 30 days. ⏳
This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working upstream
Projects
None yet
Development

No branches or pull requests

3 participants