Merge pull request #1085 from andrasio/externals-line

$it can contain a string line or plain string data.

crates/test-support/src/macros.rs

@@ -103,57 +103,3 @@ macro_rules! nu_error {
         out.into_owned()
     }};
 }
-
-#[macro_export]
-macro_rules! nu_combined {
-    (cwd: $cwd:expr, $path:expr, $($part:expr),*) => {{
-        use $crate::fs::DisplayPath;
-
-        let path = format!($path, $(
-            $part.display_path()
-        ),*);
-
-        nu_combined!($cwd, &path)
-    }};
-
-    (cwd: $cwd:expr, $path:expr) => {{
-        nu_combined!($cwd, $path)
-    }};
-
-    ($cwd:expr, $path:expr) => {{
-        pub use std::error::Error;
-        pub use std::io::prelude::*;
-        pub use std::process::{Command, Stdio};
-
-        let commands = &*format!(
-            "
-                            cd {}
-                            {}
-                            exit",
-            $crate::fs::in_directory($cwd),
-            $crate::fs::DisplayPath::display_path(&$path)
-        );
-
-        let mut process = Command::new($crate::fs::executable_path())
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()
-            .expect("couldn't run test");
-
-        let stdin = process.stdin.as_mut().expect("couldn't open stdin");
-        stdin
-            .write_all(commands.as_bytes())
-            .expect("couldn't write to stdin");
-
-        let output = process
-            .wait_with_output()
-            .expect("couldn't read from stdout/stderr");
-
-        let err = String::from_utf8_lossy(&output.stderr).into_owned();
-        let out = String::from_utf8_lossy(&output.stdout).into_owned();
-        let out = out.replace("\r\n", "");
-        let out = out.replace("\n", "");
-        (out, err)
-    }};
-}

src/commands/classified/external.rs

@@ -5,7 +5,7 @@ use futures_codec::{Decoder, Encoder, Framed};
 use log::trace;
 use nu_errors::ShellError;
 use nu_parser::ExternalCommand;
-use nu_protocol::{UntaggedValue, Value};
+use nu_protocol::{Primitive, UntaggedValue, Value};
 use std::io::{Error, ErrorKind};
 use std::ops::Deref;
 use subprocess::Exec;
@@ -82,23 +82,32 @@ pub(crate) async fn run_external_command(
     if arg_string.contains("$it") {
         let input_strings = inputs
             .iter()
-            .map(|i| {
+            .map(|i| match i {
-                i.as_string().map(|s| s.to_string()).map_err(|_| {
+                Value {
+                    value: UntaggedValue::Primitive(Primitive::String(s)),
+                    ..
+                }
+                | Value {
+                    value: UntaggedValue::Primitive(Primitive::Line(s)),
+                    ..
+                } => Ok(s.clone()),
+                _ => {
                     let arg = command.args.iter().find(|arg| arg.contains("$it"));
+
                     if let Some(arg) = arg {
-                        ShellError::labeled_error(
+                        Err(ShellError::labeled_error(
                             "External $it needs string data",
                             "given row instead of string data",
                             &arg.tag,
-                        )
+                        ))
                     } else {
-                        ShellError::labeled_error(
+                        Err(ShellError::labeled_error(
                             "$it needs string data",
                             "given something else",
                             command.name_tag.clone(),
-                        )
+                        ))
                     }
-                })
+                }
             })
             .collect::<Result<Vec<String>, ShellError>>()?;
 

tests/shell/mod.rs

@@ -1,32 +1,90 @@
 mod pipeline {
-    use test_support::fs::Stub::FileWithContent;
+    use test_support::fs::Stub::EmptyFile;
     use test_support::playground::Playground;
-    use test_support::{nu_combined, pipeline};
+    use test_support::{nu, pipeline};
 
     #[test]
-    fn it_arg_works_with_many_inputs_to_external_command() {
+    fn can_process_row_as_it_argument_to_an_external_command_given_the_it_data_is_a_string() {
-        Playground::setup("it_arg_works_with_many_inputs", |dirs, sandbox| {
+        Playground::setup("it_argument_test_1", |dirs, sandbox| {
             sandbox.with_files(vec![
-                FileWithContent("file1", "text"),
+                EmptyFile("jonathan_likes_cake.txt"),
-                FileWithContent("file2", " and more text"),
+                EmptyFile("andres_likes_arepas.txt"),
             ]);
 
-            let (stdout, stderr) = nu_combined!(
+            let actual = nu!(
                 cwd: dirs.test(), pipeline(
                 r#"
-                    echo hello world
+                    ls
-                    | split-row " "
+                    | sort-by name
+                    | get name
                     | ^echo $it
                 "#
             ));
 
             #[cfg(windows)]
-            assert_eq!("hello world", stdout);
+            assert_eq!(actual, "andres_likes_arepas.txt jonathan_likes_cake.txt");
 
             #[cfg(not(windows))]
-            assert_eq!("helloworld", stdout);
+            assert_eq!(actual, "andres_likes_arepas.txtjonathan_likes_cake.txt");
-
-            assert!(!stderr.contains("No such file or directory"));
         })
     }
+
+    #[test]
+    fn can_process_row_as_it_argument_to_an_external_command_given_the_it_data_is_one_string_line()
+    {
+        Playground::setup("it_argument_test_2", |dirs, sandbox| {
+            sandbox.with_files(vec![
+                EmptyFile("jonathan_likes_cake.txt"),
+                EmptyFile("andres_likes_arepas.txt"),
+            ]);
+
+            let actual = nu!(
+                cwd: dirs.test(), pipeline(
+                r#"
+                    ls
+                    | sort-by name
+                    | get name
+                    | lines
+                    | ^echo $it
+                "#
+            ));
+
+            #[cfg(windows)]
+            assert_eq!(actual, "andres_likes_arepas.txt jonathan_likes_cake.txt");
+
+            #[cfg(not(windows))]
+            assert_eq!(actual, "andres_likes_arepas.txtjonathan_likes_cake.txt");
+        })
+    }
+
+    mod expands_tilde {
+        use super::nu;
+
+        #[test]
+        fn as_home_directory_when_passed_as_argument_and_begins_with_tilde_to_an_external() {
+            let actual = nu!(
+                cwd: std::path::PathBuf::from("."),
+                r#"
+                    sh -c "echo ~"
+                "#
+            );
+
+            assert!(
+                !actual.contains("~"),
+                format!("'{}' should not contain ~", actual)
+            );
+        }
+
+        #[test]
+        fn does_not_expand_when_passed_as_argument_and_does_not_start_with_tilde_to_an_external() {
+            let actual = nu!(
+                cwd: std::path::PathBuf::from("."),
+                r#"
+                    sh -c "echo 1~1"
+                "#
+            );
+
+            assert_eq!(actual, "1~1");
+        }
+    }
 }