Toggle terminal protocols lazily

Closes #10494

src/builtins/fg.rs

@@ -1,7 +1,7 @@
 //! Implementation of the fg builtin.
 
 use crate::fds::make_fd_blocking;
-use crate::input_common::TERMINAL_PROTOCOLS;
+use crate::input_common::terminal_protocols_disable_ifn;
 use crate::reader::reader_write_title;
 use crate::tokenizer::tok_command;
 use crate::wutil::perror;
@@ -148,15 +148,17 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Optio
         let job_group = job.group();
         job_group.set_is_foreground(true);
         let tmodes = job_group.tmodes.borrow();
-        if job_group.wants_terminal() && tmodes.is_some() {
-            let termios = tmodes.as_ref().unwrap();
-            let res = unsafe { libc::tcsetattr(STDIN_FILENO, TCSADRAIN, termios) };
-            if res < 0 {
-                perror("tcsetattr");
+        if job_group.wants_terminal() {
+            terminal_protocols_disable_ifn();
+            if tmodes.is_some() {
+                let termios = tmodes.as_ref().unwrap();
+                let res = unsafe { libc::tcsetattr(STDIN_FILENO, TCSADRAIN, termios) };
+                if res < 0 {
+                    perror("tcsetattr");
+                }
             }
         }
     }
-    assert!(TERMINAL_PROTOCOLS.get().borrow().is_none());
     let mut transfer = TtyTransfer::new();
     transfer.to_job_group(job.group.as_ref().unwrap());
     let resumed = job.resume();

src/builtins/read.rs

@@ -12,7 +12,6 @@ use crate::env::EnvMode;
 use crate::env::Environment;
 use crate::env::READ_BYTE_LIMIT;
 use crate::env::{EnvVar, EnvVarFlags};
-use crate::input_common::terminal_protocols_enable_scoped;
 use crate::libc::MB_CUR_MAX;
 use crate::nix::isatty;
 use crate::reader::commandline_set_buffer;
@@ -586,9 +585,6 @@ pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt
 
     let stream_stdin_is_a_tty = isatty(streams.stdin_fd);
 
-    // Enable terminal protocols if noninteractive.
-    let _terminal_protocols = stream_stdin_is_a_tty.then(terminal_protocols_enable_scoped);
-
     // Normally, we either consume a line of input or all available input. But if we are reading a
     // line at a time, we need a middle ground where we only consume as many lines as we need to
     // fill the given vars.

src/exec.rs

@@ -24,6 +24,7 @@ use crate::fork_exec::postfork::{
 #[cfg(FISH_USE_POSIX_SPAWN)]
 use crate::fork_exec::spawn::PosixSpawner;
 use crate::function::{self, FunctionProperties};
+use crate::input_common::terminal_protocols_disable_ifn;
 use crate::io::{
     BufferedOutputStream, FdOutputStream, IoBufferfill, IoChain, IoClose, IoMode, IoPipe,
     IoStreams, OutputStream, SeparatedBuffer, StringOutputStream,
@@ -71,6 +72,10 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool {
         return true;
     }
 
+    if job.group().wants_terminal() {
+        terminal_protocols_disable_ifn();
+    }
+
     // Handle an exec call.
     if job.processes()[0].typ == ProcessType::exec {
         // If we are interactive, perhaps disallow exec if there are background jobs.

src/input_common.rs

@@ -1,8 +1,7 @@
 use libc::STDOUT_FILENO;
 
 use crate::common::{
-    fish_reserved_codepoint, is_windows_subsystem_for_linux, read_blocked, shell_modes, ScopeGuard,
-    ScopeGuarding,
+    fish_reserved_codepoint, is_windows_subsystem_for_linux, read_blocked, shell_modes,
 };
 use crate::env::{EnvStack, Environment};
 use crate::fd_readable_set::FdReadableSet;
@@ -429,115 +428,77 @@ pub fn update_wait_on_sequence_key_ms(vars: &EnvStack) {
     }
 }
 
-pub static TERMINAL_PROTOCOLS: MainThread<RefCell<Option<TerminalProtocols>>> =
+static TERMINAL_PROTOCOLS: MainThread<RefCell<Option<TerminalProtocols>>> =
     MainThread::new(RefCell::new(None));
 
-fn terminal_protocols_enable() {
-    assert!(TERMINAL_PROTOCOLS.get().borrow().is_none());
-    TERMINAL_PROTOCOLS
-        .get()
-        .replace(Some(TerminalProtocols::new()));
-}
+pub(crate) static IS_TMUX: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
 
-fn terminal_protocols_disable() {
-    assert!(TERMINAL_PROTOCOLS.get().borrow().is_some());
-    TERMINAL_PROTOCOLS.get().replace(None);
+pub(crate) fn terminal_protocols_enable_ifn() {
+    let mut term_protocols = TERMINAL_PROTOCOLS.get().borrow_mut();
+    if term_protocols.is_some() {
+        return;
+    }
+    *term_protocols = Some(TerminalProtocols::new());
+    reader_current_data().map(|data| data.save_screen_state());
 }
 
-pub fn terminal_protocols_enable_scoped() -> impl ScopeGuarding<Target = ()> {
-    terminal_protocols_enable();
-    ScopeGuard::new((), |()| terminal_protocols_disable())
+pub(crate) fn terminal_protocols_disable_ifn() {
+    TERMINAL_PROTOCOLS.get().replace(None);
 }
 
-pub fn terminal_protocols_disable_scoped() -> impl ScopeGuarding<Target = ()> {
-    terminal_protocols_disable();
-    ScopeGuard::new((), |()| {
-        // If a child is stopped, this will already be enabled.
-        if TERMINAL_PROTOCOLS.get().borrow().is_none() {
-            terminal_protocols_enable();
-            if let Some(data) = reader_current_data() {
-                data.save_screen_state();
-            }
-        }
-    })
+pub(crate) fn terminal_protocols_try_disable_ifn() {
+    if let Ok(mut term_protocols) = TERMINAL_PROTOCOLS.get().try_borrow_mut() {
+        *term_protocols = None;
+    }
 }
 
-pub struct TerminalProtocols {
-    focus_events: bool,
-}
+struct TerminalProtocols {}
 
 impl TerminalProtocols {
     fn new() -> Self {
-        terminal_protocols_enable_impl();
-        Self {
-            focus_events: false,
+        let sequences = concat!(
+            "\x1b[?2004h", // Bracketed paste
+            "\x1b[>4;1m",  // XTerm's modifyOtherKeys
+            "\x1b[>5u",    // CSI u with kitty progressive enhancement
+            "\x1b=",       // set application keypad mode, so the keypad keys send unique codes
+        );
+        FLOG!(
+            term_protocols,
+            format!(
+                "Enabling extended keys and bracketed paste: {:?}",
+                sequences
+            )
+        );
+        let _ = write_to_fd(sequences.as_bytes(), STDOUT_FILENO);
+        if IS_TMUX.load() {
+            let _ = write_to_fd("\x1b[?1004h".as_bytes(), STDOUT_FILENO);
         }
+        Self {}
     }
 }
 
 impl Drop for TerminalProtocols {
     fn drop(&mut self) {
-        terminal_protocols_disable_impl();
-        if self.focus_events {
+        let sequences = concat!(
+            "\x1b[?2004l",
+            "\x1b[>4;0m",
+            "\x1b[<1u", // Konsole breaks unless we pass an explicit number of entries to pop.
+            "\x1b>",
+        );
+        FLOG!(
+            term_protocols,
+            format!(
+                "Disabling extended keys and bracketed paste: {:?}",
+                sequences
+            )
+        );
+        let _ = write_to_fd(sequences.as_bytes(), STDOUT_FILENO);
+        if IS_TMUX.load() {
             let _ = write_to_fd("\x1b[?1004l".as_bytes(), STDOUT_FILENO);
         }
     }
 }
 
-fn terminal_protocols_enable_impl() {
-    let sequences = concat!(
-        "\x1b[?2004h", // Bracketed paste
-        "\x1b[>4;1m",  // XTerm's modifyOtherKeys
-        "\x1b[>5u",    // CSI u with kitty progressive enhancement
-        "\x1b=",       // set application keypad mode, so the keypad keys send unique codes
-    );
-
-    FLOG!(
-        term_protocols,
-        format!(
-            "Enabling extended keys and bracketed paste: {:?}",
-            sequences
-        )
-    );
-    let _ = write_to_fd(sequences.as_bytes(), STDOUT_FILENO);
-}
-
-fn terminal_protocols_disable_impl() {
-    let sequences = concat!(
-        "\x1b[?2004l",
-        "\x1b[>4;0m",
-        "\x1b[<1u", // Konsole breaks unless we pass an explicit number of entries to pop.
-        "\x1b>",
-    );
-    FLOG!(
-        term_protocols,
-        format!(
-            "Disabling extended keys and bracketed paste: {:?}",
-            sequences
-        )
-    );
-    let _ = write_to_fd(sequences.as_bytes(), STDOUT_FILENO);
-}
-
-pub(crate) static IS_TMUX: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
-
-pub(crate) fn focus_events_enable_ifn() {
-    if !IS_TMUX.load() {
-        return;
-    }
-    let mut term_protocols = TERMINAL_PROTOCOLS.get().borrow_mut();
-    let Some(term_protocols) = term_protocols.as_mut() else {
-        panic!()
-    };
-    if !term_protocols.focus_events {
-        term_protocols.focus_events = true;
-        let _ = write_to_fd("\x1b[?1004h".as_bytes(), STDOUT_FILENO);
-        if let Some(data) = reader_current_data() {
-            data.save_screen_state();
-        }
-    }
-}
-
 fn parse_mask(mask: u32) -> Modifiers {
     Modifiers {
         ctrl: (mask & 4) != 0,
@@ -1045,7 +1006,7 @@ pub trait InputEventQueuer {
         if let Some(evt) = self.try_pop() {
             return Some(evt);
         }
-        focus_events_enable_ifn();
+        terminal_protocols_enable_ifn();
 
         // We are not prepared to handle a signal immediately; we only want to know if we get input on
         // our fd before the timeout. Use pselect to block all signals; we will handle signals

src/parser.rs

@@ -14,7 +14,6 @@ use crate::expand::{
 };
 use crate::fds::{open_dir, BEST_O_SEARCH};
 use crate::global_safety::RelaxedAtomicBool;
-use crate::input_common::{terminal_protocols_disable_scoped, TERMINAL_PROTOCOLS};
 use crate::io::IoChain;
 use crate::job_group::MaybeJobId;
 use crate::operation_context::{OperationContext, EXPANSION_LIMIT_DEFAULT};
@@ -562,11 +561,6 @@ impl Parser {
             Some(ParseExecutionContext::new(ps.clone(), block_io.clone())),
         );
 
-        // If interactive or inside noninteractive builtin read.
-        let terminal_protocols_enabled = TERMINAL_PROTOCOLS.get().borrow().is_some();
-        let terminal_protocols_disabled =
-            terminal_protocols_enabled.then(terminal_protocols_disable_scoped);
-
         // Check the exec count so we know if anything got executed.
         let prev_exec_count = self.libdata().pods.exec_count;
         let prev_status_count = self.libdata().pods.status_count;
@@ -578,7 +572,6 @@ impl Parser {
         let new_exec_count = self.libdata().pods.exec_count;
         let new_status_count = self.libdata().pods.status_count;
 
-        drop(terminal_protocols_disabled);
         ScopeGuarding::commit(exc);
         self.pop_block(scope_block);
 

src/reader.rs

@@ -71,11 +71,9 @@ use crate::history::{
 };
 use crate::input::init_input;
 use crate::input::Inputter;
+use crate::input_common::terminal_protocols_disable_ifn;
 use crate::input_common::IS_TMUX;
-use crate::input_common::{
-    focus_events_enable_ifn, terminal_protocols_enable_scoped, CharEvent, CharInputStyle,
-    ReadlineCmd,
-};
+use crate::input_common::{terminal_protocols_enable_ifn, CharEvent, CharInputStyle, ReadlineCmd};
 use crate::io::IoChain;
 use crate::kill::{kill_add, kill_replace, kill_yank, kill_yank_rotate};
 use crate::libc::MB_CUR_MAX;
@@ -794,16 +792,14 @@ pub fn reader_init() -> impl ScopeGuarding<Target = ()> {
 
     // Set up our fixed terminal modes once,
     // so we don't get flow control just because we inherited it.
-    let mut terminal_protocols = None;
     if is_interactive_session() {
-        terminal_protocols = Some(terminal_protocols_enable_scoped());
         if unsafe { libc::getpgrp() == libc::tcgetpgrp(STDIN_FILENO) } {
             term_donate(/*quiet=*/ true);
         }
     }
     ScopeGuard::new((), move |()| {
-        let _terminal_protocols = terminal_protocols;
         restore_term_mode();
+        terminal_protocols_disable_ifn();
     })
 }
 
@@ -1968,7 +1964,7 @@ impl ReaderData {
         let mut accumulated_chars = WString::new();
 
         while accumulated_chars.len() < limit {
-            focus_events_enable_ifn();
+            terminal_protocols_enable_ifn();
             let evt = self.inputter.read_char();
             let CharEvent::Key(kevt) = &evt else {
                 event_needing_handling = Some(evt);

src/signal.rs

@@ -2,7 +2,7 @@ use std::num::NonZeroI32;
 
 use crate::common::{exit_without_destructors, restore_term_foreground_process_group_for_exit};
 use crate::event::{enqueue_signal, is_signal_observed};
-use crate::input_common::TERMINAL_PROTOCOLS;
+use crate::input_common::terminal_protocols_try_disable_ifn;
 use crate::nix::getpid;
 use crate::reader::{reader_handle_sigint, reader_sighup};
 use crate::termsize::TermsizeContainer;
@@ -89,9 +89,7 @@ extern "C" fn fish_signal_handler(
             // Handle sigterm. The only thing we do is restore the front process ID, then die.
             if !observed {
                 restore_term_foreground_process_group_for_exit();
-                if let Ok(mut term_protocols) = TERMINAL_PROTOCOLS.get().try_borrow_mut() {
-                    *term_protocols = None;
-                }
+                terminal_protocols_try_disable_ifn();
                 // Safety: signal() and raise() are async-signal-safe.
                 unsafe {
                     libc::signal(libc::SIGTERM, libc::SIG_DFL);

tests/pexpects/read.py

@@ -13,7 +13,7 @@
 
 
 def expect_read_prompt():
-    expect_re(r"\r\n?read> (\x1b\[\?1004h)?$")
+    expect_re(r"\r\n?read> $")
 
 
 def expect_marker(text):
@@ -56,12 +56,12 @@ def print_var_contents(varname, expected):
 
 # read -c (see #8633)
 sendline(r"read -c init_text somevar && echo $somevar")
-expect_re(r"\r\n?read> init_text(\x1b\[\?1004h)?$")
+expect_re(r"\r\n?read> init_text$")
 sendline("someval")
 expect_prompt("someval\r\n")
 
 sendline(r"read --command='some other text' somevar && echo $somevar")
-expect_re(r"\r\n?read> some other text(\x1b\[\?1004h)?$")
+expect_re(r"\r\n?read> some other text$")
 sendline("another value")
 expect_prompt("another value\r\n")
 

tests/pexpects/signals.py

@@ -48,7 +48,7 @@
 sendline("function postexec --on-event fish_postexec; echo fish_postexec spotted; end")
 expect_prompt()
 sendline("read")
-expect_re(r"\r\n?read> (\x1b\[\?1004h)?$", timeout=10)
+expect_re(r"\r\n?read> $", timeout=10)
 sleep(0.1)
 os.kill(sp.spawn.pid, signal.SIGINT)
 expect_str("fish_postexec spotted", timeout=10)