Repository: zhiburt/expectrl (language: rust)

Issue:
Session::spawn() method is hard to use across platforms
As things are now, `Session::spawn()` expects `std::process::Command` on Unix-like platforms and `conpty::ProcAttr` on Windows (the documentation only mentions the former). The API doesn’t expose the command type in use, this type is private. As a result, my code has to depend on `conpty` even though it doesn’t actually use it.

The way I see it, expectrl could at least expose the command type publicly. But ideally it would really take the standard `std::process::Command` structure on all platforms and convert to `conpty::ProcAttr` itself.


Discussion / hints:
More subtle differences: `std::process::Command::args()` takes anything that can be turned into an iterator, `conpty::ProcAttr::args()` only `Vec<String>`. And while the former can be called any number of times to add arguments, the latter will replace the entire arguments list. It’s such a mess, I’d really prefer expectrl to handle `conpty` internally.
Hi @palant 

Thank you for opening the issue.

> The API doesn’t expose the command type in use, this type is private. 

I think it's accessible by this route.

```rust
use expectrl::process::windows::ProcAttr;
```

> But ideally it would really take the standard std::process::Command structure on all platforms and convert to conpty::ProcAttr itself.

Yes it was meant to be so originally.
But was not possible originally because the `Command::get_` methods were on nightly only.

I was not aware that we can do it since https://blog.rust-lang.org/2021/12/02/Rust-1.57.0.html.

> More subtle differences: std::process::Command::args() takes anything that can be turned into an iterator, conpty::ProcAttr::args() only Vec<String>. And while the former can be called any number of times to add arguments, the latter will replace the entire arguments list.

Interesting. I'll investigate it.
_____

I'll try to work on `std::process::Command` as a parameter to `Session::spawn()` on windows in the nearest if you don't mind.

Fix patch:
diff --git a/src/process/windows.rs b/src/process/windows.rs
index 2388940..b3d2e80 100644
--- a/src/process/windows.rs
+++ b/src/process/windows.rs
@@ -1,13 +1,17 @@
 //! This module contains a Windows implementation of [crate::process::Process].
 
 use std::{
+    collections::HashMap,
+    ffi::OsStr,
     io::{self, Read, Result, Write},
+    iter::FromIterator,
     ops::{Deref, DerefMut},
+    process::Command,
 };
 
 use conpty::{
     io::{PipeReader, PipeWriter},
-    Process,
+    ProcAttr, Process,
 };
 
 use super::{Healthcheck, NonBlocking, Process as ProcessTrait};
@@ -23,24 +27,23 @@ use std::{
     task::{Context, Poll},
 };
 
-pub use conpty::ProcAttr;
-
 /// A windows representation of a [Process] via [conpty::Process].
 pub struct WinProcess {
     proc: Process,
 }
 
 impl ProcessTrait for WinProcess {
-    type Command = ProcAttr;
+    type Command = Command;
     type Stream = ProcessStream;
 
     fn spawn<S: AsRef<str>>(cmd: S) -> Result<Self> {
-        Self::spawn_command(ProcAttr::cmd(cmd.as_ref()))
+        Self::spawn_command(Command::new(cmd.as_ref()))
     }
 
     fn spawn_command(command: Self::Command) -> Result<Self> {
-        command
-            .spawn()
+        let attr = command_to_proc_attr(command)
+            .ok_or_else(|| to_io_error("command parsing error")(""))?;
+        attr.spawn()
             .map_err(to_io_error(""))
             .map(|proc| WinProcess { proc })
     }
@@ -167,3 +170,44 @@ impl AsyncRead for AsyncProcessStream {
         Pin::new(&mut self.stream).poll_read(cx, buf)
     }
 }
