Repository: sharkdp/bat (language: rust)

Issue:
Separate system and user config files
I've been trying to export most of my dotfile-type configs to system packages I can install. A lot of programs offer the option for both a system and a user config (e.g. git reads from `/etc/gitconfig`, then `~/.gitconfig`) although currently, bat reads just one file. It'd be nice if we could do something similar for bat as well, loading both `/etc/bat/config` and `~/.config/bat/config`.


Discussion / hints:
Sounds like a good idea to me! Thank you for the feedback.
Adding a quick note for #659: This should probably be configurable by the package maintainer.
I am very new to Rust, like very very new new. Can I try my hands on this if that's okay?
Sure! Let us know if you need help
Hi @sharkdp so, this how I am thinking of approaching this: I would just concatenate config read from `/etc/bat/config` and `~/.config/bat/config` together
@goku321 Yes, exactly. I think we should directly implement it with a list of possible paths to read from, in case there will be more (maybe OS-specific) paths in the future.
> Hi @sharkdp so, this how I am thinking of approaching this: I would just concatenate config read from `/etc/bat/config` and `~/.config/bat/config` together

You could use `bat` to concatenate those files, right ? 😉 
Is this issue still active? I can see #691 which was closed a while back. I'm keen to pick this up :)
This is still active, yes.
Should `/etc/bat/config` be static or should we expose an env var to override that also?
I would try to avoid introducing more environment variables, if possible. What do other CLI applications do? Is `/etc/bat/config` a reasonable place for a system-wide `bat` config?
Y'all Crazy 🤣🤣🤣
> Y'all Crazy 🤣🤣🤣

Off-topic.
> So out of curiosity, what would the system config paths be on the different platforms? Are there any standard conventions?
> 
> I think these might be reasonable:
> 
>     * `/etc/bat/config` on Linux
> 
>     * `<INSTALL_DIR>/config` on Windows
> 
> 
> No idea for macOS.

I propose that we use `git` as the role model for where to put the system config file.

```
% man git-config
[...]
       $(prefix)/etc/gitconfig
           System-wide configuration file.
[...]
```

This path is used both for Linux and macOS (so `#[cfg(unix)]` can be used in Rust code).

I agree with @eth-p that we need to include means to specify `$(prefix)` somehow, but to be honest I don't know of the best way to do that within the Rust ecosystem.

I am also a bit skeptical towards putting things in a `bat` dir in `/etc/` since the name `bat` is used for other packages too. On the other hand, it feels awkward to call it something like `/etc/batcat` :confounded: 

When it comes to the Windows path, I don't have access to a machine to test on, but [apparently](https://www.atlassian.com/git/tutorials/setting-up-a-repository/git-config)
> On windows this file can be found at C:\Documents and Settings\All Users\Application Data\Git\config on Windows XP, and in C:\ProgramData\Git\config on Windows Vista and newer.

I do think we need to put some extra effort into getting this right, since what we end up with is likely to stay around as a maintenance/backwards compatibility commitment for a long time.

CC @akinnane
I planning to pick this up. But I have a major inquiry since I'm still very new to Rust and to open-source in general:

