Set timeout for waiting for a key sequence to complete

[comfortablynick]

fish, version 3.1.2-1530-g06bd1e934

With a clean environment, I run these commands:

fish_vi_key_bindings

# kj -> Esc
bind -M insert kj 'if commandline -P; commandline -f cancel; else; set fish_bind_mode default; commandline -f backward-char repaint-mode; end'

Any time I press k, fish waits indefinitely for the j. Is there an equivalent of timeoutlen in Vim? By default, Vim will only wait 1 sec for j before assuming I'm just pressing k.

'timeoutlen' 'tm'       number  (default 1000)
                          global
Time in milliseconds to wait for a mapped sequence to complete.

Fish seems to assume I'm never just pressing k. There is $fish_escape_delay_ms but it doesn't seem to affect this. Shouldn't there be something like this for regular key bindings?

[ridiculousfish]

There's nothing like that now except for escape. This seems like an interesting general purpose idea - it would make multi-key bindings more tractable.

[yanshay]

Is there any way to workaround this by more complex coding using fish functions or some other way to get the 'kj'/'jk' sequence work as expected?

[faho]

Is there any way to workaround this by more complex coding using fish functions or some other way to get the 'kj'/'jk' sequence work as expected?

I can't think of anything, no. Fundamentally when you have a binding "kj" and press "k", fish will wait indefinitely to see the "j" or something else to disambiguate.

That's this issue.

[yanshay]

I managed to get this working for my "kj" scenario with a simple change to input.cpp as below, but I'm not sure about potential implications on more complex scenarios. Can someone familiar with the code comment on implications?

--- a/src/input.cpp
+++ b/src/input.cpp
@@ -475,7 +475,11 @@ class event_queue_peeker_t {
         if (idx_ == peeked_.size()) {
             char_event_t newevt{L'\0'};
             if (!timed) {
-                newevt = event_queue_.readch();
+                if (auto mevt = event_queue_.readch_timed()) {
+                    newevt = mevt.acquire();
+                } else {
+                    return false;
+                }
             } else if (auto mevt = event_queue_.readch_timed()) {
                 newevt = mevt.acquire();
             } else {

[faho]

Can someone familiar with the code comment on implications?

What you're doing is basically removing the "timed"/"not timed" distinction.

Not quite because you're not setting had_timeout_ and so next_is_char won't return false again. I would expect, from thinking about it for a bit, to have to wait for multiple timeout periods if you have multiple possible bindings - e.g. if you bind "jk" and "jj" and "jl" I think this would wait for three times the timeout? It'll check the "jj", timed, and if there's a timeout it'll go on to check "jk", timed, and if there's a timeout it'll check "jl", timed.

But fundamentally the question is more one of design: The escape timeout is for distinguishing between pressing escape on its own and pressing alt+something or a key that sends an escape sequence like e.g. up-arrow.

You're just reusing that timeout! It's set to 30 milliseconds by default, which is pretty short - just typing "jk" leasurely I can see myself hitting that. So you increase it, and now pressing escape has that delay as well.

So you want two separate timeouts, and they need to be passed through here.

[yanshay]

Not quite because you're not setting had_timeout_ and so next_is_char won't return false again. I would expect, from thinking about it for a bit, to have to wait for multiple timeout periods if you have multiple possible bindings - e.g. if you bind "jk" and "jj" and "jl" I think this would wait for three times the timeout? It'll check the "jj", timed, and if there's a timeout it'll go on to check "jk", timed, and if there's a timeout it'll check "jl", timed.

So are you saying that where I added the return false I should also set had_timeout_ to true? And if I do that would be the correct approach (other than the comment regarding separate configuration)?

So you want two separate timeouts, and they need to be passed through here.

Agree and thought of that as well. Wanted to verify that that's the proper technique to use though before I build the complete solution.

[faho]

So are you saying that where I added the return false I should also set had_timeout_ to true?

I'm saying you can just remove the if (!timed) path if you're not gonna do untimed reads.