From c5639cd9faf0baea005ea29e2d3555b9e0750736 Mon Sep 17 00:00:00 2001 From: Leon Date: Wed, 4 Jan 2023 17:59:10 +1000 Subject: [PATCH] Primitives now use color closures when printed on the command line (#7650) # Description Closes #7554 ![image](https://user-images.githubusercontent.com/83939/210177700-4890fcf2-1be9-4da9-9974-58d4ed403430.png) # 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. Co-authored-by: Reilly Wood <26268125+rgwood@users.noreply.github.com> --- crates/nu-command/src/viewers/table.rs | 17 ++++++++++- crates/nu-test-support/src/macros.rs | 12 ++++++-- src/tests.rs | 23 +++++++++++++-- src/tests/test_config_path.rs | 23 +++++++++++---- tests/fixtures/playground/config/default.toml | 3 -- tests/fixtures/playground/config/minimal.nu | 28 +++++++++++++++++++ tests/fixtures/playground/config/startup.toml | 3 -- tests/parsing/mod.rs | 12 ++++---- tests/shell/environment/env.rs | 2 +- tests/shell/pipeline/commands/external.rs | 8 +++--- 10 files changed, 104 insertions(+), 27 deletions(-) delete mode 100644 tests/fixtures/playground/config/default.toml create mode 100644 tests/fixtures/playground/config/minimal.nu delete mode 100644 tests/fixtures/playground/config/startup.toml diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 4bd07cdbae..c7efd75d57 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -348,7 +348,22 @@ fn handle_table_command( ctrlc, metadata, ), - x => Ok(x), + PipelineData::Value(v, ..) => { + // into_string() is used for serialising primitives in PipelineData::write_all_and_flush(), + // so the same is used here. + let str_representation = v.into_string("", config); + + Ok(Value::String { + val: StyleComputer::from_config(engine_state, stack) + .style_primitive(&v) + .color_style + .map(|e| e.paint(&str_representation).to_string()) + .unwrap_or(str_representation), + span: call.head, + } + .into_pipeline_data()) + } + PipelineData::Empty {} => Ok(input), } } diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index 09b81febe2..f3743b6d6c 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -158,6 +158,7 @@ macro_rules! nu { let target_cwd = $opts.cwd.unwrap_or(".".to_string()); let locale = $opts.locale.unwrap_or("en_US.UTF-8".to_string()); + let minimal_config = $opts.minimal_config.unwrap_or(true); let mut command = Command::new($crate::fs::executable_path()); command @@ -167,12 +168,16 @@ macro_rules! nu { .env(NATIVE_PATH_ENV_VAR, paths_joined) // .arg("--skip-plugins") // .arg("--no-history") - // .arg("--config-file") - // .arg($crate::fs::DisplayPath::display_path(&$crate::fs::fixtures().join("playground/config/default.toml"))) .arg(format!("-c {}", escape_quote_string(path))) .stdout(Stdio::piped()) // .stdin(Stdio::piped()) .stderr(Stdio::piped()); + // Use this minimal config in most tests. + // Notably, this disables color_config to allow string output to be more easily compared. + if minimal_config { + command.arg("--config") + .arg($crate::fs::fixtures().join("playground/config/minimal.nu").display().to_string()); + } let mut process = match command.spawn() { @@ -203,6 +208,7 @@ macro_rules! nu { struct NuOpts { cwd: Option, locale: Option, + minimal_config: Option, } nu!(@options [ ] $($token)*) @@ -276,6 +282,8 @@ macro_rules! nu_with_plugins { .arg(commands) .arg("--plugin-config") .arg(temp_plugin_file) + .arg("--config") + .arg($crate::fs::fixtures().join("playground/config/minimal.nu").display().to_string()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() diff --git a/src/tests.rs b/src/tests.rs index 125ce12f1b..04f79422ca 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -32,7 +32,17 @@ pub fn run_test_with_env(input: &str, expected: &str, env: &HashMap<&str, &str>) let name = file.path(); let mut cmd = Command::cargo_bin("nu")?; - cmd.arg(name).envs(env); + // Use this minimal config in most tests. + // Notably, this disables color_config to allow string output to be more easily compared. + cmd.arg("--config") + .arg( + nu_test_support::fs::fixtures() + .join("playground/config/minimal.nu") + .display() + .to_string(), + ) + .arg(name) + .envs(env); writeln!(file, "{}", input)?; @@ -45,7 +55,16 @@ pub fn run_test(input: &str, expected: &str) -> TestResult { let name = file.path(); let mut cmd = Command::cargo_bin("nu")?; - cmd.arg(name); + // Use this minimal config in most tests. + // Notably, this disables color_config to allow string output to be more easily compared. + cmd.arg("--config") + .arg( + nu_test_support::fs::fixtures() + .join("playground/config/minimal.nu") + .display() + .to_string(), + ) + .arg(name); cmd.env( "PWD", std::env::current_dir().expect("Can't get current dir"), diff --git a/src/tests/test_config_path.rs b/src/tests/test_config_path.rs index da661a476b..bd5587003f 100644 --- a/src/tests/test_config_path.rs +++ b/src/tests/test_config_path.rs @@ -6,12 +6,18 @@ fn test_default_config_path() { let cwd = std::env::current_dir().expect("Could not get current working directory"); let config_path = config_dir.join("nushell").join("config.nu"); - let actual = nu!(cwd: &cwd, "$nu.config-path"); - assert_eq!(actual.out, config_path.to_string_lossy().to_string()); + let actual = nu!(cwd: &cwd, minimal_config: false, "$nu.config-path"); + assert_eq!( + nu_utils::strip_ansi_string_likely(actual.out), + config_path.to_string_lossy().to_string() + ); let env_path = config_dir.join("nushell").join("env.nu"); let actual = nu!(cwd: &cwd, "$nu.env-path"); - assert_eq!(actual.out, env_path.to_string_lossy().to_string()); + assert_eq!( + nu_utils::strip_ansi_string_likely(actual.out), + env_path.to_string_lossy().to_string() + ); } #[test] @@ -25,14 +31,21 @@ fn test_alternate_config_path() { nu_path::canonicalize_with(config_file, &cwd).expect("Could not get config path"); let actual = nu!( cwd: &cwd, + minimal_config: false, format!("nu --config {:?} -c '$nu.config-path'", config_path) ); - assert_eq!(actual.out, config_path.to_string_lossy().to_string()); + assert_eq!( + nu_utils::strip_ansi_string_likely(actual.out), + config_path.to_string_lossy().to_string() + ); let env_path = nu_path::canonicalize_with(env_file, &cwd).expect("Could not get env path"); let actual = nu!( cwd: &cwd, format!("nu --env-config {:?} -c '$nu.env-path'", env_path) ); - assert_eq!(actual.out, env_path.to_string_lossy().to_string()); + assert_eq!( + nu_utils::strip_ansi_string_likely(actual.out), + env_path.to_string_lossy().to_string() + ); } diff --git a/tests/fixtures/playground/config/default.toml b/tests/fixtures/playground/config/default.toml deleted file mode 100644 index 5939aacd41..0000000000 --- a/tests/fixtures/playground/config/default.toml +++ /dev/null @@ -1,3 +0,0 @@ -skip_welcome_message = true -filesize_format = "auto" -rm_always_trash = false diff --git a/tests/fixtures/playground/config/minimal.nu b/tests/fixtures/playground/config/minimal.nu new file mode 100644 index 0000000000..018ff93548 --- /dev/null +++ b/tests/fixtures/playground/config/minimal.nu @@ -0,0 +1,28 @@ +let-env config = { + show_banner: false, + color_config: { + separator: { attr: n } + leading_trailing_space_bg: { attr: n } + header: { attr: n } + empty: { attr: n } + bool: { attr: n } + int: { attr: n } + filesize: { attr: n } + duration: { attr: n } + date: { attr: n } + range: { attr: n } + float: { attr: n } + string: { attr: n } + nothing: { attr: n } + binary: { attr: n } + cellpath: { attr: n } + row_index: { attr: n } + record: { attr: n } + list: { attr: n } + block: { attr: n } + hints: { attr: n } + }, + ls: { + use_ls_colors: false + } +} diff --git a/tests/fixtures/playground/config/startup.toml b/tests/fixtures/playground/config/startup.toml deleted file mode 100644 index c4b9e06785..0000000000 --- a/tests/fixtures/playground/config/startup.toml +++ /dev/null @@ -1,3 +0,0 @@ -skip_welcome_message = true - -startup = ["def hello-world [] { echo 'Nu World' }"] diff --git a/tests/parsing/mod.rs b/tests/parsing/mod.rs index d66ab04916..defce2dd33 100644 --- a/tests/parsing/mod.rs +++ b/tests/parsing/mod.rs @@ -8,7 +8,7 @@ fn source_file_relative_to_file() { nu source_file_relative.nu "#); - assert_eq!(actual.out, "5"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "5"); } #[test] @@ -28,7 +28,7 @@ fn run_nu_script_single_line() { nu single_line.nu "#); - assert_eq!(actual.out, "5"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "5"); } #[test] @@ -37,7 +37,7 @@ fn run_nu_script_multiline_start_pipe() { nu multiline_start_pipe.nu "#); - assert_eq!(actual.out, "4"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "4"); } #[test] @@ -46,7 +46,7 @@ fn run_nu_script_multiline_start_pipe_win() { nu multiline_start_pipe_win.nu "#); - assert_eq!(actual.out, "3"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "3"); } #[test] @@ -55,7 +55,7 @@ fn run_nu_script_multiline_end_pipe() { nu multiline_end_pipe.nu "#); - assert_eq!(actual.out, "2"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "2"); } #[test] @@ -64,7 +64,7 @@ fn run_nu_script_multiline_end_pipe_win() { nu multiline_end_pipe_win.nu "#); - assert_eq!(actual.out, "3"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "3"); } #[test] diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs index 8bf5ee854b..407ae24609 100644 --- a/tests/shell/environment/env.rs +++ b/tests/shell/environment/env.rs @@ -125,7 +125,7 @@ fn has_file_pwd() { let actual = nu!(cwd: dirs.test(), "nu spam.nu"); - assert!(actual.out.ends_with("has_file_pwd")); + assert!(nu_utils::strip_ansi_string_likely(actual.out).ends_with("has_file_pwd")); }) } diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 64cd742607..8ded804ce1 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -329,7 +329,7 @@ mod nu_commands { nu -c "echo 'foo'" "#); - assert_eq!(actual.out, "foo"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "foo"); } #[test] @@ -340,7 +340,7 @@ mod nu_commands { "#); // cargo for non rust project's exit code is 101. - assert_eq!(actual.out, "101") + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "101") }) } @@ -387,7 +387,7 @@ mod nu_script { nu script.nu "#); - assert_eq!(actual.out, "done"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "done"); } #[test] @@ -396,7 +396,7 @@ mod nu_script { nu script_multiline.nu "#); - assert_eq!(actual.out, "23"); + assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "23"); } }