From b74c125c43bb28e0b2b8f5188dac5d7348deae95 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 14:54:10 -0700 Subject: [PATCH 1/6] Support merging `--style` arguments The `overrides_with` clap builder option was removed because it interfered with the matcher's ability to retain all occurrences of `--style`. The behavior it covered is expressed within the new `forced_style_components` function. --- src/bin/bat/app.rs | 76 ++++++++++------ src/bin/bat/clap_app.rs | 31 ++----- src/style.rs | 194 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 250 insertions(+), 51 deletions(-) diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index 0d2600b2..c95c2bd1 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -2,11 +2,13 @@ use std::collections::HashSet; use std::env; use std::io::IsTerminal; use std::path::{Path, PathBuf}; +use std::str::FromStr; use crate::{ clap_app, config::{get_args_from_config_file, get_args_from_env_opts_var, get_args_from_env_vars}, }; +use bat::style::StyleComponentList; use bat::StripAnsiMode; use clap::ArgMatches; @@ -86,7 +88,6 @@ impl App { // .. and the rest at the end cli_args.for_each(|a| args.push(a)); - args }; @@ -364,34 +365,57 @@ impl App { Ok(file_input) } + fn forced_style_components(&self) -> Option { + // No components if `--decorations=never``. + if self + .matches + .get_one::("decorations") + .map(|s| s.as_str()) + == Some("never") + { + return Some(StyleComponents(HashSet::new())); + } + + // Only line numbers if `--number`. + if self.matches.get_flag("number") { + return Some(StyleComponents(HashSet::from([ + StyleComponent::LineNumbers, + ]))); + } + + // Plain if `--plain` is specified at least once. + if self.matches.get_count("plain") > 0 { + return Some(StyleComponents(HashSet::from([StyleComponent::Plain]))); + } + + // Default behavior. + None + } + fn style_components(&self) -> Result { let matches = &self.matches; - let mut styled_components = StyleComponents( - if matches.get_one::("decorations").map(|s| s.as_str()) == Some("never") { - HashSet::new() - } else if matches.get_flag("number") { - [StyleComponent::LineNumbers].iter().cloned().collect() - } else if 0 < matches.get_count("plain") { - [StyleComponent::Plain].iter().cloned().collect() - } else { - matches - .get_one::("style") - .map(|styles| { - styles - .split(',') - .map(|style| style.parse::()) - .filter_map(|style| style.ok()) - .collect::>() - }) - .unwrap_or_else(|| vec![StyleComponent::Default]) + let mut styled_components = match self.forced_style_components() { + Some(forced_components) => forced_components, + + // Parse the `--style` arguments and merge them. + None if matches.contains_id("style") => { + let lists = matches + .get_many::("style") + .expect("styles present") + .map(|v| StyleComponentList::from_str(v)) + .collect::>>()?; + + StyleComponentList::to_components(lists, self.interactive_output) + } + + // Use the default. + None => StyleComponents(HashSet::from_iter( + StyleComponent::Default + .components(self.interactive_output) .into_iter() - .map(|style| style.components(self.interactive_output)) - .fold(HashSet::new(), |mut acc, components| { - acc.extend(components.iter().cloned()); - acc - }) - }, - ); + .cloned(), + )), + }; // If `grid` is set, remove `rule` as it is a subset of `grid`, and print a warning. if styled_components.grid() && styled_components.0.remove(&StyleComponent::Rule) { diff --git a/src/bin/bat/clap_app.rs b/src/bin/bat/clap_app.rs index e70b1a5b..8e79174b 100644 --- a/src/bin/bat/clap_app.rs +++ b/src/bin/bat/clap_app.rs @@ -1,9 +1,11 @@ +use bat::style::StyleComponentList; use clap::{ crate_name, crate_version, value_parser, Arg, ArgAction, ArgGroup, ColorChoice, Command, }; use once_cell::sync::Lazy; use std::env; use std::path::{Path, PathBuf}; +use std::str::FromStr; static VERSION: Lazy = Lazy::new(|| { #[cfg(feature = "bugreport")] @@ -419,34 +421,13 @@ pub fn build_app(interactive_output: bool) -> Command { .arg( Arg::new("style") .long("style") + .action(ArgAction::Append) .value_name("components") - .overrides_with("style") - .overrides_with("plain") - .overrides_with("number") // Cannot use claps built in validation because we have to turn off clap's delimiters .value_parser(|val: &str| { - let mut invalid_vals = val.split(',').filter(|style| { - !&[ - "auto", - "full", - "default", - "plain", - "header", - "header-filename", - "header-filesize", - "grid", - "rule", - "numbers", - "snip", - #[cfg(feature = "git")] - "changes", - ].contains(style) - }); - - if let Some(invalid) = invalid_vals.next() { - Err(format!("Unknown style, '{invalid}'")) - } else { - Ok(val.to_owned()) + match StyleComponentList::from_str(val) { + Err(err) => Err(err), + Ok(_) => Ok(val.to_owned()), } }) .help( diff --git a/src/style.rs b/src/style.rs index 652b3743..8c2cc05d 100644 --- a/src/style.rs +++ b/src/style.rs @@ -138,3 +138,197 @@ impl StyleComponents { self.0.clear(); } } + +#[derive(Debug, PartialEq)] +enum ComponentAction { + Override, + Add, + Remove, +} + +impl ComponentAction { + fn extract_from_str(string: &str) -> (ComponentAction, &str) { + match string.chars().next() { + Some('-') => (ComponentAction::Remove, string.strip_prefix('-').unwrap()), + Some('+') => (ComponentAction::Add, string.strip_prefix('+').unwrap()), + _ => (ComponentAction::Override, string), + } + } +} + +/// A list of [StyleComponent] that can be parsed from a string. +pub struct StyleComponentList(Vec<(ComponentAction, StyleComponent)>); + +impl StyleComponentList { + fn expand_into(&self, components: &mut HashSet, interactive_terminal: bool) { + for (action, component) in self.0.iter() { + let subcomponents = component.components(interactive_terminal); + + use ComponentAction::*; + match action { + Override | Add => components.extend(subcomponents), + Remove => components.retain(|c| !subcomponents.contains(c)), + } + } + } + + /// Returns `true` if any component in the list was not prefixed with `+` or `-`. + fn contains_override(&self) -> bool { + self.0.iter().any(|(a, _)| *a == ComponentAction::Override) + } + + /// Combines multiple [StyleComponentList]s into a single [StyleComponents] set. + /// + /// ## Precedence + /// The most recent list will take precedence and override all previous lists + /// unless it only contains components prefixed with `-` or `+`. When this + /// happens, the list's components will be merged into the previous list. + /// + /// ## Example + /// ```text + /// [numbers,grid] + [header,changes] -> [header,changes] + /// [numbers,grid] + [+header,-grid] -> [numbers,header] + /// ``` + pub fn to_components( + lists: impl IntoIterator, + interactive_terminal: bool, + ) -> StyleComponents { + StyleComponents( + lists + .into_iter() + .fold(HashSet::new(), |mut components, list| { + if list.contains_override() { + components.clear(); + } + + list.expand_into(&mut components, interactive_terminal); + components + }), + ) + } +} + +impl Default for StyleComponentList { + fn default() -> Self { + StyleComponentList(vec![(ComponentAction::Override, StyleComponent::Default)]) + } +} + +impl FromStr for StyleComponentList { + type Err = Error; + + fn from_str(s: &str) -> Result { + Ok(StyleComponentList( + s.split(",") + .map(|s| ComponentAction::extract_from_str(s)) // If the component starts with "-", it's meant to be removed + .map(|(a, s)| Ok((a, StyleComponent::from_str(s)?))) + .collect::>>()?, + )) + } +} + +#[cfg(test)] +mod test { + use std::collections::HashSet; + use std::str::FromStr; + + use super::ComponentAction::*; + use super::StyleComponent::*; + use super::StyleComponentList; + + #[test] + pub fn style_component_list_parse() { + assert_eq!( + StyleComponentList::from_str("grid,+numbers,snip,-snip,header") + .expect("no error") + .0, + vec![ + (Override, Grid), + (Add, LineNumbers), + (Override, Snip), + (Remove, Snip), + (Override, Header), + ] + ); + + assert!(StyleComponentList::from_str("not-a-component").is_err()); + assert!(StyleComponentList::from_str("grid,not-a-component").is_err()); + assert!(StyleComponentList::from_str("numbers,-not-a-component").is_err()); + } + + #[test] + pub fn style_component_list_to_components() { + assert_eq!( + StyleComponentList::to_components( + vec![StyleComponentList::from_str("grid,numbers").expect("no error")], + false + ) + .0, + HashSet::from([Grid, LineNumbers]) + ); + } + + #[test] + pub fn style_component_list_to_components_removes_negated() { + assert_eq!( + StyleComponentList::to_components( + vec![StyleComponentList::from_str("grid,numbers,-grid").expect("no error")], + false + ) + .0, + HashSet::from([LineNumbers]) + ); + } + + #[test] + pub fn style_component_list_to_components_expands_subcomponents() { + assert_eq!( + StyleComponentList::to_components( + vec![StyleComponentList::from_str("full").expect("no error")], + false + ) + .0, + HashSet::from_iter(Full.components(true).to_owned()) + ); + } + + #[test] + pub fn style_component_list_expand_negates_subcomponents() { + assert!(!StyleComponentList::to_components( + vec![StyleComponentList::from_str("full,-numbers").expect("no error")], + true + ) + .numbers()); + } + + #[test] + pub fn style_component_list_to_components_precedence_overrides_previous_lists() { + assert_eq!( + StyleComponentList::to_components( + vec![ + StyleComponentList::from_str("grid").expect("no error"), + StyleComponentList::from_str("numbers").expect("no error"), + ], + false + ) + .0, + HashSet::from([LineNumbers]) + ); + } + + #[test] + pub fn style_component_list_to_components_precedence_merges_previous_lists() { + assert_eq!( + StyleComponentList::to_components( + vec![ + StyleComponentList::from_str("grid,header").expect("no error"), + StyleComponentList::from_str("-grid").expect("no error"), + StyleComponentList::from_str("+numbers").expect("no error"), + ], + false + ) + .0, + HashSet::from([HeaderFilename, LineNumbers]) + ); + } +} From 6e91ba83b7663d22f88788d89595513e55b4ec0f Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 15:04:20 -0700 Subject: [PATCH 2/6] Update clap/docs for merging `--style` arguments --- README.md | 10 ++++++++++ doc/long-help.txt | 6 ++++++ src/bin/bat/clap_app.rs | 6 ++++++ 3 files changed, 22 insertions(+) diff --git a/README.md b/README.md index 1e42e501..016fe834 100644 --- a/README.md +++ b/README.md @@ -515,6 +515,16 @@ and line numbers but no grid and no file header. Set the `BAT_STYLE` environment variable to make these changes permanent or use `bat`s [configuration file](https://github.com/sharkdp/bat#configuration-file). +>[!tip] +> If you specify a default style in `bat`'s config file, you can change which components +> are displayed during a single run of `bat` using the `--style` command-line argument. +> By prefixing a component with `+` or `-`, it can be added or removed from the current style. +> +> For example, if your config contains `--style=full,-snip`, you can run bat with +> `--style=-grid,+snip` to remove the grid and add back the `snip` component. +> Or, if you want to override the styles completely, you use `--style=numbers` to +> only show the line numbers. + ### Adding new syntaxes / language definitions Should you find that a particular syntax is not available within `bat`, you can follow these diff --git a/doc/long-help.txt b/doc/long-help.txt index d9cdce39..2b03490f 100644 --- a/doc/long-help.txt +++ b/doc/long-help.txt @@ -134,6 +134,12 @@ Options: set a default style, add the '--style=".."' option to the configuration file or export the BAT_STYLE environment variable (e.g.: export BAT_STYLE=".."). + When styles are specified in multiple places, the "nearest" set of styles take precedence. + The command-line arguments are the highest priority, followed by the BAT_STYLE environment + variable, and then the configuration file. If any set of styles consists entirely of + components prefixed with "+" or "-", it will modify the previous set of styles instead of + replacing them. + By default, the following components are enabled: changes, grid, header-filename, numbers, snip diff --git a/src/bin/bat/clap_app.rs b/src/bin/bat/clap_app.rs index 8e79174b..33dde980 100644 --- a/src/bin/bat/clap_app.rs +++ b/src/bin/bat/clap_app.rs @@ -442,6 +442,12 @@ pub fn build_app(interactive_output: bool) -> Command { pre-defined style ('full'). To set a default style, add the \ '--style=\"..\"' option to the configuration file or export the \ BAT_STYLE environment variable (e.g.: export BAT_STYLE=\"..\").\n\n\ + When styles are specified in multiple places, the \"nearest\" set \ + of styles take precedence. The command-line arguments are the highest \ + priority, followed by the BAT_STYLE environment variable, and then \ + the configuration file. If any set of styles consists entirely of \ + components prefixed with \"+\" or \"-\", it will modify the \ + previous set of styles instead of replacing them.\n\n\ By default, the following components are enabled:\n \ changes, grid, header-filename, numbers, snip\n\n\ Possible values:\n\n \ From 93b25d75a0db2bc5fef4e147a92af55373781046 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 17:05:46 -0700 Subject: [PATCH 3/6] Join env var options with "=" instead of " " Joining them with a space was causing certain styles (e.g. `-grid`) to be misinterpreted as a separate option. --- src/bin/bat/config.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/bin/bat/config.rs b/src/bin/bat/config.rs index 9e38dfa4..6fa18f09 100644 --- a/src/bin/bat/config.rs +++ b/src/bin/bat/config.rs @@ -146,8 +146,11 @@ pub fn get_args_from_env_vars() -> Vec { ("--style", "BAT_STYLE"), ] .iter() - .filter_map(|(flag, key)| env::var(key).ok().map(|var| [flag.to_string(), var])) - .flatten() + .filter_map(|(flag, key)| { + env::var(key) + .ok() + .map(|var| [flag.to_string(), var].join("=")) + }) .map(|a| a.into()) .collect() } From 180a77ee99e8b5bc9f4cdb017788deb595968d5f Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 17:21:03 -0700 Subject: [PATCH 4/6] Add integration tests for merging styles A huge thanks to @einfachIrgendwer0815 for helping me make sure these tests work under the MSRV CI job. --- tests/integration_tests.rs | 68 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index d6009361..8df4327c 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -2810,3 +2810,71 @@ fn strip_ansi_auto_does_not_strip_ansi_when_plain_text_by_option() { assert!(output.contains("\x1B[33mYellow")) } + +// Tests that style components can be removed with `-component`. +#[test] +fn style_components_can_be_removed() { + bat() + .arg({ + #[cfg(not(feature = "git"))] + { + "--style=full,-grid" + } + #[cfg(feature = "git")] + { + "--style=full,-grid,-changes" + } + }) + .arg("--decorations=always") + .arg("--color=never") + .write_stdin("test") + .assert() + .success() + .stdout(" STDIN\n Size: -\n 1 test\n") + .stderr(""); +} + +// Tests that style components are chosen based on the rightmost `--style` argument. +#[test] +fn style_components_can_be_overidden() { + bat() + .arg("--style=full") + .arg("--style=header,numbers") + .arg("--decorations=always") + .arg("--color=never") + .write_stdin("test") + .assert() + .success() + .stdout(" STDIN\n 1 test\n") + .stderr(""); +} + +// Tests that style components can be merged across multiple `--style` arguments. +#[test] +fn style_components_will_merge() { + bat() + .arg("--style=header,grid") + .arg("--style=-grid,+numbers") + .arg("--decorations=always") + .arg("--color=never") + .write_stdin("test") + .assert() + .success() + .stdout(" STDIN\n 1 test\n") + .stderr(""); +} + +// Tests that style components can be merged with the `BAT_STYLE` environment variable. +#[test] +fn style_components_will_merge_with_env_var() { + bat() + .env("BAT_STYLE", "header,grid") + .arg("--style=-grid,+numbers") + .arg("--decorations=always") + .arg("--color=never") + .write_stdin("test") + .assert() + .success() + .stdout(" STDIN\n 1 test\n") + .stderr(""); +} From aa3ec109b74eb6e23d1fe15ade3087ca8430eeb2 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Mon, 10 Jun 2024 17:39:55 -0700 Subject: [PATCH 5/6] First StyleComponentList should remove from 'auto' style. This happens when there are no `--style` arguments other than the one passed in as a command line argument. Prior to this change, removing a style component (e.g. `--style=-numbers`) would remove the component from an empty style component set, resulting in no styles at all. That behaviour was less intuitive than the new behaviour, which starts out with the default components and removes the line numbers. --- src/bin/bat/app.rs | 2 +- src/style.rs | 54 +++++++++++++++++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index c95c2bd1..d6628668 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -405,7 +405,7 @@ impl App { .map(|v| StyleComponentList::from_str(v)) .collect::>>()?; - StyleComponentList::to_components(lists, self.interactive_output) + StyleComponentList::to_components(lists, self.interactive_output, true) } // Use the default. diff --git a/src/style.rs b/src/style.rs index 8c2cc05d..b8d8b09f 100644 --- a/src/style.rs +++ b/src/style.rs @@ -189,22 +189,27 @@ impl StyleComponentList { /// [numbers,grid] + [header,changes] -> [header,changes] /// [numbers,grid] + [+header,-grid] -> [numbers,header] /// ``` + /// + /// ## Parameters + /// - `with_default`: If true, the styles lists will build upon the StyleComponent::Auto style. pub fn to_components( lists: impl IntoIterator, interactive_terminal: bool, + with_default: bool, ) -> StyleComponents { - StyleComponents( - lists - .into_iter() - .fold(HashSet::new(), |mut components, list| { - if list.contains_override() { - components.clear(); - } + let mut components: HashSet = HashSet::new(); + if with_default { + components.extend(StyleComponent::Auto.components(interactive_terminal)) + } - list.expand_into(&mut components, interactive_terminal); - components - }), - ) + StyleComponents(lists.into_iter().fold(components, |mut components, list| { + if list.contains_override() { + components.clear(); + } + + list.expand_into(&mut components, interactive_terminal); + components + })) } } @@ -233,6 +238,7 @@ mod test { use std::str::FromStr; use super::ComponentAction::*; + use super::StyleComponent; use super::StyleComponent::*; use super::StyleComponentList; @@ -261,6 +267,7 @@ mod test { assert_eq!( StyleComponentList::to_components( vec![StyleComponentList::from_str("grid,numbers").expect("no error")], + false, false ) .0, @@ -273,6 +280,7 @@ mod test { assert_eq!( StyleComponentList::to_components( vec![StyleComponentList::from_str("grid,numbers,-grid").expect("no error")], + false, false ) .0, @@ -285,6 +293,7 @@ mod test { assert_eq!( StyleComponentList::to_components( vec![StyleComponentList::from_str("full").expect("no error")], + false, false ) .0, @@ -296,7 +305,8 @@ mod test { pub fn style_component_list_expand_negates_subcomponents() { assert!(!StyleComponentList::to_components( vec![StyleComponentList::from_str("full,-numbers").expect("no error")], - true + true, + false ) .numbers()); } @@ -309,6 +319,7 @@ mod test { StyleComponentList::from_str("grid").expect("no error"), StyleComponentList::from_str("numbers").expect("no error"), ], + false, false ) .0, @@ -325,10 +336,29 @@ mod test { StyleComponentList::from_str("-grid").expect("no error"), StyleComponentList::from_str("+numbers").expect("no error"), ], + false, false ) .0, HashSet::from([HeaderFilename, LineNumbers]) ); } + + #[test] + pub fn style_component_list_default_builds_on_auto() { + assert_eq!( + StyleComponentList::to_components( + vec![StyleComponentList::from_str("-numbers").expect("no error"),], + true, + true + ) + .0, + { + let mut expected: HashSet = HashSet::new(); + expected.extend(Auto.components(true)); + expected.remove(&LineNumbers); + expected + } + ); + } } From 39684b85ad6c14819a7780c2feef37a888ac7f43 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 18:10:01 -0700 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 610af60a..ea47b58f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - `PrettyPrinter::squeeze_empty_lines` to support line squeezing for bat as a library, see #1441 (@eth-p) and #2665 (@einfachIrgendwer0815) - Syntax highlighting for JavaScript files that start with `#!/usr/bin/env bun` #2913 (@sharunkumar) - `bat --strip-ansi={never,always,auto}` to remove ANSI escape sequences from bat's input, see #2999 (@eth-p) +- Add or remove individual style components without replacing all styles #2929 (@eth-p) ## Bugfixes