What is `$(prefix)` in the context of Git? My understanding from the `man` page and [this](https://stackoverflow.com/questions/19713622/what-is-prefix-on-prefix-etc-gitconfig) stackoverflow question, it is an environment variable.

Fix patch:
diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml
index 8a4df2b1dc..2ccb2f3220 100644
--- a/.github/workflows/CICD.yml
+++ b/.github/workflows/CICD.yml
@@ -98,7 +98,7 @@ jobs:
       uses: actions-rs/cargo@v1
       with:
         command: test
-        args: --locked --release -- --ignored
+        args: --locked --release --test assets -- --ignored
     - name: Syntax highlighting regression test
       run: tests/syntax-tests/regression_test.sh
     - name: List of languages
@@ -108,6 +108,32 @@ jobs:
     - name: Test custom assets
       run: tests/syntax-tests/test_custom_assets.sh
 
+  test_with_system_config:
+    name: Run tests with system wide configuration
+    runs-on: ubuntu-20.04
+    steps:
+    - name: Git checkout
+      uses: actions/checkout@v2
+    - name: Prepare environment variables
+      run: |
+        echo "BAT_SYSTEM_CONFIG_PREFIX=$GITHUB_WORKSPACE/tests/examples/system_config" >> $GITHUB_ENV
+    - name: Install Rust toolchain
+      uses: actions-rs/toolchain@v1
+      with:
+        toolchain: stable
+        default: true
+        profile: minimal
+    - name: Build and install bat
+      uses: actions-rs/cargo@v1
+      with:
+        command: install
+        args: --locked --path .
+    - name: Run unit tests
+      uses: actions-rs/cargo@v1
+      with:
+        command: test
+        args: --locked --test system_wide_config -- --ignored
+
   documentation:
     name: Documentation
     runs-on: ubuntu-20.04
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 052b5a4325..47fac968ca 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Features
 
 - Make the default macOS theme depend on Dark Mode. See #2197, #1746 (@Enselic)
+- Support for separate system and user config files. See #668 (@patrickpichler)
 
 ## Bugfixes
 
diff --git a/README.md b/README.md
index bd98ddf351..b1e24339bc 100644
--- a/README.md
+++ b/README.md
@@ -639,6 +639,10 @@ A default configuration file can be created with the `--generate-config-file` op
 bat --generate-config-file
 ```
 
+There is also now a systemwide configuration file, which is located under `/etc/bat/config` on
+Linux and Mac OS and `C:\ProgramData\bat\config` on windows. If the system wide configuration
+file is present, the content of the user configuration will simply be appended to it.
+
 ### Format
 
 The configuration file is a simple list of command line arguments. Use `bat --help` to see a full list of possible options and values. In addition, you can add comments by prepending a line with the `#` character.
diff --git a/src/bin/bat/config.rs b/src/bin/bat/config.rs
index 4b4e8ff23b..696edf9e9c 100644
--- a/src/bin/bat/config.rs
+++ b/src/bin/bat/config.rs
@@ -6,6 +6,22 @@ use std::path::PathBuf;
 
 use crate::directories::PROJECT_DIRS;
 
+#[cfg(not(target_os = "windows"))]
+const DEFAULT_SYSTEM_CONFIG_PREFIX: &str = "/etc";
+
+#[cfg(target_os = "windows")]
+const DEFAULT_SYSTEM_CONFIG_PREFIX: &str = "C:\\ProgramData";
+
+pub fn system_config_file() -> PathBuf {
+    let folder = option_env!("BAT_SYSTEM_CONFIG_PREFIX").unwrap_or(DEFAULT_SYSTEM_CONFIG_PREFIX);
+    let mut path = PathBuf::from(folder);
+
+    path.push("bat");
+    path.push("config");
+
+    path
+}
+
 pub fn config_file() -> PathBuf {
     env::var("BAT_CONFIG_PATH")
         .ok()
@@ -87,11 +103,18 @@ pub fn generate_config_file() -> bat::error::Result<()> {
 }
 
 pub fn get_args_from_config_file() -> Result<Vec<OsString>, shell_words::ParseError> {
-    Ok(fs::read_to_string(config_file())
-        .ok()
-        .map(|content| get_args_from_str(&content))
-        .transpose()?
-        .unwrap_or_default())
+    let mut config = String::new();
+
+    if let Ok(c) = fs::read_to_string(system_config_file()) {
+        config.push_str(&c);
+        config.push('\n');
+    }
+
+    if let Ok(c) = fs::read_to_string(config_file()) {
+        config.push_str(&c);
+    }
+
+    get_args_from_str(&config)
 }
 
 pub fn get_args_from_env_var() -> Option<Result<Vec<OsString>, shell_words::ParseError>> {
diff --git a/src/bin/bat/main.rs b/src/bin/bat/main.rs
index 83e88fbb4b..b4e5d9b7ce 100644
--- a/src/bin/bat/main.rs
+++ b/src/bin/bat/main.rs
@@ -22,6 +22,9 @@ use crate::{
     config::{config_file, generate_config_file},
 };
 
+#[cfg(feature = "bugreport")]
+use crate::config::system_config_file;
+
 use assets::{assets_from_cache_or_binary, cache_dir, clear_assets, config_dir};
 use directories::PROJECT_DIRS;
 use globset::GlobMatcher;
@@ -259,6 +262,7 @@ fn invoke_bugreport(app: &App) {
             "NO_COLOR",
             "MANPAGER",
         ]))
+        .info(FileContent::new("System Config file", system_config_file()))
         .info(FileContent::new("Config file", config_file()))
         .info(FileContent::new(
             "Custom assets metadata",


Test patch:
diff --git a/tests/examples/bat.conf b/tests/examples/bat.conf
index 614ab40da6..4756cd8eac 100644
--- a/tests/examples/bat.conf
+++ b/tests/examples/bat.conf
@@ -1,5 +1,5 @@
 # Make sure that the pager gets executed
 --paging=always
 
-# Output a dummy message for the integration test.
+# Output a dummy message for the integration test and system wide config test.
 --pager="echo dummy-pager-from-config"
diff --git a/tests/examples/system_config/bat/config b/tests/examples/system_config/bat/config
new file mode 100644
index 0000000000..0750e8f55f
--- /dev/null
+++ b/tests/examples/system_config/bat/config
@@ -0,0 +1,5 @@
+# Make sure that the pager gets executed
+--paging=always
+
+# Output a dummy message for the integration test.
+--pager="echo dummy-pager-from-system-config"
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index c7d4c507c3..6a6263270d 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -1,9 +1,7 @@
-use assert_cmd::cargo::CommandCargoExt;
 use predicates::boolean::PredicateBooleanExt;
 use predicates::{prelude::predicate, str::PredicateStrExt};
 use serial_test::serial;
 use std::path::Path;
-use std::process::Command;
 use std::str::from_utf8;
 use tempfile::tempdir;
 
@@ -28,41 +26,14 @@ mod unix {
 use unix::*;
 
 mod utils;
+use utils::command::{bat, bat_with_config};
+
+#[cfg(unix)]
+use utils::command::bat_raw_command;
 use utils::mocked_pagers;
 
 const EXAMPLES_DIR: &str = "tests/examples";
 
-fn bat_raw_command_with_config() -> Command {
-    let mut cmd = Command::cargo_bin("bat").unwrap();
-    cmd.current_dir("tests/examples");
-    cmd.env_remove("BAT_CACHE_PATH");
-    cmd.env_remove("BAT_CONFIG_DIR");
-    cmd.env_remove("BAT_CONFIG_PATH");
-    cmd.env_remove("BAT_OPTS");
-    cmd.env_remove("BAT_PAGER");
-    cmd.env_remove("BAT_STYLE");
-    cmd.env_remove("BAT_TABS");
-    cmd.env_remove("BAT_THEME");
-    cmd.env_remove("COLORTERM");
-    cmd.env_remove("NO_COLOR");
-    cmd.env_remove("PAGER");
-    cmd
-}
-
-fn bat_raw_command() -> Command {
-    let mut cmd = bat_raw_command_with_config();
-    cmd.arg("--no-config");
-    cmd
-}
-
-fn bat_with_config() -> assert_cmd::Command {
-    assert_cmd::Command::from_std(bat_raw_command_with_config())
-}
-
-fn bat() -> assert_cmd::Command {
-    assert_cmd::Command::from_std(bat_raw_command())
-}
-
 #[test]
 fn basic() {
     bat()
diff --git a/tests/system_wide_config.rs b/tests/system_wide_config.rs
new file mode 100644
index 0000000000..7c2a9972df
--- /dev/null
+++ b/tests/system_wide_config.rs
@@ -0,0 +1,29 @@
+use predicates::{prelude::predicate, str::PredicateStrExt};
+
+mod utils;
+use utils::command::bat_with_config;
+
+// This test is ignored, as it needs a special system wide config put into place.
+// In order to run this tests, use `cargo test --test system_wide_config -- --ignored`
+#[test]
+#[ignore]
+fn use_systemwide_config() {
+    bat_with_config()
+        .arg("test.txt")
+        .assert()
+        .success()
+        .stdout(predicate::eq("dummy-pager-from-system-config\n").normalize());
+}
+
+// This test is ignored, as it needs a special system wide config put into place
+// In order to run this tests, use `cargo test --test system_wide_config -- --ignored`
+#[test]
+#[ignore]
+fn config_overrides_system_config() {
+    bat_with_config()
+        .env("BAT_CONFIG_PATH", "bat.conf")
+        .arg("test.txt")
+        .assert()
+        .success()
+        .stdout(predicate::eq("dummy-pager-from-config\n").normalize());
+}
diff --git a/tests/utils/command.rs b/tests/utils/command.rs
new file mode 100644
index 0000000000..87420f4b74
--- /dev/null
+++ b/tests/utils/command.rs
@@ -0,0 +1,38 @@
+#![allow(unused)] // Because indirectly included by e.g.integration_tests.rs, but not everything inside is used
+
+use assert_cmd::cargo::CommandCargoExt;
+use std::process::Command;
+
+pub fn bat_raw_command_with_config() -> Command {
+    let mut cmd = Command::cargo_bin("bat").unwrap();
+    cmd.current_dir("tests/examples");
+    cmd.env_remove("BAT_CACHE_PATH");
+    cmd.env_remove("BAT_CONFIG_DIR");
+    cmd.env_remove("BAT_CONFIG_PATH");
+    cmd.env_remove("BAT_OPTS");
+    cmd.env_remove("BAT_PAGER");
+    cmd.env_remove("BAT_STYLE");
+    cmd.env_remove("BAT_TABS");
+    cmd.env_remove("BAT_THEME");
+    cmd.env_remove("COLORTERM");
+    cmd.env_remove("NO_COLOR");
+    cmd.env_remove("PAGER");
+    cmd
+}
+
+#[cfg(test)]
+pub fn bat_raw_command() -> Command {
+    let mut cmd = bat_raw_command_with_config();
+    cmd.arg("--no-config");
+    cmd
+}
+
+#[cfg(test)]
+pub fn bat_with_config() -> assert_cmd::Command {
+    assert_cmd::Command::from_std(bat_raw_command_with_config())
+}
+
+#[cfg(test)]
+pub fn bat() -> assert_cmd::Command {
+    assert_cmd::Command::from_std(bat_raw_command())
+}
diff --git a/tests/utils/mocked_pagers.rs b/tests/utils/mocked_pagers.rs
index d115316d4b..07ae6fcddc 100644
--- a/tests/utils/mocked_pagers.rs
+++ b/tests/utils/mocked_pagers.rs
@@ -1,3 +1,5 @@
+#![allow(unused)] // Because indirectly included by e.g. system_wide_config.rs, but not used
+
 use assert_cmd::Command;
 use predicates::prelude::predicate;
 use std::env;
diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs
index 724efc5bfa..ca1a747a98 100644
--- a/tests/utils/mod.rs
+++ b/tests/utils/mod.rs
@@ -1,1 +1,2 @@
+pub mod command;
 pub mod mocked_pagers;