make debugging output for protocol structs better readable

src/protocol/command.rs

@@ -1,12 +1,14 @@
 use super::argument_parser::Parser;
 use super::executable_path;
 use crate::R;
+use std::ffi::OsString;
+use std::path::PathBuf;
 use std::str;
 
 #[derive(PartialEq, Eq, Debug, Clone)]
 pub struct Command {
-    pub executable: Vec<u8>,
-    pub arguments: Vec<Vec<u8>>,
+    pub executable: PathBuf,
+    pub arguments: Vec<OsString>,
 }
 
 impl Command {
@@ -35,20 +37,21 @@ impl Command {
         word.chars().map(escape_char).collect::<Vec<_>>().join("")
     }
 
-    pub fn format_arguments(arguments: Vec<Vec<u8>>) -> String {
+    pub fn format_arguments(arguments: Vec<OsString>) -> String {
         arguments
             .into_iter()
-            .map(|argument| Command::escape(String::from_utf8_lossy(&argument).to_string()))
+            .map(|argument| Command::escape(argument.to_string_lossy().into_owned()))
             .map(Command::add_quotes_if_needed)
             .collect::<Vec<String>>()
             .join(" ")
     }
 
     pub fn format(&self) -> String {
-        let executable =
-            String::from_utf8_lossy(&executable_path::canonicalize(&self.executable)).into_owned();
+        let executable = executable_path::canonicalize(&self.executable)
+            .to_string_lossy()
+            .into_owned();
         if self.arguments.is_empty() {
-            executable.to_string()
+            executable
         } else {
             format!(
                 "{} {}",
@@ -59,13 +62,11 @@ impl Command {
     }
 
     pub fn new(command: &str) -> R<Command> {
-        let mut words = Parser::parse_arguments(command)?
-            .into_iter()
-            .map(|word| word.into_bytes());
+        let mut words = Parser::parse_arguments(command)?.into_iter();
         match words.next() {
             Some(executable) => Ok(Command {
-                executable,
-                arguments: words.collect(),
+                executable: PathBuf::from(executable),
+                arguments: words.map(OsString::from).collect(),
             }),
             None => Err(format!(
                 "expected: space-separated command and arguments ({:?})",
@@ -89,8 +90,8 @@ mod command {
             assert_eq!(
                 Command::new("foo bar")?,
                 Command {
-                    executable: b"foo".to_vec(),
-                    arguments: vec![b"bar".to_vec()]
+                    executable: PathBuf::from("foo"),
+                    arguments: vec![OsString::from("bar")]
                 }
             );
             Ok(())
@@ -101,8 +102,8 @@ mod command {
             assert_eq!(
                 Command::new(r#"foo "bar baz""#)?,
                 Command {
-                    executable: b"foo".to_vec(),
-                    arguments: vec![b"bar baz".to_vec()]
+                    executable: PathBuf::from("foo"),
+                    arguments: vec![OsString::from("bar baz")]
                 }
             );
             Ok(())
@@ -113,15 +114,15 @@ mod command {
             assert_eq!(
                 Command::new(r#"foo\" bar baz"#)?,
                 Command {
-                    executable: b"foo\"".to_vec(),
-                    arguments: vec![b"bar", b"baz"].map(|arg| arg.to_vec())
+                    executable: PathBuf::from("foo\""),
+                    arguments: vec!["bar", "baz"].map(OsString::from)
                 }
             );
             assert_eq!(
                 Command::new(r#"foo\" "bar baz""#)?,
                 Command {
-                    executable: b"foo\"".to_vec(),
-                    arguments: vec![b"bar baz".to_vec()]
+                    executable: PathBuf::from("foo\""),
+                    arguments: vec![OsString::from("bar baz")]
                 }
             );
             Ok(())
@@ -132,8 +133,8 @@ mod command {
             assert_eq!(
                 Command::new(r#"foo "bar\" baz""#)?,
                 Command {
-                    executable: b"foo".to_vec(),
-                    arguments: vec![b"bar\" baz".to_vec()]
+                    executable: PathBuf::from("foo"),
+                    arguments: vec![OsString::from("bar\" baz")]
                 }
             );
             Ok(())
@@ -171,8 +172,8 @@ mod command {
             assert_eq!(
                 Command::new("foo  bar")?,
                 Command {
-                    executable: b"foo".to_vec(),
-                    arguments: vec![b"bar".to_vec()]
+                    executable: PathBuf::from("foo"),
+                    arguments: vec![OsString::from("bar")]
                 }
             );
             Ok(())
@@ -183,8 +184,8 @@ mod command {
             assert_eq!(
                 Command::new(" foo bar")?,
                 Command {
-                    executable: b"foo".to_vec(),
-                    arguments: vec![b"bar".to_vec()]
+                    executable: PathBuf::from("foo"),
+                    arguments: vec![OsString::from("bar")]
                 }
             );
             Ok(())
@@ -195,8 +196,8 @@ mod command {
             assert_eq!(
                 Command::new("foo bar ")?,
                 Command {
-                    executable: b"foo".to_vec(),
-                    arguments: vec![b"bar".to_vec()]
+                    executable: PathBuf::from("foo"),
+                    arguments: vec![OsString::from("bar")]
                 }
             );
             Ok(())
@@ -210,8 +211,8 @@ mod command {
                 assert_eq!(
                     Command::new(r#"foo "bar\nbaz""#)?,
                     Command {
-                        executable: b"foo".to_vec(),
-                        arguments: vec![b"bar\nbaz".to_vec()]
+                        executable: PathBuf::from("foo"),
+                        arguments: vec![OsString::from("bar\nbaz")]
                     }
                 );
                 Ok(())
@@ -222,8 +223,8 @@ mod command {
                 assert_eq!(
                     Command::new(r#"foo bar\ baz"#)?,
                     Command {
-                        executable: b"foo".to_vec(),
-                        arguments: vec![b"bar baz".to_vec()]
+                        executable: PathBuf::from("foo"),
+                        arguments: vec![OsString::from("bar baz")]
                     }
                 );
                 Ok(())
@@ -234,8 +235,8 @@ mod command {
                 assert_eq!(
                     Command::new(r#"foo bar\\baz"#)?,
                     Command {
-                        executable: b"foo".to_vec(),
-                        arguments: vec![br"bar\baz".to_vec()]
+                        executable: PathBuf::from("foo"),
+                        arguments: vec![OsString::from(r"bar\baz")]
                     }
                 );
                 Ok(())

src/protocol/executable_path.rs

@@ -1,9 +1,7 @@
 use quale::which;
-use std::ffi::OsStr;
-use std::os::unix::ffi::OsStrExt;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
-pub fn compare_executables(a: &[u8], b: &[u8]) -> bool {
+pub fn compare_executables(a: &Path, b: &Path) -> bool {
     canonicalize(a) == canonicalize(b)
 }
 
@@ -14,46 +12,44 @@ mod compare_executables {
 
     #[test]
     fn returns_true_if_executables_are_identical() -> R<()> {
-        let executable = b"./bin/myexec";
+        let executable = Path::new("./bin/myexec");
         assert!(compare_executables(executable, executable));
         Ok(())
     }
 
     #[test]
     fn returns_false_if_executables_are_distinct() -> R<()> {
-        let a = b"./bin/myexec";
-        let b = b"./bin/myotherexec";
+        let a = Path::new("./bin/myexec");
+        let b = Path::new("./bin/myotherexec");
         assert!(!compare_executables(a, b));
         Ok(())
     }
 
     #[test]
     fn returns_true_if_executables_match_after_lookup_in_path() -> R<()> {
         let path = which("cp").unwrap();
-        let cp_long = path.as_os_str().as_bytes();
-        let cp_short = b"cp";
-        assert!(compare_executables(cp_long, cp_short));
+        let cp_long = path;
+        let cp_short = Path::new("cp");
+        assert!(compare_executables(&cp_long, cp_short));
         Ok(())
     }
 }
 
-pub fn canonicalize(executable: &[u8]) -> Vec<u8> {
-    let path = PathBuf::from(OsStr::from_bytes(executable));
-    let file_name = match path.file_name() {
-        None => return executable.to_vec(),
+pub fn canonicalize(executable: &Path) -> PathBuf {
+    let file_name = match executable.file_name() {
+        None => return executable.into(),
         Some(f) => f,
     };
     match which(file_name) {
         Some(resolved) => {
-            if resolved == path {
-                file_name.as_bytes()
+            if resolved == executable {
+                PathBuf::from(file_name)
             } else {
-                executable
+                executable.into()
             }
         }
-        None => executable,
+        None => executable.into(),
     }
-    .to_vec()
 }
 
 #[cfg(test)]
@@ -66,40 +62,40 @@ mod canonicalize {
     fn shortens_absolute_executable_paths_if_found_in_path() -> R<()> {
         let executable = "cp";
         let resolved = which(executable).unwrap();
-        let file_name = canonicalize(resolved.as_os_str().as_bytes());
-        assert_eq!(String::from_utf8(file_name)?, "cp");
+        let file_name = canonicalize(&resolved);
+        assert_eq!(file_name, PathBuf::from("cp"));
         Ok(())
     }
 
     #[test]
     fn does_not_shorten_executable_that_is_not_in_path() -> R<()> {
-        let executable = b"/foo/doesnotexist";
+        let executable = Path::new("/foo/doesnotexist");
         let file_name = canonicalize(executable);
-        assert_eq!(String::from_utf8(file_name)?, "/foo/doesnotexist");
+        assert_eq!(file_name, PathBuf::from("/foo/doesnotexist"));
         Ok(())
     }
 
     #[test]
     fn does_not_shorten_executable_that_is_not_in_path_but_has_same_name_as_one_that_is() -> R<()> {
-        let executable = b"/not/in/path/ls";
+        let executable = Path::new("/not/in/path/ls");
         let file_name = canonicalize(executable);
-        assert_eq!(String::from_utf8(file_name)?, "/not/in/path/ls");
+        assert_eq!(file_name, PathBuf::from("/not/in/path/ls"));
         Ok(())
     }
 
     #[test]
     fn does_not_shorten_relative_path() -> R<()> {
-        let executable = b"./foo";
+        let executable = Path::new("./foo");
         let file_name = canonicalize(executable);
-        assert_eq!(String::from_utf8(file_name)?, "./foo");
+        assert_eq!(file_name, PathBuf::from("./foo"));
         Ok(())
     }
 
     #[test]
     fn does_not_modify_short_forms_if_found_in_path() -> R<()> {
-        let executable = b"ls";
+        let executable = Path::new("ls");
         let file_name = canonicalize(executable);
-        assert_eq!(String::from_utf8(file_name)?, "ls");
+        assert_eq!(file_name, PathBuf::from("ls"));
         Ok(())
     }
 }