From ea8c4e3af28d778d98fd570800c74a3e19cddf29 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Wed, 10 Jul 2024 16:16:22 -0700 Subject: [PATCH] Make pipe redirections consistent, add `err>|` etc. forms (#13334) # Description Fixes the lexer to recognize `out>|`, `err>|`, `out+err>|`, etc. Previously only the short-style forms were recognized, which was inconsistent with normal file redirections. I also integrated it all more into the normal lex path by checking `|` in a special way, which should be more performant and consistent, and cleans up the code a bunch. Closes #13331. # User-Facing Changes - Adds `out>|` (error), `err>|`, `out+err>|`, `err+out>|` as recognized forms of the pipe redirection. # Tests + Formatting All passing. Added tests for the new forms. # After Submitting - [ ] release notes --- crates/nu-parser/src/lex.rs | 103 ++++++---------------- tests/shell/pipeline/commands/external.rs | 23 +++-- 2 files changed, 44 insertions(+), 82 deletions(-) diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index 9b52467ef4..3290a774f4 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -238,6 +238,10 @@ pub fn lex_item( Some(e), ); } + } else if c == b'|' && is_redirection(&input[token_start..*curr_offset]) { + // matches err>| etc. + *curr_offset += 1; + break; } else if is_item_terminator(&block_level, c, additional_whitespace, special_tokens) { break; } @@ -301,6 +305,16 @@ pub fn lex_item( contents: TokenContents::OutGreaterGreaterThan, span, }, + b"out>|" | b"o>|" => { + err = Some(ParseError::Expected( + "`|`. Redirecting stdout to a pipe is the same as normal piping.", + span, + )); + Token { + contents: TokenContents::Item, + span, + } + } b"err>" | b"e>" => Token { contents: TokenContents::ErrGreaterThan, span, @@ -309,6 +323,10 @@ pub fn lex_item( contents: TokenContents::ErrGreaterGreaterThan, span, }, + b"err>|" | b"e>|" => Token { + contents: TokenContents::ErrGreaterPipe, + span, + }, b"out+err>" | b"err+out>" | b"o+e>" | b"e+o>" => Token { contents: TokenContents::OutErrGreaterThan, span, @@ -317,6 +335,10 @@ pub fn lex_item( contents: TokenContents::OutErrGreaterGreaterThan, span, }, + b"out+err>|" | b"err+out>|" | b"o+e>|" | b"e+o>|" => Token { + contents: TokenContents::OutErrGreaterPipe, + span, + }, b"&&" => { err = Some(ParseError::ShellAndAnd(span)); Token { @@ -580,17 +602,6 @@ fn lex_internal( // If the next character is non-newline whitespace, skip it. curr_offset += 1; } else { - let (token, err) = try_lex_special_piped_item(input, &mut curr_offset, span_offset); - if error.is_none() { - error = err; - } - 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, @@ -609,68 +620,10 @@ 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, Option) { - let c = input[*curr_offset]; - let e_pipe_len = 3; - let eo_pipe_len = 5; - let o_pipe_len = 3; - 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), - )), - None, - ); - } - 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), - )), - None, - ); - } - } else if c == b'o' { - // indicates an error if user happened to type `o>|` - if offset + o_pipe_len <= input.len() && (&input[offset..offset + o_pipe_len] == b"o>|") { - return ( - None, - Some(ParseError::Expected( - "`|`. Redirecting stdout to a pipe is the same as normal piping.", - Span::new(span_offset + offset, span_offset + offset + o_pipe_len), - )), - ); - } - // 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, - ); - } - } - (None, None) +/// True if this the start of a redirection. Does not match `>>` or `>|` forms. +fn is_redirection(token: &[u8]) -> bool { + matches!( + token, + b"o>" | b"out>" | b"e>" | b"err>" | b"o+e>" | b"e+o>" | b"out+err>" | b"err+out>" + ) } diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index d138b65766..a6efe2b6c9 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -149,17 +149,26 @@ 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 }"#); +#[rstest::rstest] +#[case("err>|")] +#[case("e>|")] +fn basic_err_pipe_works(#[case] redirection: &str) { + let actual = nu!( + r#"with-env { FOO: "bar" } { nu --testbin echo_env_stderr FOO {redirection} str length }"# + .replace("{redirection}", redirection) + ); assert_eq!(actual.out, "3"); } -#[test] -fn basic_outerr_pipe_works() { +#[rstest::rstest] +#[case("out+err>|")] +#[case("err+out>|")] +#[case("o+e>|")] +#[case("e+o>|")] +fn basic_outerr_pipe_works(#[case] redirection: &str) { let actual = nu!( - r#"with-env { FOO: "bar" } { nu --testbin echo_env_mixed out-err FOO FOO o+e>| str length }"# + r#"with-env { FOO: "bar" } { nu --testbin echo_env_mixed out-err FOO FOO {redirection} str length }"# + .replace("{redirection}", redirection) ); assert_eq!(actual.out, "7"); }