External vars (#1615)

* fix empty table and missing spans

* wip

* WIP

* WIP

* working version with vars

* tidying

* WIP

* Fix external quoting issue

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

@@ -1,3 +1,4 @@
+use crate::evaluate::evaluate_baseline_expr;
 use crate::futures::ThreadedReceiver;
 use crate::prelude::*;
 
@@ -13,10 +14,9 @@ use futures_codec::FramedRead;
 use log::trace;
 
 use nu_errors::ShellError;
-use nu_protocol::hir::{ExternalArg, ExternalCommand};
+use nu_protocol::hir::ExternalCommand;
-use nu_protocol::{ColumnPath, Primitive, Scope, ShellTypeName, UntaggedValue, Value};
+use nu_protocol::{Primitive, Scope, ShellTypeName, UntaggedValue, Value};
-use nu_source::{Tag, Tagged};
+use nu_source::Tag;
-use nu_value_ext::as_column_path;
 
 pub enum StringOrBinary {
     String(String),
@@ -82,7 +82,7 @@ impl futures_codec::Decoder for MaybeTextCodec {
     }
 }
 
-pub fn nu_value_to_string(command: &ExternalCommand, from: &Value) -> Result<String, ShellError> {
+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))
@@ -91,7 +91,7 @@ pub fn nu_value_to_string(command: &ExternalCommand, from: &Value) -> Result<Str
         unsupported => Err(ShellError::labeled_error(
             format!("needs string data (given: {})", unsupported.type_name()),
             "expected a string",
-            &command.name_tag,
+            name_tag,
         )),
     }
 }
@@ -100,7 +100,7 @@ pub(crate) async fn run_external_command(
     command: ExternalCommand,
     context: &mut Context,
     input: InputStream,
-    _scope: &Scope,
+    scope: &Scope,
     is_last: bool,
 ) -> Result<InputStream, ShellError> {
     trace!(target: "nu::run::external", "-> {}", command.name);
@@ -114,62 +114,17 @@ pub(crate) async fn run_external_command(
     }
 
     if command.has_it_argument() {
-        run_with_iterator_arg(command, context, input, is_last)
+        run_with_iterator_arg(command, context, input, scope, is_last)
     } else {
-        run_with_stdin(command, context, input, is_last)
+        run_with_stdin(command, context, input, scope, is_last)
     }
 }
 
-fn prepare_column_path_for_fetching_it_variable(
-    argument: &ExternalArg,
-) -> Result<Tagged<ColumnPath>, ShellError> {
-    // We have "$it.[contents of interest]"
-    // and start slicing from "$it.[member+]"
-    //                             ^ here.
-    let key = nu_source::Text::from(argument.deref()).slice(4..argument.len());
-
-    to_column_path(&key, &argument.tag)
-}
-
-fn prepare_column_path_for_fetching_nu_variable(
-    argument: &ExternalArg,
-) -> Result<Tagged<ColumnPath>, ShellError> {
-    // We have "$nu.[contents of interest]"
-    // and start slicing from "$nu.[member+]"
-    //                             ^ here.
-    let key = nu_source::Text::from(argument.deref()).slice(4..argument.len());
-
-    to_column_path(&key, &argument.tag)
-}
-
-fn to_column_path(
-    path_members: &str,
-    tag: impl Into<Tag>,
-) -> Result<Tagged<ColumnPath>, ShellError> {
-    let tag = tag.into();
-
-    as_column_path(
-        &UntaggedValue::Table(
-            path_members
-                .split('.')
-                .map(|x| {
-                    let member = match x.parse::<u64>() {
-                        Ok(v) => UntaggedValue::int(v),
-                        Err(_) => UntaggedValue::string(x),
-                    };
-
-                    member.into_value(&tag)
-                })
-                .collect(),
-        )
-        .into_value(&tag),
-    )
-}
-
 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();
