support external calls

crates/nu-command/src/system/exec.rs

@@ -62,7 +62,7 @@ On Windows based systems, Nushell will wait for the command to finish and then e
         command.envs(envs);
 
         // Configure args.
-        let args = crate::eval_arguments_from_call(engine_state, stack, call.assert_ast_call()?)?;
+        let args = crate::eval_arguments_from_call(engine_state, stack, call)?;
         command.args(args.into_iter().map(|s| s.item));
 
         // Execute the child process, replacing/terminating the current process

crates/nu-command/src/system/run_external.rs

@@ -1,12 +1,7 @@
 use nu_cmd_base::hook::eval_hook;
 use nu_engine::{command_prelude::*, env_to_strings, get_eval_expression};
 use nu_path::{dots::expand_ndots, expand_tilde};
-use nu_protocol::{
+use nu_protocol::{did_you_mean, process::ChildProcess, ByteStream, NuGlob, OutDest};
-    ast::{self, Expression},
-    did_you_mean,
-    process::ChildProcess,
-    ByteStream, NuGlob, OutDest,
-};
 use nu_system::ForegroundChild;
 use nu_utils::IgnoreCaseExt;
 use pathdiff::diff_paths;
@@ -55,9 +50,6 @@ impl Command for External {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        // FIXME: this currently only works with AST calls, but I think #13089 totally fixes that
-        // so if I can merge that, this should just work fine
-        let call = call.assert_ast_call()?;
         let cwd = engine_state.cwd(Some(stack))?;
 
         let name: Value = call.req(engine_state, stack, 0)?;
@@ -225,24 +217,25 @@ impl Command for External {
 pub fn eval_arguments_from_call(
     engine_state: &EngineState,
     stack: &mut Stack,
-    call: &ast::Call,
+    call: &Call,
 ) -> Result<Vec<Spanned<OsString>>, ShellError> {
     let ctrlc = &engine_state.ctrlc;
     let cwd = engine_state.cwd(Some(stack))?;
-    let mut args: Vec<Spanned<OsString>> = vec![];
+    let eval_expression = get_eval_expression(engine_state);
-    for (expr, spread) in call.rest_iter(1) {
+    let call_args = call.rest_iter_flattened(engine_state, stack, eval_expression, 1)?;
-        for arg in eval_argument(engine_state, stack, expr, spread)? {
+    let mut args: Vec<Spanned<OsString>> = Vec::with_capacity(call_args.len());
-            match arg {
+
-                // Expand globs passed to run-external
+    for arg in call_args {
-                Value::Glob { val, no_expand, .. } if !no_expand => args.extend(
+        let span = arg.span();
-                    expand_glob(&val, &cwd, expr.span, ctrlc)?
+        match arg {
-                        .into_iter()
+            // Expand globs passed to run-external
-                        .map(|s| s.into_spanned(expr.span)),
+            Value::Glob { val, no_expand, .. } if !no_expand => args.extend(
-                ),
+                expand_glob(&val, &cwd, span, ctrlc)?
-                other => {
+                    .into_iter()
-                    args.push(OsString::from(coerce_into_string(other)?).into_spanned(expr.span))
+                    .map(|s| s.into_spanned(span)),
-                }
+            ),
-            }
+            other => args
+                .push(OsString::from(coerce_into_string(engine_state, other)?).into_spanned(span)),
         }
     }
     Ok(args)
@@ -250,42 +243,17 @@ pub fn eval_arguments_from_call(
 
 /// Custom `coerce_into_string()`, including globs, since those are often args to `run-external`
 /// as well
-fn coerce_into_string(val: Value) -> Result<String, ShellError> {
+fn coerce_into_string(engine_state: &EngineState, val: Value) -> Result<String, ShellError> {
     match val {
+        Value::List { .. } => Err(ShellError::CannotPassListToExternal {
+            arg: String::from_utf8_lossy(engine_state.get_span_contents(val.span())).into_owned(),
+            span: val.span(),
+        }),
         Value::Glob { val, .. } => Ok(val),
         _ => val.coerce_into_string(),
     }
 }
 
-/// Evaluate an argument, returning more than one value if it was a list to be spread.
-fn eval_argument(
-    engine_state: &EngineState,
-    stack: &mut Stack,
-    expr: &Expression,
-    spread: bool,
-) -> Result<Vec<Value>, ShellError> {
-    let eval = get_eval_expression(engine_state);
-    match eval(engine_state, stack, expr)? {
-        Value::List { vals, .. } => {
-            if spread {
-                Ok(vals)
-            } else {
-                Err(ShellError::CannotPassListToExternal {
-                    arg: String::from_utf8_lossy(engine_state.get_span_contents(expr.span)).into(),
-                    span: expr.span,
-                })
-            }
-        }
-        value => {
-            if spread {
-                Err(ShellError::CannotSpreadAsList { span: expr.span })
-            } else {
-                Ok(vec![value])
-            }
-        }
-    }
-}
-
 /// Performs glob expansion on `arg`. If the expansion found no matches or the pattern
 /// is not a valid glob, then this returns the original string as the expansion result.
 ///

crates/nu-engine/src/compile/mod.rs

@@ -1,7 +1,8 @@
 use nu_protocol::{
     ast::{
-        Argument, Block, Call, CellPath, Expr, Expression, ListItem, Operator, PathMember,
+        Argument, Block, Call, CellPath, Expr, Expression, ExternalArgument, ListItem, Operator,
-        Pipeline, PipelineRedirection, RecordItem, RedirectionSource, RedirectionTarget,
+        PathMember, Pipeline, PipelineRedirection, RecordItem, RedirectionSource,
+        RedirectionTarget,
     },
     engine::StateWorkingSet,
     ir::{DataSlice, Instruction, IrBlock, Literal, RedirectMode},
@@ -47,6 +48,13 @@ impl RedirectModes {
         }
     }
 
+    fn with_pipe_out(&self, span: Span) -> Self {
+        RedirectModes {
+            out: Some(RedirectMode::Pipe.into_spanned(span)),
+            err: self.err.clone(),
+        }
+    }
+
     fn with_capture_out(&self, span: Span) -> Self {
         RedirectModes {
             out: Some(RedirectMode::Capture.into_spanned(span)),
@@ -120,7 +128,9 @@ fn compile_pipeline(
         // element, then it's from whatever is passed in as the mode to use.
 
         let next_redirect_modes = if let Some(next_element) = iter.peek() {
+            // If there's a next element we always pipe out
             redirect_modes_of_expression(working_set, &next_element.expr, span)?
+                .with_pipe_out(next_element.pipe.unwrap_or(next_element.expr.span))
         } else {
             redirect_modes
                 .take()
@@ -280,6 +290,26 @@ fn compile_expression(
         builder.load_empty(out_reg)
     };
 
+    let move_in_reg_to_out_reg = |builder: &mut BlockBuilder| {
+        // Ensure that out_reg contains the input value, because a call only uses one register
+        if let Some(in_reg) = in_reg {
+            if in_reg != out_reg {
+                // Have to move in_reg to out_reg so it can be used
+                builder.push(
+                    Instruction::Move {
+                        dst: out_reg,
+                        src: in_reg,
+                    }
+                    .into_spanned(expr.span),
+                )?;
+            }
+        } else {
+            // Will have to initialize out_reg with Empty first
+            builder.load_empty(out_reg)?;
+        }
+        Ok(())
+    };
+
     match &expr.expr {
         Expr::Bool(b) => lit(builder, Literal::Bool(*b)),
         Expr::Int(i) => lit(builder, Literal::Int(*i)),
@@ -340,26 +370,15 @@ fn compile_expression(
         }
         Expr::VarDecl(_) => Err(CompileError::Todo("VarDecl")),
         Expr::Call(call) => {
-            // Ensure that out_reg contains the input value, because a call only uses one register
+            move_in_reg_to_out_reg(builder)?;
-            if let Some(in_reg) = in_reg {
-                if in_reg != out_reg {
-                    // Have to move in_reg to out_reg so it can be used
-                    builder.push(
-                        Instruction::Move {
-                            dst: out_reg,
-                            src: in_reg,
-                        }
-                        .into_spanned(call.head),
-                    )?;
-                }
-            } else {
-                // Will have to initialize out_reg with Empty first
-                builder.load_empty(out_reg)?;
-            }
 
             compile_call(working_set, builder, &call, redirect_modes, out_reg)
         }
-        Expr::ExternalCall(_, _) => Err(CompileError::Todo("ExternalCall")),
+        Expr::ExternalCall(head, args) => {
+            move_in_reg_to_out_reg(builder)?;
+
+            compile_external_call(working_set, builder, head, args, redirect_modes, out_reg)
+        }
         Expr::Operator(_) => Err(CompileError::Todo("Operator")),
         Expr::RowCondition(_) => Err(CompileError::Todo("RowCondition")),
         Expr::UnaryNot(subexpr) => {
@@ -769,6 +788,38 @@ fn compile_call(
     Ok(())
 }
 
+fn compile_external_call(
+    working_set: &StateWorkingSet,
+    builder: &mut BlockBuilder,
+    head: &Expression,
+    args: &[ExternalArgument],
+    redirect_modes: RedirectModes,
+    io_reg: RegId,
+) -> Result<(), CompileError> {
+    // Pass everything to run-external
+    let run_external_id = working_set
+        .find_decl(b"run-external")
+        .ok_or_else(|| CompileError::RunExternalNotFound)?;
+
+    let mut call = Call::new(head.span);
+    call.decl_id = run_external_id;
+
+    call.arguments.push(Argument::Positional(head.clone()));
+
+    for arg in args {
+        match arg {
+            ExternalArgument::Regular(expr) => {
+                call.arguments.push(Argument::Positional(expr.clone()));
+            }
+            ExternalArgument::Spread(expr) => {
+                call.arguments.push(Argument::Spread(expr.clone()));
+            }
+        }
+    }
+
+    compile_call(working_set, builder, &call, redirect_modes, io_reg)
+}
+
 /// Compile a call to `if` as a branch-if
 fn compile_if(
     working_set: &StateWorkingSet,
@@ -998,6 +1049,7 @@ pub enum CompileError {
     InvalidKeywordCall(&'static str, Span),
     SetBranchTargetOfNonBranchInstruction,
     InstructionIndexOutOfRange(usize),
+    RunExternalNotFound,
     Todo(&'static str),
 }
 
@@ -1024,6 +1076,9 @@ impl CompileError {
             CompileError::InstructionIndexOutOfRange(index) => {
                 format!("instruction index out of range: {index}")
             }
+            CompileError::RunExternalNotFound => {
+                "run-external is not supported here, so external calls can't be compiled".into()
+            }
             CompileError::Todo(msg) => {
                 format!("TODO: {msg}")
             }