From 7cce8e6bf5a92bbc35a27e473e930b2545623b0f Mon Sep 17 00:00:00 2001 From: "Ethan P." Date: Sat, 6 Apr 2024 14:54:10 -0700 Subject: [PATCH] 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]) + ); + } +}