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

Code before:
        (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);

            // Is false if the given expression does not match any of the
            // patterns; this ensures 'less' is never silently used if BAT_PAGER
            // or --pager has been specified.
            let use_less_instead = matches!(
                (&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)
                    | (PagerSource::EnvVarPager, PagerKind::Most)

                // If PAGER=bat, silently use 'less' instead to prevent
                // recursion ...
                    | (PagerSource::EnvVarPager, PagerKind::Bat)
            );

            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):
I personally think this is quite a bit harder to read now. The previous `match` isn't great either. Maybe this should be written with an outer `if` clause instead?

```rust
let use_less_instead = if source == PagerSource::EnvVarPager {
  // full comment here
  matches!(&kind, PagerKind::More | PagerKind::Most | …)
} else {
  false
};
```

Code after:
        (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 = if 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.
                // If PAGER=bat, silently use 'less' instead to prevent
                // recursion.
                // Never silently use 'less' if BAT_PAGER or --pager has been
                // specified.
                matches!(kind, PagerKind::More | PagerKind::Most | PagerKind::Bat)
            } else {
                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),
    }
}