Skip to content

Commit

Permalink
Capture env var configurations using clap
Browse files Browse the repository at this point in the history
Parses env configurations into matches (for those that can be configured
via matches) instead of relying on dedicated env checks everywhere.

Fixes sharkdp#1152
  • Loading branch information
jacobmischka committed Oct 7, 2020
1 parent 5e0b7f0 commit 36cddbf
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 28 deletions.
14 changes: 0 additions & 14 deletions src/bin/bat/app.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::collections::HashSet;
use std::env;
use std::ffi::OsStr;
use std::str::FromStr;

use atty::{self, Stream};

Expand Down Expand Up @@ -183,7 +182,6 @@ impl App {
.matches
.value_of("tabs")
.map(String::from)
.or_else(|| env::var("BAT_TABS").ok())
.and_then(|t| t.parse().ok())
.unwrap_or(
if style_components.plain() && paging_mode == PagingMode::Never {
Expand All @@ -196,7 +194,6 @@ impl App {
.matches
.value_of("theme")
.map(String::from)
.or_else(|| env::var("BAT_THEME").ok())
.map(|s| {
if s == "default" {
String::from(HighlightingAssets::default_theme())
Expand Down Expand Up @@ -296,16 +293,6 @@ impl App {
} else if matches.is_present("plain") {
[StyleComponent::Plain].iter().cloned().collect()
} else {
let env_style_components: Option<Vec<StyleComponent>> = env::var("BAT_STYLE")
.ok()
.map(|style_str| {
style_str
.split(',')
.map(|x| StyleComponent::from_str(&x))
.collect::<Result<Vec<StyleComponent>>>()
})
.transpose()?;

matches
.value_of("style")
.map(|styles| {
Expand All @@ -315,7 +302,6 @@ impl App {
.filter_map(|style| style.ok())
.collect::<Vec<_>>()
})
.or(env_style_components)
.unwrap_or_else(|| vec![StyleComponent::Full])
.into_iter()
.map(|style| style.components(self.interactive_output))
Expand Down
4 changes: 4 additions & 0 deletions src/bin/bat/clap_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> {
app = app.arg(
Arg::with_name("tabs")
.long("tabs")
.env("BAT_TABS")
.overrides_with("tabs")
.takes_value(true)
.value_name("T")
Expand Down Expand Up @@ -303,6 +304,7 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> {
.arg(
Arg::with_name("pager")
.long("pager")
.env("BAT_PAGER")
.overrides_with("pager")
.takes_value(true)
.value_name("command")
Expand Down Expand Up @@ -335,6 +337,7 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> {
.arg(
Arg::with_name("theme")
.long("theme")
.env("BAT_THEME")
.overrides_with("theme")
.takes_value(true)
.help("Set the color theme for syntax highlighting.")
Expand All @@ -356,6 +359,7 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> {
Arg::with_name("style")
.long("style")
.value_name("components")
.env("BAT_STYLE")
// Need to turn this off for overrides_with to work as we want. See the bottom most
// example at https://docs.rs/clap/2.32.0/clap/struct.Arg.html#method.overrides_with
.use_delimiter(false)
Expand Down
24 changes: 10 additions & 14 deletions src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,16 @@ impl OutputType {

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_env = env::var("PAGER").ok().map(|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;
pager
});

let pager_from_config = pager_from_config.map(|p| p.to_string());

Expand Down
24 changes: 24 additions & 0 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,17 @@ fn pager_basic() {
.stdout(predicate::eq("pager-output\n").normalize());
}

#[test]
fn pager_config() {
bat()
.arg("--pager=echo pager-output")
.arg("--paging=always")
.arg("test.txt")
.assert()
.success()
.stdout(predicate::eq("pager-output\n").normalize());
}

#[test]
fn pager_overwrite() {
bat()
Expand All @@ -393,6 +404,19 @@ fn pager_overwrite() {
.stdout(predicate::eq("pager-output\n").normalize());
}

#[test]
fn pager_overwrite_overwrite() {
bat()
.env("PAGER", "echo other-pager")
.env("BAT_PAGER", "echo another-pager")
.arg("--pager=echo pager-output")
.arg("--paging=always")
.arg("test.txt")
.assert()
.success()
.stdout(predicate::eq("pager-output\n").normalize());
}

#[test]
fn pager_disable() {
bat()
Expand Down

0 comments on commit 36cddbf

Please sign in to comment.