From 29f2da8d186e5267f4b320aa6966c3e5d7aadca4 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 16 May 2024 10:52:19 +0200 Subject: [PATCH] Toggle terminal protocols lazily Closes #10494 --- src/builtins/fg.rs | 16 +++-- src/builtins/read.rs | 4 -- src/exec.rs | 5 ++ src/input_common.rs | 139 ++++++++++++++------------------------ src/parser.rs | 7 -- src/reader.rs | 12 ++-- src/signal.rs | 6 +- tests/pexpects/read.py | 6 +- tests/pexpects/signals.py | 2 +- 9 files changed, 74 insertions(+), 123 deletions(-) diff --git a/src/builtins/fg.rs b/src/builtins/fg.rs index fc8e961c1..bb3a12057 100644 --- a/src/builtins/fg.rs +++ b/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(); diff --git a/src/builtins/read.rs b/src/builtins/read.rs index 8987f8fc8..65a51acd6 100644 --- a/src/builtins/read.rs +++ b/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. diff --git a/src/exec.rs b/src/exec.rs index ce96fe844..09a61e2ae 100644 --- a/src/exec.rs +++ b/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. diff --git a/src/input_common.rs b/src/input_common.rs index 96ae2eeb6..96afc05bc 100644 --- a/src/input_common.rs +++ b/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>> = +static TERMINAL_PROTOCOLS: MainThread>> = 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); + +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()); } -fn terminal_protocols_disable() { - assert!(TERMINAL_PROTOCOLS.get().borrow().is_some()); +pub(crate) fn terminal_protocols_disable_ifn() { TERMINAL_PROTOCOLS.get().replace(None); } -pub fn terminal_protocols_enable_scoped() -> impl ScopeGuarding { - terminal_protocols_enable(); - ScopeGuard::new((), |()| terminal_protocols_disable()) +pub(crate) fn terminal_protocols_try_disable_ifn() { + if let Ok(mut term_protocols) = TERMINAL_PROTOCOLS.get().try_borrow_mut() { + *term_protocols = None; + } } -pub fn terminal_protocols_disable_scoped() -> impl ScopeGuarding { - 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 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 diff --git a/src/parser.rs b/src/parser.rs index b61eb4dcc..3c4859263 100644 --- a/src/parser.rs +++ b/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); diff --git a/src/reader.rs b/src/reader.rs index 5e21b7760..ff61b4801 100644 --- a/src/reader.rs +++ b/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 { // 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); diff --git a/src/signal.rs b/src/signal.rs index b333e0bff..951c932e9 100644 --- a/src/signal.rs +++ b/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); diff --git a/tests/pexpects/read.py b/tests/pexpects/read.py index 6488df56c..17e20d189 100644 --- a/tests/pexpects/read.py +++ b/tests/pexpects/read.py @@ -13,7 +13,7 @@ send, sendline, sleep, expect_prompt, expect_re, expect_str = ( 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 @@ print_var_contents("foo", "bar") # 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") diff --git a/tests/pexpects/signals.py b/tests/pexpects/signals.py index d368c3bd8..14bfa8111 100644 --- a/tests/pexpects/signals.py +++ b/tests/pexpects/signals.py @@ -48,7 +48,7 @@ expect_prompt() 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)