Externals now spawn independently. (#1230)

This commit changes the way we shell out externals when using the `"$it"` argument. Also pipes per row to an external's stdin if no `"$it"` argument is present for external commands. 

Further separation of logic (preparing the external's command arguments, getting the data for piping, emitting values, spawning processes) will give us a better idea for lower level details regarding external commands until we can find the right abstractions for making them more generic and unify within the pipeline calling logic of Nu internal's and external's.

Cargo.lock

@@ -2347,6 +2347,8 @@ dependencies = [
  "getset",
  "glob",
  "nu-build",
+ "nu-parser",
+ "nu-source",
  "tempfile",
 ]
 

crates/nu-parser/src/commands/classified/external.rs

@@ -6,6 +6,16 @@ pub struct ExternalArg {
     pub tag: Tag,
 }
 
+impl ExternalArg {
+    pub fn has(&self, name: &str) -> bool {
+        self.arg == name
+    }
+
+    pub fn is_it(&self) -> bool {
+        self.has("$it")
+    }
+}
+
 impl std::ops::Deref for ExternalArg {
     type Target = str;
 
@@ -42,6 +52,12 @@ pub struct ExternalCommand {
     pub args: ExternalArgs,
 }
 
+impl ExternalCommand {
+    pub fn has_it_argument(&self) -> bool {
+        self.args.iter().any(|arg| arg.has("$it"))
+    }
+}
+
 impl PrettyDebug for ExternalCommand {
     fn pretty(&self) -> DebugDocBuilder {
         b::typed(

crates/nu-test-support/Cargo.toml

@@ -10,6 +10,9 @@ license = "MIT"
 doctest = false
 
 [dependencies]
+nu-parser = { path = "../nu-parser", version = "0.8.0" }
+nu-source = { path = "../nu-source", version = "0.8.0" }
+
 app_dirs = "1.2.1"
 dunce = "1.0.0"
 getset = "0.0.9"

crates/nu-test-support/src/commands.rs

@@ -0,0 +1,50 @@
+use nu_parser::commands::classified::external::{ExternalArg, ExternalArgs, ExternalCommand};
+use nu_source::{Span, SpannedItem, Tag, TaggedItem};
+
+pub struct ExternalBuilder {
+    name: String,
+    args: Vec<String>,
+}
+
+impl ExternalBuilder {
+    pub fn for_name(name: &str) -> ExternalBuilder {
+        ExternalBuilder {
+            name: name.to_string(),
+            args: vec![],
+        }
+    }
+
+    pub fn arg(&mut self, value: &str) -> &mut Self {
+        self.args.push(value.to_string());
+        self
+    }
+
+    pub fn build(&mut self) -> ExternalCommand {
+        let mut path = crate::fs::binaries();
+        path.push(&self.name);
+
+        let name = path.to_string_lossy().to_string().spanned(Span::unknown());
+
+        let args = self
+            .args
+            .iter()
+            .map(|arg| {
+                let arg = arg.tagged(Tag::unknown());
+
+                ExternalArg {
+                    arg: arg.to_string(),
+                    tag: arg.tag,
+                }
+            })
+            .collect::<Vec<_>>();
+
+        ExternalCommand {
+            name: name.to_string(),
+            name_tag: Tag::unknown(),
+            args: ExternalArgs {
+                list: args,
+                span: name.span,
+            },
+        }
+    }
+}

crates/nu-test-support/src/dummies/chop.rs

@@ -1,8 +1,13 @@
 use std::io::{self, BufRead};
 
 fn main() {
-    let stdin = io::stdin();
+    if did_chop_arguments() {
+        // we are done and don't care about standard input.
+        std::process::exit(0);
+    }
 
+    // if no arguments given, chop from standard input and exit.
+    let stdin = io::stdin();
     let mut input = stdin.lock().lines();
 
     if let Some(Ok(given)) = input.next() {
@@ -20,3 +25,20 @@ fn chop(word: &str) -> &str {
 
     &word[..to]
 }
+
+fn did_chop_arguments() -> bool {
+    let args: Vec<String> = std::env::args().collect();
+
+    if args.len() > 1 {
+        let mut arguments = args.iter();
+        arguments.next();
+
+        for arg in arguments {
+            println!("{}", chop(arg));
+        }
+
+        return true;
+    }
+
+    false
+}

crates/nu-test-support/src/lib.rs

@@ -1,3 +1,4 @@
+pub mod commands;
 pub mod fs;
 pub mod macros;
 pub mod playground;

src/commands/classified/external.rs

@@ -48,6 +48,39 @@ impl Decoder for LinesCodec {
     }
 }
 
+pub fn nu_value_to_string(command: &ExternalCommand, from: &Value) -> Result<String, ShellError> {
+    match &from.value {
+        UntaggedValue::Primitive(Primitive::Int(i)) => Ok(i.to_string()),
+        UntaggedValue::Primitive(Primitive::String(s))
+        | UntaggedValue::Primitive(Primitive::Line(s)) => Ok(s.clone()),
+        UntaggedValue::Primitive(Primitive::Path(p)) => Ok(p.to_string_lossy().to_string()),
+        unsupported => Err(ShellError::labeled_error(
+            format!("$it needs string data (given: {})", unsupported.type_name()),
+            "expected a string",
+            &command.name_tag,
+        )),
+    }
+}
+
+pub fn nu_value_to_string_for_stdin(
+    command: &ExternalCommand,
+    from: &Value,
+) -> Result<Option<String>, ShellError> {
+    match &from.value {
+        UntaggedValue::Primitive(Primitive::Nothing) => Ok(None),
+        UntaggedValue::Primitive(Primitive::String(s))
+        | UntaggedValue::Primitive(Primitive::Line(s)) => Ok(Some(s.clone())),
+        unsupported => Err(ShellError::labeled_error(
+            format!(
+                "Received unexpected type from pipeline ({})",
+                unsupported.type_name()
+            ),
+            "expected a string",
+            &command.name_tag,
+        )),
+    }
+}
+
 pub(crate) async fn run_external_command(
     command: ExternalCommand,
     context: &mut Context,
@@ -56,8 +89,7 @@ pub(crate) async fn run_external_command(
 ) -> Result<Option<InputStream>, ShellError> {
     trace!(target: "nu::run::external", "-> {}", command.name);
 
-    let has_it_arg = command.args.iter().any(|arg| arg.contains("$it"));
+    if command.has_it_argument() {
-    if has_it_arg {
         run_with_iterator_arg(command, context, input, is_last).await
     } else {
         run_with_stdin(command, context, input, is_last).await
@@ -70,133 +102,76 @@ async fn run_with_iterator_arg(
     input: Option<InputStream>,
     is_last: bool,
 ) -> Result<Option<InputStream>, ShellError> {
-    let name = command.name;
+    let path = context.shell_manager.path()?;
-    let args = command.args;
-    let name_tag = command.name_tag;
-    let inputs = input.unwrap_or_else(InputStream::empty).into_vec().await;
 
-    trace!(target: "nu::run::external", "inputs = {:?}", inputs);
+    let mut inputs: InputStream = if let Some(input) = input {
+        trace_stream!(target: "nu::trace_stream::external::it", "input" = input)
+    } else {
+        InputStream::empty()
+    };
 
-    let input_strings = inputs
+    let stream = async_stream! {
-        .iter()
+        while let Some(value) = inputs.next().await {
-        .map(|i| match i {
+            let name = command.name.clone();
-            Value {
+            let name_tag = command.name_tag.clone();
-                value: UntaggedValue::Primitive(Primitive::Int(i)),
+            let home_dir = dirs::home_dir();
-                ..
+            let path = &path;
-            } => Ok(i.to_string()),
+            let args = command.args.clone();
-            Value {
+
-                value: UntaggedValue::Primitive(Primitive::String(s)),
+            let it_replacement = match nu_value_to_string(&command, &value) {
-                ..
+                Ok(value) => value,
-            }
+                Err(reason) => {
-            | Value {
+                    yield Ok(Value {
-                value: UntaggedValue::Primitive(Primitive::Line(s)),
+                        value: UntaggedValue::Error(reason),
-                ..
+                        tag: name_tag
-            } => Ok(s.clone()),
+                    });
-            Value {
+                    return;
-                value: UntaggedValue::Primitive(Primitive::Path(p)),
-                ..
-            } => Ok(p.to_string_lossy().to_string()),
-            _ => {
-                let arg = args.iter().find(|arg| arg.contains("$it"));
-                if let Some(arg) = arg {
-                    Err(ShellError::labeled_error(
-                        "External $it needs string data",
-                        "given row instead of string data",
-                        &arg.tag,
-                    ))
-                } else {
-                    Err(ShellError::labeled_error(
-                        "$it needs string data",
-                        "given something else",
-                        &name_tag,
-                    ))
                 }
-            }
+            };
-        })
+
-        .map(|result| {
+            let process_args = args.iter().filter_map(|arg| {
-            result.map(|value| {
+                if arg.chars().all(|c| c.is_whitespace()) {
-                if argument_contains_whitespace(&value) {
+                    None
-                    let arg = args.iter().find(|arg| arg.contains("$it"));
+                } else {
-                    if let Some(arg) = arg {
+                    let arg = if arg.is_it() {
-                        if !argument_is_quoted(&arg) {
+                        let value = it_replacement.to_owned();
-                            return add_quotes(&value);
+                        let value = expand_tilde(&value, || home_dir.as_ref()).as_ref().to_string();
+                        let value = {
+                            if argument_contains_whitespace(&value) && !argument_is_quoted(&value) {
+                                add_quotes(&value)
+                            } else {
+                                value
+                            }
+                        };
+                        value
+                    } else {
+                        arg.to_string()
+                    };
+
+                    Some(arg)
+                }
+            }).collect::<Vec<String>>();
+
+            match spawn(&command, &path, &process_args[..], None, is_last).await {
+                Ok(res) => {
+                    if let Some(mut res) = res {
+                        while let Some(item) = res.next().await {
+                            yield Ok(item)
                         }
                     }
                 }
-                value
+                Err(reason) => {
-            })
+                    yield Ok(Value {
-        })
+                        value: UntaggedValue::Error(reason),
-        .collect::<Result<Vec<String>, ShellError>>()?;
+                        tag: name_tag
-
+                    });
-    let home_dir = dirs::home_dir();
+                    return;
-    let commands = input_strings.iter().map(|i| {
+                }
-        let args = args.iter().filter_map(|arg| {
-            if arg.chars().all(|c| c.is_whitespace()) {
-                None
-            } else {
-                let arg = shellexpand::tilde_with_context(arg.deref(), || home_dir.as_ref());
-                Some(arg.replace("$it", &i))
             }
-        });
-
-        format!("{} {}", name, itertools::join(args, " "))
-    });
-
-    let mut process = Exec::shell(itertools::join(commands, " && "));
-
-    process = process.cwd(context.shell_manager.path()?);
-    trace!(target: "nu::run::external", "cwd = {:?}", context.shell_manager.path());
-
-    if !is_last {
-        process = process.stdout(subprocess::Redirection::Pipe);
-        trace!(target: "nu::run::external", "set up stdout pipe");
-    }
-
-    trace!(target: "nu::run::external", "built process {:?}", process);
-
-    let popen = process.detached().popen();
-    if let Ok(mut popen) = popen {
-        if is_last {
-            let _ = popen.wait();
-            Ok(None)
-        } else {
-            let stdout = popen.stdout.take().ok_or_else(|| {
-                ShellError::untagged_runtime_error("Can't redirect the stdout for external command")
-            })?;
-            let file = futures::io::AllowStdIo::new(stdout);
-            let stream = Framed::new(file, LinesCodec {});
-            let stream = stream.map(move |line| {
-                line.expect("Internal error: could not read lines of text from stdin")
-                    .into_value(&name_tag)
-            });
-            Ok(Some(stream.boxed().into()))
         }
-    } else {
+    };
-        Err(ShellError::labeled_error(
-            "Command not found",
-            "command not found",
-            name_tag,
-        ))
-    }
-}
 
-pub fn argument_contains_whitespace(argument: &str) -> bool {
+    Ok(Some(stream.to_input_stream()))
-    argument.chars().any(|c| c.is_whitespace())
-}
-
-pub fn argument_is_quoted(argument: &str) -> bool {
-    (argument.starts_with('"') && argument.ends_with('"')
-        || (argument.starts_with('\'') && argument.ends_with('\'')))
-}
-
-pub fn add_quotes(argument: &str) -> String {
-    format!("'{}'", argument)
-}
-
-pub fn remove_quotes(argument: &str) -> &str {
-    let size = argument.len();
-
-    &argument[1..size - 1]
 }
 
 async fn run_with_stdin(
@@ -205,32 +180,92 @@ async fn run_with_stdin(
     input: Option<InputStream>,
     is_last: bool,
 ) -> Result<Option<InputStream>, ShellError> {
-    let name_tag = command.name_tag;
+    let path = context.shell_manager.path()?;
-    let home_dir = dirs::home_dir();
+
+    let mut inputs: InputStream = if let Some(input) = input {
+        trace_stream!(target: "nu::trace_stream::external::stdin", "input" = input)
+    } else {
+        InputStream::empty()
+    };
+
+    let stream = async_stream! {
+        while let Some(value) = inputs.next().await {
+            let name = command.name.clone();
+            let name_tag = command.name_tag.clone();
+            let home_dir = dirs::home_dir();
+            let path = &path;
+            let args = command.args.clone();
+
+            let value_for_stdin = match nu_value_to_string_for_stdin(&command, &value) {
+                Ok(value) => value,
+                Err(reason) => {
+                    yield Ok(Value {
+                        value: UntaggedValue::Error(reason),
+                        tag: name_tag
+                    });
+                    return;
+                }
+            };
+
+            let process_args = args.iter().map(|arg| {
+                let arg = expand_tilde(arg.deref(), || home_dir.as_ref());
+                if let Some(unquoted) = remove_quotes(&arg) {
+                    unquoted.to_string()
+                } else {
+                    arg.as_ref().to_string()
+                }
+            }).collect::<Vec<String>>();
+
+            match spawn(&command, &path, &process_args[..], value_for_stdin, is_last).await {
+                Ok(res) => {
+                    if let Some(mut res) = res {
+                        while let Some(item) = res.next().await {
+                            yield Ok(item)
+                        }
+                    }
+                }
+                Err(reason) => {
+                    yield Ok(Value {
+                        value: UntaggedValue::Error(reason),
+                        tag: name_tag
+                    });
+                    return;
+                }
+            }
+        }
+    };
+
+    Ok(Some(stream.to_input_stream()))
+}
+
+async fn spawn(
+    command: &ExternalCommand,
+    path: &str,
+    args: &[String],
+    stdin_contents: Option<String>,
+    is_last: bool,
+) -> Result<Option<InputStream>, ShellError> {
+    let command = command.clone();
+    let name_tag = command.name_tag.clone();
 
     let mut process = Exec::cmd(&command.name);
 
-    for arg in command.args.iter() {
+    for arg in args {
-        // Let's also replace ~ as we shell out
+        process = process.arg(&arg);
-        let arg = shellexpand::tilde_with_context(arg.deref(), || home_dir.as_ref());
-
-        // Strip quotes from a quoted string
-        process = if arg.len() > 1 && (argument_is_quoted(&arg)) {
-            process.arg(remove_quotes(&arg))
-        } else {
-            process.arg(arg.as_ref())
-        };
     }
 
-    process = process.cwd(context.shell_manager.path()?);
+    process = process.cwd(path);
-    trace!(target: "nu::run::external", "cwd = {:?}", context.shell_manager.path());
+    trace!(target: "nu::run::external", "cwd = {:?}", &path);
 
+    // We want stdout regardless of what
+    // we are doing ($it case or pipe stdin)
     if !is_last {
         process = process.stdout(subprocess::Redirection::Pipe);
         trace!(target: "nu::run::external", "set up stdout pipe");
     }
 
-    if input.is_some() {
+    // open since we have some contents for stdin
+    if stdin_contents.is_some() {
         process = process.stdin(subprocess::Redirection::Pipe);
         trace!(target: "nu::run::external", "set up stdin pipe");
     }
@@ -238,56 +273,45 @@ async fn run_with_stdin(
     trace!(target: "nu::run::external", "built process {:?}", process);
 
     let popen = process.detached().popen();
+
     if let Ok(mut popen) = popen {
         let stream = async_stream! {
-            if let Some(mut input) = input {
+            if let Some(mut input) = stdin_contents.as_ref() {
-                let mut stdin_write = popen
+                let mut stdin_write = popen.stdin
-                    .stdin
                     .take()
                     .expect("Internal error: could not get stdin pipe for external command");
 
-                while let Some(item) = input.next().await {
+                if let Err(e) = stdin_write.write(input.as_bytes()) {
-                    match item.value {
+                    let message = format!("Unable to write to stdin (error = {})", e);
-                        UntaggedValue::Primitive(Primitive::Nothing) => {
-                            // If first in a pipeline, will receive Nothing. This is not an error.
-                        },
 
-                        UntaggedValue::Primitive(Primitive::String(s)) |
+                    yield Ok(Value {
-                            UntaggedValue::Primitive(Primitive::Line(s)) =>
+                        value: UntaggedValue::Error(ShellError::labeled_error(
-                        {
+                            message,
-                            if let Err(e) = stdin_write.write(s.as_bytes()) {
+                            "application may have closed before completing pipeline",
-                                let message = format!("Unable to write to stdin (error = {})", e);
+                            &name_tag)),
-                                yield Ok(Value {
+                        tag: name_tag
-                                    value: UntaggedValue::Error(ShellError::labeled_error(
+                    });
-                                        message,
+                    return;
-                                        "application may have closed before completing pipeline",
+                }
-                                        &name_tag,
-                                    )),
-                                    tag: name_tag,
-                                });
-                                return;
-                            }
-                        },
 
-                        // TODO serialize other primitives? https://github.com/nushell/nushell/issues/778
+                drop(stdin_write);
+            }
 
-                        v => {
+            if is_last && command.has_it_argument() {
-                            let message = format!("Received unexpected type from pipeline ({})", v.type_name());
+                if let Ok(status) = popen.wait() {
-                            yield Ok(Value {
+                    if status.success() {
-                                value: UntaggedValue::Error(ShellError::labeled_error(
+                        return;
-                                    message,
-                                    "expected a string",
-                                    &name_tag,
-                                )),
-                                tag: name_tag,
-                            });
-                            return;
-                        },
                     }
                 }
 
-                // Close stdin, which informs the external process that there's no more input
+                yield Ok(Value {
-                drop(stdin_write);
+                    value: UntaggedValue::Error(ShellError::labeled_error(
+                            "External command failed",
+                            "command failed",
+                            &name_tag)),
+                    tag: name_tag
+                });
+                return;
             }
 
             if !is_last {
@@ -295,20 +319,18 @@ async fn run_with_stdin(
                     stdout
                 } else {
                     yield Ok(Value {
-                        value: UntaggedValue::Error(
+                        value: UntaggedValue::Error(ShellError::labeled_error(
-                            ShellError::labeled_error(
+                            "Can't redirect the stdout for external command",
-                                "Can't redirect the stdout for external command",
+                            "can't redirect stdout",
-                                "can't redirect stdout",
+                            &name_tag)),
-                                &name_tag,
+                        tag: name_tag
-                            )
-                        ),
-                        tag: name_tag,
                     });
                     return;
                 };
 
                 let file = futures::io::AllowStdIo::new(stdout);
                 let stream = Framed::new(file, LinesCodec {});
+
                 let mut stream = stream.map(|line| {
                     if let Ok(line) = line {
                         line.into_value(&name_tag)
@@ -352,23 +374,58 @@ async fn run_with_stdin(
         Err(ShellError::labeled_error(
             "Command not found",
             "command not found",
-            name_tag,
+            &command.name_tag,
         ))
     }
 }
 
+fn expand_tilde<SI: ?Sized, P, HD>(input: &SI, home_dir: HD) -> std::borrow::Cow<str>
+where
+    SI: AsRef<str>,
+    P: AsRef<std::path::Path>,
+    HD: FnOnce() -> Option<P>,
+{
+    shellexpand::tilde_with_context(input, home_dir)
+}
+
+pub fn argument_contains_whitespace(argument: &str) -> bool {
+    argument.chars().any(|c| c.is_whitespace())
+}
+
+fn argument_is_quoted(argument: &str) -> bool {
+    if argument.len() < 2 {
+        return false;
+    }
+
+    (argument.starts_with('"') && argument.ends_with('"')
+        || (argument.starts_with('\'') && argument.ends_with('\'')))
+}
+
+fn add_quotes(argument: &str) -> String {
+    format!("'{}'", argument)
+}
+
+fn remove_quotes(argument: &str) -> Option<&str> {
+    if !argument_is_quoted(argument) {
+        return None;
+    }
+
+    let size = argument.len();
+
+    Some(&argument[1..size - 1])
+}
+
 #[cfg(test)]
 mod tests {
     use super::{
-        add_quotes, argument_contains_whitespace, argument_is_quoted, remove_quotes,
+        add_quotes, argument_contains_whitespace, argument_is_quoted, expand_tilde, remove_quotes,
         run_external_command, Context, OutputStream,
     };
     use futures::executor::block_on;
     use futures::stream::TryStreamExt;
     use nu_errors::ShellError;
-    use nu_parser::commands::classified::external::{ExternalArgs, ExternalCommand};
     use nu_protocol::{UntaggedValue, Value};
-    use nu_source::{Span, SpannedItem, Tag};
+    use nu_test_support::commands::ExternalBuilder;
 
     async fn read(mut stream: OutputStream) -> Option<Value> {
         match stream.try_next().await {
@@ -383,39 +440,28 @@ mod tests {
         }
     }
 
-    fn external(name: &str) -> ExternalCommand {
-        let mut path = nu_test_support::fs::binaries();
-        path.push(name);
-
-        let name = path.to_string_lossy().to_string().spanned(Span::unknown());
-
-        ExternalCommand {
-            name: name.to_string(),
-            name_tag: Tag {
-                anchor: None,
-                span: name.span,
-            },
-            args: ExternalArgs {
-                list: vec![],
-                span: name.span,
-            },
-        }
-    }
-
     async fn non_existent_run() -> Result<(), ShellError> {
-        let cmd = external("i_dont_exist.exe");
+        let cmd = ExternalBuilder::for_name("i_dont_exist.exe").build();
 
         let mut ctx = Context::basic().expect("There was a problem creating a basic context.");
 
-        assert!(run_external_command(cmd, &mut ctx, None, false)
+        let stream = run_external_command(cmd, &mut ctx, None, false)
-            .await
+            .await?
-            .is_err());
+            .expect("There was a problem running the external command.");
+
+        match read(stream.into()).await {
+            Some(Value {
+                value: UntaggedValue::Error(_),
+                ..
+            }) => {}
+            None | _ => panic!("Apparently a command was found (It's not supposed to be found)"),
+        }
 
         Ok(())
     }
 
     async fn failure_run() -> Result<(), ShellError> {
-        let cmd = external("fail");
+        let cmd = ExternalBuilder::for_name("fail").build();
 
         let mut ctx = Context::basic().expect("There was a problem creating a basic context.");
         let stream = run_external_command(cmd, &mut ctx, None, false)
@@ -452,6 +498,20 @@ mod tests {
 
     #[test]
     fn checks_quotes_from_argument_to_be_passed_in() {
+        assert_eq!(argument_is_quoted(""), false);
+
+        assert_eq!(argument_is_quoted("'"), false);
+        assert_eq!(argument_is_quoted("'a"), false);
+        assert_eq!(argument_is_quoted("a"), false);
+        assert_eq!(argument_is_quoted("a'"), false);
+        assert_eq!(argument_is_quoted("''"), true);
+
+        assert_eq!(argument_is_quoted(r#"""#), false);
+        assert_eq!(argument_is_quoted(r#""a"#), false);
+        assert_eq!(argument_is_quoted(r#"a"#), false);
+        assert_eq!(argument_is_quoted(r#"a""#), false);
+        assert_eq!(argument_is_quoted(r#""""#), true);
+
         assert_eq!(argument_is_quoted("'andrés"), false);
         assert_eq!(argument_is_quoted("andrés'"), false);
         assert_eq!(argument_is_quoted(r#""andrés"#), false);
@@ -468,7 +528,41 @@ mod tests {
 
     #[test]
     fn strips_quotes_from_argument_to_be_passed_in() {
-        assert_eq!(remove_quotes(r#"'andrés'"#), "andrés");
+        assert_eq!(remove_quotes(""), None);
-        assert_eq!(remove_quotes(r#""andrés""#), "andrés");
+
+        assert_eq!(remove_quotes("'"), None);
+        assert_eq!(remove_quotes("'a"), None);
+        assert_eq!(remove_quotes("a"), None);
+        assert_eq!(remove_quotes("a'"), None);
+        assert_eq!(remove_quotes("''"), Some(""));
+
+        assert_eq!(remove_quotes(r#"""#), None);
+        assert_eq!(remove_quotes(r#""a"#), None);
+        assert_eq!(remove_quotes(r#"a"#), None);
+        assert_eq!(remove_quotes(r#"a""#), None);
+        assert_eq!(remove_quotes(r#""""#), Some(""));
+
+        assert_eq!(remove_quotes("'andrés"), None);
+        assert_eq!(remove_quotes("andrés'"), None);
+        assert_eq!(remove_quotes(r#""andrés"#), None);
+        assert_eq!(remove_quotes(r#"andrés""#), None);
+        assert_eq!(remove_quotes("'andrés'"), Some("andrés"));
+        assert_eq!(remove_quotes(r#""andrés""#), Some("andrés"));
+    }
+
+    #[test]
+    fn expands_tilde_if_starts_with_tilde_character() {
+        assert_eq!(
+            expand_tilde("~", || Some(std::path::Path::new("the_path_to_nu_light"))),
+            "the_path_to_nu_light"
+        );
+    }
+
+    #[test]
+    fn does_not_expand_tilde_if_tilde_is_not_first_character() {
+        assert_eq!(
+            expand_tilde("1~1", || Some(std::path::Path::new("the_path_to_nu_light"))),
+            "1~1"
+        );
     }
 }

tests/main.rs

@@ -1,6 +1,6 @@
 extern crate nu_test_support;
 
 mod commands;
-mod converting_formats;
+mod format_conversions;
 mod plugins;
 mod shell;

tests/shell/mod.rs

@@ -1,71 +1 @@
-mod pipeline {
+mod pipeline;
-    use nu_test_support::{nu, nu_error};
-
-    #[test]
-    fn doesnt_break_on_utf8() {
-        let actual = nu!(cwd: ".", "echo ö");
-
-        assert_eq!(actual, "ö", "'{}' should contain ö", actual);
-    }
-
-    #[test]
-    fn can_process_stdout_of_external_piped_to_stdin_of_external() {
-        let actual = nu!(
-            cwd: ".",
-            r#"cococo "nushelll" | chop"#
-        );
-
-        assert_eq!(actual, "nushell");
-    }
-
-    #[test]
-    fn can_process_one_row_from_internal_piped_to_stdin_of_external() {
-        let actual = nu!(
-            cwd: ".",
-            r#"echo "nushelll" | chop"#
-        );
-
-        assert_eq!(actual, "nushell");
-    }
-
-    #[test]
-    fn shows_error_for_external_command_that_fails() {
-        let actual = nu_error!(
-            cwd: ".",
-            "fail"
-        );
-
-        assert!(actual.contains("External command failed"));
-    }
-
-    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: ".",
-                r#"
-                    cococo ~
-                "#
-            );
-
-            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: ".",
-                r#"
-                    cococo "1~1"
-                "#
-            );
-
-            assert_eq!(actual, "1~1");
-        }
-    }
-}

tests/shell/pipeline/commands/external.rs

@@ -0,0 +1,110 @@
+use nu_test_support::{nu, nu_error};
+
+#[test]
+fn shows_error_for_command_that_fails() {
+    let actual = nu_error!(
+        cwd: ".",
+        "fail"
+    );
+
+    assert!(actual.contains("External command failed"));
+}
+
+#[test]
+fn shows_error_for_command_not_found() {
+    let actual = nu_error!(
+        cwd: ".",
+        "ferris_is_not_here.exe"
+    );
+
+    assert!(actual.contains("Command not found"));
+}
+
+mod it_evaluation {
+    use super::nu;
+    use nu_test_support::fs::Stub::{EmptyFile, FileWithContentToBeTrimmed};
+    use nu_test_support::{pipeline, playground::Playground};
+
+    #[test]
+    fn takes_rows_of_nu_value_strings() {
+        Playground::setup("it_argument_test_1", |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
+                | cococo $it
+                | lines
+                | nth 1
+                | echo $it
+                "#
+            ));
+
+            assert_eq!(actual, "jonathan_likes_cake.txt");
+        })
+    }
+
+    #[test]
+    fn takes_rows_of_nu_value_lines() {
+        Playground::setup("it_argument_test_2", |dirs, sandbox| {
+            sandbox.with_files(vec![FileWithContentToBeTrimmed(
+                "nu_candies.txt",
+                r#"
+                    AndrásWithKitKatzz
+                    AndrásWithKitKatz
+                "#,
+            )]);
+
+            let actual = nu!(
+            cwd: dirs.test(), pipeline(
+            r#"
+                open nu_candies.txt
+                | lines
+                | chop $it
+                | lines
+                | nth 1
+                | echo $it
+                "#
+            ));
+
+            assert_eq!(actual, "AndrásWithKitKat");
+        })
+    }
+}
+
+mod tilde_expansion {
+    use super::nu;
+
+    #[test]
+    fn as_home_directory_when_passed_as_argument_and_begins_with_tilde() {
+        let actual = nu!(
+            cwd: ".",
+            r#"
+            cococo ~
+        "#
+        );
+
+        assert!(
+            !actual.contains('~'),
+            format!("'{}' should not contain ~", actual)
+        );
+    }
+
+    #[test]
+    fn does_not_expand_when_passed_as_argument_and_does_not_start_with_tilde() {
+        let actual = nu!(
+            cwd: ".",
+            r#"
+                    cococo "1~1"
+                "#
+        );
+
+        assert_eq!(actual, "1~1");
+    }
+}

tests/shell/pipeline/commands/internal.rs

@@ -0,0 +1,75 @@
+use nu_test_support::fs::Stub::FileWithContentToBeTrimmed;
+use nu_test_support::nu;
+use nu_test_support::{pipeline, playground::Playground};
+
+#[test]
+fn takes_rows_of_nu_value_strings_and_pipes_it_to_stdin_of_external() {
+    Playground::setup("internal_to_external_pipe_test_1", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "nu_times.csv",
+            r#"
+                name,rusty_luck,origin
+                Jason,1,Canada
+                Jonathan,1,New Zealand
+                Andrés,1,Ecuador
+                AndKitKatz,1,Estados Unidos
+            "#,
+        )]);
+
+        let actual = nu!(
+        cwd: dirs.test(), pipeline(
+        r#"
+            open nu_times.csv
+            | get name
+            | chop
+            | lines
+            | nth 3
+            | echo $it
+            "#
+        ));
+
+        assert_eq!(actual, "AndKitKat");
+    })
+}
+
+#[test]
+fn can_process_one_row_from_internal_and_pipes_it_to_stdin_of_external() {
+    let actual = nu!(
+        cwd: ".",
+        r#"echo "nushelll" | chop"#
+    );
+
+    assert_eq!(actual, "nushell");
+}
+
+mod tilde_expansion {
+    use super::nu;
+
+    #[test]
+    #[should_panic]
+    fn as_home_directory_when_passed_as_argument_and_begins_with_tilde() {
+        let actual = nu!(
+            cwd: ".",
+            r#"
+            echo ~
+        "#
+        );
+
+        assert!(
+            !actual.contains('~'),
+            format!("'{}' should not contain ~", actual)
+        );
+    }
+
+    #[test]
+    fn does_not_expand_when_passed_as_argument_and_does_not_start_with_tilde() {
+        let actual = nu!(
+            cwd: ".",
+            r#"
+                    echo "1~1"
+                "#
+        );
+
+        assert_eq!(actual, "1~1");
+    }
+}

tests/shell/pipeline/commands/mod.rs

@@ -0,0 +1,2 @@
+mod external;
+mod internal;

tests/shell/pipeline/mod.rs

@@ -0,0 +1,10 @@
+mod commands;
+
+use nu_test_support::nu;
+
+#[test]
+fn doesnt_break_on_utf8() {
+    let actual = nu!(cwd: ".", "echo ö");
+
+    assert_eq!(actual, "ö", "'{}' should contain ö", actual);
+}