Repository: alacritty/alacritty (language: rust)

Issue:
In Alacritty 0.13+ some keys (such as the Pause key) are now spewing "weird" text, when previously nothing happened
### System

OS: Linux (X11, xmonad with no compositor)
Version: 0.13.1 (I've confirmed this happens in 0.13.0 and does not happen in 0.12.3)

### What

On alacritty 0.12.3, I could press the Pause key, and nothing would happen. Now, on Alacritty 0.13+, pressing the Pause key (and Scroll Lock, and presumably other keys I have not discovered yet) causes some weird spew to get emitted:

On v0.12.3:

```
$ target/debug/alacritty --version
alacritty 0.12.3 (5efb0690)
$ xdotool key Pause
$
```

And on v0.13.1 (note the `^[[57362u` spew to the terminal and some keys (`362u`) getting typed without me actually typing them):

```
$ target/debug/alacritty --version
alacritty 0.13.1 (fe2a3c56)
$ xdotool key Pause; echo "done"
^[[57362udone
$ 362u
```

I performed a git bisect, and this behavior was introduced in https://github.com/alacritty/alacritty/commit/cb03806e2ab85674c45e87e1bb24dfe2fd1a918c. I tried reading through that diff, but it's beyond me. cc-ing the author (@kchibisov), and hoping he knows what's up.

### Misc

I was (sort of*) able to work around this by adding a `keyboard.binding` to my alacritty conf file to ignore these keys:

```
[keyboard]
bindings = [
    { key = "Pause", action = "None" },
    { key = "Pause", mods = "Shift", action = "None" },
]
```

*I checked, and this causes these keypresses to be ignored by the kitty keyboard protocol implementation, which I don't want. Hopefully it doesn't matter, but if it's relevant, I can explain why I sometimes press keys such as Pause that I expect to have no visual output, yet I still want them to show up in the keyboard kitty protocol implementation.


Discussion / hints:
Pause is defined as part of the kitty keyboard protocol https://sw.kovidgoyal.net/kitty/keyboard-protocol/#functional-key-definitions

I remember reading that we should send them, since they are not naturally exposed, but I can't find the text anymore saying so, should probably just not send until the mode is set.
Interesting! Can you help me understand why I see this behavior with the Pause key but not an F key like F4?
F4 has the original meaning of `^[OS`, however those media never had them.

Do you have this issue in kitty terminal when you press the `Pause` key? If you don't see that I'll update to not emit the keys for these characters, but I think I've tested and it emitted the said chars.
> F4 has the original meaning of `^[OS`

Sorry, I know very little about terminal emulators and escape sequences. I don't really understand this. Can you direct me to some reading?

> Do you have this issue in kitty terminal when you press the Pause key?

I just checked, and I do see this same behavior in kitty (v0.31.0).

I'm a bit confused though: AFAIK, I'm not using the keyboard kitty protocol here, right? So just because Alacritty got a new feature, why would the behavior of Alacritty change? Or does zsh (my shell) actually opt into this CSI 1 keyboard handling behavior and *that's* why I'm seeing a change in behavior?
I think the point was that it defined key bindings for keys never used before and that are dedicated.

The other part of the spec was to define keys for keys not usually having a special meaning, like media keys or higher F keys, because they generally don't exist. Like there's no key for F13 for example, because it's a Shift+F1, some apps allow you to define it via `F13` but it's still `Shift+F1`.

`F4` could be just suppressed by zsh so you don't see it inputted in the prompt, but you can still see what it is if you press `ctrl + v` followed by `f4` in zsh. And if you try to input into `cat` you'll get that sort of input.

@chrisduerr do we want to do anything with it given that kitty behaves the same? Like we just defined the values for rare keys.
> F4 could be just suppressed by zsh so you don't see it inputted in the prompt, but you can still see what it is if you press ctrl + v followed by f4 in zsh. And if you try to input into cat you'll get that sort of input.

Ah, that makes sense! I also see it get inserted in insert mode in vim. Why don't I see any F4 stuff get inputted in other random repls (I tried both the `python` repl and the `gdb` repl. Do they both have special handling to ignore F4?)

It makes sense to me if you want to close this issue. I do now have a question (or possibly a feature request): is there some keycode that Alacritty would ignore/drop in "regular" modes (like how Pause used to behave), but *would* pass through when this new kitty keyboard protocol is enabled? Or maybe a different question: can you point me towards how I would go about reconfiguring zsh and vim to ignore these new rare keys (similar to how they ignore F4 right now)?
I'm not sure you can ignore them in general without config in each app, I'd simply map them to `None` the way you did if you don't want bindings for them to appear in general. And keep in mind that F5 is not ignored because it has a bit different payload, and it's not even related to kitty keyboard since the value for F1-F12 are based on terminfo.

>but would pass through when this new kitty keyboard protocol is enabled? 

Numpad behaves differently, ESC alters its behavior once the mode is enabled, and some other keys. You can read the kitty keyboard protocol spec and get idea from it.
> do we want to do anything with it given that kitty behaves the same?

How about other terminals that implement the protocol?
I went through the list of terminal emulators on https://sw.kovidgoyal.net/kitty/keyboard-protocol/ and generated a complete table of behavior.

For the record:
- By "spew": I mean the `^[[57362u` + `362u` output described in the original post.
- I don't know what the right term is for "normal" mode. By this, I just mean the behavior of the terminal emulator when zsh is running. If it would be useful, I'd be happy to gather more data for other modes (like whatever mode the terminal is in when vim/neovim is running, if that's even any different than zsh).

| **Terminal Emulator** | **Pause key behavior in "normal" mode** | **Pause key behavior when kitty keyboard protocol is enabled** | **Testing Notes**                                                                                                                                                                            |
|-------------------|-------------------------------------|------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Kitty             | spew                                | key up, down, repeat all work    |                                                                                                                                                                                          |
| Alacritty         | spew                                | key up, down, repeat all work    |                                                                                                                                                                                          |
| foot              | nothing happens                     | key up, down, repeat all work    | only runs on wayland. i ran it under `weston`                                                                                                                                            |
| WezTerm           | nothing happens                     | nothing happens                                            | keyboard kitty protocol support is disabled by default, run with `wezterm --config enable_kitty_keyboard=true`                                                                           |
| rio               | nothing happens                     | nothing happens                                            | keyboard kitty protocol support is disabled by default. need to edit the config file to enable the kitty keyboard protocol (https://raphamorim.io/rio/docs/configuration-file/#keyboard) |

So, there seem to be the "camps" of behavior, and `rio` alone behaves as I want.

> I'm not sure you can ignore them in general without config in each app

@kchibisov, could you explain this a little more? This does feel like something alacritty _could_ choose to do if it wanted: if the kitty keyboard protocol is enabled, then send events for these types of keys, and if it's not enabled, then do not. That feels like what `rio` is doing.

I certainly don't know what the "right" behavior should be here. I personally would prefer for things to behave like `rio` (again, happy to explain why, but the thing I'm trying to do is so me-specific, I don't think you'd find it to be a particularly useful data point). IIUC, since this is about the terminal emulator's behavior when the kitty keyboard protocol is *not* active then [the specification](https://sw.kovidgoyal.net/kitty/keyboard-protocol/) does not apply?, perhaps it's just up to the terminal emulator to decide what should happen here (or perhaps there's some other specification that applies here?)
The right behavior is what kitty does, since it's kitty protocol. Alacritty follows what kitty does and that's about it, you better of clarifying this with kitty author, I just implement the protocol to behave consistency as much as I can with its _desired_ implementation, which is kitty implementation.

rio for example copied my initial impl until I adjusted it to match what actually kitty does.
Like I can certainly exclude, but I'd rather bring this upstream.
I can definitely bring this discussion upstream, but before I do, can you help me understand how this is a kitty protocol question? Since this is about the terminal emulator's behavior when the kitty keyboard protocol is *not* active then how does the kitty keyboard protocol specification or the behavior of kitty even matter?
https://github.com/kovidgoyal/kitty/discussions/7037
@kchibisov Arguably outside of Kitty protocol Alacritty should match XTerm.
I don't think we do so already, unless you consider some subset of xterm.

I mean, the issue is mostly about "I pressed keys that don't do anything just because I want, but suddenly they insert something", so I'm not even sure what are we fixing here.
> I don't think we do so already, unless you consider some subset of xterm.

We should mostly match URxvt, that's true. But it should always be either URxvt or XTerm. As far as I understand this new behavior matches neither and thus is somewhat inconsistent?

> I mean, the issue is mostly about "I pressed keys that don't do anything just because I want, but suddenly they insert something", so I'm not even sure what are we fixing here.

Yeah I agree with that. I personally wouldn't really mind just closing this with "just don't press that button" as a solution. But I do think it makes some sense to consider our stance on intended character output in general. I'm personally not too fond of matching Kitty for anything other than the "Kitty protocol".
>Yeah I agree with that. I personally wouldn't really mind just closing this with "just don't press that button" as a solution. But I do think it makes some sense to consider our stance on intended character output in general. I'm personally not too fond of matching Kitty for anything other than the "Kitty protocol".

The thing is that users who don't want media key to work can just unbind them, but on the other side they can have them working. The thing about xterm/urxvt is probably that they haven't ever though about these keys in the first place? 
https://github.com/kovidgoyal/kitty/commit/a9c7a85d9ace19fd827a3445e77ec89c246a87b1

So we can decide not to send.
From now on I will refer to "normal" mode as "legacy" mode as per [today's change to the kitty keyboard protocol spec](https://github.com/kovidgoyal/kitty/commit/a9c7a85d9ace19fd827a3445e77ec89c246a87b1).

How do you feel about exposing this as a config option for alacritty? Perhaps via the `keyboard.bindings` `action` setting?

- `ReceiveChar`: alacritty's current behavior, that is: sends the "spew" in legacy mode and sends the key events when kitty's keyboard protocol is enabled
- `None`: the same behavior it currently has, that is: no action in both "legacy" and "kitty keyboard protocol" modes
- `LegacyModeNone`: new feature. no action in legacy mode, sends key down/repeat/up in kitty keyboard protocol mode. In other words, this is `rio`'s current behavior described above.
- `KittyKeyboardModeNone`: new feature. I don't think this needs to exist, but I've listed it here for academic completeness. I'm not sure about the name. In legacy mode, the key would get sent, and in kitty mode, it would not.

I have a little Rust experience (and am looking for more), so I would be willing to try to implement something like this feature if you're open to it.
Like a binding for extra mode? I don't really care, whatever @chrisduerr says here, given that not emitting bindings in normal mode is correct as well.
I don't think it makes any sense to partially support CSI u like this. As Kitty states, there is no encoding in the "legacy" mode. It's not like XTerm/URxvt didn't bother implementing, there's nothing to implement.

We skipped CSI u and went straight for the Kitty protocol because there's not much point going for some half-baked solution. So I don't see why we'd introduce that half-baked CSI u mode into our "legacy" handling.
@chrisduerr besides, do we mask `CSI u` bindings from normal mode? 
You mean never sending *any* `CSI u` stuff outside of Kitty mode? I think it would make sense to go for pre-protocol behavior yeah.
Adding to this -- I found that NumpadEnter is also being interpreted incorrectly as control-C in 0.13.0 and above, is that related?

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c17768baff..5473813c104 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - No unused-key warnings will be emitted for OS-specific config keys
 - Use built-in font for sextant symbols from `U+1FB00` to `U+1FB3B`
+- Kitty encoding is not used anymore for uncommon keys unless the protocol enabled
 
 ## 0.13.1
 
diff --git a/alacritty/src/config/bindings.rs b/alacritty/src/config/bindings.rs
index 794cb04d4da..9940cb5b747 100644
--- a/alacritty/src/config/bindings.rs
+++ b/alacritty/src/config/bindings.rs
@@ -453,6 +453,7 @@ pub fn default_key_bindings() -> Vec<KeyBinding> {
         Backspace, ~BindingMode::VI, ~BindingMode::SEARCH, ~BindingMode::REPORT_ALL_KEYS_AS_ESC; Action::Esc("\x7f".into());
         Backspace, ModifiersState::ALT,     ~BindingMode::VI, ~BindingMode::SEARCH, ~BindingMode::REPORT_ALL_KEYS_AS_ESC; Action::Esc("\x1b\x7f".into());
         Backspace, ModifiersState::SHIFT,   ~BindingMode::VI, ~BindingMode::SEARCH, ~BindingMode::REPORT_ALL_KEYS_AS_ESC; Action::Esc("\x7f".into());
+        Enter => KeyLocation::Numpad, ~BindingMode::VI, ~BindingMode::SEARCH, ~BindingMode::REPORT_ALL_KEYS_AS_ESC, ~BindingMode::DISAMBIGUATE_ESC_CODES; Action::Esc("\n".into());
         // Vi mode.
         Space, ModifiersState::SHIFT | ModifiersState::CONTROL, ~BindingMode::SEARCH; Action::ToggleViMode;
         Space, ModifiersState::SHIFT | ModifiersState::CONTROL, +BindingMode::VI, ~BindingMode::SEARCH; Action::ScrollToBottom;
diff --git a/alacritty/src/input/keyboard.rs b/alacritty/src/input/keyboard.rs
index 1df1b13b2ed..b7635bd9f44 100644
--- a/alacritty/src/input/keyboard.rs
+++ b/alacritty/src/input/keyboard.rs
@@ -284,7 +284,8 @@ fn build_sequence(key: KeyEvent, mods: ModifiersState, mode: TermMode) -> Vec<u8
 
     let sequence_base = context
         .try_build_numpad(&key)
-        .or_else(|| context.try_build_named(&key))
+        .or_else(|| context.try_build_named_kitty(&key))
+        .or_else(|| context.try_build_named_normal(&key))
         .or_else(|| context.try_build_control_char_or_mod(&key, &mut modifiers))
         .or_else(|| context.try_build_textual(&key, associated_text));
 
@@ -430,45 +431,17 @@ impl SequenceBuilder {
         Some(SequenceBase::new(base.into(), SequenceTerminator::Kitty))
     }
 
-    /// Try building from [`NamedKey`].
-    fn try_build_named(&self, key: &KeyEvent) -> Option<SequenceBase> {
+    /// Try building from [`NamedKey`] using the kitty keyboard protocol encoding
+    /// for functional keys.
+    fn try_build_named_kitty(&self, key: &KeyEvent) -> Option<SequenceBase> {
         let named = match key.logical_key {
-            Key::Named(named) => named,
+            Key::Named(named) if self.kitty_seq => named,
             _ => return None,
         };
 
-        // The default parameter is 1, so we can omit it.
-        let one_based = if self.modifiers.is_empty() && !self.kitty_event_type { "" } else { "1" };
         let (base, terminator) = match named {
-            NamedKey::PageUp => ("5", SequenceTerminator::Normal('~')),
-            NamedKey::PageDown => ("6", SequenceTerminator::Normal('~')),
-            NamedKey::Insert => ("2", SequenceTerminator::Normal('~')),
-            NamedKey::Delete => ("3", SequenceTerminator::Normal('~')),
-            NamedKey::Home => (one_based, SequenceTerminator::Normal('H')),
-            NamedKey::End => (one_based, SequenceTerminator::Normal('F')),
-            NamedKey::ArrowLeft => (one_based, SequenceTerminator::Normal('D')),
-            NamedKey::ArrowRight => (one_based, SequenceTerminator::Normal('C')),
-            NamedKey::ArrowUp => (one_based, SequenceTerminator::Normal('A')),
-            NamedKey::ArrowDown => (one_based, SequenceTerminator::Normal('B')),
-            NamedKey::F1 => (one_based, SequenceTerminator::Normal('P')),
-            NamedKey::F2 => (one_based, SequenceTerminator::Normal('Q')),
-            NamedKey::F3 => {
-                // F3 in kitty protocol diverges from alacritty's terminfo.
-                if self.kitty_seq {
-                    ("13", SequenceTerminator::Normal('~'))
-                } else {
-                    (one_based, SequenceTerminator::Normal('R'))
-                }
-            },
-            NamedKey::F4 => (one_based, SequenceTerminator::Normal('S')),
-            NamedKey::F5 => ("15", SequenceTerminator::Normal('~')),
-            NamedKey::F6 => ("17", SequenceTerminator::Normal('~')),
-            NamedKey::F7 => ("18", SequenceTerminator::Normal('~')),
-            NamedKey::F8 => ("19", SequenceTerminator::Normal('~')),
-            NamedKey::F9 => ("20", SequenceTerminator::Normal('~')),
-            NamedKey::F10 => ("21", SequenceTerminator::Normal('~')),
-            NamedKey::F11 => ("23", SequenceTerminator::Normal('~')),
-            NamedKey::F12 => ("24", SequenceTerminator::Normal('~')),
+            // F3 in kitty protocol diverges from alacritty's terminfo.
+            NamedKey::F3 => ("13", SequenceTerminator::Normal('~')),
             NamedKey::F13 => ("57376", SequenceTerminator::Kitty),
             NamedKey::F14 => ("57377", SequenceTerminator::Kitty),
             NamedKey::F15 => ("57378", SequenceTerminator::Kitty),
@@ -514,6 +487,52 @@ impl SequenceBuilder {
         Some(SequenceBase::new(base.into(), terminator))
     }
 
+    /// Try building from [`NamedKey`].
+    fn try_build_named_normal(&self, key: &KeyEvent) -> Option<SequenceBase> {
+        let named = match key.logical_key {
+            Key::Named(named) => named,
+            _ => return None,
+        };
+
+        // The default parameter is 1, so we can omit it.
+        let one_based = if self.modifiers.is_empty() && !self.kitty_event_type { "" } else { "1" };
+        let (base, terminator) = match named {
+            NamedKey::PageUp => ("5", SequenceTerminator::Normal('~')),
+            NamedKey::PageDown => ("6", SequenceTerminator::Normal('~')),
+            NamedKey::Insert => ("2", SequenceTerminator::Normal('~')),
+            NamedKey::Delete => ("3", SequenceTerminator::Normal('~')),
+            NamedKey::Home => (one_based, SequenceTerminator::Normal('H')),
+            NamedKey::End => (one_based, SequenceTerminator::Normal('F')),
+            NamedKey::ArrowLeft => (one_based, SequenceTerminator::Normal('D')),
+            NamedKey::ArrowRight => (one_based, SequenceTerminator::Normal('C')),
+            NamedKey::ArrowUp => (one_based, SequenceTerminator::Normal('A')),
+            NamedKey::ArrowDown => (one_based, SequenceTerminator::Normal('B')),
+            NamedKey::F1 => (one_based, SequenceTerminator::Normal('P')),
+            NamedKey::F2 => (one_based, SequenceTerminator::Normal('Q')),
+            NamedKey::F3 => (one_based, SequenceTerminator::Normal('R')),
+            NamedKey::F4 => (one_based, SequenceTerminator::Normal('S')),
+            NamedKey::F5 => ("15", SequenceTerminator::Normal('~')),
+            NamedKey::F6 => ("17", SequenceTerminator::Normal('~')),
+            NamedKey::F7 => ("18", SequenceTerminator::Normal('~')),
+            NamedKey::F8 => ("19", SequenceTerminator::Normal('~')),
+            NamedKey::F9 => ("20", SequenceTerminator::Normal('~')),
+            NamedKey::F10 => ("21", SequenceTerminator::Normal('~')),
+            NamedKey::F11 => ("23", SequenceTerminator::Normal('~')),
+            NamedKey::F12 => ("24", SequenceTerminator::Normal('~')),
+            NamedKey::F13 => ("25", SequenceTerminator::Normal('~')),
+            NamedKey::F14 => ("26", SequenceTerminator::Normal('~')),
+            NamedKey::F15 => ("28", SequenceTerminator::Normal('~')),
+            NamedKey::F16 => ("29", SequenceTerminator::Normal('~')),
+            NamedKey::F17 => ("31", SequenceTerminator::Normal('~')),
+            NamedKey::F18 => ("32", SequenceTerminator::Normal('~')),
+            NamedKey::F19 => ("33", SequenceTerminator::Normal('~')),
+            NamedKey::F20 => ("34", SequenceTerminator::Normal('~')),
+            _ => return None,
+        };
+
+        Some(SequenceBase::new(base.into(), terminator))
+    }
+
     /// Try building escape from control characters (e.g. Enter) and modifiers.
     fn try_build_control_char_or_mod(
         &self,
@@ -596,7 +615,7 @@ impl SequenceBase {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum SequenceTerminator {
     /// The normal key esc sequence terminator defined by xterm/dec.
     Normal(char),


Test patch: