Remove timeout input events

Now that timeouts are stored in the event queue peeker, we can remove the
notion of timeout events altogether. Instead you may ask for an event with
a timeout, and get back none on timeout. This simplifies how input events
work.

src/fish_key_reader.cpp

@@ -229,13 +229,13 @@ static void process_input(bool continuous_mode) {
 
     std::fwprintf(stderr, L"Press a key:\n");
     for (;;) {
-        char_event_t evt{0};
+        maybe_t<char_event_t> evt{};
         if (reader_test_and_clear_interrupted()) {
             evt = char_event_t{shell_modes.c_cc[VINTR]};
         } else {
-            evt = queue.readch_timed(true);
+            evt = queue.readch_timed();
         }
-        if (!evt.is_char()) {
+        if (!evt || !evt->is_char()) {
             output_bind_command(bind_chars);
             if (first_char_seen && !continuous_mode) {
                 return;
@@ -243,7 +243,7 @@ static void process_input(bool continuous_mode) {
             continue;
         }
 
-        wchar_t wc = evt.get_char();
+        wchar_t wc = evt->get_char();
         prev_tstamp = output_elapsed_time(prev_tstamp, first_char_seen);
         // Hack for #3189. Do not suggest \c@ as the binding for nul, because a string containing
         // nul cannot be passed to builtin_bind since it uses C strings. We'll output the name of

src/input.cpp

@@ -462,10 +462,14 @@ class event_queue_peeker_t {
             return false;
         }
         // Grab a new event if we have exhausted what we have already peeked.
+        // Use either readch or readch_timed, per our param.
         if (idx_ == peeked_.size()) {
-            auto newevt = timed ? event_queue_.readch_timed() : event_queue_.readch();
-            if (newevt.is_timeout()) {
-                assert(timed && "Should only get timeouts from timed reads");
+            char_event_t newevt{L'\0'};
+            if (!timed) {
+                newevt = event_queue_.readch();
+            } else if (auto mevt = event_queue_.readch_timed()) {
+                newevt = mevt.acquire();
+            } else {
                 had_timeout_ = true;
                 return false;
             }

src/input_common.cpp

@@ -76,7 +76,7 @@ char_event_t input_event_queue_t::readb() {
                 if (interrupt_handler) {
                     if (auto interrupt_evt = interrupt_handler()) {
                         return *interrupt_evt;
-                    } else if (auto mc = pop_discard_timeouts()) {
+                    } else if (auto mc = try_pop()) {
                         return *mc;
                     }
                 }
@@ -94,7 +94,7 @@ char_event_t input_event_queue_t::readb() {
             if (barrier_from_poll || barrier_from_readability) {
                 if (env_universal_barrier()) {
                     // A variable change may have triggered a repaint, etc.
-                    if (auto mc = pop_discard_timeouts()) {
+                    if (auto mc = try_pop()) {
                         return *mc;
                     }
                 }
@@ -115,8 +115,8 @@ char_event_t input_event_queue_t::readb() {
             // This gives priority to the foreground.
             if (ioport > 0 && fdset.test(ioport)) {
                 iothread_service_main();
-                if (auto mc = pop_discard_timeouts()) {
-                    return *mc;
+                if (auto mc = try_pop()) {
+                    return mc.acquire();
                 }
             }
         }
@@ -143,26 +143,19 @@ void update_wait_on_escape_ms(const environment_t& vars) {
     }
 }
 
-char_event_t input_event_queue_t::pop() {
-    auto result = queue_.front();
+maybe_t<char_event_t> input_event_queue_t::try_pop() {
+    if (queue_.empty()) {
+        return none();
+    }
+    auto result = std::move(queue_.front());
     queue_.pop_front();
     return result;
 }
 
-maybe_t<char_event_t> input_event_queue_t::pop_discard_timeouts() {
-    while (has_lookahead()) {
-        auto evt = pop();
-        if (!evt.is_timeout()) {
-            return evt;
-        }
-    }
-    return none();
-}
-
 char_event_t input_event_queue_t::readch() {
     ASSERT_IS_MAIN_THREAD();
-    if (auto mc = pop_discard_timeouts()) {
-        return *mc;
+    if (auto mc = try_pop()) {
+        return mc.acquire();
     }
     wchar_t res;
     mbstate_t state = {};
@@ -199,23 +192,16 @@ char_event_t input_event_queue_t::readch() {
     }
 }
 
-char_event_t input_event_queue_t::readch_timed(bool dequeue_timeouts) {
-    char_event_t result{char_event_type_t::timeout};
-    if (has_lookahead()) {
-        result = pop();
-    } else {
-        const uint64_t usec_per_msec = 1000;
-        uint64_t timeout_usec = static_cast<uint64_t>(wait_on_escape_ms) * usec_per_msec;
-        if (select_wrapper_t::is_fd_readable(in_, timeout_usec)) {
-            result = readch();
-        }
+maybe_t<char_event_t> input_event_queue_t::readch_timed() {
+    if (auto evt = try_pop()) {
+        return evt;
     }
-    // If we got a timeout, either through dequeuing or creating, ensure it stays on the queue.
-    if (result.is_timeout()) {
-        if (!dequeue_timeouts) queue_.push_front(char_event_type_t::timeout);
-        return char_event_type_t::timeout;
+    const uint64_t usec_per_msec = 1000;
+    uint64_t timeout_usec = static_cast<uint64_t>(wait_on_escape_ms) * usec_per_msec;
+    if (select_wrapper_t::is_fd_readable(in_, timeout_usec)) {
+        return readch();
     }
-    return result;
+    return none();
 }
 
 void input_event_queue_t::push_back(const char_event_t& ch) { queue_.push_back(ch); }

src/input_common.h

@@ -99,18 +99,12 @@ enum class char_event_type_t : uint8_t {
     /// A readline event.
     readline,
 
-    /// A timeout was hit.
-    timeout,
-
     /// end-of-file was reached.
     eof,
 
     /// An event was handled internally, or an interrupt was received. Check to see if the reader
     /// loop should exit.
     check_exit,
-
-    /// There is no event. This should never happen, or is an assertion failure.
-    none,
 };
 
 /// Hackish: the input style, which describes how char events (only) are applied to the command
@@ -143,8 +137,6 @@ class char_event_t {
     /// Note that the generic self-insert case does not have any characters, so this would be empty.
     wcstring seq{};
 
-    bool is_timeout() const { return type == char_event_type_t::timeout; }
-
     bool is_char() const { return type == char_event_type_t::charc; }
 
     bool is_eof() const { return type == char_event_type_t::eof; }
@@ -171,8 +163,6 @@ class char_event_t {
         return v_.rl;
     }
 
-    explicit char_event_t() : type(char_event_type_t::none) { }
-
     /* implicit */ char_event_t(wchar_t c) : type(char_event_type_t::charc) { v_.c = c; }
 
     /* implicit */ char_event_t(readline_cmd_t rl, wcstring seq = {})
@@ -204,14 +194,13 @@ class input_event_queue_t {
 
     /// Function used by input_readch to read bytes from stdin until enough bytes have been read to
     /// convert them to a wchar_t. Conversion is done using mbrtowc. If a character has previously
-    /// been read and then 'unread' using \c input_common_unreadch, that character is returned. This
-    /// function never returns a timeout.
+    /// been read and then 'unread' using \c input_common_unreadch, that character is returned.
     char_event_t readch();
 
     /// Like readch(), except it will wait at most WAIT_ON_ESCAPE milliseconds for a
     /// character to be available for reading.
-    /// If \p dequeue_timeouts is set, remove any timeout from the queue; otherwise retain them.
-    char_event_t readch_timed(bool dequeue_timeouts = false);
+    /// \return none on timeout, the event on success.
+    maybe_t<char_event_t> readch_timed();
 
     /// Enqueue a character or a readline function to the queue of unread characters that
     /// readch will return before actually reading from fd 0.
@@ -233,11 +222,8 @@ class input_event_queue_t {
     /// \return if we have any lookahead.
     bool has_lookahead() const { return !queue_.empty(); }
 
-    /// \return the next event in the queue.
-    char_event_t pop();
-
-    /// \return the next event in the queue, discarding timeouts.
-    maybe_t<char_event_t> pop_discard_timeouts();
+    /// \return the next event in the queue, or none if the queue is empty.
+    maybe_t<char_event_t> try_pop();
 
     char_event_t readb();