From 31e1410191b9c8025c5405397eb2d1c8ae899194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korn=C3=A9l=20Csernai?= <749306+csko@users.noreply.github.com> Date: Wed, 1 Feb 2023 15:03:05 -0800 Subject: [PATCH] respect use_ansi_coloring configuration (#7912) # Description Use the `use_ansi_coloring` configuration point to decide whether the output will have colors, where possible. Related: https://github.com/nushell/nushell/issues/7676 ![image](https://user-images.githubusercontent.com/749306/215435128-cbf5f4b8-aafa-4718-bf23-3f0fd19b63ba.png) - [x] `grid -c` - [x] `perf()` # User-Facing Changes _(List of all changes that impact the user experience here. This helps us keep track of breaking changes.)_ # 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. --- crates/nu-cli/src/config_files.rs | 1 + crates/nu-cli/src/repl.rs | 116 ++++++++++++++-- crates/nu-cli/src/util.rs | 1 + crates/nu-command/src/viewers/griddle.rs | 11 +- crates/nu-utils/src/utils.rs | 36 +++-- src/main.rs | 165 ++++++++++++++++++++--- 6 files changed, 286 insertions(+), 44 deletions(-) diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 4865f937be..6bf2ccf087 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -52,6 +52,7 @@ pub fn read_plugin_file( file!(), line!(), column!(), + engine_state.get_config().use_ansi_coloring, ); } diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index af6bbc7f62..cab4bd3ef5 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -45,6 +45,7 @@ pub fn evaluate_repl( entire_start_time: Instant, ) -> Result<()> { use reedline::{FileBackedHistory, Reedline, Signal}; + let use_color = engine_state.get_config().use_ansi_coloring; // Guard against invocation without a connected terminal. // reedline / crossterm event polling will fail without a connected tty @@ -72,6 +73,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); // seed env vars @@ -91,7 +93,14 @@ pub fn evaluate_repl( .map(i64::from) .unwrap_or(0); engine_state.history_session_id = hist_sesh; - perf("setup reedline", start_time, file!(), line!(), column!()); + perf( + "setup reedline", + start_time, + file!(), + line!(), + column!(), + use_color, + ); let config = engine_state.get_config(); @@ -115,7 +124,14 @@ pub fn evaluate_repl( }; line_editor = line_editor.with_history(history); }; - perf("setup history", start_time, file!(), line!(), column!()); + perf( + "setup history", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let sys = sysinfo::System::new(); @@ -136,6 +152,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); if let Some(s) = prerun_command { @@ -161,21 +178,42 @@ pub fn evaluate_repl( if let Err(err) = engine_state.merge_env(stack, cwd) { report_error_new(engine_state, &err); } - perf("merge env", start_time, file!(), line!(), column!()); + perf( + "merge env", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); //Reset the ctrl-c handler if let Some(ctrlc) = &mut engine_state.ctrlc { ctrlc.store(false, Ordering::SeqCst); } - perf("reset ctrlc", start_time, file!(), line!(), column!()); + perf( + "reset ctrlc", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // Reset the SIGQUIT handler if let Some(sig_quit) = engine_state.get_sig_quit() { sig_quit.store(false, Ordering::SeqCst); } - perf("reset sig_quit", start_time, file!(), line!(), column!()); + perf( + "reset sig_quit", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let config = engine_state.get_config(); @@ -198,6 +236,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); start_time = std::time::Instant::now(); @@ -218,7 +257,14 @@ pub fn evaluate_repl( .with_partial_completions(config.partial_completions) .with_ansi_colors(config.use_ansi_coloring) .with_cursor_config(cursor_config); - perf("reedline builder", start_time, file!(), line!(), column!()); + perf( + "reedline builder", + start_time, + file!(), + line!(), + column!(), + use_color, + ); let style_computer = StyleComputer::from_config(engine_state, stack); @@ -238,6 +284,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); start_time = std::time::Instant::now(); @@ -246,7 +293,14 @@ pub fn evaluate_repl( report_error(&working_set, &e); Reedline::create() }); - perf("reedline menus", start_time, file!(), line!(), column!()); + perf( + "reedline menus", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let buffer_editor = if !config.buffer_editor.is_empty() { @@ -275,6 +329,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); start_time = std::time::Instant::now(); @@ -283,7 +338,14 @@ pub fn evaluate_repl( warn!("Failed to sync history: {}", e); } } - perf("sync_history", start_time, file!(), line!(), column!()); + perf( + "sync_history", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // Changing the line editor based on the found keybindings @@ -307,7 +369,14 @@ pub fn evaluate_repl( line_editor } }; - perf("keybindings", start_time, file!(), line!(), column!()); + perf( + "keybindings", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // Right before we start our prompt and take input from the user, @@ -317,7 +386,14 @@ pub fn evaluate_repl( report_error_new(engine_state, &err); } } - perf("pre-prompt hook", start_time, file!(), line!(), column!()); + perf( + "pre-prompt hook", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // Next, check all the environment variables they ask for @@ -328,12 +404,26 @@ pub fn evaluate_repl( { report_error_new(engine_state, &error) } - perf("env-change hook", start_time, file!(), line!(), column!()); + perf( + "env-change hook", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let config = engine_state.get_config(); let prompt = prompt_update::update_prompt(config, engine_state, stack, &mut nu_prompt); - perf("update_prompt", start_time, file!(), line!(), column!()); + perf( + "update_prompt", + start_time, + file!(), + line!(), + column!(), + use_color, + ); entry_num += 1; @@ -587,6 +677,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); perf( @@ -595,6 +686,7 @@ pub fn evaluate_repl( file!(), line!(), column!(), + use_color, ); } diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs index 04fa0b23f2..e14c9dfa88 100644 --- a/crates/nu-cli/src/util.rs +++ b/crates/nu-cli/src/util.rs @@ -297,6 +297,7 @@ pub fn eval_source( file!(), line!(), column!(), + engine_state.get_config().use_ansi_coloring, ); true diff --git a/crates/nu-command/src/viewers/griddle.rs b/crates/nu-command/src/viewers/griddle.rs index 3cd87cf839..9d0b44d044 100644 --- a/crates/nu-command/src/viewers/griddle.rs +++ b/crates/nu-command/src/viewers/griddle.rs @@ -72,6 +72,7 @@ prints out the list properly."# None => None, }; let use_grid_icons = config.use_grid_icons; + let use_color: bool = color_param && config.use_ansi_coloring; match input { PipelineData::Value(Value::List { vals, .. }, ..) => { @@ -82,7 +83,7 @@ prints out the list properly."# items, call, width_param, - color_param, + use_color, separator_param, env_str, use_grid_icons, @@ -99,7 +100,7 @@ prints out the list properly."# items, call, width_param, - color_param, + use_color, separator_param, env_str, use_grid_icons, @@ -121,7 +122,7 @@ prints out the list properly."# items, call, width_param, - color_param, + use_color, separator_param, env_str, use_grid_icons, @@ -170,7 +171,7 @@ fn create_grid_output( items: Vec<(usize, String, String)>, call: &Call, width_param: Option, - color_param: bool, + use_color: bool, separator_param: Option, env_str: Option, use_grid_icons: bool, @@ -198,7 +199,7 @@ fn create_grid_output( for (_row_index, header, value) in items { // only output value if the header name is 'name' if header == "name" { - if color_param { + if use_color { if use_grid_icons { let no_ansi = nu_utils::strip_ansi_unlikely(&value); let path = std::path::Path::new(no_ansi.as_ref()); diff --git a/crates/nu-utils/src/utils.rs b/crates/nu-utils/src/utils.rs index 1e4ed4c58f..09b3a7a2de 100644 --- a/crates/nu-utils/src/utils.rs +++ b/crates/nu-utils/src/utils.rs @@ -65,13 +65,31 @@ pub fn get_ls_colors(lscolors_env_string: Option) -> LsColors { } // Log some performance metrics (green text with yellow timings) -pub fn perf(msg: &str, dur: std::time::Instant, file: &str, line: u32, column: u32) { - info!( - "perf: {}:{}:{} \x1b[32m{}\x1b[0m took \x1b[33m{:?}\x1b[0m", - file, - line, - column, - msg, - dur.elapsed(), - ); +pub fn perf( + msg: &str, + dur: std::time::Instant, + file: &str, + line: u32, + column: u32, + use_color: bool, +) { + if use_color { + info!( + "perf: {}:{}:{} \x1b[32m{}\x1b[0m took \x1b[33m{:?}\x1b[0m", + file, + line, + column, + msg, + dur.elapsed(), + ); + } else { + info!( + "perf: {}:{}:{} {} took {:?}", + file, + line, + column, + msg, + dur.elapsed(), + ); + } } diff --git a/src/main.rs b/src/main.rs index 6fcb947c16..185e32c763 100644 --- a/src/main.rs +++ b/src/main.rs @@ -68,6 +68,7 @@ fn main() -> Result<()> { let parsed_nu_cli_args = parse_commandline_args(&args_to_nushell.join(" "), &mut engine_state) .unwrap_or_else(|_| std::process::exit(1)); + let use_color = engine_state.get_config().use_ansi_coloring; if let Some(level) = parsed_nu_cli_args.log_level.map(|level| level.item) { let level = if Level::from_str(&level).is_ok() { level @@ -84,7 +85,14 @@ fn main() -> Result<()> { logger(|builder| configure(&level, &target, builder))?; // info!("start logging {}:{}:{}", file!(), line!(), column!()); - perf("start logging", start_time, file!(), line!(), column!()); + perf( + "start logging", + start_time, + file!(), + line!(), + column!(), + use_color, + ); } start_time = std::time::Instant::now(); @@ -103,7 +111,14 @@ fn main() -> Result<()> { "env-path", &parsed_nu_cli_args.env_file, ); - perf("set_config_path", start_time, file!(), line!(), column!()); + perf( + "set_config_path", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // keep this condition in sync with the branches below @@ -111,7 +126,14 @@ fn main() -> Result<()> { parsed_nu_cli_args.commands.is_none() && (script_name.is_empty() || parsed_nu_cli_args.interactive_shell.is_some()), ); - perf("acquire_terminal", start_time, file!(), line!(), column!()); + perf( + "acquire_terminal", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); if let Some(t) = parsed_nu_cli_args.threads { @@ -122,7 +144,14 @@ fn main() -> Result<()> { .build_global() .expect("error setting number of threads"); } - perf("set rayon threads", start_time, file!(), line!(), column!()); + perf( + "set rayon threads", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); if let Some(testbin) = &parsed_nu_cli_args.testbin { @@ -144,7 +173,14 @@ fn main() -> Result<()> { } std::process::exit(0) } - perf("run test_bins", start_time, file!(), line!(), column!()); + perf( + "run test_bins", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let input = if let Some(redirect_stdin) = &parsed_nu_cli_args.redirect_stdin { @@ -167,12 +203,26 @@ fn main() -> Result<()> { } else { PipelineData::empty() }; - perf("redirect stdin", start_time, file!(), line!(), column!()); + perf( + "redirect stdin", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // First, set up env vars as strings only gather_parent_env_vars(&mut engine_state, &init_cwd); - perf("gather env vars", start_time, file!(), line!(), column!()); + perf( + "gather env vars", + start_time, + file!(), + line!(), + column!(), + use_color, + ); let mut stack = nu_protocol::engine::Stack::new(); @@ -185,7 +235,14 @@ fn main() -> Result<()> { parsed_nu_cli_args.plugin_file, NUSHELL_FOLDER, ); - perf("read plugins", start_time, file!(), line!(), column!()); + perf( + "read plugins", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // only want to load config and env if relative argument is provided. @@ -199,7 +256,14 @@ fn main() -> Result<()> { } else { config_files::read_default_env_file(&mut engine_state, &mut stack) } - perf("read env.nu", start_time, file!(), line!(), column!()); + perf( + "read env.nu", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); if parsed_nu_cli_args.config_file.is_some() { @@ -210,7 +274,14 @@ fn main() -> Result<()> { false, ); } - perf("read config.nu", start_time, file!(), line!(), column!()); + perf( + "read config.nu", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let ret_val = evaluate_commands( @@ -220,7 +291,14 @@ fn main() -> Result<()> { input, parsed_nu_cli_args.table_mode, ); - perf("evaluate_commands", start_time, file!(), line!(), column!()); + perf( + "evaluate_commands", + start_time, + file!(), + line!(), + column!(), + use_color, + ); match ret_val { Ok(Some(exit_code)) => std::process::exit(exit_code as i32), @@ -237,7 +315,14 @@ fn main() -> Result<()> { parsed_nu_cli_args.plugin_file, NUSHELL_FOLDER, ); - perf("read plugins", start_time, file!(), line!(), column!()); + perf( + "read plugins", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); // only want to load config and env if relative argument is provided. @@ -251,7 +336,14 @@ fn main() -> Result<()> { } else { config_files::read_default_env_file(&mut engine_state, &mut stack) } - perf("read env.nu", start_time, file!(), line!(), column!()); + perf( + "read env.nu", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); if parsed_nu_cli_args.config_file.is_some() { @@ -262,7 +354,14 @@ fn main() -> Result<()> { false, ); } - perf("read config.nu", start_time, file!(), line!(), column!()); + perf( + "read config.nu", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let ret_val = evaluate_file( @@ -272,7 +371,14 @@ fn main() -> Result<()> { &mut stack, input, ); - perf("evaluate_file", start_time, file!(), line!(), column!()); + perf( + "evaluate_file", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let last_exit_code = stack.get_env_var(&engine_state, "LAST_EXIT_CODE"); @@ -284,7 +390,14 @@ fn main() -> Result<()> { } } } - perf("get exit code", start_time, file!(), line!(), column!()); + perf( + "get exit code", + start_time, + file!(), + line!(), + column!(), + use_color, + ); ret_val } else { @@ -299,7 +412,16 @@ fn main() -> Result<()> { parsed_nu_cli_args.env_file, parsed_nu_cli_args.login_shell.is_some(), ); - perf("setup_config", start_time, file!(), line!(), column!()); + // Reload use_color from config in case it's different from the default value + let use_color = engine_state.get_config().use_ansi_coloring; + perf( + "setup_config", + start_time, + file!(), + line!(), + column!(), + use_color, + ); start_time = std::time::Instant::now(); let ret_val = evaluate_repl( @@ -309,7 +431,14 @@ fn main() -> Result<()> { parsed_nu_cli_args.execute, entire_start_time, ); - perf("evaluate_repl", start_time, file!(), line!(), column!()); + perf( + "evaluate_repl", + start_time, + file!(), + line!(), + column!(), + use_color, + ); ret_val }