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
This commit is contained in:
Devyn Cairns 2024-07-10 16:16:22 -07:00 committed by GitHub
parent 616e9faaf1
commit ea8c4e3af2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 44 additions and 82 deletions

View file

@ -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<Token>, Option<ParseError>) {
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>"
)
}

View file

@ -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");
}