Disable pipeline echo (#8292)

# Description

Change behavior of block evaluation to not print result of intermediate
commands.
Previously result of every but last pipeline in a block was printed to
stdout, and last one was returned

![image](https://user-images.githubusercontent.com/17511668/222550110-3f62fbed-432c-4b46-b9b1-4cb45a1f893e.png)
With this change results of intermediate pipelines are discarded after
they finish and the last one is returned as before:

![image](https://user-images.githubusercontent.com/17511668/222550346-f1e74f80-f6b6-4aa3-98d6-888ea4cb4915.png)
Now one should use `print` explicitly to print something to stdout

![image](https://user-images.githubusercontent.com/17511668/222923955-fda0d77b-41b4-4f91-a80f-12b0a1880c05.png)

**Note, that this behavior is not limited to functions!** The scope of
this change are all blocks. All of the below are executed as blocks and
thus exibited this behavior in the same way:

![image](https://user-images.githubusercontent.com/17511668/222924062-342c15de-4273-4bf5-8b39-fe6e3aa96076.png)

With this change outputs for all types of blocks are cleaned:

![image](https://user-images.githubusercontent.com/17511668/222924118-7d51c27e-04bb-43e5-8efe-38b484683bfe.png)


# User-Facing Changes

All types of blocks (function bodies, closures, `if` branches, `for` and
`loop` bodies e.t.c.) no longer print result of intermediate pipelines.

# 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

# 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:
Artemiy 2023-03-17 01:53:46 +03:00 committed by GitHub
parent 8543b0789d
commit 19beafa865
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 76 additions and 99 deletions

View file

@ -21,7 +21,7 @@ fn for_break_on_external_failed() {
cwd: ".", cwd: ".",
r#" r#"
for i in 1..2 { for i in 1..2 {
echo 1; print 1;
nu --testbin fail nu --testbin fail
}"# }"#
); );

View file

@ -33,7 +33,7 @@ fn loop_break_on_external_failed() {
} else { } else {
$total += 1; $total += 1;
} }
echo 1; print 1;
nu --testbin fail; nu --testbin fail;
}"# }"#
); );

View file

@ -262,10 +262,10 @@ fn test_open_block_command() {
r#" r#"
def "from blockcommandparser" [] { lines | split column ",|," } def "from blockcommandparser" [] { lines | split column ",|," }
let values = (open sample.blockcommandparser) let values = (open sample.blockcommandparser)
echo ($values | get column1 | get 0) print ($values | get column1 | get 0)
echo ($values | get column2 | get 0) print ($values | get column2 | get 0)
echo ($values | get column1 | get 1) print ($values | get column1 | get 1)
echo ($values | get column2 | get 1) print ($values | get column2 | get 1)
"# "#
); );

View file

@ -54,7 +54,7 @@ fn external_failed_should_be_caught() {
fn loop_try_break_should_be_successful() { fn loop_try_break_should_be_successful() {
let output = nu!( let output = nu!(
cwd: ".", cwd: ".",
"loop { try { echo 'successful'; break } catch { echo 'failed'; continue } }" "loop { try { print 'successful'; break } catch { print 'failed'; continue } }"
); );
assert_eq!(output.out, "successful"); assert_eq!(output.out, "successful");
@ -66,7 +66,7 @@ fn loop_catch_break_should_show_failed() {
cwd: ".", cwd: ".",
"loop { "loop {
try { invalid 1; try { invalid 1;
continue; } catch { echo 'failed'; break } continue; } catch { print 'failed'; break }
} }
" "
); );

View file

@ -26,7 +26,7 @@ fn while_auto_print_in_each_iteration() {
fn while_break_on_external_failed() { fn while_break_on_external_failed() {
let actual = nu!( let actual = nu!(
cwd: ".", cwd: ".",
"mut total = 0; while $total < 2 { $total = $total + 1; echo 1; nu --testbin fail }" "mut total = 0; while $total < 2 { $total = $total + 1; print 1; nu --testbin fail }"
); );
// Note: nu! macro auto replace "\n" and "\r\n" with "" // Note: nu! macro auto replace "\n" and "\r\n" with ""
// so our output will be `1` // so our output will be `1`

View file

@ -71,11 +71,11 @@ fn with_env_hides_variables_in_parent_scope() {
cwd: "tests/fixtures/formats", cwd: "tests/fixtures/formats",
r#" r#"
let-env FOO = "1" let-env FOO = "1"
echo $env.FOO print $env.FOO
with-env [FOO null] { with-env [FOO null] {
echo $env.FOO echo $env.FOO
} }
echo $env.FOO print $env.FOO
"# "#
); );
@ -88,9 +88,9 @@ fn with_env_shorthand_can_not_hide_variables() {
cwd: "tests/fixtures/formats", cwd: "tests/fixtures/formats",
r#" r#"
let-env FOO = "1" let-env FOO = "1"
echo $env.FOO print $env.FOO
FOO=null echo $env.FOO FOO=null print $env.FOO
echo $env.FOO print $env.FOO
"# "#
); );

View file

@ -6,10 +6,9 @@ use nu_protocol::{
Operator, PathMember, PipelineElement, Redirection, Operator, PathMember, PipelineElement, Redirection,
}, },
engine::{EngineState, ProfilingConfig, Stack}, engine::{EngineState, ProfilingConfig, Stack},
Config, DataSource, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, DataSource, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, PipelineMetadata,
PipelineMetadata, Range, ShellError, Span, Spanned, Unit, Value, VarId, ENV_VARIABLE_ID, Range, ShellError, Span, Spanned, Unit, Value, VarId, ENV_VARIABLE_ID,
}; };
use nu_utils::stdout_write_all_and_flush;
use std::collections::HashMap; use std::collections::HashMap;
use std::time::Instant; use std::time::Instant;
@ -1112,24 +1111,7 @@ pub fn eval_block(
} => { } => {
let exit_code = exit_code.take(); let exit_code = exit_code.take();
// Drain the input to the screen via tabular output input.drain()?;
let config = engine_state.get_config();
match engine_state.find_decl("table".as_bytes(), &[]) {
Some(decl_id) => {
let table = engine_state.get_decl(decl_id).run(
engine_state,
stack,
&Call::new(Span::new(0, 0)),
input,
)?;
print_or_return(table, config)?;
}
None => {
print_or_return(input, config)?;
}
};
if let Some(exit_code) = exit_code { if let Some(exit_code) = exit_code {
let mut v: Vec<_> = exit_code.collect(); let mut v: Vec<_> = exit_code.collect();
@ -1139,40 +1121,7 @@ pub fn eval_block(
} }
} }
} }
_ => { _ => input.drain()?,
// Drain the input to the screen via tabular output
let config = engine_state.get_config();
match engine_state.find_decl("table".as_bytes(), &[]) {
Some(decl_id) => {
let table = engine_state.get_decl(decl_id);
if let Some(block_id) = table.get_block_id() {
let block = engine_state.get_block(block_id);
eval_block(
engine_state,
stack,
block,
input,
redirect_stdout,
redirect_stderr,
)?;
} else {
let table = table.run(
engine_state,
stack,
&Call::new(Span::new(0, 0)),
input,
)?;
print_or_return(table, config)?;
}
}
None => {
print_or_return(input, config)?;
}
};
}
} }
input = PipelineData::empty() input = PipelineData::empty()
@ -1187,21 +1136,6 @@ pub fn eval_block(
} }
} }
fn print_or_return(pipeline_data: PipelineData, config: &Config) -> Result<(), ShellError> {
for item in pipeline_data {
if let Value::Error { error } = item {
return Err(*error);
}
let mut out = item.into_string("\n", config);
out.push('\n');
stdout_write_all_and_flush(out)?;
}
Ok(())
}
pub fn eval_subexpression( pub fn eval_subexpression(
engine_state: &EngineState, engine_state: &EngineState,
stack: &mut Stack, stack: &mut Stack,

View file

@ -200,6 +200,26 @@ impl PipelineData {
} }
} }
pub fn drain(self) -> Result<(), ShellError> {
match self {
PipelineData::Value(Value::Error { error }, _) => Err(*error),
PipelineData::Value(_, _) => Ok(()),
PipelineData::ListStream(stream, _) => stream.drain(),
PipelineData::ExternalStream { stdout, stderr, .. } => {
if let Some(stdout) = stdout {
stdout.drain()?;
}
if let Some(stderr) = stderr {
stderr.drain()?;
}
Ok(())
}
PipelineData::Empty => Ok(()),
}
}
/// Try convert from self into iterator /// Try convert from self into iterator
/// ///
/// It returns Err if the `self` cannot be converted to an iterator. /// It returns Err if the `self` cannot be converted to an iterator.

