From b5ab1922ebcca82b9c930bc21d0d349312bb3dc3 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 16:48:13 +0100
Subject: [PATCH] Fix #1413 Can't use `bat` at all! (Error: Use of bat as a
 pager is disallowed...)

Fixed by implementing the proposal by sharkdp:

* Allow PAGER=bat, but ignore the setting in bat and simply default to
  less. Unless of course, BAT_PAGER or --pager is used to overwrite the
  value of PAGER.

* Disallow the usage of bat within BAT_PAGER and --pager.
---
 src/pager.rs               | 18 ++++++++++++++----
 tests/integration_tests.rs | 25 ++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/src/pager.rs b/src/pager.rs
index f6ae7bf42f..e16588640a 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -98,10 +98,20 @@ pub(crate) fn get_pager(config_pager: Option<&str>) -> Result<Option<Pager>, Par
         Some((bin, args)) => {
             let kind = PagerKind::from_bin(bin);
 
-            // 'more' and 'most' do not supports colors; automatically use 'less' instead
-            // if the problematic pager came from the generic PAGER env var
-            let no_color_support = kind == PagerKind::More || kind == PagerKind::Most;
-            let use_less_instead = no_color_support && source == PagerSource::EnvVarPager;
+            let use_less_instead = match source {
+                PagerSource::EnvVarPager => {
+                    // 'more' and 'most' do not supports colors; automatically use 'less' instead
+                    // if the problematic pager came from the generic PAGER env var
+                    let no_color_support = kind == PagerKind::More || kind == PagerKind::Most;
+
+                    // If PAGER=bat, silently use 'less' instead to prevent recursion ...
+                    let is_self = kind == PagerKind::Bat;
+
+                    no_color_support || is_self
+                },
+                // Never silently replace with less if bat-specific means to set a pager is used
+                _ => false
+            } ;
 
             Ok(Some(if use_less_instead {
                 let no_args = vec![];
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 6128b801b9..ce44933bae 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -499,6 +499,28 @@ fn pager_disable() {
         .stdout(predicate::eq("hello world\n").normalize());
 }
 
+#[test]
+fn env_var_pager_value_bat() {
+    bat()
+        .env("PAGER", "bat")
+        .arg("--paging=always")
+        .arg("test.txt")
+        .assert()
+        .success()
+        .stdout(predicate::eq("hello world\n").normalize());
+}
+
+#[test]
+fn env_var_bat_pager_value_bat() {
+    bat()
+        .env("BAT_PAGER", "bat")
+        .arg("--paging=always")
+        .arg("test.txt")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("bat as a pager is disallowed"));
+}
+
 #[test]
 fn pager_value_bat() {
     bat()
@@ -506,7 +528,8 @@ fn pager_value_bat() {
         .arg("--paging=always")
         .arg("test.txt")
         .assert()
-        .failure();
+        .failure()
+        .stderr(predicate::str::contains("bat as a pager is disallowed"));
 }
 
 /// We shall use less instead of most if PAGER is used since PAGER