From 80463d12fb861c227e1492bc83fd4df003c4fabc Mon Sep 17 00:00:00 2001 From: Reilly Wood <26268125+rgwood@users.noreply.github.com> Date: Sun, 8 Jan 2023 21:53:52 -0800 Subject: [PATCH] Revert "Primitives now use color closures..." (#7710) This temporarily reverts commit c5639cd9faf0baea005ea29e2d3555b9e0750736 (PR https://github.com/nushell/nushell/pull/7650). See [here](https://github.com/nushell/nushell/pull/7650#issuecomment-1375036213) for details; the PR is accidentally adding ANSI escape codes to strings piped to externals. I think we should revert the PR because we're only 1-2 days away from a release; reverting it will give us more time to land+test a proper fix in the next release cycle. --- 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, 27 insertions(+), 104 deletions(-) create mode 100644 tests/fixtures/playground/config/default.toml delete mode 100644 tests/fixtures/playground/config/minimal.nu create 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 c7efd75d57..4bd07cdbae 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -348,22 +348,7 @@ fn handle_table_command( ctrlc, metadata, ), - 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), + x => Ok(x), } } diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index f3743b6d6c..09b81febe2 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -158,7 +158,6 @@ 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 @@ -168,16 +167,12 @@ 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() { @@ -208,7 +203,6 @@ macro_rules! nu { struct NuOpts { cwd: Option, locale: Option, - minimal_config: Option, } nu!(@options [ ] $($token)*) @@ -282,8 +276,6 @@ 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 04f79422ca..125ce12f1b 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -32,17 +32,7 @@ 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")?; - // 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); + cmd.arg(name).envs(env); writeln!(file, "{}", input)?; @@ -55,16 +45,7 @@ pub fn run_test(input: &str, expected: &str) -> TestResult { let name = file.path(); let mut cmd = Command::cargo_bin("nu")?; - // 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.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 bd5587003f..da661a476b 100644 --- a/src/tests/test_config_path.rs +++ b/src/tests/test_config_path.rs @@ -6,18 +6,12 @@ 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, minimal_config: false, "$nu.config-path"); - assert_eq!( - nu_utils::strip_ansi_string_likely(actual.out), - config_path.to_string_lossy().to_string() - ); + let actual = nu!(cwd: &cwd, "$nu.config-path"); + assert_eq!(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!( - nu_utils::strip_ansi_string_likely(actual.out), - env_path.to_string_lossy().to_string() - ); + assert_eq!(actual.out, env_path.to_string_lossy().to_string()); } #[test] @@ -31,21 +25,14 @@ 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!( - nu_utils::strip_ansi_string_likely(actual.out), - config_path.to_string_lossy().to_string() - ); + assert_eq!(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!( - nu_utils::strip_ansi_string_likely(actual.out), - env_path.to_string_lossy().to_string() - ); + assert_eq!(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 new file mode 100644 index 0000000000..5939aacd41 --- /dev/null +++ b/tests/fixtures/playground/config/default.toml @@ -0,0 +1,3 @@ +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 deleted file mode 100644 index 018ff93548..0000000000 --- a/tests/fixtures/playground/config/minimal.nu +++ /dev/null @@ -1,28 +0,0 @@ -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 new file mode 100644 index 0000000000..c4b9e06785 --- /dev/null +++ b/tests/fixtures/playground/config/startup.toml @@ -0,0 +1,3 @@ +skip_welcome_message = true + +startup = ["def hello-world [] { echo 'Nu World' }"] diff --git a/tests/parsing/mod.rs b/tests/parsing/mod.rs index defce2dd33..d66ab04916 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!(nu_utils::strip_ansi_string_likely(actual.out), "5"); + assert_eq!(actual.out, "5"); } #[test] @@ -28,7 +28,7 @@ fn run_nu_script_single_line() { nu single_line.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "5"); + assert_eq!(actual.out, "5"); } #[test] @@ -37,7 +37,7 @@ fn run_nu_script_multiline_start_pipe() { nu multiline_start_pipe.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "4"); + assert_eq!(actual.out, "4"); } #[test] @@ -46,7 +46,7 @@ fn run_nu_script_multiline_start_pipe_win() { nu multiline_start_pipe_win.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "3"); + assert_eq!(actual.out, "3"); } #[test] @@ -55,7 +55,7 @@ fn run_nu_script_multiline_end_pipe() { nu multiline_end_pipe.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "2"); + assert_eq!(actual.out, "2"); } #[test] @@ -64,7 +64,7 @@ fn run_nu_script_multiline_end_pipe_win() { nu multiline_end_pipe_win.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "3"); + assert_eq!(actual.out, "3"); } #[test] diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs index 407ae24609..8bf5ee854b 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!(nu_utils::strip_ansi_string_likely(actual.out).ends_with("has_file_pwd")); + assert!(actual.out.ends_with("has_file_pwd")); }) } diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 8ded804ce1..64cd742607 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!(nu_utils::strip_ansi_string_likely(actual.out), "foo"); + assert_eq!(actual.out, "foo"); } #[test] @@ -340,7 +340,7 @@ mod nu_commands { "#); // cargo for non rust project's exit code is 101. - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "101") + assert_eq!(actual.out, "101") }) } @@ -387,7 +387,7 @@ mod nu_script { nu script.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "done"); + assert_eq!(actual.out, "done"); } #[test] @@ -396,7 +396,7 @@ mod nu_script { nu script_multiline.nu "#); - assert_eq!(nu_utils::strip_ansi_string_likely(actual.out), "23"); + assert_eq!(actual.out, "23"); } }