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 { - 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, + + // 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, 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), } }