extend it-expansion to externals (#1682)

* extend it-expansion to externals

* trim the carriage return for external strings

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

@@ -9,7 +9,7 @@ use std::sync::mpsc;
 
 use bytes::{BufMut, Bytes, BytesMut};
 use futures::executor::block_on_stream;
-use futures::stream::StreamExt;
+// use futures::stream::StreamExt;
 use futures_codec::FramedRead;
 use log::trace;
 
@@ -82,20 +82,6 @@ impl futures_codec::Decoder for MaybeTextCodec {
     }
 }
 
-pub fn nu_value_to_string(name_tag: &Tag, 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!("needs string data (given: {})", unsupported.type_name()),
-            "expected a string",
-            name_tag,
-        )),
-    }
-}
-
 pub(crate) async fn run_external_command(
     command: ExternalCommand,
     context: &mut Context,
@@ -113,85 +99,7 @@ pub(crate) async fn run_external_command(
         ));
     }
 
-    if command.has_it_argument() {
+    run_with_stdin(command, context, input, scope, is_last)
-        run_with_iterator_arg(command, context, input, scope, is_last)
-    } else {
-        run_with_stdin(command, context, input, scope, is_last)
-    }
-}
-
-fn run_with_iterator_arg(
-    command: ExternalCommand,
-    context: &mut Context,
-    input: InputStream,
-    scope: &Scope,
-    is_last: bool,
-) -> Result<InputStream, ShellError> {
-    let path = context.shell_manager.path();
-
-    let mut inputs: InputStream =
-        trace_stream!(target: "nu::trace_stream::external::it", "input" = input);
-
-    let name_tag = command.name_tag.clone();
-    let scope = scope.clone();
-    let context = context.clone();
-
-    let stream = async_stream! {
-        while let Some(value) = inputs.next().await {
-            // Evaluate the expressions into values, and from values into strings for each iteration
-            let mut command_args = vec![];
-            let scope = scope.clone().set_it(value);
-            for arg in command.args.iter() {
-                let value = evaluate_baseline_expr(arg, &context.registry, &scope)?;
-                command_args.push(nu_value_to_string(&name_tag, &value)?);
-            }
-
-            let process_args = command_args
-            .iter()
-            .map(|arg| {
-                let arg = expand_tilde(arg.deref(), dirs::home_dir);
-
-                #[cfg(not(windows))]
-                {
-                    if argument_contains_whitespace(&arg) && argument_is_quoted(&arg) {
-                        if let Some(unquoted) = remove_quotes(&arg) {
-                            format!(r#""{}""#, unquoted)
-                        } else {
-                            arg.as_ref().to_string()
-                        }
-                    } else {
-                        arg.as_ref().to_string()
-                    }
-                }
-                #[cfg(windows)]
-                {
-                    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[..], InputStream::empty(), is_last) {
-                Ok(mut 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(stream.to_input_stream())
 }
 
 fn run_with_stdin(
@@ -208,7 +116,7 @@ fn run_with_stdin(
     let mut command_args = vec![];
     for arg in command.args.iter() {
         let value = evaluate_baseline_expr(arg, &context.registry, scope)?;
-        command_args.push(value.as_string()?);
+        command_args.push(value.as_string()?.trim_end_matches('\n').to_string());
     }
 
     let process_args = command_args
@@ -514,6 +422,7 @@ fn add_quotes(argument: &str) -> String {
     format!("\"{}\"", argument)
 }
 
+#[allow(unused)]
 fn remove_quotes(argument: &str) -> Option<&str> {
     if !argument_is_quoted(argument) {
         return None;

crates/nu-cli/src/commands/each.rs

@@ -4,9 +4,11 @@ use crate::context::CommandRegistry;
 use crate::prelude::*;
 
 use futures::stream::once;
-
 use nu_errors::ShellError;
-use nu_protocol::{hir::Block, ReturnSuccess, Signature, SyntaxShape};
+use nu_protocol::{
+    hir::Block, hir::Expression, hir::SpannedExpression, hir::Synthetic, ReturnSuccess, Signature,
+    SyntaxShape,
+};
 
 pub struct Each;
 
@@ -41,6 +43,16 @@ impl WholeStreamCommand for Each {
     }
 }
 
+fn is_expanded_it_usage(head: &SpannedExpression) -> bool {
+    match &*head {
+        SpannedExpression {
+            expr: Expression::Synthetic(Synthetic::String(s)),
+            ..
+        } if s == "expanded-each" => true,
+        _ => false,
+    }
+}
+
 fn each(
     each_args: EachArgs,
     context: RunnableContext,
@@ -53,8 +65,13 @@ fn each(
     let stream = async_stream! {
         while let Some(input) = input_stream.next().await {
             let mut context = Context::from_raw(&raw_args, &registry);
+
             let input_clone = input.clone();
-            let input_stream = once(async { Ok(input) }).to_input_stream();
+            let input_stream = if is_expanded_it_usage(&raw_args.call_info.args.head) {
+                InputStream::empty()
+            } else {
+                once(async { Ok(input) }).to_input_stream()
+            };
 
             let result = run_block(
                 &block,

crates/nu-protocol/src/hir.rs

@@ -81,16 +81,6 @@ impl ClassifiedCommand {
     pub fn has_it_iteration(&self) -> bool {
         match self {
             ClassifiedCommand::Internal(command) => {
-                if let SpannedExpression {
-                    expr: Expression::Literal(Literal::String(s)),
-                    ..
-                } = &*command.args.head
-                {
-                    if s == "run_external" {
-                        // For now, don't it-expand externals
-                        return false;
-                    }
-                }
                 let mut result = command.args.head.has_shallow_it_usage();
 
                 if let Some(positionals) = &command.args.positional {
@@ -109,17 +99,6 @@ impl ClassifiedCommand {
     pub fn expand_it_usage(&mut self) {
         match self {
             ClassifiedCommand::Internal(command) => {
-                if let SpannedExpression {
-                    expr: Expression::Literal(Literal::String(s)),
-                    ..
-                } = &*command.args.head
-                {
-                    if s == "run_external" {
-                        // For now, don't it-expand externals
-                        return;
-                    }
-                }
-
                 if let Some(positionals) = &mut command.args.positional {
                     for arg in positionals {
                         if let SpannedExpression {
@@ -179,7 +158,9 @@ impl Commands {
                     name_span: self.span,
                     args: hir::Call {
                         head: Box::new(SpannedExpression {
-                            expr: Expression::Synthetic(Synthetic::String("each".to_string())),
+                            expr: Expression::Synthetic(Synthetic::String(
+                                "expanded-each".to_string(),
+                            )),
                             span: self.span,
                         }),
                         named: None,

tests/shell/pipeline/commands/external.rs

@@ -1,15 +1,5 @@
 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!(