diff --git a/crates/nu-command/src/system/exec.rs b/crates/nu-command/src/system/exec.rs index 2ead55cd77..35ab9428be 100644 --- a/crates/nu-command/src/system/exec.rs +++ b/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 diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index e0947ee967..0b29ca4b7e 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/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::{ - ast::{self, Expression}, - did_you_mean, - process::ChildProcess, - ByteStream, NuGlob, OutDest, -}; +use nu_protocol::{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 { - // 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>, ShellError> { let ctrlc = &engine_state.ctrlc; let cwd = engine_state.cwd(Some(stack))?; - let mut args: Vec> = vec![]; - for (expr, spread) in call.rest_iter(1) { - for arg in eval_argument(engine_state, stack, expr, spread)? { - match arg { - // Expand globs passed to run-external - Value::Glob { val, no_expand, .. } if !no_expand => args.extend( - expand_glob(&val, &cwd, expr.span, ctrlc)? - .into_iter() - .map(|s| s.into_spanned(expr.span)), - ), - other => { - args.push(OsString::from(coerce_into_string(other)?).into_spanned(expr.span)) - } - } + let eval_expression = get_eval_expression(engine_state); + let call_args = call.rest_iter_flattened(engine_state, stack, eval_expression, 1)?; + let mut args: Vec> = Vec::with_capacity(call_args.len()); + + for arg in call_args { + let span = arg.span(); + match arg { + // Expand globs passed to run-external + Value::Glob { val, no_expand, .. } if !no_expand => args.extend( + expand_glob(&val, &cwd, span, ctrlc)? + .into_iter() + .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 { +fn coerce_into_string(engine_state: &EngineState, val: Value) -> Result { 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, 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. /// diff --git a/crates/nu-engine/src/compile/mod.rs b/crates/nu-engine/src/compile/mod.rs index bc1e824ad1..a31a126b5d 100644 --- a/crates/nu-engine/src/compile/mod.rs +++ b/crates/nu-engine/src/compile/mod.rs @@ -1,7 +1,8 @@ use nu_protocol::{ ast::{ - Argument, Block, Call, CellPath, Expr, Expression, ListItem, Operator, PathMember, - Pipeline, PipelineRedirection, RecordItem, RedirectionSource, RedirectionTarget, + Argument, Block, Call, CellPath, Expr, Expression, ExternalArgument, ListItem, Operator, + 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 - 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)?; - } + move_in_reg_to_out_reg(builder)?; 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}") }