Repository: sharkdp/bat
File: src/pager.rs (Rust)

Code before:
    let (cmd, source) = match (config_pager, &bat_pager, &pager) {
        (Some(config_pager), _, _) => (config_pager, PagerSource::Config),
        (_, Ok(bat_pager), _) => (bat_pager.as_str(), PagerSource::EnvVarBatPager),
        (_, _, Ok(pager)) => (pager.as_str(), PagerSource::EnvVarPager),
        _ => ("less", PagerSource::Default),
    };

    let parts = shell_words::split(cmd)?;
    match parts.split_first() {
        Some((bin, args)) => {
            let kind = PagerKind::from_bin(bin);

            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![];
                Pager::new("less", &no_args, PagerKind::Less, PagerSource::EnvVarPager)
            } else {
                Pager::new(bin, args, kind, source)
            }))
        }
        None => Ok(None),
    }
}


Review comment (suggestion):
Maybe this would be a bit clearer? (with added comments)
``` rust
let use_less_instead = match (source, kind) {
  (PagerSource::EnvVarPager, PagerKind::More) => true,
  (PagerSource::EnvVarPager, PagerKind::Most) => true,
  (PagerSource::EnvVarPager, PagerKind::Bat) => true,
  _ => false,
};
```

Code after:
    let (cmd, source) = match (config_pager, &bat_pager, &pager) {
        (Some(config_pager), _, _) => (config_pager, PagerSource::Config),
        (_, Ok(bat_pager), _) => (bat_pager.as_str(), PagerSource::EnvVarBatPager),
        (_, _, Ok(pager)) => (pager.as_str(), PagerSource::EnvVarPager),
        _ => ("less", PagerSource::Default),
    };

    let parts = shell_words::split(cmd)?;
    match parts.split_first() {
        Some((bin, args)) => {
            let kind = PagerKind::from_bin(bin);

            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![];
                Pager::new("less", &no_args, PagerKind::Less, PagerSource::EnvVarPager)
            } else {
                Pager::new(bin, args, kind, source)
            }))
        }
        None => Ok(None),
    }
}