Skip to content

Commit

Permalink
Support redirect stderr and stdout+stderr with a pipe (#11708)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
WindSoilder committed Feb 8, 2024
1 parent e7f1bf8 commit 58c6fea
Show file tree
Hide file tree
Showing 15 changed files with 519 additions and 105 deletions.
2 changes: 2 additions & 0 deletions crates/nu-cli/src/completions/completer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions crates/nu-cli/src/syntax_highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions crates/nu-command/src/formats/from/nuon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 20 additions & 0 deletions crates/nu-command/tests/commands/let_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
78 changes: 59 additions & 19 deletions crates/nu-command/tests/commands/redirection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
let actual = nu!(
cwd: dirs.test(),
r#"$env.BAZ = "message"; nu --testbin echo_env_mixed out-err BAZ BAZ err> tmp_file | str length"#,
);
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);
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"#,
);

// check it inside a function
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; print aa"#,
&format!("echo 3 {redirect_type} a.txt o+e>| str length")
);
assert!(actual.out.contains("messageaa"));
},
)
assert!(actual.err.contains("not allowed to use with redirection"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"
);
}

// 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]
Expand Down
Loading

0 comments on commit 58c6fea

Please sign in to comment.