From 451a9c64d38017c48f8b3430cdc63d07b325981e Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Wed, 13 Sep 2023 06:35:01 +1200 Subject: [PATCH] Change `echo` to print when not redirected (#10338) # Description This changes `echo` to work more closely to what users of other shells would expect: * when redirected, `echo` works as before and sends values through the pipeline * when not redirected, `echo` will print values to the screen/terminal # User-Facing Changes A standalone `echo` now will print to the terminal, if not redirected. The `echo` command is no longer const eval-able, as it will now print to the terminal in some cases. # Tests + Formatting # After Submitting --- crates/nu-cmd-lang/src/core_commands/echo.rs | 36 ++++++++++---------- crates/nu-cmd-lang/src/core_commands/try_.rs | 2 +- crates/nu-command/tests/commands/echo.rs | 6 ---- crates/nu-command/tests/commands/for_.rs | 2 +- crates/nu-command/tests/commands/loop_.rs | 2 +- crates/nu-command/tests/commands/while_.rs | 2 +- tests/const_/mod.rs | 2 +- 7 files changed, 23 insertions(+), 29 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/echo.rs b/crates/nu-cmd-lang/src/core_commands/echo.rs index b3dad7f46e..01105569af 100644 --- a/crates/nu-cmd-lang/src/core_commands/echo.rs +++ b/crates/nu-cmd-lang/src/core_commands/echo.rs @@ -1,6 +1,6 @@ use nu_engine::CallExt; use nu_protocol::ast::Call; -use nu_protocol::engine::{Command, EngineState, Stack, StateWorkingSet}; +use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ Category, Example, ListStream, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, @@ -31,10 +31,6 @@ it returns it. Otherwise, it returns a list of the arguments. There is usually little reason to use this over just writing the values as-is."# } - fn is_const(&self) -> bool { - true - } - fn run( &self, engine_state: &EngineState, @@ -43,17 +39,7 @@ little reason to use this over just writing the values as-is."# _input: PipelineData, ) -> Result { let args = call.rest(engine_state, stack, 0); - run(engine_state, args, call) - } - - fn run_const( - &self, - working_set: &StateWorkingSet, - call: &Call, - _input: PipelineData, - ) -> Result { - let args = call.rest_const(working_set, 0); - run(working_set.permanent(), args, call) + run(engine_state, args, stack, call) } fn examples(&self) -> Vec { @@ -79,9 +65,10 @@ little reason to use this over just writing the values as-is."# fn run( engine_state: &EngineState, args: Result, ShellError>, + stack: &mut Stack, call: &Call, ) -> Result { - args.map(|to_be_echoed| { + let result = args.map(|to_be_echoed| { let n = to_be_echoed.len(); match n.cmp(&1usize) { // More than one value is converted in a stream of values @@ -96,7 +83,20 @@ fn run( // When there are no elements, we echo the empty string std::cmp::Ordering::Less => PipelineData::Value(Value::string("", call.head), None), } - }) + }); + + // If echo is not redirected, then print to the screen (to behave in a similar way to other shells) + if !call.redirect_stdout { + match result { + Ok(pipeline) => { + pipeline.print(engine_state, stack, false, false)?; + Ok(PipelineData::Empty) + } + Err(err) => Err(err), + } + } else { + result + } } #[cfg(test)] diff --git a/crates/nu-cmd-lang/src/core_commands/try_.rs b/crates/nu-cmd-lang/src/core_commands/try_.rs index eda1172003..9890384233 100644 --- a/crates/nu-cmd-lang/src/core_commands/try_.rs +++ b/crates/nu-cmd-lang/src/core_commands/try_.rs @@ -86,7 +86,7 @@ impl Command for Try { }, Example { description: "Try to run a missing command", - example: "try { asdfasdf } catch { echo 'missing' } ", + example: "try { asdfasdf } catch { 'missing' } ", result: Some(Value::test_string("missing")), }, ] diff --git a/crates/nu-command/tests/commands/echo.rs b/crates/nu-command/tests/commands/echo.rs index 1147f45df4..44e6d81459 100644 --- a/crates/nu-command/tests/commands/echo.rs +++ b/crates/nu-command/tests/commands/echo.rs @@ -34,9 +34,3 @@ fn echo_range_handles_exclusive_down() { assert_eq!(actual.out, "[3,2]"); } - -#[test] -fn echo_const() { - let actual = nu!("const x = (echo spam); $x"); - assert_eq!(actual.out, "spam"); -} diff --git a/crates/nu-command/tests/commands/for_.rs b/crates/nu-command/tests/commands/for_.rs index cd3cabfffd..a97c32c77b 100644 --- a/crates/nu-command/tests/commands/for_.rs +++ b/crates/nu-command/tests/commands/for_.rs @@ -4,7 +4,7 @@ use nu_test_support::nu; fn for_doesnt_auto_print_in_each_iteration() { let actual = nu!(" for i in 1..2 { - echo 1 + $i }"); // Make sure we don't see any of these values in the output // As we do not auto-print loops anymore diff --git a/crates/nu-command/tests/commands/loop_.rs b/crates/nu-command/tests/commands/loop_.rs index 1acf2b3c19..e786aa4225 100644 --- a/crates/nu-command/tests/commands/loop_.rs +++ b/crates/nu-command/tests/commands/loop_.rs @@ -10,7 +10,7 @@ fn loop_doesnt_auto_print_in_each_iteration() { } else { $total += 1; } - echo 1 + 1 }"); // Make sure we don't see any of these values in the output // As we do not auto-print loops anymore diff --git a/crates/nu-command/tests/commands/while_.rs b/crates/nu-command/tests/commands/while_.rs index 7b551185fd..541b4f905e 100644 --- a/crates/nu-command/tests/commands/while_.rs +++ b/crates/nu-command/tests/commands/while_.rs @@ -11,7 +11,7 @@ fn while_sum() { #[test] fn while_doesnt_auto_print_in_each_iteration() { - let actual = nu!("mut total = 0; while $total < 2 { $total = $total + 1; echo 1 }"); + let actual = nu!("mut total = 0; while $total < 2 { $total = $total + 1; 1 }"); // 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')); diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index cd0c0cd6fd..7e033d6412 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -333,7 +333,7 @@ fn describe_const() { #[test] fn ignore_const() { - let actual = nu!("const x = (echo spam | ignore); $x == null"); + let actual = nu!(r#"const x = ("spam" | ignore); $x == null"#); assert_eq!(actual.out, "true"); }