Remove autoprinting of loop block values (#8618)

# Description

This removes autoprinting the final value of a loop, much in the same
spirit as not autoprinting values at the end of statements. As we fix
these corner cases, it becomes more consistent that to print to the
screen in a script, you use the `print` command.

This gives a noticeable performance improvement as a bonus.

Before:
```
C:\Source\nushell〉 for x in 1..10 { $x }
1
2
3
4
5
6
7
8
9
10
```
Now:
```
C:\Source\nushell〉 for x in 1..10 { $x }
C:\Source\nushell〉
```

# User-Facing Changes

**BREAKING CHANGE**

Loops like `for`, `loop`, and `while` will no longer automatically print
loop values to the screen.

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
This commit is contained in:
JT 2023-03-26 13:23:54 +13:00 committed by GitHub
parent d409171ba8
commit 5b03bca138
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 59 additions and 27 deletions

View file

@ -162,8 +162,7 @@ impl Command for For {
return Err(err);
}
Ok(pipeline) => {
let exit_code =
pipeline.print_not_formatted(&engine_state, false, false)?;
let exit_code = pipeline.drain_with_exit_code()?;
if exit_code != 0 {
break;
}

View file

@ -67,7 +67,7 @@ impl Command for Loop {
return Err(err);
}
Ok(pipeline) => {
let exit_code = pipeline.print(engine_state, stack, false, false)?;
let exit_code = pipeline.drain_with_exit_code()?;
if exit_code != 0 {
break;
}

View file

@ -77,8 +77,7 @@ impl Command for While {
return Err(err);
}
Ok(pipeline) => {
let exit_code =
pipeline.print_not_formatted(engine_state, false, false)?;
let exit_code = pipeline.drain_with_exit_code()?;
if exit_code != 0 {
break;
}

View file

@ -1,7 +1,7 @@
use nu_test_support::nu;
#[test]
fn for_auto_print_in_each_iteration() {
fn for_doesnt_auto_print_in_each_iteration() {
let actual = nu!(
cwd: ".",
r#"
@ -9,10 +9,9 @@ fn for_auto_print_in_each_iteration() {
echo 1
}"#
);
// Note: nu! macro auto replace "\n" and "\r\n" with ""
// so our output will be `11`
// that's ok, our main concern is it auto print value in each iteration.
assert_eq!(actual.out, "11");
// Make sure we don't see any of these values in the output
// As we do not auto-print loops anymore
assert!(!actual.out.contains('1'));
}
#[test]

View file

@ -1,7 +1,7 @@
use nu_test_support::nu;
#[test]
fn loop_auto_print_in_each_iteration() {
fn loop_doesnt_auto_print_in_each_iteration() {
let actual = nu!(
cwd: ".",
r#"
@ -15,10 +15,9 @@ fn loop_auto_print_in_each_iteration() {
echo 1
}"#
);
// Note: nu! macro auto replace "\n" and "\r\n" with ""
// so our output will be `111`
// that's ok, our main concern is it auto print value in each iteration.
assert_eq!(actual.out, "111");
// Make sure we don't see any of these values in the output
// As we do not auto-print loops anymore
assert!(!actual.out.contains('1'));
}
#[test]

View file

@ -11,15 +11,14 @@ fn while_sum() {
}
#[test]
fn while_auto_print_in_each_iteration() {
fn while_doesnt_auto_print_in_each_iteration() {
let actual = nu!(
cwd: ".",
"mut total = 0; while $total < 2 { $total = $total + 1; echo 1 }"
);
// Note: nu! macro auto replace "\n" and "\r\n" with ""
// so our output will be `11`
// that's ok, our main concern is it auto print value in each iteration.
assert_eq!(actual.out, "11");
// Make sure we don't see any of these values in the output
// As we do not auto-print loops anymore
assert!(!actual.out.contains('1'));
}
#[test]

View file

@ -220,6 +220,39 @@ impl PipelineData {
}
}
pub fn drain_with_exit_code(self) -> Result<i64, ShellError> {
match self {
PipelineData::Value(Value::Error { error }, _) => Err(*error),
PipelineData::Value(_, _) => Ok(0),
PipelineData::ListStream(stream, _) => {
stream.drain()?;
Ok(0)
}
PipelineData::ExternalStream {
stdout,
stderr,
exit_code,
..
} => {
if let Some(stdout) = stdout {
stdout.drain()?;
}
if let Some(stderr) = stderr {
stderr.drain()?;
}
if let Some(exit_code) = exit_code {
let result = drain_exit_code(exit_code)?;
Ok(result)
} else {
Ok(0)
}
}
PipelineData::Empty => Ok(0),
}
}
/// Try convert from self into iterator
///
/// It returns Err if the `self` cannot be converted to an iterator.
@ -854,18 +887,22 @@ pub fn print_if_stream(
// Make sure everything has finished
if let Some(exit_code) = exit_code {
let mut exit_codes: Vec<_> = exit_code.into_iter().collect();
return match exit_codes.pop() {
#[cfg(unix)]
Some(Value::Error { error }) => Err(*error),
Some(Value::Int { val, .. }) => Ok(val),
_ => Ok(0),
};
return drain_exit_code(exit_code);
}
Ok(0)
}
fn drain_exit_code(exit_code: ListStream) -> Result<i64, ShellError> {
let mut exit_codes: Vec<_> = exit_code.into_iter().collect();
match exit_codes.pop() {
#[cfg(unix)]
Some(Value::Error { error }) => Err(*error),
Some(Value::Int { val, .. }) => Ok(val),
_ => Ok(0),
}
}
impl Iterator for PipelineIterator {
type Item = Value;