From f4202361b4726bfceea429b5840c604622932807 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Thu, 26 Nov 2020 22:46:24 +0100
Subject: [PATCH 01/15] Add Pager helper with info about where the value comes
 from

In preparation of fixing issue #1063.
This is a pure refactoring with no intended functional side effects.
---
 src/lib.rs    |  2 ++
 src/output.rs | 37 ++++++++++---------------------------
 src/pager.rs  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 27 deletions(-)
 create mode 100644 src/pager.rs

diff --git a/src/lib.rs b/src/lib.rs
index 6d6dd206..3b89159f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -31,6 +31,8 @@ mod less;
 pub mod line_range;
 mod output;
 #[cfg(feature = "paging")]
+mod pager;
+#[cfg(feature = "paging")]
 pub(crate) mod paging;
 mod preprocessor;
 mod pretty_printer;
diff --git a/src/output.rs b/src/output.rs
index 689371b9..a9fe865e 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -48,37 +48,12 @@ impl OutputType {
         wrapping_mode: WrappingMode,
         pager_from_config: Option<&str>,
     ) -> Result<Self> {
-        use std::env;
         use std::ffi::OsString;
         use std::path::PathBuf;
         use std::process::{Command, Stdio};
+        use crate::pager::*;
 
-        let mut replace_arguments_to_less = false;
-
-        let pager_from_env = match (env::var("BAT_PAGER"), env::var("PAGER")) {
-            (Ok(bat_pager), _) => Some(bat_pager),
-            (_, Ok(pager)) => {
-                // less needs to be called with the '-R' option in order to properly interpret the
-                // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we
-                // therefore need to overwrite the arguments and add '-R'.
-                //
-                // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
-                // or bats '--pager' command line option.
-                replace_arguments_to_less = true;
-                Some(pager)
-            }
-            _ => None,
-        };
-
-        let pager_from_config = pager_from_config.map(|p| p.to_string());
-
-        if pager_from_config.is_some() {
-            replace_arguments_to_less = false;
-        }
-
-        let pager = pager_from_config
-            .or(pager_from_env)
-            .unwrap_or_else(|| String::from("less"));
+        let Pager { pager, source } = get_pager(pager_from_config);
 
         let pagerflags =
             shell_words::split(&pager).chain_err(|| "Could not parse pager command.")?;
@@ -94,6 +69,14 @@ impl OutputType {
                 let is_less = pager_path.file_stem() == Some(&OsString::from("less"));
 
                 let mut process = if is_less {
+                    // less needs to be called with the '-R' option in order to properly interpret the
+                    // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we
+                    // therefore need to overwrite the arguments and add '-R'.
+                    //
+                    // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
+                    // or bats '--pager' command line option.
+                    let replace_arguments_to_less = source == PagerSource::PagerEnvVar;
+
                     let mut p = Command::new(&pager_path);
                     if args.is_empty() || replace_arguments_to_less {
                         p.arg("--RAW-CONTROL-CHARS");
diff --git a/src/pager.rs b/src/pager.rs
new file mode 100644
index 00000000..62c01749
--- /dev/null
+++ b/src/pager.rs
@@ -0,0 +1,45 @@
+#[derive(Debug, PartialEq)]
+pub enum PagerSource {
+    /// From the env var BAT_PAGER
+    BatPagerEnvVar,
+
+    /// From the env var PAGER
+    PagerEnvVar,
+
+    /// From --config
+    Config,
+
+    /// No pager was specified, default is used
+    Default,
+}
+
+pub struct Pager {
+    pub pager: String,
+    pub source: PagerSource,
+}
+
+impl Pager {
+    fn new(
+        pager: &str,
+        source: PagerSource
+    ) -> Pager {
+        Pager {
+            pager: String::from(pager),
+            source,
+        }
+    }
+}
+
+pub fn get_pager(
+    pager_from_config: Option<&str>,
+) -> Pager {
+    if pager_from_config.is_some() {
+        return Pager::new(pager_from_config.unwrap(), PagerSource::Config);
+    } else {
+        return match (std::env::var("BAT_PAGER"), std::env::var("PAGER")) {
+            (Ok(bat_pager), _) => Pager::new(&bat_pager, PagerSource::BatPagerEnvVar),
+            (_, Ok(pager)) => Pager::new(&pager, PagerSource::PagerEnvVar),
+            _ => Pager::new("less", PagerSource::Default),
+        };
+    }
+}

From 986d0e9777a77b309ffc55de24fcb665adfa3bb7 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Fri, 27 Nov 2020 06:47:46 +0100
Subject: [PATCH 02/15] Ignore PAGER=most by default with a warning to stderr

closes #1063
---
 CHANGELOG.md               |  2 ++
 src/output.rs              |  5 +++++
 tests/integration_tests.rs | 24 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c866a5f..fa29e5e6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,8 @@
 
 ## Bugfixes
 
+- Ignore PAGER=most by default with a warning to stderr, but allow override with BAT_PAGER or --config, see #1063 (@Enselic)
+
 ## Other
 
 ## Syntaxes
diff --git a/src/output.rs b/src/output.rs
index a9fe865e..ac3047c2 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -66,6 +66,11 @@ impl OutputType {
                     return Err(ErrorKind::InvalidPagerValueBat.into());
                 }
 
+                if pager_path.file_stem() == Some(&OsString::from("most")) && source == PagerSource::PagerEnvVar {
+                    eprintln!("WARNING: Ignoring PAGER=\"{}\": Coloring not supported. Override with BAT_PAGER=\"{}\" or --pager \"{}\"", pager, pager, pager);
+                    return Ok(OutputType::stdout());
+                }
+
                 let is_less = pager_path.file_stem() == Some(&OsString::from("less"));
 
                 let mut process = if is_less {
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index ac11efb4..18c4dcc6 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -415,6 +415,30 @@ fn pager_value_bat() {
         .failure();
 }
 
+#[test]
+fn pager_most() {
+    bat()
+        .env("PAGER", "most")
+        .arg("--paging=always")
+        .arg("test.txt")
+        .assert()
+        .success()
+        .stderr(predicate::eq("WARNING: Ignoring PAGER=\"most\": Coloring not supported. Override with BAT_PAGER=\"most\" or --pager \"most\"\n").normalize())
+        .stdout(predicate::eq("hello world\n").normalize());
+}
+
+#[test]
+fn pager_most_with_arg() {
+    bat()
+        .env("PAGER", "most -w")
+        .arg("--paging=always")
+        .arg("test.txt")
+        .assert()
+        .success()
+        .stderr(predicate::eq("WARNING: Ignoring PAGER=\"most -w\": Coloring not supported. Override with BAT_PAGER=\"most -w\" or --pager \"most -w\"\n").normalize())
+        .stdout(predicate::eq("hello world\n").normalize());
+}
+
 #[test]
 fn alias_pager_disable() {
     bat()

From dcfe883f4be477dfe2ef3cef254d9446048a3bfe Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Mon, 28 Dec 2020 22:29:03 +0100
Subject: [PATCH 03/15] Simplify and polish pager.rs and related code

---
 CHANGELOG.md               |  2 +-
 src/output.rs              |  3 ++-
 src/pager.rs               | 35 +++++++++++++++++------------------
 tests/integration_tests.rs |  4 ++--
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d3af93ec..d3a98bb5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,7 +10,7 @@
 - Only print themes hint in interactive mode (`bat --list-themes`), see #1439 (@rsteube)
 - Make ./tests/syntax-tests/regression_test.sh work on recent versions of macOS, see #1443 (@Enselic)
 - VimL syntax highlighting fix, see #1450 (@esensar)
-- Ignore PAGER=most by default with a warning to stderr, but allow override with BAT_PAGER or --config, see #1063 (@Enselic)
+- Ignore PAGER=most with a warning to stderr, but allow override with BAT_PAGER or --config, see #1063 (@Enselic)
 
 ## Other
 
diff --git a/src/output.rs b/src/output.rs
index ac3047c2..ce311fe1 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -52,6 +52,7 @@ impl OutputType {
         use std::path::PathBuf;
         use std::process::{Command, Stdio};
         use crate::pager::*;
+        use crate::bat_warning;
 
         let Pager { pager, source } = get_pager(pager_from_config);
 
@@ -67,7 +68,7 @@ impl OutputType {
                 }
 
                 if pager_path.file_stem() == Some(&OsString::from("most")) && source == PagerSource::PagerEnvVar {
-                    eprintln!("WARNING: Ignoring PAGER=\"{}\": Coloring not supported. Override with BAT_PAGER=\"{}\" or --pager \"{}\"", pager, pager, pager);
+                    bat_warning!("Ignoring PAGER=\"{}\": Coloring not supported. Override with BAT_PAGER=\"{}\" or --pager \"{}\"", pager, pager, pager);
                     return Ok(OutputType::stdout());
                 }
 
diff --git a/src/pager.rs b/src/pager.rs
index 62c01749..0a4b1807 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -1,28 +1,27 @@
+/// If we use a pager, this enum tells us from where we were told to use it.
 #[derive(Debug, PartialEq)]
 pub enum PagerSource {
+    /// From --config
+    Config,
+
     /// From the env var BAT_PAGER
     BatPagerEnvVar,
 
     /// From the env var PAGER
     PagerEnvVar,
 
-    /// From --config
-    Config,
-
     /// No pager was specified, default is used
     Default,
 }
 
+/// A pager such as 'less', and from where we got it.
 pub struct Pager {
     pub pager: String,
     pub source: PagerSource,
 }
 
 impl Pager {
-    fn new(
-        pager: &str,
-        source: PagerSource
-    ) -> Pager {
+    fn new(pager: &str, source: PagerSource) -> Pager {
         Pager {
             pager: String::from(pager),
             source,
@@ -30,16 +29,16 @@ impl Pager {
     }
 }
 
-pub fn get_pager(
-    pager_from_config: Option<&str>,
-) -> Pager {
-    if pager_from_config.is_some() {
-        return Pager::new(pager_from_config.unwrap(), PagerSource::Config);
-    } else {
-        return match (std::env::var("BAT_PAGER"), std::env::var("PAGER")) {
-            (Ok(bat_pager), _) => Pager::new(&bat_pager, PagerSource::BatPagerEnvVar),
-            (_, Ok(pager)) => Pager::new(&pager, PagerSource::PagerEnvVar),
-            _ => Pager::new("less", PagerSource::Default),
-        };
+/// Returns what pager to use, after looking at both config and environment variables.
+pub fn get_pager(pager_from_config: Option<&str>) -> Pager {
+    match (
+        pager_from_config,
+        std::env::var("BAT_PAGER"),
+        std::env::var("PAGER"),
+    ) {
+        (Some(config), _, _) => Pager::new(config, PagerSource::Config),
+        (_, Ok(bat_pager), _) => Pager::new(&bat_pager, PagerSource::BatPagerEnvVar),
+        (_, _, Ok(pager)) => Pager::new(&pager, PagerSource::PagerEnvVar),
+        _ => Pager::new("less", PagerSource::Default),
     }
 }
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index eace816d..085e74c8 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -423,7 +423,7 @@ fn pager_most() {
         .arg("test.txt")
         .assert()
         .success()
-        .stderr(predicate::eq("WARNING: Ignoring PAGER=\"most\": Coloring not supported. Override with BAT_PAGER=\"most\" or --pager \"most\"\n").normalize())
+        .stderr(predicate::eq("\x1b[33m[bat warning]\x1b[0m: Ignoring PAGER=\"most\": Coloring not supported. Override with BAT_PAGER=\"most\" or --pager \"most\"\n").normalize())
         .stdout(predicate::eq("hello world\n").normalize());
 }
 
@@ -435,7 +435,7 @@ fn pager_most_with_arg() {
         .arg("test.txt")
         .assert()
         .success()
-        .stderr(predicate::eq("WARNING: Ignoring PAGER=\"most -w\": Coloring not supported. Override with BAT_PAGER=\"most -w\" or --pager \"most -w\"\n").normalize())
+        .stderr(predicate::eq("\x1b[33m[bat warning]\x1b[0m: Ignoring PAGER=\"most -w\": Coloring not supported. Override with BAT_PAGER=\"most -w\" or --pager \"most -w\"\n").normalize())
         .stdout(predicate::eq("hello world\n").normalize());
 }
 

From 22bdc7c20f21108ddc7be18775a8050059459ebf Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Wed, 30 Dec 2020 08:11:44 +0100
Subject: [PATCH 04/15] When PAGER=most, don't print a warning to stderr,
 silently use less instead

---
 CHANGELOG.md               |  2 +-
 src/output.rs              | 11 ++++++-----
 tests/integration_tests.rs |  4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index af46e011..f17b2da0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,7 +11,7 @@
 - Make ./tests/syntax-tests/regression_test.sh work on recent versions of macOS, see #1443 (@Enselic)
 - VimL syntax highlighting fix, see #1450 (@esensar)
 - Print an 'Invalid syntax theme settings' error message if a custom theme is broken, see #614 (@Enselic)
-- Ignore PAGER=most with a warning to stderr, but allow override with BAT_PAGER or --config, see #1063 (@Enselic)
+- If `PAGER=most` (but not `BAT_PAGER` or `--pager`), silently use `less` instead since `most` does not support colors, see #1063 (@Enselic)
 
 ## Other
 
diff --git a/src/output.rs b/src/output.rs
index ce311fe1..ed636cc4 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -52,7 +52,6 @@ impl OutputType {
         use std::path::PathBuf;
         use std::process::{Command, Stdio};
         use crate::pager::*;
-        use crate::bat_warning;
 
         let Pager { pager, source } = get_pager(pager_from_config);
 
@@ -60,16 +59,18 @@ impl OutputType {
             shell_words::split(&pager).chain_err(|| "Could not parse pager command.")?;
 
         match pagerflags.split_first() {
-            Some((pager_name, args)) => {
-                let pager_path = PathBuf::from(pager_name);
+            Some((pager_name, pager_args)) => {
+                let mut pager_path = PathBuf::from(pager_name);
+                let mut args = pager_args;
+                let empty_args = vec![];
 
                 if pager_path.file_stem() == Some(&OsString::from("bat")) {
                     return Err(ErrorKind::InvalidPagerValueBat.into());
                 }
 
                 if pager_path.file_stem() == Some(&OsString::from("most")) && source == PagerSource::PagerEnvVar {
-                    bat_warning!("Ignoring PAGER=\"{}\": Coloring not supported. Override with BAT_PAGER=\"{}\" or --pager \"{}\"", pager, pager, pager);
-                    return Ok(OutputType::stdout());
+                    pager_path = PathBuf::from("less");
+                    args = &empty_args;
                 }
 
                 let is_less = pager_path.file_stem() == Some(&OsString::from("less"));
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 085e74c8..8fec0140 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -423,8 +423,8 @@ fn pager_most() {
         .arg("test.txt")
         .assert()
         .success()
-        .stderr(predicate::eq("\x1b[33m[bat warning]\x1b[0m: Ignoring PAGER=\"most\": Coloring not supported. Override with BAT_PAGER=\"most\" or --pager \"most\"\n").normalize())
         .stdout(predicate::eq("hello world\n").normalize());
+    // TODO: How to ensure less is used?
 }
 
 #[test]
@@ -435,8 +435,8 @@ fn pager_most_with_arg() {
         .arg("test.txt")
         .assert()
         .success()
-        .stderr(predicate::eq("\x1b[33m[bat warning]\x1b[0m: Ignoring PAGER=\"most -w\": Coloring not supported. Override with BAT_PAGER=\"most -w\" or --pager \"most -w\"\n").normalize())
         .stdout(predicate::eq("hello world\n").normalize());
+    // TODO: How to ensure less is used?
 }
 
 #[test]

From bfa5342331cc17508ec2c528f2369514221ad3e4 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Wed, 30 Dec 2020 13:25:23 +0100
Subject: [PATCH 05/15] Also replace 'more' from PAGER with 'less'

But first do some quite significant refactorings to keep the code clean
and easy to understand.
---
 CHANGELOG.md  |   2 +-
 src/output.rs | 139 ++++++++++++++++++++++----------------------------
 src/pager.rs  | 102 +++++++++++++++++++++++++++++++-----
 3 files changed, 150 insertions(+), 93 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f17b2da0..7e88378d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,7 +11,7 @@
 - Make ./tests/syntax-tests/regression_test.sh work on recent versions of macOS, see #1443 (@Enselic)
 - VimL syntax highlighting fix, see #1450 (@esensar)
 - Print an 'Invalid syntax theme settings' error message if a custom theme is broken, see #614 (@Enselic)
-- If `PAGER=most` (but not `BAT_PAGER` or `--pager`), silently use `less` instead since `most` does not support colors, see #1063 (@Enselic)
+- If `PAGER` (but not `BAT_PAGER` or `--pager`) is `more` or `most`, silently use `less` instead to ensure support for colors, see #1063 (@Enselic)
 
 ## Other
 
diff --git a/src/output.rs b/src/output.rs
index ed636cc4..f0662060 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -48,90 +48,71 @@ impl OutputType {
         wrapping_mode: WrappingMode,
         pager_from_config: Option<&str>,
     ) -> Result<Self> {
-        use std::ffi::OsString;
-        use std::path::PathBuf;
+        use crate::pager::{self, PagerKind, PagerSource};
         use std::process::{Command, Stdio};
-        use crate::pager::*;
 
-        let Pager { pager, source } = get_pager(pager_from_config);
+        let pager_opt =
+            pager::get_pager(pager_from_config).chain_err(|| "Could not parse pager command.")?;
 
-        let pagerflags =
-            shell_words::split(&pager).chain_err(|| "Could not parse pager command.")?;
+        let pager = match pager_opt {
+            Some(pager) => pager,
+            None => return Ok(OutputType::stdout()),
+        };
 
-        match pagerflags.split_first() {
-            Some((pager_name, pager_args)) => {
-                let mut pager_path = PathBuf::from(pager_name);
-                let mut args = pager_args;
-                let empty_args = vec![];
-
-                if pager_path.file_stem() == Some(&OsString::from("bat")) {
-                    return Err(ErrorKind::InvalidPagerValueBat.into());
-                }
-
-                if pager_path.file_stem() == Some(&OsString::from("most")) && source == PagerSource::PagerEnvVar {
-                    pager_path = PathBuf::from("less");
-                    args = &empty_args;
-                }
-
-                let is_less = pager_path.file_stem() == Some(&OsString::from("less"));
-
-                let mut process = if is_less {
-                    // less needs to be called with the '-R' option in order to properly interpret the
-                    // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we
-                    // therefore need to overwrite the arguments and add '-R'.
-                    //
-                    // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
-                    // or bats '--pager' command line option.
-                    let replace_arguments_to_less = source == PagerSource::PagerEnvVar;
-
-                    let mut p = Command::new(&pager_path);
-                    if args.is_empty() || replace_arguments_to_less {
-                        p.arg("--RAW-CONTROL-CHARS");
-                        if single_screen_action == SingleScreenAction::Quit {
-                            p.arg("--quit-if-one-screen");
-                        }
-
-                        if wrapping_mode == WrappingMode::NoWrapping {
-                            p.arg("--chop-long-lines");
-                        }
-
-                        // Passing '--no-init' fixes a bug with '--quit-if-one-screen' in older
-                        // versions of 'less'. Unfortunately, it also breaks mouse-wheel support.
-                        //
-                        // See: http://www.greenwoodsoftware.com/less/news.530.html
-                        //
-                        // For newer versions (530 or 558 on Windows), we omit '--no-init' as it
-                        // is not needed anymore.
-                        match retrieve_less_version() {
-                            None => {
-                                p.arg("--no-init");
-                            }
-                            Some(version)
-                                if (version < 530 || (cfg!(windows) && version < 558)) =>
-                            {
-                                p.arg("--no-init");
-                            }
-                            _ => {}
-                        }
-                    } else {
-                        p.args(args);
-                    }
-                    p.env("LESSCHARSET", "UTF-8");
-                    p
-                } else {
-                    let mut p = Command::new(&pager_path);
-                    p.args(args);
-                    p
-                };
-
-                Ok(process
-                    .stdin(Stdio::piped())
-                    .spawn()
-                    .map(OutputType::Pager)
-                    .unwrap_or_else(|_| OutputType::stdout()))
-            }
-            None => Ok(OutputType::stdout()),
+        if pager.kind == PagerKind::Bat {
+            return Err(ErrorKind::InvalidPagerValueBat.into());
         }
+
+        let mut p = Command::new(pager.bin);
+        let args = pager.args;
+
+        if pager.kind == PagerKind::Less {
+            // less needs to be called with the '-R' option in order to properly interpret the
+            // ANSI color sequences printed by bat. If someone has set PAGER="less -F", we
+            // therefore need to overwrite the arguments and add '-R'.
+            //
+            // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
+            // or bats '--pager' command line option.
+            let replace_arguments_to_less = pager.source == PagerSource::PagerEnvVar;
+
+            if args.is_empty() || replace_arguments_to_less {
+                p.arg("--RAW-CONTROL-CHARS");
+                if single_screen_action == SingleScreenAction::Quit {
+                    p.arg("--quit-if-one-screen");
+                }
+
+                if wrapping_mode == WrappingMode::NoWrapping {
+                    p.arg("--chop-long-lines");
+                }
+
+                // Passing '--no-init' fixes a bug with '--quit-if-one-screen' in older
+                // versions of 'less'. Unfortunately, it also breaks mouse-wheel support.
+                //
+                // See: http://www.greenwoodsoftware.com/less/news.530.html
+                //
+                // For newer versions (530 or 558 on Windows), we omit '--no-init' as it
+                // is not needed anymore.
+                match retrieve_less_version() {
+                    None => {
+                        p.arg("--no-init");
+                    }
+                    Some(version) if (version < 530 || (cfg!(windows) && version < 558)) => {
+                        p.arg("--no-init");
+                    }
+                    _ => {}
+                }
+            } else {
+                p.args(args);
+            }
+            p.env("LESSCHARSET", "UTF-8");
+        } else {
+            p.args(args);
+        };
+
+        Ok(p.stdin(Stdio::piped())
+            .spawn()
+            .map(OutputType::Pager)
+            .unwrap_or_else(|_| OutputType::stdout()))
     }
 
     pub(crate) fn stdout() -> Self {
diff --git a/src/pager.rs b/src/pager.rs
index 0a4b1807..6b89d963 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -1,3 +1,6 @@
+use shell_words::ParseError;
+use std::env;
+
 /// If we use a pager, this enum tells us from where we were told to use it.
 #[derive(Debug, PartialEq)]
 pub enum PagerSource {
@@ -14,31 +17,104 @@ pub enum PagerSource {
     Default,
 }
 
+/// We know about some pagers, for example 'less'. This is a list of all pagers we know about
+#[derive(Debug, PartialEq)]
+pub enum PagerKind {
+    /// The pager is ourselves
+    Bat,
+
+    /// less
+    Less,
+
+    /// more
+    More,
+
+    /// most
+    Most,
+
+    /// A pager we don't know about
+    Unknown,
+}
+
+impl PagerKind {
+    fn from_bin(bin: &str) -> PagerKind {
+        use std::ffi::OsStr;
+        use std::path::Path;
+
+        let stem = Path::new(bin)
+            .file_stem()
+            .unwrap_or_else(|| OsStr::new("unknown"));
+
+        if stem == OsStr::new("bat") {
+            PagerKind::Bat
+        } else if stem == OsStr::new("less") {
+            PagerKind::Less
+        } else if stem == OsStr::new("more") {
+            PagerKind::More
+        } else if stem == OsStr::new("most") {
+            PagerKind::Most
+        } else {
+            PagerKind::Unknown
+        }
+    }
+}
+
 /// A pager such as 'less', and from where we got it.
+#[derive(Debug)]
 pub struct Pager {
-    pub pager: String,
+    // The pager binary
+    pub bin: String,
+
+    // The pager binary arguments (that we might tweak)
+    pub args: Vec<String>,
+
+    // What pager this is
+    pub kind: PagerKind,
+
+    // From where this pager comes
     pub source: PagerSource,
 }
 
 impl Pager {
-    fn new(pager: &str, source: PagerSource) -> Pager {
+    fn new(bin: &str, args: &[String], kind: PagerKind, source: PagerSource) -> Pager {
         Pager {
-            pager: String::from(pager),
+            bin: String::from(bin),
+            args: args.to_vec(),
+            kind,
             source,
         }
     }
 }
 
 /// Returns what pager to use, after looking at both config and environment variables.
-pub fn get_pager(pager_from_config: Option<&str>) -> Pager {
-    match (
-        pager_from_config,
-        std::env::var("BAT_PAGER"),
-        std::env::var("PAGER"),
-    ) {
-        (Some(config), _, _) => Pager::new(config, PagerSource::Config),
-        (_, Ok(bat_pager), _) => Pager::new(&bat_pager, PagerSource::BatPagerEnvVar),
-        (_, _, Ok(pager)) => Pager::new(&pager, PagerSource::PagerEnvVar),
-        _ => Pager::new("less", PagerSource::Default),
+pub fn get_pager(config_pager: Option<&str>) -> Result<Option<Pager>, ParseError> {
+    let bat_pager = env::var("BAT_PAGER");
+    let pager = env::var("PAGER");
+
+    let (cmd, source) = match (config_pager, &bat_pager, &pager) {
+        (Some(config_pager), _, _) => (config_pager, PagerSource::Config),
+        (_, Ok(bat_pager), _) => (bat_pager.as_str(), PagerSource::BatPagerEnvVar),
+        (_, _, Ok(pager)) => (pager.as_str(), PagerSource::PagerEnvVar),
+        _ => ("less", PagerSource::Default),
+    };
+
+    let parts = shell_words::split(cmd)?;
+    match parts.split_first() {
+        Some((bin, args)) => {
+            let kind = PagerKind::from_bin(bin);
+
+            // 'more' and 'most' does 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::PagerEnvVar;
+
+            Ok(Some(if use_less_instead {
+                let no_args = vec![];
+                Pager::new("less", &no_args, PagerKind::Less, PagerSource::PagerEnvVar)
+            } else {
+                Pager::new(bin, args, kind, source)
+            }))
+        }
+        None => Ok(None),
     }
 }

From c9efdd68edd6fede17ff21cac1d8ce0f826194f2 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Mon, 4 Jan 2021 10:23:13 +0100
Subject: [PATCH 06/15] Add integration tests for 'more' and 'most' used as
 pagers

---
 tests/integration_tests.rs | 141 ++++++++++++++++++++++++++++++++-----
 tests/mocked-pagers/more   |   2 +
 tests/mocked-pagers/most   |   2 +
 3 files changed, 127 insertions(+), 18 deletions(-)
 create mode 100755 tests/mocked-pagers/more
 create mode 100755 tests/mocked-pagers/most

diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 8fec0140..52c04975 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -1,6 +1,7 @@
 use assert_cmd::Command;
 use predicates::{prelude::predicate, str::PredicateStrExt};
-use std::path::Path;
+use std::env;
+use std::path::{Path, PathBuf};
 use std::str::from_utf8;
 
 const EXAMPLES_DIR: &str = "tests/examples";
@@ -23,6 +24,61 @@ fn bat() -> Command {
     cmd
 }
 
+/// For some tests we want mocked versions of some pagers
+/// This fn returns the absolute path to the directory with these mocked pagers
+fn get_mocked_pagers_dir() -> PathBuf {
+    let cargo_manifest_dir = env::var("CARGO_MANIFEST_DIR").expect("Missing CARGO_MANIFEST_DIR");
+    Path::new(&cargo_manifest_dir)
+        .join("tests")
+        .join("mocked-pagers")
+}
+
+/// Prepends a directory to the PATH environment variable
+/// Returns the original value for later restoration
+fn prepend_dir_to_path_env_var(dir: PathBuf) -> String {
+    // Get current PATH
+    let original_path = env::var("PATH").expect("No PATH?!");
+
+    // Add the new dir first
+    let mut split_paths = env::split_paths(&original_path).collect::<Vec<_>>();
+    split_paths.insert(0, dir);
+
+    // Set PATH with the new dir
+    let new_path = env::join_paths(split_paths).expect("Failed to join paths");
+    env::set_var("PATH", new_path);
+
+    // Return the original value for later restoration of it
+    original_path
+}
+
+/// Helper to restore the value of PATH
+fn restore_path(original_path: String) {
+    env::set_var("PATH", original_path);
+}
+
+/// Allows test to run that require our mocked versions of 'more' and 'most'
+/// in PATH. Temporarily changes PATH while the test code runs, and then restore it
+/// to avoid pollution of global state
+fn with_mocked_versions_of_more_and_most_in_path(actual_test: fn()) {
+    let original_path = prepend_dir_to_path_env_var(get_mocked_pagers_dir());
+
+    // Make sure our own variants of 'more' and 'most' is used
+    Command::new("more")
+        .assert()
+        .success()
+        .stdout("I am more\n");
+    Command::new("most")
+        .assert()
+        .success()
+        .stdout("I am most\n");
+
+    // Now run the actual test
+    actual_test();
+
+    // Make sure to restore PATH since it is global state
+    restore_path(original_path);
+}
+
 #[test]
 fn basic() {
     bat()
@@ -415,28 +471,77 @@ fn pager_value_bat() {
         .failure();
 }
 
+/// We shall use less instead of most if PAGER is used since PAGER
+/// is a generic env var
 #[test]
-fn pager_most() {
-    bat()
-        .env("PAGER", "most")
-        .arg("--paging=always")
-        .arg("test.txt")
-        .assert()
-        .success()
-        .stdout(predicate::eq("hello world\n").normalize());
-    // TODO: How to ensure less is used?
+fn pager_most_from_pager_env_var() {
+    with_mocked_versions_of_more_and_most_in_path(|| {
+        // If the output is not "I am most\n" then we know 'most' is not used
+        bat()
+            .env("PAGER", "most")
+            .arg("--paging=always")
+            .arg("test.txt")
+            .assert()
+            .success()
+            .stdout(predicate::eq("hello world\n").normalize());
+    });
 }
 
+/// If the bat-specific BAT_PAGER is used, obey the wish of the user
+/// and allow 'most'
+#[test]
+fn pager_most_from_bat_pager_env_var() {
+    with_mocked_versions_of_more_and_most_in_path(|| {
+        bat()
+            .env("BAT_PAGER", "most")
+            .arg("--paging=always")
+            .arg("test.txt")
+            .assert()
+            .success()
+            .stdout(predicate::eq("I am most\n").normalize());
+    });
+}
+
+/// Same reasoning with --pager as with BAT_PAGER
+#[test]
+fn pager_most_from_pager_arg() {
+    with_mocked_versions_of_more_and_most_in_path(|| {
+        bat()
+            .arg("--paging=always")
+            .arg("--pager=most")
+            .arg("test.txt")
+            .assert()
+            .success()
+            .stdout(predicate::eq("I am most\n").normalize());
+    });
+}
+
+/// Make sure the logic for 'most' applies even if an argument is passed
 #[test]
 fn pager_most_with_arg() {
-    bat()
-        .env("PAGER", "most -w")
-        .arg("--paging=always")
-        .arg("test.txt")
-        .assert()
-        .success()
-        .stdout(predicate::eq("hello world\n").normalize());
-    // TODO: How to ensure less is used?
+    with_mocked_versions_of_more_and_most_in_path(|| {
+        bat()
+            .env("PAGER", "most -w")
+            .arg("--paging=always")
+            .arg("test.txt")
+            .assert()
+            .success()
+            .stdout(predicate::eq("hello world\n").normalize());
+    });
+}
+
+/// Sanity check that 'more' is treated like 'most'
+#[test]
+fn pager_more() {
+    with_mocked_versions_of_more_and_most_in_path(|| {
+        bat()
+            .env("PAGER", "more")
+            .arg("--paging=always")
+            .arg("test.txt")
+            .assert()
+            .success()
+            .stdout(predicate::eq("hello world\n").normalize());
+    });
 }
 
 #[test]
diff --git a/tests/mocked-pagers/more b/tests/mocked-pagers/more
new file mode 100755
index 00000000..9b14f0b7
--- /dev/null
+++ b/tests/mocked-pagers/more
@@ -0,0 +1,2 @@
+#!/usr/bin/env bash
+echo "I am more"
diff --git a/tests/mocked-pagers/most b/tests/mocked-pagers/most
new file mode 100755
index 00000000..ec0962e5
--- /dev/null
+++ b/tests/mocked-pagers/most
@@ -0,0 +1,2 @@
+#!/usr/bin/env bash
+echo "I am most"

From df33ed05dd7ebd153f532bd38be2043f7b14d3be Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Mon, 4 Jan 2021 15:45:56 +0100
Subject: [PATCH 07/15] Run PATH-dependent tests serially

Since PATH is a shared resource.
---
 Cargo.lock                 | 94 ++++++++++++++++++++++++++++++++++++--
 Cargo.toml                 |  1 +
 tests/integration_tests.rs |  6 +++
 3 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 2475e202..16a991e3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -112,6 +112,7 @@ dependencies = [
  "semver",
  "serde",
  "serde_yaml",
+ "serial_test",
  "shell-words",
  "syntect",
  "tempdir",
@@ -218,6 +219,12 @@ version = "0.1.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
 
+[[package]]
+name = "cfg-if"
+version = "1.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
+
 [[package]]
 name = "chrono"
 version = "0.4.19"
@@ -281,7 +288,7 @@ version = "1.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ba125de2af0df55319f41944744ad91c71113bf74a4646efff39afe1f6842db1"
 dependencies = [
- "cfg-if",
+ "cfg-if 0.1.10",
 ]
 
 [[package]]
@@ -291,7 +298,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c3c7c73a2d1e9fc0886a08b93e98eb643461230d5f1925e4036204d5f2e261a8"
 dependencies = [
  "autocfg",
- "cfg-if",
+ "cfg-if 0.1.10",
  "lazy_static",
 ]
 
@@ -443,7 +450,7 @@ version = "1.0.18"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "da80be589a72651dcda34d8b35bcdc9b7254ad06325611074d9cc0fbb19f60ee"
 dependencies = [
- "cfg-if",
+ "cfg-if 0.1.10",
  "crc32fast",
  "libc",
  "miniz_oxide",
@@ -485,7 +492,7 @@ version = "0.1.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fc587bc0ec293155d5bfa6b9891ec18a1e330c234f896ea47fbada4cadbe47e6"
 dependencies = [
- "cfg-if",
+ "cfg-if 0.1.10",
  "libc",
  "wasi",
 ]
@@ -558,6 +565,15 @@ dependencies = [
  "hashbrown",
 ]
 
+[[package]]
+name = "instant"
+version = "0.1.9"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "61124eeebbd69b8190558df225adf7e4caafce0d743919e5d6b19652314ec5ec"
+dependencies = [
+ "cfg-if 1.0.0",
+]
+
 [[package]]
 name = "itoa"
 version = "0.4.6"
@@ -630,13 +646,22 @@ version = "0.5.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8dd5a6d5999d9907cda8ed67bbd137d3af8085216c2ac62de5be860bd41f304a"
 
+[[package]]
+name = "lock_api"
+version = "0.4.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dd96ffd135b2fd7b973ac026d28085defbe8983df057ced3eb4f2130b0831312"
+dependencies = [
+ "scopeguard",
+]
+
 [[package]]
 name = "log"
 version = "0.4.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4fabed175da42fed1fa0746b0ea71f412aa9d35e76e95e59b192c64b9dc2bf8b"
 dependencies = [
- "cfg-if",
+ "cfg-if 0.1.10",
 ]
 
 [[package]]
@@ -720,6 +745,31 @@ version = "0.2.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "2839e79665f131bdb5782e51f2c6c9599c133c6098982a54c794358bf432529c"
 
+[[package]]
+name = "parking_lot"
+version = "0.11.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6d7744ac029df22dca6284efe4e898991d28e3085c706c972bcd7da4a27a15eb"
+dependencies = [
+ "instant",
+ "lock_api",
+ "parking_lot_core",
+]
+
+[[package]]
+name = "parking_lot_core"
+version = "0.8.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9ccb628cad4f84851442432c60ad8e1f607e29752d0bf072cbd0baf28aa34272"
+dependencies = [
+ "cfg-if 1.0.0",
+ "instant",
+ "libc",
+ "redox_syscall",
+ "smallvec",
+ "winapi",
+]
+
 [[package]]
 name = "path_abs"
 version = "0.5.0"
@@ -959,6 +1009,12 @@ dependencies = [
  "winapi-util",
 ]
 
+[[package]]
+name = "scopeguard"
+version = "1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
+
 [[package]]
 name = "semver"
 version = "0.11.0"
@@ -1021,6 +1077,28 @@ dependencies = [
  "yaml-rust",
 ]
 
+[[package]]
+name = "serial_test"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e0bccbcf40c8938196944a3da0e133e031a33f4d6b72db3bda3cc556e361905d"
+dependencies = [
+ "lazy_static",
+ "parking_lot",
+ "serial_test_derive",
+]
+
+[[package]]
+name = "serial_test_derive"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b2acd6defeddb41eb60bb468f8825d0cfd0c2a76bc03bfd235b6a1dc4f6a1ad5"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "sha-1"
 version = "0.8.2"
@@ -1039,6 +1117,12 @@ version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b6fa3938c99da4914afedd13bf3d79bcb6c277d1b2c398d23257a304d9e1b074"
 
+[[package]]
+name = "smallvec"
+version = "1.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1a55ca5f3b68e41c979bf8c46a6f1da892ca4db8f94023ce0bd32407573b1ac0"
+
 [[package]]
 name = "std_prelude"
 version = "0.2.12"
diff --git a/Cargo.toml b/Cargo.toml
index dd4f61e3..63664537 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -74,6 +74,7 @@ default-features = false
 [dev-dependencies]
 tempdir = "0.3"
 assert_cmd = "1.0.2"
+serial_test = "0.5.1"
 predicates = "1.0.5"
 
 [build-dependencies]
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 52c04975..e5887d78 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -1,5 +1,6 @@
 use assert_cmd::Command;
 use predicates::{prelude::predicate, str::PredicateStrExt};
+use serial_test::serial;
 use std::env;
 use std::path::{Path, PathBuf};
 use std::str::from_utf8;
@@ -474,6 +475,7 @@ fn pager_value_bat() {
 /// We shall use less instead of most if PAGER is used since PAGER
 /// is a generic env var
 #[test]
+#[serial] // Because of PATH
 fn pager_most_from_pager_env_var() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         // If the output is not "I am most\n" then we know 'most' is not used
@@ -490,6 +492,7 @@ fn pager_most_from_pager_env_var() {
 /// If the bat-specific BAT_PAGER is used, obey the wish of the user
 /// and allow 'most'
 #[test]
+#[serial] // Because of PATH
 fn pager_most_from_bat_pager_env_var() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
@@ -504,6 +507,7 @@ fn pager_most_from_bat_pager_env_var() {
 
 /// Same reasoning with --pager as with BAT_PAGER
 #[test]
+#[serial] // Because of PATH
 fn pager_most_from_pager_arg() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
@@ -518,6 +522,7 @@ fn pager_most_from_pager_arg() {
 
 /// Make sure the logic for 'most' applies even if an argument is passed
 #[test]
+#[serial] // Because of PATH
 fn pager_most_with_arg() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
@@ -532,6 +537,7 @@ fn pager_most_with_arg() {
 
 /// Sanity check that 'more' is treated like 'most'
 #[test]
+#[serial] // Because of PATH
 fn pager_more() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()

From e87c554ccd0d4eb22794e1d193b8911b1d11edb1 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Mon, 4 Jan 2021 17:20:02 +0100
Subject: [PATCH 08/15] tests: Make mocked pagers work on Windows

---
 tests/integration_tests.rs   | 36 ++++++++++++++++++++++++------------
 tests/mocked-pagers/more     |  2 +-
 tests/mocked-pagers/more.bat |  1 +
 tests/mocked-pagers/most     |  2 +-
 tests/mocked-pagers/most.bat |  1 +
 5 files changed, 28 insertions(+), 14 deletions(-)
 create mode 100755 tests/mocked-pagers/more.bat
 create mode 100755 tests/mocked-pagers/most.bat

diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index e5887d78..a85f0fd4 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -34,6 +34,18 @@ fn get_mocked_pagers_dir() -> PathBuf {
         .join("mocked-pagers")
 }
 
+/// On Unix: 'most' -> 'most'
+/// On Windows: 'most' -> 'most.bat'
+fn mocked_pager(base: &str) -> String {
+    if cfg!(windows) {
+        let mut str = String::from(base);
+        str.push_str(".bat");
+        str
+    } else {
+        String::from(base)
+    }
+}
+
 /// Prepends a directory to the PATH environment variable
 /// Returns the original value for later restoration
 fn prepend_dir_to_path_env_var(dir: PathBuf) -> String {
@@ -64,14 +76,14 @@ fn with_mocked_versions_of_more_and_most_in_path(actual_test: fn()) {
     let original_path = prepend_dir_to_path_env_var(get_mocked_pagers_dir());
 
     // Make sure our own variants of 'more' and 'most' is used
-    Command::new("more")
+    Command::new(mocked_pager("more"))
         .assert()
         .success()
-        .stdout("I am more\n");
-    Command::new("most")
+        .stdout(predicate::str::contains("I am more"));
+    Command::new(mocked_pager("most"))
         .assert()
         .success()
-        .stdout("I am most\n");
+        .stdout(predicate::str::contains("I am most"));
 
     // Now run the actual test
     actual_test();
@@ -478,9 +490,9 @@ fn pager_value_bat() {
 #[serial] // Because of PATH
 fn pager_most_from_pager_env_var() {
     with_mocked_versions_of_more_and_most_in_path(|| {
-        // If the output is not "I am most\n" then we know 'most' is not used
+        // If the output is not "I am most" then we know 'most' is not used
         bat()
-            .env("PAGER", "most")
+            .env("PAGER", mocked_pager("most"))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
@@ -496,12 +508,12 @@ fn pager_most_from_pager_env_var() {
 fn pager_most_from_bat_pager_env_var() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
-            .env("BAT_PAGER", "most")
+            .env("BAT_PAGER", mocked_pager("most"))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
             .success()
-            .stdout(predicate::eq("I am most\n").normalize());
+            .stdout(predicate::str::contains("I am most"));
     });
 }
 
@@ -512,11 +524,11 @@ fn pager_most_from_pager_arg() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
             .arg("--paging=always")
-            .arg("--pager=most")
+            .arg(format!("--pager={}", mocked_pager("most")))
             .arg("test.txt")
             .assert()
             .success()
-            .stdout(predicate::eq("I am most\n").normalize());
+            .stdout(predicate::str::contains("I am most"));
     });
 }
 
@@ -526,7 +538,7 @@ fn pager_most_from_pager_arg() {
 fn pager_most_with_arg() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
-            .env("PAGER", "most -w")
+            .env("PAGER", format!("{} -w", mocked_pager("most")))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
@@ -541,7 +553,7 @@ fn pager_most_with_arg() {
 fn pager_more() {
     with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
-            .env("PAGER", "more")
+            .env("PAGER", mocked_pager("more"))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
diff --git a/tests/mocked-pagers/more b/tests/mocked-pagers/more
index 9b14f0b7..cb57549b 100755
--- a/tests/mocked-pagers/more
+++ b/tests/mocked-pagers/more
@@ -1,2 +1,2 @@
 #!/usr/bin/env bash
-echo "I am more"
+echo I am more
diff --git a/tests/mocked-pagers/more.bat b/tests/mocked-pagers/more.bat
new file mode 100755
index 00000000..61c3ac08
--- /dev/null
+++ b/tests/mocked-pagers/more.bat
@@ -0,0 +1 @@
+ECHO I am more
diff --git a/tests/mocked-pagers/most b/tests/mocked-pagers/most
index ec0962e5..29ee1ea3 100755
--- a/tests/mocked-pagers/most
+++ b/tests/mocked-pagers/most
@@ -1,2 +1,2 @@
 #!/usr/bin/env bash
-echo "I am most"
+echo I am most
diff --git a/tests/mocked-pagers/most.bat b/tests/mocked-pagers/most.bat
new file mode 100755
index 00000000..1082ee07
--- /dev/null
+++ b/tests/mocked-pagers/most.bat
@@ -0,0 +1 @@
+ECHO I am most

From fc30277cfa7c5d0df186460d572af074d0a5aa60 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 13:07:24 +0100
Subject: [PATCH 09/15] pager.rs: Limit visibilities to pub(crate)

---
 src/pager.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/pager.rs b/src/pager.rs
index 6b89d963..947f522d 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -3,7 +3,7 @@ use std::env;
 
 /// If we use a pager, this enum tells us from where we were told to use it.
 #[derive(Debug, PartialEq)]
-pub enum PagerSource {
+pub(crate) enum PagerSource {
     /// From --config
     Config,
 
@@ -19,7 +19,7 @@ pub enum PagerSource {
 
 /// We know about some pagers, for example 'less'. This is a list of all pagers we know about
 #[derive(Debug, PartialEq)]
-pub enum PagerKind {
+pub(crate) enum PagerKind {
     /// The pager is ourselves
     Bat,
 
@@ -61,7 +61,7 @@ impl PagerKind {
 
 /// A pager such as 'less', and from where we got it.
 #[derive(Debug)]
-pub struct Pager {
+pub(crate) struct Pager {
     // The pager binary
     pub bin: String,
 
@@ -87,7 +87,7 @@ impl Pager {
 }
 
 /// Returns what pager to use, after looking at both config and environment variables.
-pub fn get_pager(config_pager: Option<&str>) -> Result<Option<Pager>, ParseError> {
+pub(crate) fn get_pager(config_pager: Option<&str>) -> Result<Option<Pager>, ParseError> {
     let bat_pager = env::var("BAT_PAGER");
     let pager = env::var("PAGER");
 

From dfe7a601402b974b57f8f103fe5dfec69b69f14a Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 13:16:09 +0100
Subject: [PATCH 10/15] PagerSource: [Bat]PagerEnvVar -> EnvVar[Bat]Pager

---
 src/output.rs |  2 +-
 src/pager.rs  | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/output.rs b/src/output.rs
index 539ea645..c79535c3 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -73,7 +73,7 @@ impl OutputType {
             //
             // We only do this for PAGER (as it is not specific to 'bat'), not for BAT_PAGER
             // or bats '--pager' command line option.
-            let replace_arguments_to_less = pager.source == PagerSource::PagerEnvVar;
+            let replace_arguments_to_less = pager.source == PagerSource::EnvVarPager;
 
             if args.is_empty() || replace_arguments_to_less {
                 p.arg("--RAW-CONTROL-CHARS");
diff --git a/src/pager.rs b/src/pager.rs
index 947f522d..c99d2994 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -8,10 +8,10 @@ pub(crate) enum PagerSource {
     Config,
 
     /// From the env var BAT_PAGER
-    BatPagerEnvVar,
+    EnvVarBatPager,
 
     /// From the env var PAGER
-    PagerEnvVar,
+    EnvVarPager,
 
     /// No pager was specified, default is used
     Default,
@@ -93,8 +93,8 @@ pub(crate) fn get_pager(config_pager: Option<&str>) -> Result<Option<Pager>, Par
 
     let (cmd, source) = match (config_pager, &bat_pager, &pager) {
         (Some(config_pager), _, _) => (config_pager, PagerSource::Config),
-        (_, Ok(bat_pager), _) => (bat_pager.as_str(), PagerSource::BatPagerEnvVar),
-        (_, _, Ok(pager)) => (pager.as_str(), PagerSource::PagerEnvVar),
+        (_, Ok(bat_pager), _) => (bat_pager.as_str(), PagerSource::EnvVarBatPager),
+        (_, _, Ok(pager)) => (pager.as_str(), PagerSource::EnvVarPager),
         _ => ("less", PagerSource::Default),
     };
 
@@ -106,11 +106,11 @@ pub(crate) fn get_pager(config_pager: Option<&str>) -> Result<Option<Pager>, Par
             // 'more' and 'most' does 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::PagerEnvVar;
+            let use_less_instead = no_color_support && source == PagerSource::EnvVarPager;
 
             Ok(Some(if use_less_instead {
                 let no_args = vec![];
-                Pager::new("less", &no_args, PagerKind::Less, PagerSource::PagerEnvVar)
+                Pager::new("less", &no_args, PagerKind::Less, PagerSource::EnvVarPager)
             } else {
                 Pager::new(bin, args, kind, source)
             }))

From dd6f57e1073100ab0f85f272177efe9568550940 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 13:25:18 +0100
Subject: [PATCH 11/15] pager.rs: Some comment fixups

---
 src/pager.rs               | 12 ++++++------
 tests/integration_tests.rs |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/pager.rs b/src/pager.rs
index c99d2994..fa5693a3 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -20,7 +20,7 @@ pub(crate) enum PagerSource {
 /// We know about some pagers, for example 'less'. This is a list of all pagers we know about
 #[derive(Debug, PartialEq)]
 pub(crate) enum PagerKind {
-    /// The pager is ourselves
+    /// bat
     Bat,
 
     /// less
@@ -62,16 +62,16 @@ impl PagerKind {
 /// A pager such as 'less', and from where we got it.
 #[derive(Debug)]
 pub(crate) struct Pager {
-    // The pager binary
+    /// The pager binary
     pub bin: String,
 
-    // The pager binary arguments (that we might tweak)
+    /// The pager binary arguments (that we might tweak)
     pub args: Vec<String>,
 
-    // What pager this is
+    /// What pager this is
     pub kind: PagerKind,
 
-    // From where this pager comes
+    /// From where this pager comes
     pub source: PagerSource,
 }
 
@@ -103,7 +103,7 @@ 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' does not supports colors; automatically use 'less' instead
+            // '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;
diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index 49aea8b9..d2697564 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -91,7 +91,7 @@ fn restore_path(original_path: String) {
 fn with_mocked_versions_of_more_and_most_in_path(actual_test: fn()) {
     let original_path = prepend_dir_to_path_env_var(get_mocked_pagers_dir());
 
-    // Make sure our own variants of 'more' and 'most' is used
+    // Make sure our own variants of 'more' and 'most' are used
     Command::new(mocked_pager("more"))
         .assert()
         .success()

From c2c2b0211a7efa2058c0cf02467db1e47413d62a Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 13:26:40 +0100
Subject: [PATCH 12/15] fn mocked_pager: Simplify with format!

---
 tests/integration_tests.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index d2697564..b368ba5f 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -54,9 +54,7 @@ fn get_mocked_pagers_dir() -> PathBuf {
 /// On Windows: 'most' -> 'most.bat'
 fn mocked_pager(base: &str) -> String {
     if cfg!(windows) {
-        let mut str = String::from(base);
-        str.push_str(".bat");
-        str
+        format!("{}.bat", base)
     } else {
         String::from(base)
     }

From 7809008016aef9af72a285d1f7de9c978acba737 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 13:27:17 +0100
Subject: [PATCH 13/15] PagerKind::from(): Simplify

---
 src/pager.rs | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/pager.rs b/src/pager.rs
index fa5693a3..f6ae7bf4 100644
--- a/src/pager.rs
+++ b/src/pager.rs
@@ -38,23 +38,18 @@ pub(crate) enum PagerKind {
 
 impl PagerKind {
     fn from_bin(bin: &str) -> PagerKind {
-        use std::ffi::OsStr;
         use std::path::Path;
 
-        let stem = Path::new(bin)
+        match Path::new(bin)
             .file_stem()
-            .unwrap_or_else(|| OsStr::new("unknown"));
-
-        if stem == OsStr::new("bat") {
-            PagerKind::Bat
-        } else if stem == OsStr::new("less") {
-            PagerKind::Less
-        } else if stem == OsStr::new("more") {
-            PagerKind::More
-        } else if stem == OsStr::new("most") {
-            PagerKind::Most
-        } else {
-            PagerKind::Unknown
+            .map(|s| s.to_string_lossy())
+            .as_deref()
+        {
+            Some("bat") => PagerKind::Bat,
+            Some("less") => PagerKind::Less,
+            Some("more") => PagerKind::More,
+            Some("most") => PagerKind::Most,
+            _ => PagerKind::Unknown,
         }
     }
 }

From eda72c31b27584d61166c48b36c642cd3a51c3a3 Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 14:05:39 +0100
Subject: [PATCH 14/15] tests: Move 'mocked pagers' utils to separate file

---
 tests/integration_tests.rs   | 91 ++++++------------------------------
 tests/utils/mocked_pagers.rs | 69 +++++++++++++++++++++++++++
 tests/utils/mod.rs           |  1 +
 3 files changed, 84 insertions(+), 77 deletions(-)
 create mode 100644 tests/utils/mocked_pagers.rs
 create mode 100644 tests/utils/mod.rs

diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs
index b368ba5f..6128b801 100644
--- a/tests/integration_tests.rs
+++ b/tests/integration_tests.rs
@@ -2,15 +2,17 @@ use assert_cmd::assert::OutputAssertExt;
 use assert_cmd::cargo::CommandCargoExt;
 use predicates::{prelude::predicate, str::PredicateStrExt};
 use serial_test::serial;
-use std::env;
 use std::fs::File;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::process::{Command, Stdio};
 use std::str::from_utf8;
 
 #[cfg(unix)]
 use std::time::Duration;
 
+mod utils;
+use utils::mocked_pagers;
+
 const EXAMPLES_DIR: &str = "tests/examples";
 
 #[cfg(unix)]
@@ -41,71 +43,6 @@ fn bat() -> assert_cmd::Command {
     cmd
 }
 
-/// For some tests we want mocked versions of some pagers
-/// This fn returns the absolute path to the directory with these mocked pagers
-fn get_mocked_pagers_dir() -> PathBuf {
-    let cargo_manifest_dir = env::var("CARGO_MANIFEST_DIR").expect("Missing CARGO_MANIFEST_DIR");
-    Path::new(&cargo_manifest_dir)
-        .join("tests")
-        .join("mocked-pagers")
-}
-
-/// On Unix: 'most' -> 'most'
-/// On Windows: 'most' -> 'most.bat'
-fn mocked_pager(base: &str) -> String {
-    if cfg!(windows) {
-        format!("{}.bat", base)
-    } else {
-        String::from(base)
-    }
-}
-
-/// Prepends a directory to the PATH environment variable
-/// Returns the original value for later restoration
-fn prepend_dir_to_path_env_var(dir: PathBuf) -> String {
-    // Get current PATH
-    let original_path = env::var("PATH").expect("No PATH?!");
-
-    // Add the new dir first
-    let mut split_paths = env::split_paths(&original_path).collect::<Vec<_>>();
-    split_paths.insert(0, dir);
-
-    // Set PATH with the new dir
-    let new_path = env::join_paths(split_paths).expect("Failed to join paths");
-    env::set_var("PATH", new_path);
-
-    // Return the original value for later restoration of it
-    original_path
-}
-
-/// Helper to restore the value of PATH
-fn restore_path(original_path: String) {
-    env::set_var("PATH", original_path);
-}
-
-/// Allows test to run that require our mocked versions of 'more' and 'most'
-/// in PATH. Temporarily changes PATH while the test code runs, and then restore it
-/// to avoid pollution of global state
-fn with_mocked_versions_of_more_and_most_in_path(actual_test: fn()) {
-    let original_path = prepend_dir_to_path_env_var(get_mocked_pagers_dir());
-
-    // Make sure our own variants of 'more' and 'most' are used
-    Command::new(mocked_pager("more"))
-        .assert()
-        .success()
-        .stdout(predicate::str::contains("I am more"));
-    Command::new(mocked_pager("most"))
-        .assert()
-        .success()
-        .stdout(predicate::str::contains("I am most"));
-
-    // Now run the actual test
-    actual_test();
-
-    // Make sure to restore PATH since it is global state
-    restore_path(original_path);
-}
-
 #[test]
 fn basic() {
     bat()
@@ -577,10 +514,10 @@ fn pager_value_bat() {
 #[test]
 #[serial] // Because of PATH
 fn pager_most_from_pager_env_var() {
-    with_mocked_versions_of_more_and_most_in_path(|| {
+    mocked_pagers::with_mocked_versions_of_more_and_most_in_path(|| {
         // If the output is not "I am most" then we know 'most' is not used
         bat()
-            .env("PAGER", mocked_pager("most"))
+            .env("PAGER", mocked_pagers::from("most"))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
@@ -594,9 +531,9 @@ fn pager_most_from_pager_env_var() {
 #[test]
 #[serial] // Because of PATH
 fn pager_most_from_bat_pager_env_var() {
-    with_mocked_versions_of_more_and_most_in_path(|| {
+    mocked_pagers::with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
-            .env("BAT_PAGER", mocked_pager("most"))
+            .env("BAT_PAGER", mocked_pagers::from("most"))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
@@ -609,10 +546,10 @@ fn pager_most_from_bat_pager_env_var() {
 #[test]
 #[serial] // Because of PATH
 fn pager_most_from_pager_arg() {
-    with_mocked_versions_of_more_and_most_in_path(|| {
+    mocked_pagers::with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
             .arg("--paging=always")
-            .arg(format!("--pager={}", mocked_pager("most")))
+            .arg(format!("--pager={}", mocked_pagers::from("most")))
             .arg("test.txt")
             .assert()
             .success()
@@ -624,9 +561,9 @@ fn pager_most_from_pager_arg() {
 #[test]
 #[serial] // Because of PATH
 fn pager_most_with_arg() {
-    with_mocked_versions_of_more_and_most_in_path(|| {
+    mocked_pagers::with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
-            .env("PAGER", format!("{} -w", mocked_pager("most")))
+            .env("PAGER", format!("{} -w", mocked_pagers::from("most")))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
@@ -639,9 +576,9 @@ fn pager_most_with_arg() {
 #[test]
 #[serial] // Because of PATH
 fn pager_more() {
-    with_mocked_versions_of_more_and_most_in_path(|| {
+    mocked_pagers::with_mocked_versions_of_more_and_most_in_path(|| {
         bat()
-            .env("PAGER", mocked_pager("more"))
+            .env("PAGER", mocked_pagers::from("more"))
             .arg("--paging=always")
             .arg("test.txt")
             .assert()
diff --git a/tests/utils/mocked_pagers.rs b/tests/utils/mocked_pagers.rs
new file mode 100644
index 00000000..d115316d
--- /dev/null
+++ b/tests/utils/mocked_pagers.rs
@@ -0,0 +1,69 @@
+use assert_cmd::Command;
+use predicates::prelude::predicate;
+use std::env;
+use std::path::{Path, PathBuf};
+
+/// For some tests we want mocked versions of some pagers
+/// This fn returns the absolute path to the directory with these mocked pagers
+fn get_mocked_pagers_dir() -> PathBuf {
+    let cargo_manifest_dir = env::var("CARGO_MANIFEST_DIR").expect("Missing CARGO_MANIFEST_DIR");
+    Path::new(&cargo_manifest_dir)
+        .join("tests")
+        .join("mocked-pagers")
+}
+
+/// On Unix: 'most' -> 'most'
+/// On Windows: 'most' -> 'most.bat'
+pub fn from(base: &str) -> String {
+    if cfg!(windows) {
+        format!("{}.bat", base)
+    } else {
+        String::from(base)
+    }
+}
+
+/// Prepends a directory to the PATH environment variable
+/// Returns the original value for later restoration
+fn prepend_dir_to_path_env_var(dir: PathBuf) -> String {
+    // Get current PATH
+    let original_path = env::var("PATH").expect("No PATH?!");
+
+    // Add the new dir first
+    let mut split_paths = env::split_paths(&original_path).collect::<Vec<_>>();
+    split_paths.insert(0, dir);
+
+    // Set PATH with the new dir
+    let new_path = env::join_paths(split_paths).expect("Failed to join paths");
+    env::set_var("PATH", new_path);
+
+    // Return the original value for later restoration of it
+    original_path
+}
+
+/// Helper to restore the value of PATH
+fn restore_path(original_path: String) {
+    env::set_var("PATH", original_path);
+}
+
+/// Allows test to run that require our mocked versions of 'more' and 'most'
+/// in PATH. Temporarily changes PATH while the test code runs, and then restore it
+/// to avoid pollution of global state
+pub fn with_mocked_versions_of_more_and_most_in_path(actual_test: fn()) {
+    let original_path = prepend_dir_to_path_env_var(get_mocked_pagers_dir());
+
+    // Make sure our own variants of 'more' and 'most' are used
+    Command::new(from("more"))
+        .assert()
+        .success()
+        .stdout(predicate::str::contains("I am more"));
+    Command::new(from("most"))
+        .assert()
+        .success()
+        .stdout(predicate::str::contains("I am most"));
+
+    // Now run the actual test
+    actual_test();
+
+    // Make sure to restore PATH since it is global state
+    restore_path(original_path);
+}
diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs
new file mode 100644
index 00000000..724efc5b
--- /dev/null
+++ b/tests/utils/mod.rs
@@ -0,0 +1 @@
+pub mod mocked_pagers;

From 8dd67cca0c808e44774fb5d4b68999f63aa3cccd Mon Sep 17 00:00:00 2001
From: Martin Nordholts <enselic@gmail.com>
Date: Sun, 10 Jan 2021 14:11:57 +0100
Subject: [PATCH 15/15] Revert accidental change to
 assets/syntaxes/02_Extra/VimL

---
 assets/syntaxes/02_Extra/VimL | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/assets/syntaxes/02_Extra/VimL b/assets/syntaxes/02_Extra/VimL
index 23afc890..7ebcaa1d 160000
--- a/assets/syntaxes/02_Extra/VimL
+++ b/assets/syntaxes/02_Extra/VimL
@@ -1 +1 @@
-Subproject commit 23afc890977bb1fd43fd05e7d983f994993d4982
+Subproject commit 7ebcaa1d987be059213f06bfc0833dcaea9e0b91