diff --git a/CHANGELOG.md b/CHANGELOG.md index d1355e21..8ced404b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # unreleased +- Allow env vars to override config files, but not command-line arguments, see #1152, #1281, and #2381 (@aaronkollasch) + ## Features - Implemented `-S` and `--chop-long-lines` flags as aliases for `--wrap=never`. See #2309 (@johnmatthiggins) diff --git a/src/bin/bat/app.rs b/src/bin/bat/app.rs index d38c529d..e4ec938d 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::path::{Path, PathBuf}; -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_opts_var, get_args_from_env_vars}, }; use clap::ArgMatches; @@ -60,15 +59,20 @@ impl App { let mut cli_args = wild::args_os(); // Read arguments from bats config file - let mut args = get_args_from_env_var() + let mut args = get_args_from_env_opts_var() .unwrap_or_else(get_args_from_config_file) .map_err(|_| "Could not parse configuration file")?; // Put the zero-th CLI argument (program name) first args.insert(0, cli_args.next().unwrap()); + // env vars supersede config vars + get_args_from_env_vars() + .iter() + .for_each(|a| args.push(a.into())); + // .. and the rest at the end - cli_args.for_each(|a| args.push(a)); + cli_args.for_each(|a| args.push(a.into())); args }; @@ -203,7 +207,6 @@ impl App { .matches .get_one::("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 { @@ -216,7 +219,6 @@ impl App { .matches .get_one::("theme") .map(String::from) - .or_else(|| env::var("BAT_THEME").ok()) .map(|s| { if s == "default" { String::from(HighlightingAssets::default_theme()) @@ -321,16 +323,6 @@ impl App { } else if 0 < matches.get_count("plain") { [StyleComponent::Plain].iter().cloned().collect() } else { - let env_style_components: Option> = env::var("BAT_STYLE") - .ok() - .map(|style_str| { - style_str - .split(',') - .map(StyleComponent::from_str) - .collect::>>() - }) - .transpose()?; - matches .get_one::("style") .map(|styles| { @@ -340,7 +332,6 @@ impl App { .filter_map(|style| style.ok()) .collect::>() }) - .or(env_style_components) .unwrap_or_else(|| vec![StyleComponent::Default]) .into_iter() .map(|style| style.components(self.interactive_output)) diff --git a/src/bin/bat/config.rs b/src/bin/bat/config.rs index 696edf9e..d5577658 100644 --- a/src/bin/bat/config.rs +++ b/src/bin/bat/config.rs @@ -117,7 +117,7 @@ pub fn get_args_from_config_file() -> Result, shell_words::ParseEr get_args_from_str(&config) } -pub fn get_args_from_env_var() -> Option, shell_words::ParseError>> { +pub fn get_args_from_env_opts_var() -> Option, shell_words::ParseError>> { env::var("BAT_OPTS").ok().map(|s| get_args_from_str(&s)) } @@ -137,6 +137,19 @@ fn get_args_from_str(content: &str) -> Result, shell_words::ParseE .collect()) } +pub fn get_args_from_env_vars() -> Vec { + [ + ("--tabs", "BAT_TABS"), + ("--theme", "BAT_THEME"), + ("--pager", "BAT_PAGER"), + ("--style", "BAT_STYLE"), + ] + .iter() + .filter_map(|(flag, key)| env::var(key).ok().map(|var| [flag.into(), var.into()])) + .flatten() + .collect() +} + #[test] fn empty() { let args = get_args_from_str("").unwrap(); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index b7becf8b..91e2a934 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -508,6 +508,17 @@ fn pager_basic() { .stdout(predicate::eq("pager-output\n").normalize()); } +#[test] +fn pager_basic_arg() { + 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() @@ -532,6 +543,45 @@ fn pager_disable() { .stdout(predicate::eq("hello world\n").normalize()); } +#[test] +fn pager_arg_override_env() { + bat_with_config() + .env("BAT_CONFIG_PATH", "bat.conf") + .env("PAGER", "echo another-pager") + .env("BAT_PAGER", "echo other-pager") + .arg("--pager=echo pager-output") + .arg("--paging=always") + .arg("test.txt") + .assert() + .success() + .stdout(predicate::eq("pager-output\n").normalize()); +} + +#[test] +fn pager_env_bat_pager_override_config() { + bat_with_config() + .env("BAT_CONFIG_PATH", "bat.conf") + .env("PAGER", "echo other-pager") + .env("BAT_PAGER", "echo pager-output") + .arg("--paging=always") + .arg("test.txt") + .assert() + .success() + .stdout(predicate::eq("pager-output\n").normalize()); +} + +#[test] +fn pager_env_pager_nooverride_config() { + bat_with_config() + .env("BAT_CONFIG_PATH", "bat.conf") + .env("PAGER", "echo other-pager") + .arg("--paging=always") + .arg("test.txt") + .assert() + .success() + .stdout(predicate::eq("dummy-pager-from-config\n").normalize()); +} + #[test] fn env_var_pager_value_bat() { bat()