From ba4723cc9f77c1fa21f194ee894805258216208a Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Thu, 20 Jul 2023 19:56:46 +0800 Subject: [PATCH] Support variables/interpolation in `o>`, `e>`, `o+e>` redirect (#9747) # Description Fixes: #8517 Fixes: #9246 Fixes: #9709 Relative: #9723 ## About the change Before the pr, nushell only parse redirection target as a string(through `parse_string` call). In the pr, I'm trying to make the value more generic(using `parse_value` with `SyntaxShape::Any`) And during eval stage, we guard it to only eval `String`, `StringInterpolation`, `FullCellPath`, `FilePath`, so other type of redirection target like `1ms` won't be permitted. # User-Facing Changes After the pr: redirection support something like the following: 1. `let a = "x"; cat toolkit.nu o> $a` 2. `let a = "x"; cat toolkit.nu o> $"($a).txt"` 3. `cat toolkit.nu out> ("~/a.txt" | path expand)` --- .../nu-command/tests/commands/redirection.rs | 59 +++++++++++++++++++ crates/nu-engine/src/eval.rs | 16 ++++- crates/nu-parser/src/parser.rs | 15 +++-- 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/crates/nu-command/tests/commands/redirection.rs b/crates/nu-command/tests/commands/redirection.rs index 3582ff67ae..42e9164bbd 100644 --- a/crates/nu-command/tests/commands/redirection.rs +++ b/crates/nu-command/tests/commands/redirection.rs @@ -205,3 +205,62 @@ fn redirection_with_pipeline_works() { }, ) } + +#[test] +fn redirect_support_variable() { + Playground::setup("redirect_out_support_variable", |dirs, _sandbox| { + let output = nu!( + cwd: dirs.test(), + "let x = 'tmp_file'; echo 'hello' out> $x; open tmp_file" + ); + + assert!(output.out.contains("hello")); + + let output = nu!( + cwd: dirs.test(), + "let x = 'tmp_file'; echo 'hello' out+err> $x; open tmp_file" + ); + + assert!(output.out.contains("hello")); + }) +} + +#[test] +fn separate_redirection_support_variable() { + Playground::setup( + "external with both stdout and stderr messages, to different file", + |dirs, sandbox| { + let script_body = r#" + echo message + echo message 1>&2 + "#; + let expect_body = "message"; + + #[cfg(not(windows))] + { + sandbox.with_files(vec![FileWithContent("test.sh", script_body)]); + nu!( + cwd: dirs.test(), + r#"let o_f = "out.txt"; let e_f = "err.txt"; bash test.sh out> $o_f err> $e_f"# + ); + } + #[cfg(windows)] + { + sandbox.with_files(vec![FileWithContent("test.bat", script_body)]); + nu!( + cwd: dirs.test(), + r#"let o_f = "out.txt"; let e_f = "err.txt"; cmd /D /c test.bat out> $o_f err> $e_f"# + ); + } + // check for stdout redirection file. + let expected_out_file = dirs.test().join("out.txt"); + let actual = file_contents(expected_out_file); + assert!(actual.contains(expect_body)); + + // check for stderr redirection file. + let expected_err_file = dirs.test().join("err.txt"); + let actual = file_contents(expected_err_file); + assert!(actual.contains(expect_body)); + }, + ) +} diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 775689de08..5fcc880e98 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -795,7 +795,10 @@ pub fn eval_element_with_input( redirect_stderr, ), PipelineElement::Redirection(span, redirection, expr) => match &expr.expr { - Expr::String(_) => { + Expr::String(_) + | Expr::FullCellPath(_) + | Expr::StringInterpolation(_) + | Expr::Filepath(_) => { let exit_code = match &mut input { PipelineData::ExternalStream { exit_code, .. } => exit_code.take(), _ => None, @@ -880,7 +883,16 @@ pub fn eval_element_with_input( out: (out_span, out_expr), err: (err_span, err_expr), } => match (&out_expr.expr, &err_expr.expr) { - (Expr::String(_), Expr::String(_)) => { + ( + Expr::String(_) + | Expr::FullCellPath(_) + | Expr::StringInterpolation(_) + | Expr::Filepath(_), + Expr::String(_) + | Expr::FullCellPath(_) + | Expr::StringInterpolation(_) + | Expr::Filepath(_), + ) => { if let Some(save_command) = engine_state.find_decl(b"save", &[]) { let exit_code = match &mut input { PipelineData::ExternalStream { exit_code, .. } => exit_code.take(), diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 2cc48d3122..cc82bde3b3 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5522,8 +5522,7 @@ pub fn parse_pipeline( PipelineElement::Expression(*span, expr) } LiteElement::Redirection(span, redirection, command) => { - trace!("parsing: pipeline element: redirection"); - let expr = parse_string(working_set, command.parts[0]); + let expr = parse_value(working_set, command.parts[0], &SyntaxShape::Any); PipelineElement::Redirection(*span, redirection.clone(), expr) } @@ -5532,9 +5531,11 @@ pub fn parse_pipeline( err: (err_span, err_command), } => { trace!("parsing: pipeline element: separate redirection"); - let out_expr = parse_string(working_set, out_command.parts[0]); + let out_expr = + parse_value(working_set, out_command.parts[0], &SyntaxShape::Any); - let err_expr = parse_string(working_set, err_command.parts[0]); + let err_expr = + parse_value(working_set, err_command.parts[0], &SyntaxShape::Any); PipelineElement::SeparateRedirection { out: (*out_span, out_expr), @@ -5547,7 +5548,8 @@ pub fn parse_pipeline( } => { trace!("parsing: pipeline element: same target redirection"); let expr = parse_expression(working_set, &command.parts, is_subexpression); - let redirect_expr = parse_string(working_set, redirect_command.parts[0]); + let redirect_expr = + parse_value(working_set, redirect_command.parts[0], &SyntaxShape::Any); PipelineElement::SameTargetRedirection { cmd: (*cmd_span, expr), redirection: (*redirect_span, redirect_expr), @@ -5636,7 +5638,8 @@ pub fn parse_pipeline( trace!("parsing: pipeline element: same target redirection"); let expr = parse_expression(working_set, &command.parts, is_subexpression); - let redirect_expr = parse_string(working_set, redirect_cmd.parts[0]); + let redirect_expr = + parse_value(working_set, redirect_cmd.parts[0], &SyntaxShape::Any); Pipeline { elements: vec![PipelineElement::SameTargetRedirection {