Use event_queue_peeker_t when matching key bindings

Previously, when attempting to match a key binding, we would dequeue
events from the queue and put them back on if the binding fails. The
tricky part is timeouts: distinguishing between an escaped character and
the escape key itself. This was handled with "timeout events" and we had
to be careful to know when to discard them.

Switch to a new model: use event_queue_peeker more pervasively.
Temporarily dequeued events are stored in the peeker, and the peeker
itself remembers when it has seen a timeout. This is in preparation for
removing the idea of "timeout events" altogether.

src/input.cpp

@@ -428,32 +428,6 @@ void inputter_t::mapping_execute(const input_mapping_t &m,
     if (!m.sets_mode.empty()) input_set_bind_mode(*parser_, m.sets_mode);
 }
 
-/// Try reading the specified function mapping.
-bool inputter_t::mapping_is_match(const input_mapping_t &m) {
-    const wcstring &str = m.seq;
-
-    assert(!str.empty() && "zero-length input string passed to mapping_is_match!");
-
-    bool timed = false;
-    for (size_t i = 0; i < str.size(); ++i) {
-        auto evt = timed ? event_queue_.readch_timed() : event_queue_.readch();
-        if (!evt.is_char() || evt.get_char() != str[i]) {
-            // We didn't match the bind sequence/input mapping, (it timed out or they entered
-            // something else). Undo consumption of the read characters since we didn't match the
-            // bind sequence and abort.
-            event_queue_.push_front(evt);
-            event_queue_.insert_front(str.begin(), str.begin() + i);
-            return false;
-        }
-
-        // If we just read an escape, we need to add a timeout for the next char,
-        // to distinguish between the actual escape key and an "alt"-modifier.
-        timed = (str[i] == L'\x1B');
-    }
-
-    return true;
-}
-
 void inputter_t::queue_ch(const char_event_t &ch) {
     if (ch.is_readline()) {
         function_push_args(ch.get_readline());
@@ -463,71 +437,94 @@ void inputter_t::queue_ch(const char_event_t &ch) {
 
 void inputter_t::push_front(const char_event_t &ch) { event_queue_.push_front(ch); }
 
-/// \return the first mapping that matches, walking first over the user's mapping list, then the
+/// A class which allows accumulating input events, or returns them to the queue.
-/// preset list. \return null if nothing matches.
+/// This contains a list of events which have been dequeued, and a current index into that list.
-maybe_t<input_mapping_t> inputter_t::find_mapping() {
-    const input_mapping_t *generic = nullptr;
-    const auto &vars = parser_->vars();
-    const wcstring bind_mode = input_get_bind_mode(vars);
-
-    auto ml = input_mappings()->all_mappings();
-    for (const auto &m : *ml) {
-        if (m.mode != bind_mode) {
-            continue;
-        }
-
-        if (m.is_generic()) {
-            if (!generic) generic = &m;
-        } else if (mapping_is_match(m)) {
-            return m;
-        }
-    }
-    return generic ? maybe_t<input_mapping_t>(*generic) : none();
-}
-
-/// A class which allows accumulating input events, or return them to the queue.
 class event_queue_peeker_t {
    public:
     explicit event_queue_peeker_t(input_event_queue_t &event_queue) : event_queue_(event_queue) {}
 
-    /// \return the next event, optionally waiting for it.
+    /// \return the next event.
-    char_event_t next(bool timed = false) {
+    char_event_t next() {
-        auto event = timed ? event_queue_.readch_timed() : event_queue_.readch();
+        assert(idx_ <= peeked_.size() && "Index must not be larger than dequeued event count");
-        peeked_.push_back(event);
+        if (idx_ == peeked_.size()) {
-        return event;
+            auto event = event_queue_.readch();
+            peeked_.push_back(event);
+        }
+        return peeked_.at(idx_++);
     }
 
-    /// \return how many events are currently stored.
+    /// Check if the next event is the given character. This advances the index on success only.
-    size_t len() const { return peeked_.size(); }
+    /// If \p timed is set, then return false if this (or any other) character had a timeout.
-
+    bool next_is_char(wchar_t c, bool timed = false) {
-    /// Consume all events that have been peeked, leaving this empty.
+        assert(idx_ <= peeked_.size() && "Index must not be larger than dequeued event count");
-    void consume() { peeked_.clear(); }
+        // See if we had a timeout already.
-
+        if (timed && had_timeout_) {
-    /// Return all peeked events to the queue.
+            return false;
-    void restart() {
+        }
-        event_queue_.insert_front(peeked_.cbegin(), peeked_.cend());
+        // Grab a new event if we have exhausted what we have already peeked.
-        peeked_.clear();
+        if (idx_ == peeked_.size()) {
-    }
+            auto newevt = timed ? event_queue_.readch_timed() : event_queue_.readch();
-
+            if (newevt.is_timeout()) {
-    ~event_queue_peeker_t() { restart(); }
+                assert(timed && "Should only get timeouts from timed reads");
-
+                had_timeout_ = true;
-   private:
+                return false;
-    std::vector<char_event_t> peeked_;
+            }
-    input_event_queue_t &event_queue_;
+            peeked_.push_back(newevt);
-};
+        }
-
+        // Now we have peeked far enough, check the event.
-bool inputter_t::have_mouse_tracking_csi() {
+        // If it matches the char, then increment the index.
-    // Maximum length of any CSI is NPAR (which is nominally 16), although this does not account for
+        if (peeked_.at(idx_).maybe_char() == c) {
-    // user input intermixed with pseudo input generated by the tty emulator.
+            idx_++;
-    event_queue_peeker_t peeker(event_queue_);
+            return true;
-
+        }
-    // Check for the CSI first
-    if (peeker.next().maybe_char() != L'\x1B'
-            || peeker.next(true /* timed */).maybe_char() != L'[') {
         return false;
     }
 
-    auto next = peeker.next().maybe_char();
+    /// \return the current index.
+    size_t len() const { return idx_; }
+
+    /// Consume all events up to the current index.
+    /// Remaining events are returned to the queue.
+    void consume() {
+        event_queue_.insert_front(peeked_.cbegin() + idx_, peeked_.cend());
+        peeked_.clear();
+        idx_ = 0;
+    }
+
+    /// Reset our index back to 0.
+    void restart() { idx_ = 0; }
+
+    ~event_queue_peeker_t() {
+        assert(idx_ == 0 && "Events left on the queue - missing restart or consume?");
+        consume();
+    }
+
+   private:
+    /// The list of events which have been dequeued.
+    std::vector<char_event_t> peeked_{};
+
+    /// If set, then some previous timed event timed out.
+    bool had_timeout_{false};
+
+    /// The current index. This never exceeds peeked_.size().
+    size_t idx_{0};
+
+    /// The queue from which to read more events.
+    input_event_queue_t &event_queue_;
+};
+
+/// Try reading a mouse-tracking CSI sequence, using the given \p peeker.
+/// Events are left on the peeker and the caller must restart or consume it.
+/// \return true if matched, false if not.
+static bool have_mouse_tracking_csi(event_queue_peeker_t *peeker) {
+    // Maximum length of any CSI is NPAR (which is nominally 16), although this does not account for
+    // user input intermixed with pseudo input generated by the tty emulator.
+    // Check for the CSI first.
+    if (!peeker->next_is_char(L'\x1b') || !peeker->next_is_char(L'[', true /* timed */)) {
+        return false;
+    }
+
+    auto next = peeker->next().maybe_char();
     size_t length = 0;
     if (next == L'M') {
         // Generic X10 or modified VT200 sequence. It doesn't matter which, they're both 6 chars
@@ -538,13 +535,13 @@ bool inputter_t::have_mouse_tracking_csi() {
         // Extended (SGR/1006) mouse reporting mode, with semicolon-separated parameters for button
         // code, Px, and Py, ending with 'M' for button press or 'm' for button release.
         while (true) {
-            next = peeker.next().maybe_char();
+            next = peeker->next().maybe_char();
             if (next == L'M' || next == L'm') {
                 // However much we've read, we've consumed the CSI in its entirety.
-                length = peeker.len();
+                length = peeker->len();
                 break;
             }
-            if (peeker.len() == 16) {
+            if (peeker->len() >= 16) {
                 // This is likely a malformed mouse-reporting CSI but we can't do anything about it.
                 return false;
             }
@@ -561,18 +558,61 @@ bool inputter_t::have_mouse_tracking_csi() {
 
     // Consume however many characters it takes to prevent the mouse tracking sequence from reaching
     // the prompt, dependent on the class of mouse reporting as detected above.
-    while (peeker.len() < length) {
+    while (peeker->len() < length) {
-        (void)peeker.next();
+        (void)peeker->next();
     }
-
-    peeker.consume();
     return true;
 }
 
+/// \return true if a given \p peeker matches a given sequence of char events given by \p str.
+static bool try_peek_sequence(event_queue_peeker_t *peeker, const wcstring &str) {
+    assert(!str.empty() && "Empty string passed to try_peek_sequence");
+    wchar_t prev = L'\0';
+    for (wchar_t c : str) {
+        // If we just read an escape, we need to add a timeout for the next char,
+        // to distinguish between the actual escape key and an "alt"-modifier.
+        bool timed = prev == L'\x1B';
+        if (!peeker->next_is_char(c, timed)) {
+            return false;
+        }
+        prev = c;
+    }
+    return true;
+}
+
+/// \return the first mapping that matches, walking first over the user's mapping list, then the
+/// preset list. \return null if nothing matches.
+maybe_t<input_mapping_t> inputter_t::find_mapping(event_queue_peeker_t *peeker) {
+    const input_mapping_t *generic = nullptr;
+    const auto &vars = parser_->vars();
+    const wcstring bind_mode = input_get_bind_mode(vars);
+
+    auto ml = input_mappings()->all_mappings();
+    for (const auto &m : *ml) {
+        if (m.mode != bind_mode) {
+            continue;
+        }
+
+        // Defer generic mappings until the end.
+        if (m.is_generic()) {
+            if (!generic) generic = &m;
+            continue;
+        }
+
+        if (try_peek_sequence(peeker, m.seq)) {
+            return m;
+        }
+        peeker->restart();
+    }
+    return generic ? maybe_t<input_mapping_t>(*generic) : none();
+}
+
 void inputter_t::mapping_execute_matching_or_generic(const command_handler_t &command_handler) {
+    event_queue_peeker_t peeker(event_queue_);
+
     // Check for mouse-tracking CSI before mappings to prevent the generic mapping handler from
     // taking over.
-    if (have_mouse_tracking_csi()) {
+    if (have_mouse_tracking_csi(&peeker)) {
         // fish recognizes but does not actually support mouse reporting. We never turn it on, and
         // it's only ever enabled if a program we spawned enabled it and crashed or forgot to turn
         // it off before exiting. We swallow the events to prevent garbage from piling up at the
@@ -588,17 +628,26 @@ void inputter_t::mapping_execute_matching_or_generic(const command_handler_t &co
         // We can't/shouldn't directly manipulate stdout from `input.cpp`, so request the execution
         // of a helper function to disable mouse tracking.
         // writembs(outputter_t::stdoutput(), "\x1B[?1000l");
+        peeker.consume();
         event_queue_.push_front(char_event_t(readline_cmd_t::disable_mouse_tracking, L""));
+        return;
     }
-    else if (auto mapping = find_mapping()) {
+    peeker.restart();
+
+    // Check for ordinary mappings.
+    if (auto mapping = find_mapping(&peeker)) {
+        peeker.consume();
         mapping_execute(*mapping, command_handler);
-    } else {
+        return;
-        FLOGF(reader, L"no generic found, ignoring char...");
-        auto evt = event_queue_.readch();
-        if (evt.is_eof()) {
-            event_queue_.push_front(evt);
-        }
     }
+    peeker.restart();
+
+    FLOGF(reader, L"no generic found, ignoring char...");
+    auto evt = peeker.next();
+    if (evt.is_eof()) {
+        event_queue_.push_front(evt);
+    }
+    peeker.consume();
 }
 
 /// Helper function. Picks through the queue of incoming characters until we get to one that's not a

src/input.h

@@ -13,6 +13,7 @@
 #define FISH_BIND_MODE_VAR L"fish_bind_mode"
 #define DEFAULT_BIND_MODE L"default"
 
+class event_queue_peeker_t;
 class parser_t;
 
 wcstring describe_char(wint_t c);
@@ -67,9 +68,7 @@ class inputter_t {
     void function_push_args(readline_cmd_t code);
     void mapping_execute(const input_mapping_t &m, const command_handler_t &command_handler);
     void mapping_execute_matching_or_generic(const command_handler_t &command_handler);
-    bool mapping_is_match(const input_mapping_t &m);
+    maybe_t<input_mapping_t> find_mapping(event_queue_peeker_t *peeker);
-    bool have_mouse_tracking_csi();
-    maybe_t<input_mapping_t> find_mapping();
     char_event_t read_characters_no_readline();
 };