Merge pull request #593 from a-kenji/modularize-spawn-terminal

Modularize spawn_terminal_function

zellij-server/src/os_input_output.rs

@@ -18,6 +18,7 @@ use signal_hook::consts::*;
 use zellij_tile::data::Palette;
 use zellij_utils::{
     errors::ErrorContext,
+    input::command::{RunCommand, TerminalAction},
     ipc::{
         ClientToServerMsg, ExitReason, IpcReceiverWithContext, IpcSenderWithContext,
         ServerToClientMsg,
@@ -83,18 +84,7 @@ fn handle_command_exit(mut child: Child) {
 /// Spawns a new terminal from the parent terminal with [`termios`](termios::Termios)
 /// `orig_termios`.
 ///
-/// 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.
-///
-/// # 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(file_to_open: Option<PathBuf>, orig_termios: termios::Termios) -> (RawFd, Pid) {
+fn handle_terminal(cmd: RunCommand, orig_termios: termios::Termios) -> (RawFd, Pid) {
     let (pid_primary, pid_secondary): (RawFd, Pid) = {
         match forkpty(None, Some(&orig_termios)) {
             Ok(fork_pty_res) => {
@@ -104,29 +94,14 @@ fn spawn_terminal(file_to_open: Option<PathBuf>, orig_termios: termios::Termios)
                         // fcntl(pid_primary, FcntlArg::F_SETFL(OFlag::empty())).expect("could not fcntl");
                         child
                     }
-                    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 child = Command::new(editor)
-                                .args(&[file_to_open])
-                                .spawn()
-                                .expect("failed to spawn");
-                            handle_command_exit(child);
-                            ::std::process::exit(0);
-                        }
-                        None => {
-                            let child = Command::new(env::var("SHELL").unwrap())
-                                .spawn()
-                                .expect("failed to spawn");
-                            handle_command_exit(child);
-                            ::std::process::exit(0);
-                        }
-                    },
+                    ForkResult::Child => {
+                        let child = Command::new(cmd.command)
+                            .args(&cmd.args)
+                            .spawn()
+                            .expect("failed to spawn");
+                        handle_command_exit(child);
+                        ::std::process::exit(0);
+                    }
                 };
                 (pid_primary, pid_secondary)
             }
@@ -138,6 +113,48 @@ fn spawn_terminal(file_to_open: Option<PathBuf>, orig_termios: termios::Termios)
     (pid_primary, pid_secondary)
 }
 
+/// If a [`TerminalAction::OpenFile(file)`] 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 [`TerminalAction::RunCommand(RunCommand)`] is given, the command will be started
+/// in the new terminal.
+/// If None is given, the shell specified by environment variable `SHELL` will
+/// be started in the new terminal.
+///
+/// # Panics
+///
+/// This function will panic if both the `EDITOR` and `VISUAL` environment variables are not
+/// set.
+pub fn spawn_terminal(
+    terminal_action: Option<TerminalAction>,
+    orig_termios: termios::Termios,
+) -> (RawFd, Pid) {
+    let cmd = match terminal_action {
+        Some(TerminalAction::OpenFile(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 command =
+                PathBuf::from(env::var("EDITOR").unwrap_or_else(|_| env::var("VISUAL").unwrap()));
+
+            let args = vec![file_to_open
+                .into_os_string()
+                .into_string()
+                .expect("Not valid Utf8 Encoding")];
+            RunCommand { command, args }
+        }
+        Some(TerminalAction::RunCommand(command)) => command,
+        None => {
+            let command =
+                PathBuf::from(env::var("SHELL").expect("Could not find the SHELL variable"));
+            let args = vec![];
+            RunCommand { command, args }
+        }
+    };
+
+    handle_terminal(cmd, orig_termios)
+}
+
 #[derive(Clone)]
 pub struct ServerOsInputOutput {
     orig_termios: Arc<Mutex<termios::Termios>>,
@@ -178,8 +195,8 @@ impl AsyncReader for RawFdAsyncReader {
 pub trait ServerOsApi: Send + Sync {
     /// Sets the size of the terminal associated to file descriptor `fd`.
     fn set_terminal_size_using_fd(&self, fd: RawFd, cols: u16, rows: u16);
-    /// Spawn a new terminal, with an optional file to open in a terminal program.
-    fn spawn_terminal(&self, file_to_open: Option<PathBuf>) -> (RawFd, Pid);
+    /// Spawn a new terminal, with a terminal action.
+    fn spawn_terminal(&self, terminal_action: Option<TerminalAction>) -> (RawFd, Pid);
     /// Read bytes from the standard output of the virtual terminal referred to by `fd`.
     fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result<usize, nix::Error>;
     /// Creates an `AsyncReader` that can be used to read from `fd` in an async context
@@ -215,9 +232,9 @@ impl ServerOsApi for ServerOsInputOutput {
     fn set_terminal_size_using_fd(&self, fd: RawFd, cols: u16, rows: u16) {
         set_terminal_size_using_fd(fd, cols, rows);
     }
-    fn spawn_terminal(&self, file_to_open: Option<PathBuf>) -> (RawFd, Pid) {
+    fn spawn_terminal(&self, terminal_action: Option<TerminalAction>) -> (RawFd, Pid) {
         let orig_termios = self.orig_termios.lock().unwrap();
-        spawn_terminal(file_to_open, orig_termios.clone())
+        spawn_terminal(terminal_action, orig_termios.clone())
     }
     fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result<usize, nix::Error> {
         unistd::read(fd, buf)

zellij-server/src/pty.rs

@@ -4,7 +4,6 @@ use async_std::future::timeout as async_timeout;
 use async_std::task::{self, JoinHandle};
 use std::collections::HashMap;
 use std::os::unix::io::RawFd;
-use std::path::PathBuf;
 use std::time::{Duration, Instant};
 
 use crate::{
@@ -17,7 +16,7 @@ use crate::{
 };
 use zellij_utils::{
     errors::{get_current_ctx, ContextType, PtyContext},
-    input::layout::Layout,
+    input::{command::TerminalAction, layout::Layout},
     logging::debug_to_file,
 };
 
@@ -26,9 +25,9 @@ pub type VteBytes = Vec<u8>;
 /// Instructions related to PTYs (pseudoterminals).
 #[derive(Clone, Debug)]
 pub(crate) enum PtyInstruction {
-    SpawnTerminal(Option<PathBuf>),
-    SpawnTerminalVertically(Option<PathBuf>),
-    SpawnTerminalHorizontally(Option<PathBuf>),
+    SpawnTerminal(Option<TerminalAction>),
+    SpawnTerminalVertically(Option<TerminalAction>),
+    SpawnTerminalHorizontally(Option<TerminalAction>),
     NewTab,
     ClosePane(PaneId),
     CloseTab(Vec<PaneId>),
@@ -61,22 +60,22 @@ pub(crate) fn pty_thread_main(mut pty: Pty, maybe_layout: Option<Layout>) {
         let (event, mut err_ctx) = pty.bus.recv().expect("failed to receive event on channel");
         err_ctx.add_call(ContextType::Pty((&event).into()));
         match event {
-            PtyInstruction::SpawnTerminal(file_to_open) => {
-                let pid = pty.spawn_terminal(file_to_open);
+            PtyInstruction::SpawnTerminal(terminal_action) => {
+                let pid = pty.spawn_terminal(terminal_action);
                 pty.bus
                     .senders
                     .send_to_screen(ScreenInstruction::NewPane(PaneId::Terminal(pid)))
                     .unwrap();
             }
-            PtyInstruction::SpawnTerminalVertically(file_to_open) => {
-                let pid = pty.spawn_terminal(file_to_open);
+            PtyInstruction::SpawnTerminalVertically(terminal_action) => {
+                let pid = pty.spawn_terminal(terminal_action);
                 pty.bus
                     .senders
                     .send_to_screen(ScreenInstruction::VerticalSplit(PaneId::Terminal(pid)))
                     .unwrap();
             }
-            PtyInstruction::SpawnTerminalHorizontally(file_to_open) => {
-                let pid = pty.spawn_terminal(file_to_open);
+            PtyInstruction::SpawnTerminalHorizontally(terminal_action) => {
+                let pid = pty.spawn_terminal(terminal_action);
                 pty.bus
                     .senders
                     .send_to_screen(ScreenInstruction::HorizontalSplit(PaneId::Terminal(pid)))
@@ -218,13 +217,13 @@ impl Pty {
             task_handles: HashMap::new(),
         }
     }
-    pub fn spawn_terminal(&mut self, file_to_open: Option<PathBuf>) -> RawFd {
+    pub fn spawn_terminal(&mut self, terminal_action: Option<TerminalAction>) -> RawFd {
         let (pid_primary, pid_secondary): (RawFd, Pid) = self
             .bus
             .os_input
             .as_mut()
             .unwrap()
-            .spawn_terminal(file_to_open);
+            .spawn_terminal(terminal_action);
         let task_handle = stream_terminal_bytes(
             pid_primary,
             self.bus.senders.clone(),

zellij-server/src/unit/screen_tests.rs

@@ -6,10 +6,9 @@ use crate::{
     SessionState,
 };
 use std::sync::{Arc, RwLock};
-use zellij_utils::pane_size::PositionAndSize;
+use zellij_utils::{input::command::TerminalAction, pane_size::PositionAndSize};
 
 use std::os::unix::io::RawFd;
-use std::path::PathBuf;
 
 use zellij_utils::ipc::ClientAttributes;
 use zellij_utils::nix;
@@ -27,7 +26,7 @@ impl ServerOsApi for FakeInputOutput {
     fn set_terminal_size_using_fd(&self, _fd: RawFd, _cols: u16, _rows: u16) {
         // noop
     }
-    fn spawn_terminal(&self, _file_to_open: Option<PathBuf>) -> (RawFd, Pid) {
+    fn spawn_terminal(&self, _file_to_open: Option<TerminalAction>) -> (RawFd, Pid) {
         unimplemented!()
     }
     fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result<usize, nix::Error> {

zellij-server/src/unit/tab_tests.rs

@@ -10,12 +10,12 @@ use std::sync::{Arc, RwLock};
 use zellij_utils::pane_size::PositionAndSize;
 
 use std::os::unix::io::RawFd;
-use std::path::PathBuf;
 
 use zellij_utils::nix;
 
 use zellij_utils::{
     errors::ErrorContext,
+    input::command::TerminalAction,
     interprocess::local_socket::LocalSocketStream,
     ipc::{ClientToServerMsg, ServerToClientMsg},
 };
@@ -26,7 +26,7 @@ impl ServerOsApi for FakeInputOutput {
     fn set_terminal_size_using_fd(&self, _fd: RawFd, _cols: u16, _rows: u16) {
         // noop
     }
-    fn spawn_terminal(&self, _file_to_open: Option<PathBuf>) -> (RawFd, Pid) {
+    fn spawn_terminal(&self, _file_to_open: Option<TerminalAction>) -> (RawFd, Pid) {
         unimplemented!()
     }
     fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result<usize, nix::Error> {

zellij-server/src/wasm_vm.rs

@@ -7,8 +7,6 @@ use std::sync::{mpsc::Sender, Arc, Mutex};
 use std::thread;
 use std::time::{Duration, Instant};
 
-use zellij_utils::{serde, zellij_tile};
-
 use serde::{de::DeserializeOwned, Serialize};
 use wasmer::{
     imports, ChainableNamedResolver, Function, ImportObject, Instance, Module, Store, Value,
@@ -24,6 +22,7 @@ use crate::{
     thread_bus::{Bus, ThreadSenders},
 };
 use zellij_utils::errors::{ContextType, PluginContext};
+use zellij_utils::{input::command::TerminalAction, serde, zellij_tile};
 
 #[derive(Clone, Debug)]
 pub(crate) enum PluginInstruction {
@@ -235,7 +234,9 @@ fn host_open_file(plugin_env: &PluginEnv) {
     let path: PathBuf = wasi_read_object(&plugin_env.wasi_env);
     plugin_env
         .senders
-        .send_to_pty(PtyInstruction::SpawnTerminal(Some(path)))
+        .send_to_pty(PtyInstruction::SpawnTerminal(Some(
+            TerminalAction::OpenFile(path),
+        )))
         .unwrap();
 }
 

zellij-utils/src/input/command.rs

@@ -0,0 +1,16 @@
+//! Trigger a command
+use serde::{Deserialize, Serialize};
+use std::path::PathBuf;
+
+#[derive(Debug, Clone)]
+pub enum TerminalAction {
+    OpenFile(PathBuf),
+    RunCommand(RunCommand),
+}
+
+#[derive(Clone, Debug, Deserialize, Default, Serialize, PartialEq)]
+pub struct RunCommand {
+    pub command: PathBuf,
+    #[serde(default)]
+    pub args: Vec<String>,
+}

zellij-utils/src/input/mod.rs

@@ -1,6 +1,7 @@
 //! The way terminal input is handled.
 
 pub mod actions;
+pub mod command;
 pub mod config;
 pub mod keybinds;
 pub mod layout;