From 29431e73c20bfafc9b48c2379aba83d581d275b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Thu, 16 Jan 2020 04:05:53 -0500 Subject: [PATCH] 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 | 2 + .../src/commands/classified/external.rs | 16 + crates/nu-test-support/Cargo.toml | 3 + crates/nu-test-support/src/commands.rs | 50 ++ crates/nu-test-support/src/dummies/chop.rs | 24 +- crates/nu-test-support/src/lib.rs | 1 + src/commands/classified/external.rs | 518 +++++++++++------- .../bson.rs | 0 .../csv.rs | 0 .../json.rs | 0 .../mod.rs | 0 .../ods.rs | 0 .../sqlite.rs | 0 .../ssv.rs | 0 .../toml.rs | 0 .../tsv.rs | 0 .../url.rs | 0 .../xlsx.rs | 0 .../yaml.rs | 0 tests/main.rs | 2 +- tests/shell/mod.rs | 72 +-- tests/shell/pipeline/commands/external.rs | 110 ++++ tests/shell/pipeline/commands/internal.rs | 75 +++ tests/shell/pipeline/commands/mod.rs | 2 + tests/shell/pipeline/mod.rs | 10 + 25 files changed, 600 insertions(+), 285 deletions(-) create mode 100644 crates/nu-test-support/src/commands.rs rename tests/{converting_formats => format_conversions}/bson.rs (100%) rename tests/{converting_formats => format_conversions}/csv.rs (100%) rename tests/{converting_formats => format_conversions}/json.rs (100%) rename tests/{converting_formats => format_conversions}/mod.rs (100%) rename tests/{converting_formats => format_conversions}/ods.rs (100%) rename tests/{converting_formats => format_conversions}/sqlite.rs (100%) rename tests/{converting_formats => format_conversions}/ssv.rs (100%) rename tests/{converting_formats => format_conversions}/toml.rs (100%) rename tests/{converting_formats => format_conversions}/tsv.rs (100%) rename tests/{converting_formats => format_conversions}/url.rs (100%) rename tests/{converting_formats => format_conversions}/xlsx.rs (100%) rename tests/{converting_formats => format_conversions}/yaml.rs (100%) create mode 100644 tests/shell/pipeline/commands/external.rs create mode 100644 tests/shell/pipeline/commands/internal.rs create mode 100644 tests/shell/pipeline/commands/mod.rs create mode 100644 tests/shell/pipeline/mod.rs diff --git a/Cargo.lock b/Cargo.lock index f990bf8b5e..86b71dcc6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2347,6 +2347,8 @@ dependencies = [ "getset", "glob", "nu-build", + "nu-parser", + "nu-source", "tempfile", ] diff --git a/crates/nu-parser/src/commands/classified/external.rs b/crates/nu-parser/src/commands/classified/external.rs index e776953da5..d385316236 100644 --- a/crates/nu-parser/src/commands/classified/external.rs +++ b/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( diff --git a/crates/nu-test-support/Cargo.toml b/crates/nu-test-support/Cargo.toml index 1d4a77e086..7223aefc68 100644 --- a/crates/nu-test-support/Cargo.toml +++ b/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" diff --git a/crates/nu-test-support/src/commands.rs b/crates/nu-test-support/src/commands.rs new file mode 100644 index 0000000000..0e49a8d030 --- /dev/null +++ b/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, +} + +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::>(); + + ExternalCommand { + name: name.to_string(), + name_tag: Tag::unknown(), + args: ExternalArgs { + list: args, + span: name.span, + }, + } + } +} diff --git a/crates/nu-test-support/src/dummies/chop.rs b/crates/nu-test-support/src/dummies/chop.rs index 4e9505b00e..5b765f6402 100644 --- a/crates/nu-test-support/src/dummies/chop.rs +++ b/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 = 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 +} diff --git a/crates/nu-test-support/src/lib.rs b/crates/nu-test-support/src/lib.rs index 184cfa0a4b..e2c7f7476a 100644 --- a/crates/nu-test-support/src/lib.rs +++ b/crates/nu-test-support/src/lib.rs @@ -1,3 +1,4 @@ +pub mod commands; pub mod fs; pub mod macros; pub mod playground; diff --git a/src/commands/classified/external.rs b/src/commands/classified/external.rs index f2de35729c..0427b06591 100644 --- a/src/commands/classified/external.rs +++ b/src/commands/classified/external.rs @@ -48,6 +48,39 @@ impl Decoder for LinesCodec { } } +pub fn nu_value_to_string(command: &ExternalCommand, from: &Value) -> Result { + 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, 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, ShellError> { trace!(target: "nu::run::external", "-> {}", command.name); - let has_it_arg = command.args.iter().any(|arg| arg.contains("$it")); - if has_it_arg { + if command.has_it_argument() { 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, is_last: bool, ) -> Result, ShellError> { - let name = command.name; - let args = command.args; - let name_tag = command.name_tag; - let inputs = input.unwrap_or_else(InputStream::empty).into_vec().await; + let path = context.shell_manager.path()?; - 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 - .iter() - .map(|i| match i { - Value { - value: UntaggedValue::Primitive(Primitive::Int(i)), - .. - } => Ok(i.to_string()), - Value { - value: UntaggedValue::Primitive(Primitive::String(s)), - .. - } - | Value { - value: UntaggedValue::Primitive(Primitive::Line(s)), - .. - } => Ok(s.clone()), - Value { - 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, - )) + 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 it_replacement = match nu_value_to_string(&command, &value) { + Ok(value) => value, + Err(reason) => { + yield Ok(Value { + value: UntaggedValue::Error(reason), + tag: name_tag + }); + return; } - } - }) - .map(|result| { - result.map(|value| { - if argument_contains_whitespace(&value) { - let arg = args.iter().find(|arg| arg.contains("$it")); - if let Some(arg) = arg { - if !argument_is_quoted(&arg) { - return add_quotes(&value); + }; + + let process_args = args.iter().filter_map(|arg| { + if arg.chars().all(|c| c.is_whitespace()) { + None + } else { + let arg = if arg.is_it() { + let value = it_replacement.to_owned(); + 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::>(); + + 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 - }) - }) - .collect::, ShellError>>()?; - - let home_dir = dirs::home_dir(); - 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)) + Err(reason) => { + yield Ok(Value { + value: UntaggedValue::Error(reason), + tag: name_tag + }); + return; + } } - }); - - 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 { - 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] + Ok(Some(stream.to_input_stream())) } async fn run_with_stdin( @@ -205,32 +180,92 @@ async fn run_with_stdin( input: Option, is_last: bool, ) -> Result, ShellError> { - let name_tag = command.name_tag; - let home_dir = dirs::home_dir(); + let path = context.shell_manager.path()?; + + 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::>(); + + 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, + is_last: bool, +) -> Result, 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() { - // Let's also replace ~ as we shell out - 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()) - }; + for arg in args { + process = process.arg(&arg); } - process = process.cwd(context.shell_manager.path()?); - trace!(target: "nu::run::external", "cwd = {:?}", context.shell_manager.path()); + process = process.cwd(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 { - let mut stdin_write = popen - .stdin + if let Some(mut input) = stdin_contents.as_ref() { + let mut stdin_write = popen.stdin .take() .expect("Internal error: could not get stdin pipe for external command"); - while let Some(item) = input.next().await { - match item.value { - UntaggedValue::Primitive(Primitive::Nothing) => { - // If first in a pipeline, will receive Nothing. This is not an error. - }, + if let Err(e) = stdin_write.write(input.as_bytes()) { + let message = format!("Unable to write to stdin (error = {})", e); - UntaggedValue::Primitive(Primitive::String(s)) | - UntaggedValue::Primitive(Primitive::Line(s)) => - { - if let Err(e) = stdin_write.write(s.as_bytes()) { - let message = format!("Unable to write to stdin (error = {})", e); - yield Ok(Value { - value: UntaggedValue::Error(ShellError::labeled_error( - message, - "application may have closed before completing pipeline", - &name_tag, - )), - tag: name_tag, - }); - return; - } - }, + yield Ok(Value { + value: UntaggedValue::Error(ShellError::labeled_error( + message, + "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 => { - let message = format!("Received unexpected type from pipeline ({})", v.type_name()); - yield Ok(Value { - value: UntaggedValue::Error(ShellError::labeled_error( - message, - "expected a string", - &name_tag, - )), - tag: name_tag, - }); - return; - }, + if is_last && command.has_it_argument() { + if let Ok(status) = popen.wait() { + if status.success() { + return; } } - // Close stdin, which informs the external process that there's no more input - drop(stdin_write); + yield Ok(Value { + 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( - ShellError::labeled_error( - "Can't redirect the stdout for external command", - "can't redirect stdout", - &name_tag, - ) - ), - tag: name_tag, + value: UntaggedValue::Error(ShellError::labeled_error( + "Can't redirect the stdout for external command", + "can't redirect stdout", + &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(input: &SI, home_dir: HD) -> std::borrow::Cow +where + SI: AsRef, + P: AsRef, + HD: FnOnce() -> Option

, +{ + 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 { 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) - .await - .is_err()); + let stream = run_external_command(cmd, &mut ctx, None, false) + .await? + .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(r#""andrés""#), "andrés"); + assert_eq!(remove_quotes(""), None); + + 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" + ); } } diff --git a/tests/converting_formats/bson.rs b/tests/format_conversions/bson.rs similarity index 100% rename from tests/converting_formats/bson.rs rename to tests/format_conversions/bson.rs diff --git a/tests/converting_formats/csv.rs b/tests/format_conversions/csv.rs similarity index 100% rename from tests/converting_formats/csv.rs rename to tests/format_conversions/csv.rs diff --git a/tests/converting_formats/json.rs b/tests/format_conversions/json.rs similarity index 100% rename from tests/converting_formats/json.rs rename to tests/format_conversions/json.rs diff --git a/tests/converting_formats/mod.rs b/tests/format_conversions/mod.rs similarity index 100% rename from tests/converting_formats/mod.rs rename to tests/format_conversions/mod.rs diff --git a/tests/converting_formats/ods.rs b/tests/format_conversions/ods.rs similarity index 100% rename from tests/converting_formats/ods.rs rename to tests/format_conversions/ods.rs diff --git a/tests/converting_formats/sqlite.rs b/tests/format_conversions/sqlite.rs similarity index 100% rename from tests/converting_formats/sqlite.rs rename to tests/format_conversions/sqlite.rs diff --git a/tests/converting_formats/ssv.rs b/tests/format_conversions/ssv.rs similarity index 100% rename from tests/converting_formats/ssv.rs rename to tests/format_conversions/ssv.rs diff --git a/tests/converting_formats/toml.rs b/tests/format_conversions/toml.rs similarity index 100% rename from tests/converting_formats/toml.rs rename to tests/format_conversions/toml.rs diff --git a/tests/converting_formats/tsv.rs b/tests/format_conversions/tsv.rs similarity index 100% rename from tests/converting_formats/tsv.rs rename to tests/format_conversions/tsv.rs diff --git a/tests/converting_formats/url.rs b/tests/format_conversions/url.rs similarity index 100% rename from tests/converting_formats/url.rs rename to tests/format_conversions/url.rs diff --git a/tests/converting_formats/xlsx.rs b/tests/format_conversions/xlsx.rs similarity index 100% rename from tests/converting_formats/xlsx.rs rename to tests/format_conversions/xlsx.rs diff --git a/tests/converting_formats/yaml.rs b/tests/format_conversions/yaml.rs similarity index 100% rename from tests/converting_formats/yaml.rs rename to tests/format_conversions/yaml.rs diff --git a/tests/main.rs b/tests/main.rs index b63f3faf42..ea20e52593 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -1,6 +1,6 @@ extern crate nu_test_support; mod commands; -mod converting_formats; +mod format_conversions; mod plugins; mod shell; diff --git a/tests/shell/mod.rs b/tests/shell/mod.rs index 4751001022..eab2e1f414 100644 --- a/tests/shell/mod.rs +++ b/tests/shell/mod.rs @@ -1,71 +1 @@ -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"); - } - } -} +mod pipeline; diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs new file mode 100644 index 0000000000..325397c54b --- /dev/null +++ b/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"); + } +} diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs new file mode 100644 index 0000000000..642039792c --- /dev/null +++ b/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"); + } +} diff --git a/tests/shell/pipeline/commands/mod.rs b/tests/shell/pipeline/commands/mod.rs new file mode 100644 index 0000000000..5b7aa7e195 --- /dev/null +++ b/tests/shell/pipeline/commands/mod.rs @@ -0,0 +1,2 @@ +mod external; +mod internal; diff --git a/tests/shell/pipeline/mod.rs b/tests/shell/pipeline/mod.rs new file mode 100644 index 0000000000..6fc8a40177 --- /dev/null +++ b/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); +}