diff --git a/frontends/rioterm/src/context/mod.rs b/frontends/rioterm/src/context/mod.rs index 784487e339..2416ebdac6 100644 --- a/frontends/rioterm/src/context/mod.rs +++ b/frontends/rioterm/src/context/mod.rs @@ -51,9 +51,7 @@ pub struct Context { pub renderable_content: RenderableContent, pub messenger: Messenger, #[cfg(not(target_os = "windows"))] - pub main_fd: Arc, - #[cfg(not(target_os = "windows"))] - pub shell_pid: u32, + pty: Option<(i32, u32)>, pub rich_text_id: usize, pub dimension: ContextDimension, pub title: ContextTitle, @@ -63,19 +61,32 @@ pub struct Context { impl Drop for Context { fn drop(&mut self) { - // Shutdown the terminal's PTY. + // The performer owns the PTY and terminates its child when it shuts down. let _ = self.messenger.channel.send(Msg::Shutdown); - - // `create_dead_context` uses 1 as a placeholder PID, so guard against - // signalling init (1) or our own process group (0). - #[cfg(not(target_os = "windows"))] - if self.shell_pid > 1 { - teletypewriter::kill_pid(self.shell_pid as i32); - } } } impl Context { + fn foreground_process_name(&self) -> Option { + #[cfg(not(target_os = "windows"))] + return self.pty.as_ref().map(|(main_fd, shell_pid)| { + teletypewriter::foreground_process_name(*main_fd, *shell_pid) + }); + + #[cfg(target_os = "windows")] + None + } + + pub(crate) fn foreground_process_path(&self) -> Option { + #[cfg(not(target_os = "windows"))] + return self.pty.as_ref().and_then(|(main_fd, shell_pid)| { + teletypewriter::foreground_process_path(*main_fd, *shell_pid).ok() + }); + + #[cfg(target_os = "windows")] + None + } + #[inline] pub fn set_selection(&mut self, selection_range: Option) { let old_selection = self.renderable_content.selection_range; @@ -163,9 +174,7 @@ pub fn create_dead_context( Context { route_id, #[cfg(not(target_os = "windows"))] - main_fd: Arc::new(-1), - #[cfg(not(target_os = "windows"))] - shell_pid: 1, + pty: None, messenger: Messenger::new(sender), renderable_content: RenderableContent::new(Cursor::default()), terminal, @@ -296,9 +305,9 @@ impl ContextManager { } #[cfg(not(target_os = "windows"))] - let main_fd = pty.child.id.clone(); + let main_fd = pty.child.id; #[cfg(not(target_os = "windows"))] - let shell_pid = *pty.child.pid.clone() as u32; + let shell_pid = pty.child.pid as u32; #[cfg(target_os = "windows")] { @@ -337,9 +346,7 @@ impl ContextManager { Ok(Context { route_id, #[cfg(not(target_os = "windows"))] - main_fd, - #[cfg(not(target_os = "windows"))] - shell_pid, + pty: Some((main_fd, shell_pid)), messenger, terminal, rich_text_id, @@ -1001,10 +1008,7 @@ impl ContextManager { #[cfg(not(target_os = "windows"))] { let current_context = self.current(); - if let Ok(path) = teletypewriter::foreground_process_path( - *current_context.main_fd, - current_context.shell_pid, - ) { + if let Some(path) = current_context.foreground_process_path() { working_dir = Some(path.to_string_lossy().to_string()); } } @@ -1121,10 +1125,7 @@ impl ContextManager { #[cfg(not(target_os = "windows"))] { let current_context = self.current(); - if let Ok(path) = teletypewriter::foreground_process_path( - *current_context.main_fd, - current_context.shell_pid, - ) { + if let Some(path) = current_context.foreground_process_path() { working_dir = Some(path.to_string_lossy().to_string()); } } diff --git a/frontends/rioterm/src/context/title.rs b/frontends/rioterm/src/context/title.rs index 9af6596b0c..fe15992b8e 100644 --- a/frontends/rioterm/src/context/title.rs +++ b/frontends/rioterm/src/context/title.rs @@ -22,13 +22,7 @@ impl Default for ContextTitle { pub fn create_title_extra_from_context( context: &Context, ) -> Option { - #[cfg(unix)] - let program = - teletypewriter::foreground_process_name(*context.main_fd, context.shell_pid); - - #[cfg(not(unix))] - let program = String::default(); - + let program = context.foreground_process_name()?; Some(ContextTitleExtra { program }) } @@ -79,6 +73,41 @@ fn shorten_path(absolute: &str) -> String { } } +fn current_path( + context: &Context, +) -> Option { + context + .terminal + .lock() + .current_directory + .clone() + .and_then(|path| path.into_os_string().into_string().ok()) + .or_else(|| { + context + .foreground_process_path() + .map(|path| path.to_string_lossy().into_owned()) + }) +} + +fn variable_value( + variable: &str, + context: &Context, +) -> Option { + match variable.trim().to_ascii_lowercase().as_str() { + "columns" => Some(context.dimension.columns.to_string()), + "lines" => Some(context.dimension.lines.to_string()), + "title" => Some(context.terminal.lock().title.clone()), + "program" => Some(context.foreground_process_name().unwrap_or_default()), + "absolute_path" => Some(current_path(context).unwrap_or_default()), + "relative_path" => Some( + current_path(context) + .map(|path| shorten_path(&path)) + .unwrap_or_default(), + ), + _ => None, + } +} + #[inline] pub fn update_title( template: &str, @@ -92,144 +121,17 @@ pub fn update_title( let re = regex::Regex::new(r"\{\{(.*?)\}\}").unwrap(); for (to_replace_str, [variable]) in re.captures_iter(template).map(|c| c.extract()) { - let variables = if to_replace_str.contains("||") { - variable.split("||").collect() - } else { - vec![variable] - }; - - let mut matched = false; - for (i, scoped_variable) in variables.iter().enumerate() { - if matched { - break; + let mut variables = variable.split("||").peekable(); + while let Some(variable) = variables.next() { + let Some(value) = variable_value(variable, context) else { + continue; + }; + if value.is_empty() && variables.peek().is_some() { + continue; } - let var = scoped_variable.to_owned().trim().to_lowercase(); - match var.as_str() { - "columns" => { - new_template = new_template - .replace(to_replace_str, &context.dimension.columns.to_string()); - matched = true; - } - "lines" => { - new_template = new_template - .replace(to_replace_str, &context.dimension.lines.to_string()); - matched = true; - } - "title" => { - let terminal_title = { - let terminal = context.terminal.lock(); - terminal.title.to_string() - }; - - // In case it has a fallback and title is empty - // or - // In case is the last then we need to erase variables either way - let is_only_one = variables.len() == 1; - let is_last = i == variables.len() - 1; - if is_only_one || is_last { - new_template = - new_template.replace(to_replace_str, &terminal_title); - continue; - } - - if !terminal_title.is_empty() { - new_template = - new_template.replace(to_replace_str, &terminal_title); - matched = true; - } - } - "program" => { - #[cfg(unix)] - { - let program = teletypewriter::foreground_process_name( - *context.main_fd, - context.shell_pid, - ); - - new_template = new_template.replace(to_replace_str, &program); - matched = true; - } - } - "absolute_path" => { - { - let terminal = context.terminal.lock(); - if let Some(current_directory) = &terminal.current_directory { - if let Ok(dir_str) = - current_directory.clone().into_os_string().into_string() - { - new_template = - new_template.replace(to_replace_str, &dir_str); - matched = true; - continue; - } - }; - } - - #[cfg(unix)] - { - let path = teletypewriter::foreground_process_path( - *context.main_fd, - context.shell_pid, - ) - .map(|p| p.to_string_lossy().to_string()) - .unwrap_or_default(); - - // In case it has a fallback and path is empty - // or - // In case is the last then we need to erase variables either way - let is_only_one = variables.len() == 1; - let is_last = i == variables.len() - 1; - if is_only_one || is_last { - new_template = new_template.replace(to_replace_str, &path); - continue; - } - - if !path.is_empty() { - new_template = new_template.replace(to_replace_str, &path); - matched = true; - } - } - } - "relative_path" => { - { - let terminal = context.terminal.lock(); - if let Some(current_directory) = &terminal.current_directory { - if let Ok(dir_str) = - current_directory.clone().into_os_string().into_string() - { - new_template = new_template - .replace(to_replace_str, &shorten_path(&dir_str)); - matched = true; - continue; - } - }; - } - - #[cfg(unix)] - { - let path = teletypewriter::foreground_process_path( - *context.main_fd, - context.shell_pid, - ) - .map(|p| shorten_path(&p.to_string_lossy())) - .unwrap_or_default(); - - let is_only_one = variables.len() == 1; - let is_last = i == variables.len() - 1; - if is_only_one || is_last { - new_template = new_template.replace(to_replace_str, &path); - continue; - } - - if !path.is_empty() { - new_template = new_template.replace(to_replace_str, &path); - matched = true; - } - } - } - _ => {} - } + new_template = new_template.replace(to_replace_str, &value); + break; } } @@ -348,6 +250,11 @@ pub mod test { String::from("64") ); + assert_eq!( + update_title("{{ program || columns }}", &context), + String::from("64") + ); + assert_eq!( update_title("{{ title || title }}", &context), String::from("") diff --git a/frontends/rioterm/src/screen/mod.rs b/frontends/rioterm/src/screen/mod.rs index ad4f3f60e0..594549ae2e 100644 --- a/frontends/rioterm/src/screen/mod.rs +++ b/frontends/rioterm/src/screen/mod.rs @@ -2339,25 +2339,11 @@ impl Screen<'_> { I: IntoIterator + Debug + Copy, S: AsRef, { - #[cfg(unix)] - { - let main_fd = *self.ctx().current().main_fd; - let shell_pid = &self.ctx().current().shell_pid; - match teletypewriter::spawn_daemon(program, args, main_fd, *shell_pid) { - Ok(_) => tracing::debug!("Launched {} with args {:?}", program, args), - Err(_) => { - tracing::warn!("Unable to launch {} with args {:?}", program, args) - } - } - } - - #[cfg(windows)] - { - match teletypewriter::spawn_daemon(program, args) { - Ok(_) => tracing::debug!("Launched {} with args {:?}", program, args), - Err(_) => { - tracing::warn!("Unable to launch {} with args {:?}", program, args) - } + let cwd = self.ctx().current().foreground_process_path(); + match teletypewriter::spawn_daemon(program, args, cwd.as_deref()) { + Ok(_) => tracing::debug!("Launched {} with args {:?}", program, args), + Err(_) => { + tracing::warn!("Unable to launch {} with args {:?}", program, args) } } } diff --git a/librio/src/lib.rs b/librio/src/lib.rs index 1a0440e050..07c4e2c98a 100644 --- a/librio/src/lib.rs +++ b/librio/src/lib.rs @@ -135,10 +135,10 @@ pub enum Action { /// nothing rather than forcing unsafe impls on the embedder. #[cfg(not(target_arch = "wasm32"))] pub trait MaybeSendSync: Send + Sync {} -#[cfg(not(target_arch = "wasm32"))] -impl MaybeSendSync for T {} #[cfg(target_arch = "wasm32")] pub trait MaybeSendSync {} +#[cfg(not(target_arch = "wasm32"))] +impl MaybeSendSync for T {} #[cfg(target_arch = "wasm32")] impl MaybeSendSync for T {} @@ -580,9 +580,9 @@ impl Surface { .map_err(|err| Box::new(err) as Box)?; #[cfg(not(target_os = "windows"))] - let shell_pid = *pty.child.pid.clone() as u32; + let shell_pid = pty.child.pid as u32; #[cfg(not(target_os = "windows"))] - let main_fd = *pty.child.id; + let main_fd = pty.child.id; let machine = Machine::new( Arc::clone(&terminal), @@ -1100,11 +1100,16 @@ impl Surface { /// integration. #[cfg(feature = "pty")] pub fn foreground_process_name(&self) -> String { - #[cfg(target_os = "windows")] - return String::new(); - #[cfg(not(target_os = "windows"))] - teletypewriter::foreground_process_name(self.main_fd, self.shell_pid) + { + #[cfg(target_os = "windows")] + return String::new(); + + #[cfg(not(target_os = "windows"))] + teletypewriter::foreground_process_name(self.main_fd, self.shell_pid) + } + #[cfg(target_os = "windows")] + String::new() } /// Inject bytes into the terminal's DISPLAY (the VT parser), as if they diff --git a/teletypewriter/src/unix/mod.rs b/teletypewriter/src/unix/mod.rs index 41a47e3713..aec1ad4471 100644 --- a/teletypewriter/src/unix/mod.rs +++ b/teletypewriter/src/unix/mod.rs @@ -21,10 +21,9 @@ use std::ops::Deref; use std::os::fd::OwnedFd; use std::os::fd::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::process::CommandExt; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::ptr; -use std::sync::Arc; #[cfg(all(target_os = "linux", not(target_env = "musl")))] const TIOCSWINSZ: libc::c_ulong = 0x5414; @@ -679,10 +678,10 @@ pub fn create_pty_with_spawn( let ptsname: String = tty_ptsname(main).unwrap_or_else(|_| "".to_string()); let child_unix = Child { - id: Arc::new(main), + id: main, ptsname, - pid: Arc::new(child_process.id().try_into().unwrap()), - process: Some(child_process), + pid: child_process.id().try_into().unwrap(), + exited: false, }; Ok(Pty { @@ -765,10 +764,10 @@ pub fn create_pty_with_fork( // In the future add an option to check before release the method let ptsname: String = tty_ptsname(main).unwrap_or_else(|_| "".to_string()); let child = Child { - id: Arc::new(main), + id: main, ptsname, - pid: Arc::new(id), - process: None, + pid: id, + exited: false, }; unsafe { @@ -819,12 +818,11 @@ unsafe fn set_nonblocking(fd: libc::c_int) { #[derive(Debug)] pub struct Child { - pub id: Arc, - pub pid: Arc, + pub id: libc::c_int, + pub pid: libc::pid_t, #[allow(dead_code)] ptsname: String, - #[allow(dead_code)] - process: Option, + exited: bool, } impl Child { @@ -855,11 +853,11 @@ impl Child { /// Return the child’s exit status if it has already exited. If the child is still running, return Ok(None). /// https://linux.die.net/man/2/waitpid - pub fn waitpid(&self) -> Result, String> { + pub fn waitpid(&mut self) -> Result, String> { let mut status = 0 as libc::c_int; // If WNOHANG was specified in options and there were no children in a waitable state, then waitid() returns 0 immediately and the state of the siginfo_t structure pointed to by infop is unspecified. To distinguish this case from that where a child was in a waitable state, zero out the si_pid field before the call and check for a nonzero value in this field after the call returns. let res = - unsafe { waitpid(*self.pid, &mut status as *mut libc::c_int, libc::WNOHANG) }; + unsafe { waitpid(self.pid, &mut status as *mut libc::c_int, libc::WNOHANG) }; if res <= -1 { return Err(String::from("error")); } @@ -868,6 +866,7 @@ impl Child { return Ok(None); } + self.exited = true; Ok(Some(status)) } } @@ -887,8 +886,10 @@ impl Deref for Child { impl Drop for Child { fn drop(&mut self) { - unsafe { - libc::kill(*self.pid, libc::SIGHUP); + if !self.exited { + unsafe { + libc::kill(self.pid, libc::SIGHUP); + } } } } @@ -1051,12 +1052,7 @@ pub fn foreground_process_path( } /// Start a new process in the background. -pub fn spawn_daemon( - program: &str, - args: I, - main_fd: RawFd, - shell_pid: u32, -) -> io::Result<()> +pub fn spawn_daemon(program: &str, args: I, cwd: Option<&Path>) -> io::Result<()> where I: IntoIterator + Copy, S: AsRef, @@ -1067,7 +1063,7 @@ where .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()); - if let Ok(cwd) = foreground_process_path(main_fd, shell_pid) { + if let Some(cwd) = cwd { command.current_dir(cwd); } unsafe { diff --git a/teletypewriter/src/windows/mod.rs b/teletypewriter/src/windows/mod.rs index 0bd7475e2e..72612fa4bf 100644 --- a/teletypewriter/src/windows/mod.rs +++ b/teletypewriter/src/windows/mod.rs @@ -8,6 +8,7 @@ use std::io::{self}; use std::iter::once; use std::os::windows::ffi::OsStrExt; use std::os::windows::process::CommandExt; +use std::path::Path; use std::process::{Command, Stdio}; use std::sync::mpsc::TryRecvError; @@ -254,16 +255,21 @@ pub fn win32_string + ?Sized>(value: &S) -> Vec { OsStr::new(value).encode_wide().chain(once(0)).collect() } -pub fn spawn_daemon(program: &str, args: I) -> io::Result<()> +pub fn spawn_daemon(program: &str, args: I, cwd: Option<&Path>) -> io::Result<()> where I: IntoIterator + Copy, S: AsRef, { - Command::new(program) + let mut command = Command::new(program); + command .args(args) .stdin(Stdio::null()) .stdout(Stdio::null()) - .stderr(Stdio::null()) + .stderr(Stdio::null()); + if let Some(cwd) = cwd { + command.current_dir(cwd); + } + command .creation_flags(CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW) .spawn() .map(|_| ())