Revert "Primitives now use color closures..." (#7710)

This temporarily reverts commit c5639cd9fa
(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.
This commit is contained in:
Reilly Wood 2023-01-08 21:53:52 -08:00 committed by GitHub
parent cef05d3553
commit 80463d12fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 27 additions and 104 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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]

View File

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

View File

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