@@ -177,191 +132,47 @@ fn run_with_iterator_arg(
     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 {
-            let name = command.name.clone();
+            // Evaluate the expressions into values, and from values into strings for each iteration
-            let name_tag = command.name_tag.clone();
+            let mut command_args = vec![];
-            let home_dir = dirs::home_dir();
+            let scope = scope.clone().set_it(value);
-            let path = &path;
+            for arg in command.args.iter() {
-            let args = command.args.clone();
+                let value = evaluate_baseline_expr(arg, &context.registry, &scope)?;
+                command_args.push(nu_value_to_string(&name_tag, &value)?);
+            }
 
-            let it_replacement = {
+            let process_args = command_args
-                if command.has_it_argument() {
+            .iter()
-                    let empty_arg = ExternalArg {
+            .map(|arg| {
-                        arg: "".to_string(),
+                let arg = expand_tilde(arg.deref(), dirs::home_dir);
-                        tag: name_tag.clone()
-                    };
 
-                    let key = args.iter()
+                #[cfg(not(windows))]
-                        .find(|arg| arg.looks_like_it())
+                {
-                        .unwrap_or_else(|| &empty_arg);
+                    if argument_contains_whitespace(&arg) && argument_is_quoted(&arg) {
-
+                        if let Some(unquoted) = remove_quotes(&arg) {
-                    if args.iter().all(|arg| !arg.is_it()) {
+                            format!(r#""{}""#, unquoted)
-                        let key = match prepare_column_path_for_fetching_it_variable(&key) {
-                            Ok(keypath) => keypath,
-                            Err(reason) => {
-                                yield Ok(Value {
-                                    value: UntaggedValue::Error(reason),
-                                    tag: name_tag
-                                });
-                                return;
-                            }
-                        };
-
-                        match crate::commands::get::get_column_path(&key, &value) {
-                            Ok(field) => {
-                                match nu_value_to_string(&command, &field) {
-                                    Ok(val) => Some(val),
-                                    Err(reason) => {
-                                        yield Ok(Value {
-                                            value: UntaggedValue::Error(reason),
-                                            tag: name_tag
-                                        });
-                                        return;
-                                    },
-                                }
-                            },
-                            Err(reason) => {
-                                yield Ok(Value {
-                                    value: UntaggedValue::Error(reason),
-                                    tag: name_tag
-                                });
-                                return;
-                            }
-                        }
-                    } else {
-                        match nu_value_to_string(&command, &value) {
-                            Ok(val) => Some(val),
-                            Err(reason) => {
-                                yield Ok(Value {
-                                    value: UntaggedValue::Error(reason),
-                                    tag: name_tag
-                                });
-                                return;
-                            },
-                        }
-                    }
-                } else {
-                    None
-                }
-            };
-
-            let nu_replacement = {
-                if command.has_nu_argument() {
-                    let empty_arg = ExternalArg {
-                        arg: "".to_string(),
-                        tag: name_tag.clone()
-                    };
-
-                    let key = args.iter()
-                        .find(|arg| arg.looks_like_nu())
-                        .unwrap_or_else(|| &empty_arg);
-
-                    let nu_var = match crate::evaluate::variables::nu(&name_tag) {
-                        Ok(variables) => variables,
-                        Err(reason) => {
-                            yield Ok(Value {
-                                value: UntaggedValue::Error(reason),
-                                tag: name_tag
-                            });
-                            return;
-                        }
-                    };
-
-                    if args.iter().all(|arg| !arg.is_nu()) {
-                        let key = match prepare_column_path_for_fetching_nu_variable(&key) {
-                            Ok(keypath) => keypath,
-                            Err(reason) => {
-                                yield Ok(Value {
-                                    value: UntaggedValue::Error(reason),
-                                    tag: name_tag
-                                });
-                                return;
-                            }
-                        };
-
-                        match crate::commands::get::get_column_path(&key, &nu_var) {
-                            Ok(field) => {
-                                match nu_value_to_string(&command, &field) {
-                                    Ok(val) => Some(val),
-                                    Err(reason) => {
-                                        yield Ok(Value {
-                                            value: UntaggedValue::Error(reason),
-                                            tag: name_tag
-                                        });
-                                        return;
-                                    },
-                                }
-                            },
-                            Err(reason) => {
-                                yield Ok(Value {
-                                    value: UntaggedValue::Error(reason),
-                                    tag: name_tag
-                                });
-                                return;
-                            }
-                        }
-                    } else {
-                        match nu_value_to_string(&command, &nu_var) {
-                            Ok(val) => Some(val),
-                            Err(reason) => {
-                                yield Ok(Value {
-                                    value: UntaggedValue::Error(reason),
-                                    tag: name_tag
-                                });
-                                return;
-                            },
-                        }
-                    }
-                } else {
-                    None
-                }
-            };
-
-            let process_args = args.iter().filter_map(|arg| {
-                if arg.chars().all(|c| c.is_whitespace()) {
-                    None
-                } else {
-                    let arg = if arg.looks_like_it() {
-                        if let Some(mut value) = it_replacement.to_owned() {
-                            let mut value = expand_tilde(&value, || home_dir.as_ref()).as_ref().to_string();
-                            #[cfg(not(windows))]
-                            {
-                                value = {
-                                    if argument_contains_whitespace(&value) && !argument_is_quoted(&value) {
-                                        add_quotes(&value)
-                                    } else {
-                                        value
-                                    }
-                                };
-                            }
-                            Some(value)
                         } else {
-                            None
+                            arg.as_ref().to_string()
-                        }
-                    } else if arg.looks_like_nu() {
-                        if let Some(mut value) = nu_replacement.to_owned() {
-                            #[cfg(not(windows))]
-                            {
-                                value = {
-                                    if argument_contains_whitespace(&value) && !argument_is_quoted(&value) {
-                                        add_quotes(&value)
-                                    } else {
-                                        value
-                                    }
-                                };
-                            }
-                            Some(value)
-                        } else {
-                            None
                         }
                     } else {
-                        Some(arg.to_string())
+                        arg.as_ref().to_string()
-                    };
+                    }
-
-                    arg
                 }
-            }).collect::<Vec<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) => {
@@ -387,26 +198,28 @@ fn run_with_stdin(
     command: ExternalCommand,
     context: &mut Context,
     input: InputStream,
+    scope: &Scope,
     is_last: bool,
 ) -> Result<InputStream, ShellError> {
     let path = context.shell_manager.path();
 
     let input = trace_stream!(target: "nu::trace_stream::external::stdin", "input" = input);
 
-    let process_args = command
+    let mut command_args = vec![];
-        .args
+    for arg in command.args.iter() {
+        let value = evaluate_baseline_expr(arg, &context.registry, scope)?;
+        command_args.push(value.as_string()?);
+    }
+
+    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 argument_contains_whitespace(&arg) && !argument_is_quoted(&arg) {
-                    if let Some(unquoted) = remove_quotes(&arg) {
+                    add_quotes(&arg)
-                        format!(r#""{}""#, unquoted)
-                    } else {
-                        arg.as_ref().to_string()
-                    }
                 } else {
                     arg.as_ref().to_string()
                 }

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

@@ -6,10 +6,8 @@ use derive_new::new;
 use parking_lot::Mutex;
 
 use nu_errors::ShellError;
-use nu_protocol::hir::{
+use nu_protocol::hir::{Expression, ExternalArgs, ExternalCommand, Literal, SpannedExpression};
-    Expression, ExternalArg, ExternalArgs, ExternalCommand, Literal, SpannedExpression,
+use nu_protocol::{ReturnSuccess, Signature, SyntaxShape};
-};
-use nu_protocol::{ReturnSuccess, Scope, Signature, SyntaxShape};
 
 #[derive(Deserialize)]
 pub struct RunExternalArgs {}
@@ -17,13 +15,13 @@ pub struct RunExternalArgs {}
 #[derive(new)]
 pub struct RunExternalCommand;
 
-fn spanned_expression_to_string(expr: &SpannedExpression) -> String {
+fn spanned_expression_to_string(expr: SpannedExpression) -> String {
     if let SpannedExpression {
         expr: Expression::Literal(Literal::String(s)),
         ..
     } = expr
     {
-        s.clone()
+        s
     } else {
         "notacommand!!!".to_string()
     }
@@ -51,8 +49,9 @@ impl WholeStreamCommand for RunExternalCommand {
             ShellError::untagged_runtime_error("positional arguments unexpectedly empty")
         })?;
 
-        let mut command_args = positionals.iter();
+        let mut positionals = positionals.into_iter();
-        let name = command_args
+
+        let name = positionals
             .next()
             .map(spanned_expression_to_string)
             .ok_or_else(|| {
@@ -65,12 +64,7 @@ impl WholeStreamCommand for RunExternalCommand {
             name,
             name_tag: args.call_info.name_tag.clone(),
             args: ExternalArgs {
-                list: command_args
+                list: positionals.collect(),
-                    .map(|arg| ExternalArg {
-                        arg: spanned_expression_to_string(arg),
-                        tag: Tag::unknown_anchor(arg.span),
-                    })
-                    .collect(),
                 span: args.call_info.args.span,
             },
         };
@@ -97,11 +91,11 @@ impl WholeStreamCommand for RunExternalCommand {
                 current_errors: Arc::new(Mutex::new(vec![])),
             };
         }
+        let scope = args.call_info.scope.clone();
 
         let is_last = args.call_info.args.is_last;
         let input = args.input;
         let stream = async_stream! {
-            let scope = Scope::empty();
             let result = external::run_external_command(
                 command,
                 &mut external_context,

crates/nu-parser/src/parse.rs

@@ -1012,12 +1012,24 @@ fn classify_pipeline(
                 .name
                 .clone()
                 .map(|v| v.chars().skip(1).collect::<String>());
-            let name_span = name.span;
             // TODO this is the same as the `else` branch below, only the name differs. Find a way
             //      to share this functionality.
-            let name_iter = std::iter::once(name);
+            let mut args = vec![];
-            let args = name_iter.chain(lite_cmd.args.clone().into_iter());
+
-            let args = arguments_from_string_iter(args);
+            let (name, err) = parse_arg(SyntaxShape::String, registry, &name);
+            let name_span = name.span;
+            if error.is_none() {
+                error = err;
+            }
+            args.push(name);
+
+            for lite_arg in &lite_cmd.args {
+                let (expr, err) = parse_arg(SyntaxShape::String, registry, lite_arg);
+                if error.is_none() {
+                    error = err;
+                }
+                args.push(expr);
+            }
 
             commands.push(ClassifiedCommand::Internal(InternalCommand {
                 name: "run_external".to_string(),
@@ -1058,11 +1070,23 @@ fn classify_pipeline(
                 let trimmed = trim_quotes(&v);
                 expand_path(&trimmed)
             });
-            let name_span = name.span;
 
-            let name_iter = std::iter::once(name);
+            let mut args = vec![];
-            let args = name_iter.chain(lite_cmd.args.clone().into_iter());
+
-            let args = arguments_from_string_iter(args);
+            let (name, err) = parse_arg(SyntaxShape::String, registry, &name);
+            let name_span = name.span;
+            if error.is_none() {
+                error = err;
+            }
+            args.push(name);
+
+            for lite_arg in &lite_cmd.args {
+                let (expr, err) = parse_arg(SyntaxShape::String, registry, lite_arg);
+                if error.is_none() {
+                    error = err;
+                }
+                args.push(expr);
+            }
 
             commands.push(ClassifiedCommand::Internal(InternalCommand {
                 name: "run_external".to_string(),
@@ -1100,20 +1124,6 @@ pub fn classify_block(lite_block: &LiteBlock, registry: &dyn SignatureRegistry)
     ClassifiedBlock::new(block, error)
 }
 
-/// Parse out arguments from spanned expressions
-pub fn arguments_from_string_iter(
-    iter: impl Iterator<Item = Spanned<String>>,
-) -> Vec<SpannedExpression> {
-    iter.map(|v| {
-        // TODO parse_full_column_path
-        SpannedExpression {
-            expr: Expression::string(v.to_string()),
-            span: v.span,
-        }
-    })
-    .collect::<Vec<_>>()
-}
-
 /// Easy shorthand function to create a garbage expression at the given span
 pub fn garbage(span: Span) -> SpannedExpression {
     SpannedExpression::new(Expression::Garbage, span)

crates/nu-protocol/src/hir.rs

@@ -118,62 +118,26 @@ pub struct ExternalStringCommand {
     pub args: Vec<Spanned<String>>,
 }
 
-#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
-pub struct ExternalArg {
-    pub arg: String,
-    pub tag: Tag,
-}
-
-impl ExternalArg {
-    pub fn has(&self, name: &str) -> bool {
-        self.arg == name
-    }
-
-    pub fn is_it(&self) -> bool {
-        self.has("$it")
-    }
-
-    pub fn is_nu(&self) -> bool {
-        self.has("$nu")
-    }
-
-    pub fn looks_like_it(&self) -> bool {
-        self.arg.starts_with("$it") && (self.arg.starts_with("$it.") || self.is_it())
-    }
-
-    pub fn looks_like_nu(&self) -> bool {
-        self.arg.starts_with("$nu") && (self.arg.starts_with("$nu.") || self.is_nu())
-    }
-}
-
-impl std::ops::Deref for ExternalArg {
-    type Target = str;
-
-    fn deref(&self) -> &str {
-        &self.arg
-    }
-}
-
-#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
-pub struct ExternalArgs {
-    pub list: Vec<ExternalArg>,
-    pub span: Span,
-}
-
 impl ExternalArgs {
-    pub fn iter(&self) -> impl Iterator<Item = &ExternalArg> {
+    pub fn iter(&self) -> impl Iterator<Item = &SpannedExpression> {
         self.list.iter()
     }
 }
 
 impl std::ops::Deref for ExternalArgs {
-    type Target = [ExternalArg];
+    type Target = [SpannedExpression];
 
-    fn deref(&self) -> &[ExternalArg] {
+    fn deref(&self) -> &[SpannedExpression] {
         &self.list
     }
 }
 
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
+pub struct ExternalArgs {
+    pub list: Vec<SpannedExpression>,
+    pub span: Span,
+}
+
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
 pub struct ExternalCommand {
     pub name: String,
@@ -184,27 +148,21 @@ pub struct ExternalCommand {
 
 impl ExternalCommand {
     pub fn has_it_argument(&self) -> bool {
-        self.args.iter().any(|arg| arg.looks_like_it())
+        self.args.iter().any(|arg| match arg {
-    }
+            SpannedExpression {
-
+                expr: Expression::Path(path),
-    pub fn has_nu_argument(&self) -> bool {
+                ..
-        self.args.iter().any(|arg| arg.looks_like_nu())
+            } => match &**path {
-    }
+                Path { head, .. } => match head {
-}
+                    SpannedExpression {
-
+                        expr: Expression::Variable(Variable::It(_)),
-impl PrettyDebug for ExternalCommand {
+                        ..
-    fn pretty(&self) -> DebugDocBuilder {
+                    } => true,
-        b::typed(
+                    _ => false,
-            "external command",
+                },
-            b::description(&self.name)
+            },
-                + b::preceded(
+            _ => false,
-                    b::space(),
+        })
-                    b::intersperse(
-                        self.args.iter().map(|a| b::primitive(a.arg.to_string())),
-                        b::space(),
-                    ),
-                ),
-        )
     }
 }
 

crates/nu-protocol/src/value.rs

@@ -249,6 +249,9 @@ impl Value {
         match &self.value {
             UntaggedValue::Primitive(Primitive::String(string)) => Ok(string.clone()),
             UntaggedValue::Primitive(Primitive::Line(line)) => Ok(line.clone() + "\n"),
+            UntaggedValue::Primitive(Primitive::Path(path)) => {
+                Ok(path.to_string_lossy().to_string())
+            }
             _ => Err(ShellError::type_error("string", self.spanned_type_name())),
         }
     }

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

@@ -1,5 +1,5 @@
-use nu_protocol::hir::{ExternalArg, ExternalArgs, ExternalCommand};
+use nu_protocol::hir::{Expression, ExternalArgs, ExternalCommand, SpannedExpression};
-use nu_source::{Span, SpannedItem, Tag, TaggedItem};
+use nu_source::{Span, SpannedItem, Tag};
 
 pub struct ExternalBuilder {
     name: String,
@@ -28,13 +28,9 @@ impl ExternalBuilder {
         let args = self
             .args
             .iter()
-            .map(|arg| {
+            .map(|arg| SpannedExpression {
-                let arg = arg.tagged(Tag::unknown());
+                expr: Expression::string(arg.to_string()),
-
+                span: Span::unknown(),
-                ExternalArg {
-                    arg: arg.to_string(),
-                    tag: arg.tag,
-                }
             })
             .collect::<Vec<_>>();