From fade8defd9ca7820ad35d3913ac0befc08fdec34 Mon Sep 17 00:00:00 2001 From: Jacob Mischka Date: Tue, 6 Oct 2020 18:17:05 -0500 Subject: [PATCH] Allow env vars to override config variables, but not flags Parses env configurations into matches (for those that can be configured via matches) instead of relying on dedicated env checks everywhere. Fixes #1152 --- src/bin/bat/app.rs | 43 +++++++++++++------------------------- src/bin/bat/config.rs | 33 ++++++++++++++++++++++++++--- src/output.rs | 24 +++++++++------------ tests/integration_tests.rs | 24 +++++++++++++++++++++ 4 files changed, 79 insertions(+), 45 deletions(-) diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index 8249e3c93b..f70a079681 100644 --- a/src/bin/bat/app.rs +++ b/src/bin/bat/app.rs @@ -1,13 +1,12 @@ use std::collections::HashSet; use std::env; use std::ffi::OsStr; -use std::str::FromStr; use atty::{self, Stream}; use crate::{ clap_app, - config::{get_args_from_config_file, get_args_from_env_var}, + config::{get_args_from_config_file, get_args_from_env_vars, get_args_from_opts_env_var}, }; use clap::ArgMatches; @@ -49,28 +48,29 @@ impl App { } fn matches(interactive_output: bool) -> Result> { - let args = if wild::args_os().nth(1) == Some("cache".into()) + let mut cli_args = wild::args_os(); + let mut args = if wild::args_os().nth(1) == Some("cache".into()) || wild::args_os().any(|arg| arg == "--no-config") { // Skip the arguments in bats config file - - wild::args_os().collect::>() + Vec::new() } else { - let mut cli_args = wild::args_os(); - // Read arguments from bats config file - let mut args = get_args_from_env_var() + get_args_from_opts_env_var() .unwrap_or_else(get_args_from_config_file) - .chain_err(|| "Could not parse configuration file")?; + .chain_err(|| "Could not parse configuration file")? + }; - // Put the zero-th CLI argument (program name) first - args.insert(0, cli_args.next().unwrap()); + args.append( + &mut get_args_from_env_vars() + .chain_err(|| "Could not get args from environment variables")?, + ); - // .. and the rest at the end - cli_args.for_each(|a| args.push(a)); + // Put the zero-th CLI argument (program name) first + args.insert(0, cli_args.next().unwrap()); - args - }; + // .. and the rest at the end + cli_args.for_each(|a| args.push(a)); Ok(clap_app::build_app(interactive_output).get_matches_from(args)) } @@ -183,7 +183,6 @@ impl App { .matches .value_of("tabs") .map(String::from) - .or_else(|| env::var("BAT_TABS").ok()) .and_then(|t| t.parse().ok()) .unwrap_or( if style_components.plain() && paging_mode == PagingMode::Never { @@ -196,7 +195,6 @@ impl App { .matches .value_of("theme") .map(String::from) - .or_else(|| env::var("BAT_THEME").ok()) .map(|s| { if s == "default" { String::from(HighlightingAssets::default_theme()) @@ -296,16 +294,6 @@ impl App { } else if matches.is_present("plain") { [StyleComponent::Plain].iter().cloned().collect() } else { - let env_style_components: Option> = env::var("BAT_STYLE") - .ok() - .map(|style_str| { - style_str - .split(',') - .map(|x| StyleComponent::from_str(&x)) - .collect::>>() - }) - .transpose()?; - matches .value_of("style") .map(|styles| { @@ -315,7 +303,6 @@ impl App { .filter_map(|style| style.ok()) .collect::>() }) - .or(env_style_components) .unwrap_or_else(|| vec![StyleComponent::Full]) .into_iter() .map(|style| style.components(self.interactive_output)) diff --git a/src/bin/bat/config.rs b/src/bin/bat/config.rs index c7ce08d5c0..6a5659092a 100644 --- a/src/bin/bat/config.rs +++ b/src/bin/bat/config.rs @@ -44,8 +44,7 @@ pub fn generate_config_file() -> bat::error::Result<()> { } } - let default_config = - r#"# This is `bat`s configuration file. Each line either contains a comment or + let default_config = r#"# This is `bat`s configuration file. Each line either contains a comment or # a command-line option that you want to pass to `bat` by default. You can # run `bat --help` to get a list of all possible configuration options. @@ -89,7 +88,7 @@ pub fn get_args_from_config_file() -> Result, shell_words::ParseEr .unwrap_or_else(|| vec![])) } -pub fn get_args_from_env_var() -> Option, shell_words::ParseError>> { +pub fn get_args_from_opts_env_var() -> Option, shell_words::ParseError>> { env::var("BAT_OPTS").ok().map(|s| get_args_from_str(&s)) } @@ -109,6 +108,24 @@ fn get_args_from_str(content: &str) -> Result, shell_words::ParseE .collect()) } +pub fn get_args_from_env_vars() -> Result, shell_words::ParseError> { + let env_arg_pairs = [ + ("BAT_THEME", "theme"), + ("BAT_STYLE", "style"), + ("BAT_TABS", "tabs"), + ("BAT_PAGER", "pager"), + ]; + + Ok(env_arg_pairs + .iter() + .filter_map(|(env_var, arg)| { + env::var(env_var) + .ok() + .map(|s| OsString::from(format!("--{}={}", arg, s))) + }) + .collect()) +} + #[test] fn empty() { let args = get_args_from_str("").unwrap(); @@ -167,3 +184,13 @@ fn comments() { get_args_from_str(config).unwrap() ); } + +#[test] +fn env_var() { + env::set_var("BAT_THEME", "test"); + env::set_var("BAT_PAGER", "more"); + assert_eq!( + vec!["--theme=test", "--pager=more"], + get_args_from_env_vars().unwrap() + ); +} diff --git a/src/output.rs b/src/output.rs index e7d59d6707..1b7dcc78b9 100644 --- a/src/output.rs +++ b/src/output.rs @@ -36,20 +36,16 @@ impl OutputType { let mut replace_arguments_to_less = false; - let pager_from_env = match (env::var("BAT_PAGER"), env::var("PAGER")) { - (Ok(bat_pager), _) => Some(bat_pager), - (_, Ok(pager)) => { - // less needs to be called with the '-R' option in order to properly interpret the - // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we - // therefore need to overwrite the arguments and add '-R'. - // - // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER - // or bats '--pager' command line option. - replace_arguments_to_less = true; - Some(pager) - } - _ => None, - }; + let pager_from_env = env::var("PAGER").ok().map(|pager| { + // less needs to be called with the '-R' option in order to properly interpret the + // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we + // therefore need to overwrite the arguments and add '-R'. + // + // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER + // or bats '--pager' command line option. + replace_arguments_to_less = true; + pager + }); let pager_from_config = pager_from_config.map(|p| p.to_string()); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index da0690efcc..7a1e30a203 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -381,6 +381,17 @@ fn pager_basic() { .stdout(predicate::eq("pager-output\n").normalize()); } +#[test] +fn pager_config() { + bat() + .arg("--pager=echo pager-output") + .arg("--paging=always") + .arg("test.txt") + .assert() + .success() + .stdout(predicate::eq("pager-output\n").normalize()); +} + #[test] fn pager_overwrite() { bat() @@ -393,6 +404,19 @@ fn pager_overwrite() { .stdout(predicate::eq("pager-output\n").normalize()); } +#[test] +fn pager_overwrite_overwrite() { + bat() + .env("PAGER", "echo other-pager") + .env("BAT_PAGER", "echo another-pager") + .arg("--pager=echo pager-output") + .arg("--paging=always") + .arg("test.txt") + .assert() + .success() + .stdout(predicate::eq("pager-output\n").normalize()); +} + #[test] fn pager_disable() { bat()