From fe5055cf29fe6873fa5a1e5c24cb8b3c2f39e128 Mon Sep 17 00:00:00 2001 From: JT Date: Fri, 18 Jun 2021 13:04:51 +1200 Subject: [PATCH] Relax groups and blocks to output at pipeline level (#3643) * Relax groups and blocks to output at pipeline level * Fix up tests and add ignore command --- crates/nu-command/src/commands.rs | 2 + .../nu-command/src/commands/autoenv_trust.rs | 18 ++- .../src/commands/autoenv_untrust.rs | 22 ++-- .../src/commands/classified/external.rs | 5 +- .../src/commands/default_context.rs | 1 + crates/nu-command/src/commands/ignore.rs | 49 ++++++++ crates/nu-command/src/examples.rs | 5 + crates/nu-command/tests/commands/config.rs | 8 +- crates/nu-engine/src/evaluate/block.rs | 112 +++++++++--------- tests/shell/environment/in_sync.rs | 2 +- tests/shell/environment/nu_env.rs | 60 +++++----- 11 files changed, 177 insertions(+), 107 deletions(-) create mode 100644 crates/nu-command/src/commands/ignore.rs diff --git a/crates/nu-command/src/commands.rs b/crates/nu-command/src/commands.rs index 1ea73052d..148eb6403 100644 --- a/crates/nu-command/src/commands.rs +++ b/crates/nu-command/src/commands.rs @@ -70,6 +70,7 @@ pub(crate) mod help; pub(crate) mod histogram; pub(crate) mod history; pub(crate) mod if_; +pub(crate) mod ignore; pub(crate) mod insert; pub(crate) mod into; pub(crate) mod keep; @@ -229,6 +230,7 @@ pub(crate) use headers::Headers; pub(crate) use help::Help; pub(crate) use histogram::Histogram; pub(crate) use history::History; +pub(crate) use ignore::Ignore; pub(crate) use insert::Command as Insert; pub(crate) use keep::{Keep, KeepUntil, KeepWhile}; pub(crate) use last::Last; diff --git a/crates/nu-command/src/commands/autoenv_trust.rs b/crates/nu-command/src/commands/autoenv_trust.rs index f4322ffd6..a1f9a50c7 100644 --- a/crates/nu-command/src/commands/autoenv_trust.rs +++ b/crates/nu-command/src/commands/autoenv_trust.rs @@ -14,7 +14,9 @@ impl WholeStreamCommand for AutoenvTrust { } fn signature(&self) -> Signature { - Signature::build("autoenv trust").optional("dir", SyntaxShape::String, "Directory to allow") + Signature::build("autoenv trust") + .optional("dir", SyntaxShape::String, "Directory to allow") + .switch("quiet", "Don't output success message", Some('q')) } fn usage(&self) -> &str { @@ -23,9 +25,8 @@ impl WholeStreamCommand for AutoenvTrust { fn run_with_actions(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); - let ctx = &args.context; - let file_to_trust = match args.call_info.evaluate(&ctx)?.args.nth(0) { + let file_to_trust = match args.opt(0)? { Some(Value { value: UntaggedValue::Primitive(Primitive::String(ref path)), tag: _, @@ -40,6 +41,7 @@ impl WholeStreamCommand for AutoenvTrust { dir } }; + let quiet = args.has_flag("quiet"); let content = std::fs::read(&file_to_trust)?; @@ -55,9 +57,13 @@ impl WholeStreamCommand for AutoenvTrust { })?; fs::write(config_path, tomlstr).expect("Couldn't write to toml file"); - Ok(ActionStream::one(ReturnSuccess::value( - UntaggedValue::string(".nu-env trusted!").into_value(tag), - ))) + if quiet { + Ok(ActionStream::empty()) + } else { + Ok(ActionStream::one(ReturnSuccess::value( + UntaggedValue::string(".nu-env trusted!").into_value(tag), + ))) + } } fn is_binary(&self) -> bool { false diff --git a/crates/nu-command/src/commands/autoenv_untrust.rs b/crates/nu-command/src/commands/autoenv_untrust.rs index 8d4701521..5c36206bc 100644 --- a/crates/nu-command/src/commands/autoenv_untrust.rs +++ b/crates/nu-command/src/commands/autoenv_untrust.rs @@ -14,11 +14,9 @@ impl WholeStreamCommand for AutoenvUnTrust { } fn signature(&self) -> Signature { - Signature::build("autoenv untrust").optional( - "dir", - SyntaxShape::String, - "Directory to disallow", - ) + Signature::build("autoenv untrust") + .optional("dir", SyntaxShape::String, "Directory to disallow") + .switch("quiet", "Don't output success message", Some('q')) } fn usage(&self) -> &str { @@ -27,8 +25,7 @@ impl WholeStreamCommand for AutoenvUnTrust { fn run_with_actions(&self, args: CommandArgs) -> Result { let tag = args.call_info.name_tag.clone(); - let ctx = &args.context; - let file_to_untrust = match args.call_info.evaluate(&ctx)?.args.nth(0) { + let file_to_untrust = match args.opt(0)? { Some(Value { value: UntaggedValue::Primitive(Primitive::String(ref path)), tag: _, @@ -43,6 +40,7 @@ impl WholeStreamCommand for AutoenvUnTrust { dir } }; + let quiet = args.has_flag("quiet"); let config_path = config::default_path_for(&Some(PathBuf::from("nu-env.toml")))?; @@ -79,9 +77,13 @@ impl WholeStreamCommand for AutoenvUnTrust { })?; fs::write(config_path, tomlstr).expect("Couldn't write to toml file"); - Ok(ActionStream::one(ReturnSuccess::value( - UntaggedValue::string(".nu-env untrusted!").into_value(tag), - ))) + if quiet { + Ok(ActionStream::empty()) + } else { + Ok(ActionStream::one(ReturnSuccess::value( + UntaggedValue::string(".nu-env untrusted!").into_value(tag), + ))) + } } fn is_binary(&self) -> bool { false diff --git a/crates/nu-command/src/commands/classified/external.rs b/crates/nu-command/src/commands/classified/external.rs index c8a3a7015..5f77300db 100644 --- a/crates/nu-command/src/commands/classified/external.rs +++ b/crates/nu-command/src/commands/classified/external.rs @@ -263,7 +263,10 @@ fn spawn( "Received unexpected type from pipeline ({})", unsupported.type_name() ), - "expected a string", + format!( + "expected a string, got {} as input", + unsupported.type_name() + ), stdin_name_tag.clone(), )), tag: stdin_name_tag, diff --git a/crates/nu-command/src/commands/default_context.rs b/crates/nu-command/src/commands/default_context.rs index 791a0313e..6139eed09 100644 --- a/crates/nu-command/src/commands/default_context.rs +++ b/crates/nu-command/src/commands/default_context.rs @@ -17,6 +17,7 @@ pub fn create_default_context(interactive: bool) -> Result &str { + "ignore" + } + + fn signature(&self) -> Signature { + Signature::build("ignore") + } + + fn usage(&self) -> &str { + "Ignore the output of the previous command in the pipeline" + } + + fn run(&self, args: CommandArgs) -> Result { + let _: Vec<_> = args.input.collect(); + + Ok(OutputStream::empty()) + } + + fn examples(&self) -> Vec { + vec![Example { + description: "echo done | ignore", + example: r#"echo "There are seven words in this sentence" | size"#, + result: None, + }] + } +} + +#[cfg(test)] +mod tests { + use super::Ignore; + use super::ShellError; + + #[test] + fn examples_work_as_expected() -> Result<(), ShellError> { + use crate::examples::test as test_examples; + + test_examples(Ignore {}) + } +} diff --git a/crates/nu-command/src/examples.rs b/crates/nu-command/src/examples.rs index f470b0184..74d81835f 100644 --- a/crates/nu-command/src/examples.rs +++ b/crates/nu-command/src/examples.rs @@ -111,8 +111,13 @@ pub fn test(cmd: impl WholeStreamCommand + 'static) -> Result<(), ShellError> { let block = parse_line(sample_pipeline.example, &ctx)?; if let Some(expected) = &sample_pipeline.result { + let start = std::time::Instant::now(); let result = evaluate_block(block, &mut ctx)?; + println!("input: {}", sample_pipeline.example); + println!("result: {:?}", result); + println!("done: {:?}", start.elapsed()); + ctx.with_errors(|reasons| reasons.iter().cloned().take(1).next()) .map_or(Ok(()), Err)?; diff --git a/crates/nu-command/tests/commands/config.rs b/crates/nu-command/tests/commands/config.rs index f206afb69..abce38a7a 100644 --- a/crates/nu-command/tests/commands/config.rs +++ b/crates/nu-command/tests/commands/config.rs @@ -19,7 +19,7 @@ fn clearing_config_clears_config() { )]); assert_that!( - nu.pipeline("config clear; config get skip_welcome_message"), + nu.pipeline("config clear | ignore; config get skip_welcome_message"), says().stdout("") ); let config_contents = std::fs::read_to_string(file).expect("Could not read file"); @@ -63,7 +63,7 @@ fn config_set_sets_value() { assert_that!( //Clears config - nu.pipeline("config set key value; config get key"), + nu.pipeline("config set key value | ignore; config get key"), says().stdout("value") ); let config_contents = std::fs::read_to_string(file).expect("Could not read file"); @@ -86,7 +86,7 @@ fn config_set_into_sets_value() { assert_that!( //Clears config - nu.pipeline("echo value | config set_into key; config get key"), + nu.pipeline("echo value | config set_into key | ignore; config get key"), says().stdout("value") ); let config_contents = std::fs::read_to_string(file).expect("Could not read file"); @@ -109,7 +109,7 @@ fn config_rm_removes_value() { )]); assert_that!( - nu.pipeline("config remove key; config get key"), + nu.pipeline("config remove key | ignore; config get key"), says().stdout("") ); let config_contents = std::fs::read_to_string(file).expect("Could not read file"); diff --git a/crates/nu-engine/src/evaluate/block.rs b/crates/nu-engine/src/evaluate/block.rs index 54291c522..4a8fda67d 100644 --- a/crates/nu-engine/src/evaluate/block.rs +++ b/crates/nu-engine/src/evaluate/block.rs @@ -25,62 +25,64 @@ pub fn run_block( let num_groups = block.block.len(); for (group_num, group) in block.block.iter().enumerate() { - match output { - Ok(inp) if inp.is_empty() => {} - Ok(inp) => { - // Run autoview on the values we've seen so far - // We may want to make this configurable for other kinds of hosting - if let Some(autoview) = ctx.get_command("autoview") { - let mut output_stream = match ctx.run_command( - autoview, - Tag::unknown(), - Call::new( - Box::new(SpannedExpression::new( - Expression::Synthetic(Synthetic::String("autoview".into())), - Span::unknown(), - )), - Span::unknown(), - ), - inp, - ) { - Ok(x) => x, - Err(e) => { - return Err(e); - } - }; - match output_stream.next() { - Some(Value { - value: UntaggedValue::Error(e), - .. - }) => { - return Err(e); - } - Some(_item) => { - if let Some(err) = ctx.get_errors().get(0) { - ctx.clear_errors(); - return Err(err.clone()); - } - if ctx.ctrl_c().load(Ordering::SeqCst) { - return Ok(InputStream::empty()); - } - } - None => { - if let Some(err) = ctx.get_errors().get(0) { - ctx.clear_errors(); - return Err(err.clone()); - } - } - } - } - } - Err(e) => { - return Err(e); - } - } - output = Ok(OutputStream::empty()); - let num_pipelines = group.pipelines.len(); for (pipeline_num, pipeline) in group.pipelines.iter().enumerate() { + match output { + Ok(inp) if inp.is_empty() => {} + Ok(inp) => { + // Run autoview on the values we've seen so far + // We may want to make this configurable for other kinds of hosting + if let Some(autoview) = ctx.get_command("autoview") { + let mut output_stream = match ctx.run_command( + autoview, + Tag::unknown(), + Call::new( + Box::new(SpannedExpression::new( + Expression::Synthetic(Synthetic::String("autoview".into())), + Span::unknown(), + )), + Span::unknown(), + ), + inp, + ) { + Ok(x) => x, + Err(e) => { + return Err(e); + } + }; + match output_stream.next() { + Some(Value { + value: UntaggedValue::Error(e), + .. + }) => { + return Err(e); + } + Some(_item) => { + if let Some(err) = ctx.get_errors().get(0) { + ctx.clear_errors(); + return Err(err.clone()); + } + if ctx.ctrl_c().load(Ordering::SeqCst) { + return Ok(InputStream::empty()); + } + } + None => { + if let Some(err) = ctx.get_errors().get(0) { + ctx.clear_errors(); + return Err(err.clone()); + } + } + } + } else { + let _: Vec<_> = inp.collect(); + } + } + Err(e) => { + return Err(e); + } + } + output = Ok(OutputStream::empty()); + match output { Ok(inp) if inp.is_empty() => {} Ok(mut output_stream) => { @@ -122,7 +124,7 @@ pub fn run_block( run_pipeline(pipeline, ctx, input, external_redirection) } else { // otherwise, we're in the middle of the block, so use a default redirection - run_pipeline(pipeline, ctx, input, ExternalRedirection::Stdout) + run_pipeline(pipeline, ctx, input, ExternalRedirection::None) }; input = OutputStream::empty(); diff --git a/tests/shell/environment/in_sync.rs b/tests/shell/environment/in_sync.rs index 814ba65a1..ea537c6c3 100644 --- a/tests/shell/environment/in_sync.rs +++ b/tests/shell/environment/in_sync.rs @@ -25,7 +25,7 @@ fn setting_environment_value_to_configuration_should_pick_up_into_in_memory_envi )]); assert_that!( - nu.pipeline("config set env.USER NUNO; echo $nothing") + nu.pipeline("config set env.USER NUNO | ignore") .and_then("echo $nu.env.USER"), says().stdout("NUNO") ); diff --git a/tests/shell/environment/nu_env.rs b/tests/shell/environment/nu_env.rs index a8e90394d..a59a72ecf 100644 --- a/tests/shell/environment/nu_env.rs +++ b/tests/shell/environment/nu_env.rs @@ -62,7 +62,7 @@ fn picks_up_and_lets_go_env_keys_when_entering_trusted_directory_with_implied_cd let actual = nu!( cwd: dirs.test(), r#" - do {autoenv trust foo ; = $nothing } + do {autoenv trust -q foo ; = $nothing } foo echo $nu.env.testkey"# ); @@ -71,7 +71,7 @@ fn picks_up_and_lets_go_env_keys_when_entering_trusted_directory_with_implied_cd let actual = nu!( cwd: dirs.test(), r#" - do {autoenv trust foo; = $nothing } ; + do {autoenv trust -q foo; = $nothing } ; foo .. echo $nu.env.testkey @@ -82,8 +82,8 @@ fn picks_up_and_lets_go_env_keys_when_entering_trusted_directory_with_implied_cd let actual = nu!( cwd: dirs.test(), r#" - do {autoenv trust foo; = $nothing } ; - do {autoenv trust foo/bar; = $nothing } ; + do {autoenv trust -q foo; = $nothing } ; + do {autoenv trust -q foo/bar; = $nothing } ; foo/bar echo $nu.env.testkey echo $nu.env.bar @@ -93,7 +93,7 @@ fn picks_up_and_lets_go_env_keys_when_entering_trusted_directory_with_implied_cd //Assert bar removed after leaving bar let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; + r#"autoenv trust -q foo; foo/bar ../.. echo $nu.env.bar"# @@ -231,7 +231,7 @@ fn entry_scripts_are_called_when_revisiting_a_trusted_directory() { let actual = Trusted::in_path(&dirs, || { nu!(cwd: dirs.test(), r#" - do { rm hello.txt ; = $nothing } ; # Silence file deletion message from output + do { rm hello.txt | ignore } ; # Silence file deletion message from output cd .. cd autoenv_test_6 ls | where name == "hello.txt" | get name @@ -325,7 +325,7 @@ fn given_a_hierachy_of_trusted_directories_when_entering_in_any_nested_ones_shou let actual = Trusted::in_path(&dirs, || { nu!(cwd: dirs.test().parent().unwrap(), r#" - do { autoenv trust autoenv_test_9/nu_plugin_rb ; = $nothing } # Silence autoenv trust message from output + do { autoenv trust -q autoenv_test_9/nu_plugin_rb ; = $nothing } # Silence autoenv trust -q message from output cd autoenv_test_9/nu_plugin_rb echo $nu.env.organization "#) @@ -356,7 +356,7 @@ fn given_a_hierachy_of_trusted_directories_nested_ones_should_overwrite_variable let actual = Trusted::in_path(&dirs, || { nu!(cwd: dirs.test().parent().unwrap(), r#" - do { autoenv trust autoenv_test_10/nu_plugin_rb ; = $nothing } # Silence autoenv trust message from output + do { autoenv trust -q autoenv_test_10/nu_plugin_rb ; = $nothing } # Silence autoenv trust -q message from output cd autoenv_test_10/nu_plugin_rb echo $nu.env.organization "#) @@ -392,7 +392,7 @@ fn local_config_should_not_be_added_when_running_scripts() { let actual = Trusted::in_path(&dirs, || { nu!(cwd: dirs.test(), r#" - do { autoenv trust foo ; = $nothing } # Silence autoenv trust message from output + do { autoenv trust -q foo } # Silence autoenv trust message from output nu script.nu "#) }); @@ -420,10 +420,10 @@ fn given_a_hierachy_of_trusted_directories_going_back_restores_overwritten_varia let actual = Trusted::in_path(&dirs, || { nu!(cwd: dirs.test().parent().unwrap(), r#" - do { autoenv trust autoenv_test_11/nu_plugin_rb ; = $nothing } # Silence autoenv trust message from output + do { autoenv trust -q autoenv_test_11/nu_plugin_rb } # Silence autoenv trust message from output cd autoenv_test_11 cd nu_plugin_rb - do { rm ../.nu-env ; = $nothing } # By deleting the root nu-env we have guarantees that the variable gets restored (not by autoenv when re-entering) + do { rm ../.nu-env | ignore } # By deleting the root nu-env we have guarantees that the variable gets restored (not by autoenv when re-entering) cd .. echo $nu.env.organization "#) @@ -451,21 +451,21 @@ fn local_config_env_var_present_and_removed_correctly() { //Assert testkey is not present before entering directory let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; + r#"autoenv trust -q foo; echo $nu.env.testkey"# ); assert!(actual.err.contains("Unknown")); //Assert testkey is present in foo let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; cd foo + r#"autoenv trust -q foo; cd foo echo $nu.env.testkey"# ); assert_eq!(actual.out, "testvalue"); //Assert testkey is present also in subdirectories let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; cd foo + r#"autoenv trust -q foo; cd foo cd bar echo $nu.env.testkey"# ); @@ -473,14 +473,14 @@ fn local_config_env_var_present_and_removed_correctly() { //Assert testkey is present also when jumping over foo let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; cd foo/bar + r#"autoenv trust -q foo; cd foo/bar echo $nu.env.testkey"# ); assert_eq!(actual.out, "testvalue"); //Assert testkey removed after leaving foo let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; cd foo + r#"autoenv trust -q foo; cd foo cd .. echo $nu.env.testkey"# ); @@ -514,22 +514,22 @@ fn local_config_env_var_gets_overwritten() { //Assert overwrite_me is not present before entering directory let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; + r#"autoenv trust -q foo; echo $nu.env.overwrite_me"# ); assert!(actual.err.contains("Unknown")); //Assert overwrite_me is foo in foo let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; cd foo + r#"autoenv trust -q foo; cd foo echo $nu.env.overwrite_me"# ); assert_eq!(actual.out, "foo"); //Assert overwrite_me is bar in bar let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo | = $nothing - autoenv trust foo/bar | = $nothing + r#"autoenv trust -q foo + autoenv trust -q foo/bar cd foo cd bar echo $nu.env.overwrite_me"# @@ -538,7 +538,7 @@ fn local_config_env_var_gets_overwritten() { //Assert overwrite_me is present also when jumping over foo let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; autoenv trust foo/bar; cd foo/bar + r#"autoenv trust -q foo; autoenv trust -q foo/bar; cd foo/bar echo $nu.env.overwrite_me "# ); @@ -546,7 +546,7 @@ fn local_config_env_var_gets_overwritten() { //Assert overwrite_me removed after leaving bar let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo; autoenv trust foo/bar; cd foo + r#"autoenv trust -q foo; autoenv trust -q foo/bar; cd foo cd bar cd .. echo $nu.env.overwrite_me"# @@ -576,7 +576,7 @@ fn autoenv_test_entry_scripts() { // Make sure entryscript is run when entering directory let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo + r#"autoenv trust -q foo cd foo ls | where name == "hello.txt" | get name"# ); @@ -585,7 +585,7 @@ fn autoenv_test_entry_scripts() { // Make sure entry scripts are also run when jumping over directory let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo + r#"autoenv trust -q foo cd foo/bar ls .. | where name == "../hello.txt" | get name"# ); @@ -594,7 +594,7 @@ fn autoenv_test_entry_scripts() { // Entryscripts should not run after changing to a subdirectory. let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo | = $nothing + r#"autoenv trust -q foo cd foo rm hello.txt cd bar @@ -621,11 +621,11 @@ fn autoenv_test_exit_scripts() { // Make sure exitscript is run let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo | = $nothing + r#"autoenv trust -q foo cd foo cd .. ls foo | where name =~ "bye.txt" | length - rm foo/bye.txt; cd . + rm foo/bye.txt | ignore; cd . "# ); assert_eq!(actual.out, "1"); @@ -633,7 +633,7 @@ fn autoenv_test_exit_scripts() { // Entering a subdir should not trigger exitscripts let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo | = $nothing + r#"autoenv trust -q foo cd foo cd bar ls .. | where name =~ "bye.txt" | length"# @@ -643,11 +643,11 @@ fn autoenv_test_exit_scripts() { // Also run exitscripts when jumping over directory let actual = nu!( cwd: dirs.test(), - r#"autoenv trust foo | = $nothing + r#"autoenv trust -q foo cd foo/bar cd ../.. ls foo | where name =~ "bye.txt" | length - rm foo/bye.txt; cd ."# + rm foo/bye.txt | ignore; cd ."# ); assert_eq!(actual.out, "1"); });