From 2581dc85ee42d9dc4eda492a61e963237763c7d4 Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Sat, 13 Feb 2021 19:06:41 +0100 Subject: [PATCH 1/8] feat(cwd-pane): Add a new trait to get the cwd of a given pid This is target depend, on linux we rely on the std lib while on mac os we use the darwin_libproc crate. --- Cargo.lock | 21 +++++++++++++++++++++ Cargo.toml | 3 +++ src/common/os_input_output.rs | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index e55d9c4389..f7453c4218 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -496,6 +496,26 @@ dependencies = [ "syn", ] +[[package]] +name = "darwin-libproc" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc629b7cf42586fee31dae31f9ab73fa5ff5f0170016aa61be5fcbc12a90c516" +dependencies = [ + "darwin-libproc-sys", + "libc", + "memchr", +] + +[[package]] +name = "darwin-libproc-sys" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef0aa083b94c54aa4cfd9bbfd37856714c139d1dc511af80270558c7ba3b4816" +dependencies = [ + "libc", +] + [[package]] name = "directories-next" version = "2.0.0" @@ -2200,6 +2220,7 @@ dependencies = [ "async-std", "backtrace", "bincode", + "darwin-libproc", "directories-next", "futures", "insta", diff --git a/Cargo.toml b/Cargo.toml index 30c727b296..558fde8a8c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,9 @@ wasmer-wasi = "1.0.0" interprocess = "1.0.1" zellij-tile = { path = "zellij-tile/", version = "0.6.0" } +[target.'cfg(target_os = "macos")'.dependencies] +darwin-libproc = "0.2.0" + [dependencies.async-std] version = "1.3.0" features = ["unstable"] diff --git a/src/common/os_input_output.rs b/src/common/os_input_output.rs index 35d77fddf2..9b94b6fe5b 100644 --- a/src/common/os_input_output.rs +++ b/src/common/os_input_output.rs @@ -1,4 +1,8 @@ use crate::panes::PositionAndSize; + +#[cfg(target_os = "macos")] +use darwin_libproc; + use nix::fcntl::{fcntl, FcntlArg, OFlag}; use nix::pty::{forkpty, Winsize}; use nix::sys::signal::{kill, Signal}; @@ -13,6 +17,9 @@ use std::path::PathBuf; use std::process::{Child, Command}; use std::sync::{Arc, Mutex}; +#[cfg(target_os = "linux")] +use std::fs; + use signal_hook::{consts::signal::*, iterator::Signals}; use std::env; @@ -191,6 +198,8 @@ pub trait OsApi: Send + Sync { /// Returns a [`Box`] pointer to this [`OsApi`] struct. fn box_clone(&self) -> Box; fn receive_sigwinch(&self, cb: Box); + /// Returns the current working directory for a given pid + fn get_cwd(&self, pid: RawFd) -> Option; } impl OsApi for OsInputOutput { @@ -261,6 +270,22 @@ impl OsApi for OsInputOutput { } } } + + #[cfg(target_os = "macos")] + fn get_cwd(&self, pid: RawFd) -> Option { + match darwin_libproc::pid_cwd(pid) { + Ok(cwd) => Some(cwd), + Err(_) => None, + } + } + + #[cfg(target_os = "linux")] + fn get_cwd(&self, pid: RawFd) -> Option { + match fs::read_link(format!("/proc/{}/cwd", pid)) { + Ok(cwd) => Some(cwd), + Err(_) => None, + } + } } impl Clone for Box { From 8f419e7500925da0bd0a9213cb68b2fe39600cee Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Mon, 15 Feb 2021 18:37:58 +0100 Subject: [PATCH 2/8] feat(cwd-pane): Allow to set a working directory when spawning a terminal --- src/common/os_input_output.rs | 31 +++++++++++++++++++++++++++---- src/common/pty_bus.rs | 5 +++-- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/common/os_input_output.rs b/src/common/os_input_output.rs index 9b94b6fe5b..db6a2784e4 100644 --- a/src/common/os_input_output.rs +++ b/src/common/os_input_output.rs @@ -114,7 +114,20 @@ fn handle_command_exit(mut child: Child) { /// set. // FIXME this should probably be split into different functions, or at least have less levels // of indentation in some way -fn spawn_terminal(file_to_open: Option, orig_termios: termios::Termios) -> (RawFd, RawFd) { +fn spawn_terminal( + file_to_open: Option, + orig_termios: termios::Termios, + working_dir: Option, +) -> (RawFd, RawFd) { + // Choose the working directory to use + let dir = match working_dir { + Some(dir) => dir, + None => match env::current_dir() { + Ok(env_dir) => env_dir, + Err(_) => PathBuf::from(env::var("HOME").unwrap()), + }, + }; + let (pid_primary, pid_secondary): (RawFd, RawFd) = { match forkpty(None, Some(&orig_termios)) { Ok(fork_pty_res) => { @@ -136,6 +149,7 @@ fn spawn_terminal(file_to_open: Option, orig_termios: termios::Termios) let child = Command::new(editor) .args(&[file_to_open]) + .current_dir(dir) .spawn() .expect("failed to spawn"); handle_command_exit(child); @@ -143,6 +157,7 @@ fn spawn_terminal(file_to_open: Option, orig_termios: termios::Termios) } None => { let child = Command::new(env::var("SHELL").unwrap()) + .current_dir(dir) .spawn() .expect("failed to spawn"); handle_command_exit(child); @@ -179,7 +194,11 @@ pub trait OsApi: Send + Sync { /// [cooked mode](https://en.wikipedia.org/wiki/Terminal_mode). fn unset_raw_mode(&mut self, fd: RawFd); /// Spawn a new terminal, with an optional file to open in a terminal program. - fn spawn_terminal(&mut self, file_to_open: Option) -> (RawFd, RawFd); + fn spawn_terminal( + &mut self, + file_to_open: Option, + working_dir: Option, + ) -> (RawFd, RawFd); /// Read bytes from the standard output of the virtual terminal referred to by `fd`. fn read_from_tty_stdout(&mut self, fd: RawFd, buf: &mut [u8]) -> Result; /// Write bytes to the standard input of the virtual terminal referred to by `fd`. @@ -216,9 +235,13 @@ impl OsApi for OsInputOutput { let orig_termios = self.orig_termios.lock().unwrap(); unset_raw_mode(fd, orig_termios.clone()); } - fn spawn_terminal(&mut self, file_to_open: Option) -> (RawFd, RawFd) { + fn spawn_terminal( + &mut self, + file_to_open: Option, + working_dir: Option, + ) -> (RawFd, RawFd) { let orig_termios = self.orig_termios.lock().unwrap(); - spawn_terminal(file_to_open, orig_termios.clone()) + spawn_terminal(file_to_open, orig_termios.clone(), working_dir) } fn read_from_tty_stdout(&mut self, fd: RawFd, buf: &mut [u8]) -> Result { unistd::read(fd, buf) diff --git a/src/common/pty_bus.rs b/src/common/pty_bus.rs index 2c13222823..4f59836234 100644 --- a/src/common/pty_bus.rs +++ b/src/common/pty_bus.rs @@ -176,7 +176,7 @@ impl PtyBus { } pub fn spawn_terminal(&mut self, file_to_open: Option) -> RawFd { let (pid_primary, pid_secondary): (RawFd, RawFd) = - self.os_input.spawn_terminal(file_to_open); + self.os_input.spawn_terminal(file_to_open, None); let task_handle = stream_terminal_bytes( pid_primary, self.send_screen_instructions.clone(), @@ -191,7 +191,8 @@ impl PtyBus { let total_panes = layout.total_terminal_panes(); let mut new_pane_pids = vec![]; for _ in 0..total_panes { - let (pid_primary, pid_secondary): (RawFd, RawFd) = self.os_input.spawn_terminal(None); + let (pid_primary, pid_secondary): (RawFd, RawFd) = + self.os_input.spawn_terminal(None, None); self.id_to_child_pid.insert(pid_primary, pid_secondary); new_pane_pids.push(pid_primary); } From 2e0a5820a0ab3b3e710fc2f715d9c60217cc2dc9 Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Mon, 15 Feb 2021 19:02:55 +0100 Subject: [PATCH 3/8] feat(cwd-pane): Add an active_pane field to the PtyBus struct Allows to keep of who is the active pane. Update the active pane when spawning a terminal and expose a method to update the active pane. --- src/common/pty_bus.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/common/pty_bus.rs b/src/common/pty_bus.rs index 4f59836234..a0d4058369 100644 --- a/src/common/pty_bus.rs +++ b/src/common/pty_bus.rs @@ -82,6 +82,7 @@ pub struct PtyBus { pub send_plugin_instructions: SenderWithContext, pub receive_pty_instructions: Receiver<(PtyInstruction, ErrorContext)>, pub id_to_child_pid: HashMap, + pub active_pane: Option, os_input: Box, debug_to_file: bool, task_handles: HashMap>, @@ -170,6 +171,7 @@ impl PtyBus { receive_pty_instructions, os_input, id_to_child_pid: HashMap::new(), + active_pane: None, debug_to_file, task_handles: HashMap::new(), } @@ -185,6 +187,7 @@ impl PtyBus { ); self.task_handles.insert(pid_primary, task_handle); self.id_to_child_pid.insert(pid_primary, pid_secondary); + self.active_pane = Some(PaneId::Terminal(pid_primary)); pid_primary } pub fn spawn_terminals_for_layout(&mut self, layout: Layout) { @@ -233,6 +236,9 @@ impl PtyBus { self.close_pane(id); }); } + pub fn update_active_pane(&mut self, pane_id: PaneId) { + self.active_pane = Some(pane_id); + } } impl Drop for PtyBus { From 5d1286bae628341611a5542005521c51703541a7 Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Mon, 15 Feb 2021 19:38:54 +0100 Subject: [PATCH 4/8] feat(cwd-pane): Match Tab's active pane id with PtyBus --- src/client/tab.rs | 34 ++++++++++++++++++++++++++++++++++ src/common/errors.rs | 2 ++ src/common/mod.rs | 3 +++ src/common/pty_bus.rs | 5 +++-- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/client/tab.rs b/src/client/tab.rs index 8750589fc3..cfa36ec54d 100644 --- a/src/client/tab.rs +++ b/src/client/tab.rs @@ -326,6 +326,9 @@ impl Tab { .unwrap(); } self.active_terminal = self.panes.iter().map(|(id, _)| id.to_owned()).next(); + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); self.render(); } pub fn new_pane(&mut self, pid: PaneId) { @@ -424,6 +427,9 @@ impl Tab { self.active_terminal = Some(pid); self.render(); } + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); } pub fn horizontal_split(&mut self, pid: PaneId) { self.close_down_to_max_terminals(); @@ -1743,6 +1749,9 @@ impl Tab { } else { self.active_terminal = Some(*first_terminal); } + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); self.render(); } pub fn focus_next_pane(&mut self) { @@ -1829,6 +1838,9 @@ impl Tab { } else { self.active_terminal = Some(active_terminal.unwrap().pid()); } + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); self.render(); } pub fn move_focus_down(&mut self) { @@ -1859,6 +1871,9 @@ impl Tab { } else { self.active_terminal = Some(active_terminal.unwrap().pid()); } + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); self.render(); } pub fn move_focus_up(&mut self) { @@ -1889,6 +1904,9 @@ impl Tab { } else { self.active_terminal = Some(active_terminal.unwrap().pid()); } + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); self.render(); } pub fn move_focus_right(&mut self) { @@ -1919,6 +1937,9 @@ impl Tab { } else { self.active_terminal = Some(active_terminal.unwrap().pid()); } + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); self.render(); } fn horizontal_borders(&self, terminals: &[PaneId]) -> HashSet { @@ -2086,6 +2107,9 @@ impl Tab { self.panes.remove(&id); if self.active_terminal == Some(id) { self.active_terminal = self.next_active_pane(panes); + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); } return; } @@ -2102,6 +2126,9 @@ impl Tab { self.panes.remove(&id); if self.active_terminal == Some(id) { self.active_terminal = self.next_active_pane(panes); + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); } return; } @@ -2118,6 +2145,9 @@ impl Tab { self.panes.remove(&id); if self.active_terminal == Some(id) { self.active_terminal = self.next_active_pane(panes); + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); } return; } @@ -2134,10 +2164,14 @@ impl Tab { self.panes.remove(&id); if self.active_terminal == Some(id) { self.active_terminal = self.next_active_pane(panes); + self.send_pty_instructions + .send(PtyInstruction::UpdateActivePane(self.active_terminal)) + .unwrap(); } return; } } + // if we reached here, this is either the last pane or there's some sort of // configuration error (eg. we're trying to close a pane surrounded by fixed panes) self.panes.remove(&id); diff --git a/src/common/errors.rs b/src/common/errors.rs index d0b748ab2a..ba1a095d62 100644 --- a/src/common/errors.rs +++ b/src/common/errors.rs @@ -267,6 +267,7 @@ pub enum PtyContext { SpawnTerminalVertically, SpawnTerminalHorizontally, NewTab, + UpdateActivePane, ClosePane, CloseTab, Quit, @@ -281,6 +282,7 @@ impl From<&PtyInstruction> for PtyContext { PtyInstruction::ClosePane(_) => PtyContext::ClosePane, PtyInstruction::CloseTab(_) => PtyContext::CloseTab, PtyInstruction::NewTab => PtyContext::NewTab, + PtyInstruction::UpdateActivePane(_) => PtyContext::UpdateActivePane, PtyInstruction::Quit => PtyContext::Quit, } } diff --git a/src/common/mod.rs b/src/common/mod.rs index 860d12db14..6b4cc2f35b 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -238,6 +238,9 @@ pub fn start(mut os_input: Box, opts: CliArgs) { .unwrap(); } } + PtyInstruction::UpdateActivePane(id) => { + pty_bus.update_active_pane(id); + } PtyInstruction::ClosePane(id) => { pty_bus.close_pane(id); command_is_executing.done_closing_pane(); diff --git a/src/common/pty_bus.rs b/src/common/pty_bus.rs index a0d4058369..4ecc177578 100644 --- a/src/common/pty_bus.rs +++ b/src/common/pty_bus.rs @@ -72,6 +72,7 @@ pub enum PtyInstruction { SpawnTerminalVertically(Option), SpawnTerminalHorizontally(Option), NewTab, + UpdateActivePane(Option), ClosePane(PaneId), CloseTab(Vec), Quit, @@ -236,8 +237,8 @@ impl PtyBus { self.close_pane(id); }); } - pub fn update_active_pane(&mut self, pane_id: PaneId) { - self.active_pane = Some(pane_id); + pub fn update_active_pane(&mut self, pane_id: Option) { + self.active_pane = pane_id; } } From 21e35ede24dd20c38f8a13dbd779ec84a0b5ba2d Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Fri, 9 Apr 2021 17:57:18 +0200 Subject: [PATCH 5/8] feat(cwd-pane): Open a new pane in the current working directory Allow to open a new pane in the active pane's current directory. - Change the default spawn terminal behavior - os_input_ouput: use a pipe to get the shell pid form child to parrent --- Cargo.lock | 1 + Cargo.toml | 1 + src/common/os_input_output.rs | 43 +++++++++++++++++++++++++++-------- src/common/pty_bus.rs | 36 +++++++++++++++++++++++++---- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7453c4218..a29af4f1eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2220,6 +2220,7 @@ dependencies = [ "async-std", "backtrace", "bincode", + "byteorder", "darwin-libproc", "directories-next", "futures", diff --git a/Cargo.toml b/Cargo.toml index 558fde8a8c..19498745c2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ repository = "https://github.com/zellij-org/zellij" ansi_term = "0.12.1" backtrace = "0.3.55" bincode = "1.3.1" +byteorder = "1.4.3" directories-next = "2.0" futures = "0.3.5" libc = "0.2" diff --git a/src/common/os_input_output.rs b/src/common/os_input_output.rs index db6a2784e4..2eccf35fec 100644 --- a/src/common/os_input_output.rs +++ b/src/common/os_input_output.rs @@ -3,6 +3,7 @@ use crate::panes::PositionAndSize; #[cfg(target_os = "macos")] use darwin_libproc; +use byteorder::{BigEndian, ByteOrder}; use nix::fcntl::{fcntl, FcntlArg, OFlag}; use nix::pty::{forkpty, Winsize}; use nix::sys::signal::{kill, Signal}; @@ -118,7 +119,7 @@ fn spawn_terminal( file_to_open: Option, orig_termios: termios::Termios, working_dir: Option, -) -> (RawFd, RawFd) { +) -> (RawFd, RawFd, RawFd) { // Choose the working directory to use let dir = match working_dir { Some(dir) => dir, @@ -128,16 +129,28 @@ fn spawn_terminal( }, }; - let (pid_primary, pid_secondary): (RawFd, RawFd) = { + let (parent_fd, child_fd) = unistd::pipe().expect("failed to create pipe"); + + let (pid_primary, pid_secondary, pid_tertiary): (RawFd, RawFd, RawFd) = { match forkpty(None, Some(&orig_termios)) { Ok(fork_pty_res) => { let pid_primary = fork_pty_res.master; - let pid_secondary = match fork_pty_res.fork_result { + let (pid_secondary, pid_tertiary) = match fork_pty_res.fork_result { ForkResult::Parent { child } => { - // fcntl(pid_primary, FcntlArg::F_SETFL(OFlag::empty())).expect("could not fcntl"); fcntl(pid_primary, FcntlArg::F_SETFL(OFlag::O_NONBLOCK)) .expect("could not fcntl"); - child + + let mut buffer = [0; 4]; + unistd::close(child_fd).expect("couldn't close child_fd"); + match unistd::read(parent_fd, &mut buffer) { + Ok(_) => {} + Err(e) => { + panic!("Read operation failed: {:?}", e); + } + } + unistd::close(parent_fd).expect("couldn't close parent_fd"); + let pid: u32 = u32::from_be_bytes(buffer); + (child, pid) } ForkResult::Child => match file_to_open { Some(file_to_open) => { @@ -160,19 +173,31 @@ fn spawn_terminal( .current_dir(dir) .spawn() .expect("failed to spawn"); + + let mut buff = [0; 4]; + BigEndian::write_u32(&mut buff, child.id() as u32); + unistd::close(parent_fd).expect("couldn't close parent_fd"); + match unistd::write(child_fd, &buff) { + Ok(_) => {} + Err(e) => { + panic!("Read operation failed: {:?}", e); + } + } + unistd::close(child_fd).expect("couldn't close child_fd"); + handle_command_exit(child); ::std::process::exit(0); } }, }; - (pid_primary, pid_secondary.as_raw()) + (pid_primary, pid_secondary.as_raw(), pid_tertiary as i32) } Err(e) => { panic!("failed to fork {:?}", e); } } }; - (pid_primary, pid_secondary) + (pid_primary, pid_secondary, pid_tertiary) } #[derive(Clone)] @@ -198,7 +223,7 @@ pub trait OsApi: Send + Sync { &mut self, file_to_open: Option, working_dir: Option, - ) -> (RawFd, RawFd); + ) -> (RawFd, RawFd, RawFd); /// Read bytes from the standard output of the virtual terminal referred to by `fd`. fn read_from_tty_stdout(&mut self, fd: RawFd, buf: &mut [u8]) -> Result; /// Write bytes to the standard input of the virtual terminal referred to by `fd`. @@ -239,7 +264,7 @@ impl OsApi for OsInputOutput { &mut self, file_to_open: Option, working_dir: Option, - ) -> (RawFd, RawFd) { + ) -> (RawFd, RawFd, RawFd) { let orig_termios = self.orig_termios.lock().unwrap(); spawn_terminal(file_to_open, orig_termios.clone(), working_dir) } diff --git a/src/common/pty_bus.rs b/src/common/pty_bus.rs index 4ecc177578..83622d8205 100644 --- a/src/common/pty_bus.rs +++ b/src/common/pty_bus.rs @@ -83,6 +83,7 @@ pub struct PtyBus { pub send_plugin_instructions: SenderWithContext, pub receive_pty_instructions: Receiver<(PtyInstruction, ErrorContext)>, pub id_to_child_pid: HashMap, + pub id_to_cwd_pid: HashMap, pub active_pane: Option, os_input: Box, debug_to_file: bool, @@ -172,14 +173,20 @@ impl PtyBus { receive_pty_instructions, os_input, id_to_child_pid: HashMap::new(), + id_to_cwd_pid: HashMap::new(), active_pane: None, debug_to_file, task_handles: HashMap::new(), } } - pub fn spawn_terminal(&mut self, file_to_open: Option) -> RawFd { - let (pid_primary, pid_secondary): (RawFd, RawFd) = - self.os_input.spawn_terminal(file_to_open, None); + fn terminal_spawner( + &mut self, + file_to_open: Option, + working_directory: Option, + ) -> RawFd { + let (pid_primary, pid_secondary, pid_cwd): (RawFd, RawFd, RawFd) = self + .os_input + .spawn_terminal(file_to_open, working_directory); let task_handle = stream_terminal_bytes( pid_primary, self.send_screen_instructions.clone(), @@ -188,16 +195,37 @@ impl PtyBus { ); self.task_handles.insert(pid_primary, task_handle); self.id_to_child_pid.insert(pid_primary, pid_secondary); + self.id_to_cwd_pid.insert(pid_primary, pid_cwd); self.active_pane = Some(PaneId::Terminal(pid_primary)); pid_primary } + pub fn spawn_terminal(&mut self, file_to_open: Option) -> RawFd { + // Get pid from the current active pane + let pid = match self.active_pane { + Some(active_pane) => match active_pane { + PaneId::Terminal(id) => Some(self.id_to_cwd_pid.get(&id).unwrap()), + PaneId::Plugin(pi) => Some(self.id_to_cwd_pid.get(&(pi as i32)).unwrap()), + }, + None => None, + }; + + // Get the current working directory from our pid + let working_directory: Option = match pid { + Some(pid) => self.os_input.get_cwd(*pid), + None => None, + }; + + self.terminal_spawner(file_to_open, working_directory) + } pub fn spawn_terminals_for_layout(&mut self, layout: Layout) { let total_panes = layout.total_terminal_panes(); let mut new_pane_pids = vec![]; for _ in 0..total_panes { - let (pid_primary, pid_secondary): (RawFd, RawFd) = + let (pid_primary, pid_secondary, pid_cwd): (RawFd, RawFd, RawFd) = self.os_input.spawn_terminal(None, None); self.id_to_child_pid.insert(pid_primary, pid_secondary); + self.id_to_cwd_pid.insert(pid_primary, pid_cwd); + self.active_pane = Some(PaneId::Terminal(pid_primary)); new_pane_pids.push(pid_primary); } self.send_screen_instructions From de2532e72775b61c29c92523703b70d2bc4ebaad Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Sun, 18 Apr 2021 15:55:39 +0200 Subject: [PATCH 6/8] feat(cwd-pane): Refactor pty_bus pids storage --- src/common/pty_bus.rs | 63 ++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/src/common/pty_bus.rs b/src/common/pty_bus.rs index 83622d8205..7be32a3a92 100644 --- a/src/common/pty_bus.rs +++ b/src/common/pty_bus.rs @@ -78,12 +78,16 @@ pub enum PtyInstruction { Quit, } +pub struct ChildIds { + child: RawFd, + shell: RawFd, +} + pub struct PtyBus { pub send_screen_instructions: SenderWithContext, pub send_plugin_instructions: SenderWithContext, pub receive_pty_instructions: Receiver<(PtyInstruction, ErrorContext)>, - pub id_to_child_pid: HashMap, - pub id_to_cwd_pid: HashMap, + pub pids: HashMap, pub active_pane: Option, os_input: Box, debug_to_file: bool, @@ -172,8 +176,7 @@ impl PtyBus { send_plugin_instructions, receive_pty_instructions, os_input, - id_to_child_pid: HashMap::new(), - id_to_cwd_pid: HashMap::new(), + pids: HashMap::new(), active_pane: None, debug_to_file, task_handles: HashMap::new(), @@ -184,34 +187,45 @@ impl PtyBus { file_to_open: Option, working_directory: Option, ) -> RawFd { - let (pid_primary, pid_secondary, pid_cwd): (RawFd, RawFd, RawFd) = self + let (pid_parent, pid_child, pid_shell): (RawFd, RawFd, RawFd) = self .os_input .spawn_terminal(file_to_open, working_directory); let task_handle = stream_terminal_bytes( - pid_primary, + pid_parent, self.send_screen_instructions.clone(), self.os_input.clone(), self.debug_to_file, ); - self.task_handles.insert(pid_primary, task_handle); - self.id_to_child_pid.insert(pid_primary, pid_secondary); - self.id_to_cwd_pid.insert(pid_primary, pid_cwd); - self.active_pane = Some(PaneId::Terminal(pid_primary)); - pid_primary + self.task_handles.insert(pid_parent, task_handle); + self.pids.insert( + pid_parent, + ChildIds { + child: pid_child, + shell: pid_shell, + }, + ); + self.active_pane = Some(PaneId::Terminal(pid_parent)); + pid_parent } pub fn spawn_terminal(&mut self, file_to_open: Option) -> RawFd { // Get pid from the current active pane let pid = match self.active_pane { Some(active_pane) => match active_pane { - PaneId::Terminal(id) => Some(self.id_to_cwd_pid.get(&id).unwrap()), - PaneId::Plugin(pi) => Some(self.id_to_cwd_pid.get(&(pi as i32)).unwrap()), + PaneId::Terminal(id) => { + let pids = self.pids.get(&id).unwrap(); + Some(pids.shell) + } + PaneId::Plugin(pi) => { + let pids = self.pids.get(&(pi as i32)).unwrap(); + Some(pids.shell) + } }, None => None, }; // Get the current working directory from our pid let working_directory: Option = match pid { - Some(pid) => self.os_input.get_cwd(*pid), + Some(pid) => self.os_input.get_cwd(pid), None => None, }; @@ -221,12 +235,17 @@ impl PtyBus { let total_panes = layout.total_terminal_panes(); let mut new_pane_pids = vec![]; for _ in 0..total_panes { - let (pid_primary, pid_secondary, pid_cwd): (RawFd, RawFd, RawFd) = + let (pid_parent, pid_child, pid_shell): (RawFd, RawFd, RawFd) = self.os_input.spawn_terminal(None, None); - self.id_to_child_pid.insert(pid_primary, pid_secondary); - self.id_to_cwd_pid.insert(pid_primary, pid_cwd); - self.active_pane = Some(PaneId::Terminal(pid_primary)); - new_pane_pids.push(pid_primary); + self.pids.insert( + pid_parent, + ChildIds { + child: pid_child, + shell: pid_shell, + }, + ); + self.active_pane = Some(PaneId::Terminal(pid_parent)); + new_pane_pids.push(pid_parent); } self.send_screen_instructions .send(ScreenInstruction::ApplyLayout(( @@ -247,9 +266,9 @@ impl PtyBus { pub fn close_pane(&mut self, id: PaneId) { match id { PaneId::Terminal(id) => { - let child_pid = self.id_to_child_pid.remove(&id).unwrap(); + let child_pids = self.pids.remove(&id).unwrap(); let handle = self.task_handles.remove(&id).unwrap(); - self.os_input.kill(child_pid).unwrap(); + self.os_input.kill(child_pids.child).unwrap(); task::block_on(async { handle.cancel().await; }); @@ -272,7 +291,7 @@ impl PtyBus { impl Drop for PtyBus { fn drop(&mut self) { - let child_ids: Vec = self.id_to_child_pid.keys().copied().collect(); + let child_ids: Vec = self.pids.keys().copied().collect(); for id in child_ids { self.close_pane(PaneId::Terminal(id)); } From ec6c4fef9d96e36b373b210ffa395e3020a64cd4 Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Sun, 18 Apr 2021 16:08:29 +0200 Subject: [PATCH 7/8] feat(cwd-pane): Refactor spwan_terminal of os_input_output Discrease the level of indentation by spliting code in sub functions. Remove unnecessary working_dir match. Clarify pid names. --- src/common/os_input_output.rs | 185 ++++++++++++++++++++-------------- 1 file changed, 107 insertions(+), 78 deletions(-) diff --git a/src/common/os_input_output.rs b/src/common/os_input_output.rs index 2eccf35fec..11e70a2e30 100644 --- a/src/common/os_input_output.rs +++ b/src/common/os_input_output.rs @@ -5,7 +5,7 @@ use darwin_libproc; use byteorder::{BigEndian, ByteOrder}; use nix::fcntl::{fcntl, FcntlArg, OFlag}; -use nix::pty::{forkpty, Winsize}; +use nix::pty::{forkpty, ForkptyResult, Winsize}; use nix::sys::signal::{kill, Signal}; use nix::sys::termios; use nix::sys::wait::waitpid; @@ -102,102 +102,131 @@ fn handle_command_exit(mut child: Child) { } /// Spawns a new terminal from the parent terminal with [`termios`](termios::Termios) -/// `orig_termios`. +/// `orig_termios`. Let `handle_pty_fork` handle a successful fork, otherwire panic. +fn spawn_terminal( + file_to_open: Option, + orig_termios: termios::Termios, + working_dir: Option, +) -> (RawFd, RawFd, RawFd) { + // Create a pipe to allow the child the communicate the shell's pid to it's + // parent. + let (parent_fd, child_fd) = unistd::pipe().expect("failed to create pipe"); + + let (pid_parent, pid_child, pid_shell): (RawFd, RawFd, RawFd) = { + match forkpty(None, Some(&orig_termios)) { + Ok(fork_pty_res) => { + handle_pty_fork(fork_pty_res, file_to_open, working_dir, parent_fd, child_fd) + } + Err(e) => { + panic!("failed to fork {:?}", e); + } + } + }; + (pid_parent, pid_child, pid_shell) +} + +/// Handle a succefull pty fork. /// /// If a `file_to_open` is given, the text editor specified by environment variable `EDITOR` /// (or `VISUAL`, if `EDITOR` is not set) will be started in the new terminal, with the given /// file open. If no file is given, the shell specified by environment variable `SHELL` will /// be started in the new terminal. /// +/// If a `working_dir` is given, the shell will be started at `working_dir` +/// /// # Panics /// /// This function will panic if both the `EDITOR` and `VISUAL` environment variables are not /// set. -// FIXME this should probably be split into different functions, or at least have less levels -// of indentation in some way -fn spawn_terminal( +fn handle_pty_fork( + fork_pty_res: ForkptyResult, file_to_open: Option, - orig_termios: termios::Termios, working_dir: Option, + parent_fd: RawFd, + child_fd: RawFd, ) -> (RawFd, RawFd, RawFd) { - // Choose the working directory to use - let dir = match working_dir { - Some(dir) => dir, - None => match env::current_dir() { - Ok(env_dir) => env_dir, - Err(_) => PathBuf::from(env::var("HOME").unwrap()), - }, - }; - - let (parent_fd, child_fd) = unistd::pipe().expect("failed to create pipe"); + let pid_parent = fork_pty_res.master; + let (pid_child, pid_shell) = match fork_pty_res.fork_result { + ForkResult::Parent { child } => { + fcntl(pid_parent, FcntlArg::F_SETFL(OFlag::O_NONBLOCK)).expect("could not fcntl"); + let pid_shell: u32 = read_from_pipe(parent_fd, child_fd); + (child, pid_shell) + } + ForkResult::Child => match file_to_open { + Some(file_to_open) => { + if env::var("EDITOR").is_err() && env::var("VISUAL").is_err() { + panic!( + "Can't edit files if an editor is not defined. + To fix: define the EDITOR or VISUAL environment + variables with the path to your editor (eg. /usr/bin/vim)" + ); + } + let editor = env::var("EDITOR").unwrap_or_else(|_| env::var("VISUAL").unwrap()); - let (pid_primary, pid_secondary, pid_tertiary): (RawFd, RawFd, RawFd) = { - match forkpty(None, Some(&orig_termios)) { - Ok(fork_pty_res) => { - let pid_primary = fork_pty_res.master; - let (pid_secondary, pid_tertiary) = match fork_pty_res.fork_result { - ForkResult::Parent { child } => { - fcntl(pid_primary, FcntlArg::F_SETFL(OFlag::O_NONBLOCK)) - .expect("could not fcntl"); + let child = Command::new(editor) + .args(&[file_to_open]) + .spawn() + .expect("failed to spawn"); + handle_command_exit(child); + ::std::process::exit(0); + } + None => { + let child = match working_dir { + Some(working_dir) => Command::new(env::var("SHELL").unwrap()) + .current_dir(working_dir) + .spawn() + .expect("failed to spawn"), + None => Command::new(env::var("SHELL").unwrap()) + .spawn() + .expect("failed to spawn"), + }; - let mut buffer = [0; 4]; - unistd::close(child_fd).expect("couldn't close child_fd"); - match unistd::read(parent_fd, &mut buffer) { - Ok(_) => {} - Err(e) => { - panic!("Read operation failed: {:?}", e); - } - } - unistd::close(parent_fd).expect("couldn't close parent_fd"); - let pid: u32 = u32::from_be_bytes(buffer); - (child, pid) - } - ForkResult::Child => match file_to_open { - Some(file_to_open) => { - if env::var("EDITOR").is_err() && env::var("VISUAL").is_err() { - panic!("Can't edit files if an editor is not defined. To fix: define the EDITOR or VISUAL environment variables with the path to your editor (eg. /usr/bin/vim)"); - } - let editor = - env::var("EDITOR").unwrap_or_else(|_| env::var("VISUAL").unwrap()); + write_to_pipe(child.id() as u32, parent_fd, child_fd); - let child = Command::new(editor) - .args(&[file_to_open]) - .current_dir(dir) - .spawn() - .expect("failed to spawn"); - handle_command_exit(child); - ::std::process::exit(0); - } - None => { - let child = Command::new(env::var("SHELL").unwrap()) - .current_dir(dir) - .spawn() - .expect("failed to spawn"); + handle_command_exit(child); + ::std::process::exit(0); + } + }, + }; + (pid_parent, pid_child.as_raw(), pid_shell as i32) +} - let mut buff = [0; 4]; - BigEndian::write_u32(&mut buff, child.id() as u32); - unistd::close(parent_fd).expect("couldn't close parent_fd"); - match unistd::write(child_fd, &buff) { - Ok(_) => {} - Err(e) => { - panic!("Read operation failed: {:?}", e); - } - } - unistd::close(child_fd).expect("couldn't close child_fd"); +/// Read from a pipe given both file descriptors +/// +/// # Panics +/// +/// This function will panic if a close operation on one of the file descriptors fails or if the +/// read operation fails. +fn read_from_pipe(parent_fd: RawFd, child_fd: RawFd) -> u32 { + let mut buffer = [0; 4]; + unistd::close(child_fd).expect("Read: couldn't close child_fd"); + match unistd::read(parent_fd, &mut buffer) { + Ok(_) => {} + Err(e) => { + panic!("Read operation failed: {:?}", e); + } + } + unistd::close(parent_fd).expect("Read: couldn't close parent_fd"); + u32::from_be_bytes(buffer) +} - handle_command_exit(child); - ::std::process::exit(0); - } - }, - }; - (pid_primary, pid_secondary.as_raw(), pid_tertiary as i32) - } - Err(e) => { - panic!("failed to fork {:?}", e); - } +/// Write to a pipe given both file descriptors +/// +/// # Panics +/// +/// This function will panic if a close operation on one of the file descriptors fails or if the +/// write operation fails. +fn write_to_pipe(data: u32, parent_fd: RawFd, child_fd: RawFd) { + let mut buff = [0; 4]; + BigEndian::write_u32(&mut buff, data); + unistd::close(parent_fd).expect("Write: couldn't close parent_fd"); + match unistd::write(child_fd, &buff) { + Ok(_) => {} + Err(e) => { + panic!("Write operation failed: {:?}", e); } - }; - (pid_primary, pid_secondary, pid_tertiary) + } + unistd::close(child_fd).expect("Write: couldn't close child_fd"); } #[derive(Clone)] From a4206a6a7bd8d5f663f0361a191de0ad7a8bbe07 Mon Sep 17 00:00:00 2001 From: Quentin Rasmont Date: Sun, 18 Apr 2021 20:29:48 +0200 Subject: [PATCH 8/8] feat(cwd-pane): Fix broken tests --- src/tests/fakes.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/tests/fakes.rs b/src/tests/fakes.rs index c9504a9847..b9bb5873e3 100644 --- a/src/tests/fakes.rs +++ b/src/tests/fakes.rs @@ -153,10 +153,18 @@ impl OsApi for FakeInputOutput { .unwrap() .push(IoEvent::UnsetRawMode(pid)); } - fn spawn_terminal(&mut self, _file_to_open: Option) -> (RawFd, RawFd) { + fn spawn_terminal( + &mut self, + _file_to_open: Option, + _working_dir: Option, + ) -> (RawFd, RawFd, RawFd) { let next_terminal_id = self.stdin_writes.lock().unwrap().keys().len() as RawFd + 1; self.add_terminal(next_terminal_id); - (next_terminal_id as i32, next_terminal_id + 1000) // secondary number is arbitrary here + ( + next_terminal_id as i32, + next_terminal_id + 1000, + next_terminal_id + 2000, + ) // secondary number is arbitrary here } fn read_from_tty_stdout(&mut self, pid: RawFd, buf: &mut [u8]) -> Result { let mut read_buffers = self.read_buffers.lock().unwrap(); @@ -235,4 +243,8 @@ impl OsApi for FakeInputOutput { cb(); } } + fn get_cwd(&self, _pid: RawFd) -> Option { + let home = std::env::var("HOME").unwrap(); + Some(PathBuf::from(home)) + } }