View file

@ -75,6 +75,20 @@ impl RawStream {
known_size: self.known_size, known_size: self.known_size,
} }
} }
pub fn drain(self) -> Result<(), ShellError> {
for next in self {
match next {
Ok(val) => {
if let Value::Error { error } = val {
return Err(*error);
}
}
Err(err) => return Err(err),
}
}
Ok(())
}
} }
impl Debug for RawStream { impl Debug for RawStream {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@ -201,6 +215,15 @@ impl ListStream {
.join(separator) .join(separator)
} }
pub fn drain(self) -> Result<(), ShellError> {
for next in self {
if let Value::Error { error } = next {
return Err(*error);
}
}
Ok(())
}
pub fn from_stream( pub fn from_stream(
input: impl Iterator<Item = Value> + Send + 'static, input: impl Iterator<Item = Value> + Send + 'static,
ctrlc: Option<Arc<AtomicBool>>, ctrlc: Option<Arc<AtomicBool>>,

View file

@ -1,2 +1,2 @@
echo [1 4] | length print ([1 4] | length)
echo [2 3 5] | length print (echo [2 3 5] | length)

View file

@ -48,7 +48,7 @@ fn treats_dot_dot_as_path_not_range() {
r#" r#"
mkdir temp; mkdir temp;
cd temp; cd temp;
echo (open ../nu_times.csv).name.0 | table; print (open ../nu_times.csv).name.0 | table;
cd ..; cd ..;
rmdir temp rmdir temp
"# "#
@ -385,9 +385,9 @@ fn let_env_hides_variable() {
cwd: ".", cwd: ".",
r#" r#"
let-env TESTENVVAR = "hello world" let-env TESTENVVAR = "hello world"
echo $env.TESTENVVAR print $env.TESTENVVAR
hide-env TESTENVVAR hide-env TESTENVVAR
echo $env.TESTENVVAR print $env.TESTENVVAR
"# "#
); );
@ -401,12 +401,12 @@ fn let_env_hides_variable_in_parent_scope() {
cwd: ".", cwd: ".",
r#" r#"
let-env TESTENVVAR = "hello world" let-env TESTENVVAR = "hello world"
echo $env.TESTENVVAR print $env.TESTENVVAR
do { do {
hide-env TESTENVVAR hide-env TESTENVVAR
echo $env.TESTENVVAR print $env.TESTENVVAR
} }
echo $env.TESTENVVAR print $env.TESTENVVAR
"# "#
); );
@ -446,14 +446,14 @@ fn unlet_variable_in_parent_scope() {
cwd: ".", cwd: ".",
r#" r#"
let-env DEBUG = "1" let-env DEBUG = "1"
echo $env.DEBUG print $env.DEBUG
do { do {
let-env DEBUG = "2" let-env DEBUG = "2"
echo $env.DEBUG print $env.DEBUG
hide-env DEBUG hide-env DEBUG
echo $env.DEBUG print $env.DEBUG
} }
echo $env.DEBUG print $env.DEBUG
"# "#
); );
@ -477,7 +477,7 @@ fn proper_shadow_let_env_aliases() {
let actual = nu!( let actual = nu!(
cwd: ".", cwd: ".",
r#" r#"
let-env DEBUG = "true"; echo $env.DEBUG | table; do { let-env DEBUG = "false"; echo $env.DEBUG } | table; echo $env.DEBUG let-env DEBUG = "true"; print $env.DEBUG | table; do { let-env DEBUG = "false"; print $env.DEBUG } | table; print $env.DEBUG
"# "#
); );
assert_eq!(actual.out, "truefalsetrue"); assert_eq!(actual.out, "truefalsetrue");
@ -526,7 +526,7 @@ fn proper_shadow_load_env_aliases() {
let actual = nu!( let actual = nu!(
cwd: ".", cwd: ".",
r#" r#"
let-env DEBUG = "true"; echo $env.DEBUG | table; do { echo {DEBUG: "false"} | load-env; echo $env.DEBUG } | table; echo $env.DEBUG let-env DEBUG = "true"; print $env.DEBUG | table; do { echo {DEBUG: "false"} | load-env; print $env.DEBUG } | table; print $env.DEBUG
"# "#
); );
assert_eq!(actual.out, "truefalsetrue"); assert_eq!(actual.out, "truefalsetrue");
@ -576,7 +576,7 @@ fn proper_shadow_let_aliases() {
let actual = nu!( let actual = nu!(
cwd: ".", cwd: ".",
r#" r#"
let DEBUG = false; echo $DEBUG | table; do { let DEBUG = true; echo $DEBUG } | table; echo $DEBUG let DEBUG = false; print $DEBUG | table; do { let DEBUG = true; print $DEBUG } | table; print $DEBUG
"# "#
); );
assert_eq!(actual.out, "falsetruefalse"); assert_eq!(actual.out, "falsetruefalse");