+
+fn command_to_proc_attr(cmd: Command) -> Option<ProcAttr> {
+    let program = cmd.get_program().to_str()?;
+    let mut attr = ProcAttr::cmd(program);
+
+    if let Some(dir) = cmd.get_current_dir() {
+        let dir = dir.to_str()?;
+
+        attr = attr.current_dir(dir);
+    }
+
+    if cmd.get_args().len() > 0 {
+        let args = cmd
+            .get_args()
+            .into_iter()
+            .map(os_str_to_string)
+            .collect::<Option<Vec<String>>>()?;
+
+        attr = attr.args(args);
+    }
+
+    if cmd.get_envs().len() > 0 {
+        let envs = cmd
+            .get_envs()
+            .into_iter()
+            .filter(|(_, v)| v.is_some())
+            .map(|(k, v)| {
+                os_str_to_string(k).and_then(|k| os_str_to_string(v.unwrap()).map(|v| (k, v)))
+            })
+            .collect::<Option<Vec<(String, String)>>>()?;
+        let envs = HashMap::from_iter(envs);
+
+        attr = attr.envs(envs);
+    }
+
+    Some(attr)
+}
+
+fn os_str_to_string(s: &OsStr) -> Option<String> {
+    s.to_str().map(|s| s.to_owned())
+}
diff --git a/src/session/mod.rs b/src/session/mod.rs
index 3a9dc0b..82e59ea 100644
--- a/src/session/mod.rs
+++ b/src/session/mod.rs
@@ -56,29 +56,10 @@ impl Session {
     /// On unix
     ///
     /// ```no_run
-    /// # #[cfg(unix)]
-    /// # {
-    /// #
     /// use expectrl::session::Session;
     /// use std::process::Command;
     ///
     /// let p = Session::spawn(Command::new("cat"));
-    /// #
-    /// # }
-    /// ```
-    ///
-    /// On windows
-    ///
-    /// ```no_run
-    /// # #[cfg(windows)]
-    /// # {
-    /// #
-    /// use expectrl::session::Session;
-    /// use expectrl::process::windows::ProcAttr;
-    ///
-    /// let p = Session::spawn(ProcAttr::default().commandline(r"pwsh".to_string()));
-    /// #
-    /// # }
     /// ```
     pub fn spawn(command: <Proc as Process>::Command) -> Result<Self, Error> {
         let mut process = Proc::spawn_command(command)?;


Test patch:
diff --git a/tests/expect.rs b/tests/expect.rs
index 4c52935..adaa651 100644
--- a/tests/expect.rs
+++ b/tests/expect.rs
@@ -132,9 +132,10 @@ fn expect_n_bytes() {
 #[cfg(windows)]
 #[test]
 fn expect_n_bytes() {
-    use expectrl::{process::windows::ProcAttr, session::Session};
+    use expectrl::session::Session;
+    use std::process::Command;
 
-    let mut session = Session::spawn(ProcAttr::cmd(r#"echo Hello World"#.to_string())).unwrap();
+    let mut session = Session::spawn(Command::new("echo Hello World")).unwrap();
 
     // give shell some time
     std::thread::sleep(Duration::from_millis(300));
diff --git a/tests/io.rs b/tests/io.rs
index ea0e7f5..6702a6f 100644
--- a/tests/io.rs
+++ b/tests/io.rs
@@ -1,15 +1,9 @@
 use expectrl::{session::Session, Captures, ControlCode, Needle};
-use std::{thread, time::Duration};
-
-#[cfg(unix)]
-use std::process::Command;
+use std::{process::Command, thread, time::Duration};
 
 #[cfg(unix)]
 use expectrl::WaitStatus;
 
-#[cfg(windows)]
-use expectrl::process::windows::ProcAttr;
-
 #[cfg(feature = "async")]
 use futures_lite::{
     future::block_on,
@@ -30,8 +24,7 @@ fn send_controll() {
 #[test]
 #[cfg(windows)]
 fn send_controll() {
-    let mut proc =
-        Session::spawn(ProcAttr::cmd("powershell -C ping localhost".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("powershell -C ping localhost")).unwrap();
 
     // give powershell a bit time
     thread::sleep(Duration::from_millis(100));
@@ -62,8 +55,7 @@ fn send() {
 #[test]
 #[cfg(windows)]
 fn send() {
-    let mut proc =
-        Session::spawn(ProcAttr::default().commandline("powershell -C type".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("powershell -C type")).unwrap();
     thread::sleep(Duration::from_millis(1000));
 
     _p_send(&mut proc, "hello cat\r\n").unwrap();
@@ -95,7 +87,7 @@ fn send_line() {
 #[test]
 #[cfg(windows)]
 fn send_line() {
-    let mut proc = Session::spawn(ProcAttr::cmd("powershell -C type".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("powershell -C type")).unwrap();
 
     thread::sleep(Duration::from_millis(1000));
     _p_send_line(&mut proc, "hello cat").unwrap();
@@ -144,8 +136,7 @@ fn try_read_by_byte() {
     // it shows that on windows ECHO is turned on.
     // Mustn't it be turned down?
 
-    let mut proc =
-        Session::spawn(ProcAttr::default().commandline("powershell".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("powershell")).unwrap();
     _p_send_line(
         &mut proc,
         "while (1) { read-host | set r; if (!$r) { break }}",
@@ -204,8 +195,7 @@ fn blocking_read_after_non_blocking() {
 #[test]
 #[cfg(windows)]
 fn blocking_read_after_non_blocking() {
-    let mut proc =
-        Session::spawn(ProcAttr::default().commandline("powershell".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("powershell")).unwrap();
     _p_send_line(
         &mut proc,
         "while (1) { read-host | set r; if (!$r) { break }}",
@@ -258,8 +248,7 @@ fn try_read() {
 #[test]
 #[cfg(windows)]
 fn try_read() {
-    let mut proc =
-        Session::spawn(ProcAttr::default().commandline("powershell".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("powershell")).unwrap();
     thread::sleep(Duration::from_millis(300));
     _p_send_line(
         &mut proc,
@@ -375,7 +364,7 @@ fn try_read_after_process_exit() {
 #[test]
 #[cfg(windows)]
 fn try_read_after_process_exit() {
-    let mut proc = Session::spawn(ProcAttr::cmd("echo hello cat".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("echo hello cat")).unwrap();
 
     assert_eq!(proc.wait(None).unwrap(), 0);
 
@@ -411,7 +400,7 @@ fn try_read_to_end() {
 #[test]
 #[cfg(windows)]
 fn try_read_to_end() {
-    let mut proc = Session::spawn(ProcAttr::cmd("echo Hello World".to_string())).unwrap();
+    let mut proc = Session::spawn(Command::new("echo Hello World")).unwrap();
 
     thread::sleep(Duration::from_millis(1000));
 
@@ -435,7 +424,7 @@ fn try_read_to_end() {
 #[test]
 #[cfg(windows)]
 fn continues_try_reads() {
-    let cmd = ProcAttr::default().commandline("python3 -c \"import time; print('Start Sleep'); time.sleep(0.1); print('End of Sleep'); yn=input('input');\"".to_string());
+    let cmd = Command::new("python3 -c \"import time; print('Start Sleep'); time.sleep(0.1); print('End of Sleep'); yn=input('input');\"");
 
     let mut proc = Session::spawn(cmd).unwrap();