Query terminal before reading config

We still have terminal-specific workarounds based on TERM_PROGRAM and
others, see test/test_driver.py.  In future we should get rid of them.

They are also unreliable, potentially missing inside SSH/containers,
incorrect if a terminal was started from another terminal (#11812);
also TERM can be incorrect for many reasons.

The better criterion for terminal-specific workarounds is XTVERSION,
which has none of the above disadvantages.

Since some of the workarounds (tmux, iTerm2) need to be applied before
we draw the first prompt. This also means: before we read any config
because config may call builtin "read".

Do startup queries before reading config.

Some changes implied by this:
1. Remove a call to init_input() which is already done by env_init()
2. call initialize_tty_metadata() only after queries have returned
3. Since we call initialize_tty_metadata() before the first
   call to tty.enable_tty_protocols() in Reader::readline(),
   we can remove the redundant call from reader_interactive_init().

src/bin/fish.rs

@@ -44,6 +44,7 @@
     flog::{self, activate_flog_categories_by_pattern, set_flog_file_fd, FLOG, FLOGF},
     fprintf, function, future_feature_flags as features,
     history::{self, start_private_mode},
+    input_common::InputEventQueuer,
     io::IoChain,
     nix::{getpid, getrusage, isatty, RUsage},
     panic::panic_handler,
@@ -57,7 +58,7 @@
         get_login, is_interactive_session, mark_login, mark_no_exec, proc_init,
         set_interactive_session, Pid,
     },
-    reader::{reader_init, reader_read, term_copy_modes},
+    reader::{reader_init, reader_read, term_copy_modes, terminal_init},
     signal::{signal_clear_cancel, signal_unblock_all},
     threads::{self},
     topic_monitor,
