From 58c6fea60b0e6d29c31c55ddef10a2c27cc0bf5c Mon Sep 17 00:00:00 2001 From: Wind Date: Fri, 9 Feb 2024 01:30:46 +0800 Subject: [PATCH] Support redirect stderr and stdout+stderr with a pipe (#11708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Close: #9673 Close: #8277 Close: #10944 This pr introduces the following syntax: 1. `e>|`, pipe stderr to next command. Example: `$env.FOO=bar nu --testbin echo_env_stderr FOO e>| str length` 2. `o+e>|` and `e+o>|`, pipe both stdout and stderr to next command, example: `$env.FOO=bar nu --testbin echo_env_mixed out-err FOO FOO e+o>| str length` Note: it only works for external commands. ~There is no different for internal commands, that is, the following three commands do the same things:~ Edit: it raises errors if we want to pipes for internal commands ``` ❯ ls e>| str length Error: × `e>|` only works with external streams ╭─[entry #1:1:1] 1 │ ls e>| str length · ─┬─ · ╰── `e>|` only works on external streams ╰──── ❯ ls e+o>| str length Error: × `o+e>|` only works with external streams ╭─[entry #2:1:1] 1 │ ls e+o>| str length · ──┬── · ╰── `o+e>|` only works on external streams ╰──── ``` This can help us to avoid some strange issues like the following: `$env.FOO=bar (nu --testbin echo_env_stderr FOO) e>| str length` Which is hard to understand and hard to explain to users. # User-Facing Changes Nan # Tests + Formatting To be done # After Submitting Maybe update documentation about these syntax. --- crates/nu-cli/src/completions/completer.rs | 2 + crates/nu-cli/src/syntax_highlight.rs | 2 + crates/nu-command/src/formats/from/nuon.rs | 2 + crates/nu-command/tests/commands/let_.rs | 20 ++ .../nu-command/tests/commands/redirection.rs | 80 +++-- crates/nu-engine/src/eval.rs | 332 ++++++++++++++---- crates/nu-parser/src/flatten.rs | 4 +- crates/nu-parser/src/lex.rs | 56 ++- crates/nu-parser/src/lite_parser.rs | 44 ++- crates/nu-parser/src/parse_keywords.rs | 4 +- crates/nu-parser/src/parser.rs | 36 +- crates/nu-protocol/src/ast/block.rs | 2 + crates/nu-protocol/src/ast/pipeline.rs | 14 +- tests/shell/pipeline/commands/external.rs | 16 + tests/shell/pipeline/commands/internal.rs | 12 + 15 files changed, 520 insertions(+), 106 deletions(-) diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index 87c51eabf6..917ffb9c7f 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -129,6 +129,8 @@ impl NuCompleter { for pipeline_element in pipeline.elements { match pipeline_element { PipelineElement::Expression(_, expr) + | PipelineElement::ErrPipedExpression(_, expr) + | PipelineElement::OutErrPipedExpression(_, expr) | PipelineElement::Redirection(_, _, expr, _) | PipelineElement::And(_, expr) | PipelineElement::Or(_, expr) diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index 078dfcc5d2..d263e9a6a0 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -264,6 +264,8 @@ fn find_matching_block_end_in_block( for e in &p.elements { match e { PipelineElement::Expression(_, e) + | PipelineElement::ErrPipedExpression(_, e) + | PipelineElement::OutErrPipedExpression(_, e) | PipelineElement::Redirection(_, _, e, _) | PipelineElement::And(_, e) | PipelineElement::Or(_, e) diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index d0a6df900f..c3a75057bf 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -124,6 +124,8 @@ impl Command for FromNuon { } else { match pipeline.elements.remove(0) { PipelineElement::Expression(_, expression) + | PipelineElement::ErrPipedExpression(_, expression) + | PipelineElement::OutErrPipedExpression(_, expression) | PipelineElement::Redirection(_, _, expression, _) | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) diff --git a/crates/nu-command/tests/commands/let_.rs b/crates/nu-command/tests/commands/let_.rs index 5fa2fcc505..8f510b908e 100644 --- a/crates/nu-command/tests/commands/let_.rs +++ b/crates/nu-command/tests/commands/let_.rs @@ -60,6 +60,26 @@ fn let_pipeline_redirects_externals() { assert_eq!(actual.out, "3"); } +#[test] +fn let_err_pipeline_redirects_externals() { + let actual = nu!( + r#"let x = with-env [FOO "foo"] {nu --testbin echo_env_stderr FOO e>| str length}; $x"# + ); + + // have an extra \n, so length is 4. + assert_eq!(actual.out, "4"); +} + +#[test] +fn let_outerr_pipeline_redirects_externals() { + let actual = nu!( + r#"let x = with-env [FOO "foo"] {nu --testbin echo_env_stderr FOO o+e>| str length}; $x"# + ); + + // have an extra \n, so length is 4. + assert_eq!(actual.out, "4"); +} + #[ignore] #[test] fn let_with_external_failed() { diff --git a/crates/nu-command/tests/commands/redirection.rs b/crates/nu-command/tests/commands/redirection.rs index 79c2e7f6ef..108deb7843 100644 --- a/crates/nu-command/tests/commands/redirection.rs +++ b/crates/nu-command/tests/commands/redirection.rs @@ -333,31 +333,71 @@ fn redirection_should_have_a_target() { } #[test] -fn redirection_with_pipe() { +fn redirection_with_out_pipe() { use nu_test_support::playground::Playground; - Playground::setup( - "external with many stdout and stderr messages", - |dirs, _| { - // check for stdout + Playground::setup("redirection with out pipes", |dirs, _| { + // check for stdout + let actual = nu!( + cwd: dirs.test(), + r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ err> tmp_file | str length"#, + ); + + assert_eq!(actual.out, "8"); + // check for stderr redirection file. + let expected_out_file = dirs.test().join("tmp_file"); + let actual_len = file_contents(expected_out_file).len(); + assert_eq!(actual_len, 8); + }) +} + +#[test] +fn redirection_with_err_pipe() { + use nu_test_support::playground::Playground; + Playground::setup("redirection with err pipe", |dirs, _| { + // check for stdout + let actual = nu!( + cwd: dirs.test(), + r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ out> tmp_file e>| str length"#, + ); + + assert_eq!(actual.out, "8"); + // check for stdout redirection file. + let expected_out_file = dirs.test().join("tmp_file"); + let actual_len = file_contents(expected_out_file).len(); + assert_eq!(actual_len, 8); + }) +} + +#[test] +fn no_redirection_with_outerr_pipe() { + Playground::setup("redirection does not accept outerr pipe", |dirs, _| { + for redirect_type in ["o>", "e>", "o+e>"] { let actual = nu!( cwd: dirs.test(), - r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ err> tmp_file | str length"#, + &format!("echo 3 {redirect_type} a.txt o+e>| str length") ); - - assert_eq!(actual.out, "8"); - // check for stderr redirection file. - let expected_out_file = dirs.test().join("tmp_file"); - let actual_len = file_contents(expected_out_file).len(); - assert_eq!(actual_len, 8); - - // check it inside a function - let actual = nu!( - cwd: dirs.test(), - r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ err> tmp_file; print aa"#, + assert!(actual.err.contains("not allowed to use with redirection")); + assert!( + !dirs.test().join("a.txt").exists(), + "No file should be created on error" ); - assert!(actual.out.contains("messageaa")); - }, - ) + } + + // test for separate redirection + let actual = nu!( + cwd: dirs.test(), + "echo 3 o> a.txt e> b.txt o+e>| str length" + ); + assert!(actual.err.contains("not allowed to use with redirection")); + assert!( + !dirs.test().join("a.txt").exists(), + "No file should be created on error" + ); + assert!( + !dirs.test().join("b.txt").exists(), + "No file should be created on error" + ); + }); } #[test] diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 05c6358cd7..b0d32fe33e 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -7,8 +7,8 @@ use nu_protocol::{ }, engine::{Closure, EngineState, Stack}, eval_base::Eval, - Config, DeclId, IntoPipelineData, PipelineData, ShellError, Span, Spanned, Type, Value, VarId, - ENV_VARIABLE_ID, + Config, DeclId, IntoPipelineData, PipelineData, RawStream, ShellError, Span, Spanned, Type, + Value, VarId, ENV_VARIABLE_ID, }; use std::thread::{self, JoinHandle}; use std::{borrow::Cow, collections::HashMap}; @@ -391,6 +391,7 @@ fn might_consume_external_result(input: PipelineData) -> (PipelineData, bool) { input.is_external_failed() } +#[allow(clippy::too_many_arguments)] fn eval_element_with_input( engine_state: &EngineState, stack: &mut Stack, @@ -398,17 +399,85 @@ fn eval_element_with_input( mut input: PipelineData, redirect_stdout: bool, redirect_stderr: bool, + redirect_combine: bool, stderr_writer_jobs: &mut Vec, ) -> Result<(PipelineData, bool), ShellError> { match element { - PipelineElement::Expression(_, expr) => eval_expression_with_input( - engine_state, - stack, - expr, - input, - redirect_stdout, - redirect_stderr, - ), + PipelineElement::Expression(pipe_span, expr) + | PipelineElement::OutErrPipedExpression(pipe_span, expr) => { + if matches!(element, PipelineElement::OutErrPipedExpression(..)) + && !matches!(input, PipelineData::ExternalStream { .. }) + { + return Err(ShellError::GenericError { + error: "`o+e>|` only works with external streams".into(), + msg: "`o+e>|` only works on external streams".into(), + span: *pipe_span, + help: None, + inner: vec![], + }); + } + match expr { + Expression { + expr: Expr::ExternalCall(head, args, is_subexpression), + .. + } if redirect_combine => { + let result = eval_external( + engine_state, + stack, + head, + args, + input, + RedirectTarget::CombinedPipe, + *is_subexpression, + )?; + Ok(might_consume_external_result(result)) + } + _ => eval_expression_with_input( + engine_state, + stack, + expr, + input, + redirect_stdout, + redirect_stderr, + ), + } + } + PipelineElement::ErrPipedExpression(pipe_span, expr) => { + let input = match input { + PipelineData::ExternalStream { + stdout, + stderr, + exit_code, + span, + metadata, + trim_end_newline, + } => PipelineData::ExternalStream { + stdout: stderr, // swap stderr and stdout to get stderr piped feature. + stderr: stdout, + exit_code, + span, + metadata, + trim_end_newline, + }, + _ => { + return Err(ShellError::GenericError { + error: "`e>|` only works with external streams".into(), + msg: "`e>|` only works on external streams".into(), + span: *pipe_span, + help: None, + inner: vec![], + }) + } + }; + eval_expression_with_input( + engine_state, + stack, + expr, + input, + redirect_stdout, + redirect_stderr, + ) + } PipelineElement::Redirection(span, redirection, expr, is_append_mode) => { match &expr.expr { Expr::String(_) @@ -420,32 +489,8 @@ fn eval_element_with_input( _ => None, }; - // when nushell get Stderr Redirection, we want to take `stdout` part of `input` - // so this stdout stream can be handled by next command. - let (input, out_stream) = match (redirection, input) { - ( - Redirection::Stderr, - PipelineData::ExternalStream { - stdout, - stderr, - exit_code, - span, - metadata, - trim_end_newline, - }, - ) => ( - PipelineData::ExternalStream { - stdout: stderr, - stderr: None, - exit_code, - span, - metadata, - trim_end_newline, - }, - Some(stdout), - ), - (_, input) => (input, None), - }; + let (input, result_out_stream, result_is_out) = + adjust_stream_for_input_and_output(input, redirection); if let Some(save_command) = engine_state.find_decl(b"save", &[]) { let save_call = gen_save_call( @@ -453,7 +498,7 @@ fn eval_element_with_input( (*span, expr.clone(), *is_append_mode), None, ); - match out_stream { + match result_out_stream { None => { eval_call(engine_state, stack, &save_call, input).map(|_| { // save is internal command, normally it exists with non-ExternalStream @@ -472,7 +517,7 @@ fn eval_element_with_input( }) }) } - Some(out_stream) => { + Some(result_out_stream) => { // delegate to a different thread // so nushell won't hang if external command generates both too much // stderr and stdout message @@ -484,11 +529,25 @@ fn eval_element_with_input( save_call, input, )); - + let (result_out_stream, result_err_stream) = if result_is_out { + (result_out_stream, None) + } else { + // we need `stdout` to be an empty RawStream + // so nushell knows this result is not the last part of a command. + ( + Some(RawStream::new( + Box::new(vec![].into_iter()), + None, + *span, + Some(0), + )), + result_out_stream, + ) + }; Ok(might_consume_external_result( PipelineData::ExternalStream { - stdout: out_stream, - stderr: None, + stdout: result_out_stream, + stderr: result_err_stream, exit_code, span: *span, metadata: None, @@ -582,6 +641,7 @@ fn eval_element_with_input( input, true, redirect_stderr, + redirect_combine, stderr_writer_jobs, ) .map(|x| x.0)? @@ -599,6 +659,7 @@ fn eval_element_with_input( input, redirect_stdout, redirect_stderr, + redirect_combine, stderr_writer_jobs, ) } @@ -621,6 +682,146 @@ fn eval_element_with_input( } } +// In redirection context, if nushell gets an ExternalStream +// it might want to take a stream from `input`(if `input` is `PipelineData::ExternalStream`) +// so this stream can be handled by next command. +// +// +// 1. get a stderr redirection, we need to take `stdout` out of `input`. +// e.g: nu --testbin echo_env FOO e> /dev/null | str length +// 2. get a stdout redirection, we need to take `stderr` out of `input`. +// e.g: nu --testbin echo_env FOO o> /dev/null e>| str length +// +// Returns 3 values: +// 1. adjusted pipeline data +// 2. a result stream which is taken from `input`, it can be handled in next command +// 3. a boolean value indicates if result stream should be a stdout stream. +fn adjust_stream_for_input_and_output( + input: PipelineData, + redirection: &Redirection, +) -> (PipelineData, Option>, bool) { + match (redirection, input) { + ( + Redirection::Stderr, + PipelineData::ExternalStream { + stdout, + stderr, + exit_code, + span, + metadata, + trim_end_newline, + }, + ) => ( + PipelineData::ExternalStream { + stdout: stderr, + stderr: None, + exit_code, + span, + metadata, + trim_end_newline, + }, + Some(stdout), + true, + ), + ( + Redirection::Stdout, + PipelineData::ExternalStream { + stdout, + stderr, + exit_code, + span, + metadata, + trim_end_newline, + }, + ) => ( + PipelineData::ExternalStream { + stdout, + stderr: None, + exit_code, + span, + metadata, + trim_end_newline, + }, + Some(stderr), + false, + ), + (_, input) => (input, None, true), + } +} + +fn is_redirect_stderr_required(elements: &[PipelineElement], idx: usize) -> bool { + let elements_length = elements.len(); + if idx < elements_length - 1 { + let next_element = &elements[idx + 1]; + match next_element { + PipelineElement::Redirection(_, Redirection::Stderr, _, _) + | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _, _) + | PipelineElement::SeparateRedirection { .. } + | PipelineElement::ErrPipedExpression(..) + | PipelineElement::OutErrPipedExpression(..) => return true, + PipelineElement::Redirection(_, Redirection::Stdout, _, _) => { + // a stderr redirection, but we still need to check for the next 2nd + // element, to handle for the following case: + // cat a.txt out> /dev/null e>| lines + // + // we only need to check the next 2nd element because we already make sure + // that we don't have duplicate err> like this: + // cat a.txt out> /dev/null err> /tmp/a + if idx < elements_length - 2 { + let next_2nd_element = &elements[idx + 2]; + if matches!(next_2nd_element, PipelineElement::ErrPipedExpression(..)) { + return true; + } + } + } + _ => {} + } + } + false +} + +fn is_redirect_stdout_required(elements: &[PipelineElement], idx: usize) -> bool { + let elements_length = elements.len(); + if idx < elements_length - 1 { + let next_element = &elements[idx + 1]; + match next_element { + // is next element a stdout relative redirection? + PipelineElement::Redirection(_, Redirection::Stdout, _, _) + | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _, _) + | PipelineElement::SeparateRedirection { .. } + | PipelineElement::Expression(..) + | PipelineElement::OutErrPipedExpression(..) => return true, + + PipelineElement::Redirection(_, Redirection::Stderr, _, _) => { + // a stderr redirection, but we still need to check for the next 2nd + // element, to handle for the following case: + // cat a.txt err> /dev/null | lines + // + // we only need to check the next 2nd element because we already make sure + // that we don't have duplicate err> like this: + // cat a.txt err> /dev/null err> /tmp/a + if idx < elements_length - 2 { + let next_2nd_element = &elements[idx + 2]; + if matches!(next_2nd_element, PipelineElement::Expression(..)) { + return true; + } + } + } + _ => {} + } + } + false +} + +fn is_redirect_combine_required(elements: &[PipelineElement], idx: usize) -> bool { + let elements_length = elements.len(); + idx < elements_length - 1 + && matches!( + &elements[idx + 1], + PipelineElement::OutErrPipedExpression(..) + ) +} + pub fn eval_block_with_early_return( engine_state: &EngineState, stack: &mut Stack, @@ -655,50 +856,26 @@ pub fn eval_block( for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { let mut stderr_writer_jobs = vec![]; let elements = &pipeline.elements; - let elements_length = elements.len(); + let elements_length = pipeline.elements.len(); for (idx, element) in elements.iter().enumerate() { let mut redirect_stdout = redirect_stdout; let mut redirect_stderr = redirect_stderr; - if !redirect_stderr && idx < elements_length - 1 { - let next_element = &elements[idx + 1]; - if matches!( - next_element, - PipelineElement::Redirection(_, Redirection::Stderr, _, _) - | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _, _) - | PipelineElement::SeparateRedirection { .. } - ) { - redirect_stderr = true; - } + if !redirect_stderr && is_redirect_stderr_required(elements, idx) { + redirect_stderr = true; } - if !redirect_stdout && idx < elements_length - 1 { - let next_element = &elements[idx + 1]; - match next_element { - // is next element a stdout relative redirection? - PipelineElement::Redirection(_, Redirection::Stdout, _, _) - | PipelineElement::Redirection(_, Redirection::StdoutAndStderr, _, _) - | PipelineElement::SeparateRedirection { .. } - | PipelineElement::Expression(..) => redirect_stdout = true, - - PipelineElement::Redirection(_, Redirection::Stderr, _, _) => { - // a stderr redirection, but we still need to check for the next 2nd - // element, to handle for the following case: - // cat a.txt err> /dev/null | lines - // - // we only need to check the next 2nd element because we already make sure - // that we don't have duplicate err> like this: - // cat a.txt err> /dev/null err> /tmp/a - if idx < elements_length - 2 { - let next_2nd_element = &elements[idx + 2]; - if matches!(next_2nd_element, PipelineElement::Expression(..)) { - redirect_stdout = true - } - } - } - _ => {} + if !redirect_stdout { + if is_redirect_stdout_required(elements, idx) { + redirect_stdout = true; } + } else if idx < elements_length - 1 + && matches!(elements[idx + 1], PipelineElement::ErrPipedExpression(..)) + { + redirect_stdout = false; } + let redirect_combine = is_redirect_combine_required(elements, idx); + // if eval internal command failed, it can just make early return with `Err(ShellError)`. let eval_result = eval_element_with_input( engine_state, @@ -707,6 +884,7 @@ pub fn eval_block( input, redirect_stdout, redirect_stderr, + redirect_combine, &mut stderr_writer_jobs, ); diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 19bcfab54a..f89d0afc32 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -560,7 +560,9 @@ pub fn flatten_pipeline_element( pipeline_element: &PipelineElement, ) -> Vec<(Span, FlatShape)> { match pipeline_element { - PipelineElement::Expression(span, expr) => { + PipelineElement::Expression(span, expr) + | PipelineElement::ErrPipedExpression(span, expr) + | PipelineElement::OutErrPipedExpression(span, expr) => { if let Some(span) = span { let mut output = vec![(*span, FlatShape::Pipe)]; output.append(&mut flatten_expression(working_set, expr)); diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index 9a48f67f4f..42f0fa85da 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -6,6 +6,8 @@ pub enum TokenContents { Comment, Pipe, PipePipe, + ErrGreaterPipe, + OutErrGreaterPipe, Semicolon, OutGreaterThan, OutGreaterGreaterThan, @@ -485,8 +487,14 @@ fn lex_internal( // If the next character is non-newline whitespace, skip it. curr_offset += 1; } else { - // Otherwise, try to consume an unclassified token. + let token = try_lex_special_piped_item(input, &mut curr_offset, span_offset); + if let Some(token) = token { + output.push(token); + is_complete = false; + continue; + } + // Otherwise, try to consume an unclassified token. let (token, err) = lex_item( input, &mut curr_offset, @@ -504,3 +512,49 @@ fn lex_internal( } (output, error) } + +/// trying to lex for the following item: +/// e>|, e+o>|, o+e>| +/// +/// It returns Some(token) if we find the item, or else return None. +fn try_lex_special_piped_item( + input: &[u8], + curr_offset: &mut usize, + span_offset: usize, +) -> Option { + let c = input[*curr_offset]; + let e_pipe_len = 3; + let eo_pipe_len = 5; + let offset = *curr_offset; + if c == b'e' { + // expect `e>|` + if (offset + e_pipe_len <= input.len()) && (&input[offset..offset + e_pipe_len] == b"e>|") { + *curr_offset += e_pipe_len; + return Some(Token::new( + TokenContents::ErrGreaterPipe, + Span::new(span_offset + offset, span_offset + offset + e_pipe_len), + )); + } + if (offset + eo_pipe_len <= input.len()) + && (&input[offset..offset + eo_pipe_len] == b"e+o>|") + { + *curr_offset += eo_pipe_len; + return Some(Token::new( + TokenContents::OutErrGreaterPipe, + Span::new(span_offset + offset, span_offset + offset + eo_pipe_len), + )); + } + } else if c == b'o' { + // it can be the following case: `o+e>|` + if (offset + eo_pipe_len <= input.len()) + && (&input[offset..offset + eo_pipe_len] == b"o+e>|") + { + *curr_offset += eo_pipe_len; + return Some(Token::new( + TokenContents::OutErrGreaterPipe, + Span::new(span_offset + offset, span_offset + offset + eo_pipe_len), + )); + } + } + None +} diff --git a/crates/nu-parser/src/lite_parser.rs b/crates/nu-parser/src/lite_parser.rs index e5c22af912..c27cc35f00 100644 --- a/crates/nu-parser/src/lite_parser.rs +++ b/crates/nu-parser/src/lite_parser.rs @@ -37,6 +37,12 @@ impl LiteCommand { #[derive(Debug)] pub enum LiteElement { Command(Option, LiteCommand), + // Similar to LiteElement::Command, except the previous command's output is stderr piped. + // e.g: `e>| cmd` + ErrPipedCommand(Option, LiteCommand), + // Similar to LiteElement::Command, except the previous command's output is stderr + stdout piped. + // e.g: `o+e>| cmd` + OutErrPipedCommand(Option, LiteCommand), // final field indicates if it's in append mode Redirection(Span, Redirection, LiteCommand, bool), // SeparateRedirection variant can only be generated by two different Redirection variant @@ -272,7 +278,9 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { last_connector = token.contents; last_connector_span = Some(token.span); } - TokenContents::Pipe => { + pipe_token @ (TokenContents::Pipe + | TokenContents::ErrGreaterPipe + | TokenContents::OutErrGreaterPipe) => { if let Some(err) = push_command_to( &mut curr_pipeline, curr_command, @@ -283,8 +291,8 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { } curr_command = LiteCommand::new(); - last_token = TokenContents::Pipe; - last_connector = TokenContents::Pipe; + last_token = *pipe_token; + last_connector = *pipe_token; last_connector_span = Some(token.span); } TokenContents::Eol => { @@ -429,7 +437,35 @@ fn push_command_to( is_append_mode, )) } - None => pipeline.push(LiteElement::Command(last_connector_span, command)), + None => { + if last_connector == TokenContents::ErrGreaterPipe { + pipeline.push(LiteElement::ErrPipedCommand(last_connector_span, command)) + } else if last_connector == TokenContents::OutErrGreaterPipe { + // Don't allow o+e>| along with redirection. + for cmd in &pipeline.commands { + if matches!( + cmd, + LiteElement::Redirection { .. } + | LiteElement::SameTargetRedirection { .. } + | LiteElement::SeparateRedirection { .. } + ) { + return Some(ParseError::LabeledError( + "`o+e>|` pipe is not allowed to use with redirection".into(), + "try to use different type of pipe, or remove redirection".into(), + last_connector_span + .expect("internal error: outerr pipe missing span information"), + )); + } + } + + pipeline.push(LiteElement::OutErrPipedCommand( + last_connector_span, + command, + )) + } else { + pipeline.push(LiteElement::Command(last_connector_span, command)) + } + } } None } else if get_redirection(last_connector).is_some() { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 86ad56badc..f4072682c2 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1702,7 +1702,9 @@ pub fn parse_module_block( for pipeline in output.block.iter() { if pipeline.commands.len() == 1 { match &pipeline.commands[0] { - LiteElement::Command(_, command) => { + LiteElement::Command(_, command) + | LiteElement::ErrPipedCommand(_, command) + | LiteElement::OutErrPipedCommand(_, command) => { let name = working_set.get_span_contents(command.parts[0]); match name { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index d13b21e7a0..327b1830f5 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1299,7 +1299,9 @@ fn parse_binary_with_base( } TokenContents::Pipe | TokenContents::PipePipe + | TokenContents::ErrGreaterPipe | TokenContents::OutGreaterThan + | TokenContents::OutErrGreaterPipe | TokenContents::OutGreaterGreaterThan | TokenContents::ErrGreaterThan | TokenContents::ErrGreaterGreaterThan @@ -5479,7 +5481,9 @@ pub fn parse_pipeline( for command in &pipeline.commands[1..] { match command { - LiteElement::Command(Some(pipe_span), command) => { + LiteElement::Command(Some(pipe_span), command) + | LiteElement::ErrPipedCommand(Some(pipe_span), command) + | LiteElement::OutErrPipedCommand(Some(pipe_span), command) => { new_command.parts.push(*pipe_span); new_command.comments.extend_from_slice(&command.comments); @@ -5584,6 +5588,18 @@ pub fn parse_pipeline( PipelineElement::Expression(*span, expr) } + LiteElement::ErrPipedCommand(span, command) => { + trace!("parsing: pipeline element: err piped command"); + let expr = parse_expression(working_set, &command.parts, is_subexpression); + + PipelineElement::ErrPipedExpression(*span, expr) + } + LiteElement::OutErrPipedCommand(span, command) => { + trace!("parsing: pipeline element: err piped command"); + let expr = parse_expression(working_set, &command.parts, is_subexpression); + + PipelineElement::OutErrPipedExpression(*span, expr) + } LiteElement::Redirection(span, redirection, command, is_append_mode) => { let expr = parse_value(working_set, command.parts[0], &SyntaxShape::Any); @@ -5639,6 +5655,8 @@ pub fn parse_pipeline( } else { match &pipeline.commands[0] { LiteElement::Command(_, command) + | LiteElement::ErrPipedCommand(_, command) + | LiteElement::OutErrPipedCommand(_, command) | LiteElement::Redirection(_, _, command, _) | LiteElement::SeparateRedirection { out: (_, command, _), @@ -5743,6 +5761,8 @@ pub fn parse_block( if pipeline.commands.len() == 1 { match &pipeline.commands[0] { LiteElement::Command(_, command) + | LiteElement::ErrPipedCommand(_, command) + | LiteElement::OutErrPipedCommand(_, command) | LiteElement::Redirection(_, _, command, _) | LiteElement::SeparateRedirection { out: (_, command, _), @@ -5836,6 +5856,8 @@ pub fn discover_captures_in_pipeline_element( ) -> Result<(), ParseError> { match element { PipelineElement::Expression(_, expression) + | PipelineElement::ErrPipedExpression(_, expression) + | PipelineElement::OutErrPipedExpression(_, expression) | PipelineElement::Redirection(_, _, expression, _) | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) => { @@ -6181,6 +6203,18 @@ fn wrap_element_with_collect( PipelineElement::Expression(span, expression) => { PipelineElement::Expression(*span, wrap_expr_with_collect(working_set, expression)) } + PipelineElement::ErrPipedExpression(span, expression) => { + PipelineElement::ErrPipedExpression( + *span, + wrap_expr_with_collect(working_set, expression), + ) + } + PipelineElement::OutErrPipedExpression(span, expression) => { + PipelineElement::OutErrPipedExpression( + *span, + wrap_expr_with_collect(working_set, expression), + ) + } PipelineElement::Redirection(span, redirection, expression, is_append_mode) => { PipelineElement::Redirection( *span, diff --git a/crates/nu-protocol/src/ast/block.rs b/crates/nu-protocol/src/ast/block.rs index 9e1f60ea80..66be0a83f3 100644 --- a/crates/nu-protocol/src/ast/block.rs +++ b/crates/nu-protocol/src/ast/block.rs @@ -68,6 +68,8 @@ impl Block { if let Some(last) = last.elements.last() { match last { PipelineElement::Expression(_, expr) => expr.ty.clone(), + PipelineElement::ErrPipedExpression(_, expr) => expr.ty.clone(), + PipelineElement::OutErrPipedExpression(_, expr) => expr.ty.clone(), PipelineElement::Redirection(_, _, _, _) => Type::Any, PipelineElement::SeparateRedirection { .. } => Type::Any, PipelineElement::SameTargetRedirection { .. } => Type::Any, diff --git a/crates/nu-protocol/src/ast/pipeline.rs b/crates/nu-protocol/src/ast/pipeline.rs index 28843db365..5a5f85d9f3 100644 --- a/crates/nu-protocol/src/ast/pipeline.rs +++ b/crates/nu-protocol/src/ast/pipeline.rs @@ -13,6 +13,8 @@ pub enum Redirection { #[derive(Debug, Clone, Serialize, Deserialize)] pub enum PipelineElement { Expression(Option, Expression), + ErrPipedExpression(Option, Expression), + OutErrPipedExpression(Option, Expression), // final field indicates if it's in append mode Redirection(Span, Redirection, Expression, bool), // final bool field indicates if it's in append mode @@ -32,7 +34,9 @@ pub enum PipelineElement { impl PipelineElement { pub fn expression(&self) -> &Expression { match self { - PipelineElement::Expression(_, expression) => expression, + PipelineElement::Expression(_, expression) + | PipelineElement::ErrPipedExpression(_, expression) + | PipelineElement::OutErrPipedExpression(_, expression) => expression, PipelineElement::Redirection(_, _, expression, _) => expression, PipelineElement::SeparateRedirection { out: (_, expression, _), @@ -50,11 +54,15 @@ impl PipelineElement { pub fn span(&self) -> Span { match self { PipelineElement::Expression(None, expression) + | PipelineElement::ErrPipedExpression(None, expression) + | PipelineElement::OutErrPipedExpression(None, expression) | PipelineElement::SameTargetRedirection { cmd: (None, expression), .. } => expression.span, PipelineElement::Expression(Some(span), expression) + | PipelineElement::ErrPipedExpression(Some(span), expression) + | PipelineElement::OutErrPipedExpression(Some(span), expression) | PipelineElement::Redirection(span, _, expression, _) | PipelineElement::SeparateRedirection { out: (span, expression, _), @@ -74,6 +82,8 @@ impl PipelineElement { pub fn has_in_variable(&self, working_set: &StateWorkingSet) -> bool { match self { PipelineElement::Expression(_, expression) + | PipelineElement::ErrPipedExpression(_, expression) + | PipelineElement::OutErrPipedExpression(_, expression) | PipelineElement::Redirection(_, _, expression, _) | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) @@ -96,6 +106,8 @@ impl PipelineElement { ) { match self { PipelineElement::Expression(_, expression) + | PipelineElement::ErrPipedExpression(_, expression) + | PipelineElement::OutErrPipedExpression(_, expression) | PipelineElement::Redirection(_, _, expression, _) | PipelineElement::And(_, expression) | PipelineElement::Or(_, expression) diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index b53ac0c8c4..96ceccab96 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -142,6 +142,22 @@ fn command_substitution_wont_output_extra_newline() { assert_eq!(actual.out, "bar"); } +#[test] +fn basic_err_pipe_works() { + let actual = nu!(r#"with-env [FOO "bar"] { nu --testbin echo_env_stderr FOO e>| str length }"#); + // there is a `newline` output from nu --testbin + assert_eq!(actual.out, "4"); +} + +#[test] +fn basic_outerr_pipe_works() { + let actual = nu!( + r#"with-env [FOO "bar"] { nu --testbin echo_env_mixed out-err FOO FOO o+e>| str length }"# + ); + // there is a `newline` output from nu --testbin + assert_eq!(actual.out, "8"); +} + mod it_evaluation { use super::nu; use nu_test_support::fs::Stub::{EmptyFile, FileWithContent, FileWithContentToBeTrimmed}; diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index f85f5e41b0..c1d0dc201a 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/tests/shell/pipeline/commands/internal.rs @@ -1113,6 +1113,18 @@ fn pipe_input_to_print() { assert!(actual.err.is_empty()); } +#[test] +fn err_pipe_input_to_print() { + let actual = nu!(r#""foo" e>| print"#); + assert!(actual.err.contains("only works on external streams")); +} + +#[test] +fn outerr_pipe_input_to_print() { + let actual = nu!(r#""foo" o+e>| print"#); + assert!(actual.err.contains("only works on external streams")); +} + #[test] fn command_not_found_error_shows_not_found_2() { let actual = nu!(r#"