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.

Sign up for GitHub

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

Jump to bottom

[BUG] Backspace occasionally behaves like Delete #5155

Closed
Abraxas-Knister opened this issue Apr 24, 2024 · 24 comments
Closed

[BUG] Backspace occasionally behaves like Delete #5155

Abraxas-Knister opened this issue Apr 24, 2024 · 24 comments
Labels
bug

Comments

@Abraxas-Knister
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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

@Abraxas-Knister
Copy link
Author

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
Copy link
Contributor

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

@Abraxas-Knister
Copy link
Author 

$ 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
Copy link
Author

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
Copy link
Author

A relevant kitty issue:

kovidgoyal/kitty#4443

@krobelus
Copy link
Contributor

krobelus commented Apr 25, 2024 via email

@krobelus
Copy link
Contributor

krobelus commented Apr 25, 2024 via email

@Abraxas-Knister
Copy link
Author

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
Copy link
Contributor

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 QiBaobin mentioned this issue May 14, 2024
Closed
@QiBaobin
Copy link
Contributor

QiBaobin commented May 14, 2024 •
edited

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>
2. on-key %{info %val{key}} then shows <del> too
3. client1 still works

@krobelus
Copy link
Contributor

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

@QiBaobin
Copy link
Contributor

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

@krobelus
Copy link
Contributor

did you try write_to_debug_buffer as explained in #5155 (comment) ?

@krobelus
Copy link
Contributor

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

@QiBaobin
Copy link
Contributor

QiBaobin commented May 15, 2024 •
edited

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

QiBaobin commented May 16, 2024 •
edited

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
Copy link
Contributor

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

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

@mawww mawww closed this as completed in 7a90473 May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@QiBaobin @Screwtapello @krobelus @nonumeros @Abraxas-Knister