Skip to content

Commit

Permalink
Make $in work in catch closures (nushell#7458)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
webbedspace authored Dec 22, 2022
1 parent 216d7d0 commit 9d1cb1b
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 39 deletions.
19 changes: 13 additions & 6 deletions crates/nu-command/src/core_commands/try_.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -57,19 +59,20 @@ 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());
}
}

eval_block(
engine_state,
stack,
catch_block,
PipelineData::empty(),
// Make the error accessible with $in, too
err_value.into_pipeline_data(),
false,
false,
)
Expand All @@ -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);
}
Expand All @@ -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,
)
Expand Down
67 changes: 34 additions & 33 deletions crates/nu-command/tests/commands/try_.rs
Original file line number Diff line number Diff line change
@@ -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' }"
);

assert!(output.out.contains("345"));
})
let output = nu!(
cwd: ".",
"try { 345 } catch { echo 'hello' }"
);

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' }"
);

assert!(output.out.contains("hello"));
})
let output = nu!(
cwd: ".",
"try { foobarbaz } catch { echo '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 }"
);

assert!(output.err.contains("External command failed"));
})
let output = nu!(
cwd: ".",
"try { foobarbaz } catch { |err| $err }"
);

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' }"
);

assert!(output.out.contains("fail"));
})
let output = nu!(
cwd: ".",
"try { nu --testbin fail; echo 'success' } catch { echo 'fail' }"
);

assert!(output.out.contains("fail"));
}

0 comments on commit 9d1cb1b

Please sign in to comment.