From e9f1297b4559c2598c478c61fc84b3d351a85917 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sun, 19 Dec 2021 09:00:58 -0700 Subject: [PATCH] mux: improve Window closing behavior Finally getting around to fixing this usability wart: this commit changes the behavior of Window closing so that you can close a window containing multiplexer panes without prompting and without killing off those panes. This is achieved through some plumbing: * The mux can now advise Domains about an impending window closure, giving them the opportunity to "do things" in readiness. * The mux client domain informs the container ClientPane instances to ignore the next Pane::kill call, which would otherwise inform the mux server to kill the remote pane * Pane:can_close_without_prompting now requires a CloseReason. * ClientPane's can_close_without_prompting impl allows Window closing without prompting on the assumption that the ignore-next-kill hack above is working refs: #848 refs: #917 refs: #1224 --- docs/changelog.md | 1 + mux/src/domain.rs | 5 +++++ mux/src/lib.rs | 12 +++++++++++- mux/src/localpane.rs | 4 ++-- mux/src/pane.rs | 13 ++++++++++++- mux/src/tab.rs | 4 ++-- mux/src/termwiztermtab.rs | 4 ++-- mux/src/window.rs | 3 ++- wezterm-client/src/domain.rs | 18 +++++++++++++++++ wezterm-client/src/pane/clientpane.rs | 28 ++++++++++++++++++++++++++- wezterm-gui/src/termwindow/mod.rs | 6 +++--- 11 files changed, 85 insertions(+), 13 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 617e1c2fa..3cdc2350c 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -41,6 +41,7 @@ As features stabilize some brief notes about them will accumulate here. * Changing font scaling on Windows no longer causes the initial terminal rows/cols to be under-sized [#1381](https://github.com/wez/wezterm/issues/1381) * New version update notifications are now more coordinated between multiple wezterm GUI instances, and update related configuration now respects configuration reloading. [#1402](https://github.com/wez/wezterm/issues/1402) * [TLS domains](multiplexing.md) bootstrapping via SSH now use the `libssh` backend by default and work more reliably on Windows +* Closing a window will no longer recursively terminate contained multiplexer client panes; the window will instead be restored when you next connect to that multiplexer server. Killing/closing individual tabs/panes *will* kill the panes; this change only affects closing the window. [#848](https://github.com/wez/wezterm/issues/848) [#917](https://github.com/wez/wezterm/issues/917) [#1224](https://github.com/wez/wezterm/issues/1224) ### 20211205-192649-672c1cc1 diff --git a/mux/src/domain.rs b/mux/src/domain.rs index 48c7beb80..e926d59a6 100644 --- a/mux/src/domain.rs +++ b/mux/src/domain.rs @@ -79,6 +79,11 @@ pub trait Domain: Downcast { /// Indicates the state of the domain fn state(&self) -> DomainState; + + /// Called to advise the domain that a local window is closing. + /// This allows the domain the opportunity to eg: detach/hide + /// its tabs/panes rather than actually killing them off + fn local_window_is_closing(&self, _window_id: WindowId) {} } impl_downcast!(Domain); diff --git a/mux/src/lib.rs b/mux/src/lib.rs index e13d786fa..4c8b4d8f3 100644 --- a/mux/src/lib.rs +++ b/mux/src/lib.rs @@ -453,6 +453,10 @@ impl Mux { fn remove_window_internal(&self, window_id: WindowId) { log::debug!("remove_window_internal {}", window_id); + let domains: Vec> = self.domains.borrow().values().cloned().collect(); + for dom in domains { + dom.local_window_is_closing(window_id); + } let window = self.windows.borrow_mut().remove(&window_id); if let Some(window) = window { for tab in window.iter() { @@ -475,6 +479,7 @@ impl Mux { pub fn prune_dead_windows(&self) { if Activity::count() > 0 { + log::trace!("prune_dead_windows: Activity::count={}", Activity::count()); return; } let live_tab_ids: Vec = self.tabs.borrow().keys().cloned().collect(); @@ -486,13 +491,14 @@ impl Mux { Ok(w) => w, Err(_) => { // It's ok if our caller already locked it; we can prune later. + log::trace!("prune_dead_windows: self.windows already borrowed"); return; } }; for (window_id, win) in windows.iter_mut() { win.prune_dead_tabs(&live_tab_ids); if win.is_empty() { - log::debug!("prune_dead_windows: window is now empty"); + log::trace!("prune_dead_windows: window is now empty"); dead_windows.push(*window_id); } } @@ -516,12 +522,16 @@ impl Mux { } if self.is_empty() { + log::trace!("prune_dead_windows: is_empty, send MuxNotification::Empty"); self.notify(MuxNotification::Empty); + } else { + log::trace!("prune_dead_windows: not empty"); } } pub fn kill_window(&self, window_id: WindowId) { self.remove_window_internal(window_id); + self.prune_dead_windows(); } pub fn get_window(&self, window_id: WindowId) -> Option> { diff --git a/mux/src/localpane.rs b/mux/src/localpane.rs index 2b9f00a57..b3013d1af 100644 --- a/mux/src/localpane.rs +++ b/mux/src/localpane.rs @@ -1,5 +1,5 @@ use crate::domain::DomainId; -use crate::pane::{Pane, PaneId, Pattern, SearchResult}; +use crate::pane::{CloseReason, Pane, PaneId, Pattern, SearchResult}; use crate::renderable::*; use crate::tmux::{TmuxDomain, TmuxDomainState}; use crate::{Domain, Mux, MuxNotification}; @@ -316,7 +316,7 @@ impl Pane for LocalPane { .or_else(|| self.divine_current_working_dir()) } - fn can_close_without_prompting(&self) -> bool { + fn can_close_without_prompting(&self, _reason: CloseReason) -> bool { if let Some(proc_list) = self.divine_process_list() { log::trace!( "can_close_without_prompting? procs in pane {:#?}", diff --git a/mux/src/pane.rs b/mux/src/pane.rs index 065352b98..5fa997313 100644 --- a/mux/src/pane.rs +++ b/mux/src/pane.rs @@ -42,6 +42,17 @@ pub struct SearchResult { pub use config::keyassignment::Pattern; +/// Why a close request is being made +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum CloseReason { + /// The containing window is being closed + Window, + /// The containing tab is being close + Tab, + /// Just this tab is being closed + Pane, +} + const PASTE_CHUNK_SIZE: usize = 1024; struct Paste { @@ -328,7 +339,7 @@ pub trait Pane: Downcast { fn focus_changed(&self, _focused: bool) {} /// Certain panes are OK to be closed with impunity (no prompts) - fn can_close_without_prompting(&self) -> bool { + fn can_close_without_prompting(&self, _reason: CloseReason) -> bool { false } diff --git a/mux/src/tab.rs b/mux/src/tab.rs index 3a247eee7..f410e9101 100644 --- a/mux/src/tab.rs +++ b/mux/src/tab.rs @@ -1235,10 +1235,10 @@ impl Tab { } } - pub fn can_close_without_prompting(&self) -> bool { + pub fn can_close_without_prompting(&self, reason: CloseReason) -> bool { let panes = self.iter_panes(); for pos in &panes { - if !pos.pane.can_close_without_prompting() { + if !pos.pane.can_close_without_prompting(reason) { return false; } } diff --git a/mux/src/termwiztermtab.rs b/mux/src/termwiztermtab.rs index 7cf17e37d..c1c708149 100644 --- a/mux/src/termwiztermtab.rs +++ b/mux/src/termwiztermtab.rs @@ -4,7 +4,7 @@ //! session. use crate::domain::{alloc_domain_id, Domain, DomainId, DomainState}; -use crate::pane::{alloc_pane_id, Pane, PaneId}; +use crate::pane::{alloc_pane_id, CloseReason, Pane, PaneId}; use crate::renderable::*; use crate::tab::{SplitDirection, Tab, TabId}; use crate::window::WindowId; @@ -162,7 +162,7 @@ impl Pane for TermWizTerminalPane { self.terminal.borrow_mut().get_title().to_string() } - fn can_close_without_prompting(&self) -> bool { + fn can_close_without_prompting(&self, _reason: CloseReason) -> bool { true } diff --git a/mux/src/window.rs b/mux/src/window.rs index 06f444d1d..0da0790fb 100644 --- a/mux/src/window.rs +++ b/mux/src/window.rs @@ -1,3 +1,4 @@ +use crate::pane::CloseReason; use crate::{Mux, MuxNotification, Tab, TabId}; use std::rc::Rc; use std::sync::Arc; @@ -80,7 +81,7 @@ impl Window { pub fn can_close_without_prompting(&self) -> bool { for tab in &self.tabs { - if !tab.can_close_without_prompting() { + if !tab.can_close_without_prompting(CloseReason::Window) { return false; } } diff --git a/wezterm-client/src/domain.rs b/wezterm-client/src/domain.rs index 03244dd7c..647dfdb61 100644 --- a/wezterm-client/src/domain.rs +++ b/wezterm-client/src/domain.rs @@ -522,6 +522,24 @@ impl Domain for ClientDomain { Ok(()) } + fn local_window_is_closing(&self, window_id: WindowId) { + let mux = Mux::get().expect("to be called by mux on mux thread"); + let window = match mux.get_window(window_id) { + Some(w) => w, + None => return, + }; + + for tab in window.iter() { + for pos in tab.iter_panes_ignoring_zoom() { + if pos.pane.domain_id() == self.local_domain_id { + if let Some(client_pane) = pos.pane.downcast_ref::() { + client_pane.ignore_next_kill(); + } + } + } + } + } + fn detach(&self) -> anyhow::Result<()> { bail!("detach not implemented"); } diff --git a/wezterm-client/src/pane/clientpane.rs b/wezterm-client/src/pane/clientpane.rs index ec2f50ef1..37266f8e5 100644 --- a/wezterm-client/src/pane/clientpane.rs +++ b/wezterm-client/src/pane/clientpane.rs @@ -6,7 +6,7 @@ use async_trait::async_trait; use codec::*; use config::configuration; use mux::domain::DomainId; -use mux::pane::{alloc_pane_id, Pane, PaneId, Pattern, SearchResult}; +use mux::pane::{alloc_pane_id, CloseReason, Pane, PaneId, Pattern, SearchResult}; use mux::renderable::{RenderableDimensions, StableCursorPosition}; use mux::tab::TabId; use mux::{Mux, MuxNotification}; @@ -35,6 +35,7 @@ pub struct ClientPane { mouse: Rc>, clipboard: RefCell>>, mouse_grabbed: RefCell, + ignore_next_kill: RefCell, } impl ClientPane { @@ -90,6 +91,7 @@ impl ClientPane { palette: RefCell::new(palette), clipboard: RefCell::new(None), mouse_grabbed: RefCell::new(false), + ignore_next_kill: RefCell::new(false), } } @@ -151,6 +153,17 @@ impl ClientPane { pub fn remote_pane_id(&self) -> TabId { self.remote_pane_id } + + /// Arrange to suppress the next Pane::kill call. + /// This is a bit of a hack that we use when closing a window; + /// our Domain::local_window_is_closing impl calls this for each + /// ClientPane in the window so that closing a window effectively + /// "detaches" the window so that reconnecting later will resume + /// from where they left off. + /// It isn't perfect. + pub fn ignore_next_kill(&self) { + *self.ignore_next_kill.borrow_mut() = true; + } } #[async_trait(?Send)] @@ -332,6 +345,11 @@ impl Pane for ClientPane { } fn kill(&self) { + let mut ignore = self.ignore_next_kill.borrow_mut(); + if *ignore { + *ignore = false; + return; + } let client = Arc::clone(&self.client); let remote_pane_id = self.remote_pane_id; promise::spawn::spawn(async move { @@ -387,6 +405,14 @@ impl Pane for ClientPane { fn get_current_working_dir(&self) -> Option { self.renderable.borrow().inner.borrow().working_dir.clone() } + + fn can_close_without_prompting(&self, reason: CloseReason) -> bool { + match reason { + CloseReason::Window => true, + CloseReason::Tab => false, + CloseReason::Pane => false, + } + } } struct PaneWriter { diff --git a/wezterm-gui/src/termwindow/mod.rs b/wezterm-gui/src/termwindow/mod.rs index bba5f13ac..99b5889cb 100644 --- a/wezterm-gui/src/termwindow/mod.rs +++ b/wezterm-gui/src/termwindow/mod.rs @@ -28,7 +28,7 @@ use config::{ use luahelper::impl_lua_conversion; use mlua::FromLua; use mux::domain::{DomainId, DomainState}; -use mux::pane::{Pane, PaneId}; +use mux::pane::{CloseReason, Pane, PaneId}; use mux::renderable::RenderableDimensions; use mux::tab::{PositionedPane, PositionedSplit, SplitDirection, Tab, TabId}; use mux::window::WindowId as MuxWindowId; @@ -2064,7 +2064,7 @@ impl TermWindow { }; let pane_id = pane.pane_id(); - if confirm && !pane.can_close_without_prompting() { + if confirm && !pane.can_close_without_prompting(CloseReason::Pane) { let window = self.window.clone().unwrap(); let (overlay, future) = start_overlay_pane(self, &pane, move |pane_id, term| { confirm_close_pane(pane_id, term, mux_window_id, window) @@ -2084,7 +2084,7 @@ impl TermWindow { }; let tab_id = tab.tab_id(); let mux_window_id = self.mux_window_id; - if confirm && !tab.can_close_without_prompting() { + if confirm && !tab.can_close_without_prompting(CloseReason::Tab) { let window = self.window.clone().unwrap(); let (overlay, future) = start_overlay(self, &tab, move |tab_id, term| { confirm_close_tab(tab_id, term, mux_window_id, window)