Further simplify terminal_protocols scoping

Remove the last non scoped place where we disable protocols (just before
exec(1)); it's not necessary with the current approach because we always
disable inside eval.
There is an edge case where we don't:

    fish -ic "exec bash"

leaving bash with CSI u enabled.  Disable that also in -ic mode where we
don't have a reader.

In future we should use the same approach for restore_term_mode() but I'm
not sure which one is better.
This commit is contained in:
Johannes Altmanninger 2024-04-21 16:19:39 +02:00
parent 99bf3d0dbb
commit fd61bad946
3 changed files with 12 additions and 14 deletions

View file

@ -24,7 +24,6 @@ use crate::fork_exec::postfork::{
#[cfg(FISH_USE_POSIX_SPAWN)] #[cfg(FISH_USE_POSIX_SPAWN)]
use crate::fork_exec::spawn::PosixSpawner; use crate::fork_exec::spawn::PosixSpawner;
use crate::function::{self, FunctionProperties}; use crate::function::{self, FunctionProperties};
use crate::input_common::{terminal_protocols_disable, TERMINAL_PROTOCOLS};
use crate::io::{ use crate::io::{
BufferedOutputStream, FdOutputStream, IoBufferfill, IoChain, IoClose, IoMode, IoPipe, BufferedOutputStream, FdOutputStream, IoBufferfill, IoChain, IoClose, IoMode, IoPipe,
IoStreams, OutputStream, SeparatedBuffer, StringOutputStream, IoStreams, OutputStream, SeparatedBuffer, StringOutputStream,
@ -40,7 +39,7 @@ use crate::proc::{
print_exit_warning_for_jobs, InternalProc, Job, JobGroupRef, ProcStatus, Process, ProcessType, print_exit_warning_for_jobs, InternalProc, Job, JobGroupRef, ProcStatus, Process, ProcessType,
TtyTransfer, INVALID_PID, TtyTransfer, INVALID_PID,
}; };
use crate::reader::{reader_current_data, reader_run_count, restore_term_mode}; use crate::reader::{reader_run_count, restore_term_mode};
use crate::redirection::{dup2_list_resolve_chain, Dup2List}; use crate::redirection::{dup2_list_resolve_chain, Dup2List};
use crate::threads::{iothread_perform_cant_wait, is_forked_child}; use crate::threads::{iothread_perform_cant_wait, is_forked_child};
use crate::timer::push_timer; use crate::timer::push_timer;
@ -440,9 +439,6 @@ fn launch_process_nofork(vars: &EnvStack, p: &Process) -> ! {
// Ensure the terminal modes are what they were before we changed them. // Ensure the terminal modes are what they were before we changed them.
restore_term_mode(); restore_term_mode();
if reader_current_data().is_some() && TERMINAL_PROTOCOLS.get().borrow().is_some() {
terminal_protocols_disable();
}
// Bounce to launch_process. This never returns. // Bounce to launch_process. This never returns.
safe_launch_process(p, &actual_cmd, &argv, &*envp); safe_launch_process(p, &actual_cmd, &argv, &*envp);
} }

View file

@ -438,7 +438,7 @@ fn terminal_protocols_enable() {
.replace(Some(TerminalProtocols::new())); .replace(Some(TerminalProtocols::new()));
} }
pub fn terminal_protocols_disable() { fn terminal_protocols_disable() {
assert!(TERMINAL_PROTOCOLS.get().borrow().is_some()); assert!(TERMINAL_PROTOCOLS.get().borrow().is_some());
TERMINAL_PROTOCOLS.get().replace(None); TERMINAL_PROTOCOLS.get().replace(None);
} }
@ -454,7 +454,9 @@ pub fn terminal_protocols_disable_scoped() -> impl ScopeGuarding<Target = ()> {
// If a child is stopped, this will already be enabled. // If a child is stopped, this will already be enabled.
if TERMINAL_PROTOCOLS.get().borrow().is_none() { if TERMINAL_PROTOCOLS.get().borrow().is_none() {
terminal_protocols_enable(); terminal_protocols_enable();
reader_current_data().unwrap().save_screen_state(); if let Some(data) = reader_current_data() {
data.save_screen_state();
}
} }
}) })
} }
@ -529,7 +531,9 @@ pub(crate) fn focus_events_enable_ifn() {
if !term_protocols.focus_events { if !term_protocols.focus_events {
term_protocols.focus_events = true; term_protocols.focus_events = true;
let _ = write_to_fd("\x1b[?1004h".as_bytes(), STDOUT_FILENO); let _ = write_to_fd("\x1b[?1004h".as_bytes(), STDOUT_FILENO);
reader_current_data().unwrap().save_screen_state(); if let Some(data) = reader_current_data() {
data.save_screen_state();
}
} }
} }

View file

@ -25,7 +25,6 @@ use crate::parse_constants::{
use crate::parse_execution::{EndExecutionReason, ParseExecutionContext}; use crate::parse_execution::{EndExecutionReason, ParseExecutionContext};
use crate::parse_tree::{parse_source, ParsedSourceRef}; use crate::parse_tree::{parse_source, ParsedSourceRef};
use crate::proc::{job_reap, JobGroupRef, JobList, JobRef, ProcStatus}; use crate::proc::{job_reap, JobGroupRef, JobList, JobRef, ProcStatus};
use crate::reader::reader_current_data;
use crate::signal::{signal_check_cancel, signal_clear_cancel, Signal}; use crate::signal::{signal_check_cancel, signal_clear_cancel, Signal};
use crate::threads::{assert_is_main_thread, MainThread}; use crate::threads::{assert_is_main_thread, MainThread};
use crate::util::get_time; use crate::util::get_time;
@ -564,11 +563,10 @@ impl Parser {
Some(ParseExecutionContext::new(ps.clone(), block_io.clone())), Some(ParseExecutionContext::new(ps.clone(), block_io.clone())),
); );
let terminal_protocols_disabled = ( // If interactive or inside noninteractive builtin read.
// If interactive or inside noninteractive builtin read. let terminal_protocols_enabled = TERMINAL_PROTOCOLS.get().borrow().is_some();
reader_current_data().is_some() && TERMINAL_PROTOCOLS.get().borrow().is_some() let terminal_protocols_disabled =
) terminal_protocols_enabled.then(terminal_protocols_disable_scoped);
.then(terminal_protocols_disable_scoped);
// Check the exec count so we know if anything got executed. // Check the exec count so we know if anything got executed.
let prev_exec_count = self.libdata().pods.exec_count; let prev_exec_count = self.libdata().pods.exec_count;