From 60aad681f358fc7285ea72c77faa8782a060eab1 Mon Sep 17 00:00:00 2001 From: Ethan P Date: Mon, 22 Mar 2021 17:21:51 -0700 Subject: [PATCH 1/5] Improve handling of ANSI passthrough --- src/lib.rs | 1 + src/printer.rs | 43 ++-------- src/vscreen.rs | 212 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 35 deletions(-) create mode 100644 src/vscreen.rs diff --git a/src/lib.rs b/src/lib.rs index 0c41716ddb..3d1ea73f5b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,6 +42,7 @@ pub(crate) mod printer; pub mod style; pub(crate) mod syntax_mapping; mod terminal; +mod vscreen; pub(crate) mod wrapping; pub use pretty_printer::{Input, PrettyPrinter}; diff --git a/src/printer.rs b/src/printer.rs index 45caf15787..fa4749c1e7 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -30,6 +30,7 @@ use crate::input::OpenedInput; use crate::line_range::RangeCheckResult; use crate::preprocessor::{expand_tabs, replace_nonprintable}; use crate::terminal::{as_terminal_escaped, to_ansi_color}; +use crate::vscreen::AnsiStyle; use crate::wrapping::WrappingMode; pub(crate) trait Printer { @@ -104,7 +105,7 @@ pub(crate) struct InteractivePrinter<'a> { config: &'a Config<'a>, decorations: Vec>, panel_width: usize, - ansi_prefix_sgr: String, + ansi_style: AnsiStyle, content_type: Option, #[cfg(feature = "git")] pub line_changes: &'a Option, @@ -188,7 +189,7 @@ impl<'a> InteractivePrinter<'a> { config, decorations, content_type: input.reader.content_type, - ansi_prefix_sgr: String::new(), + ansi_style: AnsiStyle::new(), #[cfg(feature = "git")] line_changes, highlighter, @@ -466,31 +467,12 @@ impl<'a> Printer for InteractivePrinter<'a> { } else { for &(style, region) in regions.iter() { let ansi_iterator = AnsiCodeIterator::new(region); - let mut ansi_prefix: String = String::new(); for chunk in ansi_iterator { match chunk { // ANSI escape passthrough. - (text, true) => { - let is_ansi_csi = text.starts_with("\x1B["); - - if is_ansi_csi && text.ends_with('m') { - // It's an ANSI SGR sequence. - // We should be mostly safe to just append these together. - ansi_prefix.push_str(text); - if text == "\x1B[0m" { - self.ansi_prefix_sgr = "\x1B[0m".to_owned(); - } else { - self.ansi_prefix_sgr.push_str(text); - } - } else if is_ansi_csi { - // It's a regular CSI sequence. - // We should be mostly safe to just append these together. - ansi_prefix.push_str(text); - } else { - // It's probably a VT100 code. - // Passing it through is the safest bet. - write!(handle, "{}", text)?; - } + (ansi, true) => { + self.ansi_style.update(ansi); + write!(handle, "{}", ansi)?; } // Regular text. @@ -540,10 +522,7 @@ impl<'a> Printer for InteractivePrinter<'a> { "{}\n{}", as_terminal_escaped( style, - &*format!( - "{}{}{}", - self.ansi_prefix_sgr, ansi_prefix, line_buf - ), + &*format!("{}{}", self.ansi_style, line_buf), self.config.true_color, self.config.colored_output, self.config.use_italic_text, @@ -569,19 +548,13 @@ impl<'a> Printer for InteractivePrinter<'a> { "{}", as_terminal_escaped( style, - &*format!( - "{}{}{}", - self.ansi_prefix_sgr, ansi_prefix, line_buf - ), + &*format!("{}{}", self.ansi_style, line_buf), self.config.true_color, self.config.colored_output, self.config.use_italic_text, background_color ) )?; - - // Clear the ANSI prefix buffer. - ansi_prefix.clear(); } } } diff --git a/src/vscreen.rs b/src/vscreen.rs new file mode 100644 index 0000000000..0352d253c4 --- /dev/null +++ b/src/vscreen.rs @@ -0,0 +1,212 @@ +use std::fmt::{Display, Formatter}; + +// Wrapper to avoid unnecessary branching when input doesn't have ANSI escape sequences. +pub struct AnsiStyle { + attributes: Option, +} + +impl AnsiStyle { + pub fn new() -> Self { + AnsiStyle { attributes: None } + } + + pub fn update(&mut self, sequence: &str) -> bool { + match &mut self.attributes { + Some(a) => a.update(sequence), + None => { + self.attributes = Some(Attributes::new()); + self.attributes.as_mut().unwrap().update(sequence) + } + } + } +} + +impl Display for AnsiStyle { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self.attributes { + Some(ref a) => a.fmt(f), + None => Ok(()), + } + } +} + +struct Attributes { + foreground: String, + background: String, + underlined: String, + + /// The character set to use. + /// REGEX: `\^[()][AB0-3]` + charset: String, + + /// A buffer for unknown sequences. + unknown_buffer: String, + + /// ON: ^[1m + /// OFF: ^[22m + bold: String, + + /// ON: ^[2m + /// OFF: ^[22m + dim: String, + + /// ON: ^[4m + /// OFF: ^[24m + underline: String, + + /// ON: ^[3m + /// OFF: ^[23m + italic: String, + + /// ON: ^[9m + /// OFF: ^[29m + strike: String, +} + +impl Attributes { + pub fn new() -> Self { + Attributes { + foreground: "".to_owned(), + background: "".to_owned(), + underlined: "".to_owned(), + charset: "".to_owned(), + unknown_buffer: "".to_owned(), + bold: "".to_owned(), + dim: "".to_owned(), + underline: "".to_owned(), + italic: "".to_owned(), + strike: "".to_owned(), + } + } + + /// Update the attributes with an escape sequence. + /// Returns `false` if the sequence is unsupported. + pub fn update(&mut self, sequence: &str) -> bool { + let mut chars = sequence.char_indices().skip(1); + + if let Some((_, t)) = chars.next() { + match t { + '(' => self.update_with_charset('(', chars.map(|(_, c)| c)), + ')' => self.update_with_charset(')', chars.map(|(_, c)| c)), + '[' => { + if let Some((i, last)) = chars.last() { + // SAFETY: Always starts with ^[ and ends with m. + self.update_with_csi(last, &sequence[2..i]) + } else { + false + } + } + _ => self.update_with_unsupported(sequence), + } + } else { + false + } + } + + fn sgr_reset(&mut self) { + self.foreground.clear(); + self.background.clear(); + self.underlined.clear(); + self.bold.clear(); + self.dim.clear(); + self.underline.clear(); + self.italic.clear(); + self.strike.clear(); + } + + fn update_with_sgr(&mut self, parameters: &str) -> bool { + let mut iter = parameters + .split(';') + .map(|p| if p.is_empty() { "0" } else { p }) + .map(|p| p.parse::()) + .map(|p| p.unwrap_or(0)); // Treat errors as 0. + + while let Some(p) = iter.next() { + match p { + 0 => self.sgr_reset(), + 1 => self.bold = format!("\x1B[{}m", parameters), + 2 => self.dim = format!("\x1B[{}m", parameters), + 3 => self.italic = format!("\x1B[{}m", parameters), + 4 => self.underline = format!("\x1B[{}m", parameters), + 23 => self.italic.clear(), + 24 => self.underline.clear(), + 22 => { + self.bold.clear(); + self.dim.clear(); + } + 30..=39 => self.foreground = Self::parse_color(p, &mut iter), + 40..=49 => self.background = Self::parse_color(p, &mut iter), + 58..=59 => self.underlined = Self::parse_color(p, &mut iter), + 90..=97 => self.foreground = Self::parse_color(p, &mut iter), + 100..=107 => self.foreground = Self::parse_color(p, &mut iter), + _ => { + // Unsupported SGR sequence. + // Be compatible and pretend one just wasn't was provided. + } + } + } + + true + } + + fn update_with_csi(&mut self, finalizer: char, sequence: &str) -> bool { + if finalizer == 'm' { + self.update_with_sgr(sequence) + } else { + false + } + } + + fn update_with_unsupported(&mut self, sequence: &str) -> bool { + self.unknown_buffer.push_str(&sequence); + false + } + + fn update_with_charset(&mut self, kind: char, set: impl Iterator) -> bool { + self.charset = format!("\x1B{}{}", kind, set.take(1).collect::()); + true + } + + fn parse_color(color: u16, parameters: &mut dyn Iterator) -> String { + match color % 10 { + 8 => match parameters.next() { + Some(5) /* 256-color */ => format!("\x1B[{};5;{}m", color, join(";", 1, parameters)), + Some(2) /* 24-bit color */ => format!("\x1B[{};2;{}m", color, join(";", 3, parameters)), + Some(c) => format!("\x1B[{};{}m", color, c), + _ => "".to_owned(), + }, + 9 => "".to_owned(), + _ => format!("\x1B[{}m", color), + } + } +} + +impl Display for Attributes { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}{}{}{}{}{}{}{}{}", + self.foreground, + self.background, + self.underlined, + self.charset, + self.bold, + self.dim, + self.underline, + self.italic, + self.strike, + ) + } +} + +fn join( + delimiter: &str, + limit: usize, + iterator: &mut dyn Iterator, +) -> String { + iterator + .take(limit) + .map(|i| i.to_string()) + .collect::>() + .join(delimiter) +} From 5aa94e8765e992cf669fcbb71a54ae8f5a57554c Mon Sep 17 00:00:00 2001 From: Ethan P Date: Mon, 22 Mar 2021 18:13:56 -0700 Subject: [PATCH 2/5] Fix ANSI passthrough for --wrap=never --- src/printer.rs | 66 +++++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/src/printer.rs b/src/printer.rs index fa4749c1e7..b9af1363cd 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -431,33 +431,49 @@ impl<'a> Printer for InteractivePrinter<'a> { let italics = self.config.use_italic_text; for &(style, region) in regions.iter() { - let text = &*self.preprocess(region, &mut cursor_total); - let text_trimmed = text.trim_end_matches(|c| c == '\r' || c == '\n'); - write!( - handle, - "{}", - as_terminal_escaped( - style, - text_trimmed, - true_color, - colored_output, - italics, - background_color - ) - )?; + let ansi_iterator = AnsiCodeIterator::new(region); + for chunk in ansi_iterator { + match chunk { + // ANSI escape passthrough. + (ansi, true) => { + self.ansi_style.update(ansi); + write!(handle, "{}", ansi)?; + } - if text.len() != text_trimmed.len() { - if let Some(background_color) = background_color { - let mut ansi_style = Style::default(); - ansi_style.background = to_ansi_color(background_color, true_color); - let width = if cursor_total <= cursor_max { - cursor_max - cursor_total + 1 - } else { - 0 - }; - write!(handle, "{}", ansi_style.paint(" ".repeat(width)))?; + // Regular text. + (text, false) => { + let text = &*self.preprocess(text, &mut cursor_total); + let text_trimmed = text.trim_end_matches(|c| c == '\r' || c == '\n'); + + write!( + handle, + "{}", + as_terminal_escaped( + style, + &format!("{}{}", self.ansi_style, text_trimmed), + true_color, + colored_output, + italics, + background_color + ) + )?; + + if text.len() != text_trimmed.len() { + if let Some(background_color) = background_color { + let mut ansi_style = Style::default(); + ansi_style.background = + to_ansi_color(background_color, true_color); + let width = if cursor_total <= cursor_max { + cursor_max - cursor_total + 1 + } else { + 0 + }; + write!(handle, "{}", ansi_style.paint(" ".repeat(width)))?; + } + write!(handle, "{}", &text[text_trimmed.len()..])?; + } + } } - write!(handle, "{}", &text[text_trimmed.len()..])?; } } From f4558911b0ab27a33b018ddebabf96e973d362b6 Mon Sep 17 00:00:00 2001 From: Ethan P Date: Tue, 23 Mar 2021 16:41:31 -0700 Subject: [PATCH 3/5] Add test for ANSI passthrough --- tests/integration_tests.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 3e3d029a18..01131995c6 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1138,3 +1138,22 @@ fn grid_for_file_without_newline() { ) .stderr(""); } + +// Ensure that ANSI passthrough is emitted properly for both wrapping and non-wrapping printer. +#[test] +fn ansi_passthrough_emit() { + for wrapping in vec!["never", "character"] { + bat() + .arg("--paging=never") + .arg("--color=never") + .arg("--terminal-width=80") + .arg(format!("--wrap={}", wrapping)) + .arg("--decorations=always") + .arg("--style=plain") + .write_stdin("\x1B[33mColor\nColor \x1B[m\nPlain\n") + .assert() + .success() + .stdout("\x1B[33m\x1B[33mColor\n\x1B[33mColor \x1B[m\nPlain\n") + .stderr(""); + } +} From 8fa928c0b12a3566353df1c41909f15872db68ae Mon Sep 17 00:00:00 2001 From: Ethan P Date: Tue, 23 Mar 2021 16:47:42 -0700 Subject: [PATCH 4/5] Update CHANGELOG.md for #1596 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b02eb414d..29e8cba5e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ ## Bugfixes +- Fix for poor performance when ANSI escape sequences are piped to `bat`, see #1596 (@eth-p) +- Fix for incorrect handling of ANSI escape sequences when using `--wrap=never`, see #1596 (@eth-p) + ## Other - `Input::ordinary_file` and `Input::with_name` now accept `Path` rather than `OsStr` see #1571 (@matklad) From 9f36470a05fdcdbeb0fa717d3460643cf4c97c3a Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Wed, 8 Dec 2021 08:52:00 +0100 Subject: [PATCH 5/5] Run `cargo clippy --fix --all-targets --all-features` --- src/vscreen.rs | 2 +- tests/integration_tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vscreen.rs b/src/vscreen.rs index 0352d253c4..ea5d4da6f0 100644 --- a/src/vscreen.rs +++ b/src/vscreen.rs @@ -158,7 +158,7 @@ impl Attributes { } fn update_with_unsupported(&mut self, sequence: &str) -> bool { - self.unknown_buffer.push_str(&sequence); + self.unknown_buffer.push_str(sequence); false } diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 21f0fb1040..986d99527a 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -1229,7 +1229,7 @@ fn grid_for_file_without_newline() { // Ensure that ANSI passthrough is emitted properly for both wrapping and non-wrapping printer. #[test] fn ansi_passthrough_emit() { - for wrapping in vec!["never", "character"] { + for wrapping in &["never", "character"] { bat() .arg("--paging=never") .arg("--color=never")