Fix composed inputs (dead keys) in vim emulation

[jorikvanveen]

This PR fixes actions in vim emulation such as ci" or di' on keyboard layouts where quotes require multiple keystrokes. The tests that were passing before this commit still pass, but it might be wise to do some additional testing since I jumped into this codebase an hour ago and I have no idea what I'm doing (feedback is very welcome).

Release Notes:

• Fixed Linux: Dead keys / US intl. does not work as expected in Vim mode #12522

[cla-bot]

We require contributors to sign our Contributor License Agreement, and we don't have @jorikvanveen on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

[jorikvanveen]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[ConradIrwin]

@jorikvanveen I think we should probably try and fix the linux keyboard handling to more closely match the Mac behavior in this case (as this does work on macOS today - at least with a Brazilian layout - and I'm a bit concerned about special casing the accents).

Can you please let me know which keyboard layout you're using, and I'll try and test it on my local linux machine.

[jorikvanveen]

Yes, I must admit this solution is a bit hacky, though for my use case everything seems to work as far as I can tell.

I use GNOME wayland with this keyboard layout: English (US, intl., with dead keys)

[ConradIrwin]

@jorikvanveen I think #12871 is the right fix here. Does it work for you?

I was mistaken, as on macOS we were also triggering the find before composition had completed.

The main remaining downside is that it's not obvious that you're in composition mode in this case. I'm not sure what the best fix is, because we don't want to write the character to the buffer itself. Maybe we can use something like the Overlays from #12106 to render a fake IME input in this scenario.

[jorikvanveen]

Yes, that fixes it! Thank you!

[jorikvanveen]

Okay so I built the latest version of Zed on my desktop, and noticed the ci" combination didn't work. It turns out I still had my original patches active on my laptop when I made that last comment (oops). The fi" combination works fine, but I had to reapply the changes I made on line 192 to get ci" to work.

[ConradIrwin]

@jorikvanveen uh oh!

To clarify: what exactly are the keys you're pressing, and what are you expecting to have to press?

It should be the case now that we wait until composition is over before running f or t. So if you're on a Brazilian keyboard layout where " puts you into compose mode, and "a types ä you can press the keys f"a (e.g. type fä) to jump to the next ä.

[jorikvanveen]

I press the following sequence: c i ' space on a US intl. keyboard layout.
If I type this into a buffer in insert mode it results in ci'.
On my keyboard layout, when I press ', it puts me into compose mode and shows an underlined backtick to indicate it (`), when I press space it inserts the actual apostrophe '

In standard vim, when my cursor is between two apostrophes in normal mode (i.e. in a string), this sequence removes everything between between the apostrophes and puts me into insert mode so I can easily change the contents of the string.

What happens in the latest zed without my patch is that the sequence gets cancelled when I press the ' button.

To further clarify, this patch fixes the issue for me:

diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs
index 83eaa25f0..be1ff02d7 100644
--- a/crates/vim/src/vim.rs
+++ b/crates/vim/src/vim.rs
@@ -187,7 +187,7 @@ fn observe_keystrokes(keystroke_event: &KeystrokeEvent, cx: &mut WindowContext)
         if action.name().starts_with("vim::") {
             return;
         }
-    } else if cx.has_pending_keystrokes() {
+    } else if cx.has_pending_keystrokes() || keystroke_event.keystroke.ime_key.is_none() {
         return;
     }

[ConradIrwin]

@jorikvanveen I see, sorry about that.

I had been looking at f not at ci. f waits for any arbitrary character; whereas ci uses the keymap. I think your fix probably makes sense, let me play a bit.

[ConradIrwin]

Superseded by #13185. I added an additional check to only run this in the case that it's likely part of an IME transaction, otherwise c i down no longer aborted the transaction.

[jorikvanveen]

Thank you so much! Everything is working perfectly now.

(and yes I made sure to have a clean git repo this time 😅)

[ConradIrwin]

Great. Sorry for the u-turns along the way!