From 64859fc24281d5d97b7f30dcf20a3a0fb80b4e72 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 25 Jan 2025 16:07:15 +0100 Subject: [PATCH] Blocking wait for responses to startup queries At startup we query for - the cursor position (CSI 6 n) - kitty keyboard protocol support (CSI ? u) - terminfo capabilities via XTGETTCAP Since we don't wait for responses, those can leak into child processes. Some child processes like fzf cannot decode DCS replies. Plug the leak by ending each round of querying by asking for the Primary Device Attribute, and resume input processing only after a response has been received, (or ctrl-c as an escape hatch). This is a nice simplification. Tested with the lowest common denominator (putty, Terminal.app and st). Fixes #11079 --- src/builtins/fish_key_reader.rs | 13 +++- src/input.rs | 28 +++---- src/input_common.rs | 102 ++++++++++++++------------ src/reader.rs | 125 ++++++++++++++++++++------------ src/screen.rs | 2 +- tests/pexpect_helper.py | 2 + 6 files changed, 159 insertions(+), 113 deletions(-) diff --git a/src/builtins/fish_key_reader.rs b/src/builtins/fish_key_reader.rs index 021010c82..587e0f34e 100644 --- a/src/builtins/fish_key_reader.rs +++ b/src/builtins/fish_key_reader.rs @@ -7,7 +7,7 @@ //! //! Type "exit" or "quit" to terminate the program. -use std::{ops::ControlFlow, os::unix::prelude::OsStrExt}; +use std::{io::Write, ops::ControlFlow, os::unix::prelude::OsStrExt}; use libc::{STDIN_FILENO, TCSANOW, VEOF, VINTR}; @@ -21,10 +21,11 @@ use crate::{ input_common::{ enable_kitty_progressive_enhancements, kitty_progressive_enhancements_query, terminal_protocol_hacks, terminal_protocols_enable_ifn, CharEvent, ImplicitEvent, - InputEventQueue, InputEventQueuer, + InputEventQueue, InputEventQueuer, KITTY_KEYBOARD_SUPPORTED, }, key::{char_to_symbol, Key}, nix::isatty, + output::Outputter, panic::panic_handler, print_help::print_help, proc::set_interactive_session, @@ -102,8 +103,12 @@ fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) let kevt = match evt { CharEvent::Key(kevt) => kevt, CharEvent::Readline(_) | CharEvent::Command(_) => continue, - CharEvent::Implicit(ImplicitEvent::KittyKeyboardSupported) => { - enable_kitty_progressive_enhancements(); + CharEvent::Implicit(ImplicitEvent::PrimaryDeviceAttribute) => { + if KITTY_KEYBOARD_SUPPORTED.load() { + enable_kitty_progressive_enhancements( + Outputter::stdoutput().borrow_mut().by_ref(), + ); + } continue; } CharEvent::Implicit(_) => continue, diff --git a/src/input.rs b/src/input.rs index 56cb365e1..3e05b7f78 100644 --- a/src/input.rs +++ b/src/input.rs @@ -6,10 +6,9 @@ use crate::flog::FLOG; // Polyfill for Option::is_none_or(), stabilized in 1.82.0 #[allow(unused_imports)] use crate::future::IsSomeAnd; -use crate::input_common::CursorPositionBlockingWait::MouseLeft; use crate::input_common::{ - CharEvent, CharInputStyle, CursorPositionWait, ImplicitEvent, InputData, InputEventQueuer, - ReadlineCmd, ReadlineCmdEvent, READING_BUFFERED_INPUT, R_END_INPUT_FUNCTIONS, + BlockingWait, CharEvent, CharInputStyle, CursorPositionWait, ImplicitEvent, InputData, + InputEventQueuer, ReadlineCmd, ReadlineCmdEvent, READING_BUFFERED_INPUT, R_END_INPUT_FUNCTIONS, }; use crate::key::ViewportPosition; use crate::key::{self, canonicalize_raw_escapes, ctrl, Key, Modifiers}; @@ -463,29 +462,26 @@ impl<'a> InputEventQueuer for Reader<'a> { ))); } - fn cursor_position_wait(&self) -> &CursorPositionWait { - &self.cursor_position_wait + fn is_blocked(&self) -> bool { + self.blocking_wait.is_some() } - fn is_blocked_waiting_for_cursor_position(&self) -> bool { - matches!(self.cursor_position_wait, CursorPositionWait::Blocking(_)) - } - fn cursor_position_reporting_supported(&mut self) { - assert!(self.cursor_position_wait == CursorPositionWait::InitialFeatureProbe); - self.cursor_position_wait = CursorPositionWait::None; - } - fn stop_waiting_for_cursor_position(&mut self) -> bool { - if !self.is_blocked_waiting_for_cursor_position() { + fn unblock_input(&mut self) -> bool { + if !self.is_blocked() { return false; } - self.cursor_position_wait = CursorPositionWait::None; + self.blocking_wait = None; true } + fn blocking_wait(&self) -> Option<&BlockingWait> { + self.blocking_wait.as_ref() + } + fn on_mouse_left_click(&mut self, position: ViewportPosition) { FLOG!(reader, "Mouse left click", position); self.request_cursor_position( &mut Outputter::stdoutput().borrow_mut(), - CursorPositionWait::Blocking(MouseLeft(position)), + Some(CursorPositionWait::MouseLeft(position)), ); } } diff --git a/src/input_common.rs b/src/input_common.rs index 8847969dc..3557c2d2f 100644 --- a/src/input_common.rs +++ b/src/input_common.rs @@ -192,14 +192,12 @@ pub enum ImplicitEvent { FocusOut, /// Request to disable mouse tracking. DisableMouseTracking, + /// Primary DA response. + PrimaryDeviceAttribute, /// Handle mouse left click. MouseLeftClickContinuation(ViewportPosition, ViewportPosition), /// Push prompt to top. ScrollbackPushContinuation(usize), - /// The Synchronized Output feature is supported by the terminal. - SynchronizedOutputSupported, - /// Terminal reports support for the kitty keyboard protocol. - KittyKeyboardSupported, } #[derive(Debug, Clone)] @@ -450,7 +448,12 @@ static TERMINAL_PROTOCOLS: AtomicBool = AtomicBool::new(false); pub(crate) static SCROLL_FORWARD_SUPPORTED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); pub(crate) static CURSOR_UP_SUPPORTED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); -static KITTY_KEYBOARD_SUPPORTED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); +pub(crate) static KITTY_KEYBOARD_SUPPORTED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); + +pub(crate) static SYNCHRONIZED_OUTPUT_SUPPORTED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); + +pub(crate) static CURSOR_POSITION_REPORTING_SUPPORTED: RelaxedAtomicBool = + RelaxedAtomicBool::new(false); macro_rules! kitty_progressive_enhancements { () => { @@ -465,11 +468,11 @@ pub fn kitty_progressive_enhancements_query() -> &'static [u8] { b"\x1b[?u" } -pub(crate) fn enable_kitty_progressive_enhancements() -> bool { +pub(crate) fn enable_kitty_progressive_enhancements(out: &mut impl std::io::Write) -> bool { if IN_MIDNIGHT_COMMANDER_PRE_CSI_U.load() || IN_ITERM_PRE_CSI_U.load() { return false; } - let _ = write_loop(&STDOUT_FILENO, kitty_progressive_enhancements!().as_bytes()); + let _ = out.write(kitty_progressive_enhancements!().as_bytes()); true } @@ -619,16 +622,22 @@ impl InputData { } #[derive(Eq, PartialEq)] -pub enum CursorPositionBlockingWait { +pub enum CursorPositionWait { MouseLeft(ViewportPosition), ScrollbackPush, } #[derive(Eq, PartialEq)] -pub enum CursorPositionWait { - None, - InitialFeatureProbe, - Blocking(CursorPositionBlockingWait), +pub enum Queried { + NotYet, + Once, + Twice, +} + +#[derive(Eq, PartialEq)] +pub enum BlockingWait { + Startup(Queried), + CursorPosition(CursorPositionWait), } /// A trait which knows how to produce a stream of input events. @@ -636,10 +645,10 @@ pub enum CursorPositionWait { pub trait InputEventQueuer { /// Return the next event in the queue, or none if the queue is empty. fn try_pop(&mut self) -> Option { - if self.is_blocked_waiting_for_cursor_position() { + if self.is_blocked() { match self.get_input_data().queue.front()? { CharEvent::Key(_) | CharEvent::Readline(_) | CharEvent::Command(_) => { - return None; // No code execution while we're waiting for CPR. + return None; // No code execution while blocked. } CharEvent::Implicit(_) => (), } @@ -763,10 +772,10 @@ pub trait InputEventQueuer { Some(seq.chars().skip(1).map(CharEvent::from_char)), ) }; - if self.is_blocked_waiting_for_cursor_position() { + if self.is_blocked() { FLOG!( reader, - "Still waiting for cursor position report from terminal, deferring key event", + "Still blocked on response from terminal, deferring key event", key_evt ); self.push_back(key_evt); @@ -779,9 +788,9 @@ pub trait InputEventQueuer { if vintr != 0 && key == Some(Key::from_single_byte(vintr)) { FLOG!( reader, - "Received interrupt key, giving up waiting for cursor position" + "Received interrupt key, giving up waiting for response from terminal" ); - let ok = self.stop_waiting_for_cursor_position(); + let ok = self.unblock_input(); assert!(ok); } continue; @@ -980,9 +989,8 @@ pub trait InputEventQueuer { if private_mode == Some(b'?') { // DECRPM if params[0][0] == 2026 && matches!(params[1][0], 1 | 2) { - self.push_front(CharEvent::Implicit( - ImplicitEvent::SynchronizedOutputSupported, - )); + FLOG!(reader, "Synchronized output is supported"); + SYNCHRONIZED_OUTPUT_SUPPORTED.store(true); } } // DECRQM @@ -1038,15 +1046,18 @@ pub trait InputEventQueuer { if code != 0 || c != b'M' || modifiers.is_some() { return None; } - match self.cursor_position_wait() { - CursorPositionWait::None => self.on_mouse_left_click(position), - CursorPositionWait::InitialFeatureProbe => (), - CursorPositionWait::Blocking(_) => { + let Some(wait) = self.blocking_wait() else { + self.on_mouse_left_click(position); + return None; + }; + match wait { + BlockingWait::Startup(_) => {} + BlockingWait::CursorPosition(_) => { // TODO: re-queue it I guess. FLOG!( - reader, - "Ignoring mouse left click received while still waiting for Cursor Position Report" - ); + reader, + "Ignoring mouse left click received while still waiting for Cursor Position Report" + ); } } return None; @@ -1072,22 +1083,18 @@ pub trait InputEventQueuer { let y = usize::try_from(params[0][0] - 1).unwrap(); let x = usize::try_from(params[1][0] - 1).unwrap(); FLOG!(reader, "Received cursor position report y:", y, "x:", x); - let blocking_wait = match self.cursor_position_wait() { - CursorPositionWait::None => return None, - CursorPositionWait::InitialFeatureProbe => { - self.cursor_position_reporting_supported(); - return None; - } - CursorPositionWait::Blocking(blocking_wait) => blocking_wait, + let Some(BlockingWait::CursorPosition(wait)) = self.blocking_wait() else { + CURSOR_POSITION_REPORTING_SUPPORTED.store(true); + return None; }; - let continuation = match blocking_wait { - CursorPositionBlockingWait::MouseLeft(click_position) => { + let continuation = match wait { + CursorPositionWait::MouseLeft(click_position) => { ImplicitEvent::MouseLeftClickContinuation( ViewportPosition { x, y }, *click_position, ) } - CursorPositionBlockingWait::ScrollbackPush => { + CursorPositionWait::ScrollbackPush => { ImplicitEvent::ScrollbackPushContinuation(y) } }; @@ -1143,6 +1150,10 @@ pub trait InputEventQueuer { } _ => return None, }, + b'c' if private_mode == Some(b'?') => { + self.push_front(CharEvent::Implicit(ImplicitEvent::PrimaryDeviceAttribute)); + return None; + } b'u' => { if private_mode == Some(b'?') { FLOG!( @@ -1150,7 +1161,6 @@ pub trait InputEventQueuer { "Received kitty progressive enhancement flags, marking as supported" ); KITTY_KEYBOARD_SUPPORTED.store(true); - self.push_front(CharEvent::Implicit(ImplicitEvent::KittyKeyboardSupported)); return None; } @@ -1500,16 +1510,16 @@ pub trait InputEventQueuer { } } - fn cursor_position_wait(&self) -> &CursorPositionWait { - &CursorPositionWait::InitialFeatureProbe + fn blocking_wait(&self) -> Option<&BlockingWait> { + None } - fn cursor_position_reporting_supported(&mut self) {} - fn is_blocked_waiting_for_cursor_position(&self) -> bool { + fn is_blocked(&self) -> bool { false } - fn stop_waiting_for_cursor_position(&mut self) -> bool { + fn unblock_input(&mut self) -> bool { false } + fn on_mouse_left_click(&mut self, _position: ViewportPosition) {} /// Override point for when we are about to (potentially) block in select(). The default does @@ -1523,10 +1533,10 @@ pub trait InputEventQueuer { let vintr = shell_modes().c_cc[libc::VINTR]; if vintr != 0 { let interrupt_evt = CharEvent::from_key(Key::from_single_byte(vintr)); - if self.stop_waiting_for_cursor_position() { + if self.unblock_input() { FLOG!( reader, - "Received interrupt, giving up on waiting for cursor position" + "Received interrupt, giving up on waiting for terminal response" ); self.push_back(interrupt_evt); } else { diff --git a/src/reader.rs b/src/reader.rs index 088a089d1..edf0e4224 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -83,11 +83,14 @@ use crate::history::{ use crate::input::init_input; use crate::input_common::enable_kitty_progressive_enhancements; use crate::input_common::kitty_progressive_enhancements_query; -use crate::input_common::CursorPositionBlockingWait; +use crate::input_common::BlockingWait; use crate::input_common::CursorPositionWait; use crate::input_common::ImplicitEvent; use crate::input_common::InputEventQueuer; +use crate::input_common::Queried; use crate::input_common::IN_MIDNIGHT_COMMANDER_PRE_CSI_U; +use crate::input_common::KITTY_KEYBOARD_SUPPORTED; +use crate::input_common::SYNCHRONIZED_OUTPUT_SUPPORTED; use crate::input_common::{ terminal_protocol_hacks, terminal_protocols_enable_ifn, CharEvent, CharInputStyle, InputData, ReadlineCmd, @@ -123,6 +126,7 @@ use crate::proc::{ print_exit_warning_for_jobs, proc_update_jiffies, }; use crate::reader_history_search::{smartcase_flags, ReaderHistorySearch, SearchMode}; +use crate::screen::is_dumb; use crate::screen::{screen_clear, screen_force_clear_to_end, CharOffset, Screen}; use crate::signal::{ signal_check_cancel, signal_clear_cancel, signal_reset_handlers, signal_set_handlers, @@ -516,7 +520,7 @@ pub struct ReaderData { /// The representation of the current screen contents. screen: Screen, - pub cursor_position_wait: CursorPositionWait, + pub blocking_wait: Option, /// Data associated with input events. /// This is made public so that InputEventQueuer can be implemented on us. @@ -1171,7 +1175,7 @@ impl ReaderData { last_flash: Default::default(), flash_autosuggestion: false, screen: Screen::new(), - cursor_position_wait: CursorPositionWait::None, + blocking_wait: Some(BlockingWait::Startup(Queried::NotYet)), input_data, queued_repaint: false, history, @@ -1405,10 +1409,12 @@ impl ReaderData { pub fn request_cursor_position( &mut self, out: &mut Outputter, - cursor_position_wait: CursorPositionWait, + cursor_position_wait: Option, ) { - assert!(self.cursor_position_wait == CursorPositionWait::None); - self.cursor_position_wait = cursor_position_wait; + if let Some(cursor_position_wait) = cursor_position_wait { + assert!(self.blocking_wait.is_none()); + self.blocking_wait = Some(BlockingWait::CursorPosition(cursor_position_wait)); + } let _ = out.write(b"\x1b[6n"); self.save_screen_state(); } @@ -2102,6 +2108,8 @@ impl ReaderData { } } +const QUERY_PRIMARY_DEVICE_ATTRIBUTE: &[u8] = b"\x1b[0c"; + impl<'a> Reader<'a> { /// Read a command to execute, respecting input bindings. /// Return the command, or none if we were asked to cancel (e.g. SIGHUP). @@ -2157,18 +2165,22 @@ impl<'a> Reader<'a> { } } - static QUERIED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); - if !QUERIED.load() { - QUERIED.store(true); - let mut out = Outputter::stdoutput().borrow_mut(); - out.begin_buffering(); - // Query for kitty keyboard protocol support. - let _ = out.write(kitty_progressive_enhancements_query()); - // Query for cursor position reporting support. - zelf.request_cursor_position(&mut out, CursorPositionWait::InitialFeatureProbe); - // Query for synchronized output support. - let _ = out.write(b"\x1b[?2026$p"); - out.end_buffering(); + if zelf.blocking_wait == Some(BlockingWait::Startup(Queried::NotYet)) { + if is_dumb() { + zelf.blocking_wait = None; + } else { + zelf.blocking_wait = Some(BlockingWait::Startup(Queried::Once)); + let mut out = Outputter::stdoutput().borrow_mut(); + out.begin_buffering(); + // Query for kitty keyboard protocol support. + let _ = out.write(kitty_progressive_enhancements_query()); + // Query for cursor position reporting support. + zelf.request_cursor_position(&mut out, None); + // Query for synchronized output support. + let _ = out.write(b"\x1b[?2026$p"); + let _ = out.write(QUERY_PRIMARY_DEVICE_ATTRIBUTE); + out.end_buffering(); + } } // HACK: Don't abandon line for the first prompt, because @@ -2482,23 +2494,50 @@ impl<'a> Reader<'a> { .write_wstr(L!("\x1B[?1000l")); self.save_screen_state(); } + ImplicitEvent::PrimaryDeviceAttribute => { + let Some(wait) = &self.blocking_wait else { + // Rogue reply. + return ControlFlow::Continue(()); + }; + let BlockingWait::Startup(stage) = wait else { + // Rogue reply. + return ControlFlow::Continue(()); + }; + match stage { + Queried::NotYet => panic!(), + Queried::Once => { + let mut out = Outputter::stdoutput().borrow_mut(); + out.begin_buffering(); + let mut querying = false; + if KITTY_KEYBOARD_SUPPORTED.load() { + enable_kitty_progressive_enhancements(out.by_ref()); + querying = true; + } + if SYNCHRONIZED_OUTPUT_SUPPORTED.load() { + query_capabilities_via_dcs(out.by_ref()); + querying = true; + } + if querying { + let _ = out.write(QUERY_PRIMARY_DEVICE_ATTRIBUTE); + } + out.end_buffering(); + if querying { + self.save_screen_state(); + self.blocking_wait = Some(BlockingWait::Startup(Queried::Twice)); + return ControlFlow::Continue(()); + } + } + Queried::Twice => (), + } + self.unblock_input(); + } ImplicitEvent::MouseLeftClickContinuation(cursor, click_position) => { self.mouse_left_click(cursor, click_position); - self.stop_waiting_for_cursor_position(); + self.unblock_input(); } ImplicitEvent::ScrollbackPushContinuation(cursor_y) => { self.screen.push_to_scrollback(cursor_y); - self.stop_waiting_for_cursor_position(); - } - ImplicitEvent::SynchronizedOutputSupported => { - if query_capabilities_via_dcs() { - self.save_screen_state(); - } - } - ImplicitEvent::KittyKeyboardSupported => { - if enable_kitty_progressive_enhancements() { - self.save_screen_state(); - } + self.unblock_input(); } }, } @@ -2518,22 +2557,13 @@ fn xtgettcap(out: &mut impl Write, cap: &str) { let _ = write!(out, "\x1bP+q{}\x1b\\", DisplayAsHex(cap)); } -fn query_capabilities_via_dcs() -> bool { - static QUERIED: RelaxedAtomicBool = RelaxedAtomicBool::new(false); - if QUERIED.load() { - return false; - } - QUERIED.store(true); - let mut out = Outputter::stdoutput().borrow_mut(); - out.begin_buffering(); +fn query_capabilities_via_dcs(out: &mut impl std::io::Write) { let _ = out.write(b"\x1b[?2026h"); // begin synchronized update let _ = out.write(b"\x1b[?1049h"); // enable alternative screen buffer xtgettcap(out.by_ref(), "indn"); xtgettcap(out.by_ref(), "cuu"); let _ = out.write(b"\x1b[?1049l"); // disable alternative screen buffer let _ = out.write(b"\x1b[?2026l"); // end synchronized update - out.end_buffering(); - true } impl<'a> Reader<'a> { @@ -3758,13 +3788,16 @@ impl<'a> Reader<'a> { if !SCROLL_FORWARD_SUPPORTED.load() || !CURSOR_UP_SUPPORTED.load() { return; } - match self.cursor_position_wait() { - CursorPositionWait::None => self.request_cursor_position( + let Some(wait) = self.blocking_wait() else { + self.request_cursor_position( &mut Outputter::stdoutput().borrow_mut(), - CursorPositionWait::Blocking(CursorPositionBlockingWait::ScrollbackPush), - ), - CursorPositionWait::InitialFeatureProbe => (), - CursorPositionWait::Blocking(_) => { + Some(CursorPositionWait::ScrollbackPush), + ); + return; + }; + match wait { + BlockingWait::Startup(_) => panic!(), + BlockingWait::CursorPosition(_) => { // TODO: re-queue it I guess. FLOG!( reader, diff --git a/src/screen.rs b/src/screen.rs index d94dbde7b..729badb7e 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -1886,7 +1886,7 @@ fn line_shared_prefix(a: &Line, b: &Line) -> usize { } /// Returns true if we are using a dumb terminal. -fn is_dumb() -> bool { +pub(crate) fn is_dumb() -> bool { term().is_none_or(|term| { term.cursor_up.is_none() || term.cursor_down.is_none() diff --git a/tests/pexpect_helper.py b/tests/pexpect_helper.py index 145005650..85c0bab6c 100644 --- a/tests/pexpect_helper.py +++ b/tests/pexpect_helper.py @@ -175,6 +175,8 @@ class SpawnedProc(object): ) self.spawn.delaybeforesend = None self.prompt_counter = 0 + if env.get("TERM") != "dumb": + self.spawn.send('\x1b[?123c') # Primary Device Attribute def time_since_first_message(self): """Return a delta in seconds since the first message, or 0 if this is the first."""