more refactoring

2 files changed, 85 insertions, 66 deletions

diff --git a/src/pipeline/builtins/command.rs b/src/pipeline/builtins/command.rs
index fcf0bb8..83d39cb 100644
--- a/src/pipeline/builtins/command.rs
+++ b/src/pipeline/builtins/command.rs
@@ -3,7 +3,7 @@ use crate::pipeline::prelude::*;
 pub struct Command {
     exe: crate::parse::Exe,
     f: super::Builtin,
-    io: Io,
+    cfg: Cfg,
 }
 
 impl Command {
@@ -13,7 +13,7 @@ impl Command {
                 Ok(Self {
                     exe,
                     f,
-                    io: Io::new(),
+                    cfg: Cfg::new(),
                 })
             } else {
                 Err(exe)
@@ -24,15 +24,15 @@ impl Command {
     }
 
     pub fn stdin(&mut self, fh: std::fs::File) {
-        self.io.set_stdin(fh);
+        self.cfg.io.set_stdin(fh);
     }
 
     pub fn stdout(&mut self, fh: std::fs::File) {
-        self.io.set_stdout(fh);
+        self.cfg.io.set_stdout(fh);
     }
 
     pub fn stderr(&mut self, fh: std::fs::File) {
-        self.io.set_stderr(fh);
+        self.cfg.io.set_stderr(fh);
     }
 
     // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this
@@ -41,16 +41,55 @@ impl Command {
     where
         F: 'static + FnMut() -> std::io::Result<()> + Send + Sync,
     {
-        self.io.pre_exec(f);
+        self.cfg.pre_exec(f);
     }
 
     pub fn apply_redirects(&mut self, redirects: &[crate::parse::Redirect]) {
-        self.io.apply_redirects(redirects);
+        self.cfg.io.apply_redirects(redirects);
     }
 
     pub fn spawn(self, env: &Env) -> anyhow::Result<Child> {
-        let Self { f, exe, io } = self;
-        (f)(exe, env, io)
+        let Self { f, exe, cfg } = self;
+        (f)(exe, env, cfg)
+    }
+}
+
+pub struct Cfg {
+    io: Io,
+    pre_exec: Option<
+        Box<dyn 'static + FnMut() -> std::io::Result<()> + Send + Sync>,
+    >,
+}
+
+impl Cfg {
+    fn new() -> Self {
+        Self {
+            io: Io::new(),
+            pre_exec: None,
+        }
+    }
+
+    pub fn io(&self) -> &Io {
+        &self.io
+    }
+
+    // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this
+    // is just a wrapper)
+    pub unsafe fn pre_exec<F>(&mut self, f: F)
+    where
+        F: 'static + FnMut() -> std::io::Result<()> + Send + Sync,
+    {
+        self.pre_exec = Some(Box::new(f));
+    }
+
+    pub fn setup_command(mut self, cmd: &mut crate::pipeline::Command) {
+        self.io.setup_command(cmd);
+        if let Some(pre_exec) = self.pre_exec.take() {
+            // Safety: pre_exec can only have been set by calling the pre_exec
+            // method, which is itself unsafe, so the safety comments at the
+            // point where that is called are the relevant ones
+            unsafe { cmd.pre_exec(pre_exec) };
+        }
     }
 }
 
@@ -59,9 +98,6 @@ pub struct Io {
         std::os::unix::io::RawFd,
         crate::mutex::Mutex<File>,
     >,
-    pre_exec: Option<
-        Box<dyn 'static + FnMut() -> std::io::Result<()> + Send + Sync>,
-    >,
 }
 
 impl Io {
@@ -70,10 +106,7 @@ impl Io {
         fds.insert(0, crate::mutex::new(unsafe { File::input(0) }));
         fds.insert(1, crate::mutex::new(unsafe { File::output(1) }));
         fds.insert(2, crate::mutex::new(unsafe { File::output(2) }));
-        Self {
-            fds,
-            pre_exec: None,
-        }
+        Self { fds }
     }
 
     fn stdin(&self) -> Option<crate::mutex::Mutex<File>> {
@@ -127,15 +160,6 @@ impl Io {
         );
     }
 
-    // Safety: see pre_exec in async_std::os::unix::process::CommandExt (this
-    // is just a wrapper)
-    pub unsafe fn pre_exec<F>(&mut self, f: F)
-    where
-        F: 'static + FnMut() -> std::io::Result<()> + Send + Sync,
-    {
-        self.pre_exec = Some(Box::new(f));
-    }
-
     pub fn apply_redirects(&mut self, redirects: &[crate::parse::Redirect]) {
         for redirect in redirects {
             let to = match &redirect.to {
@@ -220,12 +244,6 @@ impl Io {
                 self.fds.remove(&2);
             }
         }
-        if let Some(pre_exec) = self.pre_exec.take() {
-            // Safety: pre_exec can only have been set by calling the pre_exec
-            // method, which is itself unsafe, so the safety comments at the
-            // point where that is called are the relevant ones
-            unsafe { cmd.pre_exec(pre_exec) };
-        }
     }
 }
 
diff --git a/src/pipeline/builtins/mod.rs b/src/pipeline/builtins/mod.rs
index 9c1ea9f..5c30575 100644
--- a/src/pipeline/builtins/mod.rs
+++ b/src/pipeline/builtins/mod.rs
@@ -6,7 +6,7 @@ pub use command::{Child, Command};
 type Builtin = &'static (dyn for<'a> Fn(
     crate::parse::Exe,
     &'a Env,
-    command::Io,
+    command::Cfg,
 ) -> anyhow::Result<command::Child<'a>>
               + Sync
               + Send);
@@ -28,24 +28,24 @@ static BUILTINS: once_cell::sync::Lazy<
 });
 
 macro_rules! bail {
-    ($io:expr, $exe:expr, $msg:expr $(,)?) => {
-        $io.write_stderr(
+    ($cfg:expr, $exe:expr, $msg:expr $(,)?) => {
+        $cfg.io().write_stderr(
             format!("{}: {}\n", $exe.exe().display(), $msg).as_bytes()
         )
         .await
         .unwrap();
         return std::process::ExitStatus::from_raw(1 << 8);
     };
-    ($io:expr, $exe:expr, $msg:expr, $($arg:tt)*) => {
-        $io.write_stderr(
+    ($cfg:expr, $exe:expr, $msg:expr, $($arg:tt)*) => {
+        $cfg.io().write_stderr(
             format!("{}: ", $exe.exe().display()).as_bytes()
         )
         .await
         .unwrap();
-        $io.write_stderr(format!($msg, $($arg)*).as_bytes())
+        $cfg.io().write_stderr(format!($msg, $($arg)*).as_bytes())
             .await
             .unwrap();
-        $io.write_stderr(b"\n").await.unwrap();
+        $cfg.io().write_stderr(b"\n").await.unwrap();
         return std::process::ExitStatus::from_raw(1 << 8);
     };
 }
@@ -55,19 +55,19 @@ macro_rules! bail {
 fn cd(
     exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     async fn async_cd(
         exe: crate::parse::Exe,
         _env: &Env,
-        mut io: command::Io,
+        cfg: command::Cfg,
     ) -> std::process::ExitStatus {
         let dir = exe.args().get(0).map_or("", String::as_str);
         let dir = if dir.is_empty() {
             if let Some(dir) = home(None) {
                 dir
             } else {
-                bail!(io, exe, "couldn't find current user");
+                bail!(cfg, exe, "couldn't find current user");
             }
         } else if dir.starts_with('~') {
             let path: std::path::PathBuf = dir.into();
@@ -84,7 +84,7 @@ fn cd(
                     home.join(path.strip_prefix(prefix).unwrap())
                 } else {
                     bail!(
-                        io,
+                        cfg,
                         exe,
                         "no such user: {}",
                         name.map(std::ffi::OsStr::to_string_lossy)
@@ -102,7 +102,7 @@ fn cd(
         };
         if let Err(e) = std::env::set_current_dir(&dir) {
             bail!(
-                io,
+                cfg,
                 exe,
                 "{}: {}",
                 crate::format::io_error(&e),
@@ -113,7 +113,7 @@ fn cd(
     }
 
     Ok(command::Child::new_fut(async move {
-        async_cd(exe, env, io).await
+        async_cd(exe, env, cfg).await
     }))
 }
 
@@ -121,22 +121,22 @@ fn cd(
 fn setenv(
     exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     async fn async_setenv(
         exe: crate::parse::Exe,
         _env: &Env,
-        mut io: command::Io,
+        cfg: command::Cfg,
     ) -> std::process::ExitStatus {
         let k = if let Some(k) = exe.args().get(0).map(String::as_str) {
             k
         } else {
-            bail!(io, exe, "usage: setenv key value");
+            bail!(cfg, exe, "usage: setenv key value");
         };
         let v = if let Some(v) = exe.args().get(1).map(String::as_str) {
             v
         } else {
-            bail!(io, exe, "usage: setenv key value");
+            bail!(cfg, exe, "usage: setenv key value");
         };
 
         std::env::set_var(k, v);
@@ -144,7 +144,7 @@ fn setenv(
     }
 
     Ok(command::Child::new_fut(async move {
-        async_setenv(exe, env, io).await
+        async_setenv(exe, env, cfg).await
     }))
 }
 
@@ -152,17 +152,17 @@ fn setenv(
 fn unsetenv(
     exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     async fn async_unsetenv(
         exe: crate::parse::Exe,
         _env: &Env,
-        mut io: command::Io,
+        cfg: command::Cfg,
     ) -> std::process::ExitStatus {
         let k = if let Some(k) = exe.args().get(0).map(String::as_str) {
             k
         } else {
-            bail!(io, exe, "usage: unsetenv key");
+            bail!(cfg, exe, "usage: unsetenv key");
         };
 
         std::env::remove_var(k);
@@ -170,7 +170,7 @@ fn unsetenv(
     }
 
     Ok(command::Child::new_fut(async move {
-        async_unsetenv(exe, env, io).await
+        async_unsetenv(exe, env, cfg).await
     }))
 }
 
@@ -181,17 +181,18 @@ fn unsetenv(
 fn echo(
     exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     async fn async_echo(
         exe: crate::parse::Exe,
         _env: &Env,
-        mut io: command::Io,
+        cfg: command::Cfg,
     ) -> std::process::ExitStatus {
         macro_rules! write_stdout {
             ($bytes:expr) => {
-                if let Err(e) = io.write_stdout($bytes).await {
-                    io.write_stderr(format!("echo: {}", e).as_bytes())
+                if let Err(e) = cfg.io().write_stdout($bytes).await {
+                    cfg.io()
+                        .write_stderr(format!("echo: {}", e).as_bytes())
                         .await
                         .unwrap();
                     return async_std::process::ExitStatus::from_raw(1 << 8);
@@ -212,19 +213,19 @@ fn echo(
     }
 
     Ok(command::Child::new_fut(async move {
-        async_echo(exe, env, io).await
+        async_echo(exe, env, cfg).await
     }))
 }
 
 fn and(
     mut exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     exe.shift();
     if env.latest_status().success() {
         let mut cmd = crate::pipeline::Command::new(exe);
-        io.setup_command(&mut cmd);
+        cfg.setup_command(&mut cmd);
         Ok(command::Child::new_wrapped(cmd.spawn(env)?))
     } else {
         let status = *env.latest_status();
@@ -235,7 +236,7 @@ fn and(
 fn or(
     mut exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     exe.shift();
     if env.latest_status().success() {
@@ -243,7 +244,7 @@ fn or(
         Ok(command::Child::new_fut(async move { status }))
     } else {
         let mut cmd = crate::pipeline::Command::new(exe);
-        io.setup_command(&mut cmd);
+        cfg.setup_command(&mut cmd);
         Ok(command::Child::new_wrapped(cmd.spawn(env)?))
     }
 }
@@ -251,22 +252,22 @@ fn or(
 fn command(
     mut exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     exe.shift();
     let mut cmd = crate::pipeline::Command::new_binary(exe);
-    io.setup_command(&mut cmd);
+    cfg.setup_command(&mut cmd);
     Ok(command::Child::new_wrapped(cmd.spawn(env)?))
 }
 
 fn builtin(
     mut exe: crate::parse::Exe,
     env: &Env,
-    io: command::Io,
+    cfg: command::Cfg,
 ) -> anyhow::Result<command::Child> {
     exe.shift();
     let mut cmd = crate::pipeline::Command::new_builtin(exe);
-    io.setup_command(&mut cmd);
+    cfg.setup_command(&mut cmd);
     Ok(command::Child::new_wrapped(cmd.spawn(env)?))
 }