Repository: sharkdp/bat (language: rust)

Issue:
Can't use `bat` at all! (Error: Use of bat as a pager is disallowed...)
**What version of `bat` are you using?**
bat 0.17.1

**Describe the bug you encountered:**

Whenever I use `bat`, I'm shown the following error message:

> [bat error]: Use of bat as a pager is disallowed in order to avoid infinite recursion problems

I believe this started happening after a certain update but can't exactly pinpoint which one. I've uninstalled and reinstalled `bat` again but with no luck.

**What did you expect to happen instead?**
For `bat` to display the contents as usual.

**How did you install `bat`?**
Homebrew

---

system
------

**$ uname -srm**
Darwin 20.1.0 x86_64  

**$ sw_vers**
ProductName:	macOS  
ProductVersion:	11.0.1  
BuildVersion:	20B29  

bat
---

**$ bat --version**
bat 0.17.1  

**$ env**
PAGER=bat  

bat_config
----------

bat_wrapper
-----------

No wrapper script for 'bat'.

bat_wrapper_function
--------------------

No wrapper function for 'bat'.

No wrapper function for 'cat'.

tool
----

**$ less --version**
less 487 (POSIX regular expressions)



Discussion / hints:
That's intentional, see https://github.com/sharkdp/bat/pull/1343
Personally, I don't like this restriction too (but it's not hard to overcome).

> That's intentional, see #1343

Hm. Kind of, but not to the extent documented here, actually. What we really want to avoid is infinite recursion (`bat` calls `bat` as pager, which calls `bat` as pager, …). See #1334.

What I didn't foresee is that users want to set `PAGER=bat` to use `bat` as a pager in other contexts.

So what we probably should do instead:
- 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`.
@cansurmeli Before the fix is out, as a workaround, you can pass `--paging=never` to bat.

I.e. this fails in 0.17.1:
`PAGER=bat bat file.txt`

But this works:
`PAGER=bat bat --paging=never file.txt`
Another workaround would be to use `--pager="less -FRX"`. You can also add that line to the `bat`s config file.
@sharkdp Did you see my PR with `PagerSource`? Seems to me as if that approach could be useful to implement the new behavior you propose here.
Ok, I get the picture now. I saw [#1343](https://github.com/sharkdp/bat/pull/1343) & [#1334](https://github.com/sharkdp/bat/issues/1334) before but didn't quite understand the situation.

In the meantime, `--paging=never` suffices as a workaround. Thanks for all the suggestions.
I will happily fix this bug once #1402 is merged, because it should be very simple, assuming the current refactoring proposal in that PR survives code review.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c9a24d09cf..eaca878eef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - Print an 'Invalid syntax theme settings' error message if a custom theme is broken, see #614 (@Enselic)
 - If plain mode is set and wrap is not explicitly opted in, long lines will no be truncated, see #1426
 - If `PAGER` (but not `BAT_PAGER` or `--pager`) is `more` or `most`, silently use `less` instead to ensure support for colors, see #1063 (@Enselic)
+- If `PAGER` is `bat`, silently use `less` to prevent recursion. For `BAT_PAGER` or `--pager`, exit with error, see #1413 (@Enselic)
 
 ## Other
 
diff --git a/src/pager.rs b/src/pager.rs
index f6ae7bf42f..c6a4170d90 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -98,10 +98,18 @@ 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, &kind) {
+                // 'more' and 'most' do not supports colors; automatically use 'less' instead
+                // if the problematic pager came from the generic PAGER env var
+                (PagerSource::EnvVarPager, PagerKind::More) => true,
+                (PagerSource::EnvVarPager, PagerKind::Most) => true,
+
+                // If PAGER=bat, silently use 'less' instead to prevent recursion ...
+                (PagerSource::EnvVarPager, PagerKind::Bat) => true,
+
+                // Never silently use less if BAT_PAGER or --pager has been specified
+                _ => false,
+            };
 
             Ok(Some(if use_less_instead {
                 let no_args = vec![];


Test patch:
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