From 210999b9157909e720bfe9eaff031f00a1982204 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Thu, 31 Mar 2022 09:51:06 -0700 Subject: [PATCH] clipboard: restructure how we capture OSC 52 Previously, we'd create a clipboard handler associated with a GUI window and take care to pass that down to the underlying Pane whenever we spawned a new pane. For the mux server, instead of being associated with a GUI window, the clipboard was a special RemoteClipboard that would send a PDU through to the client that spawned the window. The bug here was that when that client went away, the clipboard for that window was broken. If the mux server was the built-in mux in a gui process this could leave a tab without working OSC 52 clipboard support. This commit restructures things so that the Mux is responsible for assigning a clipboard handler that rephrases the clipboard event as a MuxNotification. Both the GUI frontend and the mux server dispatcher already listen for mux notifications and translate those events into appropriate operations on the system clipboard or Pdus to send to the client(s). refs: #1790 --- codec/src/lib.rs | 2 +- docs/changelog.md | 1 + mux/src/lib.rs | 32 +++++++ mux/src/window.rs | 18 ---- wezterm-gui/src/frontend.rs | 29 +++++- wezterm-gui/src/termwindow/clipboard.rs | 43 ++------- wezterm-gui/src/termwindow/mod.rs | 17 ++-- wezterm-gui/src/termwindow/spawn.rs | 30 ++----- wezterm-mux-server-impl/src/dispatch.rs | 14 +++ wezterm-mux-server-impl/src/sessionhandler.rs | 89 +++---------------- 10 files changed, 106 insertions(+), 169 deletions(-) diff --git a/codec/src/lib.rs b/codec/src/lib.rs index 65f29bbaf..ec2ef22b4 100644 --- a/codec/src/lib.rs +++ b/codec/src/lib.rs @@ -405,7 +405,7 @@ macro_rules! pdu { /// The overall version of the codec. /// This must be bumped when backwards incompatible changes /// are made to the types and protocol. -pub const CODEC_VERSION: usize = 20; +pub const CODEC_VERSION: usize = 21; // Defines the Pdu enum. // Each struct has an explicit identifying number. diff --git a/docs/changelog.md b/docs/changelog.md index 3adb8e7b3..531345060 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -40,6 +40,7 @@ As features stabilize some brief notes about them will accumulate here. * `CloseCurrentPane{confirm=false}` would leave behind a phantom tab/pane when used with the multiplexer. [#1277](https://github.com/wez/wezterm/issues/1277) * `CloseCurrentPane{confirm=true}` artifacts when used with the multiplexer. [#783](https://github.com/wez/wezterm/issues/783) * Scrollbar thumb could jump around/move out of bounds. Thanks to [@davidrios](https://github.com/davidrios)! [#1525](https://github.com/wez/wezterm/issues/1525) +* OSC 52 could stop working for tabs/panes spawned into the GUI via the CLI. [#1790](https://github.com/wez/wezterm/issues/1790) ### 20220319-142410-0fcdea07 diff --git a/mux/src/lib.rs b/mux/src/lib.rs index 799640b4c..cd553ff08 100644 --- a/mux/src/lib.rs +++ b/mux/src/lib.rs @@ -24,6 +24,7 @@ use std::time::Instant; use termwiz::escape::csi::{DecPrivateMode, DecPrivateModeCode, Device, Mode}; use termwiz::escape::{Action, CSI}; use thiserror::*; +use wezterm_term::{Clipboard, ClipboardSelection}; #[cfg(windows)] use winapi::um::winsock2::{SOL_SOCKET, SO_RCVBUF, SO_SNDBUF}; @@ -61,6 +62,11 @@ pub enum MuxNotification { alert: wezterm_term::Alert, }, Empty, + AssignClipboard { + pane_id: PaneId, + selection: ClipboardSelection, + clipboard: Option, + }, } static SUB_ID: AtomicUsize = AtomicUsize::new(0); @@ -566,6 +572,11 @@ impl Mux { return Ok(()); } + let clipboard: Arc = Arc::new(MuxClipboard { + pane_id: pane.pane_id(), + }); + pane.set_clipboard(&clipboard); + self.panes .borrow_mut() .insert(pane.pane_id(), Rc::clone(pane)); @@ -1067,3 +1078,24 @@ pub(crate) fn pty_size_to_terminal_size(size: portable_pty::PtySize) -> wezterm_ pixel_height: size.pixel_height as usize, } } + +struct MuxClipboard { + pane_id: PaneId, +} + +impl Clipboard for MuxClipboard { + fn set_contents( + &self, + selection: ClipboardSelection, + clipboard: Option, + ) -> anyhow::Result<()> { + let mux = + Mux::get().ok_or_else(|| anyhow::anyhow!("MuxClipboard::set_contents: no Mux?"))?; + mux.notify(MuxNotification::AssignClipboard { + pane_id: self.pane_id, + selection, + clipboard, + }); + Ok(()) + } +} diff --git a/mux/src/window.rs b/mux/src/window.rs index f349fb8d3..4709acddc 100644 --- a/mux/src/window.rs +++ b/mux/src/window.rs @@ -1,8 +1,6 @@ use crate::pane::CloseReason; use crate::{Mux, MuxNotification, Tab, TabId}; use std::rc::Rc; -use std::sync::Arc; -use wezterm_term::Clipboard; static WIN_ID: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(0); pub type WindowId = usize; @@ -12,7 +10,6 @@ pub struct Window { tabs: Vec>, active: usize, last_active: Option, - clipboard: Option>, workspace: String, } @@ -23,7 +20,6 @@ impl Window { tabs: vec![], active: 0, last_active: None, - clipboard: None, workspace: workspace.unwrap_or_else(|| { Mux::get() .expect("Window::new to be called on mux thread") @@ -46,10 +42,6 @@ impl Window { } } - pub fn set_clipboard(&mut self, clipboard: &Arc) { - self.clipboard.replace(Arc::clone(clipboard)); - } - pub fn window_id(&self) -> WindowId { self.id } @@ -60,14 +52,6 @@ impl Window { } } - fn assign_clipboard_to_tab(&self, tab: &Rc) { - if let Some(clip) = self.clipboard.as_ref() { - if let Some(pane) = tab.get_active_pane() { - pane.set_clipboard(clip); - } - } - } - fn invalidate(&self) { let mux = Mux::get().unwrap(); mux.notify(MuxNotification::WindowInvalidated(self.id)); @@ -75,14 +59,12 @@ impl Window { pub fn insert(&mut self, index: usize, tab: &Rc) { self.check_that_tab_isnt_already_in_window(tab); - self.assign_clipboard_to_tab(tab); self.tabs.insert(index, Rc::clone(tab)); self.invalidate(); } pub fn push(&mut self, tab: &Rc) { self.check_that_tab_isnt_already_in_window(tab); - self.assign_clipboard_to_tab(tab); self.tabs.push(Rc::clone(tab)); self.invalidate(); } diff --git a/wezterm-gui/src/frontend.rs b/wezterm-gui/src/frontend.rs index aef0d914a..7dbd5f123 100644 --- a/wezterm-gui/src/frontend.rs +++ b/wezterm-gui/src/frontend.rs @@ -10,7 +10,7 @@ use std::cell::RefCell; use std::collections::BTreeMap; use std::rc::Rc; use std::sync::Arc; -use wezterm_term::Alert; +use wezterm_term::{Alert, ClipboardSelection}; use wezterm_toast_notification::*; pub struct GuiFrontEnd { @@ -41,7 +41,7 @@ impl GuiFrontEnd { }); let fe = Rc::downgrade(&front_end); mux.subscribe(move |n| { - if let Some(_fe) = fe.upgrade() { + if let Some(fe) = fe.upgrade() { match n { MuxNotification::WindowWorkspaceChanged(_) | MuxNotification::ActiveWorkspaceChanged(_) => {} @@ -94,6 +94,31 @@ impl GuiFrontEnd { Connection::get().unwrap().terminate_message_loop(); } } + MuxNotification::AssignClipboard { + pane_id, + selection, + clipboard, + } => { + log::trace!( + "set clipboard in pane {} {:?} {:?}", + pane_id, + selection, + clipboard + ); + if let Some(window) = fe.known_windows.borrow().keys().next() { + window.set_clipboard( + match selection { + ClipboardSelection::Clipboard => Clipboard::Clipboard, + ClipboardSelection::PrimarySelection => { + Clipboard::PrimarySelection + } + }, + clipboard.unwrap_or_else(String::new), + ); + } else { + log::error!("Cannot assign clipboard as there are no windows"); + } + } } true } else { diff --git a/wezterm-gui/src/termwindow/clipboard.rs b/wezterm-gui/src/termwindow/clipboard.rs index caa5e7eef..3732884cd 100644 --- a/wezterm-gui/src/termwindow/clipboard.rs +++ b/wezterm-gui/src/termwindow/clipboard.rs @@ -6,50 +6,20 @@ use mux::window::WindowId as MuxWindowId; use mux::Mux; use std::rc::Rc; use std::sync::Arc; -use wezterm_term::ClipboardSelection; -use window::{Clipboard, Window, WindowOps}; - -/// ClipboardHelper bridges between the window crate clipboard -/// manipulation and the term crate clipboard interface -#[derive(Clone)] -pub struct ClipboardHelper { - pub window: Window, -} - -impl wezterm_term::Clipboard for ClipboardHelper { - fn set_contents( - &self, - selection: ClipboardSelection, - data: Option, - ) -> anyhow::Result<()> { - self.window.set_clipboard( - match selection { - ClipboardSelection::Clipboard => Clipboard::Clipboard, - ClipboardSelection::PrimarySelection => Clipboard::PrimarySelection, - }, - data.unwrap_or_else(String::new), - ); - Ok(()) - } -} +use window::{Clipboard, WindowOps}; impl TermWindow { - pub fn setup_clipboard(window: &Window, mux_window_id: MuxWindowId) -> anyhow::Result<()> { - let clipboard: Arc = Arc::new(ClipboardHelper { - window: window.clone(), - }); + pub fn setup_clipboard(mux_window_id: MuxWindowId) -> anyhow::Result<()> { let downloader: Arc = Arc::new(crate::download::Downloader::new()); let mux = Mux::get().unwrap(); - let mut mux_window = mux - .get_window_mut(mux_window_id) + let mux_window = mux + .get_window(mux_window_id) .ok_or_else(|| anyhow::anyhow!("mux doesn't know about window yet!?"))?; - mux_window.set_clipboard(&clipboard); for tab in mux_window.iter() { for pos in tab.iter_panes() { - pos.pane.set_clipboard(&clipboard); pos.pane.set_download_handler(&downloader); } } @@ -75,6 +45,11 @@ impl TermWindow { pub fn paste_from_clipboard(&mut self, pane: &Rc, clipboard: ClipboardPasteSource) { let pane_id = pane.pane_id(); + log::trace!( + "paste_from_clipboard in pane {} {:?}", + pane.pane_id(), + clipboard + ); let window = self.window.as_ref().unwrap().clone(); let clipboard = match clipboard { ClipboardPasteSource::Clipboard => Clipboard::Clipboard, diff --git a/wezterm-gui/src/termwindow/mod.rs b/wezterm-gui/src/termwindow/mod.rs index 04a3ed19b..e36d070ed 100644 --- a/wezterm-gui/src/termwindow/mod.rs +++ b/wezterm-gui/src/termwindow/mod.rs @@ -60,7 +60,6 @@ mod render; pub mod resize; mod selection; pub mod spawn; -use clipboard::ClipboardHelper; use prevcursor::PrevCursorPos; use spawn::SpawnWhere; @@ -798,7 +797,7 @@ impl TermWindow { tw.borrow_mut().window.replace(window.clone()); Self::apply_icon(&window)?; - Self::setup_clipboard(&window, mux_window_id)?; + Self::setup_clipboard(mux_window_id)?; let config_subscription = config::subscribe_to_config_reload({ let window = window.clone(); @@ -1029,6 +1028,9 @@ impl TermWindow { MuxNotification::WindowRemoved(_window_id) => { // Handled by frontend } + MuxNotification::AssignClipboard { .. } => { + // Handled by frontend + } MuxNotification::PaneAdded(_) | MuxNotification::PaneRemoved(_) | MuxNotification::WindowWorkspaceChanged(_) @@ -1153,12 +1155,6 @@ impl TermWindow { for tab in mux_window.iter() { for pos in tab.iter_panes() { if pos.pane.pane_id() == pane_id { - let clipboard: Arc = - Arc::new(ClipboardHelper { - window: window.clone(), - }); - pos.pane.set_clipboard(&clipboard); - let downloader: Arc = Arc::new(crate::download::Downloader::new()); pos.pane.set_download_handler(&downloader); @@ -1186,6 +1182,7 @@ impl TermWindow { | Alert::PaletteChanged { .. }, .. } + | MuxNotification::AssignClipboard { .. } | MuxNotification::PaneRemoved(_) | MuxNotification::WindowCreated(_) | MuxNotification::ActiveWorkspaceChanged(_) @@ -2252,9 +2249,6 @@ impl TermWindow { let size = self.terminal_size; let term_config = Arc::new(TermConfig::with_config(self.config.clone())); let src_window_id = self.mux_window_id; - let clipboard = ClipboardHelper { - window: self.window.as_ref().unwrap().clone(), - }; promise::spawn::spawn(async move { if let Err(err) = Self::spawn_command_internal( @@ -2262,7 +2256,6 @@ impl TermWindow { SpawnWhere::NewWindow, size, src_window_id, - clipboard, term_config, ) .await diff --git a/wezterm-gui/src/termwindow/spawn.rs b/wezterm-gui/src/termwindow/spawn.rs index ffaf78cd4..25b0ef127 100644 --- a/wezterm-gui/src/termwindow/spawn.rs +++ b/wezterm-gui/src/termwindow/spawn.rs @@ -1,4 +1,4 @@ -use crate::termwindow::{ClipboardHelper, MuxWindowId}; +use crate::termwindow::MuxWindowId; use anyhow::{anyhow, bail, Context}; use config::keyassignment::{SpawnCommand, SpawnTabDomain}; use config::TermConfig; @@ -24,16 +24,7 @@ impl super::TermWindow { }; let term_config = Arc::new(TermConfig::with_config(self.config.clone())); - Self::spawn_command_impl( - spawn, - spawn_where, - size, - self.mux_window_id, - ClipboardHelper { - window: self.window.as_ref().unwrap().clone(), - }, - term_config, - ) + Self::spawn_command_impl(spawn, spawn_where, size, self.mux_window_id, term_config) } fn spawn_command_impl( @@ -41,21 +32,14 @@ impl super::TermWindow { spawn_where: SpawnWhere, size: PtySize, src_window_id: MuxWindowId, - clipboard: ClipboardHelper, term_config: Arc, ) { let spawn = spawn.clone(); promise::spawn::spawn(async move { - if let Err(err) = Self::spawn_command_internal( - spawn, - spawn_where, - size, - src_window_id, - clipboard, - term_config, - ) - .await + if let Err(err) = + Self::spawn_command_internal(spawn, spawn_where, size, src_window_id, term_config) + .await { log::error!("Failed to spawn: {:#}", err); } @@ -68,7 +52,6 @@ impl super::TermWindow { spawn_where: SpawnWhere, size: PtySize, src_window_id: MuxWindowId, - clipboard: ClipboardHelper, term_config: Arc, ) -> anyhow::Result<()> { let mux = Mux::get().unwrap(); @@ -104,7 +87,6 @@ impl super::TermWindow { None }; - let clipboard: Arc = Arc::new(clipboard); let downloader: Arc = Arc::new(crate::download::Downloader::new()); let workspace = mux.active_workspace().clone(); @@ -129,7 +111,6 @@ impl super::TermWindow { .await .context("split_pane")?; pane.set_config(term_config); - pane.set_clipboard(&clipboard); pane.set_download_handler(&downloader); } else { bail!("there is no active tab while splitting pane!?"); @@ -157,7 +138,6 @@ impl super::TermWindow { // the new window being created. if window_id == src_window_id { pane.set_config(term_config); - pane.set_clipboard(&clipboard); pane.set_download_handler(&downloader); } } diff --git a/wezterm-mux-server-impl/src/dispatch.rs b/wezterm-mux-server-impl/src/dispatch.rs index 3c859b649..9f59b617d 100644 --- a/wezterm-mux-server-impl/src/dispatch.rs +++ b/wezterm-mux-server-impl/src/dispatch.rs @@ -96,6 +96,20 @@ where } handler.schedule_pane_push(pane_id); } + Ok(Item::Notif(MuxNotification::AssignClipboard { + pane_id, + selection, + clipboard, + })) => { + Pdu::SetClipboard(codec::SetClipboard { + pane_id, + clipboard, + selection, + }) + .encode_async(&mut stream, 0) + .await?; + stream.flush().await.context("flushing PDU to client")?; + } Ok(Item::Notif(MuxNotification::WindowRemoved(_window_id))) => {} Ok(Item::Notif(MuxNotification::WindowCreated(_window_id))) => {} Ok(Item::Notif(MuxNotification::WindowInvalidated(_window_id))) => {} diff --git a/wezterm-mux-server-impl/src/sessionhandler.rs b/wezterm-mux-server-impl/src/sessionhandler.rs index 0e912e441..e7d27cbee 100644 --- a/wezterm-mux-server-impl/src/sessionhandler.rs +++ b/wezterm-mux-server-impl/src/sessionhandler.rs @@ -13,7 +13,7 @@ use std::sync::{Arc, Mutex}; use std::time::Instant; use termwiz::surface::SequenceNo; use url::Url; -use wezterm_term::terminal::{Alert, Clipboard, ClipboardSelection}; +use wezterm_term::terminal::Alert; use wezterm_term::StableRowIndex; #[derive(Clone)] @@ -204,17 +204,6 @@ impl Drop for SessionHandler { impl SessionHandler { pub fn new(to_write_tx: PduSender) -> Self { - // Fixup the clipboard on the empty initial pane that is - // spawned into the mux - let mux = Mux::get().unwrap(); - for pane in mux.iter_panes() { - let clip: Arc = Arc::new(RemoteClipboard { - pane_id: pane.pane_id(), - sender: to_write_tx.clone(), - }); - pane.set_clipboard(&clip); - } - Self { to_write_tx, per_pane: HashMap::new(), @@ -537,19 +526,17 @@ impl SessionHandler { } Pdu::SpawnV2(spawn) => { - let sender = self.to_write_tx.clone(); let client_id = self.client_id.clone(); spawn_into_main_thread(async move { - schedule_domain_spawn_v2(spawn, sender, send_response, client_id); + schedule_domain_spawn_v2(spawn, send_response, client_id); }) .detach(); } Pdu::SplitPane(split) => { - let sender = self.to_write_tx.clone(); let client_id = self.client_id.clone(); spawn_into_main_thread(async move { - schedule_split_pane(split, sender, send_response, client_id); + schedule_split_pane(split, send_response, client_id); }) .detach(); } @@ -707,60 +694,24 @@ impl SessionHandler { // analysis and allow things to compile. fn schedule_domain_spawn_v2( spawn: SpawnV2, - sender: PduSender, send_response: SND, client_id: Option>, ) where SND: Fn(anyhow::Result) + 'static, { - promise::spawn::spawn( - async move { send_response(domain_spawn_v2(spawn, sender, client_id).await) }, - ) - .detach(); -} - -fn schedule_split_pane( - split: SplitPane, - sender: PduSender, - send_response: SND, - client_id: Option>, -) where - SND: Fn(anyhow::Result) + 'static, -{ - promise::spawn::spawn(async move { send_response(split_pane(split, sender, client_id).await) }) + promise::spawn::spawn(async move { send_response(domain_spawn_v2(spawn, client_id).await) }) .detach(); } -struct RemoteClipboard { - sender: PduSender, - pane_id: PaneId, +fn schedule_split_pane(split: SplitPane, send_response: SND, client_id: Option>) +where + SND: Fn(anyhow::Result) + 'static, +{ + promise::spawn::spawn(async move { send_response(split_pane(split, client_id).await) }) + .detach(); } -impl Clipboard for RemoteClipboard { - fn set_contents( - &self, - selection: ClipboardSelection, - clipboard: Option, - ) -> anyhow::Result<()> { - self.sender - .send(DecodedPdu { - serial: 0, - pdu: Pdu::SetClipboard(SetClipboard { - pane_id: self.pane_id, - clipboard, - selection, - }), - }) - .context("RemoteClipboard::set_contents send failed")?; - Ok(()) - } -} - -async fn split_pane( - split: SplitPane, - sender: PduSender, - client_id: Option>, -) -> anyhow::Result { +async fn split_pane(split: SplitPane, client_id: Option>) -> anyhow::Result { let mux = Mux::get().unwrap(); let _identity = mux.with_identity(client_id); @@ -778,12 +729,6 @@ async fn split_pane( ) .await?; - let clip: Arc = Arc::new(RemoteClipboard { - pane_id: pane.pane_id(), - sender, - }); - pane.set_clipboard(&clip); - Ok::(Pdu::SpawnResponse(SpawnResponse { pane_id: pane.pane_id(), tab_id: tab_id, @@ -792,11 +737,7 @@ async fn split_pane( })) } -async fn domain_spawn_v2( - spawn: SpawnV2, - sender: PduSender, - client_id: Option>, -) -> anyhow::Result { +async fn domain_spawn_v2(spawn: SpawnV2, client_id: Option>) -> anyhow::Result { let mux = Mux::get().unwrap(); let _identity = mux.with_identity(client_id); @@ -812,12 +753,6 @@ async fn domain_spawn_v2( ) .await?; - let clip: Arc = Arc::new(RemoteClipboard { - pane_id: pane.pane_id(), - sender, - }); - pane.set_clipboard(&clip); - Ok::(Pdu::SpawnResponse(SpawnResponse { pane_id: pane.pane_id(), tab_id: tab.tab_id(),