From 9d1cb1bfafdb08c0cceb6c891e6f29de87cfc271 Mon Sep 17 00:00:00 2001 From: Leon Date: Fri, 23 Dec 2022 01:35:41 +1000 Subject: [PATCH] Make `$in` work in `catch` closures (#7458) # Description This now works: ``` try { 'x' | math abs } catch { $in } ``` # User-Facing Changes See above. # 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. --- crates/nu-command/src/core_commands/try_.rs | 19 ++++--- crates/nu-command/tests/commands/try_.rs | 59 +++++++++++---------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/crates/nu-command/src/core_commands/try_.rs b/crates/nu-command/src/core_commands/try_.rs index 992a2cbf50..b07651a979 100644 --- a/crates/nu-command/src/core_commands/try_.rs +++ b/crates/nu-command/src/core_commands/try_.rs @@ -1,7 +1,9 @@ use nu_engine::{eval_block, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{Block, Closure, Command, EngineState, Stack}; -use nu_protocol::{Category, Example, PipelineData, Signature, SyntaxShape, Type, Value}; +use nu_protocol::{ + Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Type, Value, +}; #[derive(Clone)] pub struct Try; @@ -57,11 +59,11 @@ impl Command for Try { Err(error) | Ok(PipelineData::Value(Value::Error { error }, ..)) => { if let Some(catch_block) = catch_block { let catch_block = engine_state.get_block(catch_block.block_id); - + let err_value = Value::Error { error }; + // Put the error value in the positional closure var if let Some(var) = catch_block.signature.get_positional(0) { if let Some(var_id) = &var.var_id { - let err_value = Value::Error { error }; - stack.add_var(*var_id, err_value); + stack.add_var(*var_id, err_value.clone()); } } @@ -69,7 +71,8 @@ impl Command for Try { engine_state, stack, catch_block, - PipelineData::empty(), + // Make the error accessible with $in, too + err_value.into_pipeline_data(), false, false, ) @@ -86,6 +89,9 @@ impl Command for Try { if let Some(var) = catch_block.signature.get_positional(0) { if let Some(var_id) = &var.var_id { + // Because external command errors aren't "real" errors, + // (unless do -c is in effect) + // they can't be passed in as Nushell values. let err_value = Value::nothing(call.head); stack.add_var(*var_id, err_value); } @@ -95,7 +101,8 @@ impl Command for Try { engine_state, stack, catch_block, - PipelineData::empty(), + // The same null as in the above block is set as the $in value. + Value::nothing(call.head).into_pipeline_data(), false, false, ) diff --git a/crates/nu-command/tests/commands/try_.rs b/crates/nu-command/tests/commands/try_.rs index 254bc9a2e3..225bca76da 100644 --- a/crates/nu-command/tests/commands/try_.rs +++ b/crates/nu-command/tests/commands/try_.rs @@ -1,50 +1,51 @@ use nu_test_support::nu; -use nu_test_support::playground::Playground; #[test] fn try_succeed() { - Playground::setup("try_succeed_test", |dirs, _sandbox| { - let output = nu!( - cwd: dirs.test(), - "try { 345 } catch { echo 'hello' }" - ); + let output = nu!( + cwd: ".", + "try { 345 } catch { echo 'hello' }" + ); - assert!(output.out.contains("345")); - }) + assert!(output.out.contains("345")); } #[test] fn try_catch() { - Playground::setup("try_catch_test", |dirs, _sandbox| { - let output = nu!( - cwd: dirs.test(), - "try { foobarbaz } catch { echo 'hello' }" - ); + let output = nu!( + cwd: ".", + "try { foobarbaz } catch { echo 'hello' }" + ); - assert!(output.out.contains("hello")); - }) + assert!(output.out.contains("hello")); } #[test] fn catch_can_access_error() { - Playground::setup("try_catch_test", |dirs, _sandbox| { - let output = nu!( - cwd: dirs.test(), - "try { foobarbaz } catch { |err| $err }" - ); + let output = nu!( + cwd: ".", + "try { foobarbaz } catch { |err| $err }" + ); - assert!(output.err.contains("External command failed")); - }) + assert!(output.err.contains("External command failed")); +} + +#[test] +fn catch_can_access_error_as_dollar_in() { + let output = nu!( + cwd: ".", + "try { foobarbaz } catch { $in }" + ); + + assert!(output.err.contains("External command failed")); } #[test] fn external_failed_should_be_catched() { - Playground::setup("try_catch_test", |dirs, _sandbox| { - let output = nu!( - cwd: dirs.test(), - "try { nu --testbin fail; echo 'success' } catch { echo 'fail' }" - ); + let output = nu!( + cwd: ".", + "try { nu --testbin fail; echo 'success' } catch { echo 'fail' }" + ); - assert!(output.out.contains("fail")); - }) + assert!(output.out.contains("fail")); }