From 7cce8e6bf5a92bbc35a27e473e930b2545623b0f Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 14:54:10 -0700 Subject: [PATCH 1/5] 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 6fc85321..b9c9a4d5 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 clap::ArgMatches; use console::Term; @@ -85,7 +87,6 @@ impl App { // .. and the rest at the end cli_args.for_each(|a| args.push(a)); - args }; @@ -353,34 +354,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 b82762b6..2e376061 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")] @@ -405,34 +407,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 bbdac57426490dfe6bdf06a8f7b675c83295749b Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 15:04:20 -0700 Subject: [PATCH 2/5] Update clap/docs for merging `--style` arguments --- README.md | 10 ++++++++++ doc/long-help.txt | 13 ++++++++++--- src/bin/bat/clap_app.rs | 13 ++++++++++--- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 57baf2b0..6571c99a 100644 --- a/README.md +++ b/README.md @@ -507,6 +507,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 a6ffe962..d2cef456 100644 --- a/doc/long-help.txt +++ b/doc/long-help.txt @@ -125,9 +125,16 @@ Options: --style Configure which elements (line numbers, file headers, grid borders, Git modifications, ..) to display in addition to the file contents. The argument is a comma-separated list of - components to display (e.g. 'numbers,changes,grid') or a 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=".."). + components to display (e.g. 'numbers,changes,grid') or a 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=".."). + + 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 2e376061..877afd89 100644 --- a/src/bin/bat/clap_app.rs +++ b/src/bin/bat/clap_app.rs @@ -425,9 +425,16 @@ pub fn build_app(interactive_output: bool) -> Command { borders, Git modifications, ..) to display in addition to the \ file contents. The argument is a comma-separated list of \ components to display (e.g. 'numbers,changes,grid') or a \ - 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\ + pre-defined style ('full').\n\n\ + 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 02889048ca867163d7e23d59b0d8fb58584cc04b Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 17:05:46 -0700 Subject: [PATCH 3/5] 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 57a7b0f4bdab06892da414aa7b2119504306bd15 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 17:21:03 -0700 Subject: [PATCH 4/5] 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 d6523366..5def7751 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -2631,3 +2631,71 @@ fn highlighting_independant_from_map_syntax_case() { .stdout(expected) .stderr(""); } + +// 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 16d638f1db89a901abb5036bfc6f38414cf20cf6 Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 18:10:01 -0700 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ebdc55..9d1825c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - `bat --squeeze-blank`/`bat -s` will now squeeze consecutive empty lines, see #1441 (@eth-p) and #2665 (@einfachIrgendwer0815) - `bat --squeeze-limit` to set the maximum number of empty consecutive when using `--squeeze-blank`, see #1441 (@eth-p) and #2665 (@einfachIrgendwer0815) - `PrettyPrinter::squeeze_empty_lines` to support line squeezing for bat as a library, see #1441 (@eth-p) and #2665 (@einfachIrgendwer0815) +- Add or remove individual style components without replacing all styles #2929 (@eth-p) ## Bugfixes