From c16677fd6f1beb2bc5afa5c9f83050e5a7316db4 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 4 Jan 2026 06:32:21 +0100 Subject: [PATCH] tty_handoff: use Drop consistently We sometimes use explicit reclaim() and sometimes rely on the drop implementation. This adds an unnecesary step to reading all uses of this code. Make this consistent. Use drop everywhere though we could use explicit reclaim too. --- src/builtins/fg.rs | 1 - src/exec.rs | 1 - src/reader/reader.rs | 2 -- src/tty_handoff.rs | 59 ++++++++++++++------------------------------ 4 files changed, 18 insertions(+), 45 deletions(-) diff --git a/src/builtins/fg.rs b/src/builtins/fg.rs index e33e86131..803227842 100644 --- a/src/builtins/fg.rs +++ b/src/builtins/fg.rs @@ -157,7 +157,6 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Built if job.is_stopped() { handoff.save_tty_modes(); } - handoff.reclaim(); if resumed { Ok(SUCCESS) } else { diff --git a/src/exec.rs b/src/exec.rs index 27387639f..dd4455d2b 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -252,7 +252,6 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool { if job.is_stopped() { handoff.save_tty_modes(); } - handoff.reclaim(); true } diff --git a/src/reader/reader.rs b/src/reader/reader.rs index 9f329d891..24d09f5bd 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -2474,7 +2474,6 @@ fn eval_bind_cmd(&mut self, cmd: &wstr) { self.parser.eval(cmd, &IoChain::new()); self.parser.set_last_statuses(last_statuses); - scoped_tty.reclaim(); } /// Run a sequence of commands from an input binding. @@ -2995,7 +2994,6 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { let mut tty = TtyHandoff::new(reader_save_screen_state); tty.disable_tty_protocols(); self.compute_and_apply_completions(c); - tty.reclaim(); } } rl::PagerToggleSearch => { diff --git a/src/tty_handoff.rs b/src/tty_handoff.rs index 8b2f2fdfa..e8bd7a96a 100644 --- a/src/tty_handoff.rs +++ b/src/tty_handoff.rs @@ -349,14 +349,12 @@ pub struct TtyHandoff { // The job group which owns the tty, or empty if none. owner: Option, // Whether terminal protocols were initially enabled. - // reclaim() restores the state to this. + // Restored on drop. tty_protocols_initial: bool, // The state of terminal protocols that we set. // Note we track this separately from TTY_PROTOCOLS_ACTIVE. We undo the changes // we make. tty_protocols_applied: bool, - // Whether reclaim was called, restoring the tty to its pre-scoped value. - reclaimed: bool, // Called after writing to the TTY. on_write: fn(), } @@ -368,7 +366,6 @@ pub fn new(on_write: fn()) -> Self { owner: None, tty_protocols_initial: protocols_active, tty_protocols_applied: protocols_active, - reclaimed: false, on_write, } } @@ -400,40 +397,6 @@ pub fn to_job_group(&mut self, jg: &JobGroupRef) { } } - /// Reclaim the tty if we transferred it. - pub fn reclaim(mut self) { - self.reclaim_impl() - } - - /// Release the tty, meaning no longer restore anything in Drop - similar to `mem::forget`. - pub fn release(mut self) { - self.reclaimed = true; - } - - /// Implementation of reclaim, factored out for use in Drop. - fn reclaim_impl(&mut self) { - assert!(!self.reclaimed, "Terminal already reclaimed"); - self.reclaimed = true; - if self.owner.is_some() { - flog!(proc_pgroup, "fish reclaiming terminal"); - if unsafe { libc::tcsetpgrp(STDIN_FILENO, libc::getpgrp()) } == -1 { - flog!( - warning, - "Could not return shell to foreground:", - errno::errno() - ); - perror("tcsetpgrp"); - } - self.owner = None; - } - // Restore the terminal protocols. Note this does nothing if they were unchanged. - if self.tty_protocols_initial { - self.enable_tty_protocols(); - } else { - self.disable_tty_protocols(); - } - } - /// Save the current tty modes into the owning job group, if we are transferred. pub fn save_tty_modes(&mut self) { if let Some(ref mut owner) = self.owner { @@ -591,11 +554,25 @@ fn try_transfer(jg: &JobGroup) -> bool { } } -/// The destructor will assert if reclaim() has not been called. impl Drop for TtyHandoff { fn drop(&mut self) { - if !self.reclaimed { - self.reclaim_impl(); + if self.owner.is_some() { + flog!(proc_pgroup, "fish reclaiming terminal"); + if unsafe { libc::tcsetpgrp(STDIN_FILENO, libc::getpgrp()) } == -1 { + flog!( + warning, + "Could not return shell to foreground:", + errno::errno() + ); + perror("tcsetpgrp"); + } + self.owner = None; + } + // Restore the terminal protocols. Note this does nothing if they were unchanged. + if self.tty_protocols_initial { + self.enable_tty_protocols(); + } else { + self.disable_tty_protocols(); } } }