From c203c88c66bc5d0c2943f55a4db15f2f1b01ec2d Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 6 Feb 2021 17:05:46 -0600 Subject: [PATCH 1/3] Add and use `event_queue_t::insert_front()` This allows directly inserting multiple characters/events in one go at the front of the input queue, instead of needing to add them one-by-one in reverse order. In addition to improving performance in case of fragmented dequeue allocation, this also is less error prone since a dev need not remember to use reverse iterators when looping over a vector of peeked events. --- src/input.cpp | 17 +++++------------ src/input_common.h | 8 ++++++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/input.cpp b/src/input.cpp index ffaf6558f..c537ea0f3 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -383,10 +383,7 @@ void inputter_t::mapping_execute(const input_mapping_t &m, if (has_commands && !command_handler) { // We don't want to run commands yet. Put the characters back and return check_exit. - for (wcstring::const_reverse_iterator it = m.seq.rbegin(), end = m.seq.rend(); it != end; - ++it) { - event_queue_.push_front(*it); - } + event_queue_.insert_front(m.seq.cbegin(), m.seq.cend()); event_queue_.push_front(char_event_type_t::check_exit); return; // skip the input_set_bind_mode } else if (has_functions && !has_commands) { @@ -424,7 +421,7 @@ bool inputter_t::mapping_is_match(const input_mapping_t &m) { 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 + // something else). Undo consumption of the read characters since we didn't match the // bind sequence and abort. event_queue_.push_front(evt); while (i--) event_queue_.push_front(str[i]); @@ -496,10 +493,8 @@ char_event_t inputter_t::read_characters_no_readline() { break; } } - // Restore any readline functions, in reverse to preserve their original order. - for (auto iter = saved_events.rbegin(); iter != saved_events.rend(); ++iter) { - event_queue_.push_front(*iter); - } + // Restore any readline functions + event_queue_.insert_front(saved_events.cbegin(), saved_events.cend()); return evt_to_return; } @@ -516,9 +511,7 @@ char_event_t inputter_t::readch(const command_handler_t &command_handler) { case readline_cmd_t::self_insert_notfirst: { // Typically self-insert is generated by the generic (empty) binding. // However if it is generated by a real sequence, then insert that sequence. - for (auto iter = evt.seq.crbegin(); iter != evt.seq.crend(); ++iter) { - event_queue_.push_front(*iter); - } + event_queue_.insert_front(evt.seq.cbegin(), evt.seq.cend()); // Issue #1595: ensure we only insert characters, not readline functions. The // common case is that this will be empty. char_event_t res = read_characters_no_readline(); diff --git a/src/input_common.h b/src/input_common.h index 35cf30387..4b5ebe21f 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -221,6 +221,14 @@ class input_event_queue_t { /// Add a character or a readline function to the front of the queue of unread characters. This /// will be the next character returned by readch. void push_front(const char_event_t& ch); + + /// Add multiple characters or readline events to the front of the queue of unread characters. + /// The order of the provided events is not changed, i.e. they are not inserted in reverse + /// order. + template + void insert_front(const Iterator begin, const Iterator end) { + queue_.insert(queue_.begin(), begin, end); + } }; #endif From cc392b37744686992449d8c994b1662bcb4e1190 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 6 Feb 2021 17:11:05 -0600 Subject: [PATCH 2/3] Add RAII-based `event_queue_peeker_t` helper This is a stack-allocating utility class to peek up to N characters/events out of an `event_queue_t` object. The need for a hard-coded maximum peek length N at each call site is to avoid any heap allocation, as this would be called in a hot path on every input event. --- src/input.cpp | 42 ++++++++++++++++++++++++++++++++++++++++++ src/input_common.h | 13 +++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/input.cpp b/src/input.cpp index c537ea0f3..f53fc3e21 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -467,6 +467,48 @@ maybe_t inputter_t::find_mapping() { return generic ? maybe_t(*generic) : none(); } +template +class event_queue_peeker_t { + private: + input_event_queue_t &event_queue_; + std::array peeked_; + size_t count = 0; + bool consumed_ = false; + + public: + event_queue_peeker_t(input_event_queue_t &event_queue) + : event_queue_(event_queue) { + } + + char_event_t next(bool timed = false) { + assert(count < N && "Insufficient backing array size!"); + auto event = timed ? event_queue_.readch_timed() : event_queue_.readch(); + peeked_[count++] = event; + return event; + } + + size_t len() { + return count; + } + + void consume() { + consumed_ = true; + } + + void restart() { + if (count > 0) { + event_queue_.insert_front(peeked_.cbegin(), peeked_.cbegin() + count); + count = 0; + } + } + + ~event_queue_peeker_t() { + if (!consumed_) { + restart(); + } + } +}; + void inputter_t::mapping_execute_matching_or_generic(const command_handler_t &command_handler) { if (auto mapping = find_mapping()) { mapping_execute(*mapping, command_handler); diff --git a/src/input_common.h b/src/input_common.h index 4b5ebe21f..725d10cd8 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -107,6 +107,9 @@ enum class char_event_type_t : uint8_t { /// 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 @@ -154,11 +157,21 @@ class char_event_t { return v_.c; } + maybe_t maybe_char() const { + if (type == char_event_type_t::charc) { + return v_.c; + } else { + return none(); + } + } + readline_cmd_t get_readline() const { assert(type == char_event_type_t::readline && "Not a readline type"); 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 = {}) From eecc223c512e0db52386db3f9b7c589e4caf25f0 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 6 Feb 2021 17:13:27 -0600 Subject: [PATCH 3/3] Recognize and disable mouse-tracking CSI events Fish was previously oblivious to the existence of mouse-tracking ANSI escapes; this was mostly OK because they're disabled by default and we don't enable them, but if a TUI application that turned on mouse reporting crashed or exited without turning mouse reporting off, fish would be left in an unusable state as all mouse reporting CSI sequences would be posted to the prompt. This can be tested by executing `printf '\x1b[?1003h'` at the prompt, then clicking with any mouse button anywhere within the terminal window. Previously, this would have resulted in seeming garbage being spewed to the prompt; now, fish detects the mouse tracking CSIs posted to stdin by the terminal emulator and a) ignores them to prevent invalid input, as well as b) posts the CSI needed to disable future mouse tracking events from being emitted on subsequent mouse interactions (until re-enabled). Note that since we respond to a mouse tracking CSI rather than pre-emptively disable mouse reporting, we do not need to do any sort of feature detection to determine whether or not the terminal supports mouse reporting (otherwise, if it didn't support it and we posted the CSI anyway, we'd end up with exactly the kind of cruft posted to the prompt that we're trying to avoid). Fixes #4873 --- src/input.cpp | 61 +++++++++++++++++++++++++++++++++++++++++++++- src/input.h | 1 + src/input_common.h | 1 + src/reader.cpp | 5 ++++ 4 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/input.cpp b/src/input.cpp index f53fc3e21..aa76b17eb 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -159,6 +159,7 @@ static const input_function_metadata_t input_function_metadata[] = { {readline_cmd_t::redo, L"redo"}, {readline_cmd_t::begin_undo_group, L"begin-undo-group"}, {readline_cmd_t::end_undo_group, L"end-undo-group"}, + {readline_cmd_t::disable_mouse_tracking, L"disable-mouse-tracking"}, }; static_assert(sizeof(input_function_metadata) / sizeof(input_function_metadata[0]) == @@ -509,8 +510,66 @@ class event_queue_peeker_t { } }; +bool inputter_t::have_mouse_tracking_csi() { + event_queue_peeker_t<12> peeker(event_queue_); + + // 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(); + size_t length = 0; + if (next == L'M') { + // Generic X10 or modified VT200 sequence. It doesn't matter which, they're both 6 chars + // reporting the button that was clicked and its location. + length = 6; + } else if (next == L'P') { + // VT200 mouse highlighting. 12 characters, comes after generic button press event. + length = 12; + } else if (next == L't') { + // VT200 button released in mouse highlighting mode at valid text location. 5 chars. + length = 5; + } else if (next == L'T') { + // VT200 button released in mouse highlighting mode past end-of-line. 9 characters. + length = 9; + } else { + return false; + } + + // 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. + peeker.consume(); + while (peeker.len() != length) { + auto _ = peeker.next(); + } + + return true; +} + void inputter_t::mapping_execute_matching_or_generic(const command_handler_t &command_handler) { - if (auto mapping = find_mapping()) { + // Check for mouse-tracking CSI before mappings to prevent the generic mapping handler from + // taking over. + if (have_mouse_tracking_csi()) { + // 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 + // prompt, but don't do anything further with the received codes. To prevent this from + // breaking user interaction with the tty emulator, wasting CPU, and adding latency to the + // event queue, we turn off mouse reporting here. + // + // Since this is only called when we detect an incoming mouse reporting payload, we know the + // terminal emulator supports the xterm ANSI extensions for mouse reporting and can safely + // issue this without worrying about termcap. + FLOGF(reader, "Disabling mouse tracking"); + + // 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"); + event_queue_.push_front(char_event_t(readline_cmd_t::disable_mouse_tracking, L"")); + } + else if (auto mapping = find_mapping()) { mapping_execute(*mapping, command_handler); } else { FLOGF(reader, L"no generic found, ignoring char..."); diff --git a/src/input.h b/src/input.h index 3f10208fe..0132643ab 100644 --- a/src/input.h +++ b/src/input.h @@ -68,6 +68,7 @@ class inputter_t { 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); + bool have_mouse_tracking_csi(); maybe_t find_mapping(); char_event_t read_characters_no_readline(); }; diff --git a/src/input_common.h b/src/input_common.h index 725d10cd8..19e471993 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -83,6 +83,7 @@ enum class readline_cmd_t { begin_undo_group, end_undo_group, repeat_jump, + disable_mouse_tracking, // NOTE: This one has to be last. reverse_repeat_jump }; diff --git a/src/reader.cpp b/src/reader.cpp index 50c1788a0..dc7c27b4f 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -3781,6 +3781,11 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat el->end_edit_group(); break; } + case rl::disable_mouse_tracking: { + outputter_t &outp = outputter_t::stdoutput(); + outp.writestr(L"\x1B[?1000l"); + break; + } // Some commands should have been handled internally by inputter_t::readch(). case rl::self_insert: case rl::self_insert_notfirst: