From e1f74a6d574ecb2bd78918d9aa57651f59cf47d5 Mon Sep 17 00:00:00 2001 From: 132ikl <132@ikl.sh> Date: Fri, 29 Nov 2024 20:02:26 -0500 Subject: [PATCH] Add label rendering to try/catch rendered errors (#14477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Before this PR, you can access rendered error values that are raised in a `try/catch` block by accessing the `rendered` element of the catch error value: ``` $ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } my cool error: nu::shell::directory_not_found × Directory not found help: /home/rose/nonexist.txt does not exist ``` However, the rendered errors don't include the labels present in the real rendered error, which would look like this: ``` $ ls nonexist.txt Error: nu::shell::directory_not_found × Directory not found ╭─[entry #46:1:4] 1 │ ls nonexist.txt · ──────┬───── · ╰── directory not found ╰──── help: /home/rose/nonexist.txt does not exist ``` After this PR, the rendered error includes the labels: ``` $ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } my cool error: Error: nu::shell::directory_not_found × Directory not found ╭─[entry #4:1:10] 1 │ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } · ──────┬───── · ╰── directory not found ╰──── help: /home/rose/nonexist.txt does not exist ``` This change is accomplished by using the standard error formatting code to render an error. This respects the error theme as before without any extra scaffolding, but it means that e.g., the terminal size is also respected. I think this is fine because the way the error is rendered already changed based on config, and I think that a "rendered" error should give back _exactly_ what would be shown to the user anyway. @fdncred, let me know if you have any concerns with the way this is handled since you were the one who implemented this feature in the first place. # User-Facing Changes The `rendered` element of the `try`/`catch` error record now includes labels in the error output. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting N/A --- crates/nu-cmd-lang/src/core_commands/try_.rs | 6 +----- crates/nu-engine/src/eval_ir.rs | 18 +++++------------ .../nu-protocol/src/errors/labeled_error.rs | 17 ---------------- crates/nu-protocol/src/errors/shell_error.rs | 20 ++----------------- 4 files changed, 8 insertions(+), 53 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/try_.rs b/crates/nu-cmd-lang/src/core_commands/try_.rs index c7242550a7..4b5361c681 100644 --- a/crates/nu-cmd-lang/src/core_commands/try_.rs +++ b/crates/nu-cmd-lang/src/core_commands/try_.rs @@ -107,11 +107,7 @@ fn run_catch( if let Some(catch) = catch { stack.set_last_error(&error); - let fancy_errors = match engine_state.get_config().error_style { - nu_protocol::ErrorStyle::Fancy => true, - nu_protocol::ErrorStyle::Plain => false, - }; - let error = error.into_value(span, fancy_errors); + let error = error.into_value(&StateWorkingSet::new(engine_state), span); let block = engine_state.get_block(catch.block_id); // Put the error value in the positional closure var if let Some(var) = block.signature.get_positional(0) { diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 1aec6b30ea..ee0d8624d2 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -4,7 +4,9 @@ use nu_path::{expand_path_with, AbsolutePathBuf}; use nu_protocol::{ ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator}, debugger::DebugContext, - engine::{Argument, Closure, EngineState, ErrorHandler, Matcher, Redirection, Stack}, + engine::{ + Argument, Closure, EngineState, ErrorHandler, Matcher, Redirection, Stack, StateWorkingSet, + }, ir::{Call, DataSlice, Instruction, IrAstRef, IrBlock, Literal, RedirectMode}, ByteStreamSource, DataSource, DeclId, ErrSpan, Flag, IntoPipelineData, IntoSpanned, ListStream, OutDest, PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, ShellError, @@ -220,17 +222,8 @@ fn eval_ir_block_impl( } Err(err) => { if let Some(error_handler) = ctx.stack.error_handlers.pop(ctx.error_handler_base) { - let fancy_errors = match ctx.engine_state.get_config().error_style { - nu_protocol::ErrorStyle::Fancy => true, - nu_protocol::ErrorStyle::Plain => false, - }; // If an error handler is set, branch there - prepare_error_handler( - ctx, - error_handler, - Some(err.into_spanned(*span)), - fancy_errors, - ); + prepare_error_handler(ctx, error_handler, Some(err.into_spanned(*span))); pc = error_handler.handler_index; } else { // If not, exit the block with the error @@ -255,7 +248,6 @@ fn prepare_error_handler( ctx: &mut EvalContext<'_>, error_handler: ErrorHandler, error: Option>, - fancy_errors: bool, ) { if let Some(reg_id) = error_handler.error_register { if let Some(error) = error { @@ -266,7 +258,7 @@ fn prepare_error_handler( reg_id, error .item - .into_value(error.span, fancy_errors) + .into_value(&StateWorkingSet::new(ctx.engine_state), error.span) .into_pipeline_data(), ); } else { diff --git a/crates/nu-protocol/src/errors/labeled_error.rs b/crates/nu-protocol/src/errors/labeled_error.rs index a207178415..c76a6c78dd 100644 --- a/crates/nu-protocol/src/errors/labeled_error.rs +++ b/crates/nu-protocol/src/errors/labeled_error.rs @@ -139,23 +139,6 @@ impl LabeledError { self } - pub fn render_error_to_string(diag: impl miette::Diagnostic, fancy_errors: bool) -> String { - let theme = if fancy_errors { - miette::GraphicalTheme::unicode() - } else { - miette::GraphicalTheme::none() - }; - - let mut out = String::new(); - miette::GraphicalReportHandler::new() - .with_width(80) - .with_theme(theme) - .render_report(&mut out, &diag) - .unwrap_or_default(); - - out - } - /// Create a [`LabeledError`] from a type that implements [`miette::Diagnostic`]. /// /// # Example diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index b4b6c29cee..9e934e7f64 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -1479,15 +1479,14 @@ impl ShellError { } } - pub fn into_value(self, span: Span, fancy_errors: bool) -> Value { + pub fn into_value(self, working_set: &StateWorkingSet, span: Span) -> Value { let exit_code = self.external_exit_code(); let mut record = record! { "msg" => Value::string(self.to_string(), span), "debug" => Value::string(format!("{self:?}"), span), "raw" => Value::error(self.clone(), span), - // "labeled_error" => Value::string(LabeledError::from_diagnostic_and_render(self.clone()), span), - "rendered" => Value::string(ShellError::render_error_to_string(self.clone(), fancy_errors), span), + "rendered" => Value::string(format_shell_error(working_set, &self), span), "json" => Value::string(serde_json::to_string(&self).expect("Could not serialize error"), span), }; @@ -1507,21 +1506,6 @@ impl ShellError { span, ) } - pub fn render_error_to_string(diag: impl miette::Diagnostic, fancy_errors: bool) -> String { - let theme = if fancy_errors { - miette::GraphicalTheme::unicode() - } else { - miette::GraphicalTheme::none() - }; - let mut out = String::new(); - miette::GraphicalReportHandler::new() - .with_width(80) - .with_theme(theme) - .render_report(&mut out, &diag) - .unwrap_or_default(); - - out - } } impl From for ShellError {