Merge pull request #2381 from aaronkollasch/env-override-config-not-flags

Allow some env vars to override config variables, but not command line arguments

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Features
 
 - Implemented `-S` and `--chop-long-lines` flags as aliases for `--wrap=never`. See #2309 (@johnmatthiggins)
+- Breaking change: Environment variables can now override config file settings (but command-line arguments still have the highest precedence), see #1152, #1281, and #2381 (@aaronkollasch)
 
 ## Bugfixes
 

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;
 
@@ -50,20 +49,34 @@ impl App {
     }
 
     fn matches(interactive_output: bool) -> Result<ArgMatches> {
-        let args = if wild::args_os().nth(1) == Some("cache".into())
+        let args = if wild::args_os().nth(1) == Some("cache".into()) {
-            || wild::args_os().any(|arg| arg == "--no-config")
+            // Skip the config file and env vars
-        {
-            // Skip the arguments in bats config file
 
             wild::args_os().collect::<Vec<_>>()
+        } else if wild::args_os().any(|arg| arg == "--no-config") {
+            // Skip the arguments in bats config file
+
+            let mut cli_args = wild::args_os();
+            let mut args = get_args_from_env_vars();
+
+            // Put the zero-th CLI argument (program name) first
+            args.insert(0, cli_args.next().unwrap());
+
+            // .. and the rest at the end
+            cli_args.for_each(|a| args.push(a));
+
+            args
         } else {
             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")?;
 
+            // Selected env vars supersede config vars
+            args.extend(get_args_from_env_vars());
+
             // Put the zero-th CLI argument (program name) first
             args.insert(0, cli_args.next().unwrap());
 
@@ -203,7 +216,6 @@ impl App {
                 .matches
                 .get_one::<String>("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 +228,6 @@ impl App {
                 .matches
                 .get_one::<String>("theme")
                 .map(String::from)
-                .or_else(|| env::var("BAT_THEME").ok())
                 .map(|s| {
                     if s == "default" {
                         String::from(HighlightingAssets::default_theme())
@@ -321,16 +332,6 @@ impl App {
             } else if 0 < matches.get_count("plain") {
                 [StyleComponent::Plain].iter().cloned().collect()
             } else {
-                let env_style_components: Option<Vec<StyleComponent>> = env::var("BAT_STYLE")
-                    .ok()
-                    .map(|style_str| {
-                        style_str
-                            .split(',')
-                            .map(StyleComponent::from_str)
-                            .collect::<Result<Vec<StyleComponent>>>()
-                    })
-                    .transpose()?;
-
                 matches
                     .get_one::<String>("style")
                     .map(|styles| {
@@ -340,7 +341,6 @@ impl App {
                             .filter_map(|style| style.ok())
                             .collect::<Vec<_>>()
                     })
-                    .or(env_style_components)
                     .unwrap_or_else(|| vec![StyleComponent::Default])
                     .into_iter()
                     .map(|style| style.components(self.interactive_output))

src/bin/bat/config.rs

@@ -117,7 +117,7 @@ pub fn get_args_from_config_file() -> Result<Vec<OsString>, shell_words::ParseEr
     get_args_from_str(&config)
 }
 
-pub fn get_args_from_env_var() -> Option<Result<Vec<OsString>, shell_words::ParseError>> {
+pub fn get_args_from_env_opts_var() -> Option<Result<Vec<OsString>, shell_words::ParseError>> {
     env::var("BAT_OPTS").ok().map(|s| get_args_from_str(&s))
 }
 
@@ -137,6 +137,20 @@ fn get_args_from_str(content: &str) -> Result<Vec<OsString>, shell_words::ParseE
         .collect())
 }
 
+pub fn get_args_from_env_vars() -> Vec<OsString> {
+    [
+        ("--tabs", "BAT_TABS"),
+        ("--theme", "BAT_THEME"),
+        ("--pager", "BAT_PAGER"),
+        ("--style", "BAT_STYLE"),
+    ]
+    .iter()
+    .filter_map(|(flag, key)| env::var(key).ok().map(|var| [flag.to_string(), var]))
+    .flatten()
+    .map(|a| a.into())
+    .collect()
+}
+
 #[test]
 fn empty() {
     let args = get_args_from_str("").unwrap();

tests/examples/bat-tabs.conf

@@ -0,0 +1 @@
+--tabs=8

tests/examples/bat-theme.conf

@@ -0,0 +1 @@
+--theme=TwoDark

tests/examples/cache_source/syntaxes/c.sublime-syntax

@@ -0,0 +1,10 @@
+%YAML 1.2
+---
+name: C
+file_extensions: [c, h]
+scope: source.c
+
+contexts:
+  main:
+    - match: \b(if|else|for|while)\b
+      scope: keyword.control.c

tests/examples/cache_source/themes/example.tmTheme

@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>name</key>
+	<string>example</string>
+	<key>settings</key>
+	<array>
+		<dict>
+			<key>settings</key>
+			<dict>
+				<key>background</key>
+				<string>#222222</string>
+				<key>caret</key>
+				<string>#979797</string>
+				<key>foreground</key>
+				<string>#F8F8F8</string>
+				<key>invisibles</key>
+				<string>#777777</string>
+				<key>lineHighlight</key>
+				<string>#000000</string>
+				<key>selection</key>
+				<string>#57CCBF</string>
+			</dict>
+		</dict>
+		<dict>
+			<key>name</key>
+			<string>Comment</string>
+			<key>scope</key>
+			<string>comment</string>
+			<key>settings</key>
+			<dict>
+				<key>foreground</key>
+				<string>#777777</string>
+			</dict>
+		</dict>
+	</array>
+	<key>uuid</key>
+	<string>0123-4567-89AB-CDEF</string>
+	<key>colorSpaceName</key>
+	<string>sRGB</string>
+	<key>semanticClass</key>
+	<string>theme</string>
+</dict>
+</plist>

tests/integration_tests.rs

@@ -477,6 +477,79 @@ fn tabs_8() {
         );
 }
 
+#[test]
+fn tabs_4_env_overrides_config() {
+    bat_with_config()
+        .env("BAT_CONFIG_PATH", "bat-tabs.conf")
+        .env("BAT_TABS", "4")
+        .arg("tabs.txt")
+        .arg("--style=plain")
+        .arg("--decorations=always")
+        .assert()
+        .success()
+        .stdout(
+            "    1   2   3   4
+1   ?
+22  ?
+333 ?
+4444    ?
+55555   ?
+666666  ?
+7777777 ?
+88888888    ?
+",
+        );
+}
+
+#[test]
+fn tabs_4_arg_overrides_env() {
+    bat_with_config()
+        .env("BAT_CONFIG_PATH", "bat-tabs.conf")
+        .env("BAT_TABS", "6")
+        .arg("tabs.txt")
+        .arg("--tabs=4")
+        .arg("--style=plain")
+        .arg("--decorations=always")
+        .assert()
+        .success()
+        .stdout(
+            "    1   2   3   4
+1   ?
+22  ?
+333 ?
+4444    ?
+55555   ?
+666666  ?
+7777777 ?
+88888888    ?
+",
+        );
+}
+
+#[test]
+fn tabs_4_arg_overrides_env_noconfig() {
+    bat()
+        .env("BAT_TABS", "6")
+        .arg("tabs.txt")
+        .arg("--tabs=4")
+        .arg("--style=plain")
+        .arg("--decorations=always")
+        .assert()
+        .success()
+        .stdout(
+            "    1   2   3   4
+1   ?
+22  ?
+333 ?
+4444    ?
+55555   ?
+666666  ?
+7777777 ?
+88888888    ?
+",
+        );
+}
+
 #[test]
 fn fail_non_existing() {
     bat().arg("non-existing-file").assert().failure();
@@ -508,6 +581,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 +616,58 @@ fn pager_disable() {
         .stdout(predicate::eq("hello world\n").normalize());
 }
 
+#[test]
+fn pager_arg_override_env_withconfig() {
+    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_arg_override_env_noconfig() {
+    bat()
+        .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()
@@ -756,6 +892,102 @@ fn config_read_arguments_from_file() {
         .stdout(predicate::eq("dummy-pager-from-config\n").normalize());
 }
 
+// Ignore this test for now as `bat cache --clear` only targets the default cache dir.
+// `bat cache --clear` must clear the `--target` dir for this test to pass.
+#[cfg(unix)]
+#[test]
+#[ignore]
+fn cache_clear() {
+    let src_dir = "cache_source";
+    let tmp_dir = tempdir().expect("can create temporary directory");
+    let themes_filename = "themes.bin";
+    let syntaxes_filename = "syntaxes.bin";
+    let metadata_filename = "metadata.yaml";
+    [themes_filename, syntaxes_filename, metadata_filename]
+        .iter()
+        .map(|filename| {
+            let fp = tmp_dir.path().join(filename);
+            let mut file = File::create(fp).expect("can create temporary file");
+            writeln!(file, "dummy content").expect("can write to file");
+        })
+        .count();
+
+    // Clear the targeted cache
+    // Include the BAT_CONFIG_PATH and BAT_THEME environment variables to ensure that
+    // options loaded from a config or the environment are not inserted
+    // before the cache subcommand, which would break it.
+    bat_with_config()
+        .current_dir(Path::new(EXAMPLES_DIR).join(src_dir))
+        .env("BAT_CONFIG_PATH", "bat.conf")
+        .env("BAT_THEME", "1337")
+        .arg("cache")
+        .arg("--clear")
+        .arg("--source")
+        .arg(".")
+        .arg("--target")
+        .arg(tmp_dir.path().to_str().unwrap())
+        .assert()
+        .success()
+        .stdout(
+            predicate::str::is_match(
+                "Clearing theme set cache ... okay
+Clearing syntax set cache ... okay
+Clearing metadata file ... okay",
+            )
+            .unwrap(),
+        );
+
+    // We expect these files to be removed
+    assert!(!tmp_dir.path().join(themes_filename).exists());
+    assert!(!tmp_dir.path().join(syntaxes_filename).exists());
+    assert!(!tmp_dir.path().join(metadata_filename).exists());
+}
+
+#[cfg(unix)]
+#[test]
+fn cache_build() {
+    let src_dir = "cache_source";
+    let tmp_dir = tempdir().expect("can create temporary directory");
+    let tmp_themes_path = tmp_dir.path().join("themes.bin");
+    let tmp_syntaxes_path = tmp_dir.path().join("syntaxes.bin");
+    let tmp_acknowledgements_path = tmp_dir.path().join("acknowledgements.bin");
+    let tmp_metadata_path = tmp_dir.path().join("metadata.yaml");
+
+    // Build the cache
+    // Include the BAT_CONFIG_PATH and BAT_THEME environment variables to ensure that
+    // options loaded from a config or the environment are not inserted
+    // before the cache subcommand, which would break it.
+    bat_with_config()
+        .current_dir(Path::new(EXAMPLES_DIR).join(src_dir))
+        .env("BAT_CONFIG_PATH", "bat.conf")
+        .env("BAT_THEME", "1337")
+        .arg("cache")
+        .arg("--build")
+        .arg("--blank")
+        .arg("--source")
+        .arg(".")
+        .arg("--target")
+        .arg(tmp_dir.path().to_str().unwrap())
+        .arg("--acknowledgements")
+        .assert()
+        .success()
+        .stdout(
+            predicate::str::is_match(
+                "Writing theme set to .*/themes.bin ... okay
+Writing syntax set to .*/syntaxes.bin ... okay
+Writing acknowledgements to .*/acknowledgements.bin ... okay
+Writing metadata to folder .* ... okay",
+            )
+            .unwrap(),
+        );
+
+    // Now we expect the files to exist. If they exist, we assume contents are correct
+    assert!(tmp_themes_path.exists());
+    assert!(tmp_syntaxes_path.exists());
+    assert!(tmp_acknowledgements_path.exists());
+    assert!(tmp_metadata_path.exists());
+}
+
 #[test]
 fn utf16() {
     // The output will be converted to UTF-8 with the leading UTF-16
@@ -981,6 +1213,35 @@ fn header_full_basic() {
         .stderr("");
 }
 
+#[test]
+fn header_env_basic() {
+    bat_with_config()
+        .env("BAT_STYLE", "header-filename,header-filesize")
+        .arg("test.txt")
+        .arg("--decorations=always")
+        .arg("-r=0:0")
+        .arg("--file-name=foo")
+        .assert()
+        .success()
+        .stdout("File: foo\nSize: 12 B\n")
+        .stderr("");
+}
+
+#[test]
+fn header_arg_overrides_env() {
+    bat_with_config()
+        .env("BAT_STYLE", "header-filesize")
+        .arg("test.txt")
+        .arg("--decorations=always")
+        .arg("--style=header-filename")
+        .arg("-r=0:0")
+        .arg("--file-name=foo")
+        .assert()
+        .success()
+        .stdout("File: foo\n")
+        .stderr("");
+}
+
 #[test]
 fn header_binary() {
     bat()
@@ -1540,6 +1801,64 @@ fn no_wrapping_with_chop_long_lines() {
     wrapping_test("--chop-long-lines", false);
 }
 
+#[test]
+fn theme_arg_overrides_env() {
+    bat()
+        .env("BAT_THEME", "TwoDark")
+        .arg("--paging=never")
+        .arg("--color=never")
+        .arg("--terminal-width=80")
+        .arg("--wrap=never")
+        .arg("--decorations=always")
+        .arg("--theme=ansi")
+        .arg("--style=plain")
+        .arg("--highlight-line=1")
+        .write_stdin("Ansi Underscore Test\nAnother Line")
+        .assert()
+        .success()
+        .stdout("\x1B[4mAnsi Underscore Test\n\x1B[24mAnother Line")
+        .stderr("");
+}
+
+#[test]
+fn theme_arg_overrides_env_withconfig() {
+    bat_with_config()
+        .env("BAT_CONFIG_PATH", "bat-theme.conf")
+        .env("BAT_THEME", "TwoDark")
+        .arg("--paging=never")
+        .arg("--color=never")
+        .arg("--terminal-width=80")
+        .arg("--wrap=never")
+        .arg("--decorations=always")
+        .arg("--theme=ansi")
+        .arg("--style=plain")
+        .arg("--highlight-line=1")
+        .write_stdin("Ansi Underscore Test\nAnother Line")
+        .assert()
+        .success()
+        .stdout("\x1B[4mAnsi Underscore Test\n\x1B[24mAnother Line")
+        .stderr("");
+}
+
+#[test]
+fn theme_env_overrides_config() {
+    bat_with_config()
+        .env("BAT_CONFIG_PATH", "bat-theme.conf")
+        .env("BAT_THEME", "ansi")
+        .arg("--paging=never")
+        .arg("--color=never")
+        .arg("--terminal-width=80")
+        .arg("--wrap=never")
+        .arg("--decorations=always")
+        .arg("--style=plain")
+        .arg("--highlight-line=1")
+        .write_stdin("Ansi Underscore Test\nAnother Line")
+        .assert()
+        .success()
+        .stdout("\x1B[4mAnsi Underscore Test\n\x1B[24mAnother Line")
+        .stderr("");
+}
+
 #[test]
 fn highlighting_is_skipped_on_long_lines() {
     let expected = "\u{1b}[38;5;231m{\u{1b}[0m\u{1b}[38;5;208m\"\u{1b}[0m\u{1b}[38;5;208mapi\u{1b}[0m\u{1b}[38;5;208m\"\u{1b}[0m\u{1b}[38;5;231m:\u{1b}[0m\n".to_owned() +