From 12991cd36feedaac0ccb86be61e78a8ca4ffd261 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 18 Jun 2024 21:37:24 -0700 Subject: [PATCH] Change the error style during tests to `plain` (#13061) # Description This fixes issues with trying to run the tests with a terminal that is small enough to cause errors to wrap around, or in cases where the test environment might produce strings that are reasonably expected to wrap around anyway. "Fancy" errors are too fancy for tests to work predictably :wink: cc @abusch # User-Facing Changes - Added `--error-style` option for use with `--commands` (like `--table-mode`) # Tests + Formatting Surprisingly, all of the tests pass, including in small windows! I only had to make one change to a test for `error make` which was looking for the box drawing characters miette uses to determine whether the span label was showing up - but the plain error style output is even better and easier to match on, so this test is actually more specific now. --- benches/benchmarks.rs | 6 ++-- crates/nu-cli/src/eval_cmds.rs | 34 +++++++++++++++++-- crates/nu-cli/src/lib.rs | 2 +- .../nu-command/tests/commands/error_make.rs | 5 +-- crates/nu-test-support/src/macros.rs | 4 +++ src/command.rs | 16 +++++++-- src/run.rs | 9 +++-- 7 files changed, 62 insertions(+), 14 deletions(-) diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 84552daef9..7a23715c8d 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -47,8 +47,7 @@ fn setup_stack_and_engine_from_command(command: &str) -> (Stack, EngineState) { &mut engine, &mut stack, PipelineData::empty(), - None, - false, + Default::default(), ) .unwrap(); @@ -90,8 +89,7 @@ fn bench_command( &mut engine, &mut stack, PipelineData::empty(), - None, - false, + Default::default(), ) .unwrap(), ); diff --git a/crates/nu-cli/src/eval_cmds.rs b/crates/nu-cli/src/eval_cmds.rs index 8fa3bf30e5..13141f6174 100644 --- a/crates/nu-cli/src/eval_cmds.rs +++ b/crates/nu-cli/src/eval_cmds.rs @@ -8,15 +8,45 @@ use nu_protocol::{ }; use std::sync::Arc; +#[derive(Default)] +pub struct EvaluateCommandsOpts { + pub table_mode: Option, + pub error_style: Option, + pub no_newline: bool, +} + /// Run a command (or commands) given to us by the user pub fn evaluate_commands( commands: &Spanned, engine_state: &mut EngineState, stack: &mut Stack, input: PipelineData, - table_mode: Option, - no_newline: bool, + opts: EvaluateCommandsOpts, ) -> Result<(), ShellError> { + let EvaluateCommandsOpts { + table_mode, + error_style, + no_newline, + } = opts; + + // Handle the configured error style early + if let Some(e_style) = error_style { + match e_style.coerce_str()?.parse() { + Ok(e_style) => { + Arc::make_mut(&mut engine_state.config).error_style = e_style; + } + Err(err) => { + return Err(ShellError::GenericError { + error: "Invalid value for `--error-style`".into(), + msg: err.into(), + span: Some(e_style.span()), + help: None, + inner: vec![], + }); + } + } + } + // Translate environment variables from Strings to Values convert_env_values(engine_state, stack)?; diff --git a/crates/nu-cli/src/lib.rs b/crates/nu-cli/src/lib.rs index c4342dc3a0..6f151adad1 100644 --- a/crates/nu-cli/src/lib.rs +++ b/crates/nu-cli/src/lib.rs @@ -17,7 +17,7 @@ mod validation; pub use commands::add_cli_context; pub use completions::{FileCompletion, NuCompleter, SemanticSuggestion, SuggestionKind}; pub use config_files::eval_config_contents; -pub use eval_cmds::evaluate_commands; +pub use eval_cmds::{evaluate_commands, EvaluateCommandsOpts}; pub use eval_file::evaluate_file; pub use menus::NuHelpCompleter; pub use nu_cmd_base::util::get_init_cwd; diff --git a/crates/nu-command/tests/commands/error_make.rs b/crates/nu-command/tests/commands/error_make.rs index 0c6908d2aa..f5714c88ac 100644 --- a/crates/nu-command/tests/commands/error_make.rs +++ b/crates/nu-command/tests/commands/error_make.rs @@ -4,8 +4,9 @@ use nu_test_support::nu; fn error_label_works() { let actual = nu!("error make {msg:foo label:{text:unseen}}"); - assert!(actual.err.contains("unseen")); - assert!(actual.err.contains("╰──")); + assert!(actual + .err + .contains("label at line 1, columns 1 to 10: unseen")); } #[test] diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index c18dd5ac81..51d743e1a4 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -283,6 +283,8 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O if !with_std { command.arg("--no-std-lib"); } + // Use plain errors to help make error text matching more consistent + command.args(["--error-style", "plain"]); command .arg(format!("-c {}", escape_quote_string(&commands))) .stdout(Stdio::piped()) @@ -369,6 +371,8 @@ where .envs(envs) .arg("--commands") .arg(command) + // Use plain errors to help make error text matching more consistent + .args(["--error-style", "plain"]) .arg("--config") .arg(temp_config_file) .arg("--env-config") diff --git a/src/command.rs b/src/command.rs index 15da0e3e47..5ba8a60996 100644 --- a/src/command.rs +++ b/src/command.rs @@ -29,8 +29,10 @@ pub(crate) fn gather_commandline_args() -> (Vec, String, Vec) { } let flag_value = match arg.as_ref() { - "--commands" | "-c" | "--table-mode" | "-m" | "-e" | "--execute" | "--config" - | "--env-config" | "-I" | "ide-ast" => args.next().map(|a| escape_quote_string(&a)), + "--commands" | "-c" | "--table-mode" | "-m" | "--error-style" | "-e" | "--execute" + | "--config" | "--env-config" | "-I" | "ide-ast" => { + args.next().map(|a| escape_quote_string(&a)) + } #[cfg(feature = "plugin")] "--plugin-config" => args.next().map(|a| escape_quote_string(&a)), "--log-level" | "--log-target" | "--log-include" | "--log-exclude" | "--testbin" @@ -102,6 +104,8 @@ pub(crate) fn parse_commandline_args( let execute = call.get_flag_expr("execute"); let table_mode: Option = call.get_flag(engine_state, &mut stack, "table-mode")?; + let error_style: Option = + call.get_flag(engine_state, &mut stack, "error-style")?; let no_newline = call.get_named_arg("no-newline"); // ide flags @@ -245,6 +249,7 @@ pub(crate) fn parse_commandline_args( ide_check, ide_ast, table_mode, + error_style, no_newline, }); } @@ -278,6 +283,7 @@ pub(crate) struct NushellCliArgs { pub(crate) log_exclude: Option>>, pub(crate) execute: Option>, pub(crate) table_mode: Option, + pub(crate) error_style: Option, pub(crate) no_newline: Option>, pub(crate) include_path: Option>, pub(crate) lsp: bool, @@ -325,6 +331,12 @@ impl Command for Nu { "the table mode to use. rounded is default.", Some('m'), ) + .named( + "error-style", + SyntaxShape::String, + "the error style to use (fancy or plain). default: fancy", + None, + ) .switch("no-newline", "print the result for --commands(-c) without a newline", None) .switch( "no-config-file", diff --git a/src/run.rs b/src/run.rs index 2996bc76f8..db779b0058 100644 --- a/src/run.rs +++ b/src/run.rs @@ -7,7 +7,7 @@ use crate::{ use log::trace; #[cfg(feature = "plugin")] use nu_cli::read_plugin_file; -use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl}; +use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl, EvaluateCommandsOpts}; use nu_protocol::{ engine::{EngineState, Stack}, report_error_new, PipelineData, Spanned, @@ -114,8 +114,11 @@ pub(crate) fn run_commands( engine_state, &mut stack, input, - parsed_nu_cli_args.table_mode, - parsed_nu_cli_args.no_newline.is_some(), + EvaluateCommandsOpts { + table_mode: parsed_nu_cli_args.table_mode, + error_style: parsed_nu_cli_args.error_style, + no_newline: parsed_nu_cli_args.no_newline.is_some(), + }, ) { report_error_new(engine_state, &err); std::process::exit(1);