diff --git a/src/commands/classified.rs b/src/commands/classified.rs index c2380d4ffe..440413ddd4 100644 --- a/src/commands/classified.rs +++ b/src/commands/classified.rs @@ -225,7 +225,6 @@ impl ExternalCommand { ) -> Result { let stdin = input.stdin; let inputs: Vec> = input.objects.into_vec().await; - let name_tag = self.name_tag.clone(); trace!(target: "nu::run::external", "-> {}", self.name); trace!(target: "nu::run::external", "inputs = {:?}", inputs); @@ -235,53 +234,47 @@ impl ExternalCommand { arg_string.push_str(&arg); } + trace!(target: "nu::run::external", "command = {:?}", self.name); + let mut process; - - process = Exec::cmd(&self.name); - - trace!(target: "nu::run::external", "command = {:?}", process); - if arg_string.contains("$it") { - let mut first = true; - - for i in &inputs { - if i.as_string().is_err() { - let mut tag = None; - for arg in &self.args { - if arg.item.contains("$it") { - tag = Some(arg.tag()); + let input_strings = inputs + .iter() + .map(|i| { + i.as_string().map_err(|_| { + let arg = self.args.iter().find(|arg| arg.item.contains("$it")); + if let Some(arg) = arg { + ShellError::labeled_error( + "External $it needs string data", + "given row instead of string data", + arg.tag(), + ) + } else { + ShellError::labeled_error( + "Error: $it needs string data", + "given something else", + self.name_tag.clone(), + ) } - } - if let Some(tag) = tag { - return Err(ShellError::labeled_error( - "External $it needs string data", - "given row instead of string data", - tag, - )); - } else { - return Err(ShellError::labeled_error( - "Error: $it needs string data", - "given something else", - name_tag, - )); - } - } - if !first { - process = process.arg("&&"); - process = process.arg(&self.name); - } else { - first = false; - } + }) + }) + .collect::, ShellError>>()?; - for arg in &self.args { + let commands = input_strings.iter().map(|i| { + let args = self.args.iter().filter_map(|arg| { if arg.chars().all(|c| c.is_whitespace()) { - continue; + None + } else { + Some(arg.replace("$it", &i)) } + }); - process = process.arg(&arg.replace("$it", &i.as_string()?)); - } - } + format!("{} {}", self.name, itertools::join(args, " ")) + }); + + process = Exec::shell(itertools::join(commands, " && ")) } else { + process = Exec::cmd(&self.name); for arg in &self.args { let arg_chars: Vec<_> = arg.chars().collect(); if arg_chars.len() > 1 @@ -321,6 +314,7 @@ impl ExternalCommand { trace!(target: "nu::run::external", "next = {:?}", stream_next); + let name_tag = self.name_tag.clone(); if let Ok(mut popen) = popen { match stream_next { StreamNext::Last => { diff --git a/tests/commands_test.rs b/tests/commands_test.rs index 30636eafca..cfa6f74334 100644 --- a/tests/commands_test.rs +++ b/tests/commands_test.rs @@ -164,6 +164,28 @@ fn save_figures_out_intelligently_where_to_write_out_with_metadata() { }) } +#[test] +fn it_arg_works_with_many_inputs_to_external_command() { + Playground::setup("it_arg_works_with_many_inputs", |dirs, sandbox| { + sandbox.with_files(vec![ + FileWithContent("file1", "text"), + FileWithContent("file2", " and more text"), + ]); + + let (stdout, stderr) = nu_combined!( + cwd: dirs.test(), h::pipeline( + r#" + echo file1 file2 + | split-row " " + | cat $it + "# + )); + + assert_eq!("text and more text", stdout); + assert!(!stderr.contains("No such file or directory")); + }) +} + #[test] fn save_can_write_out_csv() { Playground::setup("save_test_2", |dirs, _| { diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index 199038b531..86c8a10e7f 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -155,6 +155,60 @@ macro_rules! nu_error { }}; } +#[macro_export] +macro_rules! nu_combined { + (cwd: $cwd:expr, $path:expr, $($part:expr),*) => {{ + use $crate::helpers::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::helpers::in_directory($cwd), + $crate::helpers::DisplayPath::display_path(&$path) + ); + + let mut process = Command::new(helpers::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) + }}; +} + pub enum Stub<'a> { FileWithContent(&'a str, &'a str), FileWithContentToBeTrimmed(&'a str, &'a str),