mawww / kakoune

mawww's experiment for a better code editor
http://kakoune.org
The Unlicense
9.7k stars 705 forks source link

[BUG] Backspace occasionally behaves like Delete #5155

Closed Abraxas-Knister closed 1 month ago

Abraxas-Knister commented 2 months ago

Version of Kakoune

v2023.08.05

Reproducer

I'm using kitty, version 0.33.1

Might have something to do with suspending and resuming (<c-z>). It does not happen in a client that has never been suspended.

Outcome

Occasionally backspace would show up as delete in the debug buffer (when the debug option has the keys flag) and behave as delete rather than backspace in any mode. The behaviour is present within anything done via this client. A typed <c-h> turns out to not be received. Any other client connected to the same server is unaffected. When I suspend the client and type something in the shell, the backspace key works.

Expectations

required text

Additional information

No response

Screwtapello commented 2 months ago

Some shells do weird things with backspace and delete; the best way to test outside of Kakoune is to run cat with no arguments and see if you can type and backspace there.

At startup, Kakoune decides what input should be mapped to the "backspace" key by examining the kernel's terminal settings. On my machine right now, in a fresh terminal it's configured to be ^? (ASCII DEL, byte 0x7F):

$ stty -a | grep -o ' erase[^;]*'
 erase = ^?

That setting is usually set by whatever terminal you're using based on its own configuration files. Programs can reset it themselves (with the stty command, or with a C API) but that doesn't modify your terminal's configuration files, so it generally doesn't change what your terminal sends when you hit the backspace key.

If, after you suspend Kakoune, you run some other app that tinkers with the kernel's terminal configuration, it may be that the terminal's configuration gets out of sync with the kernel, or the kernel gets out of sync with Kakoune, or some other misalignment.

Abraxas-Knister commented 2 months ago

I use <c-z> quite a lot. I guess I can adapt to just open another tab instead.

How would I inspect whether something changed?

krobelus commented 2 months ago

run stty -a after <c-z> and compare it with the normal case. May be a weird behavior in your shell, what shell is this?

Abraxas-Knister commented 2 months ago

GNU bash, Version 5.2.26(1)-release (x86_64-pc-linux-gnu)

Abraxas-Knister commented 2 months ago

I noticed, that C (shift plus c) is backspace when this happens. Checking through the common keyboard usage I find most other stuff normal, except for the numblock keys, which sends fancy unicode symbols.

I also checked other terminals, the problem seems to be how kitty and kakoune interact.

krobelus commented 2 months ago

I couldn't reproduce, maybe it's possible to write a script that reproduces the issue (by trying often enough)

Abraxas-Knister commented 2 months ago
$ kitty -c NONE
$ /usr/bin/kak
:set global debug keys
:b *debug*

then I hit the numblock return key (not the regular return key), and the debug buffer got the line

Client 'client0' got key ''

In that kakoune instance, the inputs of backspace and capital C still behave like they should. Backspace would become <del> and C would become <backspace> on later, undiscernible conditions.

Trying out kitty -c NONE turned up this error message:

[116 09:25:56.660704] [PARSE ERROR] The application is trying to use xterm's modifyOth
erKeys. This is superseded by the kitty keyboard protocol: https://sw.kovidgoyal.net/k
itty/keyboard-protocol/ the application should be updated to use that
Abraxas-Knister commented 2 months ago

loosely remembering modifyOtherKeys from a previous issue with Vim, I tried out whether anything of the same would happen in Vim too. It happened there too.

Abraxas-Knister commented 2 months ago

A relevant kitty issue:

https://github.com/kovidgoyal/kitty/issues/4443

krobelus commented 2 months ago

On Thu, Apr 25, 2024 at 12:28:26AM -0700, Abraxas-Knister wrote:

$ kitty -c NONE
$ /usr/bin/kak
:set global debug keys
:b *debug*

then I hit the numblock return key (not the regular return key), and the debug buffer got the line

Client 'client0' got key ''

This is unrelated and fixed in latest master by 84ea52e46 (Support CSI u numpad keys, 2023-06-17) (These key codes are defined at https://sw.kovidgoyal.net/kitty/keyboard-protocol/#functional-key-definitions)

In that kakoune instance, the inputs of backspace and capital C still behave like they should. Backspace would become <del> and C would become <backspace> on later, undiscernible conditions.

Is this caused by you pressing numblock enter? I doubt that because previously that key was simply treated as generic private-use-area code point (e.g. ignored in insert mode).

Does the problem go away after suspending an resuming again?

The fact that C changes meaning to delete sounds to me like your m_original_termios.c_cc[VERASE] gets corrupted. Can you maybe build with make sanitize=address and add some logging to track the value of this expression? For example write_to_debug_buffer(format("VERASE: {}", m_original_termios.c_cc[VERASE]));

Another possible explanation is that something changed the value of c_cc[VERASE] (not the original version but the current). Make sure to log that as well.

If you build latest master, you might also want to apply this patch (use git am) to prevent a crash on certain (lsp user mode) key bindings: @.***%3E/raw

Trying out kitty -c NONE turned up this error message:

[116 09:25:56.660704] [PARSE ERROR] The application is trying to use xterm's modifyOth
erKeys. This is superseded by the kitty keyboard protocol: https://sw.kovidgoyal.net/k
itty/keyboard-protocol/ the application should be updated to use that

This is just an irrelevant warning. We don't use full CSI u yet; maybe we should.

krobelus commented 2 months ago

On Thu, Apr 25, 2024 at 12:48:44AM -0700, Abraxas-Knister wrote:

loosely remembering modifyOtherKeys from a previous issue with Vim, I tried out whether anything of the same would happen in Vim too. It happened there too.

I don't know what you saw in Vim too. If it's just the warning, then yeah that just means it's unrelated. Or does Vim have the backspace/delete issue too? Then it's probably kitty-specific

Abraxas-Knister commented 2 months ago

I saw that in Vim, numblock enter would insert "æ", and I saw the error message from kitty. Numblock numbers and the separator key except for 5 also insert a weird glyph. The glyphs are the same for Vim and kakoune. I'll build the version you mentioned and see if it also happens to fix the C/backspace issue for me.

To be clear, I have no idea what causes the change in behaviour for C and backspace. It seems unrelated to numpad keypresses. My assumption that it had anything to do with suspending was a complete shot in the dark.

fwiw it could well be faulty memory as well.

I didn't test on Vim long enough to see whether that change in behaviour occurs there too.

nonumeros commented 2 months ago

this is vim under kitty

[PARSE ERROR] Unrecognized DCS code: 0x7a
 [PARSE ERROR] Unknown CSI code: 'm' with start_modifier: '' and end_modifier: '%' and parameters: '0'

kak under kitty

 [PARSE ERROR] The application is trying to use xterm's modifyOtherKeys. This is superseded by the kitty keyboard protocol: https://sw.kovidgoyal.net/kitty/keyboard-protocol/ the application should be updated to use that

The only ones that don't throw any parsing errors are obviously vi and nano. But neovim is clearly ahead of the big pack.

QiBaobin commented 1 month ago

I can reproduce with below steps:

Reproducer

new to open another client, (tried zellij and wezterm)
focus client0 to navigate back
c-z to background the kak
fg to bring it back
i enter insert mode

Outcome

1. behavior like <del>

  1. on-key %{info %val{key}} then shows <del> too
  2. client1 still works
krobelus commented 1 month ago

Doesn't reproduce, can you check if c_cc[VERASE] changes when it happens?

QiBaobin commented 1 month ago

@krobelus , how can I check that in the kak? I tried to add some code, but no luck.

krobelus commented 1 month ago

did you try write_to_debug_buffer as explained in https://github.com/mawww/kakoune/issues/5155#issuecomment-2076586109 ?

krobelus commented 1 month ago

or try gdb, see https://github.com/mawww/kakoune/wiki/hacking

QiBaobin commented 1 month ago

@krobelus , you are correct, c_cc[VERASE] was changed to 0 somehow. I made below patch, the two places have different c_cc[VERASE] values.

diff --git a/src/terminal_ui.cc b/src/terminal_ui.cc
index 3ad95e40..99f38809 100644
--- a/src/terminal_ui.cc
+++ b/src/terminal_ui.cc
@@ -700,6 +700,7 @@ Optional<Key> TerminalUI::get_next_key()
     if (not c)
         return {};

+    write_to_debug_buffer(format("1 c: {} VERASE: {}", *c, m_original_termios.c_cc[VERASE]));
     static constexpr auto control = [](char c) { return c & 037; };

     static auto convert = [this](Codepoint c) -> Codepoint {
@@ -709,6 +710,7 @@ Optional<Key> TerminalUI::get_next_key()
             return Key::Tab;
         if (c == ' ')
             return Key::Space;
+        write_to_debug_buffer(format("c: {} VERASE: {}", c, m_original_termios.c_cc[VERASE]));
         if (c == m_original_termios.c_cc[VERASE])
             return Key::Backspace;
         if (c == 127) // when it's not backspace

is it possible that it's related with some copy or clone in c++?

krobelus commented 1 month ago

m_original_termios.c_cc[VERASE] was changed to 0 somehow

This is really surprising because m_original_termios should only ever be written once per session, at startup in TerminalUI::TerminalUI, using tcgetattr(STDIN_FILENO, &m_original_termios);

Can you try setting a watchpoint on m_original_termios.c_cc[VERASE] in gdb to see where it changes?

QiBaobin commented 1 month ago

m_original_termios.c_cc[VERASE] was changed to 0 somehow

This is really surprising because m_original_termios should only ever be written once per session, at startup in TerminalUI::TerminalUI, using tcgetattr(STDIN_FILENO, &m_original_termios);

Can you try setting a watchpoint on m_original_termios.c_cc[VERASE] in gdb to see where it changes?

There is no gdbserver on Mac OS, so that I don't know how to do that. Another interesting thing is that m_original_termios in that two places don't seem to be one variable, I pressed many times, and they always displayed two values. It looked like a wrong value captured before correct value set to me.

krobelus commented 1 month ago

weird. Maybe print the address of m_original_termios, maybe it changes for some reason. If that (or a spurious copy) is the problem, the fix might be:

diff --git a/src/terminal_ui.cc b/src/terminal_ui.cc
index ca512ef89..d59073b9d 100644
--- a/src/terminal_ui.cc
+++ b/src/terminal_ui.cc
@@ -705,7 +705,7 @@ Optional<Key> TerminalUI::get_next_key()

     static constexpr auto control = [](char c) { return c & 037; };

-    static auto convert = [this](Codepoint c) -> Codepoint {
+    auto convert = [this](Codepoint c) -> Codepoint {
         if (c == control('m') or c == control('j'))
             return Key::Return;
         if (c == control('i'))
@@ -720,7 +720,7 @@ Optional<Key> TerminalUI::get_next_key()
             return Key::Escape;
         return c;
     };
-    static auto parse_key = [](unsigned char c) -> Key {
+    auto parse_key = [&convert](unsigned char c) -> Key {
         if (Codepoint cp = convert(c); cp > 255)
             return Key{cp};
         // Special case: you can type NUL with Ctrl-2 or Ctrl-Shift-2 or
@@ -759,7 +759,7 @@ Optional<Key> TerminalUI::get_next_key()
         return mod;
     };

-    auto parse_csi = [this]() -> Optional<Key> {
+    auto parse_csi = [this, &convert]() -> Optional<Key> {
         auto next_char = [] { return get_char().value_or((unsigned char)0xff); };
         int params[16][4] = {};
         auto c = next_char();
QiBaobin commented 1 month ago

weird. Maybe print the address of m_original_termios, maybe it changes for some reason. If that (or a spurious copy) is the problem, the fix might be:

diff --git a/src/terminal_ui.cc b/src/terminal_ui.cc
index ca512ef89..d59073b9d 100644
--- a/src/terminal_ui.cc
+++ b/src/terminal_ui.cc
@@ -705,7 +705,7 @@ Optional<Key> TerminalUI::get_next_key()

     static constexpr auto control = [](char c) { return c & 037; };

-    static auto convert = [this](Codepoint c) -> Codepoint {
+    auto convert = [this](Codepoint c) -> Codepoint {
         if (c == control('m') or c == control('j'))
             return Key::Return;
         if (c == control('i'))
@@ -720,7 +720,7 @@ Optional<Key> TerminalUI::get_next_key()
             return Key::Escape;
         return c;
     };
-    static auto parse_key = [](unsigned char c) -> Key {
+    auto parse_key = [&convert](unsigned char c) -> Key {
         if (Codepoint cp = convert(c); cp > 255)
             return Key{cp};
         // Special case: you can type NUL with Ctrl-2 or Ctrl-Shift-2 or
@@ -759,7 +759,7 @@ Optional<Key> TerminalUI::get_next_key()
         return mod;
     };

-    auto parse_csi = [this]() -> Optional<Key> {
+    auto parse_csi = [this, &convert]() -> Optional<Key> {
         auto next_char = [] { return get_char().value_or((unsigned char)0xff); };
         int params[16][4] = {};
         auto c = next_char();

You're awesome, it fixes the issue. Thanks! @krobelus

QiBaobin commented 1 month ago

@krobelus , will you open a PR for this fix?

BTW: @Abraxas-Knister , could you verify if this works for you too?