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

@@ -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),
     }
 }
 

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<String>,
             locale: Option<String>,
+            minimal_config: Option<bool>,
         }
 
         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()

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"),

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()
+    );
 }

tests/fixtures/playground/config/default.toml

@@ -1,3 +0,0 @@
-skip_welcome_message = true
-filesize_format = "auto"
-rm_always_trash = false

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
+	}
+}

tests/fixtures/playground/config/startup.toml

@@ -1,3 +0,0 @@
-skip_welcome_message = true
-
-startup = ["def hello-world [] { echo 'Nu World' }"]

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]

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

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