@@ -522,6 +523,38 @@ fn throwing_main() -> i32 {
     let parser = &Parser::new(env, CancelBehavior::Clear);
     parser.set_syncs_uvars(!opts.no_config);
 
+    #[derive(Eq, PartialEq)]
+    enum CommandSource {
+        Arguments,
+        Stdin,
+        File,
+    }
+    let command_source = if !opts.batch_cmds.is_empty() {
+        CommandSource::Arguments
+    } else if my_optind == args.len() {
+        CommandSource::Stdin
+    } else {
+        CommandSource::File
+    };
+
+    if command_source == CommandSource::Stdin && isatty(STDIN_FILENO) {
+        // Implicitly interactive mode.
+        if opts.no_exec {
+            FLOG!(
+                error,
+                "no-execute mode enabled and no script given. Exiting"
+            );
+            // above line should always exit
+            return libc::EXIT_FAILURE;
+        }
+        let mut input_queue = terminal_init();
+        let input_data = input_queue.get_input_data_mut();
+        parser
+            .pending_input
+            .borrow_mut()
+            .extend(std::mem::take(&mut input_data.queue));
+    }
+
     if !opts.no_exec && !opts.no_config {
         read_init(parser, config_paths.as_ref().unwrap());
     }
@@ -563,7 +596,7 @@ fn throwing_main() -> i32 {
     // Clear signals in case we were interrupted (#9024).
     signal_clear_cancel();
 
-    if !opts.batch_cmds.is_empty() {
+    if command_source == CommandSource::Arguments {
         // Run the commands specified as arguments, if any.
         if get_login() {
             // Do something nasty to support OpenSUSE assuming we're bash. This may modify cmds.
@@ -580,17 +613,8 @@ fn throwing_main() -> i32 {
         );
         res = run_command_list(parser, &opts.batch_cmds);
         parser.libdata_mut().exit_current_script = false;
-    } else if my_optind == args.len() {
+    } else if command_source == CommandSource::Stdin {
-        // Implicitly interactive mode.
+        res = reader_read(parser, STDIN_FILENO, &IoChain::new());
-        if opts.no_exec && isatty(libc::STDIN_FILENO) {
-            FLOG!(
-                error,
-                "no-execute mode enabled and no script given. Exiting"
-            );
-            // above line should always exit
-            return libc::EXIT_FAILURE;
-        }
-        res = reader_read(parser, libc::STDIN_FILENO, &IoChain::new());
     } else {
         let n = wcs2string(&args[my_optind]);
         let path = OsStr::from_bytes(&n);

src/builtins/fish_key_reader.rs

@@ -7,10 +7,9 @@
 //!
 //! Type "exit" or "quit" to terminate the program.
 
-use std::{cell::RefCell, ops::ControlFlow, os::unix::prelude::OsStrExt};
+use std::{ops::ControlFlow, os::unix::prelude::OsStrExt};
 
 use libc::{STDIN_FILENO, VEOF, VINTR};
-use once_cell::unsync::OnceCell;
 
 #[allow(unused_imports)]
 use crate::future::IsSomeAnd;
@@ -21,7 +20,7 @@
     future_feature_flags,
     input_common::{
         match_key_event_to_key, CharEvent, ImplicitEvent, InputEventQueue, InputEventQueuer,
-        KeyEvent, QueryResponse, QueryResultEvent, TerminalQuery,
+        KeyEvent, QueryResultEvent,
     },
     key::{char_to_symbol, Key},
     nix::isatty,
@@ -29,13 +28,11 @@
     print_help::print_help,
     proc::set_interactive_session,
     reader::{
-        check_exit_loop_maybe_warning, initial_query, reader_init, reader_sighup, set_shell_modes,
+        check_exit_loop_maybe_warning, reader_init, reader_sighup, set_shell_modes, terminal_init,
     },
-    signal::signal_set_handlers,
-    terminal::Capability,
     threads,
     topic_monitor::topic_monitor_init,
-    tty_handoff::{get_kitty_keyboard_capability, set_kitty_keyboard_capability, TtyHandoff},
+    tty_handoff::TtyHandoff,
     wchar::prelude::*,
     wgetopt::{wopt, ArgType, WGetopter, WOption},
 };
@@ -84,9 +81,13 @@ fn should_exit(
 }
 
 /// Process the characters we receive as the user presses keys.
-fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) -> BuiltinResult {
+fn process_input(
+    streams: &mut IoStreams,
+    continuous_mode: bool,
+    verbose: bool,
+    mut input_queue: InputEventQueue,
+) -> BuiltinResult {
     let mut first_char_seen = false;
-    let mut queue = InputEventQueue::new(STDIN_FILENO);
     let mut recent_chars = vec![];
     streams.err.appendln("Press a key:\n");
 
@@ -95,20 +96,15 @@ fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool)
 
     while (!first_char_seen || continuous_mode) && !check_exit_loop_maybe_warning(None) {
         use QueryResultEvent::*;
-        let kevt = match queue.readch() {
+        let kevt = match input_queue.readch() {
             CharEvent::Implicit(ImplicitEvent::Eof) => {
                 reader_sighup();
                 continue;
             }
             CharEvent::Key(kevt) => kevt,
             CharEvent::Readline(_) | CharEvent::Command(_) | CharEvent::Implicit(_) => continue,
-            CharEvent::QueryResult(Response(QueryResponse::PrimaryDeviceAttribute) | Timeout) => {
+            CharEvent::QueryResult(Timeout) => panic!("should not be querying"),
-                if get_kitty_keyboard_capability() == Capability::Unknown {
+            CharEvent::QueryResult(Response(_)) => continue,
-                    set_kitty_keyboard_capability(|| {}, Capability::NotSupported);
-                }
-                continue;
-            }
-            CharEvent::QueryResult(_) => continue,
         };
         if verbose {
             streams.out.append(L!("# decoded from: "));
@@ -154,6 +150,7 @@ fn setup_and_process_keys(
     streams: &mut IoStreams,
     continuous_mode: bool,
     verbose: bool,
+    input_queue: InputEventQueue,
 ) -> BuiltinResult {
     // We need to set the shell-modes for ICRNL,
     // in fish-proper this is done once a command is run.
@@ -173,7 +170,7 @@ fn setup_and_process_keys(
         streams.err.appendln(L!("\n"));
     }
 
-    process_input(streams, continuous_mode, verbose)
+    process_input(streams, continuous_mode, verbose, input_queue)
 }
 
 fn parse_flags(
@@ -261,7 +258,12 @@ pub fn fish_key_reader(
         return Err(STATUS_CMD_ERROR);
     }
 
-    setup_and_process_keys(streams, continuous_mode, verbose)
+    setup_and_process_keys(
+        streams,
+        continuous_mode,
+        verbose,
+        InputEventQueue::new(streams.stdin_fd),
+    )
 }
 
 pub fn main() {
@@ -311,9 +313,8 @@ fn throwing_main() -> i32 {
         return 1;
     }
 
-    signal_set_handlers(true);
+    let input_queue = terminal_init();
-    let blocking_query: OnceCell<RefCell<Option<TerminalQuery>>> = OnceCell::new();
-    initial_query(streams.stdin_fd, &blocking_query, streams.out, None);
 
-    setup_and_process_keys(&mut streams, continuous_mode, verbose).builtin_status_code()
+    setup_and_process_keys(&mut streams, continuous_mode, verbose, input_queue)
+        .builtin_status_code()
 }

src/input_common.rs

@@ -401,6 +401,8 @@ pub enum ImplicitEvent {
     /// An event was handled internally, or an interrupt was received. Check to see if the reader
     /// loop should exit.
     CheckExit,
+    /// A blocking terminal query was interrupterd with ctrl-c.
+    QueryInterrupted,
     /// Our terminal window gained focus.
     FocusIn,
     /// Our terminal window lost focus.
@@ -801,7 +803,8 @@ fn try_pop(&mut self) -> Option<CharEvent> {
         if self.is_blocked_querying() {
             use ImplicitEvent::*;
             match self.get_input_data().queue.front()? {
-                CharEvent::QueryResult(_) | CharEvent::Implicit(CheckExit | Eof) => {}
+                CharEvent::QueryResult(_)
+                | CharEvent::Implicit(CheckExit | Eof | QueryInterrupted) => {}
                 CharEvent::Key(_)
                 | CharEvent::Readline(_)
                 | CharEvent::Command(_)
@@ -964,6 +967,7 @@ fn readch(&mut self) -> CharEvent {
                             let ok = stop_query(self.blocking_query());
                             assert!(ok);
                             self.get_input_data_mut().queue.clear();
+                            self.push_front(CharEvent::Implicit(ImplicitEvent::QueryInterrupted));
                         }
                         continue;
                     }
@@ -1284,6 +1288,7 @@ fn parse_csi(&mut self, buffer: &mut Vec<u8>) -> Option<KeyEvent> {
                 _ => return None,
             },
             b'c' if private_mode == Some(b'?') => {
+                FLOG!(reader, "Received primary device attribute response");
                 self.push_front(CharEvent::QueryResult(QueryResultEvent::Response(
                     QueryResponse::PrimaryDeviceAttribute,
                 )));
@@ -1666,6 +1671,7 @@ fn enqueue_interrupt_key(&mut self) {
                     "Received interrupt, giving up on waiting for terminal response"
                 );
                 self.get_input_data_mut().queue.clear();
+                self.push_front(CharEvent::Implicit(ImplicitEvent::QueryInterrupted));
             } else {
                 self.push_front(interrupt_evt);
             }

src/parser.rs

@@ -14,7 +14,7 @@
 };
 use crate::fds::{open_dir, BEST_O_SEARCH};
 use crate::global_safety::RelaxedAtomicBool;
-use crate::input_common::TerminalQuery;
+use crate::input_common::{CharEvent, TerminalQuery};
 use crate::io::IoChain;
 use crate::job_group::MaybeJobId;
 use crate::operation_context::{OperationContext, EXPANSION_LIMIT_DEFAULT};
@@ -34,10 +34,10 @@
 use crate::wutil::perror;
 use crate::{function, FLOG};
 use libc::c_int;
-use once_cell::unsync::OnceCell;
 #[cfg(not(target_has_atomic = "64"))]
 use portable_atomic::AtomicU64;
 use std::cell::{Ref, RefCell, RefMut};
+use std::collections::VecDeque;
 use std::ffi::{CStr, OsStr};
 use std::fs::File;
 use std::io::Write;
@@ -444,7 +444,9 @@ pub struct Parser {
     /// Global event blocks.
     pub global_event_blocks: AtomicU64,
 
-    pub blocking_query: OnceCell<RefCell<Option<TerminalQuery>>>,
+    pub blocking_query: RefCell<Option<TerminalQuery>>,
+
+    pub pending_input: RefCell<VecDeque<CharEvent>>,
 }
 
 impl Parser {
@@ -463,7 +465,8 @@ pub fn new(variables: EnvStack, cancel_behavior: CancelBehavior) -> Parser {
             cancel_behavior,
             profile_items: RefCell::default(),
             global_event_blocks: AtomicU64::new(0),
-            blocking_query: OnceCell::new(),
+            blocking_query: RefCell::new(None),
+            pending_input: RefCell::new(VecDeque::new()),
         };
 
         match open_dir(CStr::from_bytes_with_nul(b".\0").unwrap(), BEST_O_SEARCH) {

src/reader.rs

@@ -25,11 +25,9 @@
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
 use once_cell::sync::Lazy;
-use once_cell::unsync::OnceCell;
 #[cfg(not(target_has_atomic = "64"))]
 use portable_atomic::AtomicU64;
 use std::borrow::Cow;
-use std::cell::RefCell;
 use std::cell::RefMut;
 use std::cell::UnsafeCell;
 use std::cmp;
@@ -40,6 +38,7 @@
 use std::ops::Range;
 use std::os::fd::BorrowedFd;
 use std::os::fd::{AsRawFd, RawFd};
+use std::os::unix::ffi::OsStrExt;
 use std::pin::Pin;
 use std::rc::Rc;
 #[cfg(target_has_atomic = "64")]
@@ -85,7 +84,7 @@
     history_session_id, in_private_mode, History, HistorySearch, PersistenceMode, SearchDirection,
     SearchFlags, SearchType,
 };
-use crate::input::init_input;
+use crate::input_common::InputEventQueue;
 use crate::input_common::InputEventQueuer;
 use crate::input_common::QueryResponse;
 use crate::input_common::{
@@ -267,28 +266,73 @@ fn querying_allowed(in_fd: RawFd) -> bool {
         && isatty(STDOUT_FILENO)
 }
 
-pub(crate) fn initial_query(
+pub fn terminal_init() -> InputEventQueue {
-    in_fd: RawFd,
+    reader_interactive_init();
-    blocking_query: &OnceCell<RefCell<Option<TerminalQuery>>>,
+
-    out: &mut impl Output,
+    let mut input_queue = InputEventQueue::new(STDIN_FILENO);
-    vars: Option<&dyn Environment>,
+
-) {
+    let _init_tty_metadata = ScopeGuard::new((), |()| {
-    blocking_query.get_or_init(|| {
         initialize_tty_metadata();
-        let query = if !querying_allowed(in_fd) {
-            None
-        } else {
-            // Query for kitty keyboard protocol support.
-            out.write_command(QueryKittyKeyboardProgressiveEnhancements);
-            out.write_command(QueryXtversion);
-            if let Some(vars) = vars {
-                query_capabilities_via_dcs(out.by_ref(), vars);
-            }
-            out.write_command(QueryPrimaryDeviceAttribute);
-            Some(TerminalQuery::Initial)
-        };
-        RefCell::new(query)
     });
+
+    if !querying_allowed(STDIN_FILENO) {
+        return input_queue;
+    }
+
+    set_shell_modes(STDIN_FILENO, "initial query");
+    {
+        let mut out = BufferedOutputter::new(Outputter::stdoutput());
+        // Query for kitty keyboard protocol support.
+        out.write_command(QueryKittyKeyboardProgressiveEnhancements);
+        out.write_command(QueryXtversion);
+        query_capabilities_via_dcs(out.by_ref());
+        out.write_command(QueryPrimaryDeviceAttribute);
+    }
+    input_queue.blocking_query().replace(TerminalQuery::Initial);
+
+    while !check_exit_loop_maybe_warning(None) {
+        use CharEvent::{Command, Implicit, Key, Readline};
+        use ImplicitEvent::{CheckExit, Eof, QueryInterrupted};
+        use QueryResultEvent::*;
+        match input_queue.readch() {
+            Implicit(Eof) => reader_sighup(),
+            Implicit(CheckExit) => {}
+            Implicit(QueryInterrupted) => break,
+            CharEvent::QueryResult(Response(QueryResponse::PrimaryDeviceAttribute) | Timeout) => {
+                if get_kitty_keyboard_capability() == Capability::Unknown {
+                    set_kitty_keyboard_capability(
+                        reader_save_screen_state,
+                        Capability::NotSupported,
+                    );
+                }
+                break;
+            }
+            CharEvent::QueryResult(Response(_)) => (),
+            Key(_) | Readline(_) | Command(_) | Implicit(_) => panic!(),
+        };
+    }
+
+    stop_query(input_queue.blocking_query());
+
+    let input_data = input_queue.get_input_data();
+    // We blocked execution of code and mappings so input function args must be empty.
+    assert!(input_data.input_function_args.is_empty());
+    if input_data.paste_buffer.is_some() {
+        // The terminal should never interleave query responses with a bracketed paste
+        // command. hence this should only happen on timeout.
+        FLOG!(
+            reader,
+            "Bracketed paste was interrupted; dropping uncommitted paste buffer"
+        )
+    }
+    assert!(input_data.event_storage.is_empty());
+    FLOGF!(
+        reader,
+        "Returning %lu pending input events",
+        input_data.queue.len()
+    );
+
+    input_queue
 }
 
 /// The stack of current interactive reading contexts.
@@ -329,7 +373,12 @@ pub fn reader_push<'a>(parser: &'a Parser, history_name: &wstr, conf: ReaderConf
     let data = current_data().unwrap();
     data.command_line_changed(EditableLineTag::Commandline, AutosuggestionUpdate::Remove);
     if !parser.interactive_initialized.swap(true) {
-        reader_interactive_init(parser);
+        // Provide value for `status current-command`
+        parser.libdata_mut().status_vars.command = L!("fish").to_owned();
+        // Also provide a value for the deprecated fish 2.0 $_ variable
+        parser
+            .vars()
+            .set_one(L!("_"), EnvMode::GLOBAL, L!("fish").to_owned());
     }
     Reader { data, parser }
 }
@@ -1548,7 +1597,7 @@ pub fn combine_command_and_autosuggestion(
 
 impl<'a> Reader<'a> {
     pub(crate) fn blocking_query(&self) -> RefMut<'_, Option<TerminalQuery>> {
-        self.parser.blocking_query.get().unwrap().borrow_mut()
+        self.parser.blocking_query.borrow_mut()
     }
 
     pub fn request_cursor_position(&mut self, out: &mut Outputter, q: CursorPositionQuery) {
@@ -2233,13 +2282,6 @@ fn readline(&mut self, nchars: Option<NonZeroUsize>) -> Option<WString> {
         // Set the new modes.
         set_shell_modes(self.conf.inputfd, "readline");
 
-        initial_query(
-            self.conf.inputfd,
-            &self.parser.blocking_query,
-            &mut BufferedOutputter::new(Outputter::stdoutput()),
-            Some(self.parser.vars()),
-        );
-
         // HACK: Don't abandon line for the first prompt, because
         // if we're started with the terminal it might not have settled,
         // so the width is quite likely to be in flight.
@@ -2263,6 +2305,7 @@ fn readline(&mut self, nchars: Option<NonZeroUsize>) -> Option<WString> {
         // Start out as initially dirty.
         self.force_exec_prompt_and_repaint = true;
 
+        self.insert_front(self.parser.pending_input.take());
         while !self.rls().finished && !check_exit_loop_maybe_warning(Some(self)) {
             // Enable tty protocols while we read input.
             tty.enable_tty_protocols();
@@ -2541,6 +2584,7 @@ fn handle_char_event(&mut self, injected_event: Option<CharEvent>) -> ControlFlo
             CharEvent::Implicit(implicit_event) => match implicit_event {
                 ImplicitEvent::Eof => reader_sighup(),
                 ImplicitEvent::CheckExit => (),
+                ImplicitEvent::QueryInterrupted => (),
                 ImplicitEvent::FocusIn => {
                     event::fire_generic(self.parser, L!("fish_focus_in").to_owned(), vec![]);
                 }
@@ -2567,15 +2611,7 @@ fn handle_char_event(&mut self, injected_event: Option<CharEvent>) -> ControlFlo
                 use QueryResponse::*;
                 use QueryResultEvent::*;
                 let query = match (&mut **query, query_result) {
-                    (Some(TerminalQuery::Initial), Response(PrimaryDeviceAttribute) | Timeout) => {
+                    (Some(TerminalQuery::Initial), _) => panic!(),
-                        if get_kitty_keyboard_capability() == Capability::Unknown {
-                            set_kitty_keyboard_capability(
-                                reader_save_screen_state,
-                                Capability::NotSupported,
-                            );
-                        }
-                        maybe_query
-                    }
                     (
                         Some(TerminalQuery::CursorPosition(cursor_pos_query)),
                         Response(CursorPosition(cursor_pos)),
@@ -2630,13 +2666,15 @@ fn send_xtgettcap_query(out: &mut impl Output, cap: &'static str) {
 
 #[allow(renamed_and_removed_lints)]
 #[allow(clippy::blocks_in_if_conditions)] // for old clippy
-fn query_capabilities_via_dcs(out: &mut impl Output, vars: &dyn Environment) {
+fn query_capabilities_via_dcs(out: &mut impl Output) {
-    if vars.get_unless_empty(L!("STY")).is_some()
+    if {
-        || vars.get_unless_empty(L!("TERM")).is_some_and(|term| {
+        use std::env::var_os;
-            let term = &term.as_list()[0];
+        var_os("STY").is_some()
-            term == "screen" || term == "screen-256color"
+            || var_os("TERM").is_some_and(|term| {
-        })
+                let screens: [&[u8]; 2] = [b"screen", b"screen-256color"];
-    {
+                screens.contains(&term.as_bytes())
+            })
+    } {
         return;
     }
     out.write_command(DecsetAlternateScreenBuffer); // enable alternative screen buffer
@@ -4490,15 +4528,12 @@ fn acquire_tty_or_exit(shell_pgid: libc::pid_t) {
 }
 
 /// Initialize data for interactive use.
-fn reader_interactive_init(parser: &Parser) {
+fn reader_interactive_init() {
     assert_is_main_thread();
 
     let mut shell_pgid = getpgrp();
     let shell_pid = getpid();
 
-    // Set up key bindings.
-    init_input();
-
     // Ensure interactive signal handling is enabled.
     signal_set_handlers_once(true);
 
@@ -4536,15 +4571,6 @@ fn reader_interactive_init(parser: &Parser) {
     }
 
     termsize_invalidate_tty();
-
-    // Provide value for `status current-command`
-    parser.libdata_mut().status_vars.command = L!("fish").to_owned();
-    // Also provide a value for the deprecated fish 2.0 $_ variable
-    parser
-        .vars()
-        .set_one(L!("_"), EnvMode::GLOBAL, L!("fish").to_owned());
-
-    initialize_tty_metadata();
 }
 
 /// Return whether fish is currently unwinding the stack in preparation to exit.