From e158adebfcab0cdd941dda578200c447d284b38b Mon Sep 17 00:00:00 2001 From: Daniel Xu Date: Sat, 28 Dec 2024 09:44:54 -0800 Subject: [PATCH 1/3] ui: Do not strip styling for unattended output The original need for stripping ansi escape sequences was so we could apply our own styling when the terminal is attended and we're doing custom terminal drawing. However, when the output is unattended, we do not have a need for stripping ansi escape sequences. Lots of CIs force coloring even though output is not attended. This is so the logs are easier to read. vmtest should respect that and not strip ansi escape sequences when output is unattended. --- src/ui.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/ui.rs b/src/ui.rs index 6f770f0..bc51fa7 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -91,12 +91,20 @@ impl Stage { // Compute new visible lines let trimmed_line = line.trim_end(); - let stripped_line = strip_ansi_codes(trimmed_line); - let styled_line = match &custom { - Some(s) => s.apply_to(stripped_line), - None => style(stripped_line).dim(), + let styled_line = if self.term.features().is_attended() { + // If output is attended, we do custom window with our own styling. + // Therefore, we need to strip away any existing formatting. + let stripped = strip_ansi_codes(trimmed_line); + let styled = match &custom { + Some(s) => s.apply_to(stripped), + None => style(stripped).dim(), + }; + styled.to_string() + } else { + // If output is not attended, we do pass through + trimmed_line.to_string() }; - self.lines.push(styled_line.to_string()); + self.lines.push(styled_line); // Unwrap should never fail b/c we're sizing with `min()` let window = self.lines.windows(self.window_size()).last().unwrap(); From 596e920ba4a5cc4dae486906e2f5107330518063 Mon Sep 17 00:00:00 2001 From: Daniel Xu Date: Sat, 28 Dec 2024 10:57:31 -0800 Subject: [PATCH 2/3] ui: Add VMTEST_NO_UI environment variable This disable the terminal drawing UI in vmtest. Previous workaround was `vmtest ... | cat -`. But that's only obvious to people who are deeply familiar with linux/unix. So add a more mundane environment variable and document it. --- src/main.rs | 6 +++++- src/ui.rs | 23 +++++++++++++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/main.rs b/src/main.rs index 514fc7f..3e705eb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,8 +13,12 @@ use regex::Regex; use vmtest::{Config, Target, Ui, VMConfig, Vmtest}; +const HELP_ENV_VARS: &str = r#"Environment variables: + VMTEST_NO_UI Set to disable UI [default: unset] +"#; + #[derive(Parser, Debug)] -#[clap(version)] +#[clap(version, disable_colored_help=true, after_help=HELP_ENV_VARS)] struct Args { /// Path to config file #[clap(short, long)] diff --git a/src/ui.rs b/src/ui.rs index bc51fa7..b48296a 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -1,4 +1,5 @@ use std::cmp::min; +use std::env; use std::sync::mpsc::{channel, Receiver}; use std::thread; @@ -26,9 +27,19 @@ struct Stage { expand: bool, } -/// Helper to clear lines depending on whether or not a tty is attached +/// Returns whether the "windowed" UI is enabled or not. +/// If disabled, output is (mostly) just passed through. +fn ui_enabled(term: &Term) -> bool { + if env::var_os("VMTEST_NO_UI").is_some() { + return false; + } + + term.features().is_attended() +} + +/// Helper to clear lines depending on whether or not UI is enabled fn clear_last_lines(term: &Term, n: usize) { - if term.features().is_attended() { + if ui_enabled(term) { term.clear_last_lines(n).unwrap(); } } @@ -68,7 +79,7 @@ impl Stage { /// We kinda hack this to return 1 if we're not writing to terminal. /// Should really find a cleaner solution. fn window_size(&self) -> usize { - if self.term.features().is_attended() { + if ui_enabled(&self.term) { min(self.lines.len(), WINDOW_LENGTH) } else { min(self.lines.len(), 1) @@ -91,8 +102,8 @@ impl Stage { // Compute new visible lines let trimmed_line = line.trim_end(); - let styled_line = if self.term.features().is_attended() { - // If output is attended, we do custom window with our own styling. + let styled_line = if ui_enabled(&self.term) { + // If UI is enabled, we do custom window with our own styling. // Therefore, we need to strip away any existing formatting. let stripped = strip_ansi_codes(trimmed_line); let styled = match &custom { @@ -133,7 +144,7 @@ impl Stage { impl Drop for Stage { fn drop(&mut self) { clear_last_lines(&self.term, self.window_size()); - if self.expand && self.term.features().is_attended() { + if self.expand && ui_enabled(&self.term) { for line in &self.lines { self.term .write_line(line) From 404f02b46619ec18ebab688d71f3b37e35c3942a Mon Sep 17 00:00:00 2001 From: Daniel Xu Date: Sat, 28 Dec 2024 11:37:28 -0800 Subject: [PATCH 3/3] ui: Do not clip strings for unattended output String clipping is only useful if we're rendering the UI. If output is unattended or user has requested UI disabled, no need to clip output. --- src/ui.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/ui.rs b/src/ui.rs index b48296a..90b44f2 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -106,10 +106,22 @@ impl Stage { // If UI is enabled, we do custom window with our own styling. // Therefore, we need to strip away any existing formatting. let stripped = strip_ansi_codes(trimmed_line); + + // Clip output to fit in terminal. + // + // Note this _does_not_ handle characters that expand to multiple columns, + // like tabs or other fancy unicode. This is known to corrupt UI visuals. There + // is no real solution to this without doing a mini terminal emulator AFAIK. + // The workaround is to set VMTEST_NO_UI. + let width = self.term.size_checked().map(|(_, w)| w).unwrap_or(u16::MAX); + let clipped = truncate_str(&stripped, width as usize, "..."); + + // Apply styling let styled = match &custom { - Some(s) => s.apply_to(stripped), - None => style(stripped).dim(), + Some(s) => s.apply_to(clipped), + None => style(clipped).dim(), }; + styled.to_string() } else { // If output is not attended, we do pass through @@ -119,16 +131,9 @@ impl Stage { // Unwrap should never fail b/c we're sizing with `min()` let window = self.lines.windows(self.window_size()).last().unwrap(); - // Get terminal width, if any - let width = match self.term.size_checked() { - Some((_, w)) => w, - _ => u16::MAX, - }; - // Print visible lines for line in window { - let clipped = truncate_str(line, width as usize - 3, "..."); - self.term.write_line(&format!("{}", clipped)).unwrap(); + self.term.write_line(line).unwrap(); } }