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] 